feat(speakers): let one person own several clusters, correctly - #474
Merged
Optic00 merged 6 commits intoAug 5, 2026
Merged
Conversation
…rect answer A diarizer splits one voice across several clusters routinely, and the wider the split the likelier it is. Assigning several clusters to the same person was already possible, but it was neither the obvious action nor fully correct. Two evidence bugs, each with a failing test first: - The mutual hard-negative loop matched only the FIRST prototype per other person (`next(...)`). When someone owned two clusters of a meeting, the second produced no negative evidence at all, so a later meeting could still match that speaker to it. - Reassigning a cluster away from someone removed ALL their negatives for that meeting and channel. If they still owned another cluster there, that cluster's evidence went with it, and the rebuild below only restores negatives for the person being confirmed now. The removal is now conditional on them no longer owning any cluster in the channel - their negatives rest on having been present at all, not on this one cluster, which is why scoping the removal to the cluster's own id would have been wrong (it would have kept a negative that only the removed confirmation ever justified). Two UI changes: - Rows are ordered by speaking time. Reviewing is voluntary and can be abandoned at any point, so the order decides how much of the transcript the first decisions cover. Channel and cluster id, the previous order, is an artifact of how the diarizer numbered its slots. - The Change picker lists people already assigned in this meeting first, marked "here". This is the answer that gets commoner the more a voice is split, and the alternative a hurried reviewer reaches for is "New person" - which records one voice as two people and makes them a hard negative against themselves, permanently suppressing their future suggestions. Verified: 918 Python tests (basis 916), 156 vitest, 17/17 speaker-review.t1 including two new specs, typecheck clean, lint 37/0 and ruff 41 as the basis. The one red test (test_bundle_mlx) is the documented pre-existing harness artifact, 10/10 green in isolation.
Four review findings, two of them on the commit before this one. Mine: looping over every prototype the other person owns here added the REVERSE negative once per match, so a person owning two clusters produced a duplicated negative in the other direction. This cluster is one piece of evidence about them however many clusters they own, so that add moved out of the loop. Pre-existing (verified failing on the parent commit): confirming the same cluster as the same person again -- the review UI's Approve on an already-confirmed row -- ran the mutual-negative loop a second time and stacked a duplicate of every negative in both directions, once more on each repeat. The evidence this cluster produced is now dropped before the loop rebuilds it, which makes the step idempotent. Every duplicate matters: hard negatives are permanent suppression, so each copy is another reason the matcher refuses a real match later. The panel decided "already assigned in this meeting" by display name. A rename can leave two profiles reading alike, and the never-assigned one would then be marked as present -- inviting exactly the misassignment the hint exists to prevent. suggest-speakers now emits confirmed_person_id alongside the name, and the comparison uses it. The field is optional, so a payload predating it simply carries no id. Sorting guarded with Number.isFinite rather than `??`: a non-numeric duration made the subtraction NaN, and `||` treats NaN as falsy, so one bad value would have silently dropped the list back to cluster-id order. Verified: 942 Python tests (basis 916), 157 vitest, 17/17 speaker-review.t1, typecheck clean, lint 37/0, ruff 41 as the basis. The one red test (test_bundle_mlx) is the documented pre-existing harness artifact.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
3 issues found across 7 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/renderer/src/components/SpeakerReviewPanel.tsx">
<violation number="1" location="app/renderer/src/components/SpeakerReviewPanel.tsx:544">
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.</violation>
</file>
<file name="tests/test_confirm_speaker_cli.py">
<violation number="1" location="tests/test_confirm_speaker_cli.py:641">
P2: The new `ConfirmSpeakerEvidenceHygieneTests` class sits below the `if __name__ == "__main__": unittest.main()` guard, so running `python tests/test_confirm_speaker_cli.py` directly won't collect its two new tests — they only run under `python -m unittest discover`. Direct-run and discovery behavior diverge silently; move the class above the guard (or move `unittest.main()` to the very end of the file) so both entry points run the same suite.</violation>
</file>
<file name="e2e/specs/speaker-review.t1.spec.ts">
<violation number="1" location="e2e/specs/speaker-review.t1.spec.ts:419">
P2: This ordering test can't catch a regression to cluster-id order: the seeded durations (245/80/30) are already monotonically decreasing with speaker id, so both the old id order and the new duration-descending order render [SPEAKER_0, SPEAKER_1, SPEAKER_2]. The test passes regardless of which ordering the panel implements, giving false confidence that the feature is covered. Consider giving at least one cluster a duration that breaks the id↔duration alignment (e.g. make SPEAKER_1 or SPEAKER_2 the longest) so the expected sequence actually differs from ascending id order.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| unittest.main() | ||
|
|
||
|
|
||
| class ConfirmSpeakerEvidenceHygieneTests(ConfirmSpeakerCliTests): |
Contributor
There was a problem hiding this comment.
P2: The new ConfirmSpeakerEvidenceHygieneTests class sits below the if __name__ == "__main__": unittest.main() guard, so running python tests/test_confirm_speaker_cli.py directly won't collect its two new tests — they only run under python -m unittest discover. Direct-run and discovery behavior diverge silently; move the class above the guard (or move unittest.main() to the very end of the file) so both entry points run the same suite.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_confirm_speaker_cli.py, line 641:
<comment>The new `ConfirmSpeakerEvidenceHygieneTests` class sits below the `if __name__ == "__main__": unittest.main()` guard, so running `python tests/test_confirm_speaker_cli.py` directly won't collect its two new tests — they only run under `python -m unittest discover`. Direct-run and discovery behavior diverge silently; move the class above the guard (or move `unittest.main()` to the very end of the file) so both entry points run the same suite.</comment>
<file context>
@@ -557,3 +636,43 @@ def test_noops_when_no_summary_file_exists(self):
unittest.main()
+
+
+class ConfirmSpeakerEvidenceHygieneTests(ConfirmSpeakerCliTests):
+ """Hard negatives are permanent suppression evidence, so a duplicate is
+ not merely untidy: every copy is another reason the matcher will refuse a
</file context>
| .locator('[data-testid^="speaker-row-"]') | ||
| .evaluateAll((els) => els.map((el) => el.getAttribute('data-testid'))); | ||
|
|
||
| expect(keys).toEqual([ |
Contributor
There was a problem hiding this comment.
P2: This ordering test can't catch a regression to cluster-id order: the seeded durations (245/80/30) are already monotonically decreasing with speaker id, so both the old id order and the new duration-descending order render [SPEAKER_0, SPEAKER_1, SPEAKER_2]. The test passes regardless of which ordering the panel implements, giving false confidence that the feature is covered. Consider giving at least one cluster a duration that breaks the id↔duration alignment (e.g. make SPEAKER_1 or SPEAKER_2 the longest) so the expected sequence actually differs from ascending id order.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At e2e/specs/speaker-review.t1.spec.ts, line 419:
<comment>This ordering test can't catch a regression to cluster-id order: the seeded durations (245/80/30) are already monotonically decreasing with speaker id, so both the old id order and the new duration-descending order render [SPEAKER_0, SPEAKER_1, SPEAKER_2]. The test passes regardless of which ordering the panel implements, giving false confidence that the feature is covered. Consider giving at least one cluster a duration that breaks the id↔duration alignment (e.g. make SPEAKER_1 or SPEAKER_2 the longest) so the expected sequence actually differs from ascending id order.</comment>
<file context>
@@ -396,3 +396,54 @@ test('the panel says how many people spoke when a cluster is known to hold more
+ .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',
</file context>
| </div> | ||
| ) : ( | ||
| (profilesQuery.data ?? []).map((profile) => ( | ||
| orderProfilesForRow(profilesQuery.data ?? [], alreadyInMeeting).map((profile) => ( |
Contributor
There was a problem hiding this comment.
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>
… turn The quote is cut at 140 characters and the clip at 20 seconds, and nothing related the two. Measured on a real 9-minute call: a 40.5 s / 742-character turn showed 19 % of its text beside 20 s of audio. Both start at the same instant, but reading one sentence while hearing twenty seconds reads as the wrong clip. Ranking by raw duration made it likelier, not less likely, because overflowing is what being long means here - the same amplification that made a drifted clip more likely to be shown before _turn_audio_range stopped drifting. Turns that survive both caps whole now rank first, longest among them, and overflowing ones only fill the remaining slots. A fitting turn is still chosen longest-first, so the clip stays long enough to recognise a voice from. On the reported meeting this replaces two truncated excerpts with turns that show their text complete; all five now do. A second finding from the same report is deliberately NOT fixed here: a turn can begin mid-sentence, because src.transcriber splits a sentence longer than LONG_SENTENCE_SPLIT_THRESHOLD_S word by word when it overlaps several diarizer segments, so one speaker never receives another's words. Marking those quotes as continuations was tried and reverted: a lower-case opening is not the signal it looks like, and guessing from it mislabelled fifteen legitimate turns across the existing tests. A truthful marker has to come from the splitter recording that it split, not from the display layer guessing, and that is a manifest change of its own.
Design for the additive slice that comes before the assignment/evidence inversion: a run id in the sidecar, prototypes that remember which run they were confirmed against, one persisted review state (generic), and run scope on both the read and the write path. Pressure-tested by two independent reviews. The central finding they both raised is in section 5: remove_speaker_evidence matches only (meeting_id, channel, sids), so after a re-diarization the recycled SPEAKER_0 makes a confirmation delete the previous person's genuine old-run prototype. Without run scope the design contradicts its own promise that nothing is deleted. Section 5 also states the trade this buys, rather than hiding it: a wrong old-run confirmation can no longer be corrected by re-confirming, only by deleting the person or via the repair CLI.
Two findings from the review of the fit ranking. Preferring turns that survive both display caps let a 0.1 s "Ja" outrank a 19 s turn whose quote missed the character cap by a single character. Fitting is not usefulness: the panel exists so a human can recognise a voice, and a fraction of a second cannot carry one however faithfully it is quoted. A clip now has to clear SAMPLE_MIN_USEFUL_SECONDS to earn the preference. That constant is marked as CHOSEN rather than measured, unlike the ground-truthed neighbour it sits next to, which answers a different question. And the audio cap was compared with `<` while the character cap uses `<=`, so a turn ending exactly at the limit counted as overflowing although it survives whole. Both are `<=` now. Also adds the implementation plan for the run-provenance slice. Verified: 946 Python tests (basis 944), ruff 41 as the basis, and the reported meeting's five excerpts are unchanged - all still show their text complete.
Contributor
There was a problem hiding this comment.
5 issues found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/superpowers/plans/2026-08-04-speaker-review-run-provenance.md">
<violation number="1" location="docs/superpowers/plans/2026-08-04-speaker-review-run-provenance.md:20">
P3: The verification gate can produce false failures because the plan hard-codes an outdated Python test baseline (`944`) that no longer matches current branch expectations. Consider updating this baseline to the current measured value (and aligned references below) so reviewers compare against the right target.</violation>
</file>
<file name="docs/superpowers/specs/2026-08-04-speaker-review-run-provenance-design.md">
<violation number="1" location="docs/superpowers/specs/2026-08-04-speaker-review-run-provenance-design.md:78">
P3: The spec changes 'Keep generic' from hiding the row to 'quietly marked', but never defines how a generic row is visually distinguished from an unreviewed one, nor the undo affordance. Without a specified indicator (e.g. an outline/check state analogous to isMarked), a persisted-but-invisible marker gives no feedback and the promised 'undo one click away' is undiscoverable, partially defeating the persistence goal. Please specify the rendered state for a generic-marked row (and its data-testid) in the build contract.</violation>
</file>
<file name="src/speaker_suggestions.py">
<violation number="1" location="src/speaker_suggestions.py:1751">
P3: The new internal `fits` boolean used only for ranking gets carried into the returned sample dicts, and since `simple_recorder.py` serializes `cluster_samples` straight into the suggest-speakers `samples` payload, the field now leaks over the API even though the documented return contract is just `{start, end, text}`. Current renderer consumers only read start/end/text so nothing breaks today, but it widens the wire schema with an implementation detail. Consider dropping `fits` from the entries before returning (for example, strip it from each chosen candidate or build the return dicts without it) so the endpoint keeps emitting exactly the documented fields.</violation>
<violation number="2" location="src/speaker_suggestions.py:1753">
P3: This `fits` condition includes a duration cap that is already guaranteed earlier, so this branch is dead logic and makes the ranking predicate harder to read. Removing the redundant check keeps behavior identical.</violation>
<violation number="3" location="src/speaker_suggestions.py:1767">
P2: Collapsed quote selection can still favor a long overflowing excerpt even after fit-first ranking, so the summary text may not reflect the most review-useful sample. This happens because ranking changed in `extract_segment_samples`, but `sample_text_from_samples` still chooses by raw duration only.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
| # recognise a voice from. | ||
| chosen = sorted( | ||
| candidates, | ||
| key=lambda c: (c["fits"], c["end"] - c["start"]), |
Contributor
There was a problem hiding this comment.
P2: Collapsed quote selection can still favor a long overflowing excerpt even after fit-first ranking, so the summary text may not reflect the most review-useful sample. This happens because ranking changed in extract_segment_samples, but sample_text_from_samples still chooses by raw duration only.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/speaker_suggestions.py, line 1767:
<comment>Collapsed quote selection can still favor a long overflowing excerpt even after fit-first ranking, so the summary text may not reflect the most review-useful sample. This happens because ranking changed in `extract_segment_samples`, but `sample_text_from_samples` still chooses by raw duration only.</comment>
<file context>
@@ -1723,12 +1737,36 @@ def extract_segment_samples(
+ # recognise a voice from.
+ chosen = sorted(
+ candidates,
+ key=lambda c: (c["fits"], c["end"] - c["start"]),
+ reverse=True,
+ )[:limit]
</file context>
| - The staleness rule exists **once**, as a shared predicate in `src/speaker_suggestions.py`, imported by `src/config.py` and `simple_recorder.py` the same way `prototype_channel_matches` already is. A second copy of the rule is a defect. | ||
| - Participants (`confirmed_participant_names`) stay **meeting-scoped, never run-scoped**. This is deliberate and the code must say so. | ||
| - The e2e fixture `writeSpeakersSidecar` (`e2e/fixtures/user-config.ts`) stays **legacy-shaped**. Its specs staying green is the backward-compatibility proof; changing it destroys the proof. | ||
| - Verification baseline to compare against, measured on this branch before Task 1: **944 Python tests**, ruff **41**, renderer lint **37 warnings / 0 errors**, typecheck clean. `tests/test_bundle_mlx.py` fails only in the full `discover` run and is green in isolation - pre-existing, not caused by this work. |
Contributor
There was a problem hiding this comment.
P3: The verification gate can produce false failures because the plan hard-codes an outdated Python test baseline (944) that no longer matches current branch expectations. Consider updating this baseline to the current measured value (and aligned references below) so reviewers compare against the right target.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/superpowers/plans/2026-08-04-speaker-review-run-provenance.md, line 20:
<comment>The verification gate can produce false failures because the plan hard-codes an outdated Python test baseline (`944`) that no longer matches current branch expectations. Consider updating this baseline to the current measured value (and aligned references below) so reviewers compare against the right target.</comment>
<file context>
@@ -0,0 +1,223 @@
+- The staleness rule exists **once**, as a shared predicate in `src/speaker_suggestions.py`, imported by `src/config.py` and `simple_recorder.py` the same way `prototype_channel_matches` already is. A second copy of the rule is a defect.
+- Participants (`confirmed_participant_names`) stay **meeting-scoped, never run-scoped**. This is deliberate and the code must say so.
+- The e2e fixture `writeSpeakersSidecar` (`e2e/fixtures/user-config.ts`) stays **legacy-shaped**. Its specs staying green is the backward-compatibility proof; changing it destroys the proof.
+- Verification baseline to compare against, measured on this branch before Task 1: **944 Python tests**, ruff **41**, renderer lint **37 warnings / 0 errors**, typecheck clean. `tests/test_bundle_mlx.py` fails only in the full `discover` run and is green in isolation - pre-existing, not caused by this work.
+- Diarization is macOS-only, but every file touched here is cross-platform Python or JS. Nothing in this slice may branch on platform.
+
</file context>
| - `SpeakerReviewPanel.tsx`: the existing "Keep generic" button calls the mutation instead of `setDismissed`. | ||
| The `dismissed` local state and its `notDismissed` filtering are removed; the marker is read from the suggestions query (`suggest-speakers` echoes `review_state` per cluster), so it survives remounts and restarts by construction. | ||
| - **Behavior change, deliberate:** today the button hides the row for the session. | ||
| Under this design the row stays visible, quietly marked as kept generic, with the undo one click away - a persisted-but-hidden row would make the undo undiscoverable. |
Contributor
There was a problem hiding this comment.
P3: The spec changes 'Keep generic' from hiding the row to 'quietly marked', but never defines how a generic row is visually distinguished from an unreviewed one, nor the undo affordance. Without a specified indicator (e.g. an outline/check state analogous to isMarked), a persisted-but-invisible marker gives no feedback and the promised 'undo one click away' is undiscoverable, partially defeating the persistence goal. Please specify the rendered state for a generic-marked row (and its data-testid) in the build contract.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/superpowers/specs/2026-08-04-speaker-review-run-provenance-design.md, line 78:
<comment>The spec changes 'Keep generic' from hiding the row to 'quietly marked', but never defines how a generic row is visually distinguished from an unreviewed one, nor the undo affordance. Without a specified indicator (e.g. an outline/check state analogous to isMarked), a persisted-but-invisible marker gives no feedback and the promised 'undo one click away' is undiscoverable, partially defeating the persistence goal. Please specify the rendered state for a generic-marked row (and its data-testid) in the build contract.</comment>
<file context>
@@ -0,0 +1,204 @@
+- `SpeakerReviewPanel.tsx`: the existing "Keep generic" button calls the mutation instead of `setDismissed`.
+ The `dismissed` local state and its `notDismissed` filtering are removed; the marker is read from the suggestions query (`suggest-speakers` echoes `review_state` per cluster), so it survives remounts and restarts by construction.
+- **Behavior change, deliberate:** today the button hides the row for the session.
+ Under this design the row stays visible, quietly marked as kept generic, with the undo one click away - a persisted-but-hidden row would make the undo undiscoverable.
+
+## 4. Staleness on the read path
</file context>
Comment on lines
+1753
to
+1754
| and (end - begin) <= SAMPLE_MAX_SECONDS | ||
| and (end - begin) >= SAMPLE_MIN_USEFUL_SECONDS |
Contributor
There was a problem hiding this comment.
P3: This fits condition includes a duration cap that is already guaranteed earlier, so this branch is dead logic and makes the ranking predicate harder to read. Removing the redundant check keeps behavior identical.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/speaker_suggestions.py, line 1753:
<comment>This `fits` condition includes a duration cap that is already guaranteed earlier, so this branch is dead logic and makes the ranking predicate harder to read. Removing the redundant check keeps behavior identical.</comment>
<file context>
@@ -1723,12 +1737,36 @@ def extract_segment_samples(
+ # preference, however completely it is quoted.
+ "fits": (
+ len(text) <= max_chars
+ and (end - begin) <= SAMPLE_MAX_SECONDS
+ and (end - begin) >= SAMPLE_MIN_USEFUL_SECONDS
+ ),
</file context>
Suggested change
| and (end - begin) <= SAMPLE_MAX_SECONDS | |
| and (end - begin) >= SAMPLE_MIN_USEFUL_SECONDS | |
| and (end - begin) >= SAMPLE_MIN_USEFUL_SECONDS |
| # survives it whole, exactly as a quote of exactly max_chars | ||
| # does. And a clip too short to place a voice earns no | ||
| # preference, however completely it is quoted. | ||
| "fits": ( |
Contributor
There was a problem hiding this comment.
P3: The new internal fits boolean used only for ranking gets carried into the returned sample dicts, and since simple_recorder.py serializes cluster_samples straight into the suggest-speakers samples payload, the field now leaks over the API even though the documented return contract is just {start, end, text}. Current renderer consumers only read start/end/text so nothing breaks today, but it widens the wire schema with an implementation detail. Consider dropping fits from the entries before returning (for example, strip it from each chosen candidate or build the return dicts without it) so the endpoint keeps emitting exactly the documented fields.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/speaker_suggestions.py, line 1751:
<comment>The new internal `fits` boolean used only for ranking gets carried into the returned sample dicts, and since `simple_recorder.py` serializes `cluster_samples` straight into the suggest-speakers `samples` payload, the field now leaks over the API even though the documented return contract is just `{start, end, text}`. Current renderer consumers only read start/end/text so nothing breaks today, but it widens the wire schema with an implementation detail. Consider dropping `fits` from the entries before returning (for example, strip it from each chosen candidate or build the return dicts without it) so the endpoint keeps emitting exactly the documented fields.</comment>
<file context>
@@ -1723,12 +1737,36 @@ def extract_segment_samples(
+ # survives it whole, exactly as a quote of exactly max_chars
+ # does. And a clip too short to place a voice earns no
+ # preference, however completely it is quoted.
+ "fits": (
+ len(text) <= max_chars
+ and (end - begin) <= SAMPLE_MAX_SECONDS
</file context>
…something to observe The T1 spec "play button fetches and plays a real audio clip, toggling to stop" has been failing on CI and passing on a Mac. Measured in the real renderer, not guessed: the fixture was a 44-byte WAV header with zero sample data, so the media element reported duration 0 and fired `ended` ~300ms after play(). PlaySampleButton flips its label back on `ended`, so the assertion was racing a state that existed for a third of a second - won on a fast local machine, lost whenever the first poll landed later. Reproduced locally by waiting 1500ms before the assertion: with the old fixture that fails with exactly the CI error, with the new one it passes. The fixture is now built in code with an explicit duration constant, and a node:test pins it: shrinking it again would not fail loudly, it would just make one spec flaky on someone else's machine weeks later. Valentin's assertion is untouched - it was right, and the fixture could not support it. A spec about playing a sample needs a sample that plays.
Optic00
merged commit Aug 5, 2026
9ac6cc4
into
stenolabs:feat/speaker-diarization
22 of 23 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three things, all in the speaker review: one person owning several clusters, the excerpt selection, and the written groundwork for the next slice.
One person, several clusters
The diarizer splits a voice routinely, and the wider it splits the likelier that gets. Assigning several clusters to the same person was already possible, but neither obvious nor fully correct.
Rows are ordered by speaking time, because reviewing is voluntary and abandonable, so the order decides how much of the transcript the first decisions cover. The Change picker lists people already assigned in this meeting first, marked "here" - the alternative a hurried reviewer reaches for is "New person", which records one voice as two people and makes them a hard negative against themselves.
suggest-speakersgainedconfirmed_person_idfor this, since display names are not identity.Excerpt selection
A quote is cut at 140 characters and its clip at 20 seconds, and nothing related the two. Measured on a real 9-minute call: a 40.5 s / 742-character turn showed 19 % of its text beside 20 s of audio, which reads as the wrong clip although both start at the same instant. Ranking by raw duration made it likelier, not less likely. Turns that survive both caps now rank first, longest among them.
Two review corrections on that: a clip has to be long enough to place a voice before it earns the preference (a 0.1 s quote was outranking a 19 s turn that missed the character cap by one character), and both caps are compared inclusively.
Groundwork, no behaviour
A design spec and its implementation plan for the next slice: run provenance in the sidecar and a persisted review state, so an interrupted review resumes and a re-diarization stops silently attributing new clusters to people confirmed against an older run. Both documents only; nothing in this PR implements them.
Verification
946 Python tests, 157 vitest, 17/17
speaker-review.t1including new specs, typecheck clean, lint 37/0 and ruff 41 as the basis.test_bundle_mlxfails only in the full discover run and is green in isolation, pre-existing.Reviewed cross-family by Codex across several rounds; every finding it raised is either fixed here or answered in the commit that rejects it.