fix: Phrase Back Translate undo collapse - #583
Draft
nabalone wants to merge 3 commits into
Draft
Conversation
Segment-boundary Undo reverts all prior boundary edits at once instead of one at a time. The undo stack pops one entry correctly; the fault is on the push side. Two waveform facts drive it, even with seconds between edits: one boundary add fires the segment callback more than once, and the waveform holds a single stale-closure callback, so pushSegmentUndo keeps photographing an out-of-date (usually baseline) segmentation. These tests model both facts (callback captured once, fired twice per add) and assert one-step-at-a-time undo. They fail against current code and encode the contract a live-ref + dedupe fix must satisfy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Live-ref push + dedupe so a stale, multi-firing waveform onSegment callback records one undo entry per real boundary change. pushSegmentUndo reads clauseSegStringRef (not a stale closure) and skips when the stack top already holds those boundaries; edits set through setClauseSeg to keep the ref current synchronously. Adds peek() to the undo stack. All 3 undo tests green; CarefulSpeech/types/stack suites green; typecheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The fix is targeted, the regression is reproduced with realistic tests, and the remaining feedback is limited to minor incremental test coverage for the new peek() helper.
Pull request overview
Fixes a guided Phrase Back Translate workflow bug where segment-boundary Undo could collapse multiple edits into a single revert by avoiding stale-closure reads of the current segmentation and preventing duplicate undo pushes from multi-fire waveform events.
Changes:
- Introduces a
clauseSegStringRef+setClauseSegwrapper soonSegmentreads/writes the live segmentation even when invoked via a stale callback closure. - Adds multi-level Undo regression tests that model the real waveform behavior (single captured callback, multi-fire per boundary add).
- Extends the phrase-segment undo stack with a
peek()API to support dedupe checks.
File summaries
| File | Description |
|---|---|
src/renderer/src/utils/phraseSegmentUndoStack.ts |
Adds peek() to support snapshot deduping in multi-level segment undo logic. |
src/renderer/src/components/PassageDetail/PassageDetailPhraseBackTranslate.undo.test.tsx |
Adds regression tests reproducing the undo-collapse scenario and validating one-step-per-click Undo. |
src/renderer/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.tsx |
Fixes stale-closure undo snapshotting by using a live ref mirror of the current segmentation and guarding against duplicate pushes. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
5
to
11
| export interface PhraseSegmentUndoStack { | ||
| push: (segmentsJson: string) => void; | ||
| pop: () => string | undefined; | ||
| /** Most recently pushed snapshot without removing it (for dedupe). */ | ||
| peek: () => string | undefined; | ||
| canUndo: () => boolean; | ||
| clear: () => void; |
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.
Stacked on
TT-7437_no-segment-switch-while-recording.What changed
Fixes the Phrase Back Translate undo collapse issue in the guided phrase record workflow.
PassageDetailPhraseBackTranslate.undo.test.tsx.PassageDetailGuidedPhraseRecord.tsxand a small tweak inphraseSegmentUndoStack.ts.Test plan
npm run test(jest, fromsrc/renderer) — new undo tests pass.🤖 Generated with Claude Code