fix(ui): show selected agent default model#100719
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73b8927905
ℹ️ 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".
| { | ||
| key: "agent:ops:session-a", | ||
| kind: "direct", | ||
| label: "Operations", | ||
| updatedAt: Date.now(), | ||
| }, |
There was a problem hiding this comment.
Cover effective model fields in clear-default test
The mocked sessions.list row here leaves out model/modelProvider, but real Gateway rows include the effective model even when no override is selected (src/gateway/session-utils.ts:2046 and src/gateway/session-utils.ts:2211). After a browser reload following a clear, modelOverrides is empty, so resolveChatModelOverrideValue reads that effective row model and the picker selects it as an explicit override instead of showing Default (agent model); add the real row fields and fix the override/default discrimination so the regression stays fixed beyond the optimistic local null.
Useful? React with 👍 / 👎.
|
Codex review: needs real behavior proof before merge. Reviewed July 6, 2026, 4:36 AM ET / 08:36 UTC. Summary PR surface: Source +15, Tests +126. Total +141 across 5 files. Reproducibility: yes. The source path shows Gateway rows can carry effective per-agent model fields while the selector treats any row model as a non-empty override unless a local null override cache entry exists. Review metrics: none identified. Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Finish the selector-state fix so the active agent default drives both the Default label and the selected/default state when the row model equals the effective agent default, then keep the focused unit and browser regression coverage. Do we have a high-confidence way to reproduce the issue? Yes. The source path shows Gateway rows can carry effective per-agent model fields while the selector treats any row model as a non-empty override unless a local null override cache entry exists. Is this the best way to solve the issue? No. Passing the selected agent default down from the chat-page owner is the right layer, but the patch also needs to keep the default option selected when the row model is just the effective agent default. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against d6c880ae9203. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +15, Tests +126. Total +141 across 5 files. View PR surface stats
Acceptance criteria:
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
Land-ready verification for
Known proof gaps: none for the changed UI behavior. The replacement preserves @hyspacex's co-author and changelog credit from #77690. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7ac4b0339
ℹ️ 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".
| restartIntent: { | ||
| ...currentRestartRequest.restartIntent, | ||
| ...restartIntent, | ||
| force: true, | ||
| reason: restartReason, |
There was a problem hiding this comment.
Make update upgrades interrupt the active drain
When an update signal arrives after runAcceptedRequest has already started draining a regular restart, this force: true is only stored on activeRestartRequest; the shutdown loop already captured the original restartIntent before it computes the drain timeout and checks restartIntent?.force, so active task/run waits continue using the old non-update restart policy. In the scenario where a config restart is draining active work and a managed update.run/update.auto arrives, the update can sit behind the old drain or hit the forced-exit watchdog instead of aborting the drain and reaching the clean update handoff path.
Useful? React with 👍 / 👎.
e7ac4b0 to
c68f7ab
Compare
c68f7ab to
4b7bcbf
Compare
Co-authored-by: Harry Xie <[email protected]>
4b7bcbf to
07b81bf
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07b81bf108
ℹ️ 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".
| const agentDefaultModel = this.context.agents.state.agentsList?.agents.find( | ||
| (agent) => agent.id === currentAgentId, | ||
| )?.model?.primary; |
There was a problem hiding this comment.
Use resolved agent defaults for picker labels
When the selected agent's model.primary is an alias or otherwise normalized by resolveDefaultModelForAgent, this passes the raw config string into the picker. agents.list builds that field from resolveAgentEffectiveModelPrimary (src/gateway/session-utils.ts:1207-1219), while sessions.list rows resolve aliases to the effective provider/model (src/gateway/session-utils.test.ts:2197-2243 covers anthropic/sonnet-4.6 becoming anthropic/claude-sonnet-4-6). After clearing an override, modelOverrides[key] === null makes the picker use this default label, so alias-backed agents show the raw alias instead of the actual catalog/default model; pass the resolved row identity or expose a resolved agent default.
Useful? React with 👍 / 👎.
|
Land-ready verification for
Known proof gaps: none for the changed UI behavior. The replacement preserves @hyspacex's co-author credit from #77690; maintainer changelog credit is isolated in #100740 to avoid coupling this exact-head UI proof to the high-traffic release-notes hunk. |
|
Merged via squash.
|
Co-authored-by: Harry Xie <[email protected]>
Co-authored-by: Harry Xie <[email protected]>
What Problem This Solves
The Control UI labels the empty model override as
Default (<global model>)even when the selected agent has its own primary model. Clearing a session override therefore shows the wrong effective model. Fixes #77440 and salvages the valid intent from #77690.Why This Change Was Made
Resolve the selected agent once at the chat-page owner, then pass only its prepared primary model into the model selector. The selector keeps the existing Gateway default as a loading/fallback value and does not reparse session keys or carry the full agents collection into a leaf component.
User Impact
After clearing a per-session override, the model picker now shows the selected agent's effective default model. Explicit overrides and the global fallback remain unchanged.
Evidence
tbx_01kwv3e6wmqvd6m1bsh7x9jcgz:pnpm test ui/src/lib/chat/model-select-state.test.ts— 12 passed.pnpm test:ui:e2e ui/src/e2e/chat-flow.e2e.test.ts— 25 real-Chromium flows passed, including agent switch, override, clear, and restored label.git diff --checkpassed.Co-authored-by: Harry Xie [email protected]