Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions app/e2e-mock-ipc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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);
}
}
Expand Down
69 changes: 65 additions & 4 deletions app/renderer/src/components/SpeakerReviewPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) => (

@cubic-dev-ai cubic-dev-ai Bot Aug 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The Change picker ordering is recomputed per row render, so larger meetings repeatedly sort the same profiles list and can make the panel feel slower. Reusing one precomputed/memoized ordered profile list for all rows would keep behavior the same with less render work.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/renderer/src/components/SpeakerReviewPanel.tsx, line 544:

<comment>The Change picker ordering is recomputed per row render, so larger meetings repeatedly sort the same profiles list and can make the panel feel slower. Reusing one precomputed/memoized ordered profile list for all rows would keep behavior the same with less render work.</comment>

<file context>
@@ -499,18 +541,37 @@ export function SpeakerReviewPanel({ summaryFile, isDiarised }: SpeakerReviewPan
                       </div>
                     ) : (
-                      (profilesQuery.data ?? []).map((profile) => (
+                      orderProfilesForRow(profilesQuery.data ?? [], alreadyInMeeting).map((profile) => (
                         <div key={profile.person_id} className="flex items-center gap-0.5">
                           <button
</file context>
Fix with cubic

<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"
Expand Down
46 changes: 46 additions & 0 deletions app/renderer/src/components/speakerReviewOrdering.test.ts
Original file line number Diff line number Diff line change
@@ -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']);
});
});
5 changes: 5 additions & 0 deletions app/renderer/src/lib/ipc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading
Loading