Skip to content

feat(diarize): per-channel acoustic speaker diarization (Sortformer) - #455

Merged
Optic00 merged 12 commits into
stenolabs:feat/speaker-diarizationfrom
valentinweyer:diarization-only
Aug 4, 2026
Merged

feat(diarize): per-channel acoustic speaker diarization (Sortformer)#455
Optic00 merged 12 commits into
stenolabs:feat/speaker-diarizationfrom
valentinweyer:diarization-only

Conversation

@valentinweyer

@valentinweyer valentinweyer commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Description

Adds acoustic speaker diarization for both stereo (mic/system) and mono recordings, splitting multiple speakers sharing one side of a call or a single mic into distinct labeled turns instead of the current flat "You"/"Others" split. This picks up issue #359, scoped down to diarization only. A separate follow-up PR covers human-confirmed cross-recording speaker identification (naming a "Speaker 2" from a past meeting), which depends on this one and is a substantially larger review surface (persistent identity storage, matching quality caveats) that deserves its own slower pass.

Diarization runs through a new Swift/CoreML sidecar (diarize-sidecar/) wrapping FluidAudio's Sortformer diarizer, invoked from Python (src.transcriber._run_steno_diarize), never from Electron, since the batch pipeline is entirely Python-orchestrated. It is macOS-only and gated in stenoai.spec on both the platform and the sidecar binary existing, so a checkout that skips the Swift build (or Windows/Linux) falls back to the existing channel-only "You"/"Others" labeling automatically. Diarization can never fail a meeting: any failure (missing binary, timeout, bad output, a single-cluster result) degrades to the legacy behavior byte-for-byte.

Also adds a live per-stage progress indicator ("Identifying speakers" with an elapsed-time counter) so a long diarization pass on a multi-hour recording no longer sits on a static "Analyzing transcript" spinner with no feedback, and a heartbeat mechanism so Electron's inactivity watchdog does not kill a long-running diarization call.

Type of Change

  • New feature

Testing

  • 538 backend unit tests passing (python -m unittest discover tests), including new coverage for the sidecar's JSON parsing (real-world quirks: FluidAudio/CoreML warning text before, between, or after the payload; a dominant-speaker gate to avoid spawning phantom speakers from misdiarization blips) and the progress/heartbeat plumbing.
  • ruff check clean on every file this PR touches.
  • Renderer tsc --noEmit clean, eslint clean (0 errors) on the new Processing stage.
  • Full T1 e2e tier (53 specs) passing, including a new processing-stages.t1.spec.ts covering the stage-transition logic end to end.
  • speaker-diarization.t2.spec.ts: synthesizes real speech via macOS say so Parakeet/whisper.cpp produce real ASR segments, points the sidecar at a fixture script, and asserts the saved transcript's per-channel labeling and cross-channel speaker numbering.
  • Beyond the automated suite, validated directly against real multi-hour, multi-speaker recordings on real hardware: built the sidecar, ran it on real audio, and confirmed the Python parser handles the actual production output (including the FluidAudio warning-prefix case) correctly end to end. A full DMG build was tested from a cold launch (no background services running) to confirm the full record/transcribe/diarize/summarize pipeline works in the packaged app.

Additional Notes

Additional Notes

Sortformer has a fixed 4-speaker-slot architecture (no speaker-count hint can be passed to it). This is a known limitation flagged in #359 and its discussion; the identity follow-up PR's discussion covers a possible "who is in this meeting" field as a future lever, which is out of scope here.

The rendered transcript UI (chat bubbles) itself is unchanged by this PR, and it already benefits directly: when a channel has two diarized speakers, e.g. two people sharing one mic with no separate system-audio channel, both show up as two visually distinct bubbles (the dominant speaker as "You", green/right; the other as grey/left), where today they would be merged into one "You" bubble. That per-channel split is the actual feature.

Its limit is at three-plus: the bubble styling only has two visual buckets ("You" vs everyone else), so if diarization finds a third or fourth distinct speaker, e.g. three people sharing one mic, or extra speakers split across both channels, the transcript text still correctly carries distinct "Speaker 2"/"Speaker 3"/"Speaker 4" labels, but the bubbles do not visually differentiate them from each other, or from "Others" — all render identically grey/left, unless a speaker has been given a confirmed real name (the identity follow-up PR). That richer per-speaker label data is already useful today regardless (readable in the raw transcript text/export, and it's what the identity PR's suggestion engine keys off of); giving 3+ distinct speakers their own visual treatment in the bubble UI is a reasonable follow-up, not something this PR does.


Summary by cubic

Adds acoustic speaker diarization for stereo and mono recordings on macOS via diarize-sidecar (Sortformer), labeling turns as “You” and “Speaker N” instead of channel-only. Adds an “Identifying speakers” stage with an elapsed timer; diarization is optional and always falls back to legacy labels if unavailable or low-confidence.

  • New Features

    • macOS-only diarize-sidecar (Swift/CoreML) using FluidAudio Sortformer; invoked from Python and bundled when present, otherwise auto-fallback to “You”/“Others”.
    • Per-channel diarization on stereo and mono: the dominant speaker per channel is “You”; additional speakers are labeled “Speaker N” by first appearance across channels; transcript parser and UI render these labels (up to four voices per channel).
    • Live diarization progress via PROGRESS:diarize:* and a heartbeat; new “Identifying speakers” stage with an elapsed time counter; progress lines are persisted in the processing log.
  • Bug Fixes

    • Diarization now counts as “on” only when multiple speaker labels are present, so empty second channels no longer drop labeled transcripts.
    • Long ASR sentences are split and reassigned at the word level to match diarizer turns, improving speaker alignment.
    • Fixed stage transitions and progress handling: ignore PROGRESS:diarize:* for summarization, clear stale sub-labels on transitions, and persist diarize progress markers.
    • Processing-stage tests stabilized by waiting for the session header before emitting events and launching with fakeAudio on CI runners without audio devices.
    • UI now treats only exact “You” (or no marker) as self; “Speaker N” and “Others” render on the left.

Written for commit 05679be. Summary will update on new commits.

Review in cubic

@Optic00 Optic00 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I ran this branch's sidecar against an independent diarization benchmark rather than reading
the diff alone. Measured on one M5 Air, scored with dscore at a 0.25 s collar against the AMI
reference RTTMs from BUTSpeechFIT/AMI-diarization-setup, on the test split (16 meetings,
9.06 h, Array1-01, one distant room microphone) - the closest public stand-in for what this
PR targets, several people sharing one channel.

