-
Notifications
You must be signed in to change notification settings - Fork 47
🤖 fix: gate context switch warnings to explicit model switches #2117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
--- _Generated with `mux` • Model: `openai:gpt-5.2-codex` • Thinking: `xhigh` • Cost: `3.52`_ <!-- mux-attribution: model=openai:gpt-5.2-codex thinking=xhigh costs=13.52 -->
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2823fabf3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Addressed the 1M-toggle warning regression by allowing same-model checks only when the effective limit changes, and added coverage for the toggle behavior. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 304b386f18
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Updated the config/policy refresh path to keep same-model warnings (like 1M toggles) by passing allowSameModel when recomputing existing warnings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f29dadc80
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Cleared stale warnings on zero-token user model switches and added hook coverage. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f655205c4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71d2218e00
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Addressed deferred switch cleanup when tokens are 0, made the agent warning test use the explicit model-change path, and restored the ContextSwitchWarning story via an explicit model switch in play. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28ca1697e4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Normalized gateway model strings for explicit switch tracking and added a regression test for gateway-prefixed pending models. |
|
Codex Review: Didn't find any major issues. You're on a roll. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review Latest fixes are up: guarded context-switch warning refresh to avoid policy churn loops, stabilized policy refresh updates, and added regression coverage. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65bd41e24e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Addressed flaking warning test and policy reason staleness: refresh guard to avoid re-warn on background model changes, PolicyContext signature includes status.reason, and added PolicyContext regression tests (plus flake rerun). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a9e02e8721
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review
|
|
Codex Review: Didn't find any major issues. Nice work! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Follow the same pattern as ProjectContext.test.tsx, WorkspaceContext.test.tsx, and ThinkingContext.test.tsx: mock @/browser/contexts/API at the top of the file before importing PolicyProvider. This fixes CI failures caused by bun module mocks leaking between test files. When another test file mocks the API context first, our tests would get the leaked mock (with api: null) instead of our intended stub, causing waitFor timeouts. Repro that previously failed: bun test src/browser/components/AppLoader.auth.test.tsx src/browser/contexts/PolicyContext.test.tsx
- Normalize gateway vs canonical model comparisons in warning logic - Move workspace reset dispatch into useLayoutEffect - Add dismiss button aria-label - Remove dead onModelChange from SlashCommandContext --- _Generated with `mux` • Model: `openai:gpt-5.2` • Thinking: `xhigh` • Cost: `$73.99`_ <!-- mux-attribution: model=openai:gpt-5.2 thinking=xhigh costs=73.99 -->
|
@codex review Pushed follow-ups:
|
Resolved conflicts by keeping SlashCommandContext free of unused callbacks. --- _Generated with `mux` • Model: `openai:gpt-5.2` • Thinking: `xhigh` • Cost: `$75.39`_ <!-- mux-attribution: model=openai:gpt-5.2 thinking=xhigh costs=75.39 -->
|
@codex review Merged origin/main into the branch and resolved the two conflicts in ChatInput/chatCommands (kept SlashCommandContext free of unused callbacks). Static-check is green. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7bf1af7641
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Skip recording explicit model-change entries when the effective model doesn't change, and clear pending entries once the workspace model diverges to avoid false warnings after sync-driven away→back transitions. Adds targeted unit tests for no-op, divergence, and rapid-switch scenarios. --- _Generated with `mux` • Model: `openai:gpt-5.2` • Thinking: `xhigh` • Cost: `$77.69`_ <!-- mux-attribution: model=openai:gpt-5.2 thinking=xhigh costs=77.69 -->
|
@codex review I pushed a follow-up commit that prevents stale explicit model-change entries from lingering and being consumed after sync-driven away→back transitions. Also adds targeted unit tests for no-op, divergence, and rapid-switch scenarios. |
This comment was marked as resolved.
This comment was marked as resolved.
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
user|agent|sync) with stale-entry protection (no-op guard + divergence clearing).Background
The context switch warning banner was showing up when the workspace model changed due to background state reconciliation (workspace entry seeding, localStorage sync, default/policy refreshes). This created noisy banners unrelated to user intent.
Design / Approach
Origin-gating (explicit vs sync)
src/browser/utils/modelChange.ts.user: user explicitly selected a modelagent: agent mode switch explicitly changed the default modelsync: background reconciliation (mount, workspace entry, localStorage sync, default refresh)Stale-entry protection (prevents false positives)
To avoid "old" explicit-change entries triggering warnings later:
What changed (review map)
Core behavior
src/browser/utils/modelChange.tssetWorkspaceModelWithOrigin(workspaceId, model, origin)(intended entry point)consumeWorkspaceModelChange(workspaceId, model)(one-shot origin consumption)readPersistedString(getModelKey(workspaceId))) to implement no-op + divergence handling.src/browser/hooks/useContextSwitchWarning.tspendingModelchanges and only queues warnings foruser|agent.src/browser/contexts/WorkspaceContext.tsx: seeds model from backend withorigin: "sync"(no banner on entry).src/browser/components/WorkspaceModeAISync.tsx: tags asagentonly for real agentId flips; otherwisesync.src/browser/components/ChatInput/index.tsx+useCreationWorkspace.ts: user selections areuser; creation/default sync issync; explicit agent switches areagent.src/browser/utils/compaction/contextSwitchCheck.tsnormalizeGatewayModel()to avoid gateway/canonical mismatches.allowSameModeloption to allow same-model warnings only when callers know the effective limit changed (used for 1M toggles).Supporting changes included in this PR
src/browser/components/ContextSwitchWarning.tsxdata-testid="context-switch-warning"for tests and small a11y tweaks (aria-labeldismiss, iconaria-hidden).src/browser/contexts/PolicyContext.tsx+PolicyContext.test.tsxsrc/browser/utils/chatCommands.tsonModelChangefrom /model command context (single source of truth becomessetPreferredModel/ ChatInput plumbing).src/browser/stories/App.chat.stories.tsxsetWorkspaceModelWithOrigin(..., "user")so the warning matches new "explicit only" gating.Validation
bun test src/browser/hooks/useContextSwitchWarning.test.tsbun test src/browser/utils/modelChange.test.tsbun test src/browser/components/WorkspaceModeAISync.test.tsxbun test src/browser/utils/compaction/contextSwitchCheck.test.tsmake static-checkNotes / Risks
setWorkspaceModelWithOrigin(..., "user"|"agent").