Skip to content

Simplify DegenerateOutputDetector and harden detector tests#2

Merged
Newarr merged 1 commit into
mainfrom
refactor/simplify-decode-loop-followup
May 11, 2026
Merged

Simplify DegenerateOutputDetector and harden detector tests#2
Newarr merged 1 commit into
mainfrom
refactor/simplify-decode-loop-followup

Conversation

@Newarr

@Newarr Newarr commented May 11, 2026

Copy link
Copy Markdown
Owner

Summary

Follow-up to #1 (Cohere/MLX decode-loop fix). Three parallel review agents (reuse, quality, efficiency) flagged a handful of cleanup opportunities in the detector and its tests. This PR applies the low-risk, no-behavior-change subset.

  • Hoist .lowercased() out of the tri-gram loop in DegenerateOutputDetector.evaluate - was lowercasing each word 3x per iteration plus once more for the unique-word check; now once up front (~6k -> ~1.5k transient String allocations on a 1,477-word transcript).
  • Drop the unreachable if words.count >= 3 guard nested inside the >= 30 branch.
  • Trim bug-story narration from the type-level doc comments on CohereMLXBackendError and DegenerateOutputDetector. The "583-second recording" / "Blaizzy fork" / gzip rationale belongs in the PR description, not in source where it will rot.
  • Replace the try? chain on the fixture load in testDegenerateOutputDetectorFlagsRepetitiveTranscripts with try so a missing/broken fixture fails loudly instead of silently asserting on "".
  • Make the integration test audio path overridable via SCRIBE_MLX_INTEGRATION_AUDIO. The dated ~/Scribe/2026-05-11-1756/audio.m4a default is kept for back-compat.

Skipped intentionally

  • Promoting degenerateOutput(reason: String, ...) to a structured DegenerationReason enum - invasive public API change for a diagnostic field.
  • Hoisting 30 / 0.08 / 0.10 / 120 to named constants - values already documented in the docstring with rationale; no other call sites.
  • Renaming CohereRustBackendTests.swift -> CohereMLXBackendTests.swift - pre-existing mismatch, out of scope.
  • Nesting CohereMLXBackendError inside CohereMLXBackend - purely stylistic.

Test plan

  • swift build clean
  • swift test - 365 tests pass, 1 integration test skipped (unchanged from main)
  • SCRIBE_MLX_INTEGRATION_AUDIO=~/Scribe/2026-05-11-1756/audio.m4a SCRIBE_RUN_MLX_INTEGRATION=1 swift test --filter testFailingRecordingTranscribesWithoutDegenerationIntegration still passes on the failing recording (optional spot check)

Follow-up cleanups on top of the Cohere/MLX decode-loop fix (PR #1):

- Hoist `.lowercased()` out of the tri-gram loop in
  `DegenerateOutputDetector.evaluate` (was lowercasing each word 3x per
  iteration plus once more for the unique-word check; now once up front).
- Drop the unreachable `if words.count >= 3` guard inside the `>= 30`
  branch.
- Trim bug-story narration from the type-level doc comments
  (`CohereMLXBackendError`, `DegenerateOutputDetector`) - the changelog
  belongs in the PR, not the source.
- Replace the `try?` chain on the fixture load in
  `testDegenerateOutputDetectorFlagsRepetitiveTranscripts` with `try` so
  a missing/broken fixture fails loudly instead of silently asserting on
  an empty string.
- Make the integration test audio path overridable via
  `SCRIBE_MLX_INTEGRATION_AUDIO` (the dated default path is kept for
  back-compat).

No behavior change. 365 tests still pass (1 integration test skipped).
@Newarr Newarr merged commit ef178e8 into main May 11, 2026
1 check failed
@Newarr Newarr deleted the refactor/simplify-decode-loop-followup branch May 11, 2026 17:48
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.

1 participant