Skip to content

feat(ui): let the prompt-cache notice be acknowledged per Session - #5496

Merged
Astro-Han merged 2 commits into
apache:mainfrom
chihumyum:feat/model-switch-warning-dismiss
Sep 19, 2026
Merged

Astro-Han merged 2 commits into
apache:mainfrom
chihumyum:feat/model-switch-warning-dismiss

Conversation

@chihumyum

@chihumyum chihumyum commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Summary

The in-session model switcher leads its list with the prompt-cache notice from #2945 (restored as a Selector row in #5444) as a disabled first row. It comes back on every open and nothing dismisses it, so a user who has read it once keeps paying a row of the popup for it in every session with history.

The row is now a regular option whose activation acknowledges the notice for that Session while the switcher remains mounted:

  • Click, Enter on the highlight, or a screen reader's activate all dismiss it. A close button nested in the row was the obvious approach and is not reachable: Astryx Selector option children are presentational, Tab closes the panel, arrow keys skip disabled rows, and Button/IconButton do not forward tabIndex/onMouseDown. The row as its own acknowledgement is operable by every input.
  • Selecting any option closes the Selector, so the acknowledgement bumps a nonce and the remount keyed on it lands open via isDefaultOpen — the same path openNonce recovery already uses. The pending flag is consumed right after that mount, so a later fresh mount in the same Session (a presentation flip) does not open on its own.
  • Acknowledgements are kept per Session while ChatModelSwitcher remains mounted, so direct Session switches preserve them. Visiting home/new-task unmounts the switcher and clears this local state; returning from there shows the notice again. The sentinel value never reaches the pending selection or onChange.
  • The notice label is passed as a node so it wraps as a sentence; the string path from feat(ui): restore list pickers for model and thinking selection #5444 start-ellipsized it like a model id. Its description line is the new switchWarningDismiss copy ("Select to dismiss" / "选择即可关闭" / "選擇即可關閉").

Refs #2939 (the notice's original design).

Verification

  • npm run astryx:surface-inventory — passes; generated inventory includes SelectorOption. Inventory tests — 19/19.
  • @maka/ui build and the focused notice regression test — pass. Scoped Biome lint, repository format:check, and git diff --check — clean.
  • New packages/ui/src/__tests__/chat-model-switcher-notice.test.tsx (linkedom): the notice is the first, activatable row; clicking it removes it without a model switch and the list is open again; a Selector remount in the same Session stays acknowledged while the switcher remains mounted; another Session raises its own; switching directly back to the first stays quiet. Fails on main at the notice row is activatable, passes here.
  • @maka/ui test:dist — 528/528.
  • apps/desktop typecheck:stories and tsc -p tsconfig.renderer.json --noEmit — clean. biome lint — clean.
  • Storybook Product/Model Picker › ExistingConversation play, en and zh-CN: 20/20 steps, now acknowledging via {ArrowDown} + {Enter} (the pointer path is the unit test), then asserting the list is open, the model rows remain, the trigger still names the current model, and close/reopen stays quiet. smoke:storybook — 380 stories / 412 renders pass.
  • Manually in the desktop dev app on macOS: a session with history shows the notice; activating it removes the row and leaves the list open; reopening the picker stays quiet; switching models still works.
  • Not run: Electron E2E. The bottom-sheet presentation was not exercised by hand; it takes the same remount path as openNonce recovery.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code implemented the switcher change, copy, test, story update, and ran the behavior verification under the author's direction and review. Codex regenerated the Astryx inventory, clarified the acknowledgement lifetime and remount documentation, and verified the inventory check/tests, UI build, focused notice test, lint, and formatting.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

The in-session model switcher leads its list with the prompt-cache
notice (apache#2945, restored as a Selector row in apache#5444) as a disabled first
row that comes back on every open; nothing dismisses it.

The row is now a regular option whose activation acknowledges the notice
for that Session. A nested close button would be unreachable: Selector
option children are presentational, Tab closes the panel, and arrow keys
skip disabled rows. Activating the row instead reaches a click, Enter on
the highlight, and assistive activation alike. Selecting any option
closes the Selector, so the acknowledgement bumps a nonce and the remount
keyed on it lands open through `isDefaultOpen`, the path `openNonce`
recovery already uses; the pending flag is consumed after that mount so
a later fresh mount in the same Session stays closed. Acknowledgements
are kept per Session for the life of the switcher, and the sentinel never
reaches the pending selection or `onChange`.

The notice label is passed as a node so it wraps as a sentence instead of
start-ellipsizing like a model id. New copy `switchWarningDismiss` in all
three locales; the story's play test now acknowledges via ArrowDown and
Enter, and a linkedom test covers the click path, the reopen, and the
per-Session memory.
@github-actions github-actions Bot added the effort/M Under 500 readable lines label Sep 18, 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.

Verified the mechanism end to end against the real Selector source — clean.

  • Sentinel guard holds on every activation path: click / Enter / Space / SR activate all funnel through selectItem → commitValue → onChange, which is the new onSelect wrapper — the sentinel is intercepted before selection.onChange, and even past that the choiceByValue lookup returns undefined and early-returns. Real values always contain : so no collision.
  • Nonce-remount is correct: the post-select close lands on the instance being unmounted anyway; the fresh Selector's mount-only effect sees reopen.pendingsurface.show(); consumption is idempotent so StrictMode double-mount can't reopen twice; isReadOnly still wins (acknowledgement dismisses without reopening).
  • Discoverability is adequate: warning-tinted glyph + wrapped sentence label + the switchWarningDismiss description + X endContent — visually distinct from model rows, and search filtering uses the string label so a model query filters the notice out (can't hit it accidentally mid-search). ArrowDown→Enter acknowledging is strictly better than main, where that chord hit a disabled dead-end.
  • Tests are real: linkedom mounts the actual switcher through the real Selector/combobox — fails on main twice over, no fixtures.

P3s:

  • Acknowledgement doesn't survive the no-session round-trip: ChatModelSwitcher unmounts whenever activeSession is undefined (composer.tsx renders NewChatModelPicker instead), so session-A → home/new-task → session-A re-raises the notice once. Trivial and arguably even desirable, but the code comment's "returning to a Session does not re-raise" overstates the guarantee — either reword or hoist the set to the Composer.
  • Double-click hazard: the second click of a fast double-click lands on the reopened list where the first model row now sits under the cursor → unintended (but visible and reversible) model switch. Inherent to self-removing rows; just worth knowing.
  • Remount artifacts: loses search query/highlight/scroll, replays the popover entry animation, and SRs get a fresh listbox with no "dismissed" announcement — all inherent to the openNonce workaround since Selector has no closeOnSelect opt-out.
  • Nit: the openNonce doc comment doesn't mention the key's new third segment.

@Astro-Han
Astro-Han merged commit 0117d76 into apache:main Sep 19, 2026
1 check passed
@chihumyum
chihumyum deleted the feat/model-switch-warning-dismiss branch September 19, 2026 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/M Under 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants