refactor: reshape speech recognizer into transcriber engine with injected components#110
Conversation
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_db41f6b2-8114-4737-aab3-c1f474b1ba05) |
…cted components - rename speech/recognizer -> speech/transcriber (STT; 'detector' stays reserved for future VAD-class components) across code, tests, tools, bench ids, and domain-boundary checks - dissolve speech/recognizer_routes: the transcriber is now a non-templated engine taking a dependencies struct (component kinds, encoder/decoder execution contracts, decode policy, pinned tokenizer sha) and owning speech/encoder, speech/decoder, speech/tokenizer component facades (sm_any + kind enum), mirroring text/generator - promote execution contracts and ASR decode policy to component level; whisper variants alias them and keep binders at the variant boundary - add speech/tokenizer/whisper detokenize machine (route effect -> component actor dispatch, truncate-to-capacity contract preserved) - rewrite transcriber lifecycle tests against the dependencies API - ride-along fixes surfaced by gate scoping: stale sortformer test namespace, docsgen bracket-depth overflow on deep transition tables, clang-format drift (lint baseline strictly shrank, 11 entries removed)
gabewillen
force-pushed
the
cleanup/speech-transcriber
branch
from
4a313e9 to
8123c8a
Compare
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_287e4c08-3d49-4901-8cfc-5a829eb362e0) |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Address PR review findings on the transcriber reshape: - Reject unknown component kinds before dispatch. The transcriber support guards only excluded the `unsupported` sentinel, but sm_any clamps any out-of-range enum value to the first variant, so a deserialized/cast kind with otherwise valid contracts would silently run the default facade. Add a variant-neutral `sm_any::is_supported_kind` predicate, expose it as `is_supported` on each speech component facade, and gate the transcriber guards on it so the engine stays model-family-neutral while rejecting out-of-range kinds. - Validate detokenize spans before decoding. The whisper tokenizer `guard_tokenizer_json_valid` accepted on JSON alone, so a direct component dispatch (bypassing the transcriber's recognize validation) with a non-empty token or transcript span whose data pointer is null would reach UB in the decode action. Extend the guard to require well-formed spans so malformed requests take the error path. - Remove the stray zero-byte a.out build artifact and ignore a.out/*.out. Add guard-level tests covering out-of-range component kinds and null-data detokenize spans.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_b15ebc03-e547-484a-848b-66d2bd81dc49) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Address second-round PR review findings: - Preserve the requested kind in sm_any. Constructing or set_kind-ing a component facade with `unsupported` or an out-of-range cast previously clamped kind() to the first variant, so the facade reported `whisper` and a direct dispatch silently ran the default machine with no way for callers to detect the "no component injected" state. sm_any now keeps the requested kind (the active machine still clamps for safe construction), so is_supported_kind(kind()) stays truthful and owners can reject dispatch to an unsupported facade. Facade comments document the owner-guard contract. - Clear transcript_size_out in the whisper detokenize begin action, mirroring the encoder/decoder begin actions, so rejected requests (invalid tokenizer JSON or malformed spans) never leave a caller reading a stale positive transcript size from a previous dispatch. Add sm_any kind-preservation tests, a tokenizer facade support test, and a machine-level test proving a rejected detokenize clears the caller's out-parameter.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_f0114db3-798f-4f82-963a-f010161caa03) |
Address the P1 Codex review finding: recognize dispatched from state_uninitialized or state_errored was marked uninitialized but routed through the shared recognize error chain, whose terminals always land in state_ready. A failed pre-initialize recognize therefore promoted the machine to ready, and the next recognize could pass guard_transcriber_ready and run the encoder/decoder/tokenizer pipeline without any successful initialize. Give each pre-initialize origin its own explicit rejection chain (uninitialized/errored error-out and error-callback decision states) whose terminals return to the origin state, reusing the existing error-out/callback guards and effects. The shared recognize error chain is now reachable only from ready-origin dispatches and correctly returns to ready. Origin awareness is modeled as distinct states per the actor rules, not context flags. Also address two unthreaded Copilot review findings on the same surface: - guard_model_contracts_supported now rejects injected contracts whose encoder and decoder embedding lengths disagree, since the decode phase slices the shared encoder-state buffer as frame_count x decoder embedding_length. - guard_encoder_success now requires that slice to fit the caller's encoder_state span (and a non-negative frame count), so effect_decode can never hand the decoder a span that reads past the caller's buffer; oversized outcomes route to the backend-error path. Add lifecycle tests proving pre-init and post-error recognizes reject and stay in their origin states with the pipeline unreachable, plus guard tests for the embedding mismatch and capacity bound.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_be4c5190-4620-404c-a09e-3c2389085832) |
|
Addressed three review-body findings that had no inline threads, all fixed in 10d00c8: Codex P1 — "Keep failed pre-init recognizes from entering ready" (first Codex review, Copilot (suppressed) — Copilot (suppressed) — missing encoder/decoder embedding-length cross-check ( Scoped quality gates (whisper_compare bench suite) exit 0. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Address three Codex P2 review findings: - Validate tokenizer JSON and decode policy before initialize succeeds. Add a component-level speech::tokenizer::event::validate asset-validation event and a validate flow in the whisper tokenizer machine (control-token JSON validation plus the same decode-policy support predicate the decoder uses). The transcriber's initialize flow now dispatches this event into the injected tokenizer actor from a new explicit state_tokenizer_validation_decision phase, so malformed tokenizer JSON or a default/unsupported decode policy fails initialization with tokenizer_invalid instead of running encode/decode work on the first recognize and surfacing a generic backend error. The transcriber stays model-family-neutral: validation semantics live in the tokenizer variant and are reached only through the injected component facade. - Reject negative token IDs before detokenizing. Direct detokenize requests containing negative IDs (including INT32_MIN) are rejected in the request guard with a distinct token_ids_invalid error before the decode action runs, and write_i32 now computes the digit magnitude in unsigned arithmetic so the minimum int32 renders without signed-negation overflow. Update the transcriber lifecycle fixtures to carry real whisper-tiny tokenizer JSON and the pinned decode policy as component-level pure data, and add tests for fail-fast initialize (malformed JSON, unsupported policy), negative-token-ID rejection, INT32_MIN rendering, and the validate flow through both the machine and the facade.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_f996a73d-99ea-46f7-83eb-465284b02c4f) |
Summary
Renames
speech/recognizertospeech/transcriber(Whisper is STT — it transcribes; "detector" stays reserved for a future VAD-class component) and replaces therecognizer_routestemplate-policy pattern with the text/generator shape: a non-templated engine that runs whatever components it is given.Rename
src/emel/speech/recognizer{,_routes}→src/emel/speech/transcriber(routes dissolved, see below),tests/speech/recognizer→tests/speech/transcriberTranscriber,SpeechTranscriber), bench backend id (emel.speech.transcriber.whisper), runtime-surface strings, CMake, and domain-boundary checks updatedShape change (components as injected contracts)
speech/transcriberis now a plainsmtaking adependenciesstruct — pure data: component kinds, encoder/decoder execution contracts, ASR decode policy, pinned tokenizer sha. It ownsspeech/encoder/speech/decoder/speech/tokenizercomponent facades and drives them via events. The engine contains no model-family names, contracts, or constants; the domain-boundary script enforces this (nowhispertoken allowed in transcriber source).any.hppwith a kind enum overemel::sm_any, mirroringtext/tokenizer/preprocessor) added for speech encoder/decoder/tokenizer; execution contracts andasr_decode_policypromoted to component level, whisper variants alias them and keep binders (bind_execution_contract) at the variant boundary.speech/tokenizer/whisperdetokenize machine — detokenization was a route effect calling detail functions; it is now a component actor dispatch with explicit validation and callback-decision states (truncate-to-caller-capacity contract preserved).Ride-along fixes (needed for gates to pass on main)
tools/paritychecker/parity_engines.cpp: restore missingparallel_matmul_lanesmember ingeneration_load_state— main does not compile the paritychecker without it (same fix already exists onfeature/moshi-personaplex-e2e)tests/diarization/sortformer/pipeline/lifecycle_tests.cpp: stalemodel::sortformer::detail::namespace reference-fbracket-depth=1024(introspection tool only) for deep transition-table instantiationsnapshots/lint/clang_format.txtstrictly shrank (grandfathered entries removed, none added).planning/architecturemachine docs regenerated (recognizer docs renamed; new transcriber + whisper tokenizer docs)Testing
EMEL_QUALITY_GATES_CHANGED_FILES=<diff> EMEL_QUALITY_GATES_BENCH_SUITE=whisper_compare scripts/quality_gates.sh— exit 0 on this branch: 100% tests (incl. real-model whisper transcriber integration test), whisper.cpp transcript parity exact-match, paritychecker full run, coverage, determinism, lint, docs, domain boundarieswhisper_single_threadperformance contract fails atorigin/main-era baselines (EMEL ~17% slower than whisper.cpp; reproduced at clean checkout with no local changes) — tracked separately for bisectingNotes for review
feature/moshi-personaplex-e2e(branchcleanup/recognizer-route-smell) for later reconciliation with the Moshi/PersonaPlex work.Note
Medium Risk
Large architectural refactor of the speech STT path and initialization/recognize state machines; behavior should be equivalent but wiring and validation timing changed (tokenizer validate on init, detokenize failure path).
Overview
Renames
speech/recognizerandrecognizer_routestospeech/transcriber, drops the route template layer, and wires STT as a plain state machine that owns injectedspeech::encoder/decoder/tokenizerfacades (any.hpp+ kind enums, mirroring text generator dependencies).Component layer: shared
execution_contract,asr_decode_policy, and encode/decode/detokenize/validate events live at component scope; Whisper variants alias them and keep binders at the variant edge.speech/tokenizer/whispergains a full detokenize/validate SM (replacing route effects that called detail directly).emel::sm_anynow preserves requested kind for unsupported sentinels and exposesis_supported_kind.Transcriber pipeline dispatches encode → decode → detokenize through injected actors; initialize runs tokenizer asset validation before model support checks. Domain-boundary and CMake/tests/bench ids move from recognizer to transcriber.
Docs/tooling: new/updated
.planning/architecturefor transcriber and whisper tokenizer; text generator initializer diagram guards tweaked;.gitignorefora.out/*.out; clang-format snapshot shrink; assorted formatting and ride-along fixes called out in the PR description.Reviewed by Cursor Bugbot for commit 26c64a7. Bugbot is set up for automated code reviews on this repo. Configure here.