It works, and the margin is large

system DER JER
all remote speech under one label (today's system-audio channel) 71.09 89.82
this branch's sidecar 29.24 39.46

Throughput ~105x realtime, about 35 s for a one-hour meeting. This is strong evidence that
the PR materially improves diarization on the case it targets. (FluidAudio's own CLI scored
33.55 on the same material, but that run used a different library version and a likely
different code path, so I would not read anything into the gap either way.)

To be precise about the baseline: it is a one-label system scored on single-channel audio,
so it stands in for the channel where Steno is blind today, not for the whole app - the mic
channel already gets "You" right by construction.

Two claims in the code comments I could not reproduce

The .highContextV2 switch made no measurable difference here. I rebuilt the sidecar with
sortformerHighContextMinDuration raised so .default is always chosen, and reran all 16
meetings: DER identical to two decimals, wall-clock identical too.

AMI meetings are 14-40 minutes, so this does not contradict the PR body's "~6.6x faster" if
that was measured on the multi-hour recordings you tested against. If the gain only appears
past some length, that would explain why I could not reproduce it on shorter material. Not
asking for a change.

Forcing .cpuAndNeuralEngine costs a little more than the comment implies. With
STENOAI_DIARIZE_COMPUTE_UNITS=all: DER 28.89 (vs 29.24) and RTFx 145 (vs 105), so GPU is
marginally more accurate and ~1.4x faster. For the live-recording path the power-first
default still looks right, and the env-var escape hatch is already there. Just recording the
numbers, since the comment reasons about the trade without them.

The phantom-speaker case, which came out better than I expected

EN2002c has 3 reference speakers and both this sidecar and upstream report 4 - the model
fills its slots whether the people exist or not. I also built a 2-speaker fixture (28.8 s,
two macOS say voices, distinctness verified spectrally first) where 0.48 s at the first
speaker change goes to a third cluster holding 1.7 % of speech.
CHANNEL_DOMINANCE_THRESHOLD = 0.92 does not catch it, since the dominant cluster there
holds only 57 %.

It does not reach the user. I drove transcribe_diarised on that file through the real
Parakeet path: the output carries exactly [You] and [Speaker 2], both correct, and no
Speaker 3. The phantom overlaps only a 1.44 s ASR segment, stays under
LONG_SENTENCE_SPLIT_THRESHOLD_S, is assigned whole to the nearest real cluster, then
dropped for carrying no text. So the dominance gate is not the only thing protecting you, and
the design holds under a case built specifically to break it. I had drafted a suggestion for
an extra minimum-cluster guard and dropped it - it would buy nothing observable and would
misattribute the participant who says "approved" once in an hour.

One unrelated thing visible in that same run: a 5.92 s sentence crossing a real speaker
boundary triggers word-level splitting, and the trailing full stop lands past the boundary,
so it opens the next turn ([Speaker 2] ... around the hiring plan / [You] . Sure, let me pull that up.). Cosmetic, and only for sentences past the 5 s threshold that straddle a
turn change.

Scope: shipping diarization before identity looks right to me

Today a three-person call renders every remote utterance as one undifferentiated Others.
Anonymous Speaker 1/2/3 is the normal first rung, turn structure is what makes such a
transcript readable, and the fallback means any failure lands exactly where users are now. I
do not think this needs the identity PR to be worth merging.

One caveat worth stating now rather than after: anonymous labels pay off in the transcript.
In summaries, search and action items, "Speaker 2 owns the follow-up" has no stable referent.
That is an argument for not letting the identity PR sit, not against merging this one.
Related: the summariser now carries speaker attributions it did not have before, and a
confidently wrong attribution is a failure class Others could not produce. Nothing I
measured suggests it is likely, but it is what I would watch first in front of users.

Since the cap will shape that follow-up, I put the measurements in #359 rather than here

  • you flagged the 4-speaker limit there yourself, and the short version is that a supplied
    participant count is worth real DER, which makes it a cheap field on a form that is going to
    ask about speakers anyway. Nothing in it argues against this PR's backend choice; if
    anything it vindicates it for a pipeline that has to estimate.

Caveats

No German, and no non-English validation at all - my German fixtures turned out to be
degenerate (one synthetic voice recorded four times), so they measure nothing. AMI is
meeting-room audio, not VoIP system audio. I would not extrapolate 29.24 to either.

Verified in passing

The stdout contamination handling is right, and worth saying because it looks like
over-engineering until you hit it: my run produced exactly the E5RT encountered an STL exception. msg = unordered_map::at: key not found. line ahead of the JSON, with clean
stderr. Scanning every [ with a real decoder and keeping the last segment-shaped array is
what makes that survivable.

Also: keep the Package.resolved pin. The newer FluidAudio I compared against scored worse
on this material, so whoever lifts it should measure first.

Happy to share the run tables, the control runs, or the harness. Nice piece of work - the
fallback discipline in particular is what makes this reviewable as a single change.

@Optic00

Optic00 commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

One concrete follow-up to my review above, which I would rather name precisely than leave as
"the cap should be documented somewhere".

This PR makes an existing FAQ answer untrue. docs/faq.mdx, under "Can Steno record
in-person meetings?":

In-person recordings are mic-only, so they have no [You] / [Others] speaker labels.

After this merges that is the exact case that gains speaker labels - the mono path is the
headline of the feature. Left as is, the docs tell the in-person user the thing they are
looking for does not exist.

It is also the natural home for the four-slot limit, which currently lives only in the PR's
Additional Notes. A replacement that covers both, worded per channel since that is the real
constraint:

Yes. Steno records from your Mac's microphone, which will pick up voices in the room. For
best results, place your laptop centrally. Steno separates up to four distinct voices per
recording channel, so an in-person meeting of four or fewer people gets per-speaker labels;
beyond that, additional speakers are merged into the four it detects.

Two reasons for putting it in this PR rather than an issue: it ships with the feature that
causes it, and "up to four per channel" is a good deal less restrictive than a bare "four
speakers" - for a call, that is four people on the far side plus you, so most users never
meet the limit and should not be told otherwise.

Not a blocker from me either way. If you would rather keep this PR code-only, say so and I
will open it as a docs issue against the merge instead.

@Optic00

Optic00 commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Heads-up on CI, since I do not think this one has been looked at: T2 pipeline (transcribe + summarize) is red on the current head (f31f2c9), and it is your own new spec. It failed
three times including both retries, so it is not a flake. Windows is green only because the
spec skips there on process.platform !== 'darwin'.

The line that fails, e2e/specs/speaker-diarization.t2.spec.ts:67:

Expected: < 0.5053125
Received:   1.5
const tailSeconds = Math.max(1.5, Math.min(3.0, micBoundarySeconds * 0.6));
expect(tailSeconds).toBeLessThan(micBoundarySeconds);

The floor and the invariant contradict each other: Math.max(1.5, ...) guarantees
tailSeconds >= 1.5, so the assertion cannot hold for any micBoundarySeconds <= 1.5. On
your machine the boundary is comfortably above that, which is why it passes locally.

But the boundary being 0.5 s is the finding that matters. makeStereoSpeechWav returns
durA + gapSeconds / 2, so with the 1 s gap the runner produced durA = 5.3 ms of audio for

Hello there, I hope you are having a wonderful and productive day today.

I ran the same say -o out.wav --data-format=LEI16@16000 call on my own Mac for comparison:
3.796 s for that sentence, 0.961 s for "Thanks a lot." So say on the GitHub-hosted
macOS runner is writing essentially nothing, roughly 170 bytes of PCM. My guess is that the
runner image ships the say binary without the voice assets, but I have not confirmed that
and it does not really matter for the fix.

What does matter is that isSayAvailable() cannot see it: it probes say -v ?, which exits
0 on the runner, so the guard stays green while the fixture is empty.

So fixing the formula alone would make the spec pass without testing anything - the WAV
would carry no speech, ASR would produce no sentence segments, and there would be nothing
for the per-channel labeling to work on. The honest fix is a duration guard on what say
actually wrote (skip loudly below, say, 1 s of audio, the way the spec already skips on a
missing model) plus a tailSeconds expression that cannot exceed the boundary, e.g.
Math.min(micBoundarySeconds * 0.6, ...) with no floor above it.

None of this touches the substance of the PR, which I still think is the right backend
choice - see the benchmark in my review above.

@Optic00

Optic00 commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Heads-up before you rebase this onto current main: processing-stages.t1 goes red, 2 of 3 tests. It is not your spec and not main alone - the spec only exists here, the queue watchdog only exists on main, and they meet for the first time in that merge.

Mechanism, read off the screen rather than inferred: after processing-complete {success:false} the label shows Analyzing transcript with data-stage="transcribing" instead of the error panel. Processing.tsx bumps generation when activeSession flips from null to the session name (or recording.status reaches recording), and the reset block then calls setStage('transcribing') during render. That runs after your error transition and undoes it.

Worth a look beyond the test: a genuinely failed recording whose session identity settles late would show Analyzing transcript instead of the failure.

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.
…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.
@Optic00
Optic00 changed the base branch from main to feat/speaker-diarization August 3, 2026 18:27
@Optic00

Optic00 commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Retargeted the base to feat/speaker-diarization — thanks for the quick rebase, it merges cleanly.

One red check left: T1, processing-stages.t1.spec.ts:95, 3 of 3 attempts in CI. I reproduced it locally on e129c0a7, where it trips one assertion earlier (112 rather than 113 — the window is sub-second).

The error panel never renders: after processing-complete {success: false} the screen still shows the "Analyzing transcript" stage card.

Mechanism, and it isn't your code. processingComplete sets setStage('error') (Processing.tsx:225). The same event also reaches the app-level listener mounted at App.tsx:89, which invalidates the queue query (useRecording.ts:500). That refetch is the first to observe the recording openProcessing() starts at spec:26 and never stops — the mock reports hasRecording: true and sessionName: 'test-session' (e2e-mock-ipc.js:296, 301). It flips both signals generation folds together (Processing.tsx:124-132), so the render-phase reset block (:143-152) runs one IPC round-trip after the error was set and clears it again: setStage('transcribing') (:145), setRetryAudioFile(null) (:150). With retryAudioFile gone, canRetry is false (:445), so there is no usable retry button either.

Evidence rather than reading: patching only :145 to setStage((s) => (s === 'error' ? s : 'transcribing')) makes 112 and 113 pass and moves the failure to line 120, the Try again click — which is exactly what :150 predicts.

I diffed the branch against its merge base 4e6fcb52: inside that handler your change is clearDiarizeTimer() + setChunkProgress(null) (:223-224). setStage('error') and the whole generation/reset machinery (:107-152) are untouched main code. So this is a new spec meeting pre-existing behaviour, not a regression you introduced.

I'd fix it on the spec side rather than in production. The reset exists so a screen that stays mounted across back-to-back recordings repaints fresh (:107-118); letting a terminal error stage survive the bump would leave a new session staring at the previous one's error panel. The cheapest fix is to let the generation settle before emitting anything — for instance have openProcessing() wait until the session name is painted in the header (displayTitle, :395, rendered at :423), since that implies the bump has already been consumed. Stopping the recording before the failure emit is also closer to how a user reaches this screen, and still leaves sessionName truthy via currentJob for canRetry. I have not tried either, so treat both as suggestions rather than a tested patch.

Everything else is green: 11 checks, including both Windows jobs and both pipeline runs.

@Optic00

Optic00 commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Here's the spec-side fix, tested rather than suggested this time — on e129c0a7, all three tests in the file, three repeats each, 9/9 green:

--- a/e2e/specs/processing-stages.t1.spec.ts
+++ b/e2e/specs/processing-stages.t1.spec.ts
@@ -28,6 +28,11 @@ async function openProcessing(page: Page) {
     window.location.hash = '/meetings/processing';
   });
   await expect(page.getByTestId('processing-stage-label')).toBeVisible();
+  // The queue poll that first reports this recording bumps Processing's
+  // `generation`, whose render-phase reset clears stage + retryAudioFile.
+  // Wait until that has landed (the header switches from 'Note' to the
+  // session name) so a later emit can't race the reset.
+  await expect(page.getByRole('heading', { name: 'test-session' })).toBeVisible();
 }

