Conversation
|
|
|
Friendly ping for review — this fixes the Soniox span-timing collapse from #6885 (end_time regressing onto a late-timed trailing token). All CI checks (unit, type-check across 3.10/3.13, ruff, Devin review, CLA) are green on the current head. The auto-assigner skipped reviewer assignment, so this may have fallen through the cracks — could a maintainer pick it up or point me at the right reviewer? Happy to update onto latest main if needed. |
…time The endpoint path builds SpeechData from the final accumulator alone. update() took start_time from the first token that carried start_ms and overwrote end_time from every token that carried end_ms. When the leading tokens of an utterance arrive without timing keys and only a trailing token has them, the emitted span collapsed onto that trailing token (e.g. two words in 180ms); a trailing token whose end_ms regressed pulled the end backwards by the same mechanism. start_time is now the earliest start_ms seen and end_time the latest end_ms seen, matching the min/max guards merged_speech_data() already applies on the interim path. Regression tests cover both mechanisms (issue livekit#6885).
294ee46 to
4e649aa
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if "start_ms" in token: | ||
| start_ms = float(token["start_ms"]) | ||
| if not self._has_start_time or start_ms < self.start_time: | ||
| self._has_start_time = True | ||
| self.start_time = start_ms |
There was a problem hiding this comment.
🟡 Late timing still collapses transcript spans
When only a trailing token is timed, _TokenAccumulator assigns that token's start and end to the entire transcript. No earlier start_ms exists, so final spans remain shorter than their text.
Learn more
The accumulator can only minimize timestamps it receives. If every leading token lacks timing, the first observed start_ms still belongs to the trailing token. The new test at test_token_accumulator_span_covers_all_timed_tokens preserves this collapsed 180 ms span, so it passes both implementations and does not verify its stated regression.
Example: Tokens " He's" and " trying" have no timing. Token "." has start_ms=43663 and end_ms=43843. Both implementations emit " He's trying." from 43.663 to 43.843 seconds.
Recommended fix: Define a fallback start source for leading untimed tokens, such as provider speech-onset metadata or another reliable stream timestamp. Track that fallback separately from token timing, and add an end-to-end assertion that the final span begins before 43.663 seconds for this case.
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Fixes #6885
Problem
A
FINAL_TRANSCRIPTemitted at a natural endpoint can carry astart_time/end_timespan far shorter than the speech its owntextrepresents — e.g."He's trying."with a 0.18s span.Root cause
_TokenAccumulator.update()tookstart_timefrom the first token that carriedstart_msand overwroteend_timefrom every token that carriedend_ms:Two failure mechanisms (both confirmed against the fake-WS harness):
start_ms/end_ms, only a trailing token has them → the span collapses onto that trailing token.end_msis smaller than an earlier one pulls the end backwards.merged_speech_data()(the interim path) already guards both withmin(...)/max(...); the final path did not.Fix
start_time= earlieststart_msseen (not just the first)end_time= latestend_msseen (never regresses)Regression tests cover both mechanisms. Verified: 2 of 3 new tests fail on the old code, all 31 soniox tests pass with the fix.