salmaelsoly commented

Member

Problem

When the user is in listen (wake word) mode and starts a recording, is_listening is never set to false. Both listening and recording run simultaneously.

Impact

The listen processor keeps buffering audio and may send spurious WakeWord messages during an active recording session. The buffer also grows without bound until the connection closes.

File

crates/hero_voice_ui/src/ws.rs:222-243 — the Start handler sets is_recording = true but does not set is_listening = false or reset listen_processor.

## Problem When the user is in listen (wake word) mode and starts a recording, `is_listening` is never set to `false`. Both listening and recording run simultaneously. ## Impact The listen processor keeps buffering audio and may send spurious `WakeWord` messages during an active recording session. The buffer also grows without bound until the connection closes. ## File `crates/hero_voice_ui/src/ws.rs:222-243` — the `Start` handler sets `is_recording = true` but does not set `is_listening = false` or reset `listen_processor`.

salmaelsoly commented

2026-04-22 13:12:27 +00:00

Author

Member

Specification: Fix Start handler not stopping listen mode

Objective

Ensure that when a client sends ClientMessage::Start while in wake-word listen mode, the listener is cleanly stopped so that listening and recording never run simultaneously.

Requirements

ClientMessage::Start must set is_listening = false in addition to is_recording = true.
ClientMessage::Start must reset listen_processor so no stale VAD buffer carries over if Listen is re-entered later.
No change to client wire protocol, no new messages, no new fields.
Behaviour of Stop, Listen, and the binary audio loop is unchanged.
The handler must remain free of inconsistent intermediate states observable by the concurrent binary-audio branch.

Files to Modify

crates/hero_voice_ui/src/ws.rs — update the ClientMessage::Start arm.

Files to Create

None.

Implementation Plan

Step 1: Update the Start handler

File: crates/hero_voice_ui/src/ws.rs

In the Ok(ClientMessage::Start { topic, audio_dir }) arm, immediately after *is_recording.lock().await = true;, add:

*is_listening.lock().await = false;
listen_processor.lock().await.reset();

Ordering rationale:

is_recording = true first — this alone is enough to suppress the wake-word branch in the binary loop (guarded by listening && !recording).
is_listening = false second — makes the state flags consistent and prevents the flag from staying stale until the next Stop.
listen_processor.reset() last — safe to run while is_recording is already true because the binary loop cannot be inside the wake-word branch once recording is true.
Existing audio_processor.reset() and topic_sid assignment follow unchanged.

Each lock is acquired, mutated, and released on its own line — no lock is held across an await on another lock — matching the pattern already used throughout this file. The binary loop acquires locks in the same order (is_recording, then is_listening, then the relevant processor), so no reordering is introduced.

Dependencies: none.

Step 2: Manual verification

No unit test is added. The bug is a state-machine flag inside an async WebSocket handler whose surrounding context (axum::WebSocket, mpsc channels, LocalTranscriber) is not currently factored for unit testing, and extracting a synchronous helper just for two flag writes plus a reset() call would add more indirection than it removes. Verification is manual against the running stack:

Start hero_voice_ui and open the UI.
Click Listen — status reads "Listening for wake word...".
Click Record — status reads "Recording started...".
Server logs show no Wake word detected! lines during the recording window.
Click Stop, then Listen again — wake-word detection still works (proves listen_processor.reset() did not break re-entry).

Dependencies: Step 1 complete.

Acceptance Criteria

ClientMessage::Start arm sets is_listening = false.
ClientMessage::Start arm calls listen_processor.lock().await.reset().
No lock is held across an await on a different lock within the Start arm.
cargo build -p hero_voice_ui succeeds.
cargo clippy -p hero_voice_ui -- -D warnings succeeds.
Manual verification steps above pass.

Notes

Effectively a two-line fix. Spec is deliberately minimal.
The existing !recording guard on the wake-word branch already prevents most user-visible impact, but the state inconsistency and the unreset buffer are real bugs.
Stop already sets both flags to false; this change brings Start into symmetry with it.

# Specification: Fix Start handler not stopping listen mode ## Objective Ensure that when a client sends `ClientMessage::Start` while in wake-word listen mode, the listener is cleanly stopped so that listening and recording never run simultaneously. ## Requirements - `ClientMessage::Start` must set `is_listening = false` in addition to `is_recording = true`. - `ClientMessage::Start` must reset `listen_processor` so no stale VAD buffer carries over if Listen is re-entered later. - No change to client wire protocol, no new messages, no new fields. - Behaviour of `Stop`, `Listen`, and the binary audio loop is unchanged. - The handler must remain free of inconsistent intermediate states observable by the concurrent binary-audio branch. ## Files to Modify - `crates/hero_voice_ui/src/ws.rs` — update the `ClientMessage::Start` arm. ## Files to Create None. ## Implementation Plan ### Step 1: Update the Start handler File: `crates/hero_voice_ui/src/ws.rs` In the `Ok(ClientMessage::Start { topic, audio_dir })` arm, immediately after `*is_recording.lock().await = true;`, add: ``` *is_listening.lock().await = false; listen_processor.lock().await.reset(); ``` Ordering rationale: 1. `is_recording = true` first — this alone is enough to suppress the wake-word branch in the binary loop (guarded by `listening && !recording`). 2. `is_listening = false` second — makes the state flags consistent and prevents the flag from staying stale until the next `Stop`. 3. `listen_processor.reset()` last — safe to run while `is_recording` is already `true` because the binary loop cannot be inside the wake-word branch once recording is true. 4. Existing `audio_processor.reset()` and `topic_sid` assignment follow unchanged. Each lock is acquired, mutated, and released on its own line — no lock is held across an `await` on another lock — matching the pattern already used throughout this file. The binary loop acquires locks in the same order (`is_recording`, then `is_listening`, then the relevant processor), so no reordering is introduced. Dependencies: none. ### Step 2: Manual verification No unit test is added. The bug is a state-machine flag inside an async WebSocket handler whose surrounding context (`axum::WebSocket`, `mpsc` channels, `LocalTranscriber`) is not currently factored for unit testing, and extracting a synchronous helper just for two flag writes plus a `reset()` call would add more indirection than it removes. Verification is manual against the running stack: 1. Start `hero_voice_ui` and open the UI. 2. Click Listen — status reads "Listening for wake word...". 3. Click Record — status reads "Recording started...". 4. Server logs show no `Wake word detected!` lines during the recording window. 5. Click Stop, then Listen again — wake-word detection still works (proves `listen_processor.reset()` did not break re-entry). Dependencies: Step 1 complete. ## Acceptance Criteria - [ ] `ClientMessage::Start` arm sets `is_listening = false`. - [ ] `ClientMessage::Start` arm calls `listen_processor.lock().await.reset()`. - [ ] No lock is held across an `await` on a different lock within the Start arm. - [ ] `cargo build -p hero_voice_ui` succeeds. - [ ] `cargo clippy -p hero_voice_ui -- -D warnings` succeeds. - [ ] Manual verification steps above pass. ## Notes - Effectively a two-line fix. Spec is deliberately minimal. - The existing `!recording` guard on the wake-word branch already prevents most user-visible impact, but the state inconsistency and the unreset buffer are real bugs. - `Stop` already sets both flags to `false`; this change brings `Start` into symmetry with it.

