fix(#796): pin the download-phase clock in tests instead of racing wall time

[djhenry]

Closes #796.

What changed and why

asset_sync::download_rate_tests::elapsed_does_not_carry_over_from_a_slow_prior_download proves the "elapsed is scoped to the CURRENT download phase, not carried over from a prior one" property (#708). It used to do that by sleeping real wall time (150ms before the first sync, 150ms between the two syncs) and then asserting the phase's own elapsed stayed under a soft < 100ms bound. That assertion was really a bet on how long the download loop's own real work took on this run — not a property of the code — and under remote-builder contention that bet lost with nothing wrong in the code under test.

This is the same shape already fixed once in this repo for #760 (commit 245de07): inject/pin the clock instead of widening the tolerance or adding a retry.

fetch_and_reassemble now takes an explicit DownloadClock — a private newtype over Option<Instant>, with a WALL constant for production and a frozen_at constructor gated #[cfg(test)] — and reads clock.now() / clock.now().saturating_duration_since(start) instead of Instant::now() / .elapsed(). sync_set and sync_changed_manifest thread the same clock down through new sync_set_with_clock / sync_changed_manifest_with_clock / fetch_and_reassemble_with_clock internals. The original function names (sync_set, sync_changed_manifest, fetch_and_reassemble... — the last of these had no other caller so it was folded away rather than kept as a dead wrapper) stay as the production entry points and always pass DownloadClock::WALL, whose now() is exactly Instant::now(). Production behaviour is byte-for-byte unchanged, and every other test call site of sync_set (~20 of them) is untouched.

Mechanism (why this removes the wall-clock dependency structurally, not just usually)

The rewritten test never sleeps. It pins the first sync's clock 10s ahead of an arbitrary reference Instant, simulating "slow prior work" with no real delay, and pins the second sync's clock a further 5s ahead, simulating "more time passing between syncs." Because a pinned clock's now() always returns the exact same instant on every call, a correctly phase-scoped start = clock.now() makes every elapsed reported during that call exactly Duration::ZERO — deterministically, independent of how fast or slow the machine actually executes the loop body between the two clock.now() reads. The assertions changed from assert!(elapsed < Duration::from_millis(100)) to assert_eq!(elapsed, Duration::ZERO), which is only possible because the test now controls "now" instead of reading it from the OS.

Confirmed this structurally, not just by re-running: the fixed test module (6 tests, including this one) reports finished in 0.00s — there is no wall-clock delay left anywhere in the test to measure.

Mutation-check, both directions

Fix in place → GREEN:

running 6 tests
test asset_sync::download_rate_tests::rate_is_bytes_over_elapsed_seconds ... ok
test asset_sync::download_rate_tests::near_zero_elapsed_below_threshold_is_guarded ... ok
test asset_sync::download_rate_tests::zero_elapsed_is_guarded_not_divided ... ok
test asset_sync::download_rate_tests::elapsed_at_threshold_yields_a_real_rate ... ok
test asset_sync::download_rate_tests::all_chunks_already_cached_never_emits_a_downloading_tick ... ok
test asset_sync::download_rate_tests::elapsed_does_not_carry_over_from_a_slow_prior_download ... ok

test result: ok. 6 passed; 0 failed; 0 ignored; 0 measured; 205 filtered out; finished in 0.00s

Bug reintroduced → RED: temporarily wrapped start in a OnceLock (static ...: OnceLock<Instant>; let start = *ONCE.get_or_init(|| clock.now());), reproducing the historical leaked/global-clock bug this test is named for — a start that isn't captured fresh on every call.

test asset_sync::download_rate_tests::elapsed_does_not_carry_over_from_a_slow_prior_download ... FAILED

thread '...' panicked at src/asset_sync.rs:1364:9:
assertion `left == right` failed: second sync's elapsed must be EXACTLY zero — a nonzero value
would mean `start` was NOT captured fresh from THIS call's own clock reading (the #708
carry-over bug this test is named for), got 5s
  left: 5s
 right: 0ns

test result: FAILED. 5 passed; 1 failed; 0 ignored; 0 measured; 205 filtered out; finished in 0.00s

The failing value (5s) is exactly the simulated gap between the two pinned clocks, reproduced identically every run — a deterministic signal from the injected clock, not noise from machine load. The mutation was then reverted; git diff against the committed version is empty at that hunk.

Full workspace test run

Finished line:

    Finished `test` profile [unoptimized + debuginfo] target(s) in 1m 59s

• running [0-9]+ tests? headers: 53
• test result: lines: 53 (equal — the lost-binary check)
• Sums across all test result: lines: 1755 passed + 0 failed + 47 ignored + 0 filtered = 1802, which equals the header-implied total
• All-zero targets (0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out): 15
• FAILED lines: 0
• failures: blocks: 0
• Non-canonical test result: lines (not starting test result: ok.): 0
• Compiler warnings: 0

This matches origin/main at f71ef16 exactly (1755/0/47/0 = 1802, 53 = 53, 15 all-zero targets) — this PR adds no test, removes none, and changes no other test's pass/fail/ignore count. The only test whose body changed is the one this issue is about.

NOT verified

• No live/E2E run. This is a pure test-fixture change to a private, in-process function (fetch_and_reassemble/sync_set/sync_changed_manifest are all private to src/asset_sync.rs); there is no new externally-observable behavior to run a live client against. Per the agent-fleet skill's exception for dead-storage-shaped internals, the mutation-checked suite is the ceiling here, not live play.
• Did not empirically reproduce the original flake under real contention (i.e. did not deliberately load the remote builder and re-run the old test to watch it fail again). The fix instead removes the wall-clock read entirely, which is a stronger claim than "flakes less often" — but I have not measured the old test's flake rate, only reasoned from the code (std::thread::sleep + a < 100ms bound racing real Instant::now() reads) and the issue's own report of one observed flake.
• Did not run the full workspace suite a second time to check run-to-run stability of anything else — one full green run, per the verification-hierarchy guidance that a single green run should not be over-claimed; the figures above are from that one run, and the mutation-check (not repetition) is what backs the "flake is gone" claim.
• No independent reviewer yet — this PR is pending the mandatory adversarial review gate before merge.

[djhenry]

Independent review — PR #808 (issue #796)

Reviewed the MERGE of fix-796-wallclock-flake (42ebd0f) into origin/main@b45e62d (post #804/#806), in an isolated worktree. Merge was clean — only crates/eqoxide-renderer/src/models.rs (unrelated, from #804/#806) plus this PR's own src/asset_sync.rs changed; no semantic overlap.

Verdict: APPROVE WITH NON-BLOCKING FINDINGS

The flake fix itself is sound: the rewritten test faithfully reproduces the #708/#796 property with a pinned clock, is mutation-discriminating in both directions (verified independently, not just re-derived from the PR body), and production behaviour really is unchanged. One real coverage gap found — see Finding 1 — but it does not reflect a defect in what's shipping today, so it's non-blocking.

Finding 1 (most significant, NON-BLOCKING): the new test no longer exercises sync_set's hardcoded clock choice

The old test called the real production entry point sync_set (there was no _with_clock split pre-PR). The new test calls sync_set_with_clock directly (src/asset_sync.rs:1325, :1356), bypassing sync_set (:444-451), whose entire distinguishing behaviour post-refactor is the one-line sync_set_with_clock(t, set, cache, progress, DownloadClock::WALL).

I checked: no test anywhere in the file inspects elapsed from a SyncProgress::Downloading tick produced through sync_set (as opposed to sync_set_with_clock) — the other ~18 test call sites of sync_set only check done/total/error outcomes, never elapsed.

I confirmed this is a real, exploitable gap by mutation, not just reasoning: temporarily changed line 450 to

sync_set_with_clock(t, set, cache, progress, DownloadClock(Some(std::time::Instant::now())))

— i.e., production sync_set now freezes the clock once at entry instead of passing WALL, so every real download's reported elapsed would always come back Duration::ZERO (and download_rate_bytes_per_sec would report no rate at all, or a wrong one, for every real sync). Ran cargo test -p eqoxide --lib (210 tests) with this mutation in place: 210 passed; 0 failed — including elapsed_does_not_carry_over_from_a_slow_prior_download ... ok. Full log tail:

test result: ok. 210 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 4.67s

Reverted; md5sum confirmed the file is back to the committed version before continuing.

So the doc comment's claim ("every production call site... passes DownloadClock::WALL... production behaviour is byte-for-byte unchanged") is true today (I read all three threaded functions and their call graph — sync_set is the only caller of sync_set_with_clock, and it's the only place DownloadClock is constructed outside #[cfg(test)]), but it is now an untested invariant: a future edit to that one line would compile, and the entire suite — 1757+ tests — would stay green while the download-rate observable silently went wrong for real users/agents (this is exactly the class of defect the project's own agent-honesty invariant ranks highest: a plausible, well-formed, false answer with no test or crash to catch it).

Suggested fix (cheap, and non-flaky unlike the deleted test): add one test that calls sync_set (not _with_clock) against a FakeTransport whose get_chunk sleeps briefly, and assert elapsed >= that_sleep_duration — a lower-bound assertion is safe under builder contention (load can only inflate elapsed, never shrink it below a real sleep), unlike the deleted test's upper-bound assertion which is exactly what flaked.

Hunt-list results

1. Does the test still test the same thing? Compared old (f71ef16:src/asset_sync.rs:1224-1288) vs new (:1283-1365) line-by-line. Same two-phase structure, same fake transports, same "phase-scoped, not carried over" property. Two real deltas, both improvements:

   • Old asserted soft bounds (< 100ms) against real sleeps; new asserts == Duration::ZERO against a pinned clock — strictly stronger, not weaker (this is the one place a "flaky test fixed by asserting less" concern would show up, and it's the opposite here).
   • Old initialized the elapsed accumulators to Duration::ZERO, so a missed Downloading tick would silently satisfy 0 < 100ms — a false pass hiding a real bug. New initializes to Duration::MAX, so a missed tick now fails loudly. Genuine improvement, called out correctly in the PR body.
   • The one loss (Finding 1): calls sync_set_with_clock instead of sync_set, so it no longer exercises the production wrapper's hardcoded clock choice.

2. Mutation fidelity — re-ran independently, plus two new mutations the author didn't try:

   • Reproduced the author's OnceLock-hoisted-start mutation shape myself via a different, equally-faithful mutation: made fetch_and_reassemble_with_clock ignore its clock parameter and read std::time::Instant::now() directly (let _ = clock; let start = std::time::Instant::now(); … elapsed: std::time::Instant::now().saturating_duration_since(start)). Result: RED, exactly as expected —

     assertion `left == right` failed: first sync's own elapsed must be EXACTLY zero...
       left: 48.2µs
      right: 0ns
     test result: FAILED. 5 passed; 1 failed; 0 ignored; 0 measured; 205 filtered out
     

     Reverted; md5 confirmed restored.
   • Made the production wrapper pass a frozen clock instead of WALL — GREEN, see Finding 1 above.
   • Did not find any other GREEN-that-should-be-RED mutation.

3. Production behaviour unchanged? Traced all three threaded functions: sync_set → sync_set_with_clock(..., DownloadClock::WALL) (:450) is the only production call; both its internal branches (Unchanged-then-repair at :503, and Changed at :524) forward the same clock they were given, never construct a new one; sync_changed_manifest_with_clock (:619) likewise only forwards clock into fetch_and_reassemble_with_clock (:634). No branch, including the error/repair path, passes anything other than the clock it received. WALL.now() (DownloadClock(None).now() → self.0.unwrap_or_else(Instant::now)) is exactly Instant::now(). Confirmed correct.

4. #[cfg(test)] scope. DownloadClock and frozen_at are both private to src/asset_sync.rs (not pub), so no downstream crate could reach them regardless of the gate — the "per-crate not per-build" trap doesn't apply here; frozen_at's only two call sites are inside mod download_rate_tests in the same file. Note (cosmetic, not a defect): the tuple field DownloadClock(Option<Instant>) itself is not #[cfg(test)]-gated, only the frozen_at associated fn is — so any code in this module (including future production code) could construct DownloadClock(Some(x)) directly without going through the gated constructor. This is what my Finding-1 mutation used. Not a blocking issue since the module is small and reviewed, but worth knowing the gate is a convenience, not a compile-time wall.

5. finished in 0.00s as proof. Read the full new test body (:1283-1365): no sleep, no SystemTime. One std::time::Instant::now() call remains (:1321, let reference = std::time::Instant::now();), but it's used only once to seed an arbitrary base instant to add simulated offsets to (reference + Duration::from_secs(10)) — never read again as "now" during the timed operations, so it does not reintroduce a real-time race. Structural read confirms the claim; the timing figure is corroborating, not load-bearing, as the PR body itself says.

6. Blast radius. Counted independently: 18 test call sites of sync_set( (not ~20 as stated — close enough given the "~"), plus the one production call site inside sync_set_observed. Diffed f71ef16..42ebd0f on src/asset_sync.rs: none of those 18 call sites appear in the diff — confirmed unchanged.

Independently re-derived figures (full workspace, this merge)

Ran rbuild <worktree> test --workspace --locked --no-fail-fast against the PR branch merged with origin/main@b45e62d, stdout+stderr to a file prefixed uniquely to this review (not the shared scratchpad log another agent could collide with).

• Finished line: Finished `test` profile [unoptimized + debuginfo] target(s) in 10m 39s
• running [0-9]+ tests? headers: 53
• test result: lines: 53 (equal — lost-binary check passes)
• Non-canonical test result: lines: 0
• All-zero targets (field-anchored: 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out): 15
• FAILED lines: 0; failures: blocks: 0
• Sum: 1757 passed + 0 failed + 47 ignored + 0 filtered = 1804 = header-implied total

This is an exact match to the given current-main baseline (b45e62d: 1757/0/47/0=1804, 53=53, 15 all-zero) — expected, since this PR rewrites one test's body without adding or removing a test. The PR body's own figures (1755/1802 against the older f71ef16 base) are consistent: the +2 passed between f71ef16 and b45e62d is accounted for by #804/#806 landing in between, not by this PR.

What I did NOT check

• No live/E2E run — agreed with the PR's own claim that there's nothing externally observable to run against (fetch_and_reassemble/sync_set/sync_changed_manifest are all private to this file; the public surface, sync_set_observed, is untouched).
• Did not load-test the OLD test under real remote-builder contention to watch it flake (not practical to reproduce after-the-fact; took the issue's one observed flake plus the mechanism analysis as sufficient).
• Did not audit every one of the ~205 other tests in the file beyond confirming the diff didn't touch their call sites and the full-suite run was green.

[djhenry]

Round 2: made the DownloadClock "cannot reach a real sync" claim actually true

The independent reviewer's non-blocking finding was correct: the type doc at
src/asset_sync.rs:208-210 claimed a pinned clock "cannot" reach a real sync because
frozen_at is #[cfg(test)]-gated and the field is "private to this module" — but the
module in question was all ~1300 lines of asset_sync.rs, so every production function in
the file (including sync_set) could already write DownloadClock(Some(t)) directly,
skipping frozen_at entirely. The reviewer proved this isn't hypothetical: it pointed
sync_set's hardcoded DownloadClock::WALL at a pinned clock and the whole --lib suite
(210 tests) still passed — the exact silent-wrong-elapsed outcome the type exists to
prevent.

The fix

Moved DownloadClock into its own private download_clock submodule inside
src/asset_sync.rs, with pub(super) on WALL, frozen_at (still #[cfg(test)]), and
now(), and the inner Option<Instant> field left private (to download_clock, not to
asset_sync). now()'s behavior is byte-for-byte unchanged.

Outside download_clock, the only ways to name a DownloadClock are WALL and the
test-gated frozen_at — so in a non-test build WALL is the only constructible value.
This is verification tier 1 (make the bad state unrepresentable), strictly stronger than
adding another test: a wiring that can't be expressed can't be wrong.

Proof — reproduced the reviewer's exact mutation

Temporarily changed sync_set's DownloadClock::WALL to a direct construction:

sync_set_with_clock(t, set, cache, progress, DownloadClock(Some(std::time::Instant::now())))

Build result (compiler error quoted verbatim):

error[E0423]: cannot initialize a tuple struct which contains private fields
   --> src/asset_sync.rs:465:50
    |
465 |     sync_set_with_clock(t, set, cache, progress, DownloadClock(Some(std::time::Instant::now())))
    |                                                  ^^^^^^^^^^^^^
    |
note: constructor is not visible here due to private fields
   --> src/asset_sync.rs:225:37
    |
225 |     pub(super) struct DownloadClock(Option<std::time::Instant>);
    |                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^ private field
help: consider making the field publicly accessible
    |
225 |     pub(super) struct DownloadClock(pub Option<std::time::Instant>);
    |                                     +++

(The doc originally guessed E0603-family; the actual observed error for direct tuple-struct
construction is E0423 — same privacy-error family, now cited exactly as measured.)

Restored the file afterward (verified via md5sum against a pre-mutation copy, then
touched so the remote builder wouldn't reuse a stale rlib). Confirmed the existing
elapsed_does_not_carry_over_from_a_slow_prior_download test — the one that exercises
frozen_at — still passes unmodified.

Full-suite figures (rbuild ... test --workspace --locked --no-fail-fast)

• Finished: Finished `test` profile [unoptimized + debuginfo] target(s) in 5m 11s
• Headers (running [0-9]+ tests?) vs test result: lines: 53 vs 53 (equal — no lost
  test binary)
• Corrupt (non-canonical) test result: lines: 0 — all 53 match the full canonical
  pattern
• All-zero-count targets (0 passed; 0 failed; 0 ignored; ...), field-anchored: 15
• passed + failed + ignored + filtered = 1755 + 0 + 47 + 0 = 1802, which equals the
  sum of the running N tests header counts (1802) — internally consistent

Vs. the reviewer's baseline (this branch merged with origin/main@b45e62d: 1757 passed
/ 0 failed / 47 ignored / 0 filtered = 1804, 53/53 headers/results, 15 all-zero): headers,
results, failed, ignored, filtered, and the all-zero count are all identical. Passed is
1755, not 1757 — investigated: origin/main has since advanced 2 commits past
b45e62d (the reviewer's merge base), and b45e62d itself (test(#779): grade the loader's y_bottom/y_extent reduction against #768's over-lift (#806)) added 2 new tests
(a property test + a pinned example test) that landed in origin/main but are not yet
merged into this branch. Running the suite on this branch alone (not merged with main)
therefore reports 2 fewer passed tests than the reviewer's merged measurement — expected
given the branch composition difference, not a regression introduced by this change. This
round adds no tests of its own.

Not checked: no live/E2E run — this is a pure compile-time visibility change with no
runtime-observable behavior (now() is byte-identical), consistent with the "nothing to
E2E" exception in the dev-process skill for dead-storage-shaped internals.

[djhenry]

Independent re-review — PR #808, round 2 (151f8df)

Reviewed the MERGE of fix-796-wallclock-flake@151f8df into current origin/main@7159140 (post #802), in a fresh worktree/branch. Clean merge — only the unrelated #802 water-nav files plus this PR's own src/asset_sync.rs touched.

Verdict: APPROVED

Round 1's non-blocking finding is now closed by a real type-system change, not just a doc rewrite. I reproduced the privacy proof myself (did not take the author's report on faith) and additionally probed three bypass routes the author didn't demonstrate. All hold.

1. Privacy proof — reproduced independently

Round-2 diff (42ebd0f..151f8df, single hunk, 33+/16- lines, confined to src/asset_sync.rs) moves DownloadClock into a private download_clock submodule; WALL/frozen_at/now() become pub(super); the Option<Instant> field stays private to the submodule, not to all of asset_sync.rs. now()'s body is byte-identical: self.0.unwrap_or_else(std::time::Instant::now), confirmed via git diff.

I copied src/asset_sync.rs aside (cp -p to preserve mtime), md5sum-verified, then ran each probe as a separate rbuild <worktree> check -p eqoxide --lib (compile-fail probes can't be batched — typeck stops before the privacy pass reaches a second error in the same run), restoring + md5sum-verifying + touching between each:

Probe A — the exact mutation from round 1 (sync_set's DownloadClock::WALL → DownloadClock(Some(Instant::now()))):

error[E0423]: cannot initialize a tuple struct which contains private fields
   --> src/asset_sync.rs:467:50
    |
467 |     sync_set_with_clock(t, set, cache, progress, DownloadClock(Some(std::time::Instant::now())))
    |                                                  ^^^^^^^^^^^^^
    |
note: constructor is not visible here due to private fields
   --> src/asset_sync.rs:227:37
    |
227 |     pub(super) struct DownloadClock(Option<std::time::Instant>);
    |                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^ private field

Verbatim match to the author's report (E0423, not the doc's original E0603 guess).

Probe B — the cfg-leak route (production sync_set calling DownloadClock::frozen_at(Instant::now()), checked with a plain non-test cargo check --lib, no --tests):

error[E0599]: no associated function or constant named `frozen_at` found for struct `DownloadClock` in the current scope
   --> src/asset_sync.rs:467:65

frozen_at doesn't merely reject the call on visibility grounds — it does not exist in a non-test build. This closes "does #[cfg(test)] actually strip it" independently of the field-privacy check.

Probe C — the pub(super)/pub(crate)-leak route (does anything OUTSIDE asset_sync.rs — e.g. app.rs — see the type at all?): added a throwaway const _PROBE_C: fn() = || { let _c: crate::asset_sync::DownloadClock = crate::asset_sync::DownloadClock::WALL; }; to the top of src/app.rs, plain check --lib:

error[E0603]: struct import `DownloadClock` is private
   --> src/app.rs:5:32
error[E0603]: struct import `DownloadClock` is private
   --> src/app.rs:5:67
error[E0624]: associated constant `WALL` is private
   --> src/app.rs:5:82

Even pub(super) doesn't leak past asset_sync.rs: the use download_clock::DownloadClock; re-export at line 243 is a private use, so the type isn't nameable from any sibling module at all — pub(super) was the right (not over-broad) visibility to use.

Other named routes, checked structurally (no compile needed):

• Clone/Copy off a test value — #[derive(Clone, Copy)] only duplicates an existing DownloadClock value; it can't synthesize a new Some(_) from nothing. Since Probes A/B establish WALL is the only value any non-test code can ever hold, Clone/Copy of that just yields another WALL. Not a bypass.
• Default — no #[derive(Default)] and no manual impl Default anywhere in the submodule (read the full 20-line submodule body directly). Not present.
• Any other constructor — read the whole mod download_clock { … } block: exactly two items besides now() are declared — WALL and frozen_at. Nothing else exists to name.
• Cross-file leak via any other path — grep -rn "DownloadClock" src/ crates/ returns hits only in src/asset_sync.rs (confirmed no other file references it, so Probe C's negative generalizes to the whole crate, not just app.rs).

All four restores verified by md5sum match to the pre-mutation copy before moving to the next probe; final git status --porcelain clean in both the worktree and (untouched throughout) the shared main checkout.

2. Doc claims vs. what's verified — clause by clause

Claim (doc, src/asset_sync.rs:199-221)	Discharged by
"Production always uses DownloadClock::WALL"	Only one production call site (:467), sole caller of sync_set_with_clock; both downstream _with_clock fns only forward the clock they're given, never construct a new one (re-verified this round, unchanged from round 1)
"WALL.now() is Instant::now()"	git diff — now() body untouched
"nothing actually stopped sync_set... from writing DownloadClock(Some(t)) directly" (round-1 state)	This is a claim about the OLD code, matches my round-1 mutation exactly
"outside this submodule the only ways to name a DownloadClock are WALL and... frozen_at"	Probes A, B, C + structural read of the submodule body — no fourth route found
"in a non-test build WALL is the only constructible value"	Probe B (frozen_at doesn't exist without cfg(test))
"the compiler rejects the mutation above... E0423"	Probe A, verbatim

No unverified "cannot"/"always"/"only" remains in the doc.

3. now() byte-identical — checked, not trusted

Confirmed directly in the 42ebd0f..151f8df diff (single hunk): the only change to now() is prepending pub(super) to its signature; the body self.0.unwrap_or_else(std::time::Instant::now) is untouched.

4. Figures reconciliation — verified the attribution, not just accepted it

Checked git diff b45e62d~1..b45e62d (the round-1 baseline's tip): exactly 2 added #[test] fns (both plain, no #[should_panic] to hide from a name-grep), 0 removed — matches the round-2 author's "1755 on the bare branch, 1757 on my b45e62d-merged baseline" attribution exactly.

Then went further, per the instruction to re-measure against current origin/main (now 7159140, #802 merged): git diff b45e62d..origin/main shows 11 added #[test] fns, 0 #[should_panic], 0 removed, confined to region_map.rs/collision.rs/water_grid.rs/walker_sim.rs (all pre-existing files, no new test binary/target created).

Base measured on this round: fix-796-wallclock-flake@151f8df merged with origin/main@7159140 (merge commit built and run in full).

Full-workspace test --workspace --locked --no-fail-fast, ~27m22s, Finished present, exit 0:

Figure	Round-1 baseline (b45e62d)	This round (151f8df+7159140)	Delta
passed	1757	1768	+11
failed	0	0	0
ignored	47	47	0
filtered	0	0	0
total	1804	1815	+11
running N tests headers	53	53	0
test result: lines	53	53	0
non-canonical result lines	0	0	0
all-zero-target (0/0/0/0/0) results	15	15	0
FAILED / ^error/panicked/Killed hits	0	0	0

The +11 passed is an exact match to the b45e62d..origin/main diff in §4 (11 added #[test] fns, 0 removed, 0 #[should_panic]) — nothing attributable to this PR's own diff, which adds/removes zero tests. Headers=results=53 both rounds confirms no new test target was created by either #802 or this PR. All-zero-target count unchanged at 15. Zero failures, zero error/panic/kill signals in either stream (stdout and stderr captured separately this round per the splice-artifact concern — no discrepancy between them).

5. Live-run exception — earned this round

Diffed 42ebd0f..151f8df in full: one hunk, confined to visibility modifiers (pub(super)) and a module wrapper, plus doc comments. No function body changed except now()'s signature (not its logic). Nothing runtime-observable moved — this is a pure compile-time encapsulation change on top of a round-1 change that already had no externally-reachable production entry point to E2E. The "nothing to E2E" exception is genuinely earned, not merely re-asserted.

What I did NOT check

• Did not re-run the round-1 mutation checks (leaked-start-via-OnceLock RED, frozen-clock-via-sync_set GREEN) — round 2 doesn't touch fetch_and_reassemble_with_clock's body or the test, only DownloadClock's visibility, so those results stand unchanged from round 1.
• No live/E2E run (see §5 for why).