Why it settles the race: the header is <h1>{displayTitle}</h1> with displayTitle = … ?? activeSession ?? 'Note' (Processing.tsx:395, 423). Once the session name is painted, the generation bump has been through a render and the reset can no longer clear anything a later emit sets. No assertion gets weaker — the error panel and the retry button are still asserted exactly as you wrote them.

Yours to take or leave, it's your PR. One weaker observation while I was in there: :51 emits the first progress line right after the visibility check, which doesn't strictly guarantee the IPC listeners are mounted. I did not see it fail, so I'd leave it alone unless it ever goes flaky.

…essing-stages.t1

openProcessing() returned before the queue poll's first report of the
mock recording finished bumping Processing.tsx's `generation`, whose
render-phase reset (:143-152) then cleared a caller's terminal stage
(setStage('error')) one round-trip later. Wait for the session name to
paint in the header first, so the reset has already landed before any
event is emitted.

Root-caused and verified (9/9 green, 3 repeats) by Ben/Optic00 on PR stenolabs#455.
… land

The generation-settle wait fixed the error-panel race but exposed a
different, pre-existing one: the renderer attaches its IPC listeners
in a useEffect that runs after the initial paint, so an event sent
right after the DOM updates can land in the gap before that effect
mounts and be silently dropped (webContents.send has no
queueing/replay). CI's runner is apparently slower/more loaded than
local, tipping this from theoretical into a real intermittent
failure across all three tests in the file.

