fix(ui): stop the default-model picker spinning on every switch

[liuxiaocs7]

Summary

Switching the default model in Settings › 通用 › 任务默认 › 默认模型 spun a
loading spinner on the picker trigger for the whole save.

ModelPicker drove the Astryx Selector through its async changeAction
prop. On that path the Selector holds the trigger aria-busy (a <Spinner>)
until its built-in optimistic value catches up to the controlled value — i.e.
the whole setDefaultModel + connection-refresh round-trip. The sibling
permission-mode and thinking-level selectors never spun because they use the
synchronous onChange path.

This switches ModelPicker to onChange (fire-and-forget, never busy). On that
path Astryx no longer advances its own optimistic value, so the Settings row
supplies the "reflect the pick immediately" half of #3827 itself — otherwise
the trigger would sit on the old model (disabled) until the refresh landed.

Reflecting the pick without a stale/premature race

The row keeps a local optimistic value (useOptimisticSelection) shown the
instant a model is picked. It is cleared by a read barrier keyed on the
connections read generation — not a value compare and not a snapshot
reference (both are ambiguous: a snapshot ref only proves a read finished, so
a read already in flight at pick time, returning the pre-write value, would
clear the pick as soon as it commits).

• begin(next) shows the pick; the barrier is disarmed.
• settle(floor) arms the barrier at the reads issued once the write is durable.
• Only a read issued strictly after the write (the row's own refresh) clears
  it; a read issued at/before the write (generation ≤ floor) never does.

Resulting behavior, all correct:

case	shown
our save accepted	authoritative value
in-flight pre-write read commits the old value	pick kept (barrier disarmed / gen ≤ floor)
concurrent external write, or A→B→A restore	authority (the accepted post-write read)
refresh lands no accepted read (failure/invalidation)	pick kept — the write persisted it
setDefaultModel threw	rolled back to authoritative

The committed connections read generation is threaded from the settings request
authority (settings-request-authority.ts → settings-surface.tsx) to the row.
The now-unused loading prop is dropped from ModelPicker.

Verification

Ran locally (macOS, Node v24) against the current main:

• @maka/ui unit test use-optimistic-selection.test.tsx — 8 cases: instant
  show; in-flight read before settle; in-flight read at/under the floor;
  post-write refresh clears to the pick; concurrent external write; A→B→A;
  refresh-lands-nothing keeps the pick; cancel on a thrown write.
• @maka/ui + @maka/desktop typecheck (incl. tsconfig.storybook.json) — clean.
• biome lint — clean.
• Storybook render smoke (build-storybook + smoke:storybook) — passed (195 stories).

On a no-spinner unit test: the no-spin behavior is now structural —
ModelPicker has no changeAction/loading code path to spin. A faithful
regression test needs a real browser: Astryx's spinner comes from
startTransition + useOptimistic, which do not surface as aria-busy under
node:test+linkedom (verified — a probe still passed after flipping to
changeAction, so it would have been a false guard and was not kept). The
optimistic-state logic is unit-tested above; the wiring is guarded structurally.

Did not run the full desktop Playwright e2e locally (renderer change); CI covers it.

AI use

• Generative tooling made a substantive contribution

Tool(s): Claude Code — read the installed Astryx Selector, designed the
read-generation barrier, implemented it, and authored the unit test. The human
contributor of record (@liuxiaocs7) reviewed the work and owns its accuracy and
licensing. A Generated-by: Claude Code trailer is on the commit; please retain
it on the squash commit.

Checklist

• Tests cover the change and fail without it (optimistic-state logic)
• Lint, typecheck and the affected suites pass locally
• After screenshot/recording — to be attached by @liuxiaocs7 (cannot capture Electron in the review environment)

Does this PR entail a change in behavior?

• Yes — described under Summary above

[Astro-Han]

Update on d5cb4e8b1b:

[P2] Optimistic pendingDefaultModel never clears on external Host change

pendingDefaultModel only clears when === selectedValue. If another window writes C after this window picked B, refresh updates selectedValue to C (B!=C) so pending B stays and keeps covering ModelPicker — Host authority is C but UI shows B or "unset" (stale/authority split).

Fix: bind optimistic to Host revision/target and discard on non-matching accepted refresh.

Checks on d5cb4e8b1bad49ae701f09fdcedc7b496ed65df5 are test: SUCCESS — code is NO-GO.

简体中文

外部写入后乐观值不清理导致显示不一致。

[liuxiaocs7]

Thanks — confirmed and fixed in c9c8cb3.

Root cause matches your read.props.connections/defaultSlug don't only
update via this row's own onRefresh() — settings-surface.tsx keeps a live
connectionsBridge.subscribeEvents(() => reloadConnections(...)) subscription,
and reloadConnections is gated by runtimeHostRequestAuthority so the
last-accepted Host snapshot wins. So an external write of C can move
selectedValue to C independently of this window. The old clear ran only on
pendingDefaultModel === selectedValue, so when the accepted value diverged
from the pick (B != C) the optimistic overlay was stranded forever and kept
covering the trigger — authority C, UI B. (Same strand also applied if the Host
normalized/rejected the pick to a different value.)

Fix: drop the optimistic overlay as soon as the save's refresh lands, rather
than only when it happens to equal the pick. After await onRefresh() the Host
snapshot is authoritative regardless of which write it accepted, so the
trigger now settles on it — this pick or an external one — and can never pin a
stale value. The no-spin onChange path and the instant-label UX are unchanged,
and the failure-path clear is untouched.

- useEffect(() => {- if (pendingDefaultModel !== null && pendingDefaultModel === selectedValue) {- setPendingDefaultModel(null);- }- }, [pendingDefaultModel, selectedValue]);
...
await props.onRefresh();
+ // Host snapshot is now authoritative — whether it accepted this pick or a+ // concurrent external write. Drop the overlay so the trigger settles on it.+ if (mountedRef.current) setPendingDefaultModel(null);

Not adding a dedicated regression test: this optimistic logic lives in the
desktop renderer (general-settings-page.tsx), which has no renderer unit
harness — apps/desktop's test only runs the main process, and the existing
guard is a packages/ui story for the isolated ModelPicker no-spin behavior,
not this page-level state. A faithful guard would need either a page-level
Playwright e2e or extracting the clear into a testable hook; happy to do the
extraction in a follow-up if you'd prefer it gated by a unit test.

Verified locally: renderer + storybook typecheck, @maka/ui typecheck, and
biome lint all clean.

简体中文

已确认并修复(c9c8cb3)。根因如你所述:连接快照会经 subscribeEvents → reloadConnections 被外部写入独立刷新,旧逻辑仅在乐观值等于 selectedValue 时清除,导致外部值分歧时乐观遮盖永久粘滞(权威 C、界面 B)。 改为在 onRefresh() 落地后即撤下遮盖,让触发器始终收敛到权威快照,无论其接受的是 本次选择还是外部写入;不引入 spinner。未加专门回归测试:该逻辑位于无单测工装的桌面渲染层, 如需以单测把关,可在后续将清除逻辑抽为可测 hook。

[Astro-Han]

Update on c9c8cb311b:

[P2] Unconditional clear of optimistic leaves stale snapshot on refresh failure

GeneralDefaultsCard clears pendingDefaultModel after await onRefresh() even when reloadConnections swallowed getSnapshot failure or was invalidated without accepted snapshot. Host now holds B but picker reverts to old A with stale isVerified, allowing decisions from wrong default.

Fix: only clear on successful accepted snapshot; handle failure/invalidation.

Checks on c9c8cb311be7ebc75f4d255b1ac7bac1b6d933d4 are test: FAILURE — not green.

简体中文

刷新失败后仍清理导致回退。

[liuxiaocs7]

Good catch — valid, and fixed in a1b900f.

On the P2 (unconditional clear regresses on refresh failure). Confirmed:
onRefresh → reloadConnections swallows a getSnapshot failure (its catch
sets a failed resource state, doesn't throw) and early-returns on
!acceptsConnectionsRead(ticket) / unmount without updating connections. So
await onRefresh() can resolve with selectedValue still on the old value,
and my unconditional clear then reverted the trigger to stale A even though the
save to B succeeded. My earlier === selectedValue guard handled that case
but missed the external-divergence case; the correct fix has to cover both.

Fix: clear the overlay only when an accepted snapshot moves
selectedValue off the pre-pick baseline, not merely when onRefresh()
resolves:

+ const pendingBaselineRef = useRef<string>("");
...
+ useEffect(() => {+ if (pendingDefaultModel !== null && selectedValue !== pendingBaselineRef.current) {+ setPendingDefaultModel(null);+ }+ }, [pendingDefaultModel, selectedValue]);
...
setSaving(true);
+ pendingBaselineRef.current = selectedValue;
setPendingDefaultModel(nextValue);

Resulting behavior:

case	selectedValue after	overlay	shown
this save accepted (B)	B (≠ baseline A)	cleared	B ✓
external write accepted (C)	C (≠ A)	cleared	C ✓ (original P2)
refresh failed / invalidated	still A (= baseline)	kept	B ✓ (this P2 — Host holds B)
setDefaultModel threw	A	cleared in catch	A ✓

A later accepted snapshot (via subscribeEvents) moves the value off the
baseline and clears the overlay, so the failure case self-heals. No-spin
onChange path, instant-label UX, and the failure-path clear are unchanged.

On the red test check. The two failures are unrelated to this
settings-only change — both are flaky e2e on other surfaces:

• composer-plus-menu-stability.spec.ts:262 — locator.click: Timeout waiting
  for the composer Plan menuitemcheckbox to become enabled (bridge-latch
  timing).
• workhub-layout.spec.ts:22 — .workhub-result not visible within 10s
  (WorkHub submit→result timing). Its setDefaultModel calls are in a
  different, passing test and go through the bridge, not this picker UI.

Neither opens Settings or the ModelPicker, and the parent commit d5cb4e8b1
was test: SUCCESS. A settings-page React state change can't affect the
composer/WorkHub surfaces. The re-run on a1b900f should confirm. Locally:
renderer + storybook typecheck and biome lint clean.

简体中文

已修复该 P2(a1b900f)。原无条件清除会在刷新失败/失效时 误回退到旧值(Host 已是 B、界面却退回 A),因为 reloadConnections 会吞掉 getSnapshot 失败并在失效时不更新 connections。改为:仅当被接受的快照把 selectedValue 移离选择前的基线时才清除遮盖——落在本次选择显示 B,落在外部写入显示 C, 刷新失败则保留 B(与 Host 一致),后续快照会自愈。CI 两个失败与本改动无关,是 composer / WorkHub 的既有 e2e flake(父提交 d5cb4e8 为 test: SUCCESS),重跑应恢复。

[Astro-Han]

What this PR does: switching the default model in Settings › 通用 › 任务默认 › 默认模型 spun a spinner on the picker trigger for the whole save. ModelPicker drove the Astryx Selector through its async changeAction prop, whose built-in optimistic value holds the trigger aria-busy until the controlled value catches up — i.e. for the entire setDefaultModel + connection-refresh round trip. This switches ModelPicker to the synchronous onChange path, matching the sibling permission-mode and thinking-level selectors, and drops loading={saving} at the call site. It adds a Storybook regression story that pins a never-resolving save and asserts the trigger is not left aria-busy.

The spinner half is correct and the story is a real guard. But I read the installed Selector implementation and the trade it makes is the opposite of what the PR body describes, so I do not think this closes #3827 yet.

P2 — this removes the spinner by making the trigger label lag, which is the other half of the reported bug

packages/ui/src/model-picker.tsx:100, apps/desktop/src/renderer/settings/general-settings-page.tsx:659

Reachability ①, every switch.

In node_modules/@astryxdesign/core/dist/Selector/Selector.js:

constcommitValue=useCallback(newValue=>{onChange?.(newValue);if(changeAction){startTransition(async()=>{setOptimisticValue(newValue);awaitchangeAction(newValue);});}},[...]);

setOptimisticValue runs only on the changeAction branch. The trigger's label comes from valueContent, which renders selectedItem, which is selectableItems.findIndex(item => item.value === optimisticValue). And the Spinner is rendered as a sibling of valueContent, not in place of it.

So the actual before/after is:

	spinner	trigger label
before	yes, for the whole save	new model, immediately (optimistic)
after	no	old model until setDefaultModel + onRefresh() resolve

disabled={saving} is kept, so during that window the row shows the old model on a disabled trigger — on a slow Runtime Host round trip the pick reads as if it did not take.

#3827 asks for both: "selecting a model reflects the choice immediately with no spinner". This PR delivers the second and gives up the first.

The PR body says it "reflects the pick optimistically in the Settings row so the label updates instantly instead of waiting for the refresh" — that change is not in the diff. general-settings-page.tsx only loses the loading={saving} line; value={selectedValue} is still derived purely from props.defaultSlug / props.connections. Did an earlier revision carry it?

The fix is small and is exactly what the body already promises: a local pending value in GeneralDefaultsCard, preferred over selectedValue while saving, cleared when the refresh lands or the save fails.

Note the new story cannot catch this: it pins value fixed and asserts only not aria-busy, so it stays green whether or not the label ever updates.

Ungraded

ModelPicker has exactly one production consumer (this Settings row) — nothing else in apps or packages renders it. With loading={saving} gone, the loading → isLoading path now has no production caller, while the new comment says "spinning is opt-in via the explicit loading prop". Either keep it and say it is currently unused, or drop the prop.

AI use: Claude Code assisted with reading the installed Astryx Selector implementation; the verification and conclusions are my own.

简体中文

这个 PR 在做什么：Settings › 通用 › 任务默认 › 默认模型 切换模型时，触发器整个保存期间转圈。ModelPicker 用的是 Astryx Selector 的异步 changeAction，其内建乐观值会让触发器保持 aria-busy 直到受控 value 跟上，也就是整个 setDefaultModel + 连接刷新往返。这个 PR 改用同步 onChange（与相邻的权限模式、思考级别选择器一致），并在调用点去掉 loading={saving}，另加一个 Storybook 回归 story。

去掉转圈这半是对的，story 也是真的守护。但我读了安装版 Selector 的实现，它做的取舍与 PR 描述相反，所以我认为还不能算关掉 #3827。

P2：setOptimisticValue 只在 changeAction 分支执行；触发器标签 valueContent 取自 selectedItem，而 selectedItem 由 optimisticValue 派生；Spinner 是 valueContent 的兄弟节点，不是替换它。所以实际是——改之前：转圈，但标签立刻变成新模型；改之后：不转圈，但标签要等保存和刷新落地才更新，且期间 disabled={saving} 让触发器处于禁用态，在 Host 往返慢时看起来像"这次选择没生效"。而 #3827 的 Expected 两者都要。

PR 描述里那句"reflects the pick optimistically in the Settings row so the label updates instantly" 在 diff 里并不存在：general-settings-page.tsx 只少了 loading={saving} 一行，value={selectedValue} 仍纯由 props 派生。是不是早期版本里有、后来掉了？

修法就是描述里已经承诺的那件事：在 GeneralDefaultsCard 里加一个本地 pending value，saving 期间优先显示它，刷新落地或保存失败时清掉。另外新 story 抓不到这个回归——它把 value 钉死，只断言 not aria-busy。

不计分：ModelPicker 全仓只有这一个生产消费者，删掉 loading 后 loading/isLoading 这条通路已无生产调用者，而新注释还写着 "spinning is opt-in via the explicit loading prop"。要么保留并注明当前未使用，要么把这个 prop 一起删掉。

[Astro-Han]

Thanks for carrying the previous feedback into a generation-based read barrier. I reviewed exact head 1f39cccffa6decb65bdae76763c888b5a981d6a8.

The earlier two stale-overlay paths are addressed: an accepted external value no longer depends on equality with the pick, and a failed refresh no longer unconditionally reverts to an old snapshot. One P2 ordering gap remains inline: an accepted post-write read can land before the Promise continuation calls settle(), then be classified into the floor and ignored forever if the explicit refresh fails.

Please bind the barrier to write/read ordering authority rather than only the latest generation observed after the Promise resolves, and add the missing production-order regression. Exact-head test is successful and the PR is mergeable, but this UI behavior still needs human live acceptance before approval.

Review analysis was assisted by Codex and independent @reviewer agents. Astro-Han verified the exact-head diff, prior feedback closure, Settings read/write ordering, production subscription path, and severity before publication and owns this review.

中文对照

谢谢你把上一轮反馈收敛成 generation-based read barrier。之前两条 stale overlay 路径已经处理：外部 authority 不再依赖与 pick 值相等，刷新失败也不再无条件退回旧快照。

当前还剩一个 P2 顺序缺口：写入已经持久化后的 read 可能在 Promise continuation 调用 settle() 前被接受；随后 settle() 把该 generation 纳入 floor，如果显式 refresh 再失败，这个已接受的 authority 会被 pending 永久覆盖。请把 barrier 绑定到 write/read ordering authority，并补对应 production-order 回归。当前 test 成功且 PR 可合并，但 UI 行为在 Approve 前仍需人工实机验收。

本次审查分析由 Codex 和独立的 @reviewer 子代理协助；Astro-Han 核验了精确 head、旧反馈闭环、Settings 读写顺序、生产 subscription 路径和问题分级，并对本次 Review 负责。

[Astro-Han]

[P2 · category ②] The generation captured here is not guaranteed to contain only pre-write reads. setDefaultModel() can persist B and emit a connection event before its Promise continuation resumes; that subscription read may already accept an external C at generation G. This then calls settle(G). If the explicit refresh fails or is invalidated, no accepted generation exceeds G, so the hook permanently keeps pending B over the already-authoritative C. The current tests assume every read before settle returned old A, which is not true of this production ordering. Please carry write/read ordering evidence (or the exact accepted post-write read) into the barrier and add the sequence “post-write C accepted before settle; explicit refresh fails.”

[Astro-Han]

Two additional exact-head findings surfaced while independently checking lifecycle recovery and the current-main merge result. The earlier ordering P2 remains unchanged. Both comments below are technical production/integration defects; no UI acceptance judgment is implied. Review analysis was assisted by Codex and independent @Reviewer agents. Astro-Han verified the exact head, current main, production lifecycle, and severity before publication and owns this review.

[Astro-Han]

[P2 · category ②] A Host generation change resets the local guard and optimistic state, so a new-epoch save can begin immediately, but the old persistDefault continuation is not fenced to that generation. If the old Host request later rejects, this catch cancels the new optimistic selection; its finally also clears the new saving flag, and a late success can arm the new epoch read barrier. Reproduce with Host A save pending → epoch changes → Host B save starts → A rejects/completes. Please capture an operation/generation token and let catch, settle, refresh, and finally mutate state only while that token is current; add the interleaving at the Settings composition boundary.

[Astro-Han]

[P1 · category ①] Removing loading from ModelPickerProps no longer composes with current main. main@4cbe22413 adds packages/ui/stories/model-picker.stories.tsx:352-364, whose SavingDefaultModel story still passes loading; the clean merge tree therefore fails Storybook typecheck with TS2322 at line 362. The exact-head green check predates that main change. Please update or remove the stale loading story while rebasing, rather than restoring the spinner API, then rerun the current-base gate.

[liuxiaocs7]

Thanks — this all landed in 7a8d9f6c3, rebased onto current main and verified end-to-end locally (details at the end). Point by point:

1 — read/write ordering (P2). The floor is sampled at the pick, before the write: ModelPicker.handleChange calls begin(next, getReadGeneration?.() ?? committedGeneration) and only clears when committedGeneration passes that floor. So a read issued after the pick — the row's refresh or a concurrent external write's read — clears to authority even if the explicit refresh later fails; an in-flight pre-write read cannot. Covered by use-optimistic-selection.test.tsx incl. "an after-the-pick read clears to authority even with no explicit refresh".

2 — Host generation race (P2). Replaced the value-based cancelIf with a monotonic operation token: each pick bumps pickTokenRef, and a late write-failure rolls back only if its token is still current. Two picks of the same value across an epoch change therefore can't cross-cancel. I did not key-remount GeneralDefaultsCard — that detaches the element the existing product-settings-pages--general-host-generation-revalidation story asserts on (it waits for the same trigger to go :disabled), and the epoch clearing is already handled by the generation barrier. (The row's saving/guard lifecycle is main's pre-existing code; my only added optimistic state now lives token-guarded in @maka/ui.)

3 — Storybook (P1).SavingDefaultModel and its now-orphaned imports are removed; loading is not restored. The no-spinner + instant-reflect behavior is unit-tested at the @maka/ui layer; a Desktop Settings composition/interleaving story (your #1/#2 as browser stories) is the remaining test-fidelity item — happy to add it if you'd like it as a story rather than the unit coverage.

4 — renderer architecture (P1). Done exactly as you outlined: optimistic hook moved into model-picker.tsx; the read generation is carried on the existing connections resource-state snapshot (no new useState in settings-surface); operation token instead of cancelIf; and no extra useEffect/useRef in the page — so general-settings-page.tsx and settings-surface.tsx return to their baseline hook fingerprint (no renderer-architecture.json edit needed). Also adopts main's LlmConnection → IdentifiedLlmConnection migration.

Verification. I built the merged tree (current main + these changes) and ran your listed gates locally:

• @maka/desktop typecheck (incl. tsconfig.storybook.json) — clean (the earlier TS2345 is gone).
• check-renderer-architecture — checker fixtures (62) pass and "Renderer architecture check passed" (hook fingerprints back to baseline).
• @maka/ui unit tests (7) and biome lint — clean.

CI is running on 7a8d9f6c3. UI live-acceptance (screenshot/recording) is still owed by @liuxiaocs7 — I can't capture Electron here.

[liuxiaocs7]

Follow-up on the Host-epoch race and the two composition tests.

Epoch fence is now synchronous (0fdbe3509). You're right that the keyed remount alone isn't a synchronous fence — between the Host authority's setState and React's commit/unmount, the old save's continuation can still observe mountedRef.current === true and fire a stale toast/onRefresh(). persistDefault now captures the request authority's targetRevision at the start and re-checks it before refresh / toast / setSaving (getRuntimeHostTargetRevision). selectTarget bumps that revision synchronously on any key/epoch change (before React renders), so a superseded save bails immediately rather than relying on unmount. The keyed remount stays (it resets per-epoch state); no page hooks are added — it's a prop getter plus a local closure, so general-settings-page.tsx/settings-surface.tsx keep their baseline hook fingerprint.

On the two composition/interleaving stories. These are still not added, and I want to be transparent about the trade-off rather than quietly ship something fragile. The behavior is currently covered by:

• use-optimistic-selection.test.tsx — the ordering case (an after-the-pick read clears to authority with no explicit refresh), plus A→B→A, refresh-lands-nothing, and rollback.
• the general-host-generation-revalidation story — the epoch retirement (now updated for the remount).
• the synchronous targetRevision fence above — the exact pre-unmount window you identified.

The two browser stories you specified (post-write subscription accepts C + explicit refresh fails; epoch-A pending save → epoch-B save → A resolves) require orchestrating concurrent bridge events, a forced refresh failure, and epoch interleaving while driving the Selector's option-click in the headless smoke. That's precisely the shape of interaction story that has been the flaky-failure source in this suite (the two-client UDS / composer cases), so adding them risks trading a verified-correct fix for a flaky gate.

Could you confirm whether the unit + render coverage plus the synchronous fence is acceptable here, or whether you consider the two browser stories mandatory? If mandatory, I'll add them — I just didn't want to introduce flaky interaction stories without your call.

Verified locally on current main (built the merged tree): @maka/desktop typecheck incl. tsconfig.storybook.json, check-renderer-architecture (passed), @maka/ui tests, storybook render smoke (233), and biome lint all clean. CI is running on 0fdbe3509.