diff --git a/ARCHITECTURE-MAP.md b/ARCHITECTURE-MAP.md index 8730275f0..447a56bdd 100644 --- a/ARCHITECTURE-MAP.md +++ b/ARCHITECTURE-MAP.md @@ -3,7 +3,7 @@ # Architecture Map **Topic:** architecture / provider / streaming / routing / usage / models / thinking -**Updated:** 2026-08-15 +**Updated:** 2026-09-23 **Tags:** #architecture #provider #streaming #routing #usage #models #thinking #byok #autocomplete **Supersedes:** - **Related:** `docs/architecture/01-20260514-open-code-provider-architecture.md` · `docs/architecture/02-20260809-provider-adapter-architecture.md` @@ -40,7 +40,7 @@ Total `src/` ≈ **16,310 lines** across ~109 files (excl. tests). Grouped by do | **Core (registry/routing)** | `src/core/` — `routing.ts` (441), `registry.ts` (142), `transport.ts` (70) | ~653 | Data-driven model registry (`MODEL_REGISTRY`), transport resolution (`resolveModelRouting`), Responses/Google SSE normalization, shared `StreamRequestOptions` contract | **Pure** — no `vscode` import, no side effects | | **Models (metadata)** | `src/models/` — `metadata.ts` (523), `modelTables.ts` (141), `metadataFetcher.ts` (102), `modelLimits.ts` (52), `modelCapabilities.ts` (16), `modelNames.ts` (29), `pricing.ts` (88) | ~951 | models.dev live metadata + bundled fallback snapshot (static data tables in `modelTables.ts`), limit/capability resolution, pricing | Live fetch may fail → bundled snapshot MUST exist | | **Usage** | `src/usage/` — `tracker.ts` (585, thin class shell), `trackerTypes.ts` (96), `trackerWindows.ts` (140), `trackerSummary.ts` (238), `dashboard.ts` (19 barrel) + `dashboard/` (`webview.ts`, `webviewData.ts`, `webviewHtml.ts`, `state.ts`, `statusBar.ts`, `targetEditor.ts`, `tooltip.ts` ≈ 1,405), `history.ts` (378), `usage.ts` (146), `goUsageSync.ts` (128), `formatting.ts` (129), `usageProfile.ts` (74), `pricing.ts` (62) | ~3,571 | Go usage tracker (types/windows/summary split out of the old god file), per-profile tracking, CLI SQLite history reader, server-usage sync, status bar + usage webview + quick-pick (webview split into state/status/webview modules) | Server meters authoritative for Session/Weekly/Monthly; device-local for Today/Yesterday | -| **Thinking** | `src/thinking/` — `provider.ts` (78), `base.ts` (75), `resolve.ts` (66), `deepseek.ts` (53), `glm.ts` (53), `kimi.ts` (81), `minimax.ts` (54), `mimo.ts` (72), `openai.ts` (57), `qwen.ts` (103), `fallback.ts` (39), `schema.ts` (106), `payload.ts` (26), `types.ts` (51) | ~914 | Per-family thinking strategy classes + config resolution (single authority = per-model config) | **Pure** — no `vscode` import; family from registry | +| **Thinking** | `src/thinking/` — `provider.ts` (78), `base.ts` (75), `resolve.ts` (94), `deepseek.ts` (53), `glm.ts` (53), `kimi.ts` (81), `minimax.ts` (54), `mimo.ts` (72), `openai.ts` (57), `qwen.ts` (103), `fallback.ts` (39), `schema.ts` (106), `payload.ts` (26), `types.ts` (51) | ~942 | Per-family thinking strategy classes + config resolution (per-model config wins over workspace — but schema-default echoes are stripped first, `stripSchemaDefaultEcho` in `resolve.ts`, issue #226) | **Pure** — no `vscode` import; family from registry | | **Request builders** | `src/request/` — `anthropic.ts` (233), `google.ts` (176), `types.ts` (117), `schema.ts` (94), `openai.ts` (95), `headers.ts` (110), `builders.ts` (17), `shared.ts` (11) | ~853 | Per-endpoint request-body builders + shared header builders (`x-opencode-session` / `x-opencode-request`) | `builders.ts` is the public barrel | | **Commands** | `src/commands/` — `agentsWindow.ts` (130), `diagnostics.ts` (41), `providers.ts` (38), `thinkingPicker.ts` (31) | ~240 | Command handlers: diagnostics, agents-window BYOK bridge, provider enable/disable, thinking picker | Thin — delegates to provider/usage modules | | **Autocomplete** | `src/autocomplete/` — `index.ts` (157), `engine.ts` (143), `provider.ts` (127), `context.ts` (89), `usage.ts` (88), `throttle.ts` (79), `prompt.ts` (60), `types.ts` (32) | ~775 | Inline code suggestions (opt-in) — FIM emulation over chat-completions, debounce/throttle, usage counters | Separate subsystem; not wired into Go cost tracker yet | @@ -282,7 +282,7 @@ flowchart LR flowchart TD A[resolve apiKey: BYOK config → per-model cache → SecretStorage cold-start] --> B[convertMessage per message
tool calls / tool results / images / thinking echo] B --> C[flatten messages + source index] - C --> D[resolve thinking config
per-model config = single authority] + C --> D[resolve thinking config
per-model config wins, schema-default echo stripped
issue #226] D --> E[vision proxy: text-only model + images → relay to vision model] E --> F[normalizeMessages + trimOldImagesFromHistoryInPlace
keep MAX_HISTORY_IMAGES_KEPT=2] F --> G[estimatePromptTokenCount → modelLimits] @@ -411,23 +411,29 @@ Reuse these before writing new logic (all under `src/` root unless noted): ## 8. Tests, Tooling & CI -### Unit tests (`src/test/` — 22 files, 312 cases) - -Pure/domain modules get co-located tests. **Harness:** Node's built-in `node --test` runner via `scripts/run-unit-tests.ts`, which collects the compiled `out/test/*.test.js` files; tests import from compiled `out/` modules with explicit `.js` extensions (Node16/ESM-style) and use `node:test` + `node:assert/strict`. Tests never need a live model — they cover the deterministic parts (message conversion, chunk parsing, token estimation, routing). Per-file case counts (312 total): - -| Test file | Cases | | Test file | Cases | -| ----------------------------- | ----- | --- | --------------------------- | ----- | -| `thinking.test.ts` | 52 | | `metadata.test.ts` | 23 | -| `goUsageTracker.test.ts` | 51 | | `autocomplete.test.ts` | 23 | -| `utils.test.ts` | 20 | | `retry.test.ts` | 17 | -| `visionProxy.test.ts` | 17 | | `registry.test.ts` | 14 | -| `toolCallAccumulator.test.ts` | 14 | | `config.test.ts` | 13 | -| `goUsageSync.test.ts` | 9 | | `autocompleteUsage.test.ts` | 9 | -| `usageProfile.test.ts` | 9 | | `reasoningHistory.test.ts` | 9 | -| `responsesRequest.test.ts` | 8 | | `modelLimits.test.ts` | 5 | -| `imageNormalizer.test.ts` | 5 | | `apiKeyResolution.test.ts` | 3 | -| `chatParts.test.ts` | 3 | | `modelNames.test.ts` | 3 | -| `providerEnablement.test.ts` | 3 | | `tokenEstimate.test.ts` | 2 | +### Unit tests (`src/test/` — 34 files, 466 cases) + +Pure/domain modules get co-located tests. **Harness:** Node's built-in `node --test` runner via `scripts/run-unit-tests.ts`, which collects the compiled `out/test/*.test.js` files; tests import from compiled `out/` modules with explicit `.js` extensions (Node16/ESM-style) and use `node:test` + `node:assert/strict`. Tests never need a live model — they cover the deterministic parts (message conversion, chunk parsing, token estimation, routing). Per-file case counts (466 total, audited 2026-09-23): + +| Test file | Cases | | Test file | Cases | +| --------------------------------- | ----- | --- | ------------------------------- | ----- | +| `thinking.test.ts` | 71 | | `goUsageTrackerWindows.test.ts` | 17 | +| `retry.test.ts` | 35 | | `responsesRequest.test.ts` | 16 | +| `goUsageTracker.test.ts` | 35 | | `registry.test.ts` | 16 | +| `metadata.test.ts` | 28 | | `routing.test.ts` | 15 | +| `utils.test.ts` | 22 | | `messages.test.ts` | 15 | +| `autocomplete.test.ts` | 21 | | `extractors.test.ts` | 15 | +| `toolCallAccumulator.test.ts` | 19 | | `config.test.ts` | 15 | +| `visionProxy.test.ts` | 17 | | `openai-request.test.ts` | 12 | +| `goUsageSync.test.ts` | 10 | | `usageProfile.test.ts` | 9 | +| `sse.test.ts` | 9 | | `reasoningHistory.test.ts` | 9 | +| `autocompleteUsage.test.ts` | 9 | | `deprecatedFilter.test.ts` | 8 | +| `schema.test.ts` | 7 | | `modelLimits.test.ts` | 5 | +| `imageNormalizer.test.ts` | 5 | | `session-header.test.ts` | 4 | +| `issue216-217-regression.test.ts` | 4 | | `providerEnablement.test.ts` | 3 | +| `modelNames.test.ts` | 3 | | `engine.test.ts` | 3 | +| `chatParts.test.ts` | 3 | | `apiKeyResolution.test.ts` | 3 | +| `tokenEstimate.test.ts` | 2 | | `agentProvider.test.ts` | 1 | ### Scripts (`scripts/`) diff --git a/CHANGELOG.md b/CHANGELOG.md index 36dbb5af1..1b2be9cd3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ All notable changes to the **OpenCode Go BYOK Provider** extension are documente - **`[Provider]` History-trim cuts are cache-stable — a session at the context ceiling keeps its prefix-cache hits (~99% instead of ~11%).** When the trimmed history landed just under the input budget, the minimal-fit trim moved the cut point on nearly every following turn — and the provider's prefix cache only reuses the bytes before the first changed message, so each moved cut re-billed the whole conversation at full input price (measured on a 614K-token session: hit rate collapsed from ~99% to ~11.4%, with only system + tools — 69,888 tokens — still cached; 207 trims fired across a ~12-hour span). Two changes now keep the cut still, with the same unit granularity and tool-group safety rules: a **low-water mark** (`budget − headroom`, new `HISTORY_TRIM_HEADROOM_*` constants: 3% of the budget, clamped to 8,192–32,768 tokens, never more than 10% of a small budget) and **cut-step alignment** to the next `HISTORY_TRIM_CUT_STEP_TOKENS` (32,768, capped at 10% of the budget) boundary of dropped payload. The step is what makes it robust: the crossing alone still hugs the mark within one unit, so sessions with ~2.7K-token units against ~1K of growth per request kept moving the cut every 1-3 requests (the live evening run: 12.4% misses for hours, hit rate down to 37-68%), while a smaller-unit morning session only looked stable by luck (large tool-result units). Simulation with production parameters: cut moves fall from 84/300 to 12/300 (evening regime) and 244/300 to 14/300 (smaller-unit regime). Four tests pin the low-water landing, the no-re-trim behavior, the re-supplied-history shape (constant cut → nested payload prefixes), and the cut-step stability; the first live ceiling crossing confirmed 15 trims with every landing ≤ 595,534 tokens (budget 613,952) and high-context misses down from 100/243 pre-fix to 3/55. Documented in `docs/issues/101-20260920-history-trim-cache-hysteresis.md`. +- **`[Thinking]` Global `opencodego.thinking.*` settings now take effect for models without a per-model pick (#226).** VS Code merges our picker schema defaults into the per-model `modelConfiguration` on every request, so any reasoning-capable model the user never configured arrived with `reasoningEffort: "off"` attached — and the resolver treated any delivered `modelConfiguration` as the single authority, letting the echoed `"off"` beat the global setting every time (the #214 symptom; diagnosis by @nickchomey). `resolveThinkingConfig` now strips override keys equal to the family's picker schema default before applying them — lossless, because VS Code itself strips default-equal values when persisting user picks, so such a value can never be a genuine user choice. Non-default per-model picks still win; the Agents-window default path is untouched (removing the schema default instead would have made host-side fallbacks pick `medium`/`high`). Documented in `docs/issues/100-20260923-issue226-thinking-default-echo.md`. + - **`[Models]` Bundled offline fallback data synced to models.dev (2026-09-08 snapshot).** Three drift classes fixed in the offline-only tables: (1) stale context limits — `kimi-k2.7-code` 256K → 262,144, `minimax-m3` / `qwen3.6-plus` / `qwen3.7-plus` → 1M (models.dev now differentiates Go vs Zen and the bundled Go values matched the Zen ones; the deliberately-capped `deepseek-v4-flash` 131,072 output and Go `minimax-m2.5` 65,536 output are kept); (2) wrong static pricing in the usage tracker — `mimo-v2.5-pro` and `deepseek-v4-pro` were 2–4x over-reported, `mimo-v2-omni` / `deepseek-v4-flash` under-reported, and `kimi-k3` ($3/$15) fell back to a 6x-under-estimating generic price; the table was rewritten from the current registry and 15 new models added; (3) the Go `fallbackModels` catalog still listed the retired `hy3-preview` and pre-June models — refreshed to the 27-model curated active set. Stale tests referencing `hy3-preview` updated to `hy3`. Documented in `docs/issues/100-20260908-bundled-model-data-sync.md`. ## [0.7.5] — 2026-09-08 diff --git a/docs/devlog.md b/docs/devlog.md index a70de9fc2..82526118e 100644 --- a/docs/devlog.md +++ b/docs/devlog.md @@ -1,6 +1,23 @@ # 🧠 OPENCODE COPILOT CHAT DEVLOG -**Branch:** `chore/models-dev-data-sync` | **Updated:** 2026-09-08 Asia/Jakarta | **Current Phase:** bundled model data sync — limits, pricing, fallback catalog synced to models.dev (2026-09-08); full lint gate pass, pending PR. +**Branch:** `chore/models-dev-data-sync` (work on `main`) | **Updated:** 2026-09-23 Asia/Jakarta | **Current Phase:** issue #226 fix — global `opencodego.thinking.*` now wins over the picker schema-default echo; verified end-to-end, pending commit. + +--- + +## ✅ Issue #226 — Schema-Default Echo Fix — 2026-09-23 + +**Scope:** follow-up to #214/#93. VS Code merges our picker schema defaults into `modelConfiguration` on every request, so untouched reasoning-capable models arrived with `reasoningEffort: "off"` and the resolver let that echo beat the global `opencodego.thinking.*` settings. + +| Decision | Rationale | +| ------------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Option B — resolver-side strip | `stripSchemaDefaultEcho()` in `resolve.ts` drops override keys equal to the family's picker schema default before applying them. Lossless: VS Code itself strips default-equal values from persisted user picks, so a default-equal delivered value can never be a genuine user choice. | +| Option A rejected | Removing `default: "off"` from the schema would make the Agents-window host fall back to `medium`/`high` (`resolveDefaultReasoningEffort`) — silent Go credit burn — and the picker would lose its initial Off display. Verified against VS Code source. | + +**Verification:** `npm run compile` clean, full lint gate pass, 466/466 unit tests (5 new #226 regression tests; one old test that encoded the buggy priority updated), e2e simulation `tmp/e2e-issue226-thinking-echo.mjs` 4/4, and a real-model run (Go, `mimo-v2.6-flash` via Copilot Chat): echo delivered → `thinkingSource=workspace` → payload carries `reasoning_effort:"high"` with HTTP 200. Per-model picks still win (`thinkingSource=modelConfiguration`). + +**Notes:** picker still paints Off when only the global default is set — by design (picker reflects per-model config only); recorded as a UX note in the issue doc, not a bug. + +Docs: `docs/issues/100-20260923-issue226-thinking-default-echo.md`, `docs/issues/93` updated (second half closed), feature doc 02 updated, CHANGELOG `[Unreleased]`. --- diff --git a/docs/features/02-20260517-per-model-thinking-controls.md b/docs/features/02-20260517-per-model-thinking-controls.md index a2e3dcc36..688e48e19 100644 --- a/docs/features/02-20260517-per-model-thinking-controls.md +++ b/docs/features/02-20260517-per-model-thinking-controls.md @@ -3,7 +3,7 @@ # Per-Model Thinking Controls **Topic:** provider / models / thinking / vscode / copilot-chat -**Updated:** 2026-08-21 +**Updated:** 2026-09-23 **Tags:** #provider #models #thinking #vscode #copilot-chat #reasoning #byok **Supersedes:** — **Related:** PR [#155](https://github.com/ltmoerdani/opencode-copilot-chat/pull/155) (per-provider strategy refactor) · feature doc [`17-20260814-data-driven-model-registry.md`](17-20260814-data-driven-model-registry.md) @@ -89,6 +89,8 @@ options.modelConfiguration; The extension reads that object and overlays only the selected model family's Thinking values onto the persistent defaults. This keeps changes scoped to the current model instead of accidentally changing every model family. +> **Schema-default echo is stripped (#226).** VS Code merges our picker schema defaults into the resolved `modelConfiguration` on every request (`resolveModelConfiguration` merges defaults in every branch) and strips default-equal values from persisted user picks — so a delivered value equal to the family's schema default (e.g. `reasoningEffort: "off"`) can never be a genuine user choice; it is the baseline being echoed back. `resolveThinkingConfig` (`src/thinking/resolve.ts`, `stripSchemaDefaultEcho`) drops those keys before applying the override, letting the global `opencodego.thinking.*` defaults take effect for models the user never configured per-model. A default-equal value therefore means "fall through to workspace", and only non-default per-model picks win. Full write-up: `docs/issues/100-20260923-issue226-thinking-default-echo.md`. + ### Command Fallback The command: diff --git a/docs/issues/100-20260923-issue226-thinking-default-echo.md b/docs/issues/100-20260923-issue226-thinking-default-echo.md new file mode 100644 index 000000000..693f96053 --- /dev/null +++ b/docs/issues/100-20260923-issue226-thinking-default-echo.md @@ -0,0 +1,76 @@ +# Issue #226 — Global `opencodego.thinking.*` Ignored: Picker Schema Default "off" Echoed Back as Per-Model Override + +**Status:** ✅ Solved — implemented + verified end-to-end +**Topic:** thinking / picker-schema / model-configuration +**Updated:** 2026-09-23 +**Tags:** #thinking #schema #modelConfiguration #agent-host +**GitHub Issue:** [ltmoerdani/opencode-copilot-chat#226](https://github.com/ltmoerdani/opencode-copilot-chat/issues/226) +**Related:** issue doc [93 — #214 thinking settings scope](93-20260903-issue214-thinking-settings-scope.md), feature doc [02 — per-model thinking controls](../features/02-20260517-per-model-thinking-controls.md) + +--- + +## Problem + +Follow-up to #214: v0.7.4 fixed the settings-scope half, but global `opencodego.thinking.*` values still never take effect while per-model picks work. Diagnosis by @nickchomey (`chatLanguageModels.json` evidence) + verification against VS Code source. + +## Root Cause (verified against VS Code source) + +Two halves, both confirmed: + +1. **Our picker schema declares `default: "off"`** for `reasoningEffort` (`src/thinking/schema.ts`: `effortProperty`, `schemaFromReasoningOptions`, `genericReasoningSchema`, plus family schemas). +2. **VS Code merges schema defaults into the resolved per-model configuration on every request.** `resolveModelConfiguration` (`chatModelConfigurationLogic.ts`) merges defaults in _every_ branch, and `_resolveModelConfigurationWithDefaults` (`languageModels.ts`) does `{...defaults, ...userConfig}` — so "user never set anything" arrives as `{reasoningEffort: "off"}`. + +Our resolver (`resolveThinkingConfig`, `src/thinking/resolve.ts`) treated _any_ delivered `modelConfiguration` as the single authority — even a value equal to the baseline — so the echoed `"off"` outranked the global setting on every request. + +Key extra evidence: `LanguageModelsService.setModelConfiguration` **strips stored values equal to the schema default**. An explicit user pick of "Off" is therefore indistinguishable from "never touched" — both resolve to the default. Treating a default-equal delivered value as "no override" is consequently lossless. + +## Fix (Option B — resolver-side) + +`src/thinking/resolve.ts`: + +- `schemaDefaultsOf(schema)` — extracts per-key picker defaults (same shape as VS Code's `extractSchemaDefaults`). +- `stripSchemaDefaultEcho(override, defaults)` — drops override keys whose delivered value equals the family's picker schema default; per-key, so a genuine choice beside an echo still applies. Returns `undefined` when nothing survives → workspace global wins. +- `resolveThinkingConfig` now applies only the _stripped_ override and passes it (not the raw `modelConfiguration`) to `provider.applyOverride`. + +**Why not Option A (remove `default: "off"` from the schema)?** Verified against VS Code source: the Agents window host reads `reasoningEffortSchema.default` as `defaultReasoningEffort` (`agentHostByokLmHandler.ts`); when absent, `resolveDefaultReasoningEffort` falls back to `'medium'`/`'high'` — removing our default would make Agents-window models think at medium/high by default (silent Go credit burn). The picker would also lose its initial Off display (`getModelConfigProperty` uses `currentConfig[key] ?? schema.default`). + +## Files Changed + +| File | Change | +| ------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `src/thinking/resolve.ts` | `schemaDefaultsOf` + `stripSchemaDefaultEcho`, resolver strips default-echo before applying override | +| `src/thinking.ts` | re-export `stripSchemaDefaultEcho` | +| `src/test/thinking.test.ts` | updated the test that encoded the buggy behavior; added #226 regression tests (echo ignored, non-default global survives, per-key stripping, non-"off" defaults like kimi-k2.7-code `"on"`) | +| `tmp/e2e-issue226-thinking-echo.mjs` | e2e simulation of the request chain (`resolveThinkingConfig → buildPayload`) | + +## Verification + +- `npm run compile` clean; `npm run lint` (full 7-check gate) pass; 466/466 unit tests pass. +- E2E simulation (`tmp/e2e-issue226-thinking-echo.mjs`) — 4/4 pass: + 1. global `mimo=high` survives the echoed default → payload carries `reasoning_effort: "high"` + 2. per-model picker choice (glm `high`) still applies + 3. untouched model stays off with empty payload (no behavior change) + 4. qwen: echo dropped, real `thinkingBudget` choice kept (budget correctly not sent while thinking off) +- ⚠️ Manual Copilot Chat test with a real Go key still pending (set global `opencodego.thinking.mimo: "high"` → new chat → confirm reasoning appears). + +### Real-model E2E (2026-09-23, Go + `mimo-v2.6-flash` via Copilot Chat) — PASS + +| Scenario | Log evidence | Result | +| ----------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------- | +| Per-model picker pick `High` | `modelConfiguration={"reasoningEffort":"high"}` `thinkingSource=modelConfiguration` payload carries `reasoning_effort:"high"` | ✅ override still applies (non-default → kept) | +| No per-model setting, global `mimo: off` | `modelConfiguration={"reasoningEffort":"off"}` (echo) `thinkingSource=workspace` — echo ignored, no `reasoning_effort` in payload | ✅ fix works (pre-fix: `thinkingSource=modelConfiguration`) | +| No per-model setting, global `mimo: high` | `modelConfiguration={"reasoningEffort":"off"}` (echo) `thinkingSource=workspace` `thinking={..."mimo":"high"...}` `thinkingPayload={..."reasoning_effort":"high","budget_tokens":32768}` `hasReasoningEffort=true`, HTTP 200 | ✅ the #214 symptom is gone end-to-end | + +## Known UX Note (not part of #226) + +The model picker's Thinking submenu keeps displaying **Off** even when a global `opencodego.thinking.*` default is in effect. This is by design of the two-layer architecture: the picker reflects only the _per-model_ `modelConfiguration` (VS Code does not surface our global settings in its UI), while the global setting fills in for models without an override — the request still carries the global value (`thinkingSource=workspace` in the log proves it). If this proves confusing, a separate enhancement (e.g. a picker label hinting at the effective global default) can be tracked independently. + +## Lessons Learned + +1. Picker schema defaults are not inert metadata: VS Code echoes them into `modelConfiguration` on every request and strips them from persisted user picks — so a default-equal delivered value can _never_ be a genuine user choice. +2. Removing a schema default changes host-side fallbacks (Agents window `resolveDefaultReasoningEffort`) — resolver-side filtering is the safe half of this fix. +3. Existing test "delivered modelConfiguration equal to workspace still reports modelConfiguration source" had encoded the buggy priority — tests can cement a bug as a contract; audit them when a spec-level assumption changes. + +--- + +Detected 2026-09-09 | Reported by @nickchomey | Fixed 2026-09-23 diff --git a/docs/issues/93-20260903-issue214-thinking-settings-scope.md b/docs/issues/93-20260903-issue214-thinking-settings-scope.md index 4e19649d1..1fdbf3144 100644 --- a/docs/issues/93-20260903-issue214-thinking-settings-scope.md +++ b/docs/issues/93-20260903-issue214-thinking-settings-scope.md @@ -1,11 +1,11 @@ # Issue #214 — Thinking Level Settings "don't do anything" → Scope-Robust Config Reading -**Status:** ✅ Solved (branch `fix/issues-204-214-batch`, commit `609b344`) — ⚠️ fix is evidence-based but root cause on insiders 1.137 not yet reproduced locally +**Status:** ✅ Solved — both halves closed: settings scope fixed here (v0.7.4, commit `609b344`); the remaining root cause (schema-default echo overriding the globals) fixed via [#226](https://github.com/ltmoerdani/opencode-copilot-chat/issues/226) — see [issue doc 100](100-20260923-issue226-thinking-default-echo.md), verified end-to-end 2026-09-23 **Topic:** thinking / configuration-scope / agent-host -**Updated:** 2026-09-03 +**Updated:** 2026-09-23 **Tags:** #thinking #settings #configuration #agent-host **GitHub Issue:** [ltmoerdani/opencode-copilot-chat#214](https://github.com/ltmoerdani/opencode-copilot-chat/issues/214) -**Related:** feature doc [02 — per-model thinking controls](../features/02-20260517-per-model-thinking-controls.md), issue doc [22 (thinking part bypass)](22-20260609-thinking-part-bypass.md) +**Related:** feature doc [02 — per-model thinking controls](../features/02-20260517-per-model-thinking-controls.md), issue doc [22 (thinking part bypass)](22-20260609-thinking-part-bypass.md), issue doc [100 — #226 schema-default echo](100-20260923-issue226-thinking-default-echo.md) --- @@ -42,7 +42,7 @@ An explicitly set workspace value wins, then an explicitly set user value, and o ## Verification - `npx tsc --noEmit` clean; 449/449 tests pass; staged-lint gate pass. -- ⚠️ **Still to reproduce on VS Code insiders 1.137** (set `opencodego.thinking.mimo: "high"` in User settings → new chat → confirm `reasoning_effort` reaches the payload). If the symptom persists there, the next suspect is host-supplied `modelConfiguration` overriding the workspace baseline (see `resolveThinkingConfig` priority). +- ✅ **RESOLVED 2026-09-23:** the symptom persisted on insiders 1.137 and the suspected second half was confirmed as the real root cause — VS Code echoes our picker schema default (`"off"`) into `modelConfiguration` on every request, which outranked the globals. Fixed and verified end-to-end via #226 (see [issue doc 100](100-20260923-issue226-thinking-default-echo.md)). This doc remains as the settings-scope half of the story. ## Lessons Learned diff --git a/src/test/thinking.test.ts b/src/test/thinking.test.ts index 0422204ba..57dc74003 100644 --- a/src/test/thinking.test.ts +++ b/src/test/thinking.test.ts @@ -4,6 +4,7 @@ import { bodyRequestsThinking, extractThinkingOverride, resolveThinkingConfig, + stripSchemaDefaultEcho, thinkingFamily, thinkingProviderFor, type ThinkingSettings, @@ -443,15 +444,47 @@ describe("resolveThinkingConfig — provenance & priority", () => { assert.equal(resolved.overrideApplied, false); }); - it("a delivered modelConfiguration equal to workspace still reports modelConfiguration source", () => { + it("a delivered schema-default echo is ignored so the workspace default wins (issue #226)", () => { const resolved = resolveThinkingConfig({ modelId: "deepseek-v4-pro", workspace: defaultSettings, modelConfiguration: { reasoningEffort: "off" }, }); assert.equal(resolved.settings.deepseek, "off"); + assert.equal(resolved.source, "workspace"); + assert.equal(resolved.overrideApplied, false); + }); + + it("a schema-default echo does not mask a non-default workspace setting (issue #226)", () => { + const resolved = resolveThinkingConfig({ + modelId: "deepseek-v4-pro", + workspace: { ...defaultSettings, deepseek: "high" }, + modelConfiguration: { reasoningEffort: "off" }, + }); + assert.equal(resolved.settings.deepseek, "high"); + assert.equal(resolved.source, "workspace"); + assert.equal(resolved.overrideApplied, false); + }); + + it("echo stripping is per-key: a real choice beside an echo still applies", () => { + const resolved = resolveThinkingConfig({ + modelId: "qwen3.6-plus", + workspace: defaultSettings, + modelConfiguration: { reasoningEffort: "off", thinkingBudget: "16384" }, + }); + assert.equal(resolved.settings.qwenBudget, "16384"); assert.equal(resolved.source, "modelConfiguration"); }); + + it("models whose schema default is not 'off' strip their own default too (kimi-k2.7-code)", () => { + const resolved = resolveThinkingConfig({ + modelId: "kimi-k2.7-code", + workspace: defaultSettings, + modelConfiguration: { reasoningEffort: "on" }, + }); + assert.equal(resolved.source, "workspace"); + assert.equal(resolved.overrideApplied, false); + }); }); describe("extractThinkingOverride", () => { @@ -464,6 +497,15 @@ describe("extractThinkingOverride", () => { assert.equal(extractThinkingOverride({}), undefined); assert.equal(extractThinkingOverride({ contextSize: 5 }), undefined); }); + + it("stripSchemaDefaultEcho drops keys equal to the declared default and keeps the rest", () => { + const defaults = { reasoningEffort: "off", thinkingBudget: "auto" }; + assert.equal(stripSchemaDefaultEcho(undefined, defaults), undefined); + assert.deepEqual(stripSchemaDefaultEcho({ reasoningEffort: "off" }, defaults), undefined); + assert.deepEqual(stripSchemaDefaultEcho({ reasoningEffort: "high" }, defaults), { reasoningEffort: "high" }); + // Keys without a declared default are kept as-is. + assert.deepEqual(stripSchemaDefaultEcho({ reasoningEffort: "on" }, {}), { reasoningEffort: "on" }); + }); }); describe("schema defaults stay aligned with THINKING_DEFAULTS", () => { diff --git a/src/thinking.ts b/src/thinking.ts index 4579aa5b3..0b9dda2f5 100644 --- a/src/thinking.ts +++ b/src/thinking.ts @@ -6,7 +6,7 @@ */ export { thinkingFamily, thinkingProviderFor } from "./thinking/provider"; export type { ThinkingProvider } from "./thinking/provider"; -export { resolveThinkingConfig, extractThinkingOverride } from "./thinking/resolve"; +export { resolveThinkingConfig, extractThinkingOverride, stripSchemaDefaultEcho } from "./thinking/resolve"; export type { ResolveThinkingConfigInput } from "./thinking/resolve"; export { schemaFromReasoningOptions, genericReasoningSchema } from "./thinking/schema"; export type { ThinkingSchema } from "./thinking/schema"; diff --git a/src/thinking/resolve.ts b/src/thinking/resolve.ts index ad725e3fc..85d7c2ef7 100644 --- a/src/thinking/resolve.ts +++ b/src/thinking/resolve.ts @@ -33,11 +33,21 @@ export function resolveThinkingConfig(input: ResolveThinkingConfigInput): Resolv let source: ThinkingSource = "workspace"; let overrideApplied = false; - // A delivered modelConfiguration always wins (even if its value equals the - // workspace baseline) — VS Code's per-model config is the single authority. - const liveOverride = extractThinkingOverride(input.modelConfiguration); + // A delivered modelConfiguration wins over the workspace baseline — but only + // when it carries a value the user actually chose. VS Code merges our picker + // schema defaults into the resolved per-model configuration on every request + // (`resolveModelConfiguration` in chatModelConfigurationLogic.ts merges + // defaults in every branch), and strips values equal to the schema default + // when persisting user picks — so a delivered value equal to our schema + // default can never be a genuine user choice; it is the picker baseline being + // echoed back (issue #226). Dropping it lets the global `opencodego.thinking.*` + // setting take effect for models the user never configured per-model. + const liveOverride = stripSchemaDefaultEcho( + extractThinkingOverride(input.modelConfiguration), + schemaDefaultsOf(provider.schema(input.metadata)), + ); if (liveOverride) { - const next = provider.applyOverride(settings, input.modelConfiguration ?? {}); + const next = provider.applyOverride(settings, { ...liveOverride }); overrideApplied = next !== settings; settings = next; source = "modelConfiguration"; @@ -49,6 +59,42 @@ export function resolveThinkingConfig(input: ResolveThinkingConfigInput): Resolv return { settings, source, overrideApplied }; } +/** + * Extract per-key defaults from a picker schema (same shape as VS Code's + * `extractSchemaDefaults`): properties with a declared `default` only. + */ +export function schemaDefaultsOf(schema: { properties: Record } | undefined): Record { + const defaults: Record = {}; + const properties = Object.entries(schema?.properties ?? {}) as Array<[string, { default?: unknown }]>; + for (const [key, prop] of properties) { + if (prop.default !== undefined) { + defaults[key] = prop.default; + } + } + return defaults; +} + +/** + * Drop override keys whose delivered value equals the family's picker schema + * default — those are VS Code echoes of the baseline, not user choices (see + * `resolveThinkingConfig`). Keys with no declared default are kept as-is. + * Returns undefined when nothing survives. + */ +export function stripSchemaDefaultEcho( + override: ThinkingOverride | undefined, + defaults: Record, +): ThinkingOverride | undefined { + if (!override) return undefined; + const kept: ThinkingOverride = {}; + for (const key of ["reasoningEffort", "thinkingMode", "thinkingBudget"] as const) { + const value = override[key]; + if (value === undefined) continue; + if (key in defaults && defaults[key] === value) continue; + kept[key] = value; + } + return Object.keys(kept).length ? kept : undefined; +} + /** * Extract the thinking-relevant keys from a `modelConfiguration` object. * Returns undefined when none of the known keys carry a string value.