Skip to content

fix(ui): keep provider-qualified model picker refs intact#51737

Open
muyouzhi6 wants to merge 1 commit intoopenclaw:mainfrom
muyouzhi6:codex/control-ui-model-picker-provider-refs
Open

fix(ui): keep provider-qualified model picker refs intact#51737
muyouzhi6 wants to merge 1 commit intoopenclaw:mainfrom
muyouzhi6:codex/control-ui-model-picker-provider-refs

Conversation

@muyouzhi6
Copy link
Copy Markdown

@muyouzhi6 muyouzhi6 commented Mar 21, 2026

Summary

  • reuse the configured model collector in the agents overview picker so provider-qualified primary, fallback, and allowlisted model refs stay selectable
  • add a regression test that keeps the chat header picker sending the full provider-qualified model when switching across providers
  • add coverage for the overview picker so it shows configured provider-qualified values instead of falling back to a synthetic Current (...) entry

Testing

  • corepack pnpm --dir ui exec vitest run src/ui/views/chat.test.ts src/ui/views/agents.test.ts src/ui/views/agents-utils.test.ts --config vitest.config.ts
  • corepack pnpm --dir ui exec vitest run src/ui/chat-model-ref.test.ts src/ui/chat/slash-command-executor.node.test.ts --config vitest.config.ts

Closes #51608
Closes #51510

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 21, 2026

Greptile Summary

This PR fixes a bug where provider-qualified model references (e.g. minimax-cn/MiniMax-M2.7, google/gemini-3-flash-preview) defined under agents.defaults.model.primary and agents.defaults.model.fallbacks were not showing up as selectable options in the agents overview picker — only models explicitly listed in the agents.defaults.models alias map were visible, causing the picker to fall back to a synthetic Current (...) entry.

Key changes:

  • resolveConfiguredModels in agents-utils.ts now delegates model ID collection to the existing resolveConfiguredCronModelSuggestions helper, which already traverses model.primary, model.fallbacks, the models alias map keys, and per-agent model entries. Alias labels from the models map are preserved via a case-insensitive lookup applied on top of the broader ID set.
  • chat.test.ts's createChatHeaderState helper is extended with defaultModel, defaultModelProvider, and modelProvider overrides so tests can exercise non-OpenAI provider scenarios without hardcoded "openai" assumptions.
  • Regression tests are added for both the overview picker (confirms provider-qualified IDs appear with correct alias labels and no synthetic fallback) and the chat header picker (confirms the full provider/model string is dispatched to sessions.patch when switching across providers).

Confidence Score: 5/5

  • This PR is safe to merge — it is a targeted, well-tested bug fix with no breaking changes to existing tests.
  • The core logic change is a clean refactor: one function delegates to another already-trusted helper, with alias labels preserved through a case-insensitive map lookup and deduplication via a seen set. All pre-existing tests continue to work with the backwards-compatible createChatHeaderState extension, and the two new test cases directly pin the previously broken behavior. No edge-case regressions or security concerns were found.
  • No files require special attention.

Last reviewed commit: "fix(ui): keep provid..."

@BingqingLyu BingqingLyu mentioned this pull request Apr 27, 2026
Open
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented Apr 28, 2026 •
edited
Loading

Codex review: needs changes before merge.

Summary
The PR updates Control UI Agents overview model option resolution to include provider-qualified configured primary/fallback refs and adds overview plus chat-header regression tests.

Reproducibility: yes. Source inspection on current main shows a config with agents.defaults.model.primary or fallbacks outside agents.defaults.models reaches the overview picker, while resolveConfiguredModels only reads the alias map and can render Current (...).

Next step before merge
A repair worker can refresh or replace the stale branch, preserve the narrow Agents overview helper change, add the changelog entry, and rerun focused UI checks.

Security
Cleared: The diff only changes Control UI TypeScript helpers and UI tests, with no CI, dependency, install, secret-handling, or publishing changes.

Review findings

  • [P3] Add the required changelog entry — ui/src/ui/views/agents-utils.ts:550
Review details

Best possible solution:

Land a refreshed narrow fix that uses the shared configured-model collector for Agents overview options, preserves alias labels, and includes active changelog plus regression coverage.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection on current main shows a config with agents.defaults.model.primary or fallbacks outside agents.defaults.models reaches the overview picker, while resolveConfiguredModels only reads the alias map and can render Current (...).

Is this the best way to solve the issue?

