Conversation
Audit of what asr-ggml 0.5.3 offers against what the SDK exposes. The addon's validator allowlists are the real vocabulary — WhisperConfig extends Record<string, unknown>, so its declared keys understate what the engine takes. Parakeet per-segment metadata was blocked outright: assertMetadataSupported rejected every engine but whisper on the premise that only whisper emits it. The parakeet output serializer sends text/start/end/id/toAppend/isEndOfTurn/ startsWord for every segment and timestampsEnabled defaults to true, so parakeet callers could not get timestamps at all. Both engines are now accepted, and the two parakeet-only flags reach callers. Stats went from 15 of the addon's 26 fields to all 26: totalTime, totalWallMs, totalSamples and processCalls (shared); whisperSampleMs, whisperBatchdMs and whisperPromptMs; totalTranscriptions, modelLoadMs, totalEncodedFrames and encoderOnCoreml. Whisper config reaches parity with max_initial_ts, no_speech_thold, seed, miscConfig.seed and backendsDir (parakeet already had the last one). The VAD event carries its detector, so callers can tell whisper's silero VAD from parakeet's energy hint, and ASR_BACKEND_IDS is exported so stats.backendId can be decoded. Backend diagnostics now ride the terminal frame, adopting the shared InferenceBackendDiagnostics mechanism the audiogen op already uses. Parakeet load config, per-call streaming options, vadParams and contextParams were already at full parity and are unchanged.
… 0.9.1 Audit of what bci-whispercpp 0.9.1 offers against what the SDK exposes. The addon is already pinned ^0.9.1, so this is parity work only. Streaming segments lost windowStartTimestep. lib/stream.js attaches it to every segment emitted in `emit: 'delta'` mode, and it is the only thing that maps a segment's window-local timestamps onto the stream timeline — without it a delta consumer cannot place its own output. Stats were diffed against BCIModel.cpp rather than against the SDK's own expectations. The native model emits 15 fields; six never reached callers: processCalls, totalTime, totalWallMs, whisperSampleMs, whisperBatchdMs and whisperPromptMs. Five more were declared and mapped that the model never emits at all — audioDurationMs, realTimeFactor, encoderMs, decoderMs and melSpecMs belong to the asr-ggml engines, so the op was promising a shape that could not arrive. Both directions are corrected. Backend diagnostics now ride the batch terminal frame, reusing the mapping added for asr-ggml. The streaming path is deliberately left alone: the addon does not populate response.stats for streams, so there is nothing to derive a verdict from. detect_language was documented as "not supported natively (rejected by the addon)". It is in the addon's own validWhisperParams; the description was inherited from the asr-ggml schema. Load config and per-call stream options were already at full parity — whisperConfig 16/16, contextParams 4/4, miscConfig 1/1, bciConfig 1/1, and windowTimesteps/hopTimesteps/emit — and are unchanged.
Contributor
License compliance — cleanNo new dependency license findings in this PR. Warn-only (shadow) mode — this check does not block merges yet. Updated automatically by the canonical license compliance workflow. NOTICE presence (advisory)Missing NOTICE (advisory, does not block):
|
GustavoA1604
previously approved these changes
Sep 18, 2026
…he backend ids Review follow-up on QVAC-24486. The previous commit opened assertMetadataSupported to parakeet, but it was unreachable: both parakeet handlers rejected metadata: true before the request got that far, and the duplex handler hard-coded false as the metadata argument. Parakeet timestamps therefore still failed. Both handlers now forward metadata and emit segment frames the way the whisper plugin does, branching the call so the op's `metadata: true` overload types the results as segments. The duplex loop still forwards endOfTurn events: segments carry no `type`, so the event branch only ever catches the typed ones. The earlier tests exercised the guard and the mapper in isolation, which is how this slipped through. parakeet-metadata-handlers.test.ts drives the real handlers with a fake model; against the previous commit it fails with the handler's own "does not support metadata: true" rejection. ASR_BACKEND_IDS was exported to TypeScript only, leaving Python with a bare backend_id and nothing to decode it. constants-registry only emits string enums, so numeric vocabularies now ride contract/numeric-constants.json the way error codes ride error-codes.json: build-numeric-constants.ts writes it, generate.py renders _generated/numeric_constants.py, and the package re-exports it. A Python test pins it to the contract. Also formats transcription-config.ts, which failed the inference pod's format check.
mexxik
previously approved these changes
Sep 18, 2026
Second review round on QVAC-24486. VAD source was added to the schema and the event mapper but never reached a caller: the whisper duplex handler rebuilt the vad frame by hand without it, and both client decoders dropped it again. It is now forwarded at all three. Its description no longer claims parakeet emits 'energy' events; that value is reserved in the addon's VadEvent type and parakeet's energy hint emits none. Diagnostics had the same gap on two fronts. The terminal frames only carried the plain field, so the profiling layer — which reads the diagnostics symbol to set event.backend — got nothing; all four handler sites now attach it as audiogen does. And the client decoded the field and discarded it, so no JS caller could read it. The duplex sessions gain a `diagnostics` promise beside `stats`, settled from the same terminal frame on the same paths. The callback that carries both is renamed onStats -> onTerminal. The new client tests drive createTranscribeStreamSession with raw wire frames in both the inference and SDK clients, rather than testing the mapper in isolation, and each fails against the previous commit. ASR_BACKEND_IDS is now checked against the addon's BackendId with `satisfies` through a type-only import, so a backend the addon adds, drops or renumbers is a compile error rather than a silently short vocabulary. Metadata docs and JSDoc no longer say whisper-only, and two comments that narrated the change are rewritten to state the end state.
Drives each of the four ASR handlers and replays its last chunk into buildOperationEvent — what the profiling wrapper passes as finalResponse — so the diagnostics symbol is shown to reach event.backend rather than only being attached. All four fail against the handlers before the symbol was attached.
…ep reaches callers The batch terminal frame carried diagnostics on the wire but never attached the profiling symbol, so event.backend stayed empty for bciTranscribe. Attach it the same way the ASR handlers do. bci-reachability.test.ts drives the real handlers: the batch terminal must feed buildOperationEvent's backend, and delta-mode segments must keep windowStartTimestep through the handler output. Both fail against the previous handler and mapper respectively. Also document windowStartTimestep on the streaming session JSDoc.
TranscribeStreamSession gained a required diagnostics promise, so the three type-level mocks in duplex-streaming.test.ts no longer satisfied it and the inference typecheck failed.
RamazTs
dismissed stale reviews from mexxik and GustavoA1604
September 18, 2026 15:11
The base branch was changed.
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
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.
🎯 What problem does this PR solve?
Gap audit of
@qvac/bci-whispercpp0.9.1 against the SDK surface. Already pinned^0.9.1, so parity work only — no version bump.windowStartTimestep.lib/stream.js:148attaches it to every segment emitted inemit: 'delta'mode, and the addon documents exactly why: a segment's own timestamps are window-local, so this is the only thing that maps them onto the stream timeline. It appeared nowhere in the SDK, leaving delta-mode output unplaceable.processCalls,totalTime,totalWallMs,whisperSampleMs,whisperBatchdMs,whisperPromptMs.audioDurationMs,realTimeFactor,encoderMs,decoderMs,melSpecMswere declared onBciAddonResponseand conditionally spread, butBCIModel.cppemits none of them — they belong to the asr-ggml engines. Harmless at runtime, but the interface promised a shape that cannot arrive.detect_languagewas documented as rejected by the addon. It is in the addon's ownvalidWhisperParams; the wording was inherited from the asr-ggml schema.📝 How does it solve it?
windowStartTimestepis carried on the shared segment shape, and only when the engine sent it — batch segments do not gain a key that would be meaningless.BCIModel.cppactually emits, and all six missing fields are mapped. The five phantom fields are gone.buildAsrBackendDiagnosticsmapping from QVAC-24486 feat[api]: close the SDK gaps against @qvac/asr-ggml 0.5.3 #4564, and the frame attaches the profiling symbol so they land on the profiling event'sbackend. The streaming path is deliberately untouched: the addon does not populateresponse.statsfor streams, so there is no verdict to derive.detect_language's description now matches the validator.Where the stats and diagnostics reach. They are on the batch
bciTranscribeterminal frame, so RPC consumers see them (Python'sbci_transcribeyields that frame) and profiling picks up the diagnostics. The JS convenience functions do not surface them:bciTranscribe()resolves to text or segments only, and neither BCI stream session (JS or Python) has astatsfield. Exposing them there changes the public return shapes, so it is left as a follow-up rather than done here.Verified already at parity and left unchanged:
whisperConfig16/16,contextParams4/4,miscConfig1/1,bciConfig1/1 (day_idx), and the per-call stream optionswindowTimesteps/hopTimesteps/emit(including thehop < windowrefine that mirrors the addon's own check).🧪 How was it tested?
lunteon both packages: clean.contract:export --checkandsdk-python generate.py --check: both exit 0, with both zod copies pinned to the version CI resolves.bci-schemas.test.ts:windowStartTimestepsurvives the mapper and is absent on batch segments; the terminal frame accepts a diagnostics payload; the full emitted stats surface round-trips.bci-reachability.test.tsdrives the real handlers rather than the schemas: the batch terminal frame feedsbuildOperationEvent'sbackend, and delta-mode segments keepwindowStartTimestepthrough the handler output. Each test fails against the code it guards (the handler without the symbol attach, and the mapper without the field).🔌 API Changes
📋 Follow-ups
bciTranscribe()drops the batch terminal frame's stats and diagnostics. Surfacing them means widening its return type, which is an API decision of its own. The stream sessions have nothing to surface until the addon reports stats for streams.computeWERis exported by the addon and surfaced nowhere. Left out on purpose: it is a scoring helper rather than part of the inference path, so exposing it is an API-design call rather than a parity fix.reload()is not a BCI gap. It exists onBCIInterface— the native binding wrapper exposed as theaddonescape hatch — but the publicBCIWhispercppclass has none, and the SDK's reload path callsmodel.reload(...). Widening the whisper-only reload union would not reach BCI.