feat(speakers): let one person own several clusters, correctly

app/e2e-mock-ipc.js

@@ -23,10 +23,41 @@
 const fs = require('fs');
 const { EXPORT_CANCELED } = require('./ipc-sentinels');
 
-// A real (silent, zero-sample) 16-bit mono 16kHz WAV file's bytes,
-// base64-encoded -- valid enough for the renderer's blob: URL + <audio>
-// playback path to actually decode, unlike an arbitrary placeholder string.
-const MINIMAL_WAV_BASE64 = 'UklGRiQAAABXQVZFZm10IBAAAAABAAEAgD4AAAB9AAACABAAZGF0YQAAAAA=';
+// A real, silent, 16-bit mono 8kHz WAV -- valid enough for the renderer's
+// blob: URL + <audio> path to actually decode, unlike a placeholder string.
+//
+// It has real DURATION, and that is the point. The previous fixture was a
+// 44-byte header with zero sample data, so the media element reported
+// duration 0 and fired `ended` about 300ms after play() -- measured in this
+// very renderer. PlaySampleButton flips its label back to "Play sample" on
+// `ended`, so the "toggling to stop" spec was racing a state that existed
+// for a third of a second: green on a Mac, red on a CI runner whose first
+// assertion poll landed after the clip had already finished. A spec about
+// playing a sample needs a sample that plays.
+const SILENT_WAV_SECONDS = 5;
+const SILENT_WAV_SAMPLE_RATE = 8000;
+
+function buildSilentWavBase64(seconds, sampleRate) {
+  const bytesPerSample = 2; // 16-bit
+  const dataBytes = seconds * sampleRate * bytesPerSample;
+  const buf = Buffer.alloc(44 + dataBytes); // zero-filled == silence
+  buf.write('RIFF', 0);
+  buf.writeUInt32LE(36 + dataBytes, 4);
+  buf.write('WAVE', 8);
+  buf.write('fmt ', 12);
+  buf.writeUInt32LE(16, 16); // PCM fmt chunk size
+  buf.writeUInt16LE(1, 20); // PCM
+  buf.writeUInt16LE(1, 22); // mono
+  buf.writeUInt32LE(sampleRate, 24);
+  buf.writeUInt32LE(sampleRate * bytesPerSample, 28); // byte rate
+  buf.writeUInt16LE(bytesPerSample, 32); // block align
+  buf.writeUInt16LE(16, 34); // bits per sample
+  buf.write('data', 36);
+  buf.writeUInt32LE(dataBytes, 40);
+  return buf.toString('base64');
+}
+
+const MINIMAL_WAV_BASE64 = buildSilentWavBase64(SILENT_WAV_SECONDS, SILENT_WAV_SAMPLE_RATE);
 
 // A deterministic meeting the transcript-export T1 spec navigates to. Seeded
 // only when STENOAI_E2E_SEED_MEETING=1 so the other T1 specs keep an empty Home.
@@ -837,6 +868,7 @@ function install({ ipcMain }) {
           suggested_name: cluster.suggested_name,
           candidates: cluster.candidates,
           confirmed_by_user: cluster.confirmed_by_user,
+          confirmed_person_id: cluster.confirmed_person_id,
         };
         cluster.status = 'none';
         cluster.suggested_person_id = null;
