Skip to content

fix(router): Sync resolved model's context window onto the Auto session - #1137

Merged
iamhatesz merged 2 commits into
masterfrom
fix/auto-context-window-sync
Sep 4, 2026
Merged

fix(router): Sync resolved model's context window onto the Auto session#1137
iamhatesz merged 2 commits into
masterfrom
fix/auto-context-window-sync

Conversation

@iamhatesz

Copy link
Copy Markdown
Contributor

Linked issue

Closes #0

What does this PR do?

After the router resolves a concrete model for a session (or subagent delegation), advertise the routed model's contextWindow and maxTokens on the active kimchi-dev/auto model. This makes Pi's context-usage accounting (getContextUsage) reflect the real window (e.g. deepseek ~1M) instead of the fixed pre-routing <=128k floor, so a fresh deepseek turn shows ~4% used rather than ~34%.

Checklist

  • I have read CONTRIBUTING.md and agree to the CLA
  • This PR links to an open issue above
  • Tests pass locally (pnpm run test)
  • Lint passes (pnpm run check)
  • Documentation updated if behavior changed

After the router resolves a concrete model for a session (or subagent
delegation), advertise the routed model's contextWindow and maxTokens on the
active kimchi-dev/auto model. This makes Pi's context-usage accounting
(getContextUsage) reflect the real window (e.g. deepseek ~1M) instead of the
fixed pre-routing <=128k floor, so a fresh deepseek turn shows ~4% used rather
than ~34%.

- autoModelForTarget now copies contextWindow/maxTokens from the resolved target
- hasTargetCapabilities compares all four fields to skip redundant setModel
- syncAutoReasoningCapabilities -> syncAutoCapabilities (renamed for accuracy)
- regression tests for the fresh-routing and session-restore sync paths

Co-Authored-By: Kimchi <noreply@kimchi.dev>
@readme-ai-writer

readme-ai-writer Bot commented Sep 4, 2026

Copy link
Copy Markdown

📚 No documentation changes were needed for this PR.

@kimchi-review

kimchi-review Bot commented Sep 4, 2026

Copy link
Copy Markdown

Kimchi Code Review

Property Value
Commit 75834bb
Author @iamhatesz
Files changed 2
Review status Completed
Comments 1 (1 warning)
Duration 87s

Summary

📊 Review Score: 78/100 (overall code quality — 0 lowest, 100 highest)
⏱️ Estimated effort to review: 2/5 (1 = trivial, 5 = very complex)

🧪 Tests: yes — Two new tests cover context window and max token syncing for routed and restored sessions. However, the restore test's expectation of a single setModel call appears inconsistent with the visible handler logic, which may call setModel twice.

📝 Found 1 issue(s). See inline comments for details.

What to expect

Kimchi will analyze the changes in this pull request and post:

  • A summary of the overall changes
  • Inline comments on specific lines with findings categorized by issue type

The review typically completes within a few minutes. This comment will be updated once the review is ready.

Interact with Kimchi
  • @getkimchi review — re-trigger a full review on the latest commit
  • @getkimchi summary — regenerate the PR summary
  • @getkimchi ignore — skip this PR (no review will be posted)
  • Reply to any inline comment to ask follow-up questions or request clarification
Configuration

Reviews are configured by your organization admin.
Review instructions, excluded directories, and severity thresholds can be adjusted per repository in the Kimchi dashboard.


Powered by Kimchi — AI-powered code review by CAST AI

@kimchi-review kimchi-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📊 Review Score: 78/100 (overall code quality — 0 lowest, 100 highest)
⏱️ Estimated effort to review: 2/5 (1 = trivial, 5 = very complex)

🧪 Tests: yes — Two new tests cover context window and max token syncing for routed and restored sessions. However, the restore test's expectation of a single setModel call appears inconsistent with the visible handler logic, which may call setModel twice.

📝 Found 1 issue(s). See inline comments for details.

Comment thread src/extensions/router/index.ts
Comment thread src/extensions/router/index.ts
…target

Addresses PR #1137 review comment: the set_model tool and the model_select
handler validated the context and vision guards against the raw
target/event.model descriptor, which for Auto reports Auto's 128K catalog
floor instead of the resolved concrete model. After Auto routed to a 1M
target, a ~200K conversation would fit the real window but be rejected (and
model_select would revert the switch) purely because Auto's descriptor stuck
at 128K.

- set_model: resolve the effective target (resolveEffectiveModel) for the
  context and vision guards; keep pi.setModel(target) so Auto stays selected.
- model_select: resolve the effective target for the context and vision guards
  and the overflow-message numbers; event.model still drives revert/compact/
  new-session identity.
- When Auto is unresolved, resolveEffectiveModel falls back to Auto's
  descriptor, so switching into an unresolved Auto with an oversized context is
  still conservatively rejected.

Regression test: ~200K context, resolved 1M Auto target, Auto descriptor at
128K -> the Auto switch is not reverted.

Co-Authored-By: Kimchi <noreply@kimchi.dev>
@iamhatesz
iamhatesz added this pull request to the merge queue Sep 4, 2026
Merged via the queue into master with commit 1689c6e Sep 4, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants