Skip to content

fix(chat): harden voice toggle against first-paint race + SSE-reconnect drift - #37

Merged
webdevtodayjason merged 1 commit into
mainfrom
fix/v3-voice-toggle-ui
May 28, 2026
Merged

fix(chat): harden voice toggle against first-paint race + SSE-reconnect drift#37
webdevtodayjason merged 1 commit into
mainfrom
fix/v3-voice-toggle-ui

Conversation

@webdevtodayjason

@webdevtodayjason webdevtodayjason commented May 27, 2026

Copy link
Copy Markdown
Owner

Summary

The operator reported the dashboard's chat-header voice toggle was not working. The /api/voice/config API path is verified working end-to-end via curl — backend writes voice.json and master broadcasts the SSE voice_config event correctly. The bug is in the client wiring.

Two latent races

1. First-paint race. The click handler read current from window.__subctlVoiceEnabled, populated by an async GET /api/voice/status fired at mount(). If the user clicked before that fetch resolved, the global was undefined → coerced to false. The handler thought voice was off and sent {enabled: true} — but when voice was actually already on, this is a no-op from the server's POV. Operator sees no visible change. Fix: read state from the button's own DOM class (chat-toolbar-btn--active). Operator-visible, can't get out of sync with itself.

2. SSE-reconnect drift. refreshVoiceEnabled() updated window.__subctlVoiceEnabled but did NOT re-render the button. If voice.json changed during a reconnect, the global refreshed but the label stayed stale. Fix: also call renderVoiceBtnState().

Defensive secondary: if the POST response shape ever drifts (no config.enabled key), handler falls back to the requested value — button still flips visually instead of silently ignoring an apparently-successful toggle.

Diff

Single file: `dashboard/public/tabs/chat.js` (+25/-4). No behavior change for the happy path — same URLs, same payloads, same response handling. Purely tightens state-source ordering.

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes

  • Improved voice toggle functionality to ensure UI state accurately reflects server configuration and remains synchronized across interactions.

Review Change Stack

…ct drift

The operator reported the dashboard's chat-header voice toggle was not
working. The /api/voice/config API path is verified working end-to-end
via curl — the backend writes voice.json and master broadcasts the SSE
voice_config event correctly. The bug surface is in the client wiring.

Two latent races that could each present as "the toggle button does
nothing":

1. Click handler read `current` from window.__subctlVoiceEnabled, which
   is populated by an async GET /api/voice/status fired at mount(). If
   the user clicks before that fetch resolves (or if it failed), the
   global is undefined — coerced to false — so the handler thinks voice
   is off and sends `{enabled: true}`. When voice was actually already
   on, this is a no-op from the server's POV and the operator sees no
   visible change. Switch to reading the button's own DOM class
   (`chat-toolbar-btn--active`) as the source of truth — the operator
   can see it, and it can't get out of sync with itself.

2. refreshVoiceEnabled() (invoked on every SSE connect/reconnect) used
   to update window.__subctlVoiceEnabled but did NOT re-render the
   button. If voice.json changed while the dashboard was reconnecting
   the SSE stream, the global would refresh but the visible button
   label would stay stale. Now it also calls renderVoiceBtnState() so
   the chat-header label stays in sync with master.

Defensive secondary: if the POST response shape ever drifts (no
`config.enabled` key), the handler now falls back to the value it
requested so the button still flips visually instead of silently
ignoring an apparently-successful toggle.

No behavior change for the happy path — same URLs, same payloads,
same response handling. Purely tightens the state-source ordering.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 27, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6d1cd3b0-88d7-42fb-87e0-9d4d11bca536

📥 Commits

Reviewing files that changed from the base of the PR and between 194b521 and b42c21b.

📒 Files selected for processing (1)
  • dashboard/public/tabs/chat.js

📝 Walkthrough

Walkthrough

This PR fixes voice toggle state synchronization in the chat interface. The click handler now derives button state from its CSS class instead of relying on a potentially-stale global, the POST response handler trusts server-echoed config, and the refresh function now re-renders the button UI to maintain consistency across all state-update paths.

Changes

Voice toggle state synchronization

Layer / File(s) Summary
Voice toggle click and POST response handling
dashboard/public/tabs/chat.js
Chat header voice toggle click handler derives enabled state from button's chat-toolbar-btn--active CSS class instead of global variable; after /api/voice/config POST, updates global and re-renders button using server-echoed j.config.enabled with fallback to requested state.
Voice status refresh button synchronization
dashboard/public/tabs/chat.js
refreshVoiceEnabled() now calls renderVoiceBtnState() when /api/voice/status returns config, synchronizing chat-header button label and state with server response instead of only updating the global variable.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A rabbit hops through voice states true,
No stale globals lead us astray,
The button now speaks what the server will do—
Each click and refresh finds the way! 🎙️✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(chat): harden voice toggle against first-paint race + SSE-reconnect drift' directly and specifically describes the main changes: fixing the chat voice toggle by addressing two identified race conditions (first-paint and SSE-reconnect drift).
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/v3-voice-toggle-ui

Comment @coderabbitai help to get the list of available commands and usage tips.

@webdevtodayjason
webdevtodayjason merged commit 60c22da into main May 28, 2026
1 of 2 checks passed
@webdevtodayjason
webdevtodayjason deleted the fix/v3-voice-toggle-ui branch May 28, 2026 02:02
webdevtodayjason added a commit that referenced this pull request May 28, 2026
…g + Hermes docs (#40)

Cuts v3.3.4 bundling the three commits since v3.3.3:
- #38 fix(claude-teams): PATH-shim collision + master→evy import leftovers
- #37 fix(chat): voice toggle first-paint race + SSE reconnect drift
- #39 chore: Hermes research docs + ORCHESTRATION log + .codegraph gitignore

Per the version-cap doctrine, stays on the v3.3.x patch line — no v3.4 / v4
bump until operator decides.

Supersedes the stale `fix/policy-snapshot-evy-rename` branch (commit
557b19f), which documented only the `_write_snapshot.ts` half of the
broken-imports breakage. The shipped v3.3.4 fix covers both `.ts`
import sites plus the six `bin/claude-*` shims.

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
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