feat(speakers): human-in-the-loop cross-recording speaker identification - #472
Merged
Optic00 merged 22 commits intoAug 4, 2026
Merged
Conversation
Adds a macOS-only Swift/CoreML sidecar (diarize-sidecar, wrapping FluidAudio's Sortformer) that diarizes the mic and system-audio channels independently, so multiple speakers sharing one side of a call (in-person conversations on mic, or multiple remote participants on system audio) get labelled "You" / "Speaker 2" / "Speaker 3" instead of being lumped together. The dominant-by-duration cluster on each channel keeps the legacy "You"/ "Others" label; other clusters are numbered by first chronological appearance across both channels. Any failure (missing binary, timeout, bad output, single-cluster result) falls back to today's exact channel-only behaviour, so this can never fail a meeting. Two fixes from testing against a real in-person recording: - is_diarised now reflects whether the output actually has more than one speaker label, not whether both channels had content — the old check discarded the whole labelled transcript whenever one channel (e.g. system audio with nothing playing) was empty. - Long Parakeet sentences that span multiple real diarizer turns (no strong punctuation break in a long run of speech) are now split at the word level and reassigned per-word, instead of forcing the entire block onto whichever diarizer segment the sentence's midpoint happened to land in.
Mono recordings (many imports — phone voice memos, single-track exports) had no channel split to fall back on, so they got zero speaker labelling, even when the track genuinely has multiple speakers. Runs steno-diarize directly against the whole file instead, reusing the same per-channel tagging/placeholder-resolution helpers the stereo path uses, treating the single track as the "You" channel — consistent with the pre-diarization convention of attributing an unlabelled mono recording to the user. A single real speaker still produces a plain, unlabelled transcript exactly as before.
Sortformer was configured with .default (fastV2_1: 0.48s of audio per CoreML invocation, ~1.04s latency) -- tuned for live/streaming responsiveness this app has no use for, since diarization only ever runs on a fully-recorded, already-finished channel. Switched to .highContextV2 (27.2s per invocation) for recordings long enough to benefit: ~56x fewer invocations for the same audio (~400 vs ~22,500 for a 3-hour file). Measured on a real ~21-minute recording: 153s -> 23s. V2, not V2.1: FluidAudio's own docs note V2.1 "may degrade when many speakers are talking simultaneously" -- a real risk given this app's crosstalk/echo findings from earlier diarization work. Real regression found and fixed during validation: highContextV2's chunk loader requires a full ~30.4s window before it emits anything at all -- a 12.25s test clip came back with zero segments. Added sortformerHighContextMinDuration (90s, real margin above the hard minimum) so recordings shorter than that keep using .default. Accuracy validated against the same real file, not just "it still runs": 98.2% agreement on the dominant speaker's per-second attribution, a near-zero spurious "4th speaker" (0.6s total) cleanly disappeared, and total detected speech stayed within ~1%. The GPU-vs-ANE compute units env var wired into the manual/backfill CLI paths in a prior commit (measured separately: 23.0s ANE vs 18.0s GPU on the same file) stays opt-in-only -- the normal per-meeting pipeline keeps the power/thermal-efficient ANE default.
Ports the diarization-relevant slice of 04c2be1 (which mixed diarization,
progress, and identity concerns in one commit) onto the standalone
diarization branch, minus everything identity-specific:
- CHANNEL_DOMINANCE_THRESHOLD: a channel with one overwhelmingly dominant
speaker is treated as single-speaker rather than spawning a phantom
second speaker from a misdiarization blip.
- CHANNEL_DETECT_TIMEOUT_S: the channel-count probe's fixed 15s timeout
silently dropped long WebM recordings to mono; scaled to 60s.
- _run_steno_diarize rewritten to Popen + two reader threads (avoids the
classic pipe-deadlock on large stdout payloads) with a real JSON
decoder scan for the last valid segment array, tolerating FluidAudio/
CoreML warning text before, between, or after the payload.
- _heartbeat_while_waiting + PROGRESS:diarize:{label}:start/:done so a
long diarization pass doesn't look hung to Electron's inactivity
watchdog or sit on a static spinner.
- Pre-processing audio start log line, so loudnorm's two-pass analysis
doesn't look like a hang on a long recording either.
The sidecar's Output contract stays a bare segment array (this branch
never extracts voiceprint embeddings), so the parser and its tests are
array-only rather than the array-or-object form the full identity branch
needs.
Renders the PROGRESS:diarize:{label}:start/:done markers (added to the
backend in the previous commit) as a real UI stage instead of a static
"Analyzing transcript" spinner sitting through a diarization pass that
can run for minutes on a long recording.
- New 'diarizing' stage with an elapsed-time ticker (this branch's
sidecar has no per-chunk checkpoint to report a percentage from, so
a plain "(Ns)" counter is the only way to show the stage is alive).
- Fixes a real bug the new diarize progress lines would otherwise hit:
the processingProgress handler used to key off ANY PROGRESS: line
unconditionally to flip transcribing -> summarizing; without a
prefix check, a PROGRESS:diarize:* line would have prematurely
jumped the stage to "summarizing" while diarization was still
running. Now branches on the PROGRESS:summarize:/PROGRESS:diarize:
prefix explicitly.
- Clears chunkProgress on every stage transition (summarize-complete,
processing-complete, retry) so a stale diarizing/summarizing
sub-label can't leak into finalizing/error/a retried run.
- main.js: PROGRESS:diarize:* markers now persisted to the on-disk
pipeline log (already true for HEARTBEAT); the live renderer forward
needed no change since the existing PROGRESS: forwarder is generic.
- New processing-stages.t1.spec.ts (mock IPC, real webContents.send
events) -- Processing.tsx had zero test coverage before this.
canRetry (Processing.tsx) requires both retryAudioFile (from processing-complete's audioFile field) and activeSession (from recording.sessionName) to be truthy. The spec reached /meetings/processing via a bare URL hash with no active mock recording, so activeSession stayed null and the retry-button assertion hung waiting on a permanently-disabled button. Start a mock recording first, matching how the screen is actually reached in real usage, and include audioFile in the failure payload.
Adds cross-recording speaker identification as a human-confirmed suggestion flow rather than silent auto-matching: validating raw embedding-similarity auto-matching against real ground truth (AMI Meeting Corpus) found that people sharing a room/mic score artificially similar regardless of true identity, which no threshold/margin tuning fixed safely. - PersonProfile/SpeakerPrototype CRUD (src/config.py) with hard-negative and context-aware (in-person vs remote) evidence. - Suggestion service (src/speaker_suggestions.py): threshold + margin + stability + hard-negative gating, same-meeting fragment merging, transcript relabeling, sample-audio/text extraction for identification. - CLI surface for backfill/validation: list/create/rename/delete person profiles, suggest-speakers, confirm-speaker, backfill-speaker-embeddings, speaker-suggestion-report, get-speaker-sample-audio. - Approval UI (SpeakerReviewPanel, useSpeakerSuggestions) wired into MeetingDetail: approve/change/new-person/keep-generic, duplicate-name prevention, delete with row-visibility preservation. - New `speakers` IPC group (main.js/preload.js/ipc.ts), with parsePythonFailureJson fixing a real bug where a graceful CLI failure's clean JSON was discarded in favor of a raw stderr-wrapped error. - e2e coverage: speaker-naming.t2 (real backend) and speaker-review.t1 (mock IPC, covers the panel's four-action interaction surface).
Re-adds the WeSpeaker embedding-extraction path (buildOverlapExcludedMasks,
resampleMask, aggregateCentroids, accumulateChunkEmbeddings,
extractSortformerEmbeddings) and the {"segments","speakers"} Output
contract stripped from the diarization-only branch, where nothing
consumed it. The identity half built on this branch needs real per-speaker
embeddings to match against stored voiceprints/person profiles.
Restores a change that predates the RFC stenolabs#327 main.js/backend-cli.js split and was lost when d487541 was cherry-picked onto post-split main.js. Python defaults to block-buffering stdout/stderr when they're piped (not a TTY), so a logger.info() call can sit unflushed for minutes on a long operation -- starving Electron's inactivity watchdog of the HEARTBEAT:/ PROGRESS: lines it needs to tell real silence from buffered-but-alive. Harmless for non-Python binaries (ollama/ffmpeg), which never read this env var.
Restores the 'diarizing' stage's embedding-percentage sub-progress
(PROGRESS:diarize:{label}:embedding:{i}/{n}, now real again since the
sidecar re-extracts embeddings) and PROGRESS:transcribe:{done}/{total}
handling, both dropped from the diarization-only branch where neither
the sidecar nor the ASR backend emitted them.
Widens the diarization-only branch's PROGRESS:diarize:-only disk-log allowlist to cover any PROGRESS: line, with the same 10s throttle HEARTBEAT already gets for the high-frequency ones (transcribe sample counts, diarize embedding chunks) -- both now real again since the sidecar re-extracts embeddings and transcribe progress is wired back in. Stage-transition markers (start/done, summarize step/reducing) stay unthrottled since they're rare.
Restores src/transcriber.py and its test file to the original identity-branch content: VOICEPRINT_DISTANCE_THRESHOLD/CONFIDENCE_MARGIN, _voiceprint_distance, _apply_voiceprint_matches, the speaker_suggestions import, allow_self_match/clusters_out on _tag_channel_segments, and speaker_clusters assembly in transcribe_diarised/_transcribe_diarised_mono. Zero upstream drift on this file (confirmed: only cloud ASR added then reverted), so restoring wholesale rather than hand-splicing onto the diarization-only branch's rewrite guarantees byte-for-byte fidelity with what's actually shipping, with no risk of a manual-port transcription error.
…ceprint Rebased onto current main (post mic-selection merge, Settings redesign, pill dock). Speaker-suggestion refinements, transcriber tweaks, and new CLI test coverage (backfill-participants, full-reprocess, meeting-pipeline transcribe-audio).
…ative hard-negatives
- Store channel on SpeakerPrototype/hard-negative entries; a
(meeting_id, diarization_speaker_id) pair alone is ambiguous since mic
and system channels number diarizer clusters independently, and this
collision was silently poisoning hard-negative evidence across channels
- confirm-speaker now reassigns a cluster on re-confirm ("Change"):
removes the superseded person's prototype and hard negatives instead of
both people keeping conflicting evidence for the same voice
- Hard-negative suppression is relative to positive evidence, not an
absolute cutoff -- a strong match no longer gets discarded by an
ordinary-distance negative from someone else's confirmation
- suggest_speakers_for_meeting enforces person exclusivity across ALL
channels of a meeting (was per-channel), assigning clusters in
best-distance order instead of sorted-id order
- Add repair-speaker-profiles CLI (dry-run by default) to clean up
existing collision-created negatives/duplicates and backfill channel
on legacy prototypes from their meeting's sidecar
- Add a self-match diagnostics section to speaker-suggestion-report
Bridges the two separate speaker-identity systems: someone confirmed as a
named PersonProfile (via confirm-speaker) had no way to power the self
("You") voiceprint from that evidence, so self-matching stayed unenrolled
even when a person's own confirmed prototypes existed.
New enroll-self-from-person CLI feeds a person's positive prototypes
(never hard_negatives) through the existing save_voiceprint running-
centroid + recent-samples machinery, oldest-first. Prefers mic-channel
prototypes (self-match only ever runs on the mic channel), falling back
to all prototypes if none are on mic.
Same gap as the diarization-only branch: canRetry (Processing.tsx) requires both retryAudioFile (from processing-complete's audioFile field) and activeSession (from recording.sessionName) to be truthy. Start a mock recording first, matching how the screen is actually reached in real usage, and include audioFile in the failure payload.
…hing Two real gaps found while verifying claims for an upstream reviewer (Ben/Optic00 on stenolabs#359), not just answering questions: 1. delete_person_profile only removed the deleted person's own profile entry. confirm-speaker writes MUTUAL hard negatives whenever two different people are confirmed in the same meeting+channel -- the deleted person's own voice sample kept living on inside everyone else's hard_negatives list forever. Now walks the deleted person's own prototypes (each already tagged with the meeting/channel/sid they were confirmed under) and reuses the existing remove_speaker_evidence to strip the matching hard-negative entry from every other profile -- the exact reverse of how confirm-speaker created them. 2. No setting existed to disable cross-recording speaker identification independently of diarization. Adds identity_matching_enabled (default on), gated at transcriber.py's _tag_channel_segments call sites: when off, allow_self_match is always False and clusters_out is always None, so per-meeting speaker embeddings never reach speaker_clusters and are never persisted to a {stem}_speakers.json sidecar. Diarization's own "Speaker N" splitting is untouched, since it only depends on diarizer segments, not embeddings. The manual backfill-speaker-embeddings CLI (the one path that bypasses the normal per-meeting flow) checks the same setting and refuses when disabled, so the toggle can't be silently bypassed. Full Config/CLI/IPC/Settings-UI wiring for the new setting follows the existing auto_install_when_idle pattern exactly.
…invariant CI found this failing 3/3 times on the macOS T2 pipeline lane, not a flake. isSayAvailable() only probed `say -v ?` (listing voices), which exits 0 even on a runner where `say` can't actually synthesize speech -- observed producing ~170 bytes of near-silent PCM (~5ms) instead of real audio, presumably missing voice assets. That let micBoundarySeconds come out at ~0.5s instead of the several seconds a real sentence takes, which fed straight into a tailSeconds formula with an unconditional 1.5s floor (Math.max(1.5, ...)) that made the very next assertion (tailSeconds < micBoundarySeconds) mathematically impossible to satisfy below a 1.5s boundary. Passed locally only because real speech synthesis on a real Mac comfortably exceeds that. Fixes both: isSayAvailable() now synthesizes a short real phrase and measures what it actually wrote, skipping loudly (existing test.skip path) when it's implausibly short, rather than trusting a voice-list probe that doesn't exercise synthesis at all. tailSeconds is now a bounded fraction of micBoundarySeconds with no floor above it, so the invariant holds for any micBoundarySeconds > 0 -- not just relying on the environment guard to keep it out of the impossible range.
This PR makes two existing statements false. docs/faq.mdx's "Can Steno record in-person meetings?" said in-person recordings have no speaker labels at all -- after this PR, the mono/mic-only path is exactly where acoustic diarization gets used, so that's now the headline case that gains labels, not the one that lacks them. docs/features/recording.mdx's "Speaker labels" section had the same gap from the other direction: it described labeling as something that only happens when system audio is on (the [You]/[Others] channel split), omitting the new within-channel acoustic split entirely. Both now describe the real constraint precisely: up to four distinct voices per channel, not four total, since diarization runs independently on each channel. Framed that way because it matters for the common case -- a two-person call is one person per channel, comfortably under the per-channel limit either side, so most users never approach it, but a flat "four speakers" would incorrectly suggest otherwise.
parsePythonFailureJson() in main.js recovers a graceful {"success":
false, "error": ...} a CLI command printed to stdout before exiting
non-zero, but runPythonScript's rejection never attached .stdout to
the Error, so the recovery always failed and every graceful CLI
failure (e.g. "already exists" on a duplicate person name) surfaced
as a generic "Python script failed with code 1: <stderr>" instead.
… speaker-identity # Conflicts: # app/e2e-mock-ipc.js # app/renderer/src/routes/Processing.tsx
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
31 issues found across 52 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="docs/features/recording.mdx">
<violation number="1" location="docs/features/recording.mdx:31">
P3: This change makes the Recording page describe per-speaker diarization, but the dedicated Speaker labels/Transcription docs still describe only `[You]`/`[Others]` and even say individual identification is not available. Readers can get conflicting product expectations depending on which page they land on, so these docs likely need to be updated together.</violation>
</file>
<file name="app/renderer/src/routes/settings/AiTab.tsx">
<violation number="1" location="app/renderer/src/routes/settings/AiTab.tsx:155">
P2: The settings copy currently promises that disabling Speaker identification stops embedding extraction, but the transcription path still runs diarization/embedding extraction and only skips storing/using those embeddings for identity matching. Updating this text avoids a privacy/behavior mismatch for users evaluating this toggle.</violation>
</file>
<file name="app/renderer/src/hooks/useSettings.ts">
<violation number="1" location="app/renderer/src/hooks/useSettings.ts:407">
P3: This key is duplicated as an inline array in both read and write hooks, so a later one-sided edit can silently break invalidation and leave stale UI state. Adding a `settingsKeys.identityMatchingEnabled()` factory (and reusing it in both hooks) keeps the key coupled like the other settings.</violation>
<violation number="2" location="app/renderer/src/hooks/useSettings.ts:414">
P2: Identity-matching toggle updates will be slower than other boolean settings because this setter invalidates and refetches after write instead of using the shared optimistic toggle path. Using `useToggleSetting` here avoids the extra read IPC round-trip and keeps the switch responsive with rollback on failure.</violation>
</file>
<file name="src/voiceprint.py">
<violation number="1" location="src/voiceprint.py:25">
P2: Speaker matching can produce incorrect distances when vectors have different lengths because zip truncates to the shorter vector. Adding an explicit length check here avoids silent false matches/rejections and surfaces bad embedding data early.</violation>
</file>
<file name="diarize-sidecar/Package.swift">
<violation number="1" location="diarize-sidecar/Package.swift:6">
P2: This package currently advertises support for macOS 14.0+, which can permit builds/runs on versions below the documented 14.4 floor. Aligning the deployment target with 14.4+ would avoid unsupported runtime paths on older macOS 14.x systems.</violation>
</file>
<file name="scripts/build-diarize-sidecar.sh">
<violation number="1" location="scripts/build-diarize-sidecar.sh:29">
P2: The build script can report success even when `codesign` fails, which hides signing problems that affect local execution/debugging of `bin/steno-diarize`. Consider skipping signing only when `codesign` is unavailable, but letting real signing failures fail the script.</violation>
</file>
<file name="app/renderer/src/hooks/useSpeakerSuggestions.ts">
<violation number="1" location="app/renderer/src/hooks/useSpeakerSuggestions.ts:87">
P3: This adds two exported hooks that are not referenced anywhere in the renderer, so they increase API surface and maintenance cost without affecting behavior. It would be cleaner to remove them until the UI actually needs them, or wire them into the intended flow now.</violation>
</file>
<file name="app/renderer/src/lib/transcriptSegments.ts">
<violation number="1" location="app/renderer/src/lib/transcriptSegments.ts:23">
P2: Diarised parsing now splits on bracketed text inside a turn, so normal content like `[12:30]` can be misread as a new speaker and corrupt speaker grouping in the transcript UI. Constraining marker detection to line starts/new-turn boundaries keeps support for custom labels (`[Speaker N]`, confirmed names) without breaking bracketed body text.</violation>
</file>
<file name="app/renderer/src/components/SpeakerReviewPanel.tsx">
<violation number="1" location="app/renderer/src/components/SpeakerReviewPanel.tsx:106">
P3: Stopping a sample before it ends leaks its blob URL, so repeated previewing can grow renderer memory. Releasing the current `audio.src` in `stop()` avoids that leak path.</violation>
<violation number="2" location="app/renderer/src/components/SpeakerReviewPanel.tsx:126">
P2: A failed `audio.play()` leaves the control stuck in Stop state even though nothing is playing, and its URL is not cleaned up. Awaiting `play()` with a catch to reset state and revoke the URL makes this failure path safe.</violation>
</file>
<file name="e2e/fixtures/say-stereo-wav.ts">
<violation number="1" location="e2e/fixtures/say-stereo-wav.ts:47">
P3: Repeated e2e runs leak temporary directories and synthesized audio files under the system temp directory. Cleaning the temporary directory in a `finally` block (or using the repository's temporary-file cleanup helper) would prevent unbounded test-environment growth.</violation>
</file>
<file name="app/renderer/src/lib/ipc.ts">
<violation number="1" location="app/renderer/src/lib/ipc.ts:485">
P2: Confirm calls can compile with an invalid argument shape and only fail at runtime, because `ConfirmSpeakerParams` allows both or neither of `personId`/`newPersonName`. Encoding the existing backend “exactly one” rule in the type would prevent these avoidable runtime errors at call sites.</violation>
</file>
<file name="app/renderer/src/routes/Processing.tsx">
<violation number="1" location="app/renderer/src/routes/Processing.tsx:289">
P2: Back-to-back recordings can inherit the previous session’s diarization ticker because this interval can survive a generation reset when the component stays mounted. Clearing it on session/generation reset (or in the listener-effect cleanup) would prevent stale progress text and watchdog-disarming activity from bleeding into the next run.</violation>
<violation number="2" location="app/renderer/src/routes/Processing.tsx:613">
P3: The stage label can briefly show stale text like diarization/transcription after summarization has already started. This happens because `chunkProgress` is reused across stages and the new render condition displays it for all active stages instead of stage-matched progress only.</violation>
</file>
<file name="tests/test_repair_speaker_profiles_cli.py">
<violation number="1" location="tests/test_repair_speaker_profiles_cli.py:73">
P3: The `collision` local is unused in this test; replacing it with `_` would remove the dead binding and clarify that the test identifies the survivor rather than the removed entry.</violation>
</file>
<file name="src/transcriber.py">
<violation number="1" location="src/transcriber.py:955">
P2: Self-voice matching can mislabel who is shown as [You] when two clusters are similarly close, because this path picks the single minimum distance and never checks runner-up separation. Applying `VOICEPRINT_CONFIDENCE_MARGIN` (best vs second-best) here would avoid auto-anchoring on ambiguous matches.</violation>
</file>
<file name="e2e/specs/speaker-naming.t2.spec.ts">
<violation number="1" location="e2e/specs/speaker-naming.t2.spec.ts:13">
P3: Module-level doc says "no real audio" but the second test (identification aids) generates a real WAV via `makeWav` and extracts sample audio from it via `getSampleAudio`. The doc over-promises the scope — inaccurate for the file as a whole.</violation>
</file>
<file name="src/speaker_suggestions.py">
<violation number="1" location="src/speaker_suggestions.py:565">
P3: Unreadable sidecars with invalid text encoding can still crash CLI flows instead of degrading gracefully, because `UnicodeDecodeError` from `read_text()` is not handled in this fallback path. Including that exception here would keep malformed-byte files on the same warning-and-None behavior as JSON parse errors.</violation>
</file>
<file name="app/settings-ipc.test.js">
<violation number="1" location="app/settings-ipc.test.js:54">
P3: Comment says 'Covers all eight' but `SPREAD_GETTERS` has 9 items after adding `get-identity-matching-enabled`. The pre-existing miscount (was 'all seven' for 8 items before this change) is now off by one again. Update to 'all nine'.</violation>
</file>
<file name="diarize-sidecar/Sources/main.swift">
<violation number="1" location="diarize-sidecar/Sources/main.swift:103">
P2: CLI/dev runs can fail with "ffmpeg not found" even when ffmpeg is installed, because discovery is limited to hardcoded paths. Adding PATH lookup here (matching backend resolver behavior) keeps sidecar startup reliable across non-Homebrew environments.</violation>
</file>
<file name="tests/test_speaker_suggestions.py">
<violation number="1" location="tests/test_speaker_suggestions.py:502">
P3: This test does not actually cover transitive merging because A and C are also within `SAME_MEETING_MERGE_DISTANCE_THRESHOLD`. Using a C embedding beyond the threshold from A but within it from B would make the test fail for implementations that lack connected-component behavior.</violation>
<violation number="2" location="tests/test_speaker_suggestions.py:529">
P3: The weighted-merge test accepts an unweighted primary embedding, so it does not verify weighted aggregation. An assertion against the expected duration-weighted normalized vector (or at least that the result differs from `a` toward `b`) would cover the intended behavior.</violation>
</file>
<file name="app/e2e-mock-ipc.js">
<violation number="1" location="app/e2e-mock-ipc.js:746">
P2: This mock path can hide renderer wiring regressions because `suggest-speakers` succeeds even for an invalid meeting stem. Adding a stem/no-sidecar failure path would make T1 catch the same class of errors as the real IPC flow.</violation>
<violation number="2" location="app/e2e-mock-ipc.js:793">
P2: Different names can collapse to the same `person_id` in the mock, which can make rename/delete hit the wrong profile and create impossible T1 states. Using a guaranteed-unique ID per new profile would keep mock behavior aligned with production.</violation>
</file>
<file name="app/main.js">
<violation number="1" location="app/main.js:2455">
P2: User-facing suggest-speaker failures can degrade to a generic spawn error instead of the backend’s specific JSON error because this catch path returns `error.message` directly. Reusing `parsePythonFailureJson(error)` here would preserve actionable messages like missing sidecar/channel/cluster.</violation>
</file>
<file name="src/config.py">
<violation number="1" location="src/config.py:338">
P1: Concurrent saves can still lose speaker identity data. Because normalization mutates `_config` after snapshot capture, later unrelated saves can treat `voiceprints`/`person_profiles` as local edits and overwrite newer on-disk values; syncing those normalized keys into `_snapshot` avoids that lost-update path.</violation>
<violation number="2" location="src/config.py:1047">
P2: Profile/prototype writes can report success even when persistence fails. These methods ignore `_save()`’s boolean result and return success objects regardless, so callers cannot surface storage errors and users can lose confirmed identity changes after process exit.</violation>
</file>
<file name="simple_recorder.py">
<violation number="1" location="simple_recorder.py:3336">
P1: `full-reprocess` currently fails immediately because `process_streaming.callback` is invoked with one positional argument missing. Passing `append_to` explicitly (e.g. `None`) keeps the command runnable.</violation>
<violation number="2" location="simple_recorder.py:3346">
P1: `full-reprocess --audio-file` can silently reprocess into a different meeting stem when the override filename differs, leaving the intended meeting unreplaced. Consider rejecting mismatched stems (or creating a temporary same-stem copy) before invoking `process_streaming`.</violation>
</file>
<file name="app/renderer/src/routes/MeetingDetail.tsx">
<violation number="1" location="app/renderer/src/routes/MeetingDetail.tsx:1293">
P2: Speaker confirmation can leave transcript labels stale on symlinked storage paths because this passes `info.summary_file` identity into `SpeakerReviewPanel`, but the open detail query is keyed by the route identity. Passing `routeSummaryFile` here keeps confirm invalidation aligned with the currently displayed meeting cache.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
Comment on lines
+338
to
+339
| self._normalize_voiceprints() | ||
| self._normalize_person_profiles() |
Contributor
There was a problem hiding this comment.
P1: Concurrent saves can still lose speaker identity data. Because normalization mutates _config after snapshot capture, later unrelated saves can treat voiceprints/person_profiles as local edits and overwrite newer on-disk values; syncing those normalized keys into _snapshot avoids that lost-update path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/config.py, line 338:
<comment>Concurrent saves can still lose speaker identity data. Because normalization mutates `_config` after snapshot capture, later unrelated saves can treat `voiceprints`/`person_profiles` as local edits and overwrite newer on-disk values; syncing those normalized keys into `_snapshot` avoids that lost-update path.</comment>
<file context>
@@ -335,6 +335,8 @@ def __init__(self, config_path: Optional[Path] = None):
self._migrate_privacy_notice_seen()
self._normalize_templates()
self._seed_sample_template()
+ self._normalize_voiceprints()
+ self._normalize_person_profiles()
</file context>
Suggested change
| self._normalize_voiceprints() | |
| self._normalize_person_profiles() | |
| self._normalize_voiceprints() | |
| self._normalize_person_profiles() | |
| if "voiceprints" in self._config: | |
| self._snapshot["voiceprints"] = copy.deepcopy(self._config["voiceprints"]) | |
| if "person_profiles" in self._config: | |
| self._snapshot["person_profiles"] = copy.deepcopy(self._config["person_profiles"]) | |
|
|
||
| # process-streaming always writes .md (never .json), regardless of the | ||
| # pre-reprocess format -- the new summary lives here from now on. | ||
| new_summary_path = output_dir / f"{meeting_stem}_summary.md" |
Contributor
There was a problem hiding this comment.
P1: full-reprocess --audio-file can silently reprocess into a different meeting stem when the override filename differs, leaving the intended meeting unreplaced. Consider rejecting mismatched stems (or creating a temporary same-stem copy) before invoking process_streaming.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At simple_recorder.py, line 3346:
<comment>`full-reprocess --audio-file` can silently reprocess into a different meeting stem when the override filename differs, leaving the intended meeting unreplaced. Consider rejecting mismatched stems (or creating a temporary same-stem copy) before invoking `process_streaming`.</comment>
<file context>
@@ -3135,6 +3208,181 @@ def _transcribe_heartbeat(done, total):
+
+ # process-streaming always writes .md (never .json), regardless of the
+ # pre-reprocess format -- the new summary lives here from now on.
+ new_summary_path = output_dir / f"{meeting_stem}_summary.md"
+
+ if folder_ids:
</file context>
| Path(notes_file).write_text(notes_text, encoding='utf-8') | ||
|
|
||
| try: | ||
| process_streaming.callback(str(audio_path), session_name, notes_file, None) |
Contributor
There was a problem hiding this comment.
P1: full-reprocess currently fails immediately because process_streaming.callback is invoked with one positional argument missing. Passing append_to explicitly (e.g. None) keeps the command runnable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At simple_recorder.py, line 3336:
<comment>`full-reprocess` currently fails immediately because `process_streaming.callback` is invoked with one positional argument missing. Passing `append_to` explicitly (e.g. `None`) keeps the command runnable.</comment>
<file context>
@@ -3135,6 +3208,181 @@ def _transcribe_heartbeat(done, total):
+ Path(notes_file).write_text(notes_text, encoding='utf-8')
+
+ try:
+ process_streaming.callback(str(audio_path), session_name, notes_file, None)
+ finally:
+ if notes_file:
</file context>
Suggested change
| process_streaming.callback(str(audio_path), session_name, notes_file, None) | |
| process_streaming.callback(str(audio_path), session_name, notes_file, None, None) |
| </section> | ||
| )} | ||
|
|
||
| <SpeakerReviewPanel summaryFile={summaryFile} isDiarised={Boolean(meeting.is_diarised)} /> |
Contributor
There was a problem hiding this comment.
P2: Speaker confirmation can leave transcript labels stale on symlinked storage paths because this passes info.summary_file identity into SpeakerReviewPanel, but the open detail query is keyed by the route identity. Passing routeSummaryFile here keeps confirm invalidation aligned with the currently displayed meeting cache.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/renderer/src/routes/MeetingDetail.tsx, line 1293:
<comment>Speaker confirmation can leave transcript labels stale on symlinked storage paths because this passes `info.summary_file` identity into `SpeakerReviewPanel`, but the open detail query is keyed by the route identity. Passing `routeSummaryFile` here keeps confirm invalidation aligned with the currently displayed meeting cache.</comment>
<file context>
@@ -1283,6 +1289,8 @@ function DetailContent({
</section>
)}
+
+ <SpeakerReviewPanel summaryFile={summaryFile} isDiarised={Boolean(meeting.is_diarised)} />
</div>
)}
</file context>
Suggested change
| <SpeakerReviewPanel summaryFile={summaryFile} isDiarised={Boolean(meeting.is_diarised)} /> | |
| <SpeakerReviewPanel summaryFile={routeSummaryFile} isDiarised={Boolean(meeting.is_diarised)} /> |
| return None | ||
| try: | ||
| return json.loads(path.read_text()) | ||
| except (json.JSONDecodeError, OSError) as e: |
Contributor
There was a problem hiding this comment.
P3: Unreadable sidecars with invalid text encoding can still crash CLI flows instead of degrading gracefully, because UnicodeDecodeError from read_text() is not handled in this fallback path. Including that exception here would keep malformed-byte files on the same warning-and-None behavior as JSON parse errors.
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 565:
<comment>Unreadable sidecars with invalid text encoding can still crash CLI flows instead of degrading gracefully, because `UnicodeDecodeError` from `read_text()` is not handled in this fallback path. Including that exception here would keep malformed-byte files on the same warning-and-None behavior as JSON parse errors.</comment>
<file context>
@@ -0,0 +1,1028 @@
+ return None
+ try:
+ return json.loads(path.read_text())
+ except (json.JSONDecodeError, OSError) as e:
+ logger.warning("Could not read speakers sidecar %s: %s", path, e)
+ return None
</file context>
Suggested change
| except (json.JSONDecodeError, OSError) as e: | |
| except (json.JSONDecodeError, UnicodeDecodeError, OSError) as e: |
|
|
||
| // Every spread-getter reads its own subcommand SILENTLY and returns | ||
| // { success:true, ...jsonData }. Covers all seven (privacy-notice-seen differs | ||
| // { success:true, ...jsonData }. Covers all eight (privacy-notice-seen differs |
Contributor
There was a problem hiding this comment.
P3: Comment says 'Covers all eight' but SPREAD_GETTERS has 9 items after adding get-identity-matching-enabled. The pre-existing miscount (was 'all seven' for 8 items before this change) is now off by one again. Update to 'all nine'.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/settings-ipc.test.js, line 54:
<comment>Comment says 'Covers all eight' but `SPREAD_GETTERS` has 9 items after adding `get-identity-matching-enabled`. The pre-existing miscount (was 'all seven' for 8 items before this change) is now off by one again. Update to 'all nine'.</comment>
<file context>
@@ -44,16 +45,17 @@ const CHANNELS = [
// Every spread-getter reads its own subcommand SILENTLY and returns
-// { success:true, ...jsonData }. Covers all seven (privacy-notice-seen differs
+// { success:true, ...jsonData }. Covers all eight (privacy-notice-seen differs
// and has its own test below).
const SPREAD_GETTERS = [
</file context>
Suggested change
| // { success:true, ...jsonData }. Covers all eight (privacy-notice-seen differs | |
| // { success:true, ...jsonData }. Covers all nine (privacy-notice-seen differs |
| merged, _ = merge_same_channel_fragments(clusters) | ||
| merged_embedding = merged["SPEAKER_0"][0] | ||
| # A dominant-weighted merge should land closer to A than to B. | ||
| self.assertLess(cosine_distance(merged_embedding, a), cosine_distance(merged_embedding, b)) |
Contributor
There was a problem hiding this comment.
P3: The weighted-merge test accepts an unweighted primary embedding, so it does not verify weighted aggregation. An assertion against the expected duration-weighted normalized vector (or at least that the result differs from a toward b) would cover the intended behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_speaker_suggestions.py, line 529:
<comment>The weighted-merge test accepts an unweighted primary embedding, so it does not verify weighted aggregation. An assertion against the expected duration-weighted normalized vector (or at least that the result differs from `a` toward `b`) would cover the intended behavior.</comment>
<file context>
@@ -0,0 +1,1147 @@
+ merged, _ = merge_same_channel_fragments(clusters)
+ merged_embedding = merged["SPEAKER_0"][0]
+ # A dominant-weighted merge should land closer to A than to B.
+ self.assertLess(cosine_distance(merged_embedding, a), cosine_distance(merged_embedding, b))
+
+ def test_does_not_merge_deliberately_different_voices(self):
</file context>
| # must still merge into one group via the A-B-C chain. | ||
| a = [1.0, 0.0] | ||
| b = [0.995, 0.0999] # dist(a,b) ~= 0.005 | ||
| c = [0.98, 0.19] # dist(b,c) ~= 0.045; dist(a,c) ~= 0.051 -- both under 0.10 anyway, |
Contributor
There was a problem hiding this comment.
P3: This test does not actually cover transitive merging because A and C are also within SAME_MEETING_MERGE_DISTANCE_THRESHOLD. Using a C embedding beyond the threshold from A but within it from B would make the test fail for implementations that lack connected-component behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_speaker_suggestions.py, line 502:
<comment>This test does not actually cover transitive merging because A and C are also within `SAME_MEETING_MERGE_DISTANCE_THRESHOLD`. Using a C embedding beyond the threshold from A but within it from B would make the test fail for implementations that lack connected-component behavior.</comment>
<file context>
@@ -0,0 +1,1147 @@
+ # must still merge into one group via the A-B-C chain.
+ a = [1.0, 0.0]
+ b = [0.995, 0.0999] # dist(a,b) ~= 0.005
+ c = [0.98, 0.19] # dist(b,c) ~= 0.045; dist(a,c) ~= 0.051 -- both under 0.10 anyway,
+ # but the point is the grouping algorithm doesn't require a single
+ # global anchor -- verified structurally via 3-way group below.
</file context>
1 task
The diarization work landed on the shared branch as the rebased commits (stenolabs#455), while this branch was cut from their pre-rebase originals, so the same changes arrived twice with different ancestry. Every conflicting hunk was that duplication and is resolved to this branch's side; the merge then introduced a second copy of formatElapsedSeconds, which is removed again. Genuinely new from the shared branch, and the only net change here: the processing-stages spec now launches with fakeAudio (the CI runner has no audio device) and waits for the generation bump to settle before emitting.
Collaborator
|
#455 is merged into Why it conflicted at all, since it looked worse than it was: #455 was rebased onto the shared branch, while this branch was cut from the pre-rebase originals. So the same diarization changes arrived twice with different ancestry — 60 conflict hunks across 7 files, all of them that duplication. None of us spotted it when we agreed on the rebase; it's nobody's bug, just a consequence of rebasing a branch someone else was already building on. How I resolved it: every conflicting hunk to this branch's side, since your identity work is the superset there ( The net change against your previous head is only this: — the three Verified before pushing: 828 Python tests OK, renderer typecheck clean, lint 37 warnings / 0 errors (unchanged), ruff unchanged — the merge touches no Python relative to your head. I did not run e2e locally; after last night I'd rather let this CI run be the signal than tell you a green macOS run means something. Still yours to decide: |
Collaborator
|
Merging this into One thing said plainly before it disappears into the branch history: the conflict resolution in
If any of that reads wrong to you, Still open and yours: |
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
2 participants
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.
Part 2 of #359 (see that issue for full background/testing status). Builds on #455 (per-channel acoustic diarization) with human-confirmed cross-recording speaker identification.
Note on the diff: this branch already carries #455's diarization commits internally (re-applied while this branch was rebuilt), and
feat/speaker-diarizationdoesn't have #455 merged yet, so this diff currently shows both. Once #455 merges, re-diffing/rebasing will shrink this down to just what's below.What's new here (on top of #455)
PersonProfile/SpeakerPrototypeCRUD (src/config.py), with hard-negative and context-aware (in-person vs. remote) evidence.src/speaker_suggestions.py): gates a candidate match on threshold, margin, stability across a meeting's segments, and hard-negative evidence from prior rejections, plus same-meeting fragment merging, transcript relabeling, and sample-audio/text extraction so a user can hear/read a clip before confirming.SpeakerReviewPanel, wired into the meeting detail view): approve / change / new-person / keep-generic, with duplicate-name prevention.speakersIPC group.delete_person_profilenow also strips the deleted person's voiceprint out of every other profile's mutual hard-negative list (previously it only removed the deleted person's own entry, so their sample kept living on inside everyone else's hard negatives indefinitely).Honest caveat
Cross-meeting matching quality isn't great yet — validating raw embedding-similarity against the AMI Meeting Corpus found that people sharing a room/mic score artificially similar regardless of true identity, and no threshold/margin tuning fixed that safely without also rejecting correct matches. This is why it's confirm-first, never a silent auto-assign. Within-meeting speaker separation (telling Speaker 1 from Speaker 2) is the reliable, validated part; cross-recording identity is a bonus when it works, not the thing carrying the feature.
Testing
python -m unittest discover tests)tsc --noEmitclean,eslintclean (no new warnings)node --test+ vitest unit tests passspeaker-diarization.t2,speaker-naming.t2,speaker-review.t1Summary by cubic
Adds human‑confirmed speaker identification across recordings on top of per‑channel acoustic diarization, with a review UI, person profiles, and a macOS diarization/embedding sidecar. Improves processing feedback and adds a setting to disable identity matching.
New Features
SpeakerReviewPanel) to approve/change/new-person/keep-generic, with sample audio preview and duplicate-name prevention.speakersIPC group and CLI (suggest/confirm/backfill/enroll-self/report, etc.).steno-diarizesidecar (Swift/CoreML + embeddings) for mono and per‑channel diarization; up to four voices per channel, with transcript relabeling to confirmed names.identity_matching_enabled), respected by the pipeline and backfill.Bug Fixes
is_diarisedreflects real multi‑speaker output; long sentences split by word timings to avoid mislabels.PYTHONUNBUFFERED=1to keep progress alive; better surfacing of graceful CLI errors; processing-stages test runs withfakeAudioon CI and waits for the generation bump.Written for commit fab3bba. Summary will update on new commits.