Skip to content

fix(desktop): make model and thinking-level changes instant without success toasts - #3749

Open
Sun-GLiang wants to merge 18 commits into
apache:mainfrom
Sun-GLiang:fix/3745-remove-model-thinking-toasts
Open

fix(desktop): make model and thinking-level changes instant without success toasts#3749
Sun-GLiang wants to merge 18 commits into
apache:mainfrom
Sun-GLiang:fix/3745-remove-model-thinking-toasts

Conversation

@Sun-GLiang

@Sun-GLiang Sun-GLiang commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Active-task model, thinking-level, and permission-mode changes complete without success toasts or persistence-only loading presentation. Localized failure feedback remains.
  • Model, thinking level, and permission mode use independent useSessionSettingIntent controllers. Controls show the latest requested value immediately, rapid changes converge on the latest intent, terminal failures roll back to the last committed value, and newer catalog revisions retire optimistic overlays.
  • Full access confirmation happens before the permission intent starts, so the existing permission value remains visible until the user confirms.
  • Optimistic model and thinking values are scoped to their controls. Session-health and execution-state decisions continue to use authoritative session data.
  • A model becomes the saved composer default only after Runtime Host confirms the requested connection/model pair.
  • Model and thinking menus expose the selected option with radio semantics, and each trigger's accessible name includes its current value.
  • Send admission, Stop behavior, and active-turn follow-up ordering are unchanged. Configuration ordering before Host turn admission remains tracked by Allow Session configuration changes while a turn is running #3347.

Fixes #3745

Verification

  • UI test suite: 229 passed.
  • Desktop test suite: 1474 passed.
  • Desktop typecheck passed.
  • Biome passed for all 17 changed TypeScript/TSX files.
  • Desktop production build passed, including the third-party notice check.
  • Diff validation passed with no whitespace errors.
  • Intent coverage verifies latest-wins coalescing, rollback to the last committed value, explicit default thinking level, catalog-revision retirement, and session teardown.
  • UI coverage verifies cross-connection model changes when two connections expose the same model ID, selected radio state for model/thinking options, and accessible trigger labels containing the current value.
  • Architecture coverage verifies that model, thinking level, and permission mode use the generic intent seam and that renderer-side setting state does not order send, Stop, or follow-up paths.
  • No product documentation update is needed because the configuration contract is unchanged; this PR changes transient control feedback.

Visual verification

Before

Rapid thinking-level changes created stacked success toasts with obsolete intermediate values.

Three stacked thinking-level success toasts after rapid switching

After — instant, no spinner

Model and thinking-level controls reflect each selection immediately without a spinner, disabled flicker, or success toast.

Instant model and thinking-level changes with no spinner

Static final state (GPT-5.6 Sol / Low):

Model and thinking-level controls updated with no spinner

AI use

  • No generative or agentic tooling authored a material portion of this change.
  • Generative or agentic tooling authored a material portion of this change.

Tool(s) and scope: OpenAI Codex implemented the session-setting intent integration, transient-feedback cleanup, regression and accessibility coverage, and verification. A Generated-by: OpenAI Codex trailer is present on each materially affected commit.

Checklist

  • Tests cover the change or the PR explains why tests are not needed.
  • Lint, formatting, and relevant test suites pass locally.
  • User-facing changes update docs or the PR explains why docs are not needed.
  • AI use is disclosed honestly, and any materially AI-authored commit includes the required Generated-by trailer.

Does this PR entail a user-facing behavior change?

  • Yes, and the PR explains the change and why no user-facing docs are needed.
  • No.

@Sun-GLiang
Sun-GLiang marked this pull request as ready for review August 25, 2026 04:54

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this head and found blocking issues.

[P2] Stop cannot cancel the pre-send model-setup flight

app-shell-chat-actions.ts:493-508 arms the turn before waiting for settleSessionModelSettings. Stop at that point only calls sessions.stop, which returns immediately when no Host turn exists, so the original send still proceeds after settings settle despite user cancellation.

[P2] Active-Turn follow-up bypasses the settlement barrier

app-shell.tsx:1940-1952 enqueues follow-ups directly without going through the settling actions.send path. A model mutation failing to settle still leaves the follow-up queued under the old model, violating the "latest mutation silently settles before immediate send" contract.

Checks on 3686d2aaf are test: success.

简体中文存在预发送取消与跟随路径两项阻断。

@Sun-GLiang

Copy link
Copy Markdown
Contributor Author

