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

app/e2e-mock-ipc.js

@@ -837,6 +837,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 +850,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 +939,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 +952,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 +1009,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/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;

e2e/specs/speaker-review.t1.spec.ts

@@ -396,3 +396,54 @@ test('the panel says how many people spoke when a cluster is known to hold more
   await expect(note).toBeVisible();
   await expect(note).toContainText('At least 6 people spoke, but only 5 could be told apart');
 });
+
+test('rows are ordered by speaking time, so the first decisions cover the most transcript', async ({
+  launchApp,
+}) => {
+  const { page } = await launchApp({
+    mockIpc: true,
+    env: { STENOAI_E2E_SEED_SPEAKER_SUGGESTIONS: '1' },
+  });
+  await openDetail(page);
+
+  // The seed's durations are 245 / 80 / 30 s (plus two filtered rows), and
+  // the cluster ids run the other way for SPEAKER_2, so channel+id order --
+  // what the panel used before -- would be a different sequence. Reviewing
+  // is voluntary and abandonable, so whoever stops after one row should
+  // have covered the biggest speaker, not the lowest-numbered slot.
+  const keys = await page
+    .getByTestId('speaker-review-panel')
+    .locator('[data-testid^="speaker-row-"]')
+    .evaluateAll((els) => els.map((el) => el.getAttribute('data-testid')));
+
+  expect(keys).toEqual([
+    'speaker-row-mic:SPEAKER_0',
+    'speaker-row-mic:SPEAKER_1',
+    'speaker-row-mic:SPEAKER_2',
+  ]);
+});
+
+test('the Change picker offers people already assigned in this meeting first', async ({
+  launchApp,
+}) => {
+  const { page } = await launchApp({
+    mockIpc: true,
+    env: { STENOAI_E2E_SEED_SPEAKER_SUGGESTIONS: '1' },
+  });
+  await openDetail(page);
+
+  // One person owning several clusters is the normal case once the diarizer
+  // splits a voice, and it has to be at least as easy to say as "New
+  // person" -- naming the same voice twice makes it a hard negative against
+  // itself, which suppresses that speaker's future suggestions for good.
+  await page.getByTestId('speaker-approve-mic:SPEAKER_0').click();
+  await expect(page.getByTestId('speaker-row-mic:SPEAKER_0')).toContainText('Confirmed as Julian');
+
+  await page.getByTestId('speaker-change-mic:SPEAKER_1').click();
+  const names = await page
+    .locator('[data-testid^="speaker-pick-person-"]')
+    .evaluateAll((els) => els.map((el) => el.textContent?.trim() ?? ''));
+
+  expect(names[0]).toContain('Julian');
+  expect(names[0]).toContain('here');
+});

simple_recorder.py