@@ -849,6 +881,7 @@ function install({ ipcMain }) {
         if (cluster.confirmed_by_user) {
           clearedFrom.push(cluster.confirmed_by_user);
           cluster.confirmed_by_user = null;
+          cluster.confirmed_person_id = null;
         }
       } else if (cluster.prevSuggestion) {
         Object.assign(cluster, cluster.prevSuggestion);
@@ -937,6 +970,7 @@ function install({ ipcMain }) {
       const previous = channelSuggestions[diarizationSpeakerId] || {
         speech_duration_seconds: 0, segment_count: 0, first_timestamp: null,
         sample_text: null, is_likely_artifact: false, confirmed_by_user: null,
+        confirmed_person_id: null,
       };
       channelSuggestions[diarizationSpeakerId] = {
         ...previous,
@@ -949,6 +983,10 @@ function install({ ipcMain }) {
         // SpeakerPrototype), so it survives a simulated navigate-away-and-back
         // (a fresh suggest-speakers refetch) even after this panel unmounts.
         confirmed_by_user: person.display_name,
+        // The id travels with the name: the panel decides which people
+        // already hold a cluster of this meeting by id, never by display
+        // name (a rename can leave two profiles reading alike).
+        confirmed_person_id: person.person_id,
       };
 
       return {
@@ -1002,6 +1040,7 @@ function install({ ipcMain }) {
               suggestion.suggested_person_id = null;
               suggestion.suggested_name = null;
               suggestion.confirmed_by_user = null;
+              suggestion.confirmed_person_id = null;
               suggestion.candidates = suggestion.candidates.filter((c) => c.person_id !== id);
             }
           }

app/ipc-contract.test.js

@@ -287,3 +287,27 @@ test('every extracted seam module is required AND invoked in main.js (reachabili
     `extracted module(s) not wired into main.js (dead registration?): ${unwired.join('; ')}`,
   );
 });
+
+// The T1 speaker spec asserts the play button toggles to "Stop sample" while
+// a clip is playing. PlaySampleButton flips that label back on the media
+// element's `ended` event, so the assertion is only stable while the fixture
+// clip is still running. The original fixture was a 44-byte header with zero
+// sample data: measured in the real renderer, it reported duration 0 and
+// fired `ended` ~300ms after play(). That is why the spec was green on a Mac
+// and red on CI - the runner's first assertion poll landed after the clip had
+// already finished.
+//
+// Pinned here rather than left to the spec, because shrinking the fixture
+// again would not fail loudly: it would just make that one test flaky, on
+// someone else's machine, weeks later.
+test('the mock sample clip is long enough for a playing state to be observable', () => {
+  const mock = require('./e2e-mock-ipc.js');
+  void mock; // required only so a syntax error in the fixture fails here too
+
+  const match = MOCK.match(/const SILENT_WAV_SECONDS = (\d+)/);
+  assert.ok(match, 'e2e-mock-ipc.js no longer declares SILENT_WAV_SECONDS');
+  assert.ok(
+    Number(match[1]) >= 2,
+    `the fixture clip is ${match[1]}s; under ~2s the "toggling to stop" assertion races the ended event`,
+  );
+});

app/renderer/src/components/SpeakerReviewPanel.tsx

@@ -67,6 +67,23 @@ function namesCollide(a: string, b: string): boolean {
   return a.trim().toLowerCase() === b.trim().toLowerCase();
 }
 
+/** People already assigned somewhere in this meeting first, the rest after,
+ *  each group alphabetical. The picker is the only route to "this cluster is
+ *  someone I already named here", and that answer gets commoner the more the
+ *  diarizer splits a voice -- burying it in a global list of everyone ever
+ *  named is what pushes a hurried reviewer towards "New person" instead. */
+export function orderProfilesForRow<T extends { display_name: string; person_id: string }>(
+  profiles: T[],
+  alreadyInMeeting: Set<string>,
+): T[] {
+  return [...profiles].sort((a, b) => {
+    const aHere = alreadyInMeeting.has(a.person_id);
+    const bHere = alreadyInMeeting.has(b.person_id);
+    if (aHere !== bHere) return aHere ? -1 : 1;
+    return a.display_name.localeCompare(b.display_name);
+  });
+}
+
 // "mic" is always the device owner's own recording side (in-person audio);
 // "system" is loopback capture of the other call participant(s) -- see
 // determine_recording_type (src/speaker_suggestions.py) for the same
@@ -257,8 +274,33 @@ export function SpeakerReviewPanel({ summaryFile, isDiarised }: SpeakerReviewPan
       rows.push({ channel, diarizationSpeakerId, suggestion });
     }
   }
+  // Most speaking time first. Reviewing is voluntary and can be abandoned at
+  // any point, so the order decides how much of the transcript the first
+  // couple of decisions actually cover -- and the more the diarizer splits a
+  // recording, the further that diverges from channel/cluster-id order,
+  // which is only an artifact of how the diarizer numbered its slots.
+  // Number.isFinite, not `?? 0`: a non-numeric duration would make the
+  // subtraction NaN, and `||` treats NaN as falsy, so a single bad value
+  // would silently drop the whole list back to cluster-id order.
+  const speechSeconds = (row: Row) =>
+    Number.isFinite(row.suggestion.speech_duration_seconds)
+      ? row.suggestion.speech_duration_seconds
+      : 0;
   rows.sort(
-    (a, b) => a.channel.localeCompare(b.channel) || a.diarizationSpeakerId.localeCompare(b.diarizationSpeakerId),
+    (a, b) =>
+      speechSeconds(b) - speechSeconds(a)
+      || a.channel.localeCompare(b.channel)
+      || a.diarizationSpeakerId.localeCompare(b.diarizationSpeakerId),
+  );
+  // People this meeting has already been given a cluster for. Under an
+  // over-segmenting diarizer one person owns several clusters, so this is
+  // the set the reviewer reaches for most, not the long tail of everyone
+  // they have ever named.
+  // By person_id, never by display name: a rename can leave two profiles
+  // reading alike, and marking the wrong one as present here would invite
+  // exactly the misassignment this is meant to prevent.
+  const alreadyInMeeting = new Set(
+    rows.map((row) => row.suggestion.confirmed_person_id).filter((id): id is string => !!id),
   );
   const notDismissed = rows.filter((row) => !dismissed.has(rowKey(row)));
   // A row a human has explicitly marked stays in the main list even if its