Addressed both blocking paths in ee34dd0.

  • Stop now cancels and disarms a local pre-send settings flight before Runtime Host admission; when no local flight exists, it retains the existing sessions.stop behavior.
  • Active-turn follow-ups now settle the latest model/thinking intent before enqueueing. A terminal settlement failure skips enqueue and leaves the composer draft available.

Added regression coverage for local cancellation, preserved Host stop behavior, successful follow-up settlement, and failed settlement. Desktop tests pass 1496/1496; typecheck, Biome, and the production build also pass.

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update on ee34dd0026:

I found blocking issues that persist/drift in this head.

[P1] Follow-up can still start a new root after Stop

A follow-up submitted on an active turn waits for settings, then submitFollowUpAfterModelSettings has no cancellation identity. If the user presses Stop during the wait, preSendSettingsRef is cleared but the follow-up waiter is not cancelled — it still enqueues and the idle next_turn root starts.

[P2] Root/follow-up dispatches without re-checking surface owner

After the settings wait, the dispatch does not re-verify surface owner. Switching from surface A to B during the wait still sends with A's intent.

[P2] Thinking intent rolls back another client's new model

A stale thinking intent holding an old (model, thinking) tuple can overwrite a newer model B committed by another client.

Checks on ee34dd0026 are test: failure (slash-menu unrelated flake but still not green) — not green.

简体中文存在跟随提交与所有者/模型回滚三项阻断。

@Sun-GLiang

Copy link
Copy Markdown
Contributor Author

Addressed the latest settlement/readiness findings in b1ae2a7.

  • Existing-session sends now capture the active settings settlement synchronously at send entry, before transcript or readiness awaits. A mutation failure remains observable after its intent is retired, so the send is skipped and the draft stays intact.
  • Readiness now distinguishes an authoritative model overlay from the absence of a model override. A model adopted from a thinking-mutation response is checked before the first readiness probe, even when there was no local model selection.
  • Mutation response fencing now uses a per-session catalog generation and only treats a response as stale when the session moved to a different model context. A same-model success is accepted even if an older catalog snapshot committed first, so it is not retried and cannot be negated by a later retry failure.

Added regression coverage for all three races. Desktop tests pass 1516/1516; production build, typecheck, Biome, and diff checks also pass.

@Sun-GLiang Sun-GLiang changed the title fix(desktop): remove model and thinking-level success toasts fix(desktop): make model and thinking-level changes instant without success toasts Aug 25, 2026

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this head and found blocking issues.

[P2] use-session-model-settings-intent.ts:118 — model equality used as ordering token

A thinking write M/high commits, another client then commits M/low and the new catalog snapshot arrives before the older IPC response. Because model is unchanged, the stale M/high is accepted and stamped at the current revision, leaving the overlay wrong if the best-effort refresh fails. Carry a causal token (host revision) through mutation result and catalog.

[P2] chat-model-switcher.tsx:231 — accessibility loses current/success state after toast removal

Fixed aria-label replaces visible label, current check is aria-hidden, no selected state, and status only announces warnings. After removing the toast, AT cannot read the current model or hear success.

[P3] app-shell.tsx:1773 — cancellation epoch tombstones accumulate

Every Stop inserts an epoch, but clearSessionRendererState never cleans it, leaving renderer-lifetime tombstones.

Checks on b1ae2a754f are test: success.

简体中文存在两项正确性与一项可及性阻断,另有一项生命周期 P3。

@Astro-Han

Copy link
Copy Markdown
Contributor

Thanks for the thorough work here — the test coverage in particular is careful. But I'd like to redirect the approach before this goes further.

The seam already exists. apps/desktop/src/renderer/use-session-setting-intent.ts is a 144-line generic hook that already does what the new 420-line use-session-model-settings-intent.ts does: optimistic overlay, latest-intent-wins coalescing, rollback to the committed value on failure, retirement against catalogRevision. Plan mode and orchestration mode already use it (app-shell.tsx:928, :940). Model and thinking level should join it rather than get a parallel implementation with its own version counters.

Two of the three reported problems need no new machinery. Stacked toasts and the wrong from model both disappear when the success toasts go. That from comes from latestAssistantModelId(messages) — the transcript, not the configuration before the switch — which is exactly why A → B → C can report A → C. It is also the only Desktop consumer of latestAssistantModelId, and the only reason the whole messages array is a dependency of the settings-action factory.