emitUntil() resends on a short interval until the expected UI change
is actually observed, rather than firing once and hoping. Safe to
repeat: every handler this spec drives is idempotent once its target
state is reached, and the loop stops as soon as the check passes.
@Optic00

Optic00 commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Correction on the patch I posted above: I verified it on macOS only, and it made things worse. Before it, T1 failed on one test; with it (2060434f) tests 1 and 2 fail instead, with the emitted progress event having no effect at all. Please feel free to revert that line — it isn't the fix, and I shouldn't have called it tested when the environment it fails in wasn't part of the test.

The lead I'd chase instead: openProcessing() starts a real recording, but all three launchApp calls omit fakeAudio: true. Every other spec in the suite that records sets it, including notification-navigation.t1 on main. On a runner with no audio device getUserMedia rejects, and the resulting failure path opens a second window — which would make emit()'s BrowserWindow.getAllWindows()[0] the wrong target from that point on. That is a permanent misroute rather than a race, which is exactly what "five seconds of resending changed nothing" looks like, and it fits the macOS/Ubuntu split.

I am not asserting that yet. I have it running on our own fork right now with fakeAudio: true plus a probe that logs the window list at every emit, so one run distinguishes it either way, and I'll report the result here whichever way it comes out.

@Optic00

Optic00 commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Result, as promised. It is fakeAudio, and it's the only thing missing.

I took your current head (33369815, i.e. after your emitUntil revert) and changed nothing but the three launchApp calls:

-  const { app, page } = await launchApp({ mockIpc: true });
+  const { app, page } = await launchApp({ mockIpc: true, fakeAudio: true });

On the same Ubuntu runner the whole T1 suite goes green — 68 passed, including all three tests in this file:

✓ processing-stages.t1.spec.ts:48  walks transcribing -> diarizing -> summarizing -> finalizing …
✓ processing-stages.t1.spec.ts:80  shows a ticking elapsed-time counter throughout diarization …
✓ processing-stages.t1.spec.ts:100 a processing failure swaps to the error panel …

Why: openProcessing() starts a real recording, and the CI runner has no audio device, so getUserMedia rejects and the capture-failure path runs instead. Every other spec in the suite that records sets fakeAudio for exactly this reason — the fixture starts Chromium with --use-fake-device-for-media-stream. It's the same class of failure the live-transcript specs hit back in July, which is why it only ever showed up in CI and never locally.

Your revert of emitUntil was right — with the flag set, the plain emit calls are enough. The openProcessing wait can stay or go; it is green either way, and it does prevent the error-panel race on its own.

Evidence, in case you want to look: run on our fork, branch built from your head with that one change.

The CI runner has no audio device, so openProcessing()'s recording start
fails there and the emitted progress events never take effect. Every other
spec in the suite that records already sets this flag. Verified green on the
Ubuntu runner from this exact head: 68 passed.
@Optic00
Optic00 marked this pull request as ready for review August 4, 2026 04:51
@Optic00
Optic00 requested a review from ruzin as a code owner August 4, 2026 04:51
@Optic00

Optic00 commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

As agreed, I've pushed the fix onto this branch rather than leaving it waiting: 05679be9, the three fakeAudio: true calls and nothing else. It's an additive commit on top of your head, so it's a plain fast-forward for you — and if you'd rather solve it differently, revert it without asking, no hard feelings.

I've also taken it out of draft. The draft status was my own suggestion from back when this targeted main; now that it targets feat/speaker-diarization, which exists specifically for this feature, that reason is gone.

Once CI is green I'll merge it into feat/speaker-diarization as a merge commit, not a squash, so your individual commits and their authorship stay intact.

That should also clean up #472 for you: with the diarization commits in the base branch, merging feat/speaker-diarization into speaker-identity will collapse that diff from +12k down to the identity work alone, and it picks up this test fix on the way. The one thing left there afterwards is speaker-review.t1:177, which expects a button labelled "Stop sample" and finds "Play sample" — that's a question about your feature's intent rather than test plumbing, so I've left it for you.

@cubic-dev-ai cubic-dev-ai Bot left a comment

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.

11 issues found across 20 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: Docs currently imply acoustic speaker separation applies only when system audio is enabled, which can mislead microphone-only users about available labeling behavior. Consider describing diarization as available on microphone-only recordings too, with channel split only when system audio is present.</violation>
</file>

<file name="docs/faq.mdx">

<violation number="1" location="docs/faq.mdx:93">
P2: This FAQ now promises per-speaker labeling for in-person meetings as a guaranteed behavior, which can mislead Windows users or macOS installs running fallback mode. Consider scoping this to macOS/availability and phrasing as best-effort rather than guaranteed.</violation>
</file>

<file name="stenoai.spec">

<violation number="1" location="stenoai.spec:260">
P2: Non-mac builds can still bundle `steno-diarize` if the file exists, because the new darwin-only `elif` misses and the generic `else` path still includes it under `ollama/`. Consider explicitly handling `base == 'steno-diarize'` and `continue` so non-darwin really skips this sidecar.</violation>
</file>

<file name="e2e/fixtures/say-stereo-wav.ts">

