Non-blocking items surfaced in the PR #154 (#135) round-4 review — all confirmed by the reviewer as non-bugs (cosmetic / readability / coverage), deferred rather than expanding the PR further. Worth closing before v0.9.0 ships.
1. End-to-end multiple-orphan recovery test (highest value)
All end-to-end recover() tests use exactly one orphan WAV; multiple orphans is the canonical post-crash scenario. Ordering is covered at the planner level (CrashRecoveryPlannerTests.multipleUnprocessedWavsAllReturnedAscending) but not end-to-end in the recovered transcript. Suggested test (from the review): seed chunk 0 in session.json, drop m-1.wav + m-2.wav as orphans, recover(), assert the transcript covers all 3 chunks.
2. Nil-return path + stale session.json cleanup
ChunkedSessionRecovery line ~56: guard !state.chunks.isEmpty else { return nil } is reachable when orphan WAVs exist but every one transcribes empty. That path is untested, and it returns nil without deleting session.json (cosmetic — the caller deletes the sentinel so recovery isn't re-attempted; not data-loss). Add a FakeEngine-returns-empty test and a try? FileManager.removeItem(session.json) in the guard.
3. Document safeRestartChunkIndex's full-path contract
CrashRecoveryPlanner.safeRestartChunkIndex passes sentinel.systemAudioPath (a full absolute path) to stripSegmentSuffix, which extracts the bare session ID. Correct and consistent across all three call sites (verified by the two floor tests), but the signature doesn't advertise that it accepts a full path — a one-line doc comment would prevent a maintainer from double-stripping.
4. (optional) TranscriptionRunner double-optional readability
The SpeakerSampleLocator.durations(of:).first.flatMap({$0}) chain (pre-existing) could use the explicit two-level if let form. No logic change; purely stylistic.
Device-test on-device already exercises the real recovery flow as a v0.9.0 release gate; these harden the automated coverage around it.
Non-blocking items surfaced in the PR #154 (#135) round-4 review — all confirmed by the reviewer as non-bugs (cosmetic / readability / coverage), deferred rather than expanding the PR further. Worth closing before v0.9.0 ships.
1. End-to-end multiple-orphan recovery test (highest value)
All end-to-end
recover()tests use exactly one orphan WAV; multiple orphans is the canonical post-crash scenario. Ordering is covered at the planner level (CrashRecoveryPlannerTests.multipleUnprocessedWavsAllReturnedAscending) but not end-to-end in the recovered transcript. Suggested test (from the review): seed chunk 0 insession.json, dropm-1.wav+m-2.wavas orphans,recover(), assert the transcript covers all 3 chunks.2. Nil-return path + stale
session.jsoncleanupChunkedSessionRecoveryline ~56:guard !state.chunks.isEmpty else { return nil }is reachable when orphan WAVs exist but every one transcribes empty. That path is untested, and it returnsnilwithout deletingsession.json(cosmetic — the caller deletes the sentinel so recovery isn't re-attempted; not data-loss). Add aFakeEngine-returns-empty test and atry? FileManager.removeItem(session.json)in the guard.3. Document
safeRestartChunkIndex's full-path contractCrashRecoveryPlanner.safeRestartChunkIndexpassessentinel.systemAudioPath(a full absolute path) tostripSegmentSuffix, which extracts the bare session ID. Correct and consistent across all three call sites (verified by the two floor tests), but the signature doesn't advertise that it accepts a full path — a one-line doc comment would prevent a maintainer from double-stripping.4. (optional)
TranscriptionRunnerdouble-optional readabilityThe
SpeakerSampleLocator.durations(of:).first.flatMap({$0})chain (pre-existing) could use the explicit two-levelif letform. No logic change; purely stylistic.Device-test on-device already exercises the real recovery flow as a v0.9.0 release gate; these harden the automated coverage around it.