TT-7666, TT-7432 fix: one task per segment in PBT Transcribe, and a fresh name for every take

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 `<offlineData>/media/<basename>` 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

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). */

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');

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);

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);
+    });
+  });
 });

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(<PassageDetailTranscribe width={400} artifactTypeId="at1" />);
+  };
+
+  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);
+  });
+});

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)}
     >
       <Grid
         container

src/renderer/src/components/PassageDetail/carefulSpeech/CarefulSpeechControls.tsx

@@ -364,6 +364,7 @@ export default function CarefulSpeechControls({
             />
             {phase === 'recorded' && !readOnly && (
               <IconButton
+                id={`${controlIdPrefix}-clear`}
                 aria-label={strings.clearRecording}
                 onClick={onClearRecording}
               >

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<IRow | undefined>(
+    (best, row) =>
+      !best || isNewerTake(row.mediafile, best.mediafile) ? row : best,
+    undefined
+  );
 }
 
 /** Named-region key for Phrase BT segment boundaries for a language. */