<violation number="1" location="e2e/fixtures/say-stereo-wav.ts:113">
P3: The temp dirs (and the `say`-generated WAV files inside them) created by makeStereoSpeechWav and the isSayAvailable probe are never removed, so every e2e run leaks a fresh `stenoai-e2e-say-*` plus a probe dir under os.tmpdir. That diverges from the repo convention (other specs/fixtures rmSync their temp dirs) and accumulates garbage on long-lived dev machines and CI agents. Wrap the body in a try/finally that calls `rmSync(dir, {recursive:true, force:true})`, and delete the probe dir on the non-throwing path of isSayAvailable.</violation>
</file>

<file name="diarize-sidecar/Sources/main.swift">

<violation number="1" location="diarize-sidecar/Sources/main.swift:218">
P3: Segment ordering can change between runs when two segments have the same start time, because the comparator does not break ties after dictionary-derived iteration order. Adding deterministic tie-breakers (for example `end` then `speakerId`) keeps diarization JSON stable for downstream processing and tests.</violation>
</file>

<file name="src/transcriber.py">

<violation number="1" location="src/transcriber.py:605">
P3: The `if idx is None: continue` guard in the word-splitting branch is unreachable: `_find_nearest_diar_segment` returns `None` only when `diar_segments` is empty, and the function returns early in that case before reaching the token loop. Harmless, but it's dead defensive code that reads as if the loop can handle a missing segment — it can't — which is mildly misleading about the invariant. Consider dropping the guard (or documenting why it's unreachable).</violation>

<violation number="2" location="src/transcriber.py:766">
P2: Malformed diarizer JSON can still crash transcription instead of falling back to legacy labels. The conversion of `raw_segments` to floats is outside failure handling, so bad `start`/`end` fields raise and bypass the safe `None` fallback path.</violation>

<violation number="3" location="src/transcriber.py:1831">
P3: The mono path duplicates the turn-collapse + labelled-transcript assembly that the stereo path already implements. Since both now feed from the same `_tag_channel_segments`/`_resolve_speaker_placeholders` pipeline, extracting a small shared helper (e.g. `turns = _collapse_turns(tagged)` returning `(start, speaker, parts)`) would let both paths reuse the exact same turn-boundary logic and avoid future drift between the two labelling formats. Low severity, but the duplication is real and easy to unify.</violation>
</file>

<file name="e2e/specs/speaker-diarization.t2.spec.ts">

<violation number="1" location="e2e/specs/speaker-diarization.t2.spec.ts:73">
P3: The tail cap at 3.0s only guarantees SPEAKER_0 dominance, but the backend additionally rejects a two-cluster channel via CHANNEL_DOMINANCE_THRESHOLD=0.92 in src/transcriber._cluster_channel_labels. For this fixture, boundary/(boundary+3.0) crosses that gate once micBoundarySeconds exceeds ~34.5s (a sufficiently slow/long run of utterance A), which silently falls back to legacy single-label and fails the [Speaker 2]/[Speaker 3] assertions even though the fixture looks correct. The comment's 'dominant so two clusters pass' reasoning skips that separate, stricter condition. The hardcoded sentence stays well under this today, so it's a latent fragility; consider guarding the invariant explicitly so a future voice/longer sentence doesn't turn into a confusing flake.</violation>
</file>

<file name="app/renderer/src/routes/Processing.tsx">

<violation number="1" location="app/renderer/src/routes/Processing.tsx:580">
P2: The diarize elapsed ticker is not torn down when `generation` changes. If the user starts a new recording while the previous meeting is still in its long diarization stretch, the render-phase reset (setStage('transcribing') + setChunkProgress(null)) runs, but the old `setInterval` from the prior diarization keeps firing and re-writes a stale "Diarizing … channel… (Ns)" label onto the new, freshly-transcribing stage — since the StageCard now shows `chunkProgress` in every non-finalizing/error stage. That's the same category of stale-label leak the feature's tests specifically guard against, just on the generation path instead of the error/finalizing path. Consider clearing the timer in the generation-reset block (alongside `setChunkProgress(null)`), or adding `generation` to the IPC effect's dependency array so the interval is cleaned up on a new generation. Note the reset block runs during render, so a `clearInterval` ref-mutation there is consistent with the existing render-phase state resets.</violation>
</file>

<file name="app/renderer/src/lib/transcriptSegments.ts">

<violation number="1" location="app/renderer/src/lib/transcriptSegments.ts:23">
P2: The diarised-segment regex was widened from an explicit `(You|Others)` to match any `[^\]]+` content, so any bracketed text that appears inside a segment body — not just a real `[Speaker N]` marker — is now treated as a new speaker boundary. For example `[You] Call me at [5:00] tomorrow` is parsed into a phantom `5:00` speaker segment (rendered as an 'Others' bubble), and text after a lone trailing bracket gets dropped. This regresses the previous behavior where bracketed content stayed with its segment. Recommend constraining the marker to the actual label set the pipeline emits (e.g. `(?:You|Others|Speaker \d+)`) in both the captured group and the lookahead, so only genuine markers split segments.</violation>
</file>

Tip: instead of fixing issues one by one fix them all with cubic
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic

Comment thread docs/faq.mdx

<Accordion title="Can Steno record in-person meetings?">
Yes. Steno records from your Mac's microphone, which will pick up voices in the room. For best results, place your laptop centrally. In-person recordings are mic-only, so they have no `[You]` / `[Others]` speaker labels.
Yes. Steno records from your Mac's microphone, which will pick up voices in the room. For best results, place your laptop centrally. Steno separates up to four distinct voices per recording channel, so an in-person meeting of four or fewer people gets per-speaker labels; beyond that, additional speakers are merged into the four it detects.

@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.

P2: This FAQ now promises per-speaker labeling for in-person meetings as a guaranteed behavior, which can mislead Windows users or macOS installs running fallback mode. Consider scoping this to macOS/availability and phrasing as best-effort rather than guaranteed.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/faq.mdx, line 93:

<comment>This FAQ now promises per-speaker labeling for in-person meetings as a guaranteed behavior, which can mislead Windows users or macOS installs running fallback mode. Consider scoping this to macOS/availability and phrasing as best-effort rather than guaranteed.</comment>

<file context>
@@ -90,7 +90,7 @@ Steno can capture system audio -- the audio playing through your Mac's speakers
 
 <Accordion title="Can Steno record in-person meetings?">
-Yes. Steno records from your Mac's microphone, which will pick up voices in the room. For best results, place your laptop centrally. In-person recordings are mic-only, so they have no `[You]` / `[Others]` speaker labels.
+Yes. Steno records from your Mac's microphone, which will pick up voices in the room. For best results, place your laptop centrally. Steno separates up to four distinct voices per recording channel, so an in-person meeting of four or fewer people gets per-speaker labels; beyond that, additional speakers are merged into the four it detects.
 </Accordion>
 
</file context>
Suggested change
Yes. Steno records from your Mac's microphone, which will pick up voices in the room. For best results, place your laptop centrally. Steno separates up to four distinct voices per recording channel, so an in-person meeting of four or fewer people gets per-speaker labels; beyond that, additional speakers are merged into the four it detects.
Yes. Steno records from your Mac's microphone, which will pick up voices in the room. For best results, place your laptop centrally. On macOS, Steno can label up to four speakers per recording channel when diarization is available; if unavailable or if more than four people are speaking, labels may fall back or speakers may be merged.
Fix with cubic

Comment thread stenoai.spec
if base in ('ffmpeg', 'ffmpeg.exe'):
# Put ffmpeg at the root of the bundle for easy PATH access
binaries.append((filepath, '.'))
elif base == 'steno-diarize' and _IS_DARWIN:

@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.

P2: Non-mac builds can still bundle steno-diarize if the file exists, because the new darwin-only elif misses and the generic else path still includes it under ollama/. Consider explicitly handling base == 'steno-diarize' and continue so non-darwin really skips this sidecar.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At stenoai.spec, line 260:

<comment>Non-mac builds can still bundle `steno-diarize` if the file exists, because the new darwin-only `elif` misses and the generic `else` path still includes it under `ollama/`. Consider explicitly handling `base == 'steno-diarize'` and `continue` so non-darwin really skips this sidecar.</comment>

<file context>
@@ -257,6 +257,15 @@ if os.path.exists(ollama_bin_dir):
             if base in ('ffmpeg', 'ffmpeg.exe'):
                 # Put ffmpeg at the root of the bundle for easy PATH access
                 binaries.append((filepath, '.'))
+            elif base == 'steno-diarize' and _IS_DARWIN:
+                # macOS-only Swift/CoreML diarization sidecar (built by
+                # scripts/build-diarize-sidecar.sh). Root-level like ffmpeg
</file context>
Fix with cubic

Comment thread src/transcriber.py
Comment on lines +766 to +777
"start": float(s["start"]),
"end": float(s["end"]),
"speaker": str(s["speakerId"]),
}
for s in raw_segments
),
key=lambda s: s["start"],
)
return _merge_close_diar_segments(segments, STENO_DIARIZE_MERGE_GAP_S)


