diff --git a/src/renderer/cypress/support/pbtHarness.tsx b/src/renderer/cypress/support/pbtHarness.tsx index 725d14045..6e643e9af 100644 --- a/src/renderer/cypress/support/pbtHarness.tsx +++ b/src/renderer/cypress/support/pbtHarness.tsx @@ -60,6 +60,8 @@ import type { MediaFileD } from '../../src/model'; import { boldDefaultSegParams } from '../../src/components/PassageDetail/carefulSpeech/boldCarefulSpeechSegParams'; import { regionsJsonFromList } from '../../src/components/PassageDetail/carefulSpeech/carefulSpeechBoundary'; import { prettySegment } from '../../src/utils/prettySegment'; +import { parseMediaLanguageBcp47 } from '../../src/utils/mediaLanguage'; +import { phraseBtBoundaryRegionName } from '../../src/components/PassageDetail/carefulSpeech/matchesGuidedOutputRow'; import PassageDetailPhraseBackTranslate from '../../src/components/PassageDetail/PassageDetailPhraseBackTranslate'; // --------------------------------------------------------------------------- @@ -97,8 +99,8 @@ export const USER_ID = 'user-1'; export const ORG_ID = 'org-1'; export const STEP_LANGUAGE = 'English|en'; export const STEP_BCP47 = 'en'; -/** Named-region bucket PBT stores its boundaries in (phraseBtBoundaryRegionName). */ -export const BT_REGION_NAME = `BT:${STEP_BCP47}`; +/** Named-region bucket PBT stores its boundaries in. */ +export const BT_REGION_NAME = phraseBtBoundaryRegionName(STEP_BCP47); /** Fake S3 host the intercepted POST hands back for the audio PUT. */ const FAKE_AUDIO_HOST = 'https://pbt-test.invalid'; @@ -144,6 +146,8 @@ interface ServerState { generation: number; /** Outstanding lagged writes, so a reset can cancel them. */ pendingTimers: ReturnType[]; + /** ids requested through GET /mediafiles/:id/fileurl (in request order). */ + fileurlRequestedIds: string[]; } const serverState: ServerState = { @@ -154,6 +158,7 @@ const serverState: ServerState = { nextRemoteId: 1000, generation: 0, pendingTimers: [], + fileurlRequestedIds: [], }; /** Reset the fake server. Call from beforeEach before installing intercepts. */ @@ -168,6 +173,7 @@ export function resetPbtServer(options?: { serverState.pendingTimers = []; serverState.generation += 1; serverState.takes = []; + serverState.fileurlRequestedIds = []; serverState.nextRemoteId = 1000; serverState.putDelayMs = options?.putDelayMs ?? 0; serverState.fileurlDelayMs = options?.fileurlDelayMs ?? 0; @@ -182,6 +188,11 @@ export function postedTakes(): PostedTake[] { return serverState.takes; } +/** mediafile ids requested by MediaRecord load calls (GET /mediafiles/:id/fileurl). */ +export function fileurlRequestedIds(): string[] { + return serverState.fileurlRequestedIds; +} + /** Make later uploads fail (used mid-spec for save-failure paths). */ export function failNextUploads(putStatus = 500) { serverState.failPutWithStatus = putStatus; @@ -269,6 +280,10 @@ export function installPbtServer() { // Existing-take load (useFetchUrlNow → GET mediafiles//fileurl). cy.intercept('GET', '**/mediafiles/*/fileurl', (req) => { + const match = /\/mediafiles\/([^/]+)\/fileurl(?:\?|$)/.exec(req.url); + if (match?.[1]) { + serverState.fileurlRequestedIds.push(decodeURIComponent(match[1])); + } req.reply({ statusCode: 200, delay: serverState.fileurlDelayMs, @@ -378,6 +393,15 @@ export interface MountPbtOptions { segments?: SegmentSpec[]; /** Indices that already have a saved take when the step opens. */ existingTakes?: number[]; + /** Exact seeded takes for multi-language and duplicate-segment cases. */ + existingTakeRows?: Array<{ + segmentIndex: number; + languagebcp47: string; + remoteId: string; + performedBy?: string; + }>; + /** Step language stamped in Step Settings (`Name|bcp47`). */ + stepLanguage?: string; /** Source audio length (s). Must cover the last segment end. */ durationSec?: number; /** ms before an uploaded take shows up in rowData (0 = immediate). */ @@ -419,7 +443,7 @@ function interiorBoundaries( .filter((t) => t > 0.01 && t < durationSec - 0.01); } -function segmentsAttribute(segments: SegmentSpec[]): string { +function segmentsAttribute(segments: SegmentSpec[], bcp47: string): string { const regions: IRegion[] = segments.map((s) => ({ start: s.start, end: s.end, @@ -427,7 +451,7 @@ function segmentsAttribute(segments: SegmentSpec[]): string { })); return JSON.stringify([ { - name: BT_REGION_NAME, + name: phraseBtBoundaryRegionName(bcp47), regionInfo: regionsJsonFromList(regions, boldDefaultSegParams), }, ]); @@ -437,7 +461,8 @@ function takeRecord( id: string, remoteId: string, sourceSegments: string, - performedBy: string | null + performedBy: string | null, + languagebcp47 = STEP_LANGUAGE ): MediaFileD { return { type: 'mediafile', @@ -450,7 +475,7 @@ function takeRecord( originalFile: `${id}.ogg`, audioUrl: `${FAKE_AUDIO_HOST}/audio/${id}.wav`, sourceSegments, - languagebcp47: STEP_LANGUAGE, + languagebcp47, performedBy, dateCreated: new Date(2026, 0, 1).toISOString(), segments: '[]', @@ -467,10 +492,12 @@ function takeRecord( function seedRecords(memory: Memory, options: MountPbtOptions) { const segments = options.segments ?? []; + const stepLanguage = options.stepLanguage ?? STEP_LANGUAGE; + const stepBcp47 = parseMediaLanguageBcp47(stepLanguage); const stepTool = JSON.stringify({ tool: 'phraseBackTranslate', settings: JSON.stringify({ - language: STEP_LANGUAGE, + language: stepLanguage, artifactTypeId: ARTIFACT_TYPE_ID, }), }); @@ -536,7 +563,7 @@ function seedRecords(memory: Memory, options: MountPbtOptions) { contentType: 'audio/wav', originalFile: 'vern.wav', audioUrl: `${FAKE_AUDIO_HOST}/audio/vern.wav`, - segments: segmentsAttribute(segments), + segments: segmentsAttribute(segments, stepBcp47), transcription: '', }, relationships: { @@ -546,19 +573,37 @@ function seedRecords(memory: Memory, options: MountPbtOptions) { }, ]; - (options.existingTakes ?? []).forEach((idx) => { - const seg = segments[idx]; - if (!seg) return; - records.push( - takeRecord( - `mf-take-${idx}`, - String(500 + idx), - JSON.stringify({ start: seg.start, end: seg.end, label: '' }), - 'Existing Speaker' - // eslint-disable-next-line @typescript-eslint/no-explicit-any - ) as any - ); - }); + if ((options.existingTakeRows?.length ?? 0) > 0) { + options.existingTakeRows?.forEach((row) => { + const seg = segments[row.segmentIndex]; + if (!seg) return; + records.push( + takeRecord( + `mf-take-${row.remoteId}`, + row.remoteId, + JSON.stringify({ start: seg.start, end: seg.end, label: '' }), + row.performedBy ?? 'Existing Speaker', + row.languagebcp47 + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ) as any + ); + }); + } else { + (options.existingTakes ?? []).forEach((idx) => { + const seg = segments[idx]; + if (!seg) return; + records.push( + takeRecord( + `mf-take-${idx}`, + String(500 + idx), + JSON.stringify({ start: seg.start, end: seg.end, label: '' }), + 'Existing Speaker', + stepLanguage + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ) as any + ); + }); + } memory.cache.update((t) => // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -693,7 +738,11 @@ function PbtHarnessInner({ options, memory, blob }: HarnessProps) { id, take.remoteId, take.sourceSegments, - take.performedBy + take.performedBy, + // The real pull-after-upload returns what was posted; stamping + // every uploaded take English hid whether the step language ever + // reached the upload at all. + take.languagebcp47 // eslint-disable-next-line @typescript-eslint/no-explicit-any ) as any ) diff --git a/src/renderer/src/components/PassageDetail/PassageDetailCarefulSpeech.test.tsx b/src/renderer/src/components/PassageDetail/PassageDetailCarefulSpeech.test.tsx index a1323a799..4e7127fc2 100644 --- a/src/renderer/src/components/PassageDetail/PassageDetailCarefulSpeech.test.tsx +++ b/src/renderer/src/components/PassageDetail/PassageDetailCarefulSpeech.test.tsx @@ -102,7 +102,7 @@ jest.mock('./carefulSpeech/useGuidedPhraseSegments', () => ({ setPhraseSegString: jest.fn(), bootstrapped: true, ensureSegments: jest.fn().mockResolvedValue(true), - resetForMediafile: jest.fn(), + resetForScope: jest.fn(), resegmentWithParams: jest.fn().mockResolvedValue(false), resetToDefaultSegments: jest.fn().mockResolvedValue(false), persistPhraseSegments: jest.fn().mockResolvedValue(undefined), diff --git a/src/renderer/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.stepScope.test.tsx b/src/renderer/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.stepScope.test.tsx new file mode 100644 index 000000000..952e79675 --- /dev/null +++ b/src/renderer/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.stepScope.test.tsx @@ -0,0 +1,292 @@ +import { act, cleanup, render, waitFor } from '@testing-library/react'; +import { IRegion } from '../../crud/useWavesurferRegions'; + +/** + * TT-7643 - a team can configure one Phrase Back Translation step per language, + * and every one of them renders this same component against the same vernacular + * audio. Where the route does not remount it between steps (the mobile route + * did not), the instance is reused, and everything it holds for the step just + * left - the recording pass, the clause it is parked on, and the take mounted + * in its recorder - carried into the next language's step, so that step played + * the other language's audio. + * + * The component now scopes its own reset to the step as well as the mediafile, + * so a reused instance still opens the next step from scratch. + */ + +const regions: IRegion[] = [ + { start: 0, end: 10, label: '' }, + { start: 10, end: 20, label: '' }, + { start: 20, end: 30, label: '' }, +]; + +let mockCompleted = new Set(); +let controlsProps: Record | undefined; +let mockRecordingRow: + | { mediafile: { id: string; attributes?: Record } } + | undefined; + +/** Step settings by step id: two PBT steps, different languages. */ +const stepSettings: Record> = { + 'step-sena': { artifactTypeId: 'art1', language: 'Sena|seh' }, + 'step-hebrew': { artifactTypeId: 'art1', language: 'Hebrew|he' }, +}; + +const stubControls = { + isReady: jest.fn(() => true), + isPlaying: jest.fn(() => false), + gotoTime: jest.fn().mockResolvedValue(undefined), + setPlay: jest.fn(), + applyRegionColors: jest.fn(), + loadRegionsJson: jest.fn(), +}; + +const ctx: { + _seg: IRegion | undefined; + currentSegmentIndex: number; + currentstep: string; + [k: string]: unknown; +} = { + _seg: regions[0], + currentSegmentIndex: 0, + currentstep: 'step-sena', + passage: { id: 'p1', type: 'passage' }, + playerMediafile: { id: 'm1', type: 'mediafile' }, + mediafileId: 'm1', + rowData: [], + section: { id: 's1', type: 'section' }, + setPlaying: jest.fn(), + setRecording: jest.fn(), + forceRefresh: jest.fn(), + getCurrentSegment: jest.fn(() => ctx._seg), + isBoldWorkflow: false, + carefulSpeechSegParams: {}, + setCarefulSpeechSegParams: jest.fn(), + setStepComplete: jest.fn().mockResolvedValue(undefined), + stepComplete: jest.fn(() => false), + setCurrentSegment: jest.fn((region: IRegion) => { + ctx._seg = region; + ctx.currentSegmentIndex += 1; + }), +}; + +jest.mock('../../context/usePassageDetailContext', () => () => ctx); + +const mockResetForScope = jest.fn(); +jest.mock('./carefulSpeech/useGuidedPhraseSegments', () => ({ + useGuidedPhraseSegments: () => ({ + phraseSegString: '[]', + setPhraseSegString: jest.fn(), + bootstrapped: true, + ensureSegments: jest.fn().mockResolvedValue(true), + resetForScope: mockResetForScope, + resegmentWithParams: jest.fn().mockResolvedValue(false), + resetToDefaultSegments: jest.fn().mockResolvedValue(false), + persistPhraseSegments: jest.fn().mockResolvedValue(undefined), + }), +})); + +jest.mock('../../utils/namedSegments', () => { + const actual = jest.requireActual('../../utils/namedSegments'); + return { ...actual, getSortedRegions: jest.fn(() => regions) }; +}); + +jest.mock('./carefulSpeech/carefulSpeechCompletion', () => { + const actual = jest.requireActual('./carefulSpeech/carefulSpeechCompletion'); + return { + ...actual, + getCompletedClauseIndices: jest.fn(() => mockCompleted), + getRecordingForClause: jest.fn(() => mockRecordingRow), + }; +}); + +jest.mock('../../crud', () => ({ + ArtifactTypeSlug: { PhraseBackTranslation: 'phrase-back-translation' }, + remoteIdGuid: jest.fn((_t: string, id: string) => id), + useArtifactType: () => ({ getTypeId: () => 'art1' }), + useStepTool: (step: string) => ({ settings: stepSettings[step] ?? {} }), +})); +jest.mock('../../crud/related', () => ({ related: () => 'p1' })); +jest.mock('../../utils/useStepPermission', () => ({ + useStepPermissions: () => ({ canDoSectionStep: () => true }), +})); +// Echo the postfix the config built, so a spec can see what the take would be +// named. Real signature: (passage, plan, memory, artifactType, offline, postfix). +jest.mock('../../utils/passageDefaultFilename', () => ({ + passageDefaultFilename: (...args: unknown[]) => + `GEN001_014-019${args[5]}_plan`, +})); +jest.mock('../../selector', () => ({ + sharedSelector: jest.fn(), + mediaTabSelector: jest.fn(), + mediaTitleSelector: jest.fn(), +})); +jest.mock('react-redux', () => ({ + useSelector: () => ({ + uploadFailed: 'Upload Failed!', + pendingUploadRetryOne: 'Retry', + }), + shallowEqual: jest.fn(), +})); +jest.mock('../../context/useGlobal', () => ({ + useGlobal: (key: string) => + key === 'memory' + ? [ + { keyMap: {}, update: jest.fn().mockResolvedValue(undefined) }, + jest.fn(), + ] + : [undefined, jest.fn()], +})); +jest.mock('../../context/UnsavedContext', () => { + const ReactActual = jest.requireActual('react'); + return { + UnsavedContext: ReactActual.createContext({ + state: { + startSave: jest.fn(), + waitForSave: jest.fn().mockResolvedValue(undefined), + }, + }), + }; +}); +jest.mock('../../hoc/useOrbitData', () => ({ + useOrbitData: () => [ + { id: 'm1', type: 'mediafile', attributes: { versionNumber: 1 } }, + ], +})); + +jest.mock('./PassageDetailPlayer', () => ({ + __esModule: true, + default: (props: Record) => { + const ref = props.controlsRef as { current: unknown } | undefined; + if (ref) ref.current = stubControls; + return
; + }, +})); + +jest.mock('./carefulSpeech/CarefulSpeechControls', () => ({ + __esModule: true, + default: (props: Record) => { + controlsProps = props; + return
; + }, +})); + +// imported after the mocks so the component picks them up +import { PassageDetailGuidedPhraseRecord } from './PassageDetailGuidedPhraseRecord'; +import { phraseBackTranslateConfig } from './guidedPhraseRecord/types'; +import { ArtifactTypeSlug } from '../../crud/artifactTypeSlug'; +import { NamedRegions } from '../../utils/namedSegments'; + +const config = phraseBackTranslateConfig( + ArtifactTypeSlug.PhraseBackTranslation, + NamedRegions.BackTranslation +); + +const strings = { + allComplete: 'All segments recorded', + unitLabel: 'Segment: {0}', + clearRecording: 'Clear', + combineWithNext: 'Combine', + fewerUnits: 'Fewer', + moreUnits: 'More', + nextUnit: 'Next', + splitUnit: 'Split', + speaker: 'Speaker', + startRecording: 'Start', + undo: 'Undo', + noStepLanguage: 'Configure a language', +}; + +const ui = () => ( + +); + +const mountAndSettle = async () => { + const utils = render(ui()); + await waitFor(() => expect(controlsProps).toBeDefined()); + await waitFor(() => expect(stubControls.gotoTime).toHaveBeenCalled()); + return utils; +}; + +beforeEach(() => { + mockCompleted = new Set(); + controlsProps = undefined; + mockRecordingRow = undefined; + ctx._seg = regions[0]; + ctx.currentSegmentIndex = 0; + ctx.currentstep = 'step-sena'; + jest.clearAllMocks(); + stubControls.isReady.mockReturnValue(true); + stubControls.isPlaying.mockReturnValue(false); + stubControls.gotoTime.mockResolvedValue(undefined); +}); + +afterEach(() => cleanup()); + +describe('PassageDetailGuidedPhraseRecord - step scope (TT-7643)', () => { + it('opens the next language step from scratch when the instance is reused', async () => { + // Sena: every segment recorded, so the step opens in review mode with the + // recorder mounted on Sena's take. + mockCompleted = new Set([0, 1, 2]); + mockRecordingRow = { mediafile: { id: 'sena-take-1' } }; + const { rerender } = await mountAndSettle(); + expect(controlsProps?.recordingPassStarted).toBe(true); + expect(controlsProps?.showRecorder).toBe(true); + expect(controlsProps?.recordingMediaId).toBe('sena-take-1'); + + // Move to the Hebrew step without unmounting - nothing is recorded there. + mockCompleted = new Set(); + mockRecordingRow = undefined; + ctx.currentstep = 'step-hebrew'; + await act(async () => { + rerender(ui()); + }); + + await waitFor(() => + expect(controlsProps?.recordingPassStarted).toBe(false) + ); + expect(controlsProps?.showRecorder).toBe(false); + expect(controlsProps?.recordingMediaId).toBeUndefined(); + // The boundaries the new step opens on are the segment hook's business, + // and this suite mocks that hook - so whether the reset it is handed + // actually re-reads the next language's bucket is asserted against the + // real hook in useGuidedPhraseSegments.test.tsx, not here. + expect(mockResetForScope).toHaveBeenCalledWith('m1'); + }); + + it('names a take so it cannot collide with another language of the same segment', async () => { + // The uploaded file name is what the media cache is keyed on: `dataPath` + // resolves a mediafile's audioUrl to `/media/`, so + // two takes that upload under one name share one cached file and the first + // one downloaded is what plays. Segment index and source version were in + // the name but the step language was not, so Hebrew segment 1 and Sena + // segment 1 were the same file (TT-7643). + const { rerender } = await mountAndSettle(); + const senaName = controlsProps?.defaultFilename as string; + expect(senaName).toContain('seh'); + + ctx.currentstep = 'step-hebrew'; + await act(async () => { + rerender(ui()); + }); + await waitFor(() => + expect(controlsProps?.defaultFilename).not.toEqual(senaName) + ); + expect(controlsProps?.defaultFilename).toContain('he'); + }); + + it('records against the language of the step now showing', async () => { + const { rerender } = await mountAndSettle(); + expect(controlsProps?.languagebcp47).toBe('Sena|seh'); + + ctx.currentstep = 'step-hebrew'; + await act(async () => { + rerender(ui()); + }); + await waitFor(() => expect(controlsProps?.languagebcp47).toBe('Hebrew|he')); + }); +}); diff --git a/src/renderer/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.tsx b/src/renderer/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.tsx index bbcf9ad38..79ade9f69 100644 --- a/src/renderer/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.tsx +++ b/src/renderer/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.tsx @@ -196,7 +196,7 @@ export function PassageDetailGuidedPhraseRecord({ ); const bootstrapPollRef = useRef | null>(null); const bootstrapCompletedRef = useRef(false); - const lastResetMediafileRef = useRef(undefined); + const lastResetScopeRef = useRef(undefined); const initialPositionDoneRef = useRef(false); const suppressClauseAutoPlayRef = useRef(0); const playClauseInFlightRef = useRef(false); @@ -388,7 +388,7 @@ export function PassageDetailGuidedPhraseRecord({ setPhraseSegString: setClauseSegString, bootstrapped, ensureSegments, - resetForMediafile, + resetForScope, resegmentWithParams, resetToDefaultSegments, persistPhraseSegments: persistClauseSegments, @@ -520,7 +520,11 @@ export function PassageDetailGuidedPhraseRecord({ ); const defaultFilename = useMemo(() => { - const postfix = config.buildFilenamePostfix(currentIndex, currentVersion); + const postfix = config.buildFilenamePostfix( + currentIndex, + currentVersion, + stepLanguageBcp47 + ); return passageDefaultFilename( passage, plan, @@ -537,6 +541,7 @@ export function PassageDetailGuidedPhraseRecord({ offline, currentIndex, currentVersion, + stepLanguageBcp47, config, ]); @@ -838,9 +843,22 @@ export function PassageDetailGuidedPhraseRecord({ // mid-entry, clobbering an already-started recording pass and dropping the // user back into the listen pass (TT-7360). Only reset once per actual // mediafile change. - if (lastResetMediafileRef.current === mediafileId) return; - lastResetMediafileRef.current = mediafileId; - resetForMediafile(mediafileId); + // + // The step is part of that identity, not just the mediafile: a team can + // configure a Phrase BT step per language, and every one of them renders + // this same component against the same vernacular. Where the route does not + // remount it between steps, keying the reset on the mediafile alone carried + // the previous language's recording pass - its clause index, its baseline + // boundaries, its optimistic greens, and the take showing in the recorder - + // into the next language's step (TT-7643). + if (!currentstep) return; + const stepScope = `${mediafileId ?? ''}|${currentstep}`; + if (lastResetScopeRef.current === stepScope) return; + lastResetScopeRef.current = stepScope; + resetForScope(mediafileId); + // The legacy claim is per step language, so a reused instance has to be + // allowed to run it again for the step just moved to. + claimRanRef.current = false; bootstrapCompletedRef.current = false; setPhase('bootstrapping'); setCurrentIndex(0); @@ -865,12 +883,13 @@ export function PassageDetailGuidedPhraseRecord({ setEntryPositioned(false); suppressClauseAutoPlayRef.current = 0; setHighlightPlayButton(false); - // Gate only on the stable mediafileId string. resetForMediafile's identity - // changes whenever the mediafile record is updated (e.g. persisting combined - // clause segments), which would otherwise re-fire this reset and drop the - // user from the recording pass back into the listen pass (TT-7360). + // Gate only on the stable mediafileId / currentstep strings. + // resetForScope's identity changes whenever the mediafile record is + // updated (e.g. persisting combined clause segments), which would otherwise + // re-fire this reset and drop the user from the recording pass back into + // the listen pass (TT-7360). // eslint-disable-next-line react-hooks/exhaustive-deps - }, [mediafileId]); + }, [mediafileId, currentstep]); useEffect(() => { if (!mediafileId || !stepEnabled) return; diff --git a/src/renderer/src/components/PassageDetail/PassageDetailPhraseBackTranslate.cy.tsx b/src/renderer/src/components/PassageDetail/PassageDetailPhraseBackTranslate.cy.tsx index 40de12702..88099205c 100644 --- a/src/renderer/src/components/PassageDetail/PassageDetailPhraseBackTranslate.cy.tsx +++ b/src/renderer/src/components/PassageDetail/PassageDetailPhraseBackTranslate.cy.tsx @@ -13,6 +13,7 @@ import { mountPbt, waitForPbtReady, + fileurlRequestedIds, postedTakes, waitForUploads, expectSegmentColors, @@ -217,3 +218,117 @@ describe('PBT out-of-order recording', () => { }); }); }); + +/** + * A team can configure one PBT step per language, so the same passage carries + * takes from several of them, recorded against the very same segment. A step + * must only ever see - and play - its own language's takes (TT-7643). + */ +describe('PBT language scoping', () => { + it('loads this step language take when another language has one too', () => { + mountPbt({ + segments: SEGMENTS, + stepLanguage: 'Hebrew|he', + existingTakeRows: [ + // Higher remote id than the Hebrew take, so a chooser that ignores + // language would land on the Sena one. + { + segmentIndex: 0, + languagebcp47: 'Sena|seh', + remoteId: '999', + performedBy: 'Sena Speaker', + }, + { + segmentIndex: 0, + languagebcp47: 'Hebrew|he', + remoteId: '101', + performedBy: 'Hebrew Speaker', + }, + // Hebrew is finished, so the step opens in review mode on segment 1 + // with its take mounted in the recorder - the moment the wrong take + // would become audible. + { + segmentIndex: 1, + languagebcp47: 'Hebrew|he', + remoteId: '102', + performedBy: 'Hebrew Speaker', + }, + { + segmentIndex: 2, + languagebcp47: 'Hebrew|he', + remoteId: '103', + performedBy: 'Hebrew Speaker', + }, + ], + }); + waitForPbtReady(); + + cy.wrap(null, { timeout: 20000 }).should(() => { + expect( + fileurlRequestedIds().length, + 'a take was loaded' + ).to.be.greaterThan(0); + }); + cy.then(() => { + expect( + fileurlRequestedIds(), + 'only this step language was ever fetched' + ).to.not.include('999'); + expect(fileurlRequestedIds()[0]).to.equal('101'); + }); + }); + + it('treats another language take as no take at all', () => { + mountPbt({ + segments: SEGMENTS, + stepLanguage: 'Hebrew|he', + existingTakeRows: [ + { + segmentIndex: 0, + languagebcp47: 'Sena|seh', + remoteId: '999', + performedBy: 'Sena Speaker', + }, + ], + }); + waitForPbtReady(); + + // Nothing recorded in Hebrew yet: the listen pass, not review mode. + cy.get(PBT.start).should('be.visible'); + expectSegmentColors([ + SEGMENT_COLOR.current, + SEGMENT_COLOR.pending, + SEGMENT_COLOR.pending, + ]); + cy.then(() => + expect(fileurlRequestedIds(), 'no foreign take fetched').to.not.include( + '999' + ) + ); + }); + + it('uploads a take under a name no other language can take', () => { + // The name a take uploads under is the name its audio is cached under on + // disk: dataPath resolves a mediafile's audioUrl to + // `/media/` and hands back whatever file is already + // sitting there. The name carried the segment index and the source version + // but not the language, so the Hebrew step's segment 1 uploaded as exactly + // the name the Sena step's segment 1 had already cached - and Sena is what + // played (TT-7643). + mountPbt({ segments: SEGMENTS, stepLanguage: 'Hebrew|he' }); + waitForPbtReady(); + startRecordingPass(); + recordAndSettle(1); + + cy.then(() => { + const posted = postedTakes()[0]; + expect(posted?.languagebcp47, 'take is stamped').to.equal('Hebrew|he'); + expect(posted?.originalFile, 'name carries the language').to.contain( + '_he' + ); + // What the Sena step would have uploaded for this same segment. + const senaName = (posted?.originalFile ?? '').replace('_he', '_seh'); + expect(posted?.originalFile).to.not.equal(senaName); + }); + }); +}); diff --git a/src/renderer/src/components/PassageDetail/carefulSpeech/claimLegacyPhraseBt.test.ts b/src/renderer/src/components/PassageDetail/carefulSpeech/claimLegacyPhraseBt.test.ts index 0b913433c..a18ac6801 100644 --- a/src/renderer/src/components/PassageDetail/carefulSpeech/claimLegacyPhraseBt.test.ts +++ b/src/renderer/src/components/PassageDetail/carefulSpeech/claimLegacyPhraseBt.test.ts @@ -54,6 +54,73 @@ describe('planLegacyPhraseBtClaim', () => { expect(segs).toContain(phraseBtBoundaryRegionName('fr')); }); + it('leaves untagged takes alone once a second language has boundaries', () => { + // Sena has already been back-translated on this passage, so an untagged + // take may be Sena's. The Hebrew step must not adopt it (TT-7643). + const multiLang = { + ...vern, + attributes: { + segments: updateSegments( + phraseBtBoundaryRegionName('seh'), + vern.attributes.segments, + JSON.stringify({ params: {}, regions: [{ start: 0, end: 4 }] }) + ), + }, + }; + const result = planLegacyPhraseBtClaim({ + languageName: 'Hebrew', + languageBcp47: 'he', + artifactTypeId: 'art1', + vernacularMedia: [multiLang], + outputMedia: [untagged, tagged], + }); + expect(result.languageUpdates.size).toBe(0); + // The step still gets its own boundaries seeded from the legacy bucket. + expect(result.segmentUpdates.has('v1')).toBe(true); + }); + + it('still claims when only this language has boundaries', () => { + const mine = { + ...vern, + attributes: { + segments: updateSegments( + phraseBtBoundaryRegionName('he'), + vern.attributes.segments, + JSON.stringify({ params: {}, regions: [{ start: 0, end: 4 }] }) + ), + }, + }; + const result = planLegacyPhraseBtClaim({ + languageName: 'Hebrew', + languageBcp47: 'he', + artifactTypeId: 'art1', + vernacularMedia: [mine], + outputMedia: [untagged], + }); + expect(result.languageUpdates.get('p1')).toBe('Hebrew|he'); + }); + + it('ignores an empty bucket for another language', () => { + const emptyOther = { + ...vern, + attributes: { + segments: updateSegments( + phraseBtBoundaryRegionName('seh'), + vern.attributes.segments, + JSON.stringify({ params: {}, regions: [] }) + ), + }, + }; + const result = planLegacyPhraseBtClaim({ + languageName: 'Hebrew', + languageBcp47: 'he', + artifactTypeId: 'art1', + vernacularMedia: [emptyOther], + outputMedia: [untagged], + }); + expect(result.languageUpdates.get('p1')).toBe('Hebrew|he'); + }); + it('does not overwrite an existing language bucket', () => { const already = { ...vern, diff --git a/src/renderer/src/components/PassageDetail/carefulSpeech/claimLegacyPhraseBt.ts b/src/renderer/src/components/PassageDetail/carefulSpeech/claimLegacyPhraseBt.ts index 9f5a982ae..87fc0d950 100644 --- a/src/renderer/src/components/PassageDetail/carefulSpeech/claimLegacyPhraseBt.ts +++ b/src/renderer/src/components/PassageDetail/carefulSpeech/claimLegacyPhraseBt.ts @@ -1,5 +1,6 @@ import { related } from '../../../crud/related'; import { MediaFileD } from '../../../model'; +import { tryParseJSON } from '../../../utils/tryParseJson'; import { getSegments, NamedRegions, @@ -29,9 +30,45 @@ export interface IClaimLegacyPhraseBtResult { segmentUpdates: Map; } +const BUCKET_PREFIX = phraseBtBoundaryRegionName(''); + +/** + * True when a vernacular already carries Phrase BT boundaries for some language + * other than `bcp47` - i.e. this passage is already being back-translated into + * more than one language. + */ +function hasOtherLanguageBoundaries( + vernacularMedia: MediaFileD[], + bcp47: string +): boolean { + const mine = phraseBtBoundaryRegionName(bcp47).toLowerCase(); + return vernacularMedia.some((v) => { + const all = v.attributes?.segments ?? '[]'; + const parsed = tryParseJSON(all); + if (!Array.isArray(parsed)) return false; + return parsed.some((entry) => { + const name = String((entry as { name?: unknown })?.name ?? ''); + const lower = name.toLowerCase(); + if (!lower.startsWith(BUCKET_PREFIX.toLowerCase())) return false; + if (lower === mine) return false; + return hasPhraseRegions(getSegments(name, all)); + }); + }); +} + /** * Claim untagged outputs and copy legacy `BT` into `BT:${bcp47}` when empty. * Does not touch Retell or other artifact types. + * + * Claiming is for data recorded before takes were stamped with a language, + * which by definition belongs to whichever single language the team was working + * in. Once a passage has boundaries for a second language the premise is gone: + * an untagged take could belong to either step, and handing it to whichever one + * happened to open played one language's audio in the other's step (TT-7643). + * So the language claim is skipped there - an unclaimed take stays out of every + * step rather than joining the wrong one. Copying legacy boundaries into this + * language's empty bucket is unaffected: boundaries are per language and the + * copy never overwrites an existing bucket. */ export function planLegacyPhraseBtClaim( args: IClaimLegacyPhraseBtArgs @@ -43,8 +80,13 @@ export function planLegacyPhraseBtClaim( const languageUpdates = new Map(); const segmentUpdates = new Map(); const bucket = phraseBtBoundaryRegionName(args.languageBcp47); + const multiLanguage = hasOtherLanguageBoundaries( + args.vernacularMedia, + args.languageBcp47 + ); for (const m of args.outputMedia) { + if (multiLanguage) break; if (related(m, 'artifactType') !== args.artifactTypeId) continue; const existing = parseMediaLanguageBcp47(m.attributes?.languagebcp47); if (existing === 'und' || !m.attributes?.languagebcp47) { diff --git a/src/renderer/src/components/PassageDetail/carefulSpeech/useGuidedPhraseSegments.test.tsx b/src/renderer/src/components/PassageDetail/carefulSpeech/useGuidedPhraseSegments.test.tsx new file mode 100644 index 000000000..df92bafbc --- /dev/null +++ b/src/renderer/src/components/PassageDetail/carefulSpeech/useGuidedPhraseSegments.test.tsx @@ -0,0 +1,81 @@ +import { act, renderHook } from '@testing-library/react'; +import { createRef } from 'react'; +import { MediaFileD } from '../../../model'; +import { WSAudioPlayerControls } from '../../WSAudioPlayer'; +import { phraseBtBoundaryRegionName } from './matchesGuidedOutputRow'; + +/** + * TT-7643 - a Phrase BT step per language reads its own `BT:` boundary + * bucket off the same vernacular audio. The reset has to key on that bucket as + * well as the mediafile, or a step change is rejected as "same audio, nothing + * to do" and the next language opens on the previous language's boundaries. + */ + +jest.mock('../Internalization/useProjectSegmentSave', () => ({ + useProjectSegmentSave: () => jest.fn().mockResolvedValue(undefined), +})); + +import { useGuidedPhraseSegments } from './useGuidedPhraseSegments'; + +const bucket = (name: string, regions: { start: number; end: number }[]) => ({ + name, + regionInfo: JSON.stringify({ params: {}, regions }), +}); + +const SENA = [{ start: 0, end: 11 }]; +const HEBREW = [ + { start: 0, end: 4 }, + { start: 4, end: 9 }, +]; + +const mediafile = { + id: 'mf-vern', + type: 'mediafile', + attributes: { + segments: JSON.stringify([ + bucket(phraseBtBoundaryRegionName('seh'), SENA), + bucket(phraseBtBoundaryRegionName('he'), HEBREW), + ]), + }, + relationships: {}, +} as unknown as MediaFileD; + +const controlsRef = createRef() as React.RefObject< + WSAudioPlayerControls | undefined +> as React.RefObject; + +const regionsOf = (json: string) => + (JSON.parse(json) as { regions?: { start: number; end: number }[] }).regions; + +describe('useGuidedPhraseSegments - reset scope', () => { + it('re-reads boundaries when the language bucket changes on the same audio', () => { + const { result, rerender } = renderHook( + ({ namedRegion }: { namedRegion: string }) => + useGuidedPhraseSegments(mediafile, controlsRef, { namedRegion }), + { initialProps: { namedRegion: phraseBtBoundaryRegionName('seh') } } + ); + + act(() => result.current.resetForScope(mediafile.id)); + expect(regionsOf(result.current.phraseSegString)).toEqual(SENA); + + // Same vernacular, next language's step. + rerender({ namedRegion: phraseBtBoundaryRegionName('he') }); + act(() => result.current.resetForScope(mediafile.id)); + expect(regionsOf(result.current.phraseSegString)).toEqual(HEBREW); + }); + + it('stays put when neither the audio nor the bucket moved', () => { + const { result } = renderHook(() => + useGuidedPhraseSegments(mediafile, controlsRef, { + namedRegion: phraseBtBoundaryRegionName('he'), + }) + ); + + act(() => result.current.resetForScope(mediafile.id)); + act(() => result.current.setPhraseSegString('{"params":{},"regions":[]}')); + // A repeat call must not undo work done since - the guard that keeps + // StrictMode's double-invoke from clobbering a started pass (TT-7360). + act(() => result.current.resetForScope(mediafile.id)); + expect(regionsOf(result.current.phraseSegString)).toEqual([]); + }); +}); diff --git a/src/renderer/src/components/PassageDetail/carefulSpeech/useGuidedPhraseSegments.ts b/src/renderer/src/components/PassageDetail/carefulSpeech/useGuidedPhraseSegments.ts index dd3b851ce..4e72f2c89 100644 --- a/src/renderer/src/components/PassageDetail/carefulSpeech/useGuidedPhraseSegments.ts +++ b/src/renderer/src/components/PassageDetail/carefulSpeech/useGuidedPhraseSegments.ts @@ -50,7 +50,7 @@ export function useGuidedPhraseSegments( const [phraseSegString, setPhraseSegString] = useState('{}'); const [bootstrapped, setBootstrapped] = useState(false); const bootstrapInProgress = useRef(false); - const mediafileIdRef = useRef(undefined); + const scopeRef = useRef(undefined); const persistSegmentBucket = useCallback( async ( @@ -88,16 +88,27 @@ export function useGuidedPhraseSegments( if (hasPhraseRegions(regionJson)) setPhraseSegString(regionJson); }, [mediafile, persistSegments, singleSegmentMode, readRegionJson]); - const resetForMediafile = useCallback( + /** + * Re-read boundaries for the scope now showing, unless it is the one already + * loaded. + * + * The bucket is part of that scope, not just the audio: a team can configure + * a Phrase BT step per language, and each reads its own `BT:` + * boundaries off the same vernacular. Keyed on the mediafile alone, a step + * change was rejected as a no-op and the next language opened on the previous + * language's phrase boundaries (TT-7643). + */ + const resetForScope = useCallback( (mediafileId: string | undefined) => { - if (mediafileIdRef.current === mediafileId) return; - mediafileIdRef.current = mediafileId; + const scope = `${mediafileId ?? ''}|${namedRegion}`; + if (scopeRef.current === scope) return; + scopeRef.current = scope; bootstrapInProgress.current = false; setBootstrapped(false); setPhraseSegString('{}'); if (mediafileId) hydrateFromMediafile(); }, - [hydrateFromMediafile] + [hydrateFromMediafile, namedRegion] ); const loadRegionsOnPlayer = useCallback( @@ -283,7 +294,7 @@ export function useGuidedPhraseSegments( setPhraseSegString, bootstrapped, ensureSegments, - resetForMediafile, + resetForScope, resegmentWithParams, resetToDefaultSegments, persistPhraseSegments: (regionJson: string) => diff --git a/src/renderer/src/components/PassageDetail/guidedPhraseRecord/types.test.ts b/src/renderer/src/components/PassageDetail/guidedPhraseRecord/types.test.ts index dd92412a3..1fb961201 100644 --- a/src/renderer/src/components/PassageDetail/guidedPhraseRecord/types.test.ts +++ b/src/renderer/src/components/PassageDetail/guidedPhraseRecord/types.test.ts @@ -54,4 +54,28 @@ describe('guidedPhraseRecord config', () => { expect(config.buildFilenamePostfix(0, 2)).toBe('backtranslation1_v2'); expect(config.buildFilenamePostfix(1, 2)).toBe('backtranslation2_v2s1'); }); + + it('buildFilenamePostfix separates the languages of the same segment', () => { + // Media is cached on disk under the uploaded file's name (dataPath maps a + // mediafile's audioUrl to `/media/`), so a name + // shared by two takes means one cached file for both, and the first one + // cached is what plays. A Phrase BT step per language records the same + // segment of the same vernacular, so the language has to be in the name + // (TT-7643). + const config = phraseBackTranslateConfig( + ArtifactTypeSlug.PhraseBackTranslation, + NamedRegions.BackTranslation + ); + expect(config.buildFilenamePostfix(0, 1, 'seh')).toBe( + 'backtranslation1_v1_seh' + ); + expect(config.buildFilenamePostfix(0, 1, 'he')).toBe( + 'backtranslation1_v1_he' + ); + expect(config.buildFilenamePostfix(1, 1, 'he')).toBe( + 'backtranslation2_v1s1_he' + ); + // Steps with no configured language keep the names they always had. + expect(config.buildFilenamePostfix(0, 1)).toBe('backtranslation1_v1'); + }); }); diff --git a/src/renderer/src/components/PassageDetail/guidedPhraseRecord/types.ts b/src/renderer/src/components/PassageDetail/guidedPhraseRecord/types.ts index bc05feb9d..b582628bf 100644 --- a/src/renderer/src/components/PassageDetail/guidedPhraseRecord/types.ts +++ b/src/renderer/src/components/PassageDetail/guidedPhraseRecord/types.ts @@ -45,8 +45,21 @@ export interface GuidedPhraseRecordConfig { sequentialUnitNavAroundRecord: boolean; /** Persist segment map on vernacular named regions (false for Retell). */ persistSegments: boolean; - /** Filename postfix for a unit at `unitIndex` (0-based) on `sourceVersion`. */ - buildFilenamePostfix: (unitIndex: number, sourceVersion: number) => string; + /** + * Filename postfix for a unit at `unitIndex` (0-based) on `sourceVersion`. + * + * The result has to be unique per take, not just pretty: the uploaded name + * becomes the media object's name, and `dataPath` resolves a mediafile's + * audioUrl to `/media/`. Two takes uploaded under one + * name therefore share a single cached file, and whichever was cached first + * is what plays for both. `languageBcp47` is passed for the steps that can + * have a sibling step over the same audio in another language (TT-7643). + */ + buildFilenamePostfix: ( + unitIndex: number, + sourceVersion: number, + languageBcp47?: string + ) => string; } const carefulSpeechBoundaryDefaults = { @@ -93,10 +106,13 @@ export function phraseBackTranslateConfig( multiLevelSegmentUndo: phraseBoundaryTools, sequentialUnitNavAroundRecord: phraseBoundaryTools, persistSegments: phraseBoundaryTools, - buildFilenamePostfix: (unitIndex, sourceVersion) => { + buildFilenamePostfix: (unitIndex, sourceVersion, languageBcp47) => { const base = `${artifactSlug}${unitIndex + 1}_v${sourceVersion}`; - if (unitIndex > 0) return `${base}s${unitIndex}`; - return base; + const unit = unitIndex > 0 ? `${base}s${unitIndex}` : base; + // A Phrase BT step per language records the same segment of the same + // vernacular, so without the language every one of them uploads under + // the same name. Takes made before this stay on their old names. + return languageBcp47 ? `${unit}_${languageBcp47}` : unit; }, }; } diff --git a/src/renderer/src/routes/PassageDetail.tsx b/src/renderer/src/routes/PassageDetail.tsx index b53adb8c2..8906673fc 100644 --- a/src/renderer/src/routes/PassageDetail.tsx +++ b/src/renderer/src/routes/PassageDetail.tsx @@ -81,11 +81,27 @@ const MobileStep = () => { ) : tool === ToolSlug.Verses ? ( ) : tool === ToolSlug.CarefulSpeech ? ( - + // Keyed for the same reason as the transcription steps below: a team can + // configure several guided-record steps in a row, and they all render the + // one PassageDetailGuidedPhraseRecord component. (TT-7643) + ) : tool === ToolSlug.PhraseBackTranslate && isBoldWorkflow ? ( - + ) : tool === ToolSlug.PhraseBackTranslate && !isBoldWorkflow ? ( - + // A team can have one Phrase Back Translation step per language. Without a + // key the reused instance carries the previous step's recording pass - + // including the take loaded in its recorder - into the next language's + // step, so the other language's audio plays there. (TT-7643) + ) : boldClauseTranscription ? ( // Key on currentstep so the shared transcription component remounts when // moving between the adjacent Careful- and LWC-Transcription steps. Desktop