From a610c90c256935fd699cc3235d0f7af6dac87836 Mon Sep 17 00:00:00 2001 From: sudacode Date: Thu, 3 Sep 2026 23:12:24 -0700 Subject: [PATCH] fix(anki): report stale timing reviews after preview and waveform awaits Playback and ffmpeg waveform analysis both span long enough for the review to end underneath them (decision watchdog, overlay teardown). Both paths reported success or a generic failure afterwards, so the modal stayed open on a review main had already dropped. --- src/main/runtime/media-timing-review.test.ts | 38 +++++++++++++++++++- src/main/runtime/media-timing-review.ts | 17 ++++++++- 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/src/main/runtime/media-timing-review.test.ts b/src/main/runtime/media-timing-review.test.ts index 27207e80..fad9b587 100644 --- a/src/main/runtime/media-timing-review.test.ts +++ b/src/main/runtime/media-timing-review.test.ts @@ -78,6 +78,8 @@ async function startActiveMediaTimingReview( options: { maxMediaDuration?: number; decisionTimeoutMs?: number; + generateWaveform?: () => Promise; + play?: () => Promise; } = {}, ) { const previewCalls: Array<[number, number]> = []; @@ -94,12 +96,13 @@ async function startActiveMediaTimingReview( }), getCurrentMediaPath: () => '/video/show.mkv', getMpvExecutablePath: () => 'mpv', - generateWaveform: async () => [], + generateWaveform: options.generateWaveform ?? (async () => []), decisionTimeoutMs: options.decisionTimeoutMs, createPreviewSession: () => ({ start: async () => undefined, play: async (startTime, endTime) => { previewCalls.push([startTime, endTime]); + await options.play?.(); }, stop: async () => undefined, onPlaybackEnded: () => undefined, @@ -821,3 +824,36 @@ test('media timing review forwards the hidden player finishing a preview to the playback.ended(); assert.deepEqual(endedReviewIds, [payload.reviewId]); }); + +test('preview reports a stale review when the review ends during playback', async () => { + let endReview: (() => Promise) | null = null; + const { runtime, payload, pendingDecision } = await startActiveMediaTimingReview({ + play: async () => { + await endReview?.(); + }, + }); + endReview = () => runtime.dispose(); + + assert.deepEqual( + await runtime.previewRange({ reviewId: payload.reviewId, startTime: 10, endTime: 12 }), + { ok: false, stale: true, message: 'This timing review is no longer active.' }, + ); + await pendingDecision; +}); + +test('waveform reports a stale review when the review ends during analysis', async () => { + let endReview: (() => Promise) | null = null; + const { runtime, payload, pendingDecision } = await startActiveMediaTimingReview({ + generateWaveform: async () => { + await endReview?.(); + return [0.1, 0.9, 0.2]; + }, + }); + endReview = () => runtime.dispose(); + + assert.deepEqual( + await runtime.getWaveform({ reviewId: payload.reviewId, startTime: 8, endTime: 14 }), + { ok: false, stale: true, message: 'This timing review is no longer active.' }, + ); + await pendingDecision; +}); diff --git a/src/main/runtime/media-timing-review.ts b/src/main/runtime/media-timing-review.ts index d14befa7..b78a23cb 100644 --- a/src/main/runtime/media-timing-review.ts +++ b/src/main/runtime/media-timing-review.ts @@ -463,8 +463,16 @@ export function createMediaTimingReviewRuntime(deps: MediaTimingReviewRuntimeDep return staleReviewResult(); } await previewSession.play(request.startTime, request.endTime); + // Playback spans the whole clip, so the review can end (watchdog, teardown) while + // it runs; reporting success would leave the modal open on a dead review. + if (active !== current) { + return staleReviewResult(); + } return { ok: true }; } catch (error) { + if (active !== current) { + return staleReviewResult(); + } return { ok: false, message: `Audio preview unavailable: ${error instanceof Error ? error.message : String(error)}`, @@ -503,11 +511,18 @@ export function createMediaTimingReviewRuntime(deps: MediaTimingReviewRuntimeDep ? { audioStreamIndex: current.audioStreamIndex } : {}), }); - if (active !== current || peaks.length < 2 || peaks.some((peak) => !Number.isFinite(peak))) { + // ffmpeg decoding runs long enough for the review to end underneath it. + if (active !== current) { + return staleReviewResult(); + } + if (peaks.length < 2 || peaks.some((peak) => !Number.isFinite(peak))) { return { ok: false, message: 'Timing waveform is unavailable.' }; } return { ok: true, peaks }; } catch { + if (active !== current) { + return staleReviewResult(); + } return { ok: false, message: 'Timing waveform is unavailable.' }; } }