def _cluster_channel_labels(diar_segments: list[dict], legacy_label: str) -> Optional[dict[str, str]]:

@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.

P2: Malformed diarizer JSON can still crash transcription instead of falling back to legacy labels. The conversion of raw_segments to floats is outside failure handling, so bad start/end fields raise and bypass the safe None fallback path.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/transcriber.py, line 766:

<comment>Malformed diarizer JSON can still crash transcription instead of falling back to legacy labels. The conversion of `raw_segments` to floats is outside failure handling, so bad `start`/`end` fields raise and bypass the safe `None` fallback path.</comment>

<file context>
@@ -411,6 +488,400 @@ def _drop_per_segment_bleed(
+    segments = sorted(
+        (
+            {
+                "start": float(s["start"]),
+                "end": float(s["end"]),
+                "speaker": str(s["speakerId"]),
</file context>
Suggested change
"start": float(s["start"]),
"end": float(s["end"]),
"speaker": str(s["speakerId"]),
}
for s in raw_segments
),
key=lambda s: s["start"],
)
return _merge_close_diar_segments(segments, STENO_DIARIZE_MERGE_GAP_S)
def _cluster_channel_labels(diar_segments: list[dict], legacy_label: str) -> Optional[dict[str, str]]:
try:
segments = sorted(
(
{
"start": float(s["start"]),
"end": float(s["end"]),
"speaker": str(s["speakerId"]),
}
for s in raw_segments
),
key=lambda s: s["start"],
)
except (KeyError, TypeError, ValueError) as e:
logger.warning("steno-diarize emitted malformed segment payload: %s", e)
return None
return _merge_close_diar_segments(segments, STENO_DIARIZE_MERGE_GAP_S)
Fix with cubic

style={{ color: 'var(--fg-1)', fontFamily: 'var(--font-sans)' }}
>
{chunkProgress && stage === 'summarizing' ? chunkProgress : STAGE_LABEL[stage]}
{chunkProgress && stage !== 'finalizing' && stage !== 'error' ? chunkProgress : STAGE_LABEL[stage]}

@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.

P2: The diarize elapsed ticker is not torn down when generation changes. If the user starts a new recording while the previous meeting is still in its long diarization stretch, the render-phase reset (setStage('transcribing') + setChunkProgress(null)) runs, but the old setInterval from the prior diarization keeps firing and re-writes a stale "Diarizing … channel… (Ns)" label onto the new, freshly-transcribing stage — since the StageCard now shows chunkProgress in every non-finalizing/error stage. That's the same category of stale-label leak the feature's tests specifically guard against, just on the generation path instead of the error/finalizing path. Consider clearing the timer in the generation-reset block (alongside setChunkProgress(null)), or adding generation to the IPC effect's dependency array so the interval is cleaned up on a new generation. Note the reset block runs during render, so a clearInterval ref-mutation there is consistent with the existing render-phase state resets.

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

<comment>The diarize elapsed ticker is not torn down when `generation` changes. If the user starts a new recording while the previous meeting is still in its long diarization stretch, the render-phase reset (setStage('transcribing') + setChunkProgress(null)) runs, but the old `setInterval` from the prior diarization keeps firing and re-writes a stale "Diarizing … channel… (Ns)" label onto the new, freshly-transcribing stage — since the StageCard now shows `chunkProgress` in every non-finalizing/error stage. That's the same category of stale-label leak the feature's tests specifically guard against, just on the generation path instead of the error/finalizing path. Consider clearing the timer in the generation-reset block (alongside `setChunkProgress(null)`), or adding `generation` to the IPC effect's dependency array so the interval is cleaned up on a new generation. Note the reset block runs during render, so a `clearInterval` ref-mutation there is consistent with the existing render-phase state resets.</comment>

<file context>
@@ -523,10 +572,12 @@ function StageCard({
           style={{ color: 'var(--fg-1)', fontFamily: 'var(--font-sans)' }}
         >
-          {chunkProgress && stage === 'summarizing' ? chunkProgress : STAGE_LABEL[stage]}
+          {chunkProgress && stage !== 'finalizing' && stage !== 'error' ? chunkProgress : STAGE_LABEL[stage]}
         </span>
       </div>
</file context>
Fix with cubic

// the previous one.
const DIARISED_SEGMENT_RE =
/(?:\[(\d{1,3}:\d{2}(?::\d{2})?)\]\s*)?\[(You|Others)\]\s*([\s\S]*?)(?=(?:\[\d{1,3}:\d{2}(?::\d{2})?\]\s*)?\[(?:You|Others)\]|$)/g;
/(?:\[(\d{1,3}:\d{2}(?::\d{2})?)\]\s*)?\[([^\]]+)\]\s*([\s\S]*?)(?=(?:\[\d{1,3}:\d{2}(?::\d{2})?\]\s*)?\[[^\]]+\]|$)/g;

@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.

P2: The diarised-segment regex was widened from an explicit (You|Others) to match any [^\]]+ content, so any bracketed text that appears inside a segment body — not just a real [Speaker N] marker — is now treated as a new speaker boundary. For example [You] Call me at [5:00] tomorrow is parsed into a phantom 5:00 speaker segment (rendered as an 'Others' bubble), and text after a lone trailing bracket gets dropped. This regresses the previous behavior where bracketed content stayed with its segment. Recommend constraining the marker to the actual label set the pipeline emits (e.g. (?:You|Others|Speaker \d+)) in both the captured group and the lookahead, so only genuine markers split segments.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/renderer/src/lib/transcriptSegments.ts, line 23:

