Deferred from the PR #175 review loop. Neither is a merge blocker; both are real and worth closing.
1. CaptureDiagnostics.qualityAnomalyCount is untested
qualityAnomalyCount is the computation that ultimately decides whether a user sees "capture anomalies" on a finished recording. The surrounding paths are covered — the JSON round-trip (ProvenanceCompatTests.currentShapeRoundTrips) and the file-read (CaptureQualityNoticeTests) — but nothing builds a CaptureDiagnostics ring, records a mix of events, and asserts the filtered count.
That is the seam where the calibration decision actually lives: the whole point of qualityCompromising is that a benign recovered .streamStopError must NOT count while a .rateDrift must. A test should pin exactly that, so a future edit to the set can't quietly re-include the benign kinds and destroy the signal (see gotcha #62).
Suggested: record one .streamStopError + one .micFormatDetected + one .rateDrift into a ring, assert anomalyCount sees the anomalies and qualityAnomalyCount sees only the rate drift.
2. Zero-frame source pair skips the archive duration guard
AudioArchiver.archive:
let expectedSeconds = Double(max(micFile.length, sysFile.length)) / sampleRate
For a pair of zero-frame WAVs this is 0.0, and verify has guard let expectedSeconds, expectedSeconds > 0 else { return } — so the duration check is skipped and the archive falls back to the weak "non-empty file + has an audio track" bar before the source WAVs are deleted.
An empty recording is contrived, but an early-crash orphan can plausibly be near-zero, and this is the guard that licenses deleting the only lossless copy (gotcha #60). Refusing to archive a zero-frame pair outright is probably the right answer — there is nothing to archive, so deleting the sources buys nothing.
Context
Both surfaced in the auto-review of #175, which found several genuine defects — including three cases where a fix introduced or left open the counterpart of the race it closed. Worth keeping the loop honest rather than letting these lapse.
Deferred from the PR #175 review loop. Neither is a merge blocker; both are real and worth closing.
1.
CaptureDiagnostics.qualityAnomalyCountis untestedqualityAnomalyCountis the computation that ultimately decides whether a user sees "capture anomalies" on a finished recording. The surrounding paths are covered — the JSON round-trip (ProvenanceCompatTests.currentShapeRoundTrips) and the file-read (CaptureQualityNoticeTests) — but nothing builds aCaptureDiagnosticsring, records a mix of events, and asserts the filtered count.That is the seam where the calibration decision actually lives: the whole point of
qualityCompromisingis that a benign recovered.streamStopErrormust NOT count while a.rateDriftmust. A test should pin exactly that, so a future edit to the set can't quietly re-include the benign kinds and destroy the signal (see gotcha #62).Suggested: record one
.streamStopError+ one.micFormatDetected+ one.rateDriftinto a ring, assertanomalyCountsees the anomalies andqualityAnomalyCountsees only the rate drift.2. Zero-frame source pair skips the archive duration guard
AudioArchiver.archive:For a pair of zero-frame WAVs this is
0.0, andverifyhasguard let expectedSeconds, expectedSeconds > 0 else { return }— so the duration check is skipped and the archive falls back to the weak "non-empty file + has an audio track" bar before the source WAVs are deleted.An empty recording is contrived, but an early-crash orphan can plausibly be near-zero, and this is the guard that licenses deleting the only lossless copy (gotcha #60). Refusing to archive a zero-frame pair outright is probably the right answer — there is nothing to archive, so deleting the sources buys nothing.
Context
Both surfaced in the auto-review of #175, which found several genuine defects — including three cases where a fix introduced or left open the counterpart of the race it closed. Worth keeping the loop honest rather than letting these lapse.