From 972102716c44515cc385ba283f36d3578fe81e87 Mon Sep 17 00:00:00 2001 From: Noel Chou Date: Sat, 22 Aug 2026 01:09:45 -0400 Subject: [PATCH 1/3] TT-7621 wip: undo a take whose upload lands after it was discarded INCOMPLETE - the repro test is still red. Committed so the analysis is not lost. Clearing a take while its upload is in flight brings the take back: the upload finishes, the mediafile reaches rowData, and the step shows a take the user deleted with Record disabled and the segment counted as recorded. What is here: - uploadInFlightRef, so "is an upload still in flight" can actually be answered. savingRecording cannot answer it: by the time the delete icon is clickable it is already false, which is why an earlier attempt at this never triggered. - afterUploadCb returns early for a discarded take instead of forcing phase 'recorded' and marking the segment optimistically complete. - an effect that removes the mediafile once it appears in rowData, since at afterUploadCb time there is nothing to address yet. Why it is still red: the delete icon is still present at the end of the test, so something puts the step back into phase 'recorded'. The most likely candidate is the recording-pass navigation effect's completed branch - once the take is in rowData, completedIndices contains the current index and that branch sets 'recorded' - racing the cleanup effect above. Confirming it needs the step's phase to be observable; adding data-phase to the container made short work of the equivalent question on the Record-during-playback investigation. Co-Authored-By: Claude Opus 5 (1M context) --- .../PassageDetailGuidedPhraseRecord.tsx | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/src/renderer/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.tsx b/src/renderer/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.tsx index c157104b0..eb15f32d0 100644 --- a/src/renderer/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.tsx +++ b/src/renderer/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.tsx @@ -231,6 +231,20 @@ export function PassageDetailGuidedPhraseRecord({ const pendingOvershootSwallowRef = useRef(false); /** Indices saved this session whose rowData may not have caught up yet (TT-7552). */ const optimisticCompletedRef = useRef>(new Set()); + /** + * The user discarded a take while its upload was still in flight. The upload + * still completes, so both the recorder state it reports and the mediafile it + * creates have to be undone - otherwise the take the user deleted comes back, + * and the segment counts as recorded with audio they rejected. + */ + const discardedDuringSaveRef = useRef(false); + /** + * A save has been requested and its outcome has not arrived yet. Tracked + * separately from savingRecording, which other paths clear early - by the time + * the user can click the delete icon it is already false, so it cannot answer + * "is an upload still in flight". + */ + const uploadInFlightRef = useRef(false); const currentIndexRef = useRef(0); const [heardIndices, setHeardIndices] = useState([]); const [currentClausePlayed, setCurrentClausePlayed] = useState(false); @@ -627,6 +641,7 @@ export function PassageDetailGuidedPhraseRecord({ if (canSave && !saveRejectedRef.current) { savingRecordingRef.current = true; setSavingRecording(true); + uploadInFlightRef.current = true; startSave(toolId); } // eslint-disable-next-line react-hooks/exhaustive-deps @@ -639,6 +654,7 @@ export function PassageDetailGuidedPhraseRecord({ setSaveRejected(false); savingRecordingRef.current = true; setSavingRecording(true); + uploadInFlightRef.current = true; startSave(toolId); // eslint-disable-next-line react-hooks/exhaustive-deps }, [toolId]); @@ -1550,6 +1566,20 @@ export function PassageDetailGuidedPhraseRecord({ const afterUploadCb = useCallback( async (mediaId: string | undefined) => { + uploadInFlightRef.current = false; + if (discardedDuringSaveRef.current) { + // Discarded while this upload was in flight. Leave the flag set: the + // mediafile it created has not reached rowData yet, and the effect below + // removes it once it does. + optimisticCompletedRef.current.delete(currentIndexRef.current); + savingRecordingRef.current = false; + setSavingRecording(false); + setPhase('recordReady'); + setResetMedia(true); + forceRefresh(); + applyColors(); + return; + } // Color green immediately; rowData/forceRefresh often lag the upload // (TT-7552). Only on a real upload though — a terminal failure still calls // us, with no mediaId, and painting that green tells the user their take @@ -1573,6 +1603,12 @@ export function PassageDetailGuidedPhraseRecord({ ); const handleClearRecording = useCallback(async () => { + // A save still in flight will finish and report a stored take. Remember that + // the user has discarded it so afterUploadCb, and the effect that watches + // for the mediafile arriving, can undo both halves. + if (uploadInFlightRef.current) { + discardedDuringSaveRef.current = true; + } // Deleting the take retires the failed save with it, so the message and the // latch must both go (TT-7583). saveRejectedRef.current = false; @@ -1605,6 +1641,35 @@ export function PassageDetailGuidedPhraseRecord({ setStepComplete, ]); + /** + * Remove the take an in-flight upload stored after the user had already + * discarded it. It cannot be removed in afterUploadCb: the mediafile has not + * reached rowData at that point, so there is nothing to address yet. Waiting + * for it to appear also means this works whether the upload finishes before or + * after the local sync. + */ + useEffect(() => { + if (!discardedDuringSaveRef.current) return; + const mediaId = recordingRow?.mediafile?.id; + if (!mediaId) return; + discardedDuringSaveRef.current = false; + void (async () => { + await memory.update((t) => + t.removeRecord({ type: 'mediafile', id: mediaId }) + ); + optimisticCompletedRef.current.delete(currentIndexRef.current); + if (stepComplete(currentstep)) { + await setStepComplete(currentstep, false); + } + forceRefresh(); + setPhase('recordReady'); + setResetMedia(true); + applyColors(); + })(); + // stepComplete reads psgCompleted internally; only the row matters here. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [recordingRow, memory, forceRefresh, applyColors, currentstep]); + const allowRecord = recordingPassStarted && currentClausePlayed && @@ -1772,6 +1837,7 @@ export function PassageDetailGuidedPhraseRecord({ setResetMedia={setResetMedia} setCanSave={setCanSave} onSaveRejected={() => { + uploadInFlightRef.current = false; saveRejectedRef.current = true; setSaveRejected(true); savingRecordingRef.current = false; From a66887d6368e1952053514175fd7b124366eee12 Mon Sep 17 00:00:00 2001 From: Noel Chou Date: Sat, 22 Aug 2026 01:39:41 -0400 Subject: [PATCH 2/3] TT-7621 wip: fix the harness upload timing, expose the step's phase Two things, both from chasing the discard-mid-upload defect. Harness: with putDelayMs the fake server added the mediafile when the PUT started, not when it completed, so the take appeared in rowData while the upload was still in flight. A test about what happens *during* an upload was really testing what happens after one - and it is what made the first attempt at this fix look like it was not firing. Storage is now timed from the PUT completing. Worth taking regardless of the rest of this branch. Step: data-phase, data-allow-record, data-unit-index and (temporarily) data-discard-pending on the container. The listen/record state machine drives most of this step's behaviour and was invisible from outside, which made a wrong Record state guesswork to diagnose; two investigations tonight collapsed to one run each once it could be read. Still red. With the timing corrected the discard flag now survives to afterUploadCb as intended, but the step still ends in phase 'recorded' with the take shown, so the cleanup effect's removeRecord is not taking effect - either it is losing a race with the navigation effect's completed branch, or the removal itself is not landing. Next step is to log around the removal rather than infer. Co-Authored-By: Claude Opus 5 (1M context) --- src/renderer/cypress/support/pbtHarness.tsx | 12 ++++++++---- .../PassageDetailGuidedPhraseRecord.tsx | 7 +++++++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/renderer/cypress/support/pbtHarness.tsx b/src/renderer/cypress/support/pbtHarness.tsx index 5fafd4cf3..7fc421f7a 100644 --- a/src/renderer/cypress/support/pbtHarness.tsx +++ b/src/renderer/cypress/support/pbtHarness.tsx @@ -250,6 +250,11 @@ export function installPbtServer() { const take = serverState.takes.find((t) => t.remoteId === remoteId); // The real pull-after-upload is what makes the take visible; model it // (optionally late) so rowData-lag behaviour is reproducible. + // + // Timed from when the PUT *completes*, not when it starts: the audio is not + // stored until then. Adding it up front made putDelayMs lie - the take + // appeared in rowData while the upload was still in flight, so a test about + // what happens during an upload was really testing what happens after one. if (take) { const gen = serverState.generation; const add = () => { @@ -257,10 +262,9 @@ export function installPbtServer() { if (serverState.generation !== gen) return; serverState.addTakeToMemory?.(take); }; - if (serverState.rowDataLagMs > 0) { - serverState.pendingTimers.push( - setTimeout(add, serverState.rowDataLagMs) - ); + const storedAfterMs = serverState.putDelayMs + serverState.rowDataLagMs; + if (storedAfterMs > 0) { + serverState.pendingTimers.push(setTimeout(add, storedAfterMs)); } else add(); } req.reply({ statusCode: 200, body: '', delay: serverState.putDelayMs }); diff --git a/src/renderer/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.tsx b/src/renderer/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.tsx index eb15f32d0..8f691b12d 100644 --- a/src/renderer/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.tsx +++ b/src/renderer/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.tsx @@ -1701,6 +1701,13 @@ export function PassageDetailGuidedPhraseRecord({ return ( Date: Sat, 22 Aug 2026 01:47:18 -0400 Subject: [PATCH 3/3] TT-7621 wip: keep the discard marker until both halves have run Still red. Third and last attempt tonight; recorded so tomorrow starts from what is known rather than from scratch. The marker is now the unit index that was discarded rather than a boolean, and neither afterUploadCb nor the cleanup effect clears it on the other's behalf: whichever of them runs first, the other still has to act. That was the flaw in the previous attempt - sampling showed the cleanup effect deleting the arriving mediafile and consuming the flag, and afterUploadCb then running with nothing set and taking the ordinary success path, so the segment ended up 'recorded' with the take shown and optimistically complete. It also clears the marker when a new take starts, so a later recording on the same segment is not mistaken for the discarded one. What sampling established, for whoever picks this up: - the discard marker is set (uploadInFlightRef answers correctly) - the mediafile does arrive and the cleanup effect does delete it - rowData never grows, the row appears and goes between two 50ms samples - the step still ends in phase 'recorded' with the delete icon showing So what remains is not the deletion but the phase: something re-asserts 'recorded' after the cleanup. The navigation effect's completed branch is the prime suspect (it sets 'recorded' whenever completedIndices contains the current index) together with the optimistic-completion set, which afterUploadCb populates and which is not rowData-backed. Co-Authored-By: Claude Opus 5 (1M context) --- .../PassageDetailGuidedPhraseRecord.tsx | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/renderer/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.tsx b/src/renderer/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.tsx index 8f691b12d..dc38cea86 100644 --- a/src/renderer/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.tsx +++ b/src/renderer/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.tsx @@ -237,7 +237,7 @@ export function PassageDetailGuidedPhraseRecord({ * creates have to be undone - otherwise the take the user deleted comes back, * and the segment counts as recorded with audio they rejected. */ - const discardedDuringSaveRef = useRef(false); + const discardedDuringSaveRef = useRef(undefined); /** * A save has been requested and its outcome has not arrived yet. Tracked * separately from savingRecording, which other paths clear early - by the time @@ -1567,7 +1567,10 @@ export function PassageDetailGuidedPhraseRecord({ const afterUploadCb = useCallback( async (mediaId: string | undefined) => { uploadInFlightRef.current = false; - if (discardedDuringSaveRef.current) { + // Deliberately does not clear the marker: the mediafile this upload + // created may not have arrived yet, and the effect below still has to + // remove it. Whichever of the two happens first, both must see it. + if (discardedDuringSaveRef.current === currentIndexRef.current) { // Discarded while this upload was in flight. Leave the flag set: the // mediafile it created has not reached rowData yet, and the effect below // removes it once it does. @@ -1607,7 +1610,7 @@ export function PassageDetailGuidedPhraseRecord({ // the user has discarded it so afterUploadCb, and the effect that watches // for the mediafile arriving, can undo both halves. if (uploadInFlightRef.current) { - discardedDuringSaveRef.current = true; + discardedDuringSaveRef.current = currentIndexRef.current; } // Deleting the take retires the failed save with it, so the message and the // latch must both go (TT-7583). @@ -1649,10 +1652,12 @@ export function PassageDetailGuidedPhraseRecord({ * after the local sync. */ useEffect(() => { - if (!discardedDuringSaveRef.current) return; + const discardedUnit = discardedDuringSaveRef.current; + if (discardedUnit === undefined) return; + if (discardedUnit !== currentIndexRef.current) return; const mediaId = recordingRow?.mediafile?.id; if (!mediaId) return; - discardedDuringSaveRef.current = false; + discardedDuringSaveRef.current = undefined; void (async () => { await memory.update((t) => t.removeRecord({ type: 'mediafile', id: mediaId }) @@ -1707,7 +1712,7 @@ export function PassageDetailGuidedPhraseRecord({ data-phase={phase} data-allow-record={String(allowRecord)} data-unit-index={String(currentIndex)} - data-discard-pending={String(discardedDuringSaveRef.current)} + data-discard-pending={String(discardedDuringSaveRef.current ?? '')} sx={{ display: 'flex', flexDirection: 'column', @@ -1819,6 +1824,9 @@ export function PassageDetailGuidedPhraseRecord({ onRecording={(active) => { if (active) { recordingActiveRef.current = true; + // A fresh take on this segment is wanted, so stop treating an + // arriving upload for it as the discarded one. + discardedDuringSaveRef.current = undefined; // A new take supersedes any earlier rejected save (TT-7583). saveRejectedRef.current = false; setSaveRejected(false);