Repository navigation
feat(dart): add provider-neutral voice capture coordinator - #51
Pushkraj-Space wants to merge 3 commits into
Conversation
harshsaver
left a comment
There was a problem hiding this comment.
Thanks for this, @Pushkraj-Space. The coordinator tracks docs/voice-runtime.md closely, the generation guard and ordered lane are careful, and the scenario fixtures are exactly what #36 asked for. Locally: format/analyze are clean, 68/68 Dart tests pass, and check_conformance.py, Python and TS all pass.
Process: #50 is now merged, so please rebase so this diff is only 0262058. #52 also edits tool/check_conformance.py, so whichever lands second needs a small rebase.
Blocking
-
Warm push-to-talk drops the second utterance on a fast re-press (
lib/src/coordinator.dart:643-670,558-560).finalizePendingis one flag shared across parks.- The sequence: hold →
release()(flush 1 pending) →start()(resume) → speak →release()(flush 2 queued). - When flush 1 completes,
_completeWarmFlushclearsfinalizePending, takes the utterance and armswarmHoldwhile flush 2 is still running at the provider. - The second hold's finals then hit
_gateOpen == false && !finalizePendingand are discarded, and flush 2's completion returns early. - Repro with
_Harness: frame 1 →release()→start(holdToTalk)→ frame 2 →release()→Final('synthetic one')+completeFlush()→Final('synthetic two')+completeFlush(). This delivers['synthetic one']; expected both.
Please track flushes per park (an outstanding-flush count or a per-
_parkWarmtoken), and only clearfinalizePendingor armwarmHoldwhen the latest park's flush settles. Also store the_parkWarmdeadline timer (:648) in a field soresume()andend()cancel it; today a flush timeout that fires after a resume delivers the resumed hold's finals early. Add this sequence as a unit test, and ideally as awarm-rapid-reholdscenario.
Nits (optional)
coordinator.dart:208-211: the broadcastevents/stateChangescontrollers are never closed. Consider adispose().coordinator.dart:489-495: drop-oldest in the pre-roll/backpressure lane is silent. A warning event or counter would help debugging.coordinator.dart:533-538: a throwingfinalize()/setInputGate()is swallowed and reportstimedOut: false. Either document that providers must also emitProviderFailure, or surface the error.coordinator.dart:609-613: a source reportingstoppedduring startup waits forstartup_timeout. Failing fast would be friendlier.
Address the blocking review on october-dev#51 by tracking each warm park separately, settling its tail inside the ordered provider lane, and allowing only the latest park to clear finalizePending or start warmHold. Cancel the stored flush deadline on resume, terminal finalization, and end; ignore duplicate or overdue acknowledgements. Address all four optional comments: add idempotent coordinator disposal, expose droppedFrames in the snapshot, surface provider control failures, and fail promptly when the source stops during startup. Add deterministic regression tests and the shared warm-rapid-rehold scenario. Dart: 80 tests pass; format and analysis are clean. Shared conformance, Python, and TypeScript checks and tests pass.
|
Addressed the blocking issue and all four optional comments:
Process: the local main history includes merged #50 ( Verification: Dart 3.13.3: offline dependency resolution with the lockfile enforced, format check (15 files unchanged), |
|
The rapid re-hold fix remains in place: each warm park owns its flush completion, obsolete deadlines are cancelled, and both utterances survive a fast re-press. The four optional review points are addressed by idempotent The audit's callback regression is now fixed and tested. A throwing The scratch branch has been rebased onto main Verification on the rebased tree plus these fixes: Dart format (15 files, 0 changed), analysis, and all 84 tests pass, including 11 scenario replays. Shared conformance: 15 fixture sets, 44 lines; 11 scenarios, 167 steps. Python: 1 test passes. TypeScript check and 1 test pass using available local dependencies. The committed scratch branch merges cleanly into |
Add VoiceProvider/ProviderSession recognition contracts and the VoiceCaptureCoordinator, the single owner of one capture generation: per-generation session and provider ownership, a synchronous input gate on release, a bounded pre-roll drained through one ordered provider lane, single-flight finalization with an exactly-once utterance callback, warm push-to-talk parking, hands-free endpointing, and idempotent stop. Every wait (startup, endpoint, finalize, shutdown, warm hold) is bounded through an injected Scheduler with a ManualScheduler for deterministic tests; no new dependency. Freeze the lifecycle as language-neutral scenarioSets in the conformance manifest with a documented driver vocabulary and emit contract, replay them in the Dart scenario test, and validate them in the repository checker. All changes are additive to murmur.v1 and the existing fixtures. Closes october-dev#36 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Address the blocking review on october-dev#51 by tracking each warm park separately, settling its tail inside the ordered provider lane, and allowing only the latest park to clear finalizePending or start warmHold. Cancel the stored flush deadline on resume, terminal finalization, and end; ignore duplicate or overdue acknowledgements. Address all four optional comments: add idempotent coordinator disposal, expose droppedFrames in the snapshot, surface provider control failures, and fail promptly when the source stops during startup. Add deterministic regression tests and the shared warm-rapid-rehold scenario. Dart: 80 tests pass; format and analysis are clean. Shared conformance, Python, and TypeScript checks and tests pass.
Address the audit's callback-exception regression in warm and terminal capture. Report original app errors and stack traces asynchronously to the delivery zone without failing capture or swallowing finalization errors. Keep release/finalize completion and exactly-once delivery intact. Add four regression tests for acknowledgements and timeouts, including continued warm capture and late acknowledgements. Document stop failures, callback error handling, dispose(), and per-generation droppedFrames. Dart: 84 tests pass; format and analysis clean. Shared conformance: 11 scenarios, 167 steps. Python and TypeScript checks/tests pass.
37ae0ef to
5647abb
Compare
harshsaver
left a comment
There was a problem hiding this comment.
Thanks @Pushkraj-Space. Re-review of 5647abb: the blocking item is resolved, approving.
- Rapid warm re-hold: I re-ran the original repro (frame 1 -> release -> start(holdToTalk) -> frame 2 -> release -> Final one + flush -> Final two + flush). It now delivers
['synthetic one', 'synthetic two'], clearsfinalizePending, and parks warm-muted until warmHold. A three-press variant also delivers all three. The per-park token (coordinator.dart:692-731), the stored_warmFlushTimercancelled in resume/finalizeTerminal/end, the unit test (coordinator_test.dart:320) and thewarm-rapid-reholdscenario cover it well. - All four nits are addressed.
- Local: dart format/analyze clean, 84/84 tests,
check_conformance.pypasses (11 scenarios, 167 steps).
To merge: #52 and #54 just landed, so this now conflicts in tool/check_conformance.py (both sides add functions; keep both and combine the final summary print) and CHANGELOG.md (keep both). Please merge main and push; I checked all three combined locally (103/103 tests, conformance passes), so it's mechanical. We'll merge as soon as CI is green.
Also heads-up: your #55 adds a competing lib/src/provider.dart (mute()/unmute(), void finalize()) vs this PR's contract (setInputGate, finalize() returning FinalizationOutcome). #55 should build on this one's contract.
Non-blocking, fine as follow-ups:
coordinator.dart:692-710: the second park's deadline starts at release, but its flush can't be sent until the first flush acks and the gate reopens, so a slow first flush uses up the second park's budget and a later final is dropped. Defensible as a total latency bound; a one-line doc comment saying so would help.test/support/fake_connector.dart:98-100usesbroadcast(sync: true), the pattern we just removed from #52's example. Harmless today, but switching to async delivery (or to #54's public fake) would keep the fakes consistent;:10-28also duplicates #54's format/payload helpers.CHANGELOG.md:9-16: nothing has been released, so the "Preserve both utterances..." and "Report utterance callback exceptions..." bullets describe review fixes to an unreleased feature. Consider folding them into the feature entry.
|
Correction to my note above about #55: I had the direction backwards. #55 implements the dedicated provider-contract issue (#7), and its description already says this PR will rebase onto it. #55 is now merged, so please:
Since the rebase changes the coordinator's provider integration, I'll re-review once it's pushed. The approval above covers the coordinator logic as reviewed at 5647abb. |
Closes #36
Stacked on #50 (
issue-34-idiomatic-runtime-interfaces): the coordinator builds onVoiceConnectorandVoiceSession, so this diff includes that commit until #50 merges. Review the top commit (0262058) for the changes specific to this PR.Summary
Adds the two runtime contracts from
docs/voice-runtime.mdto the pure-Dartmurmur_protocolpackage:VoiceProvider/ProviderSession(lib/src/provider.dart): the recognition adapter. SerializedaddFramewhose awaited future is the backpressure signal, a warmsetInputGatedistinct from terminalfinalize, a single-subscriptionProviderEventstream (partial / final / rejected / readiness / amplitude / failure / closed), and idempotentstop.ProviderCapabilitydeclares optional behaviors, each with a defined absent behavior.VoiceCaptureCoordinator(lib/src/coordinator.dart): the single owner of one capture generation. Per generation it connects a freshVoiceSession, opens aProviderSession, gates and finalizes input, assembles the utterance once, and emits orderedmurmur.v1events.CaptureSnapshotis the small immutable view a UI renders.Schedulerseam (lib/src/scheduler.dart):SystemSchedulerfor production,ManualSchedulerfor deterministic tests under virtual time. Nofake_asyncor any other new dependency.No proto or wire-model change.
SessionControl, everyRuntimeEventarm, andSessionState(includingWARM_MUTED) already exist. The existingfixtureSetscorpus and the Python, TypeScript, and Rust runners are untouched.Invariants enforced
release()closes the coordinator gate synchronously. Frames accepted before the gate closes, pre-roll frames buffered while the provider opens, and gate/finalize control calls all flow through one ordered lane to the provider, so nothing reorders and a warm resume cannot reopen the gate before the previous flush completes. Pre-roll is bounded byPreRoll{maxFrames,maxBytes}with drop-oldest.finalize()calls share one future;onUtteranceFinalizedfires at most once per finalization, with rejected and empty finals never buffered.stop()is idempotent before, during, and after startup; repeated calls share one bounded cleanup.CaptureTimeouts{startup, endpoint, finalize, shutdown, warmHold}with typed outcomes: startup timeout is a retryablestartup_timeouterror, finalize timeout is graceful (FinalizationOutcome.timedOut), shutdown timeout detaches cleanup that then never publishes.CaptureReadinessPolicy(default: PCM sample threshold, never "any frame").Modes: tap-to-speak, hold-to-talk (warm push-to-talk when the provider has
warmGate, terminal finalize otherwise), hands-free (finals combined acrossendpoint, one utterance).wakePhraseandunspecifiedare rejected.Shared lifecycle fixtures
conformance/manifest.jsongainsscenarioManifestVersion,scenarioTimeoutsMs, andscenarioSets. Each scenario is a JSONL file of ordered steps: a language-neutral driver vocabulary for the connector/provider boundary (start,release,connect_ok,provider_open_fail,frame,final,provider_closed,advanceMs, …) andemitsteps carrying the exactmurmur.v1RuntimeEventthe coordinator must produce next. The emit contract (1-based per-session sequence, clock-drivenmonotonicTimeUs, session id scheme, explicit default booleans) is documented inconformance/README.mdso a second SDK reproduces the lifecycle byte for byte.Scenarios:
release-during-startup,late-final,empty-final,provider-disconnect,finalize-timeout,rapid-restart,repeated-stop,warm-mute-cycle, plusstartup-timeoutandstartup-failureso every failure step in the vocabulary is exercised.test/scenario_test.dartreplays them through the coordinator with scriptable fakes underManualScheduler;tool/check_conformance.pyvalidates the manifest shape, step vocabulary, eachemitbody via the existingdiagnose(), and the emit contract.Decisions beyond the written plan
warmHoldtimeout; the four planned waits did not bound warm push-to-talk parking.release()during startup cancels the generation (start()completes withcancelled), mirroring the [Core] Finish runtime interfaces on murmur.v1 #34 rule that a terminator wins against a pending start.start()while warm-muted for the same source and mode resumes the generation instead of superseding it.VoiceSessionstoppedwhile listening finalizes the utterance, so file and stored-audio sources end cleanly;errorfails the generation.Verification
dart format --set-exit-if-changed,dart analyze,dart test: 68 tests pass (race matrix intest/coordinator_test.dart, 10 scenario replays, existing runtime and conformance tests).python3 tool/check_conformance.py: 15 sets, 44 lines; 10 scenarios, 145 steps.Acceptance criteria
lib/src/provider.dart,lib/src/coordinator.dartmurmur.v1without Dart-only fieldsemitsteps areRuntimeEventbodies onlyscenarioSets+ emit contractrelease()+ ordered lanefinalize()CaptureTimeoutsstop()🤖 Generated with Claude Code