@@ -5016,11 +5016,26 @@ def confirm_speaker(meeting_stem, channel, diarization_speaker_id, person_id, ne
         if not removed or existing_person["person_id"] == person["person_id"]:
             continue
         reassigned_from.append(existing_person["display_name"])
-        config.remove_speaker_evidence(
-            existing_person["person_id"], meeting_id=meeting_stem,
-            channel=channel, channel_recording_type=channel_recording_type,
-            negative=True,
+        # Their negatives here rest on them having been present in this
+        # channel at all, not on this one cluster -- so drop them only once
+        # they own NO cluster here any more. Under many-to-one one person
+        # legitimately owns several clusters of a meeting; taking one away
+        # used to strip the negatives the clusters they KEEP still justify,
+        # and the rebuild below only restores negatives for the person being
+        # confirmed now, so that evidence was simply lost.
+        still_present = any(
+            p.get("meeting_id") == meeting_stem
+            and prototype_channel_matches(p, channel, channel_recording_type)
+            for p in (config.get_person_profile(existing_person["person_id"]) or {}).get(
+                "prototypes",
+            ) or []
         )
+        if not still_present:
+            config.remove_speaker_evidence(
+                existing_person["person_id"], meeting_id=meeting_stem,
+                channel=channel, channel_recording_type=channel_recording_type,
+                negative=True,
+            )
         for other in config.get_person_profiles():
             if other["person_id"] == existing_person["person_id"]:
                 continue
@@ -5063,30 +5078,61 @@ def confirm_speaker(meeting_stem, channel, diarization_speaker_id, person_id, ne
         sid for sid in clusters
         if sid != resolved_id and not clusters[sid][1].contains_multiple_speakers
     ]
+    # Rebuild rather than append. Confirming the same cluster as the same
+    # person again -- the review UI's Approve on an already-confirmed row, or
+    # simply redoing an assignment -- used to run the loop below a second
+    # time and stack a duplicate of every negative in both directions, once
+    # more on each repeat. Dropping the evidence this cluster produced first
+    # makes the whole step idempotent: the loop then writes exactly the set
+    # the current assignments justify.
+    for existing_person in config.get_person_profiles():
+        config.remove_speaker_evidence(
+            existing_person["person_id"], meeting_id=meeting_stem,
+            channel=channel, channel_recording_type=channel_recording_type,
+            sids=fragment_ids, negative=True,
+        )
+    config.remove_speaker_evidence(
+        person["person_id"], meeting_id=meeting_stem,
+        channel=channel, channel_recording_type=channel_recording_type,
+        negative=True,
+    )
+
     hard_negatives_added = []
     for other_person in config.get_person_profiles():
         if other_person["person_id"] == person["person_id"]:
             continue
-        match = next(
-            (p for p in (other_person.get("prototypes") or [])
-             if p.get("meeting_id") == meeting_stem
-             and p.get("diarization_speaker_id") in other_sids
-             and prototype_channel_matches(p, channel, channel_recording_type)),
-            None,
-        )
-        if match is None:
+        # EVERY cluster that person owns here, not just the first one. One
+        # person legitimately owns several clusters of a meeting -- the
+        # diarizer splits a voice, and the reviewer assigns both halves to
+        # them. Matching only the first prototype left the second cluster
+        # with no negative evidence at all, so a later meeting could still
+        # match this speaker to it.
+        matches = [
+            p for p in (other_person.get("prototypes") or [])
+            if p.get("meeting_id") == meeting_stem
+            and p.get("diarization_speaker_id") in other_sids
+            and prototype_channel_matches(p, channel, channel_recording_type)
+        ]
+        if not matches:
             continue
-        other_sid = match["diarization_speaker_id"]
-        other_embedding, other_context = clusters[other_sid]
-        config.add_speaker_prototype(
-            person["person_id"], other_embedding,
-            recording_type=other_context.recording_type, meeting_id=meeting_stem,
-            diarization_speaker_id=other_sid,
-            speech_duration_seconds=other_context.speech_duration_seconds,
-            segment_count=other_context.segment_count,
-            created_from="user_confirmed", negative=True,
-            channel=channel,
-        )
+        # One negative per THEIR cluster, in this direction only: "the person
+        # I am confirming is none of those clusters".
+        for match in matches:
+            other_sid = match["diarization_speaker_id"]
+            other_embedding, other_context = clusters[other_sid]
+            config.add_speaker_prototype(
+                person["person_id"], other_embedding,
+                recording_type=other_context.recording_type, meeting_id=meeting_stem,
+                diarization_speaker_id=other_sid,
+                speech_duration_seconds=other_context.speech_duration_seconds,
+                segment_count=other_context.segment_count,
+                created_from="user_confirmed", negative=True,
+                channel=channel,
+            )
+        # And exactly ONE the other way: THIS cluster is a single piece of
+        # evidence about them, however many clusters they own. Adding it per
+        # match duplicated it, and every copy is another reason the matcher
+        # refuses a real match for them later.
         config.add_speaker_prototype(
             other_person["person_id"], embedding,
             recording_type=context.recording_type, meeting_id=meeting_stem,
@@ -5533,6 +5579,7 @@ def suggest_speakers(meeting_stem):
             # is gone the moment the panel unmounts (e.g. navigating away
             # and back). This survives both.
             confirmed_by_user = None
+            confirmed_person_id = None
             for person in profiles:
                 if any(
                     p.get("meeting_id") == meeting_stem
@@ -5541,6 +5588,11 @@ def suggest_speakers(meeting_stem):
                     for p in (person.get("prototypes") or [])
                 ):
                     confirmed_by_user = person["display_name"]
+                    # The id as well as the name: display names are not a
+                    # stable identity (a rename can make two profiles read
+                    # alike), and the panel uses this to tell which people
+                    # already hold a cluster of THIS meeting.
+                    confirmed_person_id = person["person_id"]
                     break
             cluster_out[sid] = {
                 "status": r.status,
@@ -5602,6 +5654,7 @@ def suggest_speakers(meeting_stem):
                 # sidecar or from confirm-speaker -- purely a UI hint.
                 "is_likely_artifact": avg_turn < SUGGESTION_MIN_AVG_TURN_SECONDS,
                 "confirmed_by_user": confirmed_by_user,
+                "confirmed_person_id": confirmed_person_id,
             }
         channels_out[channel_name] = cluster_out