<comment>The diarised-segment regex was widened from an explicit `(You|Others)` to match any `[^\]]+` content, so any bracketed text that appears inside a segment body — not just a real `[Speaker N]` marker — is now treated as a new speaker boundary. For example `[You] Call me at [5:00] tomorrow` is parsed into a phantom `5:00` speaker segment (rendered as an 'Others' bubble), and text after a lone trailing bracket gets dropped. This regresses the previous behavior where bracketed content stayed with its segment. Recommend constraining the marker to the actual label set the pipeline emits (e.g. `(?:You|Others|Speaker \d+)`) in both the captured group and the lookahead, so only genuine markers split segments.</comment>

<file context>
@@ -3,28 +3,32 @@
+// the previous one.
 const DIARISED_SEGMENT_RE =
-  /(?:\[(\d{1,3}:\d{2}(?::\d{2})?)\]\s*)?\[(You|Others)\]\s*([\s\S]*?)(?=(?:\[\d{1,3}:\d{2}(?::\d{2})?\]\s*)?\[(?:You|Others)\]|$)/g;
+  /(?:\[(\d{1,3}:\d{2}(?::\d{2})?)\]\s*)?\[([^\]]+)\]\s*([\s\S]*?)(?=(?:\[\d{1,3}:\d{2}(?::\d{2})?\]\s*)?\[[^\]]+\]|$)/g;
 
 export function parseTranscript(text: string, isDiarised: boolean): Segment[] {
</file context>
Suggested change
/(?:\[(\d{1,3}:\d{2}(?::\d{2})?)\]\s*)?\[([^\]]+)\]\s*([\s\S]*?)(?=(?:\[\d{1,3}:\d{2}(?::\d{2})?\]\s*)?\[[^\]]+\]|$)/g;
/(?:\[(\d{1,3}:\d{2}(?::\d{2})?)\]\s*)?\[(You|Others|Speaker \d+)\]\s*([\s\S]*?)(?=(?:\[\d{1,3}:\d{2}(?::\d{2})?\]\s*)?\[(?:You|Others|Speaker \d+)\]|$)/g;
Fix with cubic

destPath: string,
opts: { micUtteranceA: string; micUtteranceB: string; systemUtterance: string },
): StereoSpeechResult {
const dir = mkdtempSync(path.join(tmpdir(), 'stenoai-e2e-say-'));

@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 temp dirs (and the say-generated WAV files inside them) created by makeStereoSpeechWav and the isSayAvailable probe are never removed, so every e2e run leaks a fresh stenoai-e2e-say-* plus a probe dir under os.tmpdir. That diverges from the repo convention (other specs/fixtures rmSync their temp dirs) and accumulates garbage on long-lived dev machines and CI agents. Wrap the body in a try/finally that calls rmSync(dir, {recursive:true, force:true}), and delete the probe dir on the non-throwing path of isSayAvailable.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At e2e/fixtures/say-stereo-wav.ts, line 113:

<comment>The temp dirs (and the `say`-generated WAV files inside them) created by makeStereoSpeechWav and the isSayAvailable probe are never removed, so every e2e run leaks a fresh `stenoai-e2e-say-*` plus a probe dir under os.tmpdir. That diverges from the repo convention (other specs/fixtures rmSync their temp dirs) and accumulates garbage on long-lived dev machines and CI agents. Wrap the body in a try/finally that calls `rmSync(dir, {recursive:true, force:true})`, and delete the probe dir on the non-throwing path of isSayAvailable.</comment>

<file context>
@@ -0,0 +1,127 @@
+  destPath: string,
+  opts: { micUtteranceA: string; micUtteranceB: string; systemUtterance: string },
+): StereoSpeechResult {
+  const dir = mkdtempSync(path.join(tmpdir(), 'stenoai-e2e-say-'));
+
+  const uttA = synthesize(opts.micUtteranceA, path.join(dir, 'mic_a.wav'));
</file context>
Fix with cubic

end: Double(seg.endTime)
)
}
.sorted { $0.start < $1.start }

@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: Segment ordering can change between runs when two segments have the same start time, because the comparator does not break ties after dictionary-derived iteration order. Adding deterministic tie-breakers (for example end then speakerId) keeps diarization JSON stable for downstream processing and tests.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At diarize-sidecar/Sources/main.swift, line 218:

<comment>Segment ordering can change between runs when two segments have the same start time, because the comparator does not break ties after dictionary-derived iteration order. Adding deterministic tie-breakers (for example `end` then `speakerId`) keeps diarization JSON stable for downstream processing and tests.</comment>

