From f450318dc0bc7112a5fdec219bdec641f26c8628 Mon Sep 17 00:00:00 2001 From: Noel Chou Date: Tue, 25 Aug 2026 21:32:07 -0400 Subject: [PATCH 1/3] TT-7621 test: blob-load dead-states and never-terminating bootstrap poll (red) Failing repros for the defensive-hardening half of the PBT hung-state report. Kept as a separate commit so the fixes that follow are demonstrably what turns them green. - useFetchMediaBlob: a download that is an S3/CDN error page (text/html or application/xml) dispatched neither FETCHED nor ERROR, so blobStat stayed PENDING forever and the reference player's context `loading` never cleared (top player stuck "Loading..."). - useFetchMediaBlob: a persistently-403 object drove an unbounded RESET->PENDING->403 loop, re-issuing a signed-URL request and a blob GET every turn (the network storm in the report) and never reaching a terminal state. - useGuidedPhraseSegments.ensureSegments: when auto-segment finds no boundaries it returned false forever, leaving the 250ms bootstrap poll (and its effect churn) spinning. It should fall back to one full-length segment once audio is loaded, and return false only while the player has no audio yet. Co-Authored-By: Claude Opus 4.8 --- .../useGuidedPhraseSegments.test.ts | 75 +++++++++++++ .../src/crud/useFetchMediaBlob.test.ts | 105 ++++++++++++++++++ 2 files changed, 180 insertions(+) create mode 100644 src/renderer/src/components/PassageDetail/carefulSpeech/useGuidedPhraseSegments.test.ts create mode 100644 src/renderer/src/crud/useFetchMediaBlob.test.ts diff --git a/src/renderer/src/components/PassageDetail/carefulSpeech/useGuidedPhraseSegments.test.ts b/src/renderer/src/components/PassageDetail/carefulSpeech/useGuidedPhraseSegments.test.ts new file mode 100644 index 000000000..55ff60506 --- /dev/null +++ b/src/renderer/src/components/PassageDetail/carefulSpeech/useGuidedPhraseSegments.test.ts @@ -0,0 +1,75 @@ +/** + * TT-7621 regression test for the Phrase Back Translate bootstrap. + * + * `PassageDetailGuidedPhraseRecord` runs a 250ms poll that calls + * `ensureSegments()` until it returns true. When auto-segment legitimately finds + * no boundaries (e.g. audio the silence math cannot split) `ensureSegments` + * returned false forever, so the poll — and its effect churn — never stopped. + * + * With audio actually loaded (duration > 0) it must instead fall back to a + * single full-length segment and return true, so the step can settle. Returning + * false stays correct only while the player has no audio yet. + */ +import { renderHook, act } from '@testing-library/react'; + +jest.mock('../Internalization/useProjectSegmentSave', () => ({ + useProjectSegmentSave: () => jest.fn().mockResolvedValue(undefined), +})); + +import { useGuidedPhraseSegments } from './useGuidedPhraseSegments'; +import { hasPhraseRegions } from './carefulSpeechBoundary'; + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +function makeControls(overrides: Record = {}): any { + return { + current: { + isReady: () => true, + getDuration: () => 10, + runAutoSegment: jest.fn().mockResolvedValue(0), + getRegionsJson: () => '{}', + loadRegionsJson: jest.fn(), + applyRegionColors: jest.fn(), + ...overrides, + }, + }; +} + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +const mediafile: any = { id: 'v1', attributes: { segments: '[]' } }; + +describe('useGuidedPhraseSegments.ensureSegments (TT-7621)', () => { + it('falls back to one full-length segment when auto-segment finds none', async () => { + const controls = makeControls(); + const { result } = renderHook(() => + useGuidedPhraseSegments(mediafile, controls, { + namedRegion: 'BT:en', + persistSegments: true, + }) + ); + + let ok: boolean | undefined; + await act(async () => { + ok = await result.current.ensureSegments(); + }); + + expect(ok).toBe(true); + expect(hasPhraseRegions(result.current.phraseSegString)).toBe(true); + }); + + it('still returns false while the player has no audio loaded', async () => { + const controls = makeControls({ getDuration: () => 0 }); + const { result } = renderHook(() => + useGuidedPhraseSegments(mediafile, controls, { + namedRegion: 'BT:en', + persistSegments: true, + }) + ); + + let ok: boolean | undefined; + await act(async () => { + ok = await result.current.ensureSegments(); + }); + + expect(ok).toBe(false); + }); +}); diff --git a/src/renderer/src/crud/useFetchMediaBlob.test.ts b/src/renderer/src/crud/useFetchMediaBlob.test.ts new file mode 100644 index 000000000..51f18f1f1 --- /dev/null +++ b/src/renderer/src/crud/useFetchMediaBlob.test.ts @@ -0,0 +1,105 @@ +/** + * TT-7621 regression tests for the reference-audio blob loader. + * + * Two ways `useFetchMediaBlob` could leave the Phrase Back Translate step's top + * player stuck on "Loading..." forever (context `loading` never clears because + * `fetching.current` is never reset): + * + * 1. The signed URL resolves but the download is an error page (S3/CloudFront + * returns HTML/XML with a 200). The old code dispatched neither FETCHED nor + * ERROR for a text/html|application/xml blob, so `blobStat` stayed PENDING. + * 2. A persistently-403 object drove an unbounded RESET->PENDING->403 loop, + * re-issuing a signed-URL request and a blob GET on every turn (the network + * storm in the hung-PBT report), and never reaching a terminal state. + * + * Both must end in ERROR so the caller can stop waiting. + */ +import { renderHook, act, waitFor } from '@testing-library/react'; + +const mockMediaClean = { + status: 0, + error: null, + url: '', + id: '', + remoteId: '', + cancelled: false, +}; + +// eslint-disable-next-line prefer-const +let mockMediaState: typeof mockMediaClean = { ...mockMediaClean }; +const mockFetchMediaUrl = jest.fn(); +// eslint-disable-next-line @typescript-eslint/no-explicit-any +let mockLoadBlob: (url: string, cb: (u: string, b?: Blob) => void) => void; + +jest.mock('./useFetchMediaUrl', () => ({ + __esModule: true, + mediaClean: mockMediaClean, + default: () => ({ + fetchMediaUrl: mockFetchMediaUrl, + mediaState: mockMediaState, + }), +})); + +jest.mock('../context/useGlobal', () => ({ + useGlobal: () => [undefined, () => {}], +})); + +jest.mock('../utils/loadBlob', () => ({ + loadBlob: (url: string, cb: (u: string, b?: Blob) => void) => + mockLoadBlob(url, cb), +})); + +import { useFetchMediaBlob, BlobStatus } from './useFetchMediaBlob'; + +beforeEach(() => { + mockMediaState = { ...mockMediaClean }; + mockFetchMediaUrl.mockReset(); +}); + +describe('useFetchMediaBlob (TT-7621)', () => { + it('dispatches ERROR (not a permanent PENDING) when the download is an error page', async () => { + mockLoadBlob = (url, cb) => + cb(url, new Blob(['nope'], { type: 'application/xml' })); + + const { result, rerender } = renderHook(() => useFetchMediaBlob()); + act(() => { + result.current[1]('m1'); + }); + + // The signed URL arrives; the effect now runs loadBlob against it. + mockMediaState = { ...mockMediaClean, id: 'm1', url: 'https://s3.invalid/x.wav' }; + act(() => { + rerender(); + }); + + await waitFor(() => + expect(result.current[0].blobStat).toBe(BlobStatus.ERROR) + ); + }); + + it('reaches a terminal ERROR after bounded 403 retries instead of looping forever', async () => { + mockLoadBlob = (_url, cb) => cb('403 Forbidden', undefined); + + const { result, rerender } = renderHook(() => useFetchMediaBlob()); + act(() => { + result.current[1]('m1'); + }); + + // Feed a fresh signed URL each turn, exactly as a real re-request would, and + // let the RESET/PENDING cycle run. It must converge, not spin. + for (let i = 0; i < 16; i++) { + mockMediaState = { + ...mockMediaClean, + id: 'm1', + url: `https://s3.invalid/x.wav?sig=${i}`, + }; + // eslint-disable-next-line no-await-in-loop + await act(async () => { + rerender(); + }); + if (result.current[0].blobStat === BlobStatus.ERROR) break; + } + + expect(result.current[0].blobStat).toBe(BlobStatus.ERROR); + }); +}); From 4a4b11427d230a77538e3b431788d4d18928bda2 Mon Sep 17 00:00:00 2001 From: Noel Chou Date: Tue, 25 Aug 2026 21:37:04 -0400 Subject: [PATCH 2/3] TT-7621 fix: stop the blob-load dead-states and the never-ending bootstrap poll Defensive hardening for the PBT hung-state report. Each is an independent way the step could stop settling; none is the core wavesurfer revoke race (that riskier root-cause fix is deferred to after the next release). useFetchMediaBlob: - A download that is an S3/CDN error page (text/html or application/xml) now dispatches ERROR instead of nothing, so blobStat can no longer stay PENDING forever - which stranded the reference player's context `loading` true and the top player on "Loading...". - The 403 -> RESET -> re-request cycle is now capped (MAX_URL_RESETS). A URL that keeps 403ing (a real permission problem, not expiry) surfaces the error instead of re-issuing a signed-URL request and a blob GET on every turn - one of the request storms in the report. useGuidedPhraseSegments.ensureSegments: - When auto-segment finds no boundaries it now falls back to one full-length segment (once audio is loaded) instead of returning false forever, so the 250ms bootstrap poll in PassageDetailGuidedPhraseRecord - and its effect churn - can stop. It still returns false while the player has no audio yet. Greens the repros committed in the previous change. jest: useFetchMediaBlob + useGuidedPhraseSegments 4/4; MediaPlayer + MediaRecord 44 pass / 3 skip; tsc clean. Co-Authored-By: Claude Opus 4.8 --- .../carefulSpeech/useGuidedPhraseSegments.ts | 27 ++++++++++--------- .../src/crud/useFetchMediaBlob.test.ts | 6 ++++- src/renderer/src/crud/useFetchMediaBlob.ts | 25 ++++++++++++++++- 3 files changed, 44 insertions(+), 14 deletions(-) diff --git a/src/renderer/src/components/PassageDetail/carefulSpeech/useGuidedPhraseSegments.ts b/src/renderer/src/components/PassageDetail/carefulSpeech/useGuidedPhraseSegments.ts index dd3b851ce..3b22643c6 100644 --- a/src/renderer/src/components/PassageDetail/carefulSpeech/useGuidedPhraseSegments.ts +++ b/src/renderer/src/components/PassageDetail/carefulSpeech/useGuidedPhraseSegments.ts @@ -170,19 +170,22 @@ export function useGuidedPhraseSegments( ); regionJson = ctrl.getRegionsJson?.() ?? '{}'; if (!hasPhraseRegions(regionJson) && (count ?? 0) <= 0) { - // Claude's suggestion for possible future implementation: auto-segment can legitimately yield nothing (e.g. audio - // the silence math can't split), and returning false leaves the - // 250ms bootstrap poll in PassageDetailGuidedPhraseRecord spinning - // forever. Consider falling back to createSingleSegmentJson() here - // when getDuration() > 0, and returning false only while the player - // has no audio loaded yet. - return false; + // Auto-segment can legitimately yield nothing (e.g. audio the silence + // math can't split). Fall back to one full-length segment so the + // 250ms bootstrap poll in PassageDetailGuidedPhraseRecord can stop; + // returning false there left it - and its effect churn - spinning + // forever (TT-7621). createSingleSegmentJson returns false only while + // the player has no audio yet, which is the one case we still defer. + const single = createSingleSegmentJson(); + if (!single) return false; + regionJson = single; + } else { + const toSave = regionsJsonFromList( + parseRegions(regionJson).regions, + boldDefaultSegParams + ); + regionJson = toSave; } - const toSave = regionsJsonFromList( - parseRegions(regionJson).regions, - boldDefaultSegParams - ); - regionJson = toSave; } allSegs = (await persistSegmentBucket(namedRegion, regionJson, allSegs)) ?? diff --git a/src/renderer/src/crud/useFetchMediaBlob.test.ts b/src/renderer/src/crud/useFetchMediaBlob.test.ts index 51f18f1f1..3ca7e8d99 100644 --- a/src/renderer/src/crud/useFetchMediaBlob.test.ts +++ b/src/renderer/src/crud/useFetchMediaBlob.test.ts @@ -67,7 +67,11 @@ describe('useFetchMediaBlob (TT-7621)', () => { }); // The signed URL arrives; the effect now runs loadBlob against it. - mockMediaState = { ...mockMediaClean, id: 'm1', url: 'https://s3.invalid/x.wav' }; + mockMediaState = { + ...mockMediaClean, + id: 'm1', + url: 'https://s3.invalid/x.wav', + }; act(() => { rerender(); }); diff --git a/src/renderer/src/crud/useFetchMediaBlob.ts b/src/renderer/src/crud/useFetchMediaBlob.ts index 41c747e6e..7f681a89a 100644 --- a/src/renderer/src/crud/useFetchMediaBlob.ts +++ b/src/renderer/src/crud/useFetchMediaBlob.ts @@ -1,4 +1,4 @@ -import { useEffect, useReducer, useState } from 'react'; +import { useEffect, useReducer, useRef, useState } from 'react'; import useFetchMediaUrl, { IMediaState, mediaClean } from './useFetchMediaUrl'; import { useGlobal } from '../context/useGlobal'; import { loadBlob } from '../utils/loadBlob'; @@ -69,11 +69,21 @@ const stateReducer = (state: IBlobState, action: Action): IBlobState => { } }; +/** + * A 403 on the signed URL means it expired: we drop the URL and re-request a + * fresh one (RESET -> PENDING). But a URL that keeps coming back 403 - a genuine + * permission problem, not expiry - would loop that forever, re-issuing a + * signed-URL request and a blob GET every turn (part of the TT-7621 network + * storm). Cap the re-requests, then surface the error. + */ +const MAX_URL_RESETS = 3; + export const useFetchMediaBlob = () => { const [reporter] = useGlobal('errorReporter'); const [mediaId, setMediaId] = useState(''); const { fetchMediaUrl, mediaState } = useFetchMediaUrl(reporter); const [state, dispatch] = useReducer(stateReducer, blobClean); + const resetTriesRef = useRef(0); const fetchBlob = (url: string) => { setMediaId(url); @@ -82,6 +92,7 @@ export const useFetchMediaBlob = () => { type retValue = [IBlobState, typeof fetchBlob]; useEffect(() => { + resetTriesRef.current = 0; fetchMediaUrl({ id: mediaId }); dispatch({ type: BlobStatus.PENDING, @@ -96,6 +107,12 @@ export const useFetchMediaBlob = () => { loadBlob(mediaState.url, (urlOrError, blob) => { if (!blob) { if (urlOrError.includes('403')) { + if (resetTriesRef.current >= MAX_URL_RESETS) { + // Not expiry - the object keeps 403ing. Stop re-requesting. + dispatch({ type: BlobStatus.ERROR, payload: urlOrError }); + return; + } + resetTriesRef.current += 1; fetchMediaUrl({ id: '' }); dispatch({ type: BlobStatus.RESET, payload: mediaState }); } else { @@ -105,8 +122,14 @@ export const useFetchMediaBlob = () => { } // we have a blob blob if (blob.type !== 'text/html' && blob.type !== 'application/xml') { + resetTriesRef.current = 0; const url = urlOrError; dispatch({ type: BlobStatus.FETCHED, payload: { url, blob } }); + } else { + // An HTML/XML body is an error page (S3/CDN), not audio. Terminate + // instead of leaving blobStat PENDING forever, which stranded the + // reference player on "Loading..." (TT-7621). + dispatch({ type: BlobStatus.ERROR, payload: urlOrError }); } }); } catch (errorResult: unknown) { From b0fcd8dc120eea42f2d036da722d8b37637eccc5 Mon Sep 17 00:00:00 2001 From: Noel Chou Date: Fri, 28 Aug 2026 09:00:41 -0400 Subject: [PATCH 3/3] TT-7621 fix: address Copilot review on useFetchMediaBlob - Stay IDLE (not PENDING) on the initial [mediaId] effect when mediaId is empty, so a consumer reading loading from blobStat === PENDING does not show a spurious spinner before the first fetchBlob. - Name the unexpected content type in the ERROR payload when the downloaded body is an HTML/XML error page, so the logged error is actionable. Co-Authored-By: Claude Opus 4.8 --- src/renderer/src/crud/useFetchMediaBlob.ts | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/renderer/src/crud/useFetchMediaBlob.ts b/src/renderer/src/crud/useFetchMediaBlob.ts index 7f681a89a..760ececb1 100644 --- a/src/renderer/src/crud/useFetchMediaBlob.ts +++ b/src/renderer/src/crud/useFetchMediaBlob.ts @@ -93,6 +93,13 @@ export const useFetchMediaBlob = () => { useEffect(() => { resetTriesRef.current = 0; + if (!mediaId) { + // Nothing requested yet - stay IDLE rather than PENDING, so a consumer + // that reads its loading state from blobStat === PENDING does not show a + // spurious spinner before the first fetchBlob (Copilot). + dispatch({ type: BlobStatus.IDLE, payload: undefined }); + return; + } fetchMediaUrl({ id: mediaId }); dispatch({ type: BlobStatus.PENDING, @@ -128,8 +135,12 @@ export const useFetchMediaBlob = () => { } else { // An HTML/XML body is an error page (S3/CDN), not audio. Terminate // instead of leaving blobStat PENDING forever, which stranded the - // reference player on "Loading..." (TT-7621). - dispatch({ type: BlobStatus.ERROR, payload: urlOrError }); + // reference player on "Loading..." (TT-7621). Name the unexpected + // content type so the logged error is actionable (Copilot). + dispatch({ + type: BlobStatus.ERROR, + payload: `unexpected content type ${blob.type}: ${urlOrError}`, + }); } }); } catch (errorResult: unknown) {