Skip to content

refactor(ui): move model selection to Astryx Selector - #1714

Merged
Astro-Han merged 3 commits into
mainfrom
refactor/1565-model-selection
Jul 31, 2026
Merged

refactor(ui): move model selection to Astryx Selector#1714
Astro-Han merged 3 commits into
mainfrom
refactor/1565-model-selection

Conversation

@Astro-Han

@Astro-Han Astro-Han commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Summary

Refs #1565 (Slice 8, flat PR B: model selection/search).

  • Replace the Maka-owned model picker combobox, Popover, query/filter, highlight, scroll, keyboard, focus, listbox, and option authorities with a product composition over Astryx Selector 0.1.9.
  • Keep product ownership limited to provider/model shaping, provider sections and option marks, known or unknown current-value presentation, and persistence actions.
  • Move Thinking Level out of the model popup footer into an adjacent, explicit Astryx Selector while preserving new-chat and active-session persistence.
  • Make AppShell the single session-keyed action owner for both model and Thinking Level mutations. The adjacent controls share one pending state, exclude same-session races in both directions, and leave another session usable while a prior session settles.
  • Use public Selector values as the provider-mark lookup seam instead of relying on Astryx to preserve product-only option fields.
  • Update the composer, chat model switcher, General Settings default-model control, related CSS, copy, contracts, Storybook states, and Electron journeys.
  • Delete the now-unused legacy picker trigger recipe from ui.tsx; no Collapsible, Accordion, Toggle, or Progress implementation is changed.

This is an independent flat boundary: it builds and tests from current main, does not depend on another PR, and does not change CommandPalette, SearchModal, mention popup, PowerSearch, chat turns, tool activity, or task ledger ownership.

Verification

  • npm --workspace @maka/ui test — 285 passed.
  • npm --workspace @maka/desktop test — 2700 passed, including workspace dependency builds and console/a11y/copy checks.
  • npm --workspace @maka/desktop run build — passed.
  • npm --workspace @maka/desktop run typecheck — passed.
  • npm --workspace @maka/desktop run build-storybook — passed.
  • npm run lint — passed.
  • npm run format:check — passed.
  • Targeted Electron Playwright — 5 passed: Settings provider-mark containment/alignment; empty active-descendant behavior; keyboard scrolling; popup hit testing/focus restoration/long-label alignment; and Thinking Level create/reload persistence plus a real active-session model switch.
  • Behavior tests cover same-session model/Thinking exclusion in both directions, cross-session availability during a delayed mutation, failure release/retry, provider grouping/value-to-mark mapping, and unknown/empty current values.
  • Storybook covers default, search, no results, unknown current, empty catalog, pending, adjacent Thinking Level, and the real General Settings empty-model-catalog state.
  • Captured and inspected a real Electron model-selector frame at apps/desktop/test-results/model-selector-live.png (local ignored evidence, not a screenshot baseline). The live DOM/computed-style checks cover trigger/popup hit testing, provider mark/label alignment, long labels, and adjacent Thinking Level alignment.

Capability changes

  • The former pinned-exempt search behavior is intentionally removed. Unknown-current and not-set values are ordinary Astryx options and are filtered like every other option.
  • Astryx 0.1.9 searches option labels, so matching only a provider section heading is no longer supported.
  • Provider marks remain on model options. The trigger-only mark is removed because Astryx optimistically changes its selected label while the controlled product value is still persisting; keeping a separately controlled mark could display a new model label with the old provider identity.
  • Astryx owns its no-result presentation and search accessible name; Maka no longer carries parallel localized empty/search-ARIA copy. The product-owned search placeholder remains localized.

Review focus

Review the ownership subtraction: Maka maps product data and owns session-keyed persistence admission; Astryx remains the sole search, option, optimistic UI, keyboard, focus, scrolling, and popup authority. General Settings keeps its existing settings-save owner, and new-chat selection is synchronous local draft state.

@Astro-Han
Astro-Han force-pushed the refactor/1565-model-selection branch from d42c846 to 4bcd4e5 Compare July 31, 2026 16:59
@Astro-Han
Astro-Han merged commit 6636ec4 into main Jul 31, 2026
3 checks passed
@Astro-Han
Astro-Han deleted the refactor/1565-model-selection branch July 31, 2026 17:07
jackwener pushed a commit that referenced this pull request Jul 31, 2026
…log (#1723)

The plan-reminder edit/duplicate flow defers opening the Astryx form dialog
by two requestAnimationFrames after the row menu closes (the menu stashes an
intent, runs it on close, then remounts a closed form session before Astryx
observes a false->true transition). Astryx Dialog restores focus on Escape
to whatever was `document.activeElement` when it opened, and the menu's own
close-focus-return sits in that same window. On a loaded CI runner the
captured element can land on <body> instead of the menu trigger, so Escape
strands focus away from the row and `expect(menu).toBeFocused()` times out.

This made the `plan-reminders` E2E flaky on docs-only PR #1719 (same code
tree passed on #1714 and failed on #1719; a rerun went green), and the
suite runs with retries: 0 so the flake blocks unrelated PRs.

Keep a ref to each row's menu trigger and re-focus it inside the deferred
intent, right before the dialog opens, so the dialog captures and later
restores focus to the row's trigger deterministically — independent of the
menu-close vs deferred-open micro-timing.

Verified: packages/ui typecheck + 296 contract tests, biome lint, and the
plan-reminders E2E (12/12 with --repeat-each 6).
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.

1 participant