Part of this belongs to #3347, which we have not decided to do. Settle-before-send, Stop cancelling an in-flight settings write, and gating follow-ups on settlement all address "the send may use a configuration the Host has not committed yet". That is #3347's problem, and it puts renderer-side ordering in front of Host turn admission. Please drop those here.

Suggested shape — this should come out as a net deletion:

  • route setModel, setThinkingLevel and setPermissionMode through the existing useSessionSettingIntent. For permission mode, move the bypass confirmation to the call site: the hook stays unaware of it, and the control keeps showing the old value until the user confirms;
  • delete sessionModelChangeRegistry, permissionModeChangeRegistry, pendingSessionModelBySession, pendingPermissionModeBySession, the modelChangePending / permissionModePending prop chains, and pending / loading on the switchers;
  • delete the success toasts and the copy, label helpers and transcript lookup that exist only for them. Failure toasts stay.

Two smaller things worth closing while you are here: pendingSessionModelChangesRef is keyed by session and shared between the model and thinking-level writes, so each blocks the other even though the Host exposes them as independent calls; and "why can't I change this now" is derived three times (composer.tsx:1345, app-shell.tsx:1242, app-shell.tsx:3095) with two separate sets of copy.

简体中文

感谢你细致的工作,测试尤其扎实。但在继续之前我想调整一下方向。

这个 seam 已经存在。 apps/desktop/src/renderer/use-session-setting-intent.ts 是一个 144 行的泛型 hook,新增的 420 行 use-session-model-settings-intent.ts 做的是同一件事:乐观 overlay、最新意图胜出的合流、失败回滚到 committed 值、按 catalogRevision 退休。Plan 模式和编排模式已经在用它(app-shell.tsx:928:940)。模型和思考级别应该接进去,而不是在旁边再实现一套带版本号的。

三个问题里有两个不需要新机制。 toast 堆叠和 from 取错模型,删掉成功 toast 就都没了。那个 from 取自 latestAssistantModelId(messages)——转录,而不是切换前的配置,这正是 A → B → C 会报成 A → C 的原因。它也是 Desktop 侧 latestAssistantModelId 的唯一消费者,以及整个 messages 数组成为 settings-action 工厂依赖的唯一原因。

有一部分属于 #3347,而我们还没有决定要做它。 发送前 settle、Stop 取消在途的设置写入、follow-up 进队列前先 settle,解决的都是「发送时用的配置可能还没提交到 Host」——那是 #3347 的问题,而且它让渲染层排在了 Host 的 Turn 准入之前。请从本 PR 移除。

建议的形状,结果应该是净删除:

  • setModelsetThinkingLevelsetPermissionMode 全部走既有的 useSessionSettingIntent。权限模式的 bypass 确认弹窗提到调用点:hook 不需要知道它存在,用户确认之前控件继续显示旧值;
  • 删除 sessionModelChangeRegistrypermissionModeChangeRegistrypendingSessionModelBySessionpendingPermissionModeBySessionmodelChangePending / permissionModePending 两条 props 链,以及切换器上的 pending / loading
  • 删除成功 toast,以及只为它们存在的文案、标签辅助函数和转录取源。失败 toast 保留。

另有两处顺手可以收掉:pendingSessionModelChangesRef 以 session 为 key,被模型写入和思考级别写入共用,于是两者互相阻塞,而 Host 侧本来就是两条独立调用;另外「为什么现在不能改」被推导了三遍(composer.tsx:1345app-shell.tsx:1242app-shell.tsx:3095),用了两套不同的文案。

@Astro-Han

Copy link
Copy Markdown
Contributor

Following up with the full reasoning behind that suggestion — it is a fairly large redirect, so it seems only fair to show the whole picture rather than just the conclusion.

Two mechanisms for one fact

Desktop currently has two different answers to "a session setting write is in flight", and which one you get depends on which setting you touched.

Plan / orchestration mode Model / thinking / permission
Implementation useSessionSettingIntent (144 lines, generic) pending registry + React state + toast
Representation one intent record keysRef and a React state slot, kept in sync by hand
Rapid changes latest intent wins second click silently dropped (if (has) return)
During the write control stays usable, shows the new value disabled + spinner, still shows the old value
On success nothing toast
On failure rolls back to the committed value toast only, no rollback

Every one of these five settings goes through the same shape on the Host side — the same invokeSessionSummary call, the same SessionSummary return. The divergence is entirely in the renderer.

flowchart TB
  subgraph A["useSessionSettingIntent — 144 lines, generic"]
    A1["Plan mode"] --> AI["desired / committed<br/>committedAtCatalogRevision / inFlight"]
    A2["Orchestration mode"] --> AI
    AI --> AO["overlay: shows immediately, latest wins"]
    AI --> AW["write loop: re-reads desired<br/>rolls back to committed on failure"]
    AW --> AR["refreshCatalog, retire overlay by catalogRevision"]
  end
  subgraph B["pending registry + toast"]
    B1["Model"] --> BR["keysRef: Set of sessionId"]
    B2["Thinking level"] --> BR
    B3["Permission mode"] --> BR2["keysRef: Set of key"]
    BR --> BS["pendingSessionModelBySession"]
    BR2 --> BS2["pendingPermissionModeBySession"]
    BS --> BD["disabled + spinner"]
    BS2 --> BD
    BR --> BX["repeat click dropped"]
    BW["write succeeds"] --> BT["toast.success"] --> BF["refreshSessions"]
  end
Loading

What the difference looks like in time

sequenceDiagram
  autonumber
  participant U as User
  participant C as Control
  participant S as settings-actions
  participant R as pending registry
  participant H as Runtime Host
  Note over U,H: today, model / thinking / permission
  U->>C: pick B
  C->>S: setSessionModel(B)
  S->>R: keysRef.add(sessionId)
  S->>S: setPendingBySession(true)
  Note over C: disabled + spinner, still shows A
  U->>C: quickly pick C
  C->>S: setSessionModel(C)
  S-->>C: has(sessionId), returns early — C is lost
  S->>H: sessions:setModel(B)
  H-->>S: SessionSummary(B)
  S->>S: latestAssistantModelId(messages) for the "from" label
  S->>U: toast "A to B"
  S->>H: refreshSessions()
  S->>R: delete + setPendingBySession(false)
Loading
sequenceDiagram
  autonumber
  participant U as User
  participant C as Control
  participant I as useSessionSettingIntent
  participant H as Runtime Host
  Note over U,H: today, plan / orchestration
  U->>C: pick B
  C->>I: request(sessionId, B)
  I->>C: overlay = B, shown immediately, not disabled
  I->>H: write(B)
  U->>C: quickly pick C
  C->>I: request(sessionId, C)
  I->>I: desired = C, no second write started
  I->>C: overlay = C
  H-->>I: committed(B)
  I->>H: write(C)
  H-->>I: committed(C)
  I->>H: refreshCatalog()
  I->>I: newer catalogRevision retires the overlay
Loading

The second diagram is the behaviour your issue asks for. It is already in the tree.

What disappears if the three settings move over

  • pendingSessionModelBySession and pendingPermissionModeBySession — two state slots, plus their selectors, setters, AppShellSessionUiState fields, and the two reads in use-app-shell-session-ui-reads
  • sessionModelChangeRegistry and permissionModeChangeRegistry — two registry instances, four keysRef hand-offs, and the teardown in app-shell-effects
  • three add/delete pairs in app-shell-session-settings-actions.ts, each written twice (once on the ref, once on the state slot), plus omitSessionKey
  • the modelChangePending and permissionModePending prop chains, and pending / loading on both switchers
  • the success toasts, their copy in both locales, modelLabel / modelEndpointLabel, and the latestAssistantModelId(messages) lookup — which also removes the entire messages array from the settings-action factory's dependencies, so the actions stop being rebuilt on every transcript change

Nothing new is added: useSessionSettingIntent is generic over the value, and the only adapter needed is SessionSummary to boolean in write.

For permission mode, the bypass confirmation moves to the call site — confirm first, then request. The hook never learns that a dialog exists, and the control keeps showing the old value until the user confirms, which is the behaviour we want anyway.

What we give up

The control is no longer disabled while a write is in flight. That is the point of your issue, so I think it is a gain — but it is a real behaviour change and worth stating plainly.

Two smaller things in the same area

pendingSessionModelChangesRef is keyed by session and shared between the model write and the thinking-level write, so changing one blocks the other. The Host exposes them as two independent calls; the coupling is an artifact of the shared key.

"Why can't I change this right now" is derived in three places — composer.tsx:1345, app-shell.tsx:1242, app-shell.tsx:3095 — from slightly different inputs, with two separate sets of copy. app-shell.tsx:3091 documents a race that came from reading the persisted status instead of the live turn witness; the copy in composer.tsx still takes activeSession.status as one of its inputs.

Neither is a blocker. They are cheap to close while the area is open, and both go away on their own once the three settings share one path.

