feat(clients): provider-grouped model picker with fast, verbosity, and inherited-thinking controls#110339
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3cf0dea40c
ℹ️ 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".
|
|
||
| /// `models.list` currently has no fast-support capability field. Keep the | ||
| /// control available and let the gateway validate the session patch. | ||
| public var selectedModelSupportsFastMode: Bool { |
There was a problem hiding this comment.
Hide fast mode where the provider cannot honor it
For any selected model whose provider has no fast-mode runtime mapping (for example Google/Gemini or local providers), this always renders the Fast control and lets the user persist a fastMode override. I checked the web control and it explicitly limits the toggle to providers with wire support (ui/src/lib/chat/model-select-state.ts:70-74), while sessions.patch only validates the value shape and stores it without provider capability checks (src/gateway/sessions-patch.ts:452-462). This makes the native picker advertise a setting that can be a no-op for unsupported providers, so please gate it from the model/provider metadata or add a real gateway capability/validation before showing it.
Useful? React with 👍 / 👎.
|
Codex review: needs real behavior proof before merge. Reviewed July 18, 2026, 12:47 AM ET / 04:47 UTC. Summary PR surface: Docs 0, Other +4188. Total +4188 across 44 files. Reproducibility: yes. from source: select a model whose provider has no Fast runtime mapping, then open the native control; the changed capability predicate always returns true and exposes a setting the gateway can store without provider validation. Review metrics: 1 noteworthy metric.
Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Copy recommended automerge instructionNext step before merge
Security Review findings
Review detailsBest possible solution: Use the existing web Fast-support rule through a shared capability contract or a native-equivalent provider gate, then attach redacted native evidence showing provider grouping plus supported and unsupported Fast, inherited thinking, and verbosity-reset behavior. Do we have a high-confidence way to reproduce the issue? Yes, from source: select a model whose provider has no Fast runtime mapping, then open the native control; the changed capability predicate always returns true and exposes a setting the gateway can store without provider validation. Is this the best way to solve the issue? No: provider grouping and inherited controls are a reasonable parity direction, but the unconditional Fast predicate is not the best implementation because web already limits Fast to supported providers. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 4e0cc187a21c. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Docs 0, Other +4188. Total +4188 across 44 files. View PR surface stats
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
Review history (3 earlier review cycles)
|
…d inherited-thinking controls
…t inherited verbosity
3cf0dea to
ea61ff3
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea61ff3f3d
ℹ️ 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".
| let acceptedOverride = next == nil ? nil : (result?.fastMode ?? next) | ||
| let acceptedEffective = result?.effectiveFastMode | ||
| ?? result?.fastMode | ||
| ?? acceptedOverride | ||
| ?? baselineEffectiveFastMode |
There was a problem hiding this comment.
Don't reuse stale effective fast state on default clear
When the user changes Fast from an explicit override back to Default (inherited), the live sessions.patch response does not include a resolved effectiveFastMode for this fast-only patch (the gateway response only adds resolved model/thinking metadata in src/gateway/server-methods/sessions-mutations.ts:297-307, and the stored entry has no effective field). In that scenario these fallbacks all become nil until baselineEffectiveFastMode, so an explicit On→Default clear keeps rendering Fast even when the inherited/default config is off (or Off→Default keeps rendering Standard when the inherited default is fast) until a later session-list refresh happens to correct it. Please force/reuse an authoritative session-list projection for clears instead of writing the previous effective state back as accepted.
Useful? React with 👍 / 👎.
…venience flagged by periphery
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: feedbff828
ℹ️ 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".
|
|
||
| public var thinkingOverrideIsInherited: Bool { | ||
| if self.hasAppliedLiveSessions { | ||
| return self.currentSessionEntry()?.thinkingLevel == nil |
There was a problem hiding this comment.
Keep inherited thinking after model-only patches
When a session is inheriting its thinking level and the user changes only the model, the gateway returns resolved.thinkingLevel as the effective default for the new model (src/gateway/server-methods/sessions-mutations.ts:297-307), and the model-selection path writes that patchResult.thinkingLevel back into the local session row (apps/shared/OpenClawKit/Sources/OpenClawChatUI/ChatViewModel.swift:1616-1624). This new inherited check then treats the row as an explicit override because it only tests for nil, so the picker flips from Default (inherited) to <level> (override) even though no thinking override was persisted; preserve override-vs-effective state instead of deriving inheritance from the merged resolved value.
Useful? React with 👍 / 👎.
|
Merged via squash.
|
…d inherited-thinking controls (openclaw#110339) * feat(clients): provider-grouped model picker with fast, verbosity, and inherited-thinking controls * chore(i18n): translate model-control strings and regenerate platform catalogs * feat(clients): inherited-default restore for fast mode and verbosity controls * fix(clients): preserve control state in thinking snapshots and persist inherited verbosity * chore(i18n): resync native inventory after rebase onto message-reader main * chore(i18n): realign locale artifacts after rebase onto message-reader main * refactor(clients): drop superseded fast-mode accessors and picker convenience flagged by periphery * chore(i18n): resync inventory line refs after dead-code removal
What Problem This Solves
The native macOS chat's model picker is a flat list with no provider context, and it exposes none of the web chat's per-session model controls: Fast mode, verbosity, or an explicit "inherit default" thinking choice. Round 3a of the macOS/web parity effort (follow-up to #109712, #109995, #110019).
Why This Change Was Made
Continue converging the native macOS chat on web-chat capability using shared
OpenClawKitcode so iOS benefits from the same surfaces later.ChatModelPickerStore.swift,ChatComposer.swift,ChatWindowShell.swift).off/on/full), and an explicitDefault (inherited)thinking option with optimistic patches, per-target serialization, and target-aware rollback (ChatViewModel+ModelControls.swift,ChatViewModel+Thinking.swift).sessions.patchfields:thinkingLevel(null = reset),fastMode(true/false/"auto"/null),verboseLevel("off"/"on"/"full"/null) — verified againstpackages/gateway-protocol/src/schema/sessions.tsandsrc/gateway/sessions-patch.ts.docs/platforms/mac/webchat.md; i18n catalogs regenerated and translated.User Impact
macOS chat users can pick models grouped by provider and adjust Fast mode, verbosity, and thinking inheritance per session from the composer, matching the web chat. Shared implementation keeps iOS ready to adopt the same controls.
Evidence
pnpm format:swiftclean,pnpm lint:swift0 violations,pnpm native:i18n:checkchanged=false, apple/android i18n vitest gates green.packages/gateway-protocol/src/schema/sessions.ts:373,src/gateway/sessions-patch.ts:452,src/gateway/sessions-patch.test.ts:501.Review-driven hardening (post-spec)
Default (inherited)selection that patches explicit null, matching web's clear-override contract (ui/src/pages/chat/chat-session.tsswitchChatFastMode); legacy"auto"overrides display via effective state and are not re-selectable, mirroring web.sessions.listrestore.openclaw.webchat.verboseLevelvia the additiveonVerbosePreferenceChangedcallback (macOS consumer coordinated).