TT-7621 fix: make More/Fewer Segments move the way they say - #532
Closed
nabalone wants to merge 2 commits into
Closed
TT-7621 fix: make More/Fewer Segments move the way they say#532nabalone wants to merge 2 commits into
nabalone wants to merge 2 commits into
Conversation
Tapping Fewer Segments could leave more segments than before (TT-7543). More/Fewer only nudge the auto-segment parameters and re-run silence detection, and that is not monotonic: one nudge of "fewer" lowers the silence threshold, which can split the audio further instead of merging it. Whatever came back was applied regardless of direction. Both buttons now keep nudging in the requested direction until the segment count actually moves that way, up to a small attempt budget, and if it never does they put back the map the user had rather than applying a result they did not ask for. The silent no-op is still there when re-segmenting cannot do better - the button does nothing and says nothing. Telling the user needs a new localized string, so it belongs in its own change rather than being bundled in here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three findings, all correct.
Phantom undo entry (bug): the undo point was taken before the attempts, so a tap
that could not move the count left the undo button lit for a change that never
happened - and pressing it reset the selection to the first segment and cleared
listen progress. The undo point is now taken only when a result is accepted, and
takes the map explicitly rather than reading state the attempts have moved on.
Up to five writes per tap: each attempt persisted its result to the mediafile,
including the ones being rejected. resegmentWithParams takes { persist: false }
for trials, and only the accepted result is written.
Restore skipped for an empty starting map: the hasPhraseRegions guard meant a
user with no segments kept the last rejected attempt. Since nothing is persisted
during the attempts, the restore is now just state and player, unconditionally.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Claude opened this but it is out of scope for my current card and I believe Greg has other changes planned for the more/fewer segments algorithm |
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.
Fix 5 of the PBT series. Into #527. Addresses TT-7543.
The fault
Tapping Fewer Segments could leave more segments than before.
More/Fewer only nudge the auto-segment parameters and re-run silence detection, and that is not monotonic: one nudge of "fewer" lowers the silence threshold, which can split the audio further instead of merging it. Whatever came back was applied regardless of direction.
The fix
Both buttons keep nudging in the requested direction until the segment count actually moves that way (small attempt budget), and if it never does they put back the map the user had rather than applying a result they did not ask for.
Still outstanding
The silent no-op half of TT-7543 remains: when re-segmenting cannot do better, the button does nothing and says nothing. Greg's note on that ticket asks the same question ("Should we be putting up a message?"). Telling the user needs a new localized string, which means an XLIFF round-trip, so it belongs in its own change rather than bundled here.
Verification
Note on the run: with the dev server and another browser up, CT timings roughly double and tests can time out spuriously — the two failures I first saw here were load, and passed cleanly on a re-run.
🤖 Generated with Claude Code