salmaelsoly commented

2026-04-22 13:41:22 +00:00

Author

Member

Test Results

Command: cargo test --workspace

Total: 18
Passed: 18
Failed: 0
Ignored (doctests): 3

Breakdown:

hero_voice lib unit tests: 13 passed
hero_voice_sdk lib unit tests: 5 passed
All other crates: no tests defined

No test was added for this change. The bug is a state-machine flag in an async WebSocket handler with no unit-test scaffolding for the surrounding handler. The spec chose manual verification against the running stack; see the spec comment for the steps.

cargo clippy -p hero_voice_ui --all-targets -- -D warnings also passes.

## Test Results Command: `cargo test --workspace` - Total: 18 - Passed: 18 - Failed: 0 - Ignored (doctests): 3 Breakdown: - `hero_voice` lib unit tests: 13 passed - `hero_voice_sdk` lib unit tests: 5 passed - All other crates: no tests defined No test was added for this change. The bug is a state-machine flag in an async WebSocket handler with no unit-test scaffolding for the surrounding handler. The spec chose manual verification against the running stack; see the spec comment for the steps. `cargo clippy -p hero_voice_ui --all-targets -- -D warnings` also passes.

salmaelsoly commented

2026-04-22 13:42:29 +00:00

Author

Member

Implementation Summary

Changes

Single-file change: crates/hero_voice_ui/src/ws.rs, inside the ClientMessage::Start arm.

Two lines added, plus a short doc comment explaining why:

*is_listening.lock().await = false; — clears the wake-word listening flag, bringing Start into symmetry with Stop (which already clears both flags).
listen_processor.lock().await.reset(); — drops the stale VAD buffer so wake-word detection gets a clean slate the next time the client sends Listen.

Scope preserved

No changes to the client wire protocol, no new message types, no new fields.
Behaviour of Stop, Listen, and the binary audio loop is unchanged.
No lock is held across an await on another lock; lock-acquisition order matches the rest of the file.

Acceptance criteria — status

ClientMessage::Start arm sets is_listening = false.
ClientMessage::Start arm calls listen_processor.lock().await.reset().
No lock held across an await on a different lock.
cargo build -p hero_voice_ui succeeds.
cargo clippy -p hero_voice_ui --all-targets -- -D warnings succeeds.
cargo test --workspace passes (18 total, 18 passed, 0 failed).
Manual verification on the running stack (Listen then Record; no Wake word detected! during recording; Listen re-entry still works after Stop) — to be performed on merge or by the reviewer.

Files changed

crates/hero_voice_ui/src/ws.rs — +5 lines (2 functional, 3 comment).

## Implementation Summary ### Changes Single-file change: `crates/hero_voice_ui/src/ws.rs`, inside the `ClientMessage::Start` arm. Two lines added, plus a short doc comment explaining why: - `*is_listening.lock().await = false;` — clears the wake-word listening flag, bringing `Start` into symmetry with `Stop` (which already clears both flags). - `listen_processor.lock().await.reset();` — drops the stale VAD buffer so wake-word detection gets a clean slate the next time the client sends `Listen`. ### Scope preserved - No changes to the client wire protocol, no new message types, no new fields. - Behaviour of `Stop`, `Listen`, and the binary audio loop is unchanged. - No lock is held across an `await` on another lock; lock-acquisition order matches the rest of the file. ### Acceptance criteria — status - [x] `ClientMessage::Start` arm sets `is_listening = false`. - [x] `ClientMessage::Start` arm calls `listen_processor.lock().await.reset()`. - [x] No lock held across an `await` on a different lock. - [x] `cargo build -p hero_voice_ui` succeeds. - [x] `cargo clippy -p hero_voice_ui --all-targets -- -D warnings` succeeds. - [x] `cargo test --workspace` passes (18 total, 18 passed, 0 failed). - [ ] Manual verification on the running stack (Listen then Record; no `Wake word detected!` during recording; Listen re-entry still works after Stop) — to be performed on merge or by the reviewer. ### Files changed `crates/hero_voice_ui/src/ws.rs` — +5 lines (2 functional, 3 comment).