Partly. Reusing the shared configured-model collector is the narrow maintainable fix for the Agents overview picker, but the PR still needs a current-main refresh or replacement and the required changelog entry before merge.

Full review comments:

  • [P3] Add the required changelog entry — ui/src/ui/views/agents-utils.ts:550
    This is a user-facing Control UI fix, but the PR does not update CHANGELOG.md. Repo policy requires a single-line active Fixes entry before merge.
    Confidence: 0.88

Overall correctness: patch is correct
Overall confidence: 0.86

Acceptance criteria:

  • pnpm test ui/src/ui/views/agents.test.ts ui/src/ui/views/agents-utils.test.ts ui/src/ui/chat-model-ref.test.ts ui/src/ui/chat-model-select-state.test.ts ui/src/ui/views/chat.test.ts
  • pnpm exec oxfmt --check --threads=1 ui/src/ui/views/agents-utils.ts ui/src/ui/views/agents.test.ts ui/src/ui/views/chat.test.ts CHANGELOG.md
  • git diff --check
  • pnpm check:changed

What I checked:

  • Current main omits configured primary/fallback refs: resolveConfiguredModels still returns only entries from agents.defaults.models; if that alias map is absent, it returns an empty list before buildModelOptions falls back to Current (...) for the effective model. (ui/src/ui/views/agents-utils.ts:643, 06056926a099)
  • Shared collector already covers the requested configured refs: resolveConfiguredCronModelSuggestions collects agents.defaults.model.primary, agents.defaults.model.fallbacks, alias-map keys, and per-agent model entries, which is the broader source this PR reuses. (ui/src/ui/views/agents-utils.ts:597, 06056926a099)
  • Affected overview picker is live: The Agents overview primary-model select calls buildModelOptions(configForm, effectivePrimary, params.modelCatalog, selectedPrimary), so the narrow helper controls the visible options. (ui/src/ui/views/agents-panels-overview.ts:163, 06056926a099)
  • Model picker contract expects exact provider-qualified selections: The docs say user session selections from the model picker are exact provider/model selections, and the Control UI model picker uses the configured model view rather than the full catalog. Public docs: docs/concepts/models.md. (docs/concepts/models.md:63, 06056926a099)
  • Required changelog entry is absent: An exact changelog search found no entry for this PR, the linked issues, or the provider-qualified model picker fix; the active ### Fixes section is available for the required single-line entry. (CHANGELOG.md:48, 06056926a099)
  • PR branch needs refresh before merge: The provided PR context shows the PR is open and unmerged at head 0fd06e3294977876b558db816172838fd60a8995, based on an older base SHA than current main; the prior ClawSweeper review also reported it unmergeable. (0fd06e329497)

Likely related people:

  • steipete: Prior ClawSweeper context identifies this handle as the author of shared Control UI model-ref centralization and chat model resolution extraction that owns the chat-header side of this behavior. (role: recent maintainer and chat model-ref owner; confidence: high; commits: 7e8f5ca71b7d, 9082795b108b, e0b4f3b995cc; files: ui/src/ui/chat-model-ref.ts, ui/src/ui/chat-model-select-state.ts, ui/src/ui/views/agents-utils.ts)
  • vignesh07: Prior ClawSweeper context identifies a recent Agents UI fix by this handle touching effective model handling, the overview panel, the agents utility helper, and related tests. (role: recent Agents UI maintainer; confidence: medium; commits: 384a590e54e6; files: ui/src/ui/views/agents-panels-overview.ts, ui/src/ui/views/agents-utils.ts, ui/src/ui/views/agents.test.ts)
  • vincentkoc: Local blame in this checkout attributes the affected Agents UI helper and overview lines to the shallow history boundary commit, making this a useful routing candidate but not strong feature-introduction proof. (role: recent maintainer in local shallow history; confidence: low; commits: 03d04c243b86; files: ui/src/ui/views/agents-utils.ts, ui/src/ui/views/agents-panels-overview.ts, ui/src/ui/chat-model-ref.ts)

Remaining risk / open question:

  • The source branch is stale against current main and should be refreshed or replaced before relying on the exact diff.
  • Targeted UI tests were not rerun in this read-only pass because pnpm is unavailable; rerun the listed validation after refreshing the branch.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 06056926a099.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

app: web-ui App: web-ui size: S

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: Control UI Model Picker Sends Wrong Provider Prefix for All Models [Bug] Model selector UI incorrectly combines provider name with model ID

1 participant

@muyouzhi6