diff --git a/docs/adr/0009-phrase-bt-language-scoping.md b/docs/adr/0009-phrase-bt-language-scoping.md index 65b02f7d2..1840c20ab 100644 --- a/docs/adr/0009-phrase-bt-language-scoping.md +++ b/docs/adr/0009-phrase-bt-language-scoping.md @@ -14,3 +14,5 @@ Non-BOLD **Phrase Back Translation** (and **Retell Back Translation**) must supp - BOLD **LWC Translation** keeps shared **`clause`** boundaries; may still stamp **LWC language** on recordings without per-language clause maps - Language scoping is an **allowlist** keyed on `artifactStampsStepLanguage` — **Phrase BT only**, the one artifact whose takes carry `languagebcp47`. **Whole Back Translation** is recorded by `PassageDetailItem`, which never stamps a language, and vernacular / Q&A / Retell use the org vernacular — a Transcribe step's `language` is their ASR / font / spell-check language only, so scoping their task lists by it would match nothing - The allowlist is deliberately **narrower than `isPhraseSegmentArtifact`**, which also covers **Careful Speech**. Those two predicates answer different questions: `isPhraseSegmentArtifact` means "uses segmented regions on the waveform" (still true of Careful Speech, and still what `phraseRegions` / `phraseArtifactSlug` key on), while `artifactStampsStepLanguage` means "takes carry a language". Careful Speech is BOLD-only — `CAREFUL_SPEECH_CONFIG` sets `requireBoldWorkflow`, which makes `stepLanguageField` resolve to `undefined`, so its takes are recorded untagged and BOLD keeps shared **`clause`** boundaries rather than per-language ones +- **The uploaded file name is part of the discriminator, not decoration.** On the desktop app a take's audio is resolved by name: `useFetchMediaUrl` calls `dataPath(mediafile.audioUrl, PathType.MEDIA)`, which maps to `/media/` and returns that file if it exists — the mediafile id is never consulted, and `store/upload/actions.tsx` stages every online upload into that same folder. Two takes uploaded under one name therefore share one cached file, and the first one cached is what plays for both. `buildFilenamePostfix` must keep every axis that separates one take from another: segment index, source version, **step language**, and a **per-attempt token** (clearing a take deletes its mediafile but not the audio cached under its name, so a re-record that reused the name played the discarded take back — TT-7432). TT-7643 +- **The Transcribe task list shows one take per segment.** A segment can carry several takes — one saved while `rowData` had not caught up, an upload retried, an offline row merged back — and the phrase step itself only ever shows the newest (`pickLatestGuidedOutputRow`). The task list, built from artifact type + language alone, showed all of them, so superseded takes read as extra work. It now collapses to the newest take per `sourceMedia` + `sourceSegments` for phrase-segment artifacts (`latestTakePerSourceSegment`). This does **not** hide a take whose boundaries no longer match the current segment map: it is still the only take for its own region, and hiding it would hide transcription work already done on it. TT-7666 diff --git a/src/renderer/cypress/support/pbtHarness.tsx b/src/renderer/cypress/support/pbtHarness.tsx index 6e643e9af..52226a2d5 100644 --- a/src/renderer/cypress/support/pbtHarness.tsx +++ b/src/renderer/cypress/support/pbtHarness.tsx @@ -80,6 +80,7 @@ export const PBT = { prevUnit: '#phrase-back-translate-prev-unit', nextUnit: '#phrase-back-translate-next-unit', speaker: '#phrase-back-translate-speaker', + clear: '#phrase-back-translate-clear', retrySave: '#phrase-back-translate-retry-save', dockedRecord: '[data-cy="phrase-back-translate-docked-record"]', /** The record control itself (RecordButton renders role=button + aria-disabled). */ diff --git a/src/renderer/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.stepScope.test.tsx b/src/renderer/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.stepScope.test.tsx index 952e79675..199fb9be3 100644 --- a/src/renderer/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.stepScope.test.tsx +++ b/src/renderer/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.stepScope.test.tsx @@ -279,6 +279,44 @@ describe('PassageDetailGuidedPhraseRecord - step scope (TT-7643)', () => { expect(controlsProps?.defaultFilename).toContain('he'); }); + it('names each attempt at a segment apart from the one it replaces', async () => { + // Clearing a take deletes its mediafile but not the audio cached under its + // name, and the next attempt at the same segment in the same language + // built the very same name - so `dataPath` handed the new take the old + // take's file and the recording the user had just discarded played back + // (TT-7432). + await mountAndSettle(); + const firstAttempt = controlsProps?.defaultFilename as string; + expect(firstAttempt).toBeTruthy(); + + await act(async () => { + (controlsProps?.onRecording as (active: boolean) => void)(true); + }); + + await waitFor(() => + expect(controlsProps?.defaultFilename).not.toEqual(firstAttempt) + ); + // Still this segment, in this language - only the attempt is new. + expect(controlsProps?.defaultFilename).toContain('backtranslation1'); + expect(controlsProps?.defaultFilename).toContain('seh'); + }); + + it('holds a name steady for the length of one recording', async () => { + await mountAndSettle(); + await act(async () => { + (controlsProps?.onRecording as (active: boolean) => void)(true); + }); + const whileRecording = controlsProps?.defaultFilename as string; + // The name is chosen when recording starts and has to survive every + // re-render between there and the upload, or MediaRecord would save under + // a different name than the one the step showed. + await act(async () => { + (controlsProps?.onRecording as (active: boolean) => void)(false); + }); + await waitFor(() => expect(controlsProps).toBeDefined()); + expect(controlsProps?.defaultFilename).toEqual(whileRecording); + }); + it('records against the language of the step now showing', async () => { const { rerender } = await mountAndSettle(); expect(controlsProps?.languagebcp47).toBe('Sena|seh'); diff --git a/src/renderer/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.tsx b/src/renderer/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.tsx index 79ade9f69..fbd668799 100644 --- a/src/renderer/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.tsx +++ b/src/renderer/src/components/PassageDetail/PassageDetailGuidedPhraseRecord.tsx @@ -104,6 +104,9 @@ interface IProps { */ const SPURIOUS_STOP_WINDOW_MS = 250; +/** When a take was made, short and filename-safe. */ +const takeStamp = (): string => Date.now().toString(36); + function findClauseIndex(clauseRegions: IRegion[], region: IRegion): number { return clauseRegions.findIndex( (r) => @@ -220,6 +223,16 @@ export function PassageDetailGuidedPhraseRecord({ localStorage.getItem(config.speakerLocalKey) ?? '' ); const [showRecorder, setShowRecorder] = useState(false); + /** + * When this attempt at a segment was made, in the name the take uploads + * under. Clearing a take deletes its mediafile but not the audio cached + * under its name, and `dataPath` resolves a mediafile's audioUrl by that + * name - so a re-record that reused the name played back the take the user + * had just discarded (TT-7432). Renewed when capture starts, and held steady + * from there through the upload. Take *order* comes from `dateCreated`, not + * from this. + */ + const [takeToken, setTakeToken] = useState(takeStamp); const [resetMedia, setResetMedia] = useState(false); const [statusText, setStatusText] = useState(''); const [canSave, setCanSave] = useState(false); @@ -520,11 +533,12 @@ export function PassageDetailGuidedPhraseRecord({ ); const defaultFilename = useMemo(() => { - const postfix = config.buildFilenamePostfix( - currentIndex, - currentVersion, - stepLanguageBcp47 - ); + const postfix = config.buildFilenamePostfix({ + unitIndex: currentIndex, + sourceVersion: currentVersion, + languageBcp47: stepLanguageBcp47, + takeToken, + }); return passageDefaultFilename( passage, plan, @@ -542,6 +556,7 @@ export function PassageDetailGuidedPhraseRecord({ currentIndex, currentVersion, stepLanguageBcp47, + takeToken, config, ]); @@ -1863,6 +1878,7 @@ export function PassageDetailGuidedPhraseRecord({ onRecording={(active) => { if (active) { recordingActiveRef.current = true; + setTakeToken(takeStamp()); // A new take supersedes any earlier rejected save (TT-7583). saveRejectedRef.current = false; setSaveRejected(false); diff --git a/src/renderer/src/components/PassageDetail/PassageDetailPhraseBackTranslate.cy.tsx b/src/renderer/src/components/PassageDetail/PassageDetailPhraseBackTranslate.cy.tsx index 88099205c..310304ad2 100644 --- a/src/renderer/src/components/PassageDetail/PassageDetailPhraseBackTranslate.cy.tsx +++ b/src/renderer/src/components/PassageDetail/PassageDetailPhraseBackTranslate.cy.tsx @@ -32,6 +32,7 @@ import { sourcePlay, startRecordingPass, recordAndSettle, + waitForRecorderIdle, } from '../../../cypress/support/pbtHarness'; const SEGMENTS = SEGMENTS_3; @@ -331,4 +332,30 @@ describe('PBT language scoping', () => { expect(posted?.originalFile).to.not.equal(senaName); }); }); + + it('uploads a re-record under a name of its own', () => { + // Clearing a take deletes its mediafile but not the audio cached under + // its name, so a second attempt at the same segment that reuses the name + // reads back the take the user just discarded (TT-7432). + mountPbt({ segments: SEGMENTS, stepLanguage: 'Hebrew|he' }); + waitForPbtReady(); + startRecordingPass(); + recordAndSettle(1); + + cy.get(PBT.clear).click(); + waitForRecorderIdle(); + expectRecordEnabled(); + recordAndSettle(2); + + cy.then(() => { + const [first, second] = postedTakes(); + expect(first?.parsedSegments, 'same segment both times').to.deep.equal( + second?.parsedSegments + ); + expect( + second?.originalFile, + 'the retake does not reuse the cleared take name' + ).to.not.equal(first?.originalFile); + }); + }); }); diff --git a/src/renderer/src/components/PassageDetail/PassageDetailTranscribe.test.tsx b/src/renderer/src/components/PassageDetail/PassageDetailTranscribe.test.tsx index 2e7af2b02..929f34323 100644 --- a/src/renderer/src/components/PassageDetail/PassageDetailTranscribe.test.tsx +++ b/src/renderer/src/components/PassageDetail/PassageDetailTranscribe.test.tsx @@ -1,7 +1,14 @@ import React from 'react'; import { render, screen } from '@testing-library/react'; -let captured: { hasPermission?: boolean; curRole?: string } = {}; +let captured: { + hasPermission?: boolean; + curRole?: string; + collapseSegmentTakes?: boolean; +} = {}; + +/** Artifact the step under test is configured for; drives the slug mocks. */ +let artifactSlug = 'vernacular'; const linkedSharedResource = { id: 'sr1', @@ -18,7 +25,10 @@ const passageDetailCtx = { orgWorkflowSteps: [ { id: 'step-transcribe', - attributes: { sequencenum: 1, tool: '{"tool":"transcribe","settings":{}}' }, + attributes: { + sequencenum: 1, + tool: '{"tool":"transcribe","settings":{}}', + }, }, ], setStepComplete: jest.fn(), @@ -30,7 +40,10 @@ const passageDetailCtx = { sharedResource: undefined as unknown, }; -jest.mock('../../context/usePassageDetailContext', () => () => passageDetailCtx); +jest.mock( + '../../context/usePassageDetailContext', + () => () => passageDetailCtx +); jest.mock('../../context/PassageDetailContext', () => ({ PassageDetailContext: React.createContext({ setState: jest.fn() }), @@ -39,9 +52,11 @@ jest.mock('../../context/PassageDetailContext', () => ({ jest.mock('../../context/TranscriberContext', () => ({ TranscriberProvider: (props: { curRole?: string; + collapseSegmentTakes?: boolean; children?: React.ReactNode; }) => { captured.curRole = props.curRole; + captured.collapseSegmentTakes = props.collapseSegmentTakes; return <>{props.children}; }, })); @@ -83,20 +98,21 @@ jest.mock('../../crud', () => ({ jest.mock('../../crud/useArtifactType', () => ({ useArtifactType: () => ({ localizedArtifactTypeFromId: () => 'bt', - slugFromId: () => 'vernacular', + slugFromId: () => artifactSlug, }), })); jest.mock('../../crud/artifactTypeSlug', () => ({ ArtifactTypeSlug: { CarefulSpeech: 'carefulspeech' }, artifactStampsStepLanguage: () => false, - isPhraseSegmentArtifact: () => false, + isPhraseSegmentArtifact: (slug: string) => + slug === 'backtranslation' || slug === 'carefulspeech', })); jest.mock('../../crud/related', () => ({ - related: jest.fn(), + related: jest.fn(() => 'mf1'), __esModule: true, - default: jest.fn(), + default: jest.fn(() => 'mf1'), })); jest.mock('../../utils/useStepPermission', () => ({ @@ -155,6 +171,8 @@ import { PassageDetailTranscribe } from './PassageDetailTranscribe'; describe('PassageDetailTranscribe linked note (TT-5873)', () => { beforeEach(() => { captured = {}; + artifactSlug = 'vernacular'; + passageDetailCtx.rowData = []; passageDetailCtx.sharedResource = undefined; passageDetailCtx.mediafileId = 'mf1'; }); @@ -175,3 +193,43 @@ describe('PassageDetailTranscribe linked note (TT-5873)', () => { expect(captured.curRole).toBe('view'); }); }); + +/** + * TT-7666 - a phrase step records one take per segment and shows only the + * newest, but nothing prunes the ones it replaced. The task list was built + * from artifact type and step language alone, so every superseded take + * arrived as its own transcribe task. + */ +describe('PassageDetailTranscribe segment takes (TT-7666)', () => { + beforeEach(() => { + captured = {}; + artifactSlug = 'vernacular'; + passageDetailCtx.rowData = []; + passageDetailCtx.sharedResource = undefined; + passageDetailCtx.mediafileId = 'mf1'; + }); + + const renderForArtifact = (slug: string) => { + artifactSlug = slug; + passageDetailCtx.rowData = [ + { artifactType: 'bt', mediafile: { id: 'take-1' } }, + ] as unknown as typeof passageDetailCtx.rowData; + render(); + }; + + it('collapses a Phrase BT segment to its newest take', () => { + renderForArtifact('backtranslation'); + expect(captured.collapseSegmentTakes).toBe(true); + }); + + it('collapses Careful Speech the same way', () => { + renderForArtifact('carefulspeech'); + expect(captured.collapseSegmentTakes).toBe(true); + }); + + it('leaves a step that is not per-segment listing everything', () => { + // Whole BT, Q&A, Retell: one take for the passage, nothing to collapse. + renderForArtifact('wholebacktranslation'); + expect(captured.collapseSegmentTakes).toBe(false); + }); +}); diff --git a/src/renderer/src/components/PassageDetail/PassageDetailTranscribe.tsx b/src/renderer/src/components/PassageDetail/PassageDetailTranscribe.tsx index 72de9a697..d6dbf7923 100644 --- a/src/renderer/src/components/PassageDetail/PassageDetailTranscribe.tsx +++ b/src/renderer/src/components/PassageDetail/PassageDetailTranscribe.tsx @@ -317,6 +317,7 @@ export function PassageDetailTranscribe({ width, artifactTypeId }: IProps) { artifactTypeId={artifactTypeId} curRole={curRole as string} stepLanguageBcp47={stepLanguageBcp47} + collapseSegmentTakes={Boolean(phraseArtifactSlug)} > {phase === 'recorded' && !readOnly && ( diff --git a/src/renderer/src/components/PassageDetail/carefulSpeech/matchesGuidedOutputRow.ts b/src/renderer/src/components/PassageDetail/carefulSpeech/matchesGuidedOutputRow.ts index bab8496e5..742cf6f4c 100644 --- a/src/renderer/src/components/PassageDetail/carefulSpeech/matchesGuidedOutputRow.ts +++ b/src/renderer/src/components/PassageDetail/carefulSpeech/matchesGuidedOutputRow.ts @@ -1,4 +1,5 @@ import { related } from '../../../crud/related'; +import { isNewerTake } from '../../../crud/latestTakePerSourceSegment'; import { IRow } from '../../../context/PassageDetailContext'; import { mediaMatchesStepLanguage } from '../../../utils/mediaLanguage'; @@ -44,15 +45,13 @@ export function matchesGuidedOutputRow( return mediaMatchesStepLanguage(row.mediafile, opts.languageBcp47); } +/** The take the step shows, by the same rule that prunes the ones it hides. */ export function pickLatestGuidedOutputRow(matches: IRow[]): IRow | undefined { - if (matches.length === 0) return undefined; - if (matches.length === 1) return matches[0]; - return [...matches].sort((a, b) => { - const da = a.mediafile?.attributes?.dateCreated ?? ''; - const db = b.mediafile?.attributes?.dateCreated ?? ''; - if (da !== db) return db.localeCompare(da); - return (b.mediafile?.id ?? '').localeCompare(a.mediafile?.id ?? ''); - })[0]; + return matches.reduce( + (best, row) => + !best || isNewerTake(row.mediafile, best.mediafile) ? row : best, + undefined + ); } /** Named-region key for Phrase BT segment boundaries for a language. */ diff --git a/src/renderer/src/components/PassageDetail/guidedPhraseRecord/types.test.ts b/src/renderer/src/components/PassageDetail/guidedPhraseRecord/types.test.ts index 1fb961201..5678e539a 100644 --- a/src/renderer/src/components/PassageDetail/guidedPhraseRecord/types.test.ts +++ b/src/renderer/src/components/PassageDetail/guidedPhraseRecord/types.test.ts @@ -51,8 +51,12 @@ describe('guidedPhraseRecord config', () => { ArtifactTypeSlug.PhraseBackTranslation, NamedRegions.BackTranslation ); - expect(config.buildFilenamePostfix(0, 2)).toBe('backtranslation1_v2'); - expect(config.buildFilenamePostfix(1, 2)).toBe('backtranslation2_v2s1'); + expect( + config.buildFilenamePostfix({ unitIndex: 0, sourceVersion: 2 }) + ).toBe('backtranslation1_v2'); + expect( + config.buildFilenamePostfix({ unitIndex: 1, sourceVersion: 2 }) + ).toBe('backtranslation2_v2s1'); }); it('buildFilenamePostfix separates the languages of the same segment', () => { @@ -66,16 +70,46 @@ describe('guidedPhraseRecord config', () => { ArtifactTypeSlug.PhraseBackTranslation, NamedRegions.BackTranslation ); - expect(config.buildFilenamePostfix(0, 1, 'seh')).toBe( - 'backtranslation1_v1_seh' - ); - expect(config.buildFilenamePostfix(0, 1, 'he')).toBe( + const parts = { unitIndex: 0, sourceVersion: 1 }; + expect( + config.buildFilenamePostfix({ ...parts, languageBcp47: 'seh' }) + ).toBe('backtranslation1_v1_seh'); + expect(config.buildFilenamePostfix({ ...parts, languageBcp47: 'he' })).toBe( 'backtranslation1_v1_he' ); - expect(config.buildFilenamePostfix(1, 1, 'he')).toBe( - 'backtranslation2_v1s1_he' - ); + expect( + config.buildFilenamePostfix({ + unitIndex: 1, + sourceVersion: 1, + languageBcp47: 'he', + }) + ).toBe('backtranslation2_v1s1_he'); // Steps with no configured language keep the names they always had. - expect(config.buildFilenamePostfix(0, 1)).toBe('backtranslation1_v1'); + expect(config.buildFilenamePostfix(parts)).toBe('backtranslation1_v1'); + }); + + it('buildFilenamePostfix separates one attempt at a segment from the next', () => { + // Clearing a take deletes its mediafile but not the audio cached under + // its name, so a re-record that reused the name played the discarded take + // back (TT-7432). + const config = phraseBackTranslateConfig( + ArtifactTypeSlug.PhraseBackTranslation, + NamedRegions.BackTranslation + ); + const parts = { unitIndex: 0, sourceVersion: 1, languageBcp47: 'seh' }; + expect(config.buildFilenamePostfix({ ...parts, takeToken: 'aaa' })).toBe( + 'backtranslation1_v1_seh_taaa' + ); + expect( + config.buildFilenamePostfix({ ...parts, takeToken: 'aaa' }) + ).not.toEqual(config.buildFilenamePostfix({ ...parts, takeToken: 'bbb' })); + // Careful Speech records per clause and needs the same separation. + expect( + CAREFUL_SPEECH_CONFIG.buildFilenamePostfix({ + unitIndex: 1, + sourceVersion: 3, + takeToken: 'zzz', + }) + ).toBe('carefulspeech2_v3_tzzz'); }); }); diff --git a/src/renderer/src/components/PassageDetail/guidedPhraseRecord/types.ts b/src/renderer/src/components/PassageDetail/guidedPhraseRecord/types.ts index b582628bf..a309e0c0a 100644 --- a/src/renderer/src/components/PassageDetail/guidedPhraseRecord/types.ts +++ b/src/renderer/src/components/PassageDetail/guidedPhraseRecord/types.ts @@ -23,6 +23,23 @@ export interface IGuidedPhraseRecordControlStrings { resetConfirmBoundaries?: string; } +/** What distinguishes one guided-phrase take from another. */ +export interface GuidedPhraseFilenameParts { + /** 0-based segment/clause index. */ + unitIndex: number; + /** Version of the vernacular being recorded against. */ + sourceVersion: number; + /** Step language, for steps that can have a sibling step in another one. */ + languageBcp47?: string; + /** + * When this attempt was made, distinguishing it from the ones it replaces. + * Clearing a take deletes its mediafile but not the audio cached under its + * name, so without this the next attempt at the same segment reads back the + * discarded one. + */ + takeToken?: string; +} + export interface GuidedPhraseRecordConfig { namedRegion: NamedRegions; defaultArtifactSlug: ArtifactTypeSlug; @@ -46,20 +63,15 @@ export interface GuidedPhraseRecordConfig { /** Persist segment map on vernacular named regions (false for Retell). */ persistSegments: boolean; /** - * Filename postfix for a unit at `unitIndex` (0-based) on `sourceVersion`. + * Filename postfix for one take. * * 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). + * is what plays for both (TT-7643). */ - buildFilenamePostfix: ( - unitIndex: number, - sourceVersion: number, - languageBcp47?: string - ) => string; + buildFilenamePostfix: (opts: GuidedPhraseFilenameParts) => string; } const carefulSpeechBoundaryDefaults = { @@ -81,8 +93,10 @@ export const CAREFUL_SPEECH_CONFIG: GuidedPhraseRecordConfig = { containerId: 'careful-speech', requireBoldWorkflow: true, ...carefulSpeechBoundaryDefaults, - buildFilenamePostfix: (unitIndex, sourceVersion) => - `carefulspeech${unitIndex + 1}_v${sourceVersion}`, + buildFilenamePostfix: ({ unitIndex, sourceVersion, takeToken }) => + `carefulspeech${unitIndex + 1}_v${sourceVersion}${ + takeToken ? `_t${takeToken}` : '' + }`, }; export function phraseBackTranslateConfig( @@ -106,13 +120,19 @@ export function phraseBackTranslateConfig( multiLevelSegmentUndo: phraseBoundaryTools, sequentialUnitNavAroundRecord: phraseBoundaryTools, persistSegments: phraseBoundaryTools, - buildFilenamePostfix: (unitIndex, sourceVersion, languageBcp47) => { + buildFilenamePostfix: ({ + unitIndex, + sourceVersion, + languageBcp47, + takeToken, + }) => { const base = `${artifactSlug}${unitIndex + 1}_v${sourceVersion}`; 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; + const lang = languageBcp47 ? `${unit}_${languageBcp47}` : unit; + return `${lang}${takeToken ? `_t${takeToken}` : ''}`; }, }; } diff --git a/src/renderer/src/context/TranscriberContext.tsx b/src/renderer/src/context/TranscriberContext.tsx index 70a38384b..88edddc96 100644 --- a/src/renderer/src/context/TranscriberContext.tsx +++ b/src/renderer/src/context/TranscriberContext.tsx @@ -33,6 +33,7 @@ import { } from '../crud'; import { mediaFileName } from '../crud/media'; import { mediaMatchesStepLanguage } from '../utils/mediaLanguage'; +import { latestTakePerSourceSegment } from '../crud/latestTakePerSourceSegment'; import StickyRedirect from '../components/StickyRedirect'; import { useSelector } from 'react-redux'; import { useDispatch } from 'react-redux'; @@ -133,9 +134,16 @@ interface IProps { curRole?: string; /** Step language. When set (and not `und`), only media tagged with it become tasks. */ stepLanguageBcp47?: string; + /** + * Set for the phrase steps (Phrase BT, Careful Speech), whose takes are one + * per segment of the vernacular. Only the newest take of a segment becomes a + * task; see {@link latestTakePerSourceSegment}. + */ + collapseSegmentTakes?: boolean; } const TranscriberProvider = (props: IProps) => { - const { artifactTypeId, curRole, stepLanguageBcp47 } = props; + const { artifactTypeId, curRole, stepLanguageBcp47, collapseSegmentTakes } = + props; const [isDetail] = useState(artifactTypeId !== undefined); const passages = useOrbitData('passage'); const sections = useOrbitData('section'); @@ -200,10 +208,19 @@ const TranscriberProvider = (props: IProps) => { return; } m = m.filter((mf) => mediaMatchesStepLanguage(mf, stepLanguageBcp47)); + if (collapseSegmentTakes) m = latestTakePerSourceSegment(m); setPlanMedia(m); planMediaRef.current = m; // eslint-disable-next-line react-hooks/exhaustive-deps - }, [mediafiles, devPlan, artifactId, stepLanguageBcp47, pasId, memory]); + }, [ + mediafiles, + devPlan, + artifactId, + stepLanguageBcp47, + collapseSegmentTakes, + pasId, + memory, + ]); const setRows = (rowData: IRowData[]) => { setState((state: ICtxState) => { diff --git a/src/renderer/src/crud/latestTakePerSourceSegment.test.ts b/src/renderer/src/crud/latestTakePerSourceSegment.test.ts new file mode 100644 index 000000000..26d55d842 --- /dev/null +++ b/src/renderer/src/crud/latestTakePerSourceSegment.test.ts @@ -0,0 +1,115 @@ +import { MediaFileD } from '../model'; +import { latestTakePerSourceSegment } from './latestTakePerSourceSegment'; + +/** + * TT-7666 - the Transcribe task list was built from artifact type (and, since + * TT-7557, step language) alone, so every take a segment had ever carried + * arrived as its own task. Superseded takes read as extra work to transcribe, + * and the transcriber could not tell which one the phrase step is showing. + */ + +const take = ( + id: string, + seg: { start: number; end: number } | string | null, + dateCreated: string, + sourceMedia = 'mf-vern' +): MediaFileD => + ({ + id, + type: 'mediafile', + attributes: { + dateCreated, + sourceSegments: + seg === null + ? null + : typeof seg === 'string' + ? seg + : JSON.stringify({ ...seg, label: '' }), + }, + relationships: sourceMedia + ? { sourceMedia: { data: { type: 'mediafile', id: sourceMedia } } } + : {}, + }) as unknown as MediaFileD; + +const ids = (media: MediaFileD[]) => media.map((m) => m.id); + +describe('latestTakePerSourceSegment', () => { + it('keeps only the newest take of a segment', () => { + const media = [ + take('old', { start: 0, end: 3 }, '2026-08-01T10:00:00Z'), + take('new', { start: 0, end: 3 }, '2026-08-02T10:00:00Z'), + take('other-segment', { start: 3, end: 6 }, '2026-08-01T10:00:00Z'), + ]; + expect(ids(latestTakePerSourceSegment(media))).toEqual([ + 'new', + 'other-segment', + ]); + }); + + it('keeps the newest whichever order they arrive in', () => { + const media = [ + take('new', { start: 0, end: 3 }, '2026-08-02T10:00:00Z'), + take('old', { start: 0, end: 3 }, '2026-08-01T10:00:00Z'), + ]; + expect(ids(latestTakePerSourceSegment(media))).toEqual(['new']); + }); + + it('breaks a dateCreated tie the way the phrase step does', () => { + const media = [ + take('aaa', { start: 0, end: 3 }, '2026-08-01T10:00:00Z'), + take('zzz', { start: 0, end: 3 }, '2026-08-01T10:00:00Z'), + ]; + // isNewerTake, which the phrase step's own pick also calls. + expect(ids(latestTakePerSourceSegment(media))).toEqual(['zzz']); + }); + + it('treats the same segment on a different vernacular as its own', () => { + const media = [ + take('v1-take', { start: 0, end: 3 }, '2026-08-01T10:00:00Z', 'mf-v1'), + take('v2-take', { start: 0, end: 3 }, '2026-08-02T10:00:00Z', 'mf-v2'), + ]; + expect(ids(latestTakePerSourceSegment(media))).toEqual([ + 'v1-take', + 'v2-take', + ]); + }); + + it('ignores boundary noise below the display precision', () => { + const media = [ + take('old', { start: 0, end: 3.001 }, '2026-08-01T10:00:00Z'), + take('new', { start: 0, end: 3.002 }, '2026-08-02T10:00:00Z'), + ]; + expect(ids(latestTakePerSourceSegment(media))).toEqual(['new']); + }); + + it('leaves media that are not per-segment takes alone', () => { + const media = [ + take('vernacular', null, '2026-08-01T10:00:00Z'), + take('whole-bt', '{}', '2026-08-02T10:00:00Z'), + take('unparseable', 'not json', '2026-08-03T10:00:00Z'), + ]; + expect(ids(latestTakePerSourceSegment(media))).toEqual([ + 'vernacular', + 'whole-bt', + 'unparseable', + ]); + }); + + it('leaves a take whose vernacular cannot be read alone', () => { + // Boundaries alone do not make two takes the same segment - without a + // sourceMedia they could be cut from different vernaculars, and collapsing + // them would drop a task that is really someone else's work. + const media = [ + take('no-source-old', { start: 0, end: 3 }, '2026-08-01T10:00:00Z', ''), + take('no-source-new', { start: 0, end: 3 }, '2026-08-02T10:00:00Z', ''), + ]; + expect(ids(latestTakePerSourceSegment(media))).toEqual([ + 'no-source-old', + 'no-source-new', + ]); + }); + + it('returns an empty list unchanged', () => { + expect(latestTakePerSourceSegment([])).toEqual([]); + }); +}); diff --git a/src/renderer/src/crud/latestTakePerSourceSegment.ts b/src/renderer/src/crud/latestTakePerSourceSegment.ts new file mode 100644 index 000000000..07046afa2 --- /dev/null +++ b/src/renderer/src/crud/latestTakePerSourceSegment.ts @@ -0,0 +1,65 @@ +import { MediaFile } from '../model'; +import { related } from './related'; + +/** + * Collapse guided-phrase takes to one per segment: the newest. + * + * A segment can end up with several takes if the user re-records - nothing prunes the superseded + * ones - and only the newest should be shown. Use this filter to avoid showing the dead extra takes, + * as happened in the Transcribe task list before TT-7666. + * + * Takes whose `sourceSegments` cannot be read are left alone: they are not + * per-segment takes, so there is nothing to collapse them against. So are takes + * with no readable `sourceMedia`: without knowing which vernacular a take was + * cut from, boundaries alone do not say two takes are of the same segment, and + * collapsing on them would drop a task that is really someone else's work. + */ + +/** `start|end` at 2dp, or undefined when this is not a per-segment take. */ +function segmentKey(seg: string | undefined | null): string | undefined { + if (!seg) return undefined; + try { + const parsed = JSON.parse(seg) as { start?: number; end?: number }; + if (typeof parsed?.start !== 'number' || typeof parsed?.end !== 'number') { + return undefined; + } + return `${parsed.start.toFixed(2)}|${parsed.end.toFixed(2)}`; + } catch { + return undefined; + } +} + +/** + * Which of two takes of a segment supersedes the other: the later + * `dateCreated`, and on a tie the higher id, so the answer is stable whatever + * order the rows arrive in. `pickLatestGuidedOutputRow` shows what this keeps, + * so it has to agree - it calls this. + */ +export function isNewerTake( + candidate: MediaFile | undefined, + incumbent: MediaFile | undefined +): boolean { + const dc = candidate?.attributes?.dateCreated ?? ''; + const di = incumbent?.attributes?.dateCreated ?? ''; + if (dc !== di) return dc > di; + return (candidate?.id ?? '') > (incumbent?.id ?? ''); +} + +export function latestTakePerSourceSegment( + media: T[] +): T[] { + const winners = new Map(); + const perSegment = new Set(); + for (const m of media) { + const seg = segmentKey(m.attributes?.sourceSegments); + if (seg === undefined) continue; + const source = related(m, 'sourceMedia'); + if (!source) continue; + perSegment.add(m); + const key = `${source}|${seg}`; + const held = winners.get(key); + if (!held || isNewerTake(m, held)) winners.set(key, m); + } + const kept = new Set(winners.values()); + return media.filter((m) => !perSegment.has(m) || kept.has(m)); +}