@@ -499,18 +541,37 @@ export function SpeakerReviewPanel({ summaryFile, isDiarised }: SpeakerReviewPan
                         No known people yet
                       </div>
                     ) : (
-                      (profilesQuery.data ?? []).map((profile) => (
+                      orderProfilesForRow(profilesQuery.data ?? [], alreadyInMeeting).map((profile) => (
                         <div key={profile.person_id} className="flex items-center gap-0.5">
                           <button
                             type="button"
                             onClick={() => {
                               setChangeOpenFor(null);
                               confirm(row, { personId: profile.person_id });
                             }}
-                            className="flex min-w-0 flex-1 items-center truncate rounded-md px-2 py-1.5 text-left text-[13px] transition-colors hover:bg-[color:var(--surface-hover)]"
+                            className="flex min-w-0 flex-1 items-center gap-1.5 truncate rounded-md px-2 py-1.5 text-left text-[13px] transition-colors hover:bg-[color:var(--surface-hover)]"
                             style={{ color: 'var(--fg-1)' }}
+                            data-testid={`speaker-pick-person-${profile.person_id}`}
                           >
-                            {profile.display_name}
+                            <span className="truncate">{profile.display_name}</span>
+                            {alreadyInMeeting.has(profile.person_id) && (
+                              // The diarizer splits one voice across several
+                              // clusters routinely, so "this is the person I
+                              // already named above" is a frequent, correct
+                              // answer -- and one that has to be visibly
+                              // available, because the alternative a hurried
+                              // user reaches for is "New person", which
+                              // records the same voice as two people and
+                              // makes them a hard negative against
+                              // themselves.
+                              <span
+                                className="shrink-0 text-[11px]"
+                                style={{ color: 'var(--fg-2)' }}
+                                title="Already assigned in this meeting"
+                              >
+                                here
+                              </span>
+                            )}
                           </button>
                           <button
                             type="button"

app/renderer/src/components/speakerReviewOrdering.test.ts

@@ -0,0 +1,46 @@
+import { describe, expect, it } from 'vitest';
+
+import { orderProfilesForRow } from './SpeakerReviewPanel';
+
+const p = (display_name: string) => ({ display_name, person_id: `id-${display_name.toLowerCase()}` });
+
+describe('orderProfilesForRow', () => {
+  it('puts people already assigned in this meeting first', () => {
+    const ordered = orderProfilesForRow(
+      [p('Zoe'), p('Alice'), p('Max')],
+      new Set(['id-max']),
+    );
+    expect(ordered.map((x) => x.display_name)).toEqual(['Max', 'Alice', 'Zoe']);
+  });
+
+  it('keeps each group alphabetical', () => {
+    const ordered = orderProfilesForRow(
+      [p('Zoe'), p('Alice'), p('Max'), p('Bea')],
+      new Set(['id-max', 'id-zoe']),
+    );
+    expect(ordered.map((x) => x.display_name)).toEqual(['Max', 'Zoe', 'Alice', 'Bea']);
+  });
+
+  it('leaves the order alone when nobody is assigned yet', () => {
+    const ordered = orderProfilesForRow([p('Zoe'), p('Alice')], new Set());
+    expect(ordered.map((x) => x.display_name)).toEqual(['Alice', 'Zoe']);
+  });
+
+  it('does not mutate the list it was given', () => {
+    const input = [p('Zoe'), p('Alice')];
+    orderProfilesForRow(input, new Set(['id-alice']));
+    expect(input.map((x) => x.display_name)).toEqual(['Zoe', 'Alice']);
+  });
+});
+
+describe('orderProfilesForRow identity', () => {
+  it('matches on person_id, not on the display name', () => {
+    // Two profiles can read alike after a rename. Marking the never-assigned
+    // one as present in this meeting would invite the exact misassignment
+    // the "here" hint exists to prevent.
+    const assigned = { display_name: 'Alex', person_id: 'id-a' };
+    const other = { display_name: 'Alex', person_id: 'id-b' };
+    const ordered = orderProfilesForRow([other, assigned], new Set(['id-a']));
+    expect(ordered.map((x) => x.person_id)).toEqual(['id-a', 'id-b']);
+  });
+});

app/renderer/src/lib/ipc.ts

@@ -496,6 +496,11 @@ export interface SpeakerSuggestion {
    *  confirmed. Persists across navigation/reload unlike the panel's
    *  transient post-click feedback, which is plain component state. */
   confirmed_by_user: string | null;
+  /** The same confirmation's person_id. Display names are not identity -- a
+   *  rename can leave two profiles reading alike -- so anything deciding
+   *  WHICH person a cluster went to compares this, not the name. Optional:
+   *  a payload predating this field simply carries no id. */
+  confirmed_person_id?: string | null;
 }
 export type SuggestSpeakersResponse = Result<{
   meeting_id: string;