TT-7666 fix: PBT Transcribe shows duplicate audio after a segment boundary adjustment - #574
Closed
nabalone wants to merge 7 commits into
Closed
TT-7666 fix: PBT Transcribe shows duplicate audio after a segment boundary adjustment#574nabalone wants to merge 7 commits into
nabalone wants to merge 7 commits into
Conversation
… step
A team can configure one Phrase Back Translation step per language, and
every one of them renders the same PassageDetailGuidedPhraseRecord
against the same vernacular audio. The mobile route rendered that
component unkeyed, so moving between two PBT steps reused the instance -
and the component resets its per-step state only when the vernacular
mediafile changes, which it does not here. The recording pass, the
segment it was parked on, the baseline boundaries, the optimistic greens
and the take mounted in its recorder all carried into the next
language's step, which then played the previous language's audio.
The route now keys the guided-record steps on currentstep, the way the
desktop grid already does with <Paper key={currentstep}> and the way the
adjacent transcription steps were keyed for TT-7503. The component also
scopes its own reset to the step as well as the mediafile, so a reused
instance still opens the next step from scratch.
The legacy claim had a second cross-language path: it stamped every
untagged take of its artifact type with the opening step's language.
That premise only holds for data recorded before takes carried a
language, in a project working in one language. Once the passage has
Phrase BT boundaries for another language an untagged take could belong
to either step, so the claim is now skipped there rather than handing
the take to whichever step happened to open.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rs in the PBT harness parseMediaLanguageBcp47 and phraseBtBoundaryRegionName already own the `Name|bcp47` parse and the `BT:<bcp47>` bucket name; the harness had grown its own copies of both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t changes useGuidedPhraseSegments guarded its own reset on the mediafile id, so a step change on the same vernacular was rejected as a no-op: bootstrapped stayed true and ensureSegments short-circuited on the phrase string already loaded. The next language's step opened on the previous language's boundaries even though the parent had asked for a reset. The bucket is part of that scope, not just the audio - each Phrase BT step reads its own BT:<bcp47> regions off the same vernacular - so the guard now keys on both, and the function is named resetForScope to say so. Covered by a test against the real hook rather than the mocked one (Devin review). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… collide The real cause of PBT audio from one language playing in another: on the desktop app a take's audio is resolved by FILE NAME, not by id. useFetchMediaUrl calls dataPath(mediafile.audioUrl, PathType.MEDIA), which maps to <offlineData>/media/<basename> and hands back whatever file is already sitting there. The upload name is passageDefaultFilename + buildFilenamePostfix, which carried the passage, the segment index and the source version but not the step language. A team can configure one Phrase BT step per language, and every one of them records the same segment of the same vernacular - so Hebrew segment 1 uploaded under exactly the name Sena segment 1 had already cached, and the first file cached is what played for both. buildFilenamePostfix now takes the step bcp47 and appends it. Steps with no configured language (Careful Speech) keep the names they always had, so existing files are untouched. Also fixes a harness defect that hid this: the fake server stamped every uploaded take English|en rather than the language that was posted. Note the earlier commits on this branch do not fix the reported defect - the desktop route already remounted the step on currentstep. They stay because they are correct on their own terms: the mobile route lacked that parity, and the segment hook's reset had to key on the language bucket as well as the audio. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Failing tests first. A Careful Speech / Phrase BT take records the slice of vernacular it covers in `sourceSegments`, and that is its only link back to a segment - segments are boundaries on the vernacular's named regions, not records, so moving one rewrites the slices in place. Takes made before the move answer to a segment that no longer exists, and recording the moved segments again leaves both generations attached to the same vernacular. The record step only shows the takes matching the boundaries it is reading, but the Transcribe task list is built from every take there is: two segments, four tasks. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`selectCurrentPhraseTakes` keeps the newest take of each current segment and drops the takes that name a segment the boundaries have moved away from, so adjusting a boundary and recording again leaves one task per segment instead of one per take ever recorded. It hides rather than deletes: a take is only dropped where boundaries exist to judge it against. With no boundaries to read - a vernacular whose segments cannot be parsed, an artifact that records none, the plan-level task list that has no step context - the takes come through untouched, as do takes with no `sourceSegments` at all (Retell, and anything older than segment maps). Hiding audio on a guess would be worse than a duplicate row. The segment-matching and newest-take rules now live in one place, crud/phraseTakes, reached by both the step (carefulSpeechCompletion, matchesGuidedOutputRow) and the context layer that builds the task list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two changes to the same comparison. The tolerance is inclusive. It is half the 0.1s grid `prettySegment` rounds to, so a boundary rounded to tenths lands exactly on it, and a strict `<` called such a take stale and hid a recording the UI showed as matching. Anything wider is a boundary someone actually moved, which is what stale is meant to mean. `regionsMatch` compares two regions already parsed, so `regionMatchesClause` no longer parses `sourceSegments` to ask whether it names a region and then parses it again to compare, and `selectCurrentPhraseTakes` parses each take once rather than once per region. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
nabalone
force-pushed
the
TT-7666_pbt-stale-takes
branch
from
September 3, 2026 13:30
9104b14 to
8a2543a
Compare
Collaborator
Author
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The fix is narrowly scoped, integrates at the correct task-list construction layer, and is backed by targeted new unit/component tests covering key edge cases.
Pull request overview
This PR fixes TT-7666 in the renderer’s Transcribe flow by ensuring the task list only includes the newest take per current phrase segment after segment-boundary adjustments, preventing “stale” pre-adjustment takes from appearing as duplicate transcription tasks.
Changes:
- Added
crud/phraseTakesutilities to parse/match phrasesourceSegments, choose the newest take deterministically, and filter takes to those matching the current segment boundaries. - Plumbed phrase segment boundaries from
PassageDetailTranscribeintoTranscriberProvider, applying the filtering where the transcribe task list is assembled. - Added focused Jest coverage for the selector and updated
PassageDetailTranscribetests to verify boundary propagation.
File summaries
| File | Description |
|---|---|
src/renderer/src/crud/phraseTakes.ts |
New shared logic for matching takes to current phrase segments (with tolerance) and selecting the newest take per segment. |
src/renderer/src/crud/phraseTakes.test.ts |
Unit tests covering stale-take removal, tolerance edge cases, passthrough rules, and stable newest-take selection. |
src/renderer/src/context/TranscriberContext.tsx |
Applies selectCurrentPhraseTakes (when boundaries are provided) during task media list construction. |
src/renderer/src/components/PassageDetail/PassageDetailTranscribe.tsx |
Computes phrase segment boundaries for phrase artifacts and passes them into TranscriberProvider. |
src/renderer/src/components/PassageDetail/PassageDetailTranscribe.test.tsx |
Verifies phrase boundaries are passed for phrase artifacts and omitted otherwise. |
src/renderer/src/components/PassageDetail/carefulSpeech/matchesGuidedOutputRow.ts |
Reuses the shared newest-take comparator to keep “latest take” selection consistent. |
src/renderer/src/components/PassageDetail/carefulSpeech/carefulSpeechCompletion.ts |
Reuses the shared region parsing/matching logic (including inclusive tolerance) to centralize segment matching rules. |
Review details
- Files reviewed: 19/19 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Fixes TT-7666 — the PBT Transcribe step listed four recordings to transcribe where the step had two segments.
Replaces #570, which GitHub would not let me retarget from
TT-7643_pbt-cross-language-audiotodevelop("part of a stack"). Same three commits, now rebased ontodevelopand standing alone — TT-7643 (#561) is no longer a prerequisite.The defect
A Careful Speech / Phrase BT take records the slice of vernacular it covers in
sourceSegments, and that is its only link back to a segment: segments are boundaries on the vernacular's named regions, not records, so moving one rewrites the slices in place. Takes recorded before the move are left answering to a segment that no longer exists.The record step never shows them — it only offers the takes matching the boundaries it is reading — but the Transcribe task list is built from every take there is. Two segments, four tasks.
The fix
selectCurrentPhraseTakes(new,crud/phraseTakes) keeps the newest take of each current segment and drops the takes naming a segment the boundaries have moved away from.PassageDetailTranscribepasses the boundaries it is reading down toTranscriberProvider, which applies it where the task list is built.It hides rather than deletes, and only where there are boundaries to judge against. With none to read — a vernacular whose segments cannot be parsed, an artifact that records no phrase segments, the plan-level task list that has no step context — takes come through untouched, as do takes with no
sourceSegmentsat all (Retell, and anything older than segment maps). Hiding audio on a guess would be worse than a duplicate row.The tolerance on the comparison is inclusive: it is half the 0.1s grid
prettySegmentrounds to, so a boundary rounded to tenths lands exactly on it, and a strict<would have called such a take stale and hidden a recording the UI showed as matching.The segment-matching and newest-take rules now live in one place, reached by both the step (
carefulSpeechCompletion,matchesGuidedOutputRow) and the context layer.Test plan
src/renderer/src/crud/phraseTakes.test.ts— stale/duplicate takes, the no-boundaries and no-sourceSegmentspassthroughs, exactly-on-tolerance and past-tolerance boundaries, newest-take tie-breaking.PassageDetailTranscribe.test.tsx— the boundaries reachTranscriberProviderfor a phrase artifact and are absent otherwise.npx jest src/crud/phraseTakes.test.ts src/components/PassageDetail/PassageDetailTranscribe.test.tsx src/components/PassageDetail/carefulSpeech→ 11 suites, 69 tests green;npm run typecheckclean.Reviews already processed on #570
Devin: 0 bugs, 0 flags. Copilot round 1 (redundant double
JSON.parse) fixed. Copilot round 2 asked forimport typeon type-only imports, citing@typescript-eslint/consistent-type-imports; that rule is set only in the legacysrc/renderer/.eslintrc.cjs, which ESLint 9 ignores in favour of the flateslint.config.mjs, and with noverbatimModuleSyntaxTypeScript elides those imports anyway.🤖 Generated with Claude Code