<file context>
@@ -0,0 +1,232 @@
+                    end: Double(seg.endTime)
+                )
+            }
+            .sorted { $0.start < $1.start }
+
+        let encoded = try JSONEncoder().encode(segments)
</file context>
Suggested change
.sorted { $0.start < $1.start }
.sorted { ($0.start, $0.end, $0.speakerId) < ($1.start, $1.end, $1.speakerId) }
Fix with cubic

Comment on lines +73 to +75
const tailSeconds = Math.min(micBoundarySeconds * 0.6, 3.0);
expect(tailSeconds).toBeLessThan(micBoundarySeconds);

@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 tail cap at 3.0s only guarantees SPEAKER_0 dominance, but the backend additionally rejects a two-cluster channel via CHANNEL_DOMINANCE_THRESHOLD=0.92 in src/transcriber._cluster_channel_labels. For this fixture, boundary/(boundary+3.0) crosses that gate once micBoundarySeconds exceeds ~34.5s (a sufficiently slow/long run of utterance A), which silently falls back to legacy single-label and fails the [Speaker 2]/[Speaker 3] assertions even though the fixture looks correct. The comment's 'dominant so two clusters pass' reasoning skips that separate, stricter condition. The hardcoded sentence stays well under this today, so it's a latent fragility; consider guarding the invariant explicitly so a future voice/longer sentence doesn't turn into a confusing flake.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At e2e/specs/speaker-diarization.t2.spec.ts, line 73:

<comment>The tail cap at 3.0s only guarantees SPEAKER_0 dominance, but the backend additionally rejects a two-cluster channel via CHANNEL_DOMINANCE_THRESHOLD=0.92 in src/transcriber._cluster_channel_labels. For this fixture, boundary/(boundary+3.0) crosses that gate once micBoundarySeconds exceeds ~34.5s (a sufficiently slow/long run of utterance A), which silently falls back to legacy single-label and fails the [Speaker 2]/[Speaker 3] assertions even though the fixture looks correct. The comment's 'dominant so two clusters pass' reasoning skips that separate, stricter condition. The hardcoded sentence stays well under this today, so it's a latent fragility; consider guarding the invariant explicitly so a future voice/longer sentence doesn't turn into a confusing flake.</comment>

<file context>
@@ -0,0 +1,129 @@
+    // where `say` produced near-silent audio (micBoundarySeconds well under
+    // 1.5s). isSayAvailable() now measures real synthesized duration, so
+    // that shouldn't recur, but this formula no longer depends on it either.
+    const tailSeconds = Math.min(micBoundarySeconds * 0.6, 3.0);
+    expect(tailSeconds).toBeLessThan(micBoundarySeconds);
+
</file context>
Suggested change
const tailSeconds = Math.min(micBoundarySeconds * 0.6, 3.0);
expect(tailSeconds).toBeLessThan(micBoundarySeconds);
const tailSeconds = Math.min(micBoundarySeconds * 0.6, 3.0);
// Keep the two-cluster ratio below CHANNEL_DOMINANCE_THRESHOLD (0.92) so the
// backend's _cluster_channel_labels doesn't silently downgrade to legacy.
expect(tailSeconds).toBeGreaterThan(micBoundarySeconds * 0.09);
expect(tailSeconds).toBeLessThan(micBoundarySeconds);
Fix with cubic

Comment thread src/transcriber.py
t_start = float(token.get("start") or 0.0)
t_end = float(token.get("end") or t_start)
idx = _find_nearest_diar_segment(t_start, t_end, diar_segments)
if idx is None:

@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 if idx is None: continue guard in the word-splitting branch is unreachable: _find_nearest_diar_segment returns None only when diar_segments is empty, and the function returns early in that case before reaching the token loop. Harmless, but it's dead defensive code that reads as if the loop can handle a missing segment — it can't — which is mildly misleading about the invariant. Consider dropping the guard (or documenting why it's unreachable).

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/transcriber.py, line 605:

<comment>The `if idx is None: continue` guard in the word-splitting branch is unreachable: `_find_nearest_diar_segment` returns `None` only when `diar_segments` is empty, and the function returns early in that case before reaching the token loop. Harmless, but it's dead defensive code that reads as if the loop can handle a missing segment — it can't — which is mildly misleading about the invariant. Consider dropping the guard (or documenting why it's unreachable).</comment>

<file context>
@@ -411,6 +488,400 @@ def _drop_per_segment_bleed(
+                t_start = float(token.get("start") or 0.0)
+                t_end = float(token.get("end") or t_start)
+                idx = _find_nearest_diar_segment(t_start, t_end, diar_segments)
+                if idx is None:
+                    continue
+                if run_index is not None and idx != run_index:
</file context>
Fix with cubic

Comment thread src/transcriber.py
tagged = _resolve_speaker_placeholders(tagged)

turns: list[tuple[float, str, list[str]]] = []
for start, speaker, text in tagged:

@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 mono path duplicates the turn-collapse + labelled-transcript assembly that the stereo path already implements. Since both now feed from the same _tag_channel_segments/_resolve_speaker_placeholders pipeline, extracting a small shared helper (e.g. turns = _collapse_turns(tagged) returning (start, speaker, parts)) would let both paths reuse the exact same turn-boundary logic and avoid future drift between the two labelling formats. Low severity, but the duplication is real and easy to unify.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/transcriber.py, line 1831:

<comment>The mono path duplicates the turn-collapse + labelled-transcript assembly that the stereo path already implements. Since both now feed from the same `_tag_channel_segments`/`_resolve_speaker_placeholders` pipeline, extracting a small shared helper (e.g. `turns = _collapse_turns(tagged)` returning `(start, speaker, parts)`) would let both paths reuse the exact same turn-boundary logic and avoid future drift between the two labelling formats. Low severity, but the duplication is real and easy to unify.</comment>

<file context>
@@ -1305,6 +1799,52 @@ def transcribe_diarised(self, audio_filepath: Path, language: str = "en") -> Opt
+        tagged = _resolve_speaker_placeholders(tagged)
+
+        turns: list[tuple[float, str, list[str]]] = []
+        for start, speaker, text in tagged:
+            if turns and turns[-1][1] == speaker:
+                turns[-1][2].append(text)
</file context>
Fix with cubic

@Optic00
Optic00 merged commit af55d2f into stenolabs:feat/speaker-diarization Aug 4, 2026
12 checks passed
Optic00 added a commit to valentinweyer/stenoai that referenced this pull request Aug 4, 2026
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants