feat(ios): hide the thinking-level control for models without reasoning support#100875
Conversation
openclaw-barnacle
Bot
added
app: ios
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de0cd20782
ℹ️ 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".
| showsPicker = self.thinkingPickerIsAvailable( | ||
| for: session, | ||
| modelChoice: self.sessionModelChoice(for: session)) |
There was a problem hiding this comment.
When flushing a queued command for a background/older session that is not present in the currently loaded self.sessions list, session is nil here, but sessionModelChoice(for: nil) and resolvedThinkingLevelOptions(for: nil) fall back to the current defaults; if those defaults are off-only or reasoning == false, the queued command is downgraded to "off". I checked performOutboxFlush loads all persisted commands while bootstrap only refreshes a limited sessions list, so this is reachable and regresses the previous behavior of sending the level captured on the command.
Useful? React with 👍 / 👎.
de0cd20 to
1b53863
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b5386363c
ℹ️ 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 resolved = self.resolvedThinkingLevelOptions(for: session) | ||
| let gatewayAllowsOnlyOff = resolved.isGatewayMetadata && | ||
| resolved.options.allSatisfy { $0.id == "off" } | ||
| return !gatewayAllowsOnlyOff && modelChoice?.reasoning != false |
There was a problem hiding this comment.
For models whose provider thinking profile is explicitly preserved even when the catalog reports reasoning: false (for example the preserveWhenCatalogReasoningFalse paths in src/auto-reply/thinking.ts and Claude Fable/Gemini policies), sessions.list can return multiple thinkingLevels while models.list still exposes reasoning: false. This check hides the picker and makes sends use off anyway, so those supported model-specific thinking levels become unavailable in the native chat UI. Prefer treating non-off gateway thinkingLevels as authoritative, or expose the same preservation contract through the model choice.
Useful? React with 👍 / 👎.
|
Codex review: needs real behavior proof before merge. Reviewed July 6, 2026, 10:30 AM ET / 14:30 UTC. Summary PR surface: Other +610. Total +610 across 11 files. Reproducibility: yes. Source inspection shows non-off gateway Review metrics: 2 noteworthy metrics.
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:
Next step before merge
Maintainer decision needed
Security Review findings
Review detailsBest possible solution: Make gateway session/default thinking metadata authoritative, preserve raw queued thinking until flush-time gating, initialize or block the send gate until model/session metadata is loaded, and require redacted real iOS/macOS proof before merge. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection shows non-off gateway Is this the best way to solve the issue? No. The native surface is plausible, but the current implementation is not the best fix because it lets generic model metadata override gateway-resolved thinking support and gates queued/bootstrap sends before all model state is known. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 94a0d2ec3b79. Label changesLabel justifications:
Evidence reviewedPR surface: Other +610. Total +610 across 11 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 (2 earlier review cycles)
|
clawsweeper
Bot
added
rating: 🧂 unranked krab
1b53863 to
6e355a0
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e355a00cd
ℹ️ 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".
| sessionKey: session.key, | ||
| text: text, | ||
| thinking: self.thinkingLevel, | ||
| thinking: self.effectiveThinkingLevelForSend, |
There was a problem hiding this comment.
When a text send is queued while a model change that hides the picker is still in flight, this stores off permanently. If that sessions.patch(model) later fails or rolls back to a reasoning-capable model, performOutboxFlush correctly waits for the patch and re-evaluates the gate, but it can only re-evaluate the already-downgraded queued value, so the command sends without the user's stored reasoning level. Capture the raw thinkingLevel here and let the flush-time gate decide, matching the live send path that gates after waiting for model patches.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
the
merge-risk: 🚨 session-state
6e355a0 to
6f71621
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f7162101a
ℹ️ 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".
| /// Setter is module-internal for the thinking-level extension only. | ||
| public internal(set) var thinkingLevelOptions: [OpenClawChatThinkingLevelOption] | ||
| /// Setter is module-internal for the thinking-level extension only. | ||
| public internal(set) var showsThinkingPicker = true |
There was a problem hiding this comment.
When macOS restores a persisted non-off thinking level, a fresh bootstrap can make healthOK true after requestHistory/pollHealthIfNeeded but before fetchSessions/fetchModels populate this new gate, while canSend still allows a draft during loading. Because showsThinkingPicker starts true and applyHistoryPayload does not project payload.sessionInfo thinking/model metadata into sessions, effectiveThinkingLevelForSend can forward high/medium for a non-reasoning or off-only model in that window; src/agents/agent-command.ts rejects explicit unsupported thinking levels. Initialize the gate from available history/session metadata or keep sends/outbox capture gated until the session/model metadata sync completes.
Useful? React with 👍 / 👎.
|
Merged via squash.
|
1 participant
Related: #100699
What Problem This Solves
Fixes an issue where iOS/macOS chat users would see and set a thinking-level control even when the selected model does not support configurable reasoning effort — the control silently did nothing for those models, which reads as broken.
Why This Change Was Made
The gateway already computes per-model thinking capability: models with catalog
reasoning: falseresolve to an off-only thinking profile, andmodels.listcarries a per-modelreasoningflag. This PR consumes that existing metadata client-side — no protocol changes. The shared chat view model now hides the thinking picker when (a) gateway-provided session/default thinking metadata resolves to off-only, or (b) the locally selected model choice carriesreasoning == false(which covers the window right after a model switch, before the session metadata refreshes). Legacy gateways that send no thinking metadata keep the picker (fail open). While the control is gated off, sends carry an effective thinking level ofoffinstead of the stored preference — the gateway treats an explicit unsupported level as a hard error (agent-command.tsthrows for explicit levels the model doesn't support), and with the control hidden the user would have no way to fix a stale non-off level. The stored preference itself is never mutated, so switching back to a reasoning-capable model restores the user's level. Session thinking metadata is preserved across same-model patches but deliberately dropped when the model changes (stale metadata must not gate the new model). Thereasoningflag is threaded throughOpenClawChatModelChoiceand both transports (iOS gateway transport decode, macOS webchat mapping) with default-nil compatibility for existing call sites.User Impact
The thinking-level control now appears only when it actually does something for the selected model. Switching to a non-reasoning model hides it immediately; switching back restores it.
Evidence
swift test --package-path apps/shared/OpenClawKit— full suite green;ChatViewModelTestsnow 147 tests including: off-only metadata hides, multi-level shows, metadata-absent legacy shows, localreasoning == falsehides immediately despite stale multi-level metadata, default-sentinel selection resolves the session model for gating, the current-level augmentation cannot flip gating, same-model patches preserve thinking metadata while model changes drop it, and gated sends carryoffwhile the stored level survives;ChatViewModelOutboxTests(20) cover queued/retry sends re-evaluating the gate per session at flush time.xcodebuild build-for-testing+ focusedIOSGatewayChatTransportTestsgreen (reasoning decode assertion added); macOSswift buildgreen.swiftformat --lintvia the iOS filelist — clean.