[4/12] feat: intelligence layer — diarization, LLM cleanup, runtime qualification, benchmarks

[Mvkd108]

Status: REVIEW ONLY — do not merge. PR 4 of 12 in a stacked series; stacked on #12. This PR shows only its own chunk. Series intro, divergence notes and full map: #10. Next: #14.

This PR (4/12): intelligence layer — diarization, LLM cleanup, runtime qualification, benchmarks

Commits:

• 87a148d feat: native speaker diarization and speaker alias service — Services/NativeDiarizationClient.cs, Services/SpeakerAliasService.cs
• cd1c540 feat: local LLM text cleanup and transcription pipeline — Services/NativeTextCleanupService.cs, Services/FillerWordFilter.cs, Services/TranscriptionPipelineService.cs
• 57cec50 feat: native runtime qualification and status mapping — Services/NativeRuntimeQualificationService.cs, Services/RuntimeStatusMapper.cs
• a2f6b7a feat: transcription benchmarking and capture retention policy — Services/TranscriptionBenchmarkService.cs, Services/BenchmarkCaptureRetentionPolicy.cs

10 files, +2,015.

Review focus

• NativeDiarizationClient + SpeakerAliasService — speaker embedding pipeline, alias assignment stability across segments.
• NativeTextCleanupService — off by default; verify graceful fallback to the raw transcript when the GGUF model is absent or LLamaSharp fails (no hard dependency).
• NativeRuntimeQualificationService + RuntimeStatusMapper — CPU/CUDA capability detection and the user-facing readiness strings.
• TranscriptionBenchmarkService + BenchmarkCaptureRetentionPolicy — benchmark methodology; retention of captured audio used for benchmarks (privacy).

Expected — do not flag: not wired into the app until PR 9/10.

Known limits

• Qwen GGUF cleanup is v1 and disabled by default. There is no guided model-download UX: the user must place the GGUF in the model cache manually. Flagged as a roadmap item, not a defect of this PR.

Test evidence

• Full stack at tip: build clean (--no-restore); 484/484 passing.
• Suite lands in PR 11; classes covering this chunk: TextAndSummaryTests, CapturePrivacyAndRuntimeTests.

---

Previous: #12 | Next: #14

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 67438507-4069-4a3a-b42c-2f529496b31d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[socket-security]

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	nuget/​llamasharp.backend.cpu@​0.27.0
	nuget/​llamasharp@​0.27.0

View full report

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[claude]

Claude finished @Mvkd108's task in 3m 25s —— View job

---

Claude finished reviewing this PR

Tasks

• Gather context (PR body, diff, CLAUDE.md)
• Review NativeDiarizationClient + SpeakerAliasService
• Review NativeTextCleanupService + FillerWordFilter + TranscriptionPipelineService
• Review NativeRuntimeQualificationService + RuntimeStatusMapper
• Review TranscriptionBenchmarkService + BenchmarkCaptureRetentionPolicy
• Post consolidated review feedback

---

Summary

Solid chunk overall — the model-verification, graceful-fallback, and determinism-check patterns are consistent with the rest of the stack. A few findings worth a look before this is wired in at PR 9/10.

Findings

1. Stale disposed model reused after a failed reload — NativeTextCleanupService.EnsureModelLoaded (Services/NativeTextCleanupService.cs:304-320)

private void EnsureModelLoaded(string modelPath)
{
    if (_model is not null && string.Equals(_loadedModelPath, modelPath, StringComparison.OrdinalIgnoreCase))
    {
        return;
    }

    _model?.Dispose();
    _modelParams = new ModelParams(modelPath) { ... };
    _model = LLamaWeights.LoadFromFile(_modelParams);   // <- if this throws...
    _loadedModelPath = modelPath;
}

If LoadFromFile throws on a reload (switching to a different GGUF path than the currently-loaded one), _model still points at the just-Dispose()d instance (the assignment never completes) and _loadedModelPath is left unchanged at the old value. If a later call happens to pass that same old modelPath again, the early-return guard sees _model is not null && _loadedModelPath == modelPath and skips reloading — but _model is disposed, so the subsequent _model.CreateContext(...) will throw ObjectDisposedException. It's caught by the outer try/catch in CleanupAsync, so this degrades silently to "return raw transcript" rather than corrupting anything — but it's a real latent bug and worth a null-out on failure, e.g.:

_model?.Dispose();
_model = null;
_loadedModelPath = null;
_modelParams = new ModelParams(modelPath) { ... };
_model = LLamaWeights.LoadFromFile(_modelParams);
_loadedModelPath = modelPath;

Narrow trigger (requires two distinct GGUF files in the cache and a failed reload of one after a successful load of the other), but since "Known limits" says users manually drop GGUF files into the cache, having >1 present is plausible.

Fix this →

2. BenchmarkCaptureRetentionPolicy is defined but never called anywhere (Services/BenchmarkCaptureRetentionPolicy.cs)

ShouldRetain isn't referenced anywhere in this diff (or the rest of the tree). TranscriptionBenchmarkService.FindBenchmarkAudioPath reads last-dictation.wav / last-meeting-system.wav directly with only a Length > 44 check — it doesn't consult the retention policy at all. Given the review focus explicitly calls out "retention of captured audio used for benchmarks (privacy)," this is presumably intentional per the PR's "not wired into the app until PR 9/10" note, but as it stands there's no code path anywhere that purges/filters captures per this policy yet — worth confirming the enforcement lands in the wiring PR rather than being dropped.

3. SpeakerAliasService.Apply — sequential substitution could cascade across aliases (Services/SpeakerAliasService.cs:14-29)

Aliases are applied one at a time, mutating result in place, ordered longest-key-first. If one alias's value happens to equal another alias's key (e.g. {"speaker_0": "speaker_1", "speaker_1": "Alice"}), the second replacement will re-match text that was just substituted by the first, producing an unintended double-hop. Low practical risk since source keys are diarization labels (speaker_N) and values are human names/aliases in different namespaces, but worth a defensive note (e.g. apply all substitutions against the original text with a single-pass tokenizer/map rather than sequential in-place replace) if user-editable aliases are ever chained.

Minor / non-blocking

• NativeTextCleanupService.SpeakerLineRegex/BracketSpeakerLineRegex (NativeTextCleanupService.cs:15-21) duplicate TranscriptionPipelineService.TimestampSpeakerLine/BracketSpeakerLine (TranscriptionPipelineService.cs:8-14) almost verbatim. Could be consolidated into a shared helper to avoid the two drifting apart later.

What looked good

• NativeDiarizationClient's model download/verification path (HTTPS-only enforcement, SHA-256 pinning, atomic temp-file + rename, cross-process lock, path-containment check in TryDeleteDirectory) is careful and consistent with the pattern used elsewhere in the stack.
• NativeTextCleanupService.CleanupAsync fails closed correctly — every failure path (runtime unavailable, no model, exception, unsafe-cleanup heuristic, structure-changed) returns the original raw transcript rather than partially-cleaned output.
• TranscriptionBenchmarkService/NativeRuntimeQualificationService determinism checks (transcript hash, segment-layout hash, provider consistency, model-instance-reuse) are a genuinely useful methodology for catching drift across runs.
• Dependency addition (LLamaSharp / LLamaSharp.Backend.Cpu 0.27.0, CPU-only) matches the "off by default, no hard dependency" design described in the PR body.

Not flagged (per PR's own scope note)

Not wired into MainWindow/settings/UI yet — expected, per PR body ("not wired into the app until PR 9/10").
· Branch: feat/transcription-benchmarking