From 09257e9e9790a18a9043dff0c5dc3c33cf17ff34 Mon Sep 17 00:00:00 2001 From: Szymon Sypniewicz Date: Mon, 11 May 2026 18:49:57 +0100 Subject: [PATCH] Address codex review of CohereMLXBackend degenerate-output detector MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ups to PR #1 raised by an independent codex review: 1. Privacy: drop `sample` from `CohereMLXBackendError.degenerateOutput`. `TranscriptionWorker` persists `String(describing: error)` into the failed `transcript.md` and `metadata.json` artifacts, so the 120-char transcript snippet that used to ride along on the error was landing in on-disk failure metadata. The sample now stays in the unified log at `.private` privacy level for live debugging only. 2. False-positive floor: require the dominant tri-gram to repeat at least 5 times before firing, in addition to the >8% fraction check. At the 30-word minimum, 3 occurrences of an ordinary tri-gram (e.g. "thank you very") cleared the fraction gate at 3/28 ≈ 11% and terminally failed otherwise-usable short transcripts. The observed real-world loop fires 12+ times so the new floor is still well within range. New regression test guards the short-transcript-with-three-repeats case; the existing throw test now also asserts that `String(describing:)` on the error does NOT contain transcript content. --- .../Engines/CohereMLXBackend.swift | 22 ++++++++++++--- .../Engines/CohereRustBackendTests.swift | 28 +++++++++++++++++-- 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/Sources/TranscriberCore/Engines/CohereMLXBackend.swift b/Sources/TranscriberCore/Engines/CohereMLXBackend.swift index f66eed4..f52735f 100644 --- a/Sources/TranscriberCore/Engines/CohereMLXBackend.swift +++ b/Sources/TranscriberCore/Engines/CohereMLXBackend.swift @@ -72,8 +72,14 @@ public protocol CohereMLXAudioDurationReading: Sendable { /// Terminal errors surfaced by `CohereMLXBackend.transcribe(_:)`. Marked /// terminal so `TranscriptionWorker.isTransient` (which classifies by type) /// does not retry — deterministic local inference would just loop again. +/// +/// `reason` is a structural summary only (tri-gram stats, unique-word fraction) +/// and never contains transcript content. `TranscriptionWorker` persists +/// `String(describing: error)` into `transcript.md`/`metadata.json` on +/// failure, so anything embedded in this case lands on disk — keep +/// transcript text out of the payload. public enum CohereMLXBackendError: Error, Sendable, Equatable { - case degenerateOutput(reason: String, sample: String) + case degenerateOutput(reason: String) } /// Heuristic guard against decode-loop output (model emits the same phrase @@ -96,7 +102,12 @@ enum DegenerateOutputDetector { let key = "\(words[i]) \(words[i + 1]) \(words[i + 2])" counts[key, default: 0] += 1 } + // Require BOTH an absolute minimum repeat count AND a high fraction. + // The fraction alone would flag a 30-word transcript with three + // repeats of "thank you very" (3/28 ≈ 11%) as degenerate; an + // observed real-world loop has 12+ repeats, well above the floor. if let (top, n) = counts.max(by: { $0.value < $1.value }), + n >= 5, Double(n) / Double(total) > 0.08 { return "tri-gram \"\(top)\" repeats \(n)/\(total) times" } @@ -168,9 +179,12 @@ public final class CohereMLXBackend: TranscriptionEngine, @unchecked Sendable { ) let output = try await adapter.transcribe(localRequest) if let reason = DegenerateOutputDetector.evaluate(output.text) { - let sample = String(output.text.prefix(120)) - Log.engine.error("Cohere MLX produced degenerate output: \(reason, privacy: .public)") - throw CohereMLXBackendError.degenerateOutput(reason: reason, sample: sample) + // Sample stays in the unified log at `.private` for live debugging + // and is intentionally NOT carried on the thrown error — the + // worker persists `String(describing:)` to disk and meeting + // content must not land in the failure artifact. + Log.engine.error("Cohere MLX produced degenerate output: \(reason, privacy: .public); sample: \(output.text.prefix(120), privacy: .private)") + throw CohereMLXBackendError.degenerateOutput(reason: reason) } let utterances: [EngineResponse.Utterance] if output.segments.isEmpty { diff --git a/Tests/TranscriberCoreTests/Engines/CohereRustBackendTests.swift b/Tests/TranscriberCoreTests/Engines/CohereRustBackendTests.swift index 169e37c..aae4812 100644 --- a/Tests/TranscriberCoreTests/Engines/CohereRustBackendTests.swift +++ b/Tests/TranscriberCoreTests/Engines/CohereRustBackendTests.swift @@ -407,16 +407,38 @@ final class CohereMLXBackendTests: XCTestCase { XCTFail("Backend must throw on degenerate output, not silently return") } catch let error as CohereMLXBackendError { switch error { - case .degenerateOutput(let reason, let sample): + case .degenerateOutput(let reason): XCTAssertTrue(reason.contains("tri-gram") || reason.contains("unique-word"), "Reason should identify the failure mode, got: \(reason)") - XCTAssertFalse(sample.isEmpty, "Sample should include a snippet of the failing transcript") - XCTAssertLessThanOrEqual(sample.count, 120, "Sample is capped at 120 chars") + let serialized = String(describing: error) + XCTAssertFalse( + serialized.contains("I think that's what I'm hearing"), + "String(describing:) must not embed transcript content; TranscriptionWorker persists this verbatim to disk" + ) } } catch { XCTFail("Expected CohereMLXBackendError.degenerateOutput, got \(error)") } } + + func testDetectorDoesNotFireOnShortTranscriptWithFewRepeats() { + // 30+ words, "thank you very" tri-gram appears 3 times. Before the + // `n >= 5` floor was added this fired (3/28 ≈ 11% > 8%) and the + // backend threw, costing the user a usable transcript. + let words: [String] = [ + "thank", "you", "very", "much", "for", + "joining", "the", "call", "today", + "thank", "you", "very", "much", "again", + "before", "we", "wrap", + "thank", "you", "very", "kindly", + "and", "have", "a", "wonderful", "rest", "of", "your", "afternoon", "everyone" + ] + XCTAssertGreaterThanOrEqual(words.count, 30) + XCTAssertNil( + DegenerateOutputDetector.evaluate(words.joined(separator: " ")), + "Three repeats of a common tri-gram in a short transcript is not degenerate" + ) + } } private final class RecordingLocalAdapter: CohereMLXTranscribing, @unchecked Sendable {