fix(vscode-lm): reliable auto context condensing#710
Conversation
Port of simurg79/Roo-Code#11 into Zoo-Code.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdates the VS Code LM model catalog, adds a ChangesVS Code LM condense gate fix
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/vscode-lm-condense-fix.md:
- Around line 1-5: The changeset file `.changeset/vscode-lm-condense-fix.md` was
created but should not be included in this PR since changesets are managed by
maintainers outside the normal development workflow. Delete the entire
`.changeset/vscode-lm-condense-fix.md` file and allow maintainers to create the
proper changeset entry separately.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 011fdda6-5a92-4a6e-a80c-e1b4d20c91ab
📒 Files selected for processing (13)
.changeset/vscode-lm-condense-fix.mdpackages/types/src/__tests__/vscode-llm.spec.tspackages/types/src/providers/vscode-llm.tssrc/api/index.tssrc/api/providers/__tests__/vscode-lm.spec.tssrc/api/providers/vscode-lm.tssrc/core/context-management/__tests__/context-management.spec.tssrc/core/context-management/index.tssrc/core/task/Task.tswebview-ui/src/components/chat/TaskHeader.tsxwebview-ui/src/components/chat/__tests__/TaskHeader.spec.tsxwebview-ui/src/components/ui/hooks/__tests__/useSelectedModel.spec.tswebview-ui/src/components/ui/hooks/useSelectedModel.ts
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Add targeted tests for the previously-uncovered ported branches: the availableInputTokens<=0 fallback to 100% in willManageContext/manageContext, getCondenseContextWindow() guard fallbacks, and the vscode-lm UI family-miss window resolution. Raises patch coverage to satisfy the codecov/patch 80% gate.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/api/providers/__tests__/vscode-lm.spec.ts`:
- Around line 504-521: The test is mutating the static model row for the
selector family, but the mocked client currently uses a different family so
VsCodeLmHandler never consults that row. Update the test setup in the
VsCodeLmHandler/getCondenseContextWindow case so the mockLanguageModelChat
family matches "claude-opus-4.8", or avoid assigning client so the selector
family path is used; this ensures the zeroed maxInputTokens row is actually
exercised and the live-window fallback assertion is valid.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7cff98c0-cb87-4d62-9e6d-29e09bf5e9b0
📒 Files selected for processing (2)
src/api/providers/__tests__/vscode-lm.spec.tssrc/core/context-management/__tests__/context-management.spec.ts
…w guard test - Remove .changeset/vscode-lm-condense-fix.md (changesets are maintainer-managed per AGENTS.md; CodeRabbit flagged). - Fix getCondenseContextWindow() non-positive-guard test so the selector family (claude-opus-4.8) drives the lookup and the zeroed static row actually exercises the maxInputTokens > 0 guard before falling back.
edelauna
left a comment
There was a problem hiding this comment.
Thanks for addressing this, had some comments around the implementation.
| // contextWindow MUST equal maxInputTokens: that is the exact value the gate consumes via | ||
| // getModel().info.contextWindow = Math.max(0, client.maxInputTokens) in src/api/providers/vscode-lm.ts, | ||
| // so the UI bar and the condense gate share a single source of truth. |
There was a problem hiding this comment.
This comment says the gate consumes getModel().info.contextWindow, but Task.ts now calls getCondenseContextWindow?.() as the primary path (which returns the static table's maxInputTokens). getModel() is the fallback, not the primary. Worth updating the comment so it doesn't mislead future readers?
There was a problem hiding this comment.
Fixed in d128a5c — corrected the comment to state the condense gate's primary window comes from getCondenseContextWindow() (the static-table maxInputTokens), with getModel().info.contextWindow as the fallback.
| expect(result.current.provider).toBe("vscode-lm") | ||
| expect(result.current.id).toBe(`copilot/${family}`) | ||
| // The bar and the condense gate share one source of truth: contextWindow === maxInputTokens. | ||
| expect(result.current.info?.contextWindow).toBe(vscodeLlmModels[family].maxInputTokens) |
There was a problem hiding this comment.
This test uses vscodeLlmDefaultModelId (claude-sonnet-4.5) where contextWindow === maxInputTokens === 167790. If someone accidentally swapped listedModel.maxInputTokens for listedModel.contextWindow on line 324 of useSelectedModel.ts, this test would still pass. Adding one test with claude-opus-4.8 (the only row where contextWindow: 679560 ≠ maxInputTokens: 197897) would catch that mutation.
There was a problem hiding this comment.
Done in d128a5c — added a claude-opus-4.8 case (contextWindow 679560 != maxInputTokens 197897) asserting contextWindow === maxInputTokens and not the advertised window, so a field swap is now caught.
| const reservedForOutput = maxTokens && maxTokens > 0 ? maxTokens : 0 | ||
| const availableInputTokens = contextWindow - reservedForOutput | ||
| const contextPercent = availableInputTokens > 0 ? (100 * prevContextTokens) / availableInputTokens : 100 |
There was a problem hiding this comment.
This changes the contextPercent denominator from contextWindow to contextWindow - maxTokens for every provider where maxTokens > 0, not just vscode-lm. For example, Anthropic with a 200K window and maxTokens=8192 will see condense fire ~4% earlier than before. Was that intentional? If so, might be worth a note in the PR description since it's a behavioral change for all providers.
There was a problem hiding this comment.
Addressed in d128a5c — scoped the available-input-space denominator to vscode-lm only via an opt-in flag (useAvailableInputForContextPercent), derived in Task.ts from the getCondenseContextWindow seam (only vscode-lm implements it). All other providers now divide by the full context window again, so there's no behavioral change for Anthropic/etc. The maxTokens:-1 reserve guard remains global. Added tests proving both the default full-window path and the vscode-lm opt-in path.
…lm; address review Address review feedback from edelauna on Zoo-Code-Org#710: - Scope the available-input-space condense percent denominator to vscode-lm only (via the getCondenseContextWindow seam); all other providers keep dividing by the full context window. The maxTokens:-1 reserve guard remains global. - Correct the misleading useSelectedModel comment: the gate's primary window is getCondenseContextWindow() (static maxInputTokens), not getModel().info.contextWindow. - Strengthen the listed-family test with a claude-opus-4.8 case (contextWindow != maxInputTokens) to catch a field swap.
Simplify comments added in PR Zoo-Code-Org#710 to be brief and rationale-focused; no logic, assertions, or test values changed.
Related GitHub Issue
Closes #714
Description
Fixes unreliable automatic context condensing on the VS Code LM (
vscode-lm) provider. The provider reportsmaxTokens: -1(unlimited) and an inflated live context window, so the auto-condense gate computed usage against the wrong denominator and effectively never fired even when the UI context gauge showed the window as full.maxTokens: -1(unlimited) as the default output reserve in bothwillManageContextandmanageContextinstead of letting a negative value distort the window math.contextWindow - reservedForOutput), matching the UI gauge, with a safe fallback to the full window when the reserve is unknown/unlimited.getCondenseContextWindow()ApiHandlerseam; only the vscode-lm provider overrides it to drive the gate from the curated static-tablemaxInputTokens. Every other provider falls back tomodelInfo.contextWindow(behavior unchanged).claude-sonnet-4.5).maxTokensas zero reserve and resolves an unlisted family to the default-model window.Test Procedure
packages/types→vscode-llm.spec.ts: model catalog invariants.src→context-management.spec.ts: reserve guard + available-input denominator, including theavailableInputTokens <= 0 → 100%fallback and end-to-endmanageContextsummarization.src→vscode-lm.spec.ts:getCondenseContextWindow()resolution (static family, live fallback, non-positive guard).webview-ui→TaskHeader.spec.tsx,useSelectedModel.spec.ts: UI reserve/window guards.codecov/patchat 88.88% (≥ 80% target).Pre-Submission Checklist
Documentation Updates
Additional Notes
Port of simurg79/Roo-Code#11 into Zoo-Code. Applied by context (paths map 1:1); the upstream version bump was intentionally omitted.
Summary by CodeRabbit
Summary
New Features
Bug Fixes
Tests