feat(meeting): in-person mode with per-speaker mic diarization - #100
feat(meeting): in-person mode with per-speaker mic diarization#100gbrunoo wants to merge 2 commits into
Conversation
Add an Online/In person capture mode to the meeting window. In-person records the microphone only (no system audio, no Screen Recording permission) and diarizes the mic track so each person in the room becomes a numbered speaker, with no privileged "You". - MeetingMode persisted on MeetingTranscript (decodeIfPresent, defaults to online so archived transcripts keep rendering) - Timestamped MicAudioChunk so the diarizer has a mic timeline - In-person forces diarization on and bypasses echo suppression (no remote audio to echo); labels reuse .others(speakerIndex:) - Header mode toggle (locked while recording); in-person shows a single Room level; up to four participants (Sortformer limit) - History sidebar now closed by default; opening it widens the panel and closing restores the prior compact width unless the user resized it - Header controls never wrap: Start/Stop and the level indicator collapse to icon-only / hide when the window is too narrow
sebsto
left a comment
sebsto
left a comment
There was a problem hiding this comment.
Reviewed against the #97 design — this is a faithful Model B implementation, no silent divergence. Everyone in person becomes a numbered .others(speakerIndex:) with no privileged "You" (so speakerColor/displayName needed no change), MeetingMode rides on MeetingTranscript with a decodeIfPresent .online default, the timestamped MicAudioChunk mirrors SystemAudioChunk (the sample→startTime bookkeeping is line-for-line the same as the system path), diarization is forced on in person, echo suppression is bypassed, the header toggle is locked while recording, and .inPerson returns before touching ScreenCaptureKit so the Screen Recording prompt is genuinely avoided. Cold-start chunks fall back to .others(nil) → "Others", matching the answer to Q3.
Privacy/security is clean: no new network or telemetry, and the debug logs carry sample counts and timestamps, never audio or transcript text. Concurrency is tidy too — the diarizer is driven by exactly one path per mode, so there's no concurrent-ingest race, and the new AppKit callback captures [weak self].
Everything below is Low. The one worth acting on is coverage: the three new behaviors (mode back-compat, the in-person label path, and the echo-suppression bypass) all ship without tests, and this repo normally tests exactly this kind of change.
(No build verification from my side — Apple-platform code; CI on the Mac runners is green.)
| self.speakerNames = | ||
| try container.decodeIfPresent([String: String].self, forKey: .speakerNames) ?? [:] | ||
| self.title = try container.decodeIfPresent(String.self, forKey: .title) | ||
| self.mode = try container.decodeIfPresent(MeetingMode.self, forKey: .mode) ?? .online |
There was a problem hiding this comment.
[Tests] No back-compat decode test for mode. The repo already tests this exact class of change — TranscriptStoreTests.swift:75 ("Legacy JSON without title decodes with no title") and :31 for speakerNames. Worth the sibling test: a legacy transcript JSON with no mode key decodes as .online, and .inPerson survives an encode/decode round-trip. Cheap to add and it locks the archived-file guarantee in place.
| /// room saying similar short phrases). | ||
| private func record(_ entry: MeetingTranscriptEntry) { | ||
| guard settingsStore.meetingEchoSuppressionEnabled else { | ||
| guard mode == .online, settingsStore.meetingEchoSuppressionEnabled else { |
There was a problem hiding this comment.
[Maintainability] This reads the live mode rather than the frozen transcript.mode. transcript.mode is documented (MeetingTranscript.swift:89-92) as "Fixed at startMeeting() and never changed mid-session" — it's the canonical session mode, and it's already set at start. Gating on it here (and in transcribeMicAudio/warmUpDiarizerIfEnabled) would make the echo-suppression bypass depend on that invariant instead of on the header staying locked. It's correct today only because the toggle is disabled while recording.
[Tests] The in-person bypass itself is uncovered — MeetingEchoSuppressionTests drive appendSuppressingEcho at the transcript level, not the mode gate in record(), so nothing asserts that in person a would-be echo is kept.
| speakerIndex = await diarizer.dominantSpeaker( | ||
| in: chunk.startTime...chunkEnd) | ||
| } | ||
| speaker = .others(speakerIndex: speakerIndex) |
There was a problem hiding this comment.
[Tests] The in-person label path has no coverage. Nothing asserts that a resolved chunk yields .others(index) while a cold-start / diarization-off chunk yields .others(nil) (rendered "Others"), and never .you. MeetingStateManagerTests.swift:103 ("MeetingSpeaker display names are correct") is the natural place to extend — this is the behavioural heart of the feature.
| minWidth: showHistory | ||
| ? Self.compactMinimumWidth + Self.historyWidthIncrement | ||
| : Self.compactMinimumWidth, | ||
| minHeight: 420 |
There was a problem hiding this comment.
[Maintainability] This PR nicely shares width through compactMinimumWidth / historyWidthIncrement so the panel and the view can't drift apart. The min-height is the odd one out: 420 is a bare literal here while MeetingWindowPanel.minimumHeight is a named 420 — the same value declared in two spots. Fold it into a shared constant the same way you did the widths.
| minHeight: 420 | |
| minHeight: Self.minimumHeight |
(with a static let minimumHeight: CGFloat = 420 alongside the width constants, referenced from the panel too.)
Addresses review feedback on sebsto#100. - Gate labelling, diarizer warmup and echo suppression on transcript.mode (the canonical session mode, fixed at startMeeting) rather than the live header binding, so correctness no longer rests on the toggle being disabled while recording - Share minimumHeight between MeetingTranscriptView and MeetingWindowPanel, matching how the widths are already shared - Extract micSpeaker(mode:diarizedIndex:) so the label decision is testable without microphone hardware - Add tests: mode back-compat (legacy JSON decodes .online) and round-trip; in-person labels .others(index)/.others(nil) and never .you; in-person keeps a would-be echo, with an online control proving the bypass is what changed the outcome
2 participants
Add an Online/In person capture mode to the meeting window. In-person records the microphone only (no system audio, no Screen Recording permission) and diarizes the mic track so each person in the room becomes a numbered speaker, with no privileged "You".