Skip to content

QVAC-23965 feat[api]: describe whisper + parakeet modelConfig fields - #4068

Open
lauripiisang wants to merge 1 commit into
mainfrom
QVAC-23965-describe-transcription
Open

QVAC-23965 feat[api]: describe whisper + parakeet modelConfig fields#4068
lauripiisang wants to merge 1 commit into
mainfrom
QVAC-23965-describe-transcription

Conversation

@lauripiisang

Copy link
Copy Markdown
Contributor

🎯 What problem does this PR solve?

  • The whisper and parakeet transcription load-time modelConfig fields shipped without descriptions, so the generated contract consumed by qvac configure and the generated clients couldn't say what any field means.

📝 How does it solve it?

  • whisper (whisperConfigSchema): adds .describe() to the scalar fields and the nested vad_params, contextParams, miscConfig, plus audio_format and vadModelSrc. Text sourced from @qvac/asr-ggml docs/whisper-addon-help.md / README.
  • parakeet (parakeetRuntimeConfigSchema): adds .describe() to every field, sourced from @qvac/asr-ggml docs/PARAKEET-README.md; the two fields that carried only JSDoc (streamingEnergyVad, language) are converted to descriptions so they reach the contract.
  • Regenerates contract/schema.json and the Python generated models.

Part of QVAC-23933.

Flagged (needs addon-owner input)

11 whisper fields map 1:1 to upstream whisper_full_params and have no human-readable semantics documented in-repo, so they are intentionally left without a description (in-code TODO):

no_context, single_segment, print_realtime, print_timestamps, token_timestamps, thold_ptsum, split_on_word, max_tokens, debug_mode, suppress_blank, length_penalty.

The 9 deprecated parakeet ONNX placeholder fields are also left as-is (they only exist to raise a structured migration error).

🧪 How was it tested?

  • bun run contract:export regenerated; contract:check, typecheck, lint, format, and full test:unit (incl. contract-export freshness) pass.
  • packages/inference typecheck + lint pass.
  • packages/sdk-python generate.py --check passes.

🔌 API Changes

Additive only:

whisperConfigSchema.shape.language.description
// "Transcription language (ISO 639-1) or 'auto' to detect."

@lauripiisang
lauripiisang requested review from a team as code owners August 25, 2026 15:13
@github-actions

Copy link
Copy Markdown
Contributor

Review Status

Current Status: ❌ PENDING
Approvals so far: none

Pending reviews: Needs 1 Management or Team Lead, and 1 more from Management, Team Lead, or Member.

@github-actions

Copy link
Copy Markdown
Contributor

License compliance — clean

No 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):

  • ./.github/actions/release-merge-guard
  • ./docs/website
  • ./packages/ggml-coload-smoke
  • ./packages/fabric/test/integration
  • ./packages/inference-addon-cpp/mobile
  • ./packages/sdk/e2e
  • ./packages/llm-llamacpp/benchmarks/performance
  • ./packages/llm-llamacpp/benchmarks/server
  • ./packages/vla-ggml/sim/server
  • ./packages/embed-llamacpp/benchmarks/performance
  • ./packages/embed-llamacpp/benchmarks/server
  • ./packages/asr-ggml/benchmarks/server

backendsDir: z
.string()
.optional()
.describe('Root directory for dynamically-loaded ggml backend `.so` files. Defaults to `prebuilds/`.'),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Both SDK Pod Checks runs fail the packages/inference format check on this file, which also causes Merge Guard to fail.
  • Please run the package formatter and commit its output so this PR can pass CI.

.int()
.nonnegative()
.optional()
.describe('ASR encoder left-context window in ms; `-1` keeps the model default (10000).'),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • This description says callers can pass -1, but .nonnegative() rejects that value before it reaches the addon. The same mismatch exists on streamingRightLookaheadMs below.
  • The addon itself uses -1 when these fields are omitted, so either allow -1 in both schemas or document that callers must omit the fields to retain the model defaults.
  • Please add coverage for the chosen contract so the generated TypeScript and Python clients do not advertise an unusable value.

samples_overlap: z
.number()
.optional()
.describe('Overlap between consecutive speech segments (0–1).')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • This advertises an inclusive 0–1 range, but the streaming driver resolves samples_overlap with vadParams.samples_overlap || defaults.samplesOverlap; an explicit 0 therefore silently becomes the default overlap.
  • Please either resolve the driver value with ?? so zero works, or change the documented/validated range if zero is intentionally unsupported, with a regression test for streaming config construction.

- Add .describe() to whisperConfigSchema scalar/nested fields (top-level, vad_params, contextParams, miscConfig, vadModelSrc) sourced from @qvac/asr-ggml whisper-addon-help.md / README
- Add .describe() to every parakeetRuntimeConfigSchema field sourced from @qvac/asr-ggml PARAKEET-README.md; existing JSDoc-only fields (streamingEnergyVad, language) converted to describes
- 11 whisper fields map 1:1 to upstream whisper_full_params with no in-repo semantics (no_context, single_segment, print_realtime, print_timestamps, token_timestamps, thold_ptsum, split_on_word, max_tokens, debug_mode, suppress_blank, length_penalty) — left undescribed with a TODO for the addon owner
- Regenerate contract/schema.json and the Python generated models
@lauripiisang
lauripiisang force-pushed the QVAC-23965-describe-transcription branch from 1d1c233 to 9ce3671 Compare August 26, 2026 14:38

@GustavoA1604 GustavoA1604 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Field descriptions (all map 1:1 to whisper_full_params):

  • no_context — Do not carry past transcription forward as the decoder's initial prompt; each window is decoded independently. Avoids earlier text biasing later chunks and repetition-loop carryover.
  • single_segment — Force the whole audio into one output segment. For streaming/short clips where you don't want splitting.
  • print_realtime — whisper.cpp prints results to stderr as it decodes. Upstream advises against it; use the segment callback instead. Diagnostic only.
  • print_timestamps — Only affects print_realtime output: prefixes each printed line with [t0 --> t1]. No effect on returned data.
  • token_timestamps — [EXPERIMENTAL] Compute per-token timestamps (populates t0/t1 in whisper_token_data). Needed for word/token-level timing.
  • thold_ptsum — Timestamp-token sum-probability threshold (~0.01) used when deriving token-level timestamps. Handler enforces 0 ≤ x ≤ 1.
  • split_on_word — When max_len > 0, split segments on word boundaries instead of mid-token.
  • max_tokens — Max tokens per segment; 0 = no limit. Handler enforces ≥ 0.
  • debug_mode — [EXPERIMENTAL] Extra debug output (e.g. dumps the log-mel). Off in normal use.
  • suppress_blank — Suppresses the blank/leading-space token at the start of sampling (OpenAI decoding.py behavior).
  • length_penalty — Beam-search length penalty. Note: upstream default -1.0 means "disabled", but our handler rejects negatives (≥ 0 required), so the disabled sentinel can't be set via config — default stays -1 unless a caller passes ≥ 0. Worth noting in docs.

About detect_language:

  • Confirmed rejected natively — there is no detect_language handler key in WHISPER_MAIN_HANDLERS; unknown keys throw InvalidArgument (WhisperConfig.cpp:48).
  • Intentionally not exposed: auto-detection is driven by language: "auto", which sets language = nullptr and keeps detect_language = false on purpose (WhisperHandlers.cpp:249-256).
  • Setting detect_language = true makes whisper_full return right after detecting the language and emit no segments — that's why it's suppressed.
  • Action: don't document it as a config field; document language: "auto" as the way to auto-detect.

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.

3 participants