TT-7621 fix: follow a segment change the waveform reports - #536
Draft
nabalone wants to merge 2 commits into
Draft
Conversation
Base automatically changed from
TT-7621_pbt-segment-selection-and-recorder-state
to
develop
August 24, 2026 22:33
Recording segment 1, then segment 3, then clicking back to segment 2 filed the next take on segment 3, on top of the take already there. The step learns that the selection moved by watching currentSegmentIndex, but that field's numbering is not agreed between its writers: usePlayerLogic writes the waveform's 1-based sorted position (0 meaning "whole"), Mark Verses writes a table row, and this component writes a 0-based clause index. So a real move can arrive carrying the number the previous writer used - selecting segment 2 puts 1+1 there, which is what the step itself had just written for segment 3 - and the navigation effect never re-runs. The waveform selection and playhead move, the step does not, and the take goes wherever the step still thinks it is. Adds currentSegmentSeq to PassageDetailContext: a token bumped whenever the current segment actually changes. The two navigation effects watch it instead of inferring a change from the index. The step never used the number itself - it derives its own index from the region - so it now depends only on what it actually needs. Unifying the numbering would remove the need for this and is worth doing, but it touches every caller across Mark Verses, the players and this step, so it is deliberately not in here. There is also a latent off-by-one at useWavesurferRegions.tsx:955 that the same discrepancy causes; see the write-up that accompanies this branch. Test: "follows a click back to an earlier segment and files the take there" now passes, and asserts the take's uploaded source-segments is segment 2's span and that segment 3 still holds exactly one take. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ADR 0010. Writes up the 0-based/1-based split in PassageDetailContext.currentSegmentIndex: who writes which convention, who reads which, what it broke on this ticket, and the latent off-by-one at useWavesurferRegions.tsx:955 that the same split causes. Deferred on purpose - unifying touches every caller across Mark Verses, the players and the guided-record step, and Noel's call is to do it after the next release. The ADR says what becomes removable then (currentSegmentSeq and the two navigation dependency lists) and what does not (the other TT-7621 fixes, which are unrelated to the numbering). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
nabalone
force-pushed
the
TT-7621_pbt-follow-segment-change
branch
from
August 25, 2026 12:40
1cae219 to
60cd79d
Compare
nabalone
marked this pull request as draft
August 25, 2026 12:51
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
1 participant
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.
Split from #527 so #527 can remain tests-only.
What this PR contains
currentSegmentIndexnumbering mismatch and why a change token is usedWhy this split exists
This logic fix was previously included in #527. It was moved out so #527 can be reviewed/merged strictly as test infrastructure and defect-repro coverage.
Stacking
TT-7621_pbt-segment-selection-and-recorder-state(TT-7621 test: PBT CT harness and defect repro coverage #527)