refactor(voices): row taxonomy + model-driven idempotent render for the voice menus#492
Merged
Conversation
Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01Y1oVsdBKdATjojxQprFhR4
Every interactive element in a voice menu now declares what it IS (custom-voice / restored-voice / door / preview / host-voice); anything unmarked is inert. Replaces the classify-by-absence isBuiltInVoiceButton model whose open-world negative match made every new element a host voice until proven otherwise (the ▶-deselects trap, the #485 crash class). Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01Y1oVsdBKdATjojxQprFhR4
…ctor The base keeps only cross-surface concerns (auth/pref wiring, a gather-then-render refreshMenu, introduceVoice); renderMenu(voices, stored) is the single idempotent render path. GridVoiceSelector owns the button-grid surfaces: row factory, id-scoped reconciliation (no bulk button teardown — the #485 crash class is structurally gone), one-site host-row adoption via the positive taxonomy, and selection as a pure render of the stored preference. The pin field and its render-then-detect-then-re-render recursion die: the stored voice is fetched WITH the catalog and handed to curateShortlist, whose current-first rule pins synchronously. Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01Y1oVsdBKdATjojxQprFhR4
The expand listener collapses to one gather-then-render (TTS-off still adopts host rows, tops up Pi extras, and keeps the door); PiVoiceSettings' four-call constructor sequence becomes refreshMenu(). Existing specs are transformed to drive renderMenu(voices, stored) directly — every product invariant assertion (cap, door, uncapped grid, tier badge, hidden-stored- voice visibility, Pi7/8 survival, stale-pin clearance) is preserved, now without flushAsync pin-recursion choreography. Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01Y1oVsdBKdATjojxQprFhR4
populateVoices(voices, el) becomes renderMenu(voices, storedVoice): the stored voice arrives with the catalog (base gather-then-render), so the trigger label is right on first paint, curateShortlist pins the current voice synchronously, and three mechanisms die — the read-selection-from- button-label block, the async getVoice fallback with its pin writeback and conditional re-populate, and the pinnedCustomVoiceId field. applySelectedVoice shrinks to a pure re-mark (never tears down an open menu). Claude specs updated to drive renderMenu directly; every invariant assertion preserved. Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01Y1oVsdBKdATjojxQprFhR4
Grandfathering (deprecated stored voice renders + selectable), current- voice-never-vanishes under shuffled re-renders, no-silent-swap (incl. the deliberate no-mark-when-hidden improvement), door singularity + placement, adoption idempotence across render + observer paths, and taxonomy completeness (every grid-created control classifies non-unknown). Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01Y1oVsdBKdATjojxQprFhR4
This was referenced Jul 4, 2026
rosscado
added a commit
that referenced
this pull request
Jul 4, 2026
…495) Every open runs renderMenu via refreshMenu, whose fetches resolve from warm in-memory caches in microtasks — which trusted clicks let run BETWEEN the trigger's listener and the document outside-click listener. The wipe-and-rebuild render detached the click's event.target mid- bubble, so contains(event.target) read the opening click as "outside" and closed the menu in the tick it opened. (Masked pre-#492 by the old base refreshMenu's 100ms sleep; invisible to synthetic el.click(), which dispatches without microtask checkpoints between listeners.) The trigger and menu container are now stable nodes: created once, updated/refilled in place (replaceChildren), duplicate defense stays in the constructor. Verified on live claude.ai with a trusted-click CDP probe: repro'd {display:none, aria-expanded:false} on the old build, double-open green on this one. Claude-Session: https://claude.ai/code/session_019NoPTGPfcHjnjkcBWZqR6C Co-authored-by: Claude Fable 5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The same failure kept recurring in
src/tts/VoiceMenu.ts: #485 (bulkquerySelectorAll("button")teardown crashed on #482's nested ▶), the Patch A pin-wipe / Pi7-8 deletion (imperative re-render on shared state), #483 stuck (a row that IS a<button>can't grow a ▶ sibling), and the classify-by-absence trap (any unmarked element read as a host voice — clicking it would unset the user's voice). Each was the code's shape biting, not a one-off. The caution map lists this refactor as a wanted candidate (§6: "the VoiceMenu populate paths → one button factory + SelectionState").What changed
src/tts/VoiceMenuControls.ts(new) — positive, exhaustive taxonomy. Every interactive element declares what it IS (custom-voice/restored-voice/door/preview/host-voice); anything unmarked is inert. Host buttons are adopted (stamped + wired) at exactly one site; the stamp doubles as the wired-once guard, fixing the duplicate-listener-per-expand in the oldregisterVoiceChangeHandler.src/tts/VoiceMenu.ts— slim generic base. Keeps cross-surface concerns only (auth/pref wiring,introduceVoice,ttsRequiresSignIn) plus the new contract:refreshMenu()gathers the catalog AND the stored voice together, then calls the single abstractrenderMenu(voices, storedVoice). Selection/pin state is never mirrored in fields or recovered from the DOM — the stored preference is the model; the DOM is a render of it.src/tts/GridVoiceSelector.ts(new) — reconciling render for Pi's button grids. Row factory + id-scoped reconciliation (insert/move/remove bydata-voice-id; unchanged rows keep DOM identity and listeners). Nothing bulk-removes buttons, so the #485 crash class is structurally impossible, not guarded against. The pin field and its render→detect→re-render recursion die: the stored id goes straight intocurateShortlist, whose current-first rule pins synchronously. The row root is still the<button>today —selectTargetOfis the one seam a wrapper-row future (#483's ▶) changes.Hosts.
PiVoiceMenu/PiVoiceSettingsmove onto the grid (expand listener collapses to one gather-then-render;handleExistingVoiceButtonsand the 3-call expand sequence die).ClaudeVoiceMenumigrates to the base contract but its rendering is deliberately NOT redesigned —populateVoicesbecomesrenderMenu(voices, stored), deleting the read-selection-from-button-label block, the async getVoice fallback + pin writeback, andpinnedCustomVoiceId.VoiceCuration.tsuntouched. No Preact (per doc/preact-component-conventions.md).Deliberate behavior changes (small, both improvements)
Tests
VoiceMenuControls.spec(taxonomy),GridVoiceSelector.spec(17 cases: idempotence, DOM-identity, cap/door, extras survival, adoption, ▶-inertness),VoiceMenuInvariants.spec(grandfathering, current-voice-never-vanishes, no-silent-swap, door singularity, adoption idempotence, taxonomy completeness).renderMenudirectly; every product invariant assertion preserved (and the flushAsync pin choreography deleted).npm test(tsc + Jest + Vitest, 198 files / 1755 tests) ✅ · Layer 3 E2E (13/13) ✅Layer-4 real-host evidence
.expandednever fires, zero SayPi rows, zero console errors, both builds. Filed as pi.ai: SayPi voice block never renders in the in-chat voice menu (expansion/menu detection misses Pi's current DOM) #491 with the full evidence. Decoration itself (call button etc.) verified working with this build on live pi.ai.Unblocks #483; refs #485, #470/#472, #491.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Y1oVsdBKdATjojxQprFhR4