fix(tui): use effective model window in context inspector#2521
Draft
cyq1017 wants to merge 1 commit into
Draft
Conversation
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the context_usage function in context_inspector.rs to determine the context window using the effective model for budget (app.effective_model_for_budget()) instead of the raw &app.model. It also adds a unit test to verify that the context inspector correctly resolves and displays the context window limit when the model is set to "auto". There are no review comments, and I have no feedback to provide.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
/contextestimates its max context window from the literal UI model string. In auto mode that string isauto, so the inspector falls back to the legacy DeepSeek 128k window even after routing has resolved the turn to a concrete V4 model.Change
Use the app effective model for the context-window lookup while keeping the displayed model label unchanged.
Refs #2487. This only addresses the
/contextwindow display slice from the stalled-turn investigation; it does not claim to fix stalled turn dispatch/liveness.Verification
cargo test -p codewhale-tui inspector_uses_effective_auto_model_context_window --all-features --locked -- --nocapturecargo test -p codewhale-tui context_inspector --all-features --locked -- --nocapturecargo fmt --all -- --checkcargo check -p codewhale-tui --all-features --lockedcargo clippy -p codewhale-tui --all-features --locked -- -D warningsgit diff --check origin/main..HEADGreptile Summary
This PR fixes the
/contextinspector displaying an incorrect (128K) context window when the app is inautomodel mode, by switching the window-size lookup fromapp.model(the literal"auto"string) toapp.effective_model_for_budget()(which resolves to the last concrete model the router used). The display label is unchanged.context_usageis correct:effective_model_for_budget()already handles theNone/"auto"fallback case by returningDEFAULT_TEXT_MODEL(deepseek-v4-pro).Model: auto) and the resolved window size (/1000000 tokens) after a concrete V4 routing.Confidence Score: 4/5
Safe to merge; the change is minimal and well-tested, and the only note is a pre-existing inconsistency in sibling commands that was intentionally left out of scope.
The one-line fix correctly delegates to
effective_model_for_budget(), the new test covers both the display-label invariant and the resolved window size, and the fallback path (DEFAULT_TEXT_MODEL→deepseek-v4-pro) is coherent. The only observation is thatcommands/status.rsandsidebar.rscarry the same old pattern, meaning those surfaces will still display 128K for auto mode — but the PR explicitly scopes itself to the/contextinspector.commands/status.rsandsidebar.rshave the same&app.modelpattern and would benefit from a follow-up pass.Important Files Changed
context_usagenow callseffective_model_for_budget()instead of readingapp.modeldirectly, correctly resolving auto-mode to the last dispatched concrete model. New test covers the happy path. Two sibling usages of&app.modelincommands/status.rsandsidebar.rsremain unfixed (pre-existing, out of scope per PR description).Sequence Diagram
sequenceDiagram participant User participant TUI as TUI /context participant Inspector as context_inspector participant App participant Models as models::context_window_for_model User->>TUI: types /context TUI->>Inspector: "build_context_inspector_text(&app)" Inspector->>App: app.model (display label only) Note over Inspector,App: "auto" string used for display Inspector->>App: app.effective_model_for_budget() App-->>Inspector: "deepseek-v4-pro" (resolved from last_effective_model) Inspector->>Models: context_window_for_model("deepseek-v4-pro") Models-->>Inspector: Some(1_000_000) Inspector-->>TUI: "Model: auto ... ~X/1000000 tokens" TUI-->>User: shows correct 1M windowReviews (1): Last reviewed commit: "fix(tui): use effective model window in ..." | Re-trigger Greptile