Happy to talk through any of this if you see it differently — you have been in this code more recently than I have.

简体中文

补上建议背后的完整理由。这是一次幅度不小的方向调整,只给结论不太公平。

一个事实,两套机制

Desktop 现在对「会话设置写入在途」有两个不同的答案,取决于你动的是哪个设置。

Plan / 编排模式 模型 / 思考级别 / 权限
实现 useSessionSettingIntent(144 行泛型) pending 注册表 + React state + toast
表示 一条 intent 记录 keysRef 一个 state 槽,手工保持同步
快速连改 最新意图胜出 第二次点击被静默丢弃if (has) return
写入期间 控件可用,显示新值 禁用 + 转圈,仍显示旧值
成功时 无提示 toast
失败时 回滚到 committed 值 只弹 toast,不回滚

这五个设置在 Host 侧走的是完全相同的形状——同一个 invokeSessionSummary,同样返回 SessionSummary。分叉全部发生在渲染层。

上面两张时序图分别对应这两条路径。第二张就是你 issue 里想要的行为,它已经在仓库里了。

三个设置迁过去之后会消失的东西

  • pendingSessionModelBySessionpendingPermissionModeBySession 两个 state 槽,及其 selector、setter、AppShellSessionUiState 字段和 use-app-shell-session-ui-reads 里的两处读取
  • sessionModelChangeRegistrypermissionModeChangeRegistry 两个注册表实例、四处 keysRef 传参,以及 app-shell-effects 里的清理
  • app-shell-session-settings-actions.ts 里三对 add/delete,每对都写两遍(ref 一次、state 槽一次),以及 omitSessionKey
  • modelChangePending / permissionModePending 两条 props 链,和两个切换器上的 pending / loading
  • 成功 toast、两种语言的文案、modelLabel / modelEndpointLabel,以及 latestAssistantModelId(messages)——这也顺带把整个 messages 数组从 settings-action 工厂的依赖里去掉,动作不再随转录每次变化而重建

不需要新增任何东西:useSessionSettingIntent 对值是泛型的,唯一需要的适配是 write 里把 SessionSummary 转成 boolean

权限模式的 bypass 确认弹窗移到调用点——先确认,再 request。hook 完全不需要知道有弹窗,用户确认之前控件继续显示旧值,这本来也是我们想要的行为。

让出的东西

写入在途时控件不再被禁用。这正是你 issue 的目的,所以我认为是收益,但它确实是一次行为变化,应该明说。

同一片区域的两个小问题

pendingSessionModelChangesRef 以 session 为 key,被模型写入和思考级别写入共用,改一个会挡住另一个。Host 侧本来就是两条独立调用,这个耦合只是共用 key 的副产物。

「为什么现在不能改」在三个地方各推导了一遍——composer.tsx:1345app-shell.tsx:1242app-shell.tsx:3095——输入略有差异,还用了两套文案。app-shell.tsx:3091 记录过一次因为读持久化 status 而不是实时 turn witness 导致的竞态;composer.tsx 那份仍然把 activeSession.status 当作输入之一。

两个都不阻塞。趁这块打开时收掉很便宜,而且三个设置合并到同一条路径后它们会自行消失。

如果你有不同看法,随时聊——这块代码你比我更熟。

@Sun-GLiang
Sun-GLiang force-pushed the fix/3745-remove-model-thinking-toasts branch from 071ec88 to 3fdf585 Compare August 25, 2026 09:37

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update on 3fdf5857a0:

[P2] Model intent fire-and-forget allows send on stale model

app-shell.tsx:3030-3032 fires modelIntent.request() without waiting, and app-shell-chat-actions.ts:349-350 send() no longer waits for settleSessionModelSettings. The Host setModel IPC can still be pending when sessions.send runs, so the turn can start with the old model A even though the UI shows B.

Fix: restore a per-session settlement barrier between model change and send, or carry expected model/version into send and retry on mismatch.

Checks on 3fdf5857a0 pending — code is NO-GO independent of CI.

简体中文存在模型意图与发送竞态导致的错模型启动。

@Sun-GLiang
Sun-GLiang force-pushed the fix/3745-remove-model-thinking-toasts branch from 23d3fb7 to 597cfe7 Compare August 25, 2026 10:39
@Sun-GLiang
Sun-GLiang force-pushed the fix/3745-remove-model-thinking-toasts branch from 1b31027 to 064af22 Compare August 25, 2026 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(desktop): make model and thinking-level changes instant without success toasts

2 participants