ScriptLine: accept empty text as "model produced no transcript" signal#520
Merged
Conversation
Distinguish ``text=""`` (model was called, returned no transcript — a
real signal) from ``text=None`` (field absent / model never called).
The earlier validator used falsiness (``not values.get("text")``),
treating both the same and rejecting any ``ScriptLine`` whose text was
empty even when its speaker was also unset.
In practice this only bit ASR backends honest enough to emit empty
output on unintelligible / silent audio. The trigger we observed: IBM
Granite-Speech (``ibm-granite/granite-speech-3.3-8b``) on heavily-
enhanced short clips (1.4–3.4s after speech enhancement) — it
correctly decoded "no speech" and emitted ``{"text": ""}``, which the
``ScriptLine`` validator then refused with ``"At least text or speaker
must be provided."``. Less-honest ASR backends (Whisper hallucinates
filler on silence, the LLM-augmented Canary-Qwen / Qwen3-ASR pass the
prompt's implicit "say something" pressure through and almost always
emit text) silently slipped past the same validator by producing a
non-empty string. The vulnerability lives in every backend that
constructs ``ScriptLine(text=...)`` directly — Granite was just the
only one that surfaced it in this dataset.
There was also a subtle inconsistency: a whitespace-only input like
``text=" "`` survived the validator (``" "`` is truthy) and was then
stripped to ``""`` by the field validator, while an explicit ``""``
was rejected up front. The new presence-based rule (``text is None
and speaker is None``) treats both consistently.
Negative-timestamp validation and the whitespace-stripping field
validator are unchanged.
Tests in ``src/tests/utils/data_structures/script_line_test.py``
cover the Granite-style empty-transcript path, the whitespace-normalize
case, both-fields-absent rejection, speaker-only construction, and
preservation of existing happy-path behavior (timestamps, stripping,
``from_dict``).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the ScriptLine data structure to allow empty strings for text and speaker fields, distinguishing them from None to represent valid empty transcripts. Comprehensive tests were added to verify this behavior. Feedback highlights an inconsistency in the from_dict method where an internal helper still uses truthiness checks, which would cause empty strings to be filtered out during deserialization. It is recommended to update this helper and add tests for nested chunks to ensure consistent behavior.
#1 (high) — `_is_meaningful` filter in `from_dict` still used truthy checks (`bool(c.get("text"))`), which silently dropped `{"text": ""}` child chunks even though the root validator now accepts them. That defeats the PR's whole point for payloads that pass through `from_dict` (subprocess IPC, saved JSON, MMS-aligner outputs that go through the dict shape). Switched to `is not None` so the filter only drops children whose text AND speaker are wholly absent / None — the same invariant the root validator enforces. Comment block updated to explain why empty-text chunks must survive and what the filter is now guarding against. #2 (medium) — added two tests that would have caught the inconsistency above: - test_from_dict_preserves_empty_text_in_nested_chunks: payload with `chunks=[{"text": ""}]` survives `from_dict` and the empty chunk is preserved. - test_from_dict_still_drops_chunks_missing_both_text_and_speaker: the original `_is_meaningful` purpose (filtering placeholder children that would trip the root validator) is preserved — only its over-aggressive truthiness behavior is loosened. Tests: 13/13 pass (was 11; +2 new). Pre-commit clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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.
Summary
Stop rejecting
ScriptLine(text="")as invalid when itsspeakerfield is also unset. Empty text is a valid "the ASR was called but produced no transcript" signal — distinct fromtext=None("the ASR was never called"). The pre-change validator collapsed both into a single failure case, which caused legitimately-empty ASR output to crash the audio-analysis pipeline.Why
Surfaced during a full-pipeline run on a b2ai pediatric voice subject. Out of 92 audio clips, 5 failed with:
All 5 were the same shape: short clips (1.4–3.4 s) on the
enhanced_16kpass where Granite-Speech correctly decoded "no recognizable speech" and emitted{"text": ""}. TheScriptLinevalidator then rejected the construction, the per-clip ASR result was lost, and the pass's PII / alignment / corroboration logic ran with one ASR backend silently missing.The vulnerability is shared across every senselab ASR backend (
nemo.py:145,granite.py:142,canary_qwen.py:191,qwen.py:223,huggingface.py:284via.from_dict()) — each one constructsScriptLine(text=...)directly. Granite-Speech happens to be the one that triggers it in this dataset because it's the most honest of the bunch: when its acoustic model decodes nothing, it returns"". Whisper hallucinates filler ("Thanks for watching!", "..."), and the LLM-augmented Canary-Qwen and Qwen3-ASR's instruct-tuned heads almost always emit something under their "Transcribe the following:" prompt, so they slip past the same validator. The bug is inScriptLine, not Granite.The fix
src/senselab/utils/data_structures/script_line.py:78-93— single-line semantic change from a falsiness check to a presence check:In Python
not ""isTrue. The previous rule rejectedtext=""exactly the same astext=Noneor missing-entirely. The new rule treatstext=""andtext=" "andtext="hello"as all "provided" — onlyNone(or absent) counts as "missing." Whitespace-only strings remain valid: the existing field validator strips them, sotext=" "andtext=""converge on the same final state. (Previously they took different validator paths andtext=" "survived whiletext=""did not — a real inconsistency this also closes.)Behavior preserved unchanged:
ScriptLine(text="hello")— works (it always did).ScriptLine(speaker="spk1")— works (diarization-only line).ScriptLine()with both fields absent or bothNone— still raises.Behavior change
ScriptLine(text="")now constructs cleanly (was raisingValidationError).ScriptLine(text="", start=0.0, end=1.5)now constructs cleanly.ScriptLine.from_dict({"text": ""})now constructs cleanly.Downstream consumers (
global_summary, the harvesters, the cross-ASR corroboration inpii.py) all already handle empty / whitespace text gracefully — they filter it from_build_full_text, they checkif t.strip()before scanning, they drop empty utterances from voting. So letting an emptyScriptLineexist in the report just means the "Granite was called on this clip and decided there's no speech" signal is preserved rather than discarded. No downstream code needs updating.Tests
New file
src/tests/utils/data_structures/script_line_test.py— 11 tests covering both directions of the rule.Empty / whitespace / None text handling:
test_empty_text_without_speaker_is_accepted—ScriptLine(text="")now constructs.test_whitespace_only_text_is_accepted_and_normalized_to_empty—text=" "survives validation, gets stripped to"". Fixes the inconsistency where" "was previously accepted but""wasn't.test_text_none_without_speaker_still_rejected—ScriptLine(),ScriptLine(text=None), andScriptLine(text=None, speaker=None)all still raise.Speaker-only / mixed:
test_only_speaker_provided_is_accepted—ScriptLine(speaker="spk1")works (diarization line with no transcript).test_empty_text_with_speaker_is_accepted—ScriptLine(text="", speaker="spk1")works (speaker turn detected, ASR returned nothing for it — common at silence boundaries).Happy-path regression:
test_normal_text_unchanged—ScriptLine(text="hello world")constructs and preserves the text.test_text_stripped_of_surrounding_whitespace—text=" hello "strips to"hello". Unchanged behavior; asserted to guard against future regressions.from_dict:test_from_dict_accepts_empty_text_entry—ScriptLine.from_dict({"text": ""})now constructs.test_from_dict_rejects_dict_with_no_text_or_speaker—ScriptLine.from_dict({})still raises.Field-validator interaction:
test_empty_text_with_timestamps_is_accepted—ScriptLine(text="", start=0.0, end=1.5)works.test_negative_timestamp_still_rejected_independent_of_text— timestamp validator stays orthogonal;ScriptLine(text="", start=-1.0)still raises.All 11 pass under
--noconftest. Pre-commit (ruff + mypy + codespell) clean.The existing
forced_alignment_test.py(the only other test that constructsScriptLinevia.from_dict) continues to pass — it uses a real fixture, not an empty-text edge case.Test plan
uv run pytest src/tests/utils/data_structures/script_line_test.py --noconftest— 11 / 11 pass.Granite.transcribe_with_granite_speech(audio)returning[ScriptLine(text="")]from the worker no longer raises at the host-side construction site.bash scripts/submit_subject_audio.sh /path/to/dataset sub-XXXon a subject that previously had Granite failures and confirm exit code 0 across all clips, with the affected clips'summary.jsonnow showing Granite-Speech asokrather than dropped.Out-of-band reviewer notes
Why this is a
ScriptLinechange, not agranite.pychange. Could fix this in the Granite wrapper alone (e.g.,granite.py:142filters empty-text and returns[]instead of[ScriptLine(text="")]). That would silence the symptom for Granite but leave the latent vulnerability in every other backend — the moment a future Whisper / Canary / Qwen build returns an empty transcript the bug resurfaces. Fixing it once in the validator addresses the class of bug.Why empty is a meaningful signal. "ASR was called, returned nothing" is genuinely different from "ASR was never called." The first is evidence that the audio doesn't contain decodable speech according to this model; multiple ASRs returning empty on the same clip is a corroborated "no-speech" signal. Collapsing both into
text=None/ construction-failure throws away that distinction. The pipeline's existing cross-ASR-model corroboration logic (used bypii.pyand indirectly byglobal_summary) is happier with emptyScriptLines than with missing ones.No call-site changes needed. Every
ScriptLine(text=...)construction site I traced (5 insenselab/audio/tasks/speech_to_text/, plustalk_bank_helpers.py) was already passing along whatever the upstream model returned. The change just allows the empty case through; it doesn't require existing producers to do anything new.Branch is independent of
feat/pii-subprocess-venv— they touch unrelated files and either can merge first. They're filed offalphaseparately to keep the diffs and blame surfaces clean.Version
Published prerelease version:
1.3.1-alpha.33Changelog
🐛 Bug Fix
Authors: 3