Problem
resolve_model_path_uses_cache_and_does_not_rescan in
crates/core/src/transcription/model/tests.rs (lines 329–359) contains a
hard-coded 1 100 ms sleep at line 341:
// tests.rs:338–342
// Sleep so snapshot B gets an unambiguously newer mtime than A; a fresh
// `resolve_model_path_in_dir` scan sorts by mtime desc and would pick B. If
// the cache is honored, we still get A — proving no rescan (TR-3).
std::thread::sleep(std::time::Duration::from_millis(1100));
let snapshot_b = create_parakeet_snapshot(tmp.path(), "snap-b");
The sleep exists solely to guarantee that the filesystem assigns a strictly
newer mtime to snap-b than to snap-a, satisfying the mtime-descending
sort inside resolve_model_path_in_dir (catalog.rs lines 144–152):
// catalog.rs:144–152
let modified = entry
.metadata()
.and_then(|meta| meta.modified())
.unwrap_or(std::time::SystemTime::UNIX_EPOCH);
candidates.push((modified, path));
// ...
candidates.sort_by(|a, b| b.0.cmp(&a.0));
Without the sleep, snap-a and snap-b can share the same 1-second-resolution
mtime on HFS+/APFS (and many Linux filesystems), making sort order undefined and
the assertion unreliable.
Why it matters
- CI latency — this single test adds ≥1.1 s of wall-clock time unconditionally;
it runs on every cargo test invocation of ansible-core.
- Flake risk — the approach depends on filesystem mtime granularity. On
filesystems with sub-second precision (ext4, btrfs, APFS) the sleep is usually
sufficient, but any CI environment whose clock is throttled or where tmpfs
coalesces timestamps can still produce equal mtimes, causing a non-deterministic
test failure.
- Coupling to a private implementation detail — the test verifies cache
correctness (a public-ish invariant) by exploiting mtime ordering (a private
scan heuristic). If the scan strategy changes, the test breaks for the wrong
reason.
Proposed change
Preferred approach — inject deterministic mtimes (no sleep):
- Add
filetime as a [dev-dependencies] entry in crates/core/Cargo.toml.
- After creating
snap-a, call
filetime::set_file_mtime(&snap_a, filetime::FileTime::from_unix_time(1_000_000, 0)).
- After creating
snap-b, set its mtime one second later:
filetime::FileTime::from_unix_time(1_000_001, 0).
- Delete
std::thread::sleep(...) at line 341.
- The rest of the test body (lines 343–358) is unchanged.
Alternative — assert cache hit through an observable signal (no mtime dependency):
If pinning filetime as a dependency is undesirable, the cache invariant can be
asserted without relying on mtime ordering at all:
- After
refresh_download_status() caches snap-a, delete the snap-b
directory's parent from the filesystem (so a rescan would return
Err/None), then assert resolve_model_path still returns snap-a. This
proves the cache is consulted without requiring any ordering guarantee.
- The existing
resolve_model_path_falls_back_when_cache_stale test (lines
361–380) already covers the inverse path (stale cache → fallback rescan), so
the test suite stays complete either way.
Tests that must change: only resolve_model_path_uses_cache_and_does_not_rescan
(lines 329–359). No other tests touch the sleep; no production code changes.
Recommendation / triage
DO — pure test improvement, removes CI latency + flake. Prefer setting
deterministic mtimes (filetime crate or set_file_mtime) over sleeping; or
assert cache behavior through an observable (no rescan) signal.
Acceptance criteria
Traceability
- clawpatch finding ID:
fnd_sig-feat-service-60d65539ae-3efc_ddc627fe81
- Revalidate command:
clawpatch revalidate --finding fnd_sig-feat-service-60d65539ae-3efc_ddc627fe81 --reasoning-effort high
- Verified against current code at
crates/core/src/transcription/model/tests.rs:341 and crates/core/src/transcription/model/catalog.rs:144–152 (no line drift from the original finding).
- No coupled issues at this time.
Problem
resolve_model_path_uses_cache_and_does_not_rescanincrates/core/src/transcription/model/tests.rs(lines 329–359) contains ahard-coded 1 100 ms sleep at line 341:
The sleep exists solely to guarantee that the filesystem assigns a strictly
newer
mtimetosnap-bthan tosnap-a, satisfying the mtime-descendingsort inside
resolve_model_path_in_dir(catalog.rs lines 144–152):Without the sleep,
snap-aandsnap-bcan share the same 1-second-resolutionmtime on HFS+/APFS (and many Linux filesystems), making sort order undefined and
the assertion unreliable.
Why it matters
it runs on every
cargo testinvocation ofansible-core.filesystems with sub-second precision (ext4, btrfs, APFS) the sleep is usually
sufficient, but any CI environment whose clock is throttled or where tmpfs
coalesces timestamps can still produce equal mtimes, causing a non-deterministic
test failure.
correctness (a public-ish invariant) by exploiting mtime ordering (a private
scan heuristic). If the scan strategy changes, the test breaks for the wrong
reason.
Proposed change
Preferred approach — inject deterministic mtimes (no sleep):
filetimeas a[dev-dependencies]entry incrates/core/Cargo.toml.snap-a, callfiletime::set_file_mtime(&snap_a, filetime::FileTime::from_unix_time(1_000_000, 0)).snap-b, set its mtime one second later:filetime::FileTime::from_unix_time(1_000_001, 0).std::thread::sleep(...)at line 341.Alternative — assert cache hit through an observable signal (no mtime dependency):
If pinning
filetimeas a dependency is undesirable, the cache invariant can beasserted without relying on mtime ordering at all:
refresh_download_status()cachessnap-a, delete thesnap-bdirectory's parent from the filesystem (so a rescan would return
Err/None), then assertresolve_model_pathstill returnssnap-a. Thisproves the cache is consulted without requiring any ordering guarantee.
resolve_model_path_falls_back_when_cache_staletest (lines361–380) already covers the inverse path (stale cache → fallback rescan), so
the test suite stays complete either way.
Tests that must change: only
resolve_model_path_uses_cache_and_does_not_rescan(lines 329–359). No other tests touch the sleep; no production code changes.
Recommendation / triage
DO — pure test improvement, removes CI latency + flake. Prefer setting
deterministic mtimes (
filetimecrate orset_file_mtime) over sleeping; orassert cache behavior through an observable (no rescan) signal.
Acceptance criteria
std::thread::sleepremoved fromresolve_model_path_uses_cache_and_does_not_rescan.snap-a) wins over a newer unscanned snapshot (snap-b).refresh_download_status(), the newer snapshot is picked.cargo test -p ansible-corepasses with no flaky failures on CI.resolve_model_pathandrefresh_download_statusunchanged.refresh_populates_resolved_path_cache,resolve_model_path_falls_back_when_cache_stale,delete_model_invalidates_resolved_path_cache) continue to pass unmodified.Traceability
fnd_sig-feat-service-60d65539ae-3efc_ddc627fe81crates/core/src/transcription/model/tests.rs:341andcrates/core/src/transcription/model/catalog.rs:144–152(no line drift from the original finding).