fix(#751): seed shadow_caster_selection's corpus per-scene, not from one shared stream - #825
Conversation
…one shared stream extracted_planner_matches_the_pre_740_loops_over_a_random_corpus drove its whole 400-scene corpus off one shared Rng advanced continuously across scenes, so every scene's draws depended on wherever the previous scene's draws left the stream — an edit to any earlier fixture decision silently shifted every later scene. This already dropped mutant M14 (sort_by -> sort_unstable_by) from killed to surviving once, while fixing #747's B1 (see eq-fixture-edits-are-not-local). Pull scene construction into build_scene(scene: usize), seeded from Rng(0x5EED_740 ^ scene as u64) alone and never threaded across calls, so cross-scene coupling is structurally impossible rather than merely absent-by-inspection. Add scene_fixtures_are_independent_of_call_order_and_neighboring_scenes, a property test that builds the same scenes under different call orders/neighbors and requires byte-identical output. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HQVEpaaKeXsZcW9VT2roeV
|
CHANGES REQUESTED Independent adversarial review of The code change is right and does what the issue asks — I confirmed that by measurement, below. What I verified as CORRECT (so it is not re-litigated)Every re-measured coverage number in the doc comment is exact. Not taken on trust — I Matches the doc's Workspace figures reproduce your report exactly (
Reconciled by name: Your MUT-1 reproduces exactly. With the The fix works at the level the issue cares about. Adding one extra draw to scene 7 only: M14 survives the reshuffle (see F1): under F1 — BLOCKING. The PR inverts what issue #751 says about scope.PR body, under "NOT verified, and not claimed":
Issue #751 says the opposite. Its closing sentence:
The sweep was scoped out of #747 and explicitly scoped into #751. The PR reassigns the Materially, this is not just bookkeeping. The corpus really did move: cull 317→315, clip guard Fix: either run the sweep, or state plainly that you are deviating from the issue's stated scope and F2 — BLOCKING. "Structurally impossible" is refuted by a mutation that SURVIVED.
and The stated mechanism does not entail the stated conclusion. I wrote MUT-6, which reintroduces let mut rng = if scene == 0 {
Rng(0x5EED_740)
} else {
let (prev_cands, _, _, _) = build_scene(scene - 1);
Rng(0x5EED_740 ^ scene as u64 ^ prev_cands.len() as u64)
};Under MUT-6, editing what scene So: the no-cross-call- F3 — BLOCKING.
|
| draw offset | 0 | 1 | 2 | 3 | 4 | 5 | 15 |
|---|---|---|---|---|---|---|---|
| chi² | 0.30 | 1.20 | 7.85 | 2.05 | 399.90 | 2.20 | 47.30 |
Draw offset 4 is not an arbitrary offset — it is rng.range(10), the first candidate's model-kind
selector, in every scene. Its histogram across 400 scenes is [39, 1, 0, 0, 0, 18, 81, 111, 78, 72]. Confirmed on the real code, not the replica:
MEASURED cand0 Absent=38 Static=1 Skinned=358 (of which no-anim 0)
So in this corpus, candidate index 0 is Cand::statik in 1 of 397 non-empty scenes and
Cand::skinned (skinned, no anim state) in 0 — against ~79 and ~40 expected. Under the old
single shared stream that position was properly uniform. Second symptom: the dense-scene population
104 + rng.range(60) leaves 19 consecutive values (10..28) unrealised across its 80 scenes,
chi² 140.5 on 59 df; against 500 control multinomials the observed 29 empty buckets and 19-long
empty run both have P < 1/500 (control medians: 16 empty, longest run 3).
This is not XOR-specific — it is inherent to seeding this LCG with small near-consecutive values.
Worst chi² over draw offsets 0..40:
| seeding | worst chi² | candidate-0 kinds (Absent / Static / skinned-no-anim / playing) |
|---|---|---|
0x5EED_740 ^ scene (this PR) |
399.90 | 39 / 1 / 0 / 360 |
0x5EED_740 + scene |
574.15 | 0 / 0 / 5 / 395 |
scene alone |
570.20 | 1 / 148 / 128 / 123 |
splitmix64(0x5EED_740 + scene) |
20.65 | 37 / 82 / 46 / 235 |
The fix is a few lines — run the scene index through a mixing function before it becomes an LCG
seed. I checked that it does not cost coverage: with splitmix64, truncation 80, cull 313, clip guard 386, ties 365, player 95/49/125/131, nearby 330/363/389, 80/80 mixed, 0 all-skinned — every
floor still comfortably clear.
Note the shape of this: #751 is a corpus-quality issue, and as it stands the PR trades one corpus
defect (cross-scene coupling) for another (cross-scene correlation at fixed draw offsets, killing
a model kind at a structural position). Worth fixing before it becomes the next "the corpus happened
to cover it" story.
F4 — minor, informational. Nothing pins the seed formula.
Rng(scene as u64) and Rng(0x5EED_740.wrapping_add(scene as u64)) both clear every floor and pass
the property test, so neither the constant nor the operator is graded by anything. Not a defect —
the constant is arbitrary — but if F3 is fixed by adding a mixing step, be aware nothing will detect
that step being deleted later.
Mutations run
| # | mutation | where | result |
|---|---|---|---|
| MUT-1 | shared stream carried across build_scene calls (static Mutex<u64>) — your reported one |
real build | RED 18 passed / 1 failed, property test only, correct message at :1012 |
| MUT-6 | invented: scene k seeded from scene k-1's recomputed fixture — the issue's coupling with no cross-call Rng |
real build | SURVIVED 19/19 → F2 |
| MUT-A | identical seed Rng(0x5EED_740) for every scene |
replica | RED (player/None = 0, floor is ≥ 20) |
| MUT-B | Rng(scene as u64) |
replica | SURVIVED (all floors clear) → F4 |
| MUT-C | Rng(0x5EED_740.wrapping_add(scene)) |
replica | SURVIVED (all floors clear) → F4 |
| MUT-D | invented: one extra draw in scene 7 only | replica | locality confirmed — 1 scene changes post-#751, 393 pre-#751 |
| M14 | planner sort_by → sort_unstable_by |
replica | KILLED post-#751 (308/400 scenes diverge) |
Replica fidelity control: the standalone replica reproduces all twelve published pre-#751 numbers
and all nine post-#751 numbers exactly, and the post-#751 set was independently re-confirmed against
the real binary.
NOT verified
- M6, MUT-A, MUT-B, MUT-C, M10, M11 from test(#740): device-free coverage for shadow caster selection — 14/16 mutants killed, and the premise that no refactor was needed does not hold #747's table — not re-run by anyone. Only M14, and via
the replica rather than the real planner. - MUT-A/B/C/D and M14 were not run against the real test binary; replica only.
- The
41cca4ebaseline workspace figures were taken as given, not measured by me. - I did not check whether any test outside
shadow_caster_selection.rsdepended on the old draw
sequence — the file is self-contained (build_sceneis private to the target), and the full
workspace run is green, but I did not audit for it directly. - Nothing in fix(#803): make "no exits" and "could not read the exits" different types #821's or test(#805): check the corpus's zone accounting per zone, not only in its last statement #824's files was touched or examined.
…bility claim, run the deferred mutation sweep Round 2 per independent review of PR #825: - F3: 0x5EED_740 ^ scene had a measured LCG affine-correlation defect (chi²=399.90 at the model-kind-selector draw offset). Scramble the seed through splitmix64 before constructing the Rng (the LCG itself is unchanged); worst-offset chi² drops to ~20.9. Every downstream coverage number in the doc comments was re-measured from scratch against the new stream, not carried over. - F2: retract two "structurally impossible" cross-scene-coupling claims, falsified by an independent reviewer's MUT-6 (scene k's fixture folds in a freshly recomputed build_scene(k-1) -- satisfies "no Rng crosses a call boundary" but still couples k to k-1's generation logic; measured 19/19 green = SURVIVED). Replace with precisely scoped language (order/neighbor independence only, not edit-locality) and add a source-text pin test, build_scenes_dependency_is_its_own_index_and_nothing_else, that kills the literal MUT-6 shape and discloses what it does not catch. - F1: the mutation sweep is #751's own tracked deliverable, not "scoped out on purpose" as round 1's PR body incorrectly claimed (that claim cited #751 as authority for the opposite of what #751's text says). Run it from scratch on the real build: M6/MUT-A/MUT-B/MUT-C/M14 KILLED, M10/M11 SURVIVED (documented dead code) -- matches #747's original results exactly, nothing it killed now survives. File goes from 19 to 20 tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
CHANGES REQUESTED — round 2 (final pass), commit All three round-1 findings are properly closed. One blocking finding remains, in the new 1. What is now verified (no findings)F1 — the mutation sweep is real and applied at the production call site. The seven-row table F3 — the re-measured coverage numbers are exact on a real build, not from a replica: matching every figure in the PR body. The round-1 correlation defect is genuinely gone: Workspace, measured on this branch merged with the current base
Δ against current 2. BLOCKING — B1: the pin's brace scan can be truncated by a brace in a comment, and then it passes silently
The region isolation walks bytes and stops at the first point where Measured (R2-M3). This exact edit, with the literal MUT-6 coupling: fn build_scene(scene: usize) -> (Vec<Cand>, Option<Cand>, [f32; 3], [f32; 3]) {
// Each arm of the `match rng.range(10)` below closes with a } on its own line.
let prior = if scene > 0 { build_scene(scene - 1).0.len() as u64 } else { 0 };
let mut rng = Rng(splitmix64(0x5EED_740 ^ scene as u64 ^ prior));→ Why this is not covered by the disclosure: the rustdoc excuses couplings reached "without the Why it is not purely adversarial: the truncating shape is ordinary Rust — Validated fix (I ran both halves): let body = &after_sig[open + 1..close];
assert!(
body.trim_end().ends_with("(cands, player, light, player_pos)"),
"the brace scan did not reach the end of build_scene, so this guard is scanning \
only a truncated prefix and is silently blind"
);
Any equivalent reach assertion is fine (a body-length floor, or asserting the region ends at the real I ran the scanner rather than re-deriving it, with a reach control — the violating shape placed at
The positive control also confirms the author's own reported result exactly, and confirms my round-1 3. Non-blockingN1 — the evasion disclosure is accurate, but the test name overstates what is enforced. I routed N2 — "same technique and caveat as N3 — the PR body's workspace figure 4. Mutations run this round
All results are from real 5. What I did not verify
Merge recommendation: fix B1 (4 lines, remedy validated above) and this is good to merge. N1–N3 |
Round 2 independent review (B1, blocking): the source-text pin that checks build_scene's own body for a nested call to itself used brace-depth counting with no lexer, so a `}` inside a comment, string, or char literal earlier in the body could close the scan early. With MUT-6's recursive coupling plus one such stray-`}` comment, the scan truncated to ~58 bytes, silently missing the recursive call sitting just outside the scanned window (20/0, SURVIVED) — a failure mode outside the pin's own disclosed limitations, which only excused an *absent* token via indirection, not a *present* token outside a truncated scan. Fix: assert the scan actually reached build_scene's real tail (its literal final expression) before trusting the truncated body as complete. A brace that closes the scan early now leaves that tail outside `body`, so the truncation is caught by the tail's absence and fails loudly and honestly, instead of the depth counter's say-so being trusted silently. Kept brace-depth counting rather than switching to the sibling pin's `\npub fn`-split technique: build_scene is private and immediately followed by another function's doc comment that quotes `build_scene(scene)` in prose, so the equivalent `\nfn` split swallows that prose and false-fails on the unmutated file (confirmed by trying it first). Measured both directions on the real build: unmutated file 20/0 (no observable change), MUT-6 + stray-`}` comment 19/1 (terminator assert fires with an honest message instead of a silent all-clear). Also (non-blocking nits from the same review): - Renamed the pin from build_scenes_dependency_is_its_own_index_and_nothing_else to build_scenes_source_text_contains_no_literal_call_to_itself, and narrowed its doc comment to match its true, narrower proven property: no literal, whitespace-exact `build_scene(` substring in build_scene's own scanned source text — not "no dependency on another scene of any kind." - Corrected a false "same technique" claim against encode_shadow_pass_calls_the_planner_this_file_grades, which splits on `\npub fn` and has no brace-balance dependency; explained why that technique isn't viable here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HQVEpaaKeXsZcW9VT2roeV
…its last statement Closes #805. `faithful_walker_drift_corpus` checked its zone accounting in the corpus's **last statement**, so a zone that never terminates (#763, blackburrow) skipped the completeness check entirely: the run died inside the loop and the guard that was supposed to notice the missing zone never executed. A guard placed after the loop cannot grade a run that does not reach the end of the loop. The check now runs **per zone**, at the point each zone is closed out, so a zone is accounted for before the corpus can die on the next one. The terminal check stays as a backstop rather than being replaced — it is the only thing that can see a zone that was never opened at all. An aborted run now prints an `[ABORTED while opening {zone}]` block instead of falling silent. It is tagged PARTIAL, says in its own words that it is not a corpus score, names the zone it died on, and each rollup's `Display` prints INCOMPLETE with its hole. **On an abandoned run no rollup is published at all** — both publication points are after the loop — which is what makes `Closes #805` the right verb rather than a partial fix. The author's own round-1 claim that the water arm "was walked" was **retracted by the author** before the reviewer raised it, falsified by a real run: `install()` returns `Err` iff the zone is `Unmeasured`, and that arm `continue`s before any journey pair is sampled (0.48s, a row of dashes). The reviewer re-derived the retraction independently rather than accepting it — `walker_sim.rs:630-634` is the sole path into `unmeasured`, and the other `add` site is reachable only with a `Measured` zone. Nine mutations, reviewer-run, every one reverted and the file md5-verified afterwards (`cce0cab1a55719c05435456ad14da0a5`): - hard-coding the zone label, dropping the `#423` arm, and gating on neither `unmeasured` nor `unaccounted` are all **RED**, each at the specifically-named test, one of them on the right panic string rather than merely on a panic; - the author's `if true { continue; }` repro is **RED** at three gate tests while the "stays silent when every zone is closed" test stays green — a false-positive control, not just a positive one; - `if false { continue; }` is **RED** at that same silence test, which is the control proving the guard does not simply always fire; - a **call-site** kill in the reach control is RED. This matters: the author's M2 is a *body* mutation, and a body-wrap cannot distinguish "the guard is dead" from "the predicate is false". It is admissible here only because the killing signal is a post-loop `UNREACHED` panic — a reach signal — and because a real call-site mutation exists separately and is RED. - deleting the `[ABORTED …]` `println!` **SURVIVES**, and so does deleting the real corpus call site under a full workspace run. Both are disclosed below rather than papered over. Figures measured by the reviewer on head `3667927` merged with `223ac9b`, streams captured separately: compile sentinel `Finished \`test\` profile … in 36m 39s`, rbuild exit 0 · **55 `running N tests?` headers = 55 `test result:` lines** (equality is the lost-binary check) · **0** non-canonical result lines · **14** targets with nothing to run, anchored on the full field triple · **1828 passed + 0 failed + 47 ignored + 0 filtered = 1875** = the header sum · zero `FAILED`. That is +5 against the 1870 baseline, reconciled by name to the five new tests, with the dropped `..._still_open` confirmed absent by grep. The `--workspace` guard-elision control (R9) deserves its own line: with the real corpus call site deleted, the sorted `test … ok|ignored` set is `diff -q`-identical to the unmutated merge at 1875 lines and all five figures match — so the author's self-measured control that this change adds no hidden coverage is reproduced, not accepted. NOT verified, and not claimed: - The reviewer's figures were measured against `223ac9b`, not against the current `origin/main` (`e821812`). The only change between them is #825, which edits one renderer *test* file and adds two tests; this PR edits `tests/walker_sim.rs`. The two are textually and behaviourally disjoint, but the +2 is inferred, not re-measured on the merged tree. - No live corpus run backs the two runs the doc cites (`crushbone,felwithea`; `akanon,crushbone`) — no baked GLBs. The blackburrow hang (#763) was not reproduced by either agent. - The `[ABORTED …]` line's honesty framing is pinned by nothing — deleting the `println!` survives, and the three `should_panic` strings pin only bucket names. Filed as a follow-up. - The replacement universal at `walker_sim.rs:871-872` — "a per-zone refusal could never fire where the terminal one would not" — is **false as written**: an all-`unmeasured` run dies at the earlier `tot_walked > 0` assert, so the terminal check never runs while the new gate does. The intended property (no new false positives) is true and was confirmed; the sentence is not. Filed as a follow-up rather than fixed here. - `WaterRollup`'s complete `Display` form carries no INCOMPLETE marker, and the gate prints both rollups when only one is dirty. Unreachable in today's corpus by lockstep add/skip; filed. - The `skipped` bucket is exempted from the gate on a rationale that applies equally to `unmeasured`, which *is* gated. The asymmetry is real and was undisclosed; filed. - The 47 `#[ignore]`d tests were not run (#777). CI runs `cargo test --workspace --locked` with no `--ignored`.
…nstead of copying them Addresses #819 — deliberately NOT `Closes`, see below. `fine_tier_corpus_route_success_and_cost` declared its own `LOCAL_REACH` and `LOCAL_CELL` instead of using the `pub const`s in `steering`. #818/#733 promoted `LOCAL_REACH` to `pub` precisely so `drive_walk` and `resync_cursor` could not judge two different carrots; this corpus predates the promotion and was never swept into it, so a future change to either shared constant would have left it measuring the old number and reporting a confident green about a tier production no longer runs. Both are now imported. `LOCAL_BOUND` stays a private local copy: no `pub const LOCAL_BOUND` exists anywhere in the repo, and every site that needs it (`walker.rs:1507`, `steering.rs:1063`, `tests/walker_sim.rs:348`) restates the literal with a comment pointing back to `drive_walk`. Importing it would require deciding where it lives, which #819 does not decide. Those three citations were re-checked at source before merge, not taken from the PR body. WHY THIS IS `Addresses` AND NOT `Closes` — the acceptance bar is not met. #819 asks for a test that goes RED when the copy comes back. The reviewer enumerated the reintroduction paths: a duplicate `const LOCAL_REACH` alongside the import is a duplicate-definition error, and a shadowing `let LOCAL_REACH = 24.0` is a refutable-pattern error — but **deleting the `use` and restoring both private consts (a straight revert of this hunk) compiles clean and nothing catches it, ever.** That is the realistic regression path and it is open after this merge. No assertion in that test can close it, and the reason is structural rather than a missing check: carrots are generated once via `carrot_along(&route, i, from, LOCAL_REACH)` and the same set feeds both the OLD and NEW comparators, so any change to the constant moves both sides symmetrically and `assert_eq!(tot_old_only, 0)` / `assert!(tot_new_ok >= tot_old_ok)` stay satisfied at any value. The test is incapable of observing the constant it consumes. The author's own mutation says the same thing and was disclosed rather than dressed up: `steering::LOCAL_REACH` 24.0 → 200.0 collapsed route success from 97.5% to 4.6% (240 pairs, blackburrow) and the test still reported `ok`. What that does prove is that the import is live and load-bearing — it rules out a dead-import false positive — and that is the honest extent of the claim. A source-scanning guard was considered and rejected. This repo has measured seven distinct ways a source-text pin proves a call is *written* rather than *reached* (#799), and the analogous `doc_citations` machinery was itself found covering a fraction of its intended corpus (#778). An eighth text scan defending a latent drift risk is a worse trade than an open, precisely-stated issue. #819 stays open with the revert path recorded on it. Drift check: none had occurred. Verified at this branch's actual parent — both private copies matched the shared values exactly (`LOCAL_REACH = 24.0`, `LOCAL_CELL = 2.0`). A latent hazard closed, not a live bug fixed. The `24.0` literal sweep was re-run workspace-wide, not just in-crate: `CURSOR_RESYNC_MAX_HOP = 24.0` is rejected as a deliberately separate constant whose doc frames the relationship as conceptual rather than mechanical, and the remaining hits are unrelated coordinates and depths. Figures measured by the reviewer on head `72a9dae` merged with `5fba300`, streams captured separately, run-completion confirmed by process exit rather than by the compile sentinel: `Finished \`test\` profile … in 3m 10s` · **55 `running N tests?` headers = 55 `test result:` lines** (equality is the lost-binary check) · **0** non-canonical result lines · **14** targets with the full `0 passed; 0 failed; 0 ignored` triple · **1830 passed + 0 failed + 47 ignored + 0 filtered = 1877** = the header sum. Identical to plain `5fba300`, zero test-count movement — which is what an import-for-identical-literal swap should look like. The author's own run on the older base reconciles exactly: `1823 + 0 + 47 + 0 = 1870` plus #825's +2 and #824's +5. NOT verified, and not claimed: - The `#[ignore]`d asset-gated corpus test was not re-run by the reviewer — no baked zone GLBs. Only the author ran it, and only to observe the mutation's effect. - The reviewer's reintroduction probes were done via a standalone `rustc` repro rather than by rebuilding the real `collision.rs`, so the two compile-error rows are established for the language rule, not for this file specifically. The third row — the one that matters — needs no probe: it is the diff read backwards. - No live run. This is a test-constant change with nothing observable in game. - The 47 `#[ignore]`d tests were not run (#777).
1 participant
What changed
Fixes #751.
extracted_planner_matches_the_pre_740_loops_over_a_random_corpusincrates/eqoxide-renderer/tests/shadow_caster_selection.rsdrove its whole 400-scene corpus offone shared
Rng(0x5EED_740)advanced continuously across scenes and across everyplayer/nearby/light decision within a scene. That made every draw after the first a hidden
dependency of everything drawn before it — an edit to any earlier fixture decision silently shifted
every later scene, including ones that look unrelated by inspection. This already cost a mutant kill
once: fixing #747's B1 (giving the player-
Absentbranch corpus coverage) added a draw earlier inthe stream, which shifted the one incidental tie that was killing mutant M14 (
sort_by→sort_unstable_by) — it silently went from killed to surviving.fn build_scene(scene: usize) -> (Vec<Cand>, Option<Cand>, [f32; 3], [f32; 3])(crates/eqoxide-renderer/tests/shadow_caster_selection.rs:704), with noRng(or anything derived from one) threaded across a call boundary. There is no variable in thefile through which one scene's draws could reach another scene's.
build_scene(scene).Cand: PartialEqso scene fixtures can be compared directly.scene_fixtures_are_independent_of_call_order_and_neighboring_scenes: builds a handful ofprobe scenes in isolation, then again interleaved with unrelated "noise" scenes immediately
before/after each probe, then again in reverse overall order, and asserts byte-identical fixtures
every time — the executable form of "scene order cannot affect the result." A fixed-order
differential test alone can't exercise this.
splitmix64seedfinalizer (a real, measured correlation defect in the naive per-scene seed), a retraction of two
"structurally impossible" claims plus a source-text pin for the concrete counterexample that
falsified them, and a full from-scratch mutation sweep (this issue's own deliverable, not scoped
out of it).
source-text pin had its own silent-pass gap (a brace-depth scan with no lexer), closed with a
terminator-reached assertion; the pin was also renamed and its doc comment corrected to match its
true, narrower proven property.
The file is now 20 tests (was 18 before this issue, 19 after round 1 of this PR; still 20 after
round 3 — that round only rewrote an existing test's body and doc comment, adding none).
Round 2 fixes (per independent review,
#825review comment)F1 — the mutation sweep is #751's own deliverable; it was falsely claimed to be scoped out
Round 1 of this PR shipped a "NOT verified" note claiming the sweep was "scoped out of this issue on
purpose," citing #751 as authority. That was backwards: the sweep was deferred out of #747, and
#751's own issue text names doing the sweep as its own deliverable ("This issue tracks doing it
as its own change, with its own from-scratch mutation re-validation"). That note was a false claim
about scope, aimed at the next reader, in a tracked file — retracted outright, not softened. The
sweep below is that deliverable, run from scratch against the real build on this branch.
Every mutation applied at the exact production call site in
crates/eqoxide-renderer/src/pass.rs,one at a time, via a cp-aside/md5sum/touch revert cycle (never left applied), built with
rbuild ... test -p eqoxide-renderer --test shadow_caster_selection --locked --no-fail-fast:>=→>on theu_slotboundextracted_planner_matches_the_pre_740_loops_over_a_random_corpus,joint_slots_never_outrun_uniform_slots,selection_is_bounded_at_exactly_shadow_caster_slots,the_player_takes_the_first_slot_and_shrinks_the_nearby_budgetu_slot >= SHADOW_CASTER_SLOTSbound outrightAbsentarm also doesu_slot += 1a_player_with_no_model_casts_nothing_and_consumes_no_slot,extracted_planner_matches_the_pre_740_loops_over_a_random_corpusu_slot += 1out of bothmatches unconditionallya_player_with_no_model_casts_nothing_and_consumes_no_slot,extracted_planner_matches_the_pre_740_loops_over_a_random_corpus,static_casters_take_a_uniform_slot_but_no_joint_slotclip_count != 0 &&guardj_slot >= SHADOW_CASTER_SLOTS { continue; }guardsort_by→sort_unstable_byextracted_planner_matches_the_pre_740_loops_over_a_random_corpus(diverges at scene 0 of 400)Every result matches #747's original kill/survive pattern exactly — nothing #747 killed now
survives under the new per-scene,
splitmix64-scrambled corpus.pass.rswas reverted after eachmutation and confirmed clean by
md5sumagainst the pre-mutation copy; a final baseline run afterthe full sweep is 20/0 green.
F2 — retracted two "structurally impossible" claims; added a source-text pin for the reviewer's MUT-6 counterexample
Two places in the file claimed the per-scene extraction made cross-scene coupling "structurally
impossible." That's false, and an independent review's constructed MUT-6 proves it: mutate
build_scene(scene)so that, whenscene > 0, it folds in a value from a freshly recomputedcall to
build_scene(scene - 1)— noRng, or anything derived from one, is threaded across a callboundary (satisfying the property this file actually tests), yet scene
k's fixture is stillcoupled to scene
k − 1's (now-recomputed) generation logic.build_scene(k)remains a purefunction of
kalone under this mutation, soscene_fixtures_are_independent_of_call_order_and_neighboring_scenescannot see the difference. Measured on the real build: MUT-6 leaves all 19 (pre-pin-test) tests
green — SURVIVED.
Both doc-comment instances (on
build_sceneand on the differential test) are retracted and replacedwith precise language: the extraction guarantees order/neighbor-call independence only — no
variable anywhere holds an
Rngacross a call boundary — and explicitly does not guaranteeedit-locality (that scene
k's definition doesn't reference scenek − 1's recomputed output).Added
build_scenes_source_text_contains_no_literal_call_to_itself(renamed in round 3; see below)(
crates/eqoxide-renderer/tests/shadow_caster_selection.rs:1109): a source-text scan, isolatingbuild_scene's own body by brace-depth counting (not a naive\nfnsplit, which swalloweddoc-comment prose mentioning
build_scene(scene)and false-failed at baseline the first time thiswas written — fixed before landing), that fails if the body contains a nested
build_scene(call.Verified RED under the literal MUT-6 mutation (only this test fails) and GREEN at baseline
(20/20). Its rustdoc explicitly discloses what it does not catch: the same coupling reached
through an intermediate helper function rather than a direct recursive call — that gap is real and
not claimed to be closed.
F3 — the seed formula had a measured correlation defect; fixed with a
splitmix64finalizer, every downstream number re-measured from scratchRng(0x5EED_740 ^ scene as u64)fixed the cross-scene coupling but introduced a narrower, realdefect: an LCG's n-th output is affine in its seed, so 400 seeds differing only in their low bits
don't decorrelate in a handful of steps. Independently reproduced (a standalone replica of the exact
LCG/
rangearithmetic, not trusted from the review comment) at the LCG-step offset producing eachscene's first candidate's model-kind selector: chi²(9 df) = 399.90 against a ~40/80/280 nominal
split (
Absent=39 Static=1 Skinned=360observed). Folding the seed through asplitmix64finalizerbefore constructing the
Rng(the LCG itself unchanged) drops the worst offset found scanning thefirst 25 draws to ~20.9 — two orders of magnitude closer, with every coverage floor in the file
still comfortably clear.
Applied:
Rng(splitmix64(0x5EED_740 ^ scene as u64))(
crates/eqoxide-renderer/tests/shadow_caster_selection.rs:705). Every downstream coverage numberwas re-measured from scratch against the new stream (not carried over from round 1 or the
review), via a temporary
eprintln!before the floor asserts, run once, then removed:(Absent/Static/Skinned)
retuned, per the file's own "smoke alarms, not safeguards" doc comment
under the new seeding (trunc_mixed=80, trunc_all_skinned=0) — no change needed there
MUT-1 (the original cross-scene-coupling mutation, a
static Mutex<u64>carrying state acrossbuild_scenecalls) was re-run under the newsplitmix64seed and is still correctly killed: 19/1,only
scene_fixtures_are_independent_of_call_order_and_neighboring_scenesfails. The differentialtest (
extracted_planner_matches_the_pre_740_loops_over_a_random_corpus) passes against there-measured corpus.
Round 3 fixes (per independent review,
#825review comment)B1 (blocking) — the round-2 source-text pin's brace-depth scan had no lexer, and could truncate silently
build_scenes_dependency_is_its_own_index_and_nothing_else(round 2's pin, above) isolatedbuild_scene's body by counting{/}bytes with no awareness of comments or string/char literals.A
}inside any of those — e.g. a//comment containing a bare}— closes the scan early exactlylike a real closing brace would, and the resulting
bodysilently omits whatever comes after,including a recursive call the test exists to catch. Independent review measured this directly:
MUT-6 (the round-2 recursive-coupling mutation) plus one such stray-
}comment placed early inbuild_sceneshrank the scanned region to ~58 bytes and left the test green — SURVIVED, 20/0 —with the real recursive call sitting just outside the truncated window. That's a gap the pin's own
disclosure didn't cover: the disclosed gaps (below) are all cases where the literal token
build_scene(is genuinely absent from the body via indirection; this one had the token present, justoutside where the scan looked.
Fix: after locating the candidate closing brace, assert the scan actually reached
build_scene's realtail — its literal final expression,
(cands, player, light, player_pos), the last text in thefunction before its true closing brace. A brace that closes the scan early leaves that tail outside
body, so a truncated scan is now caught by the tail's absence and fails loudly, instead of thedepth counter's say-so being trusted silently. This is a targeted assertion, not a full lexer, and the
rustdoc says so explicitly.
Kept brace-depth counting rather than switching to the sibling pin's technique
(
encode_shadow_pass_calls_the_planner_this_file_gradessplits on the next\npub fn):build_sceneis private and is immediately followed by another function's doc comment that itself quotes
build_scene(scene)in prose, so the equivalent\nfnsplit swallows that prose and false-fails onthe unmutated file — confirmed by trying it first, before brace-depth counting was written.
Measured both directions on the real build, independently (not trusted from the review comment):
}comment: 19 passed; 1 failed, the pin failing with"the brace-depth scan did not reach build_scene's real end (only 168 bytes scanned) — ..."— a loud, correctly-attributedfailure instead of the previous silent all-clear.
The guard's honest limit, as now stated in its own rustdoc: it proves only that no literal,
whitespace-exact
build_scene(substring appears inbuild_scene's own scanned source text — notthat the function has no dependency on another scene of any kind, and not that every byte between the
opening and closing brace was interpreted correctly (only that the scan reached at least as far as the
known tail marker). The three previously disclosed evasions — an intermediate helper function,
build_scene (scene - 1)with inserted whitespace, and a function-pointer alias — are all still realand still not claimed to be closed.
N1 — the pin's name overstated its proven property
Renamed
build_scenes_dependency_is_its_own_index_and_nothing_elsetobuild_scenes_source_text_contains_no_literal_call_to_itself, and narrowed its doc comment to match:two of the disclosed evasion routes are direct self-references, but the old name read as a general
"no coupling of any kind" claim, which this syntactic pin does not prove.
N2 — corrected a false "same technique" claim
The pin's rustdoc claimed to use "the same technique" as
encode_shadow_pass_calls_the_planner_this_file_grades. It doesn't: that sibling pin splits the sourceon the next
\npub fn, with no brace-balance dependency at all (viable there only becauseencode_shadow_passispub). This pin's rustdoc now explains the difference and why the\npub fnapproach isn't viable for
build_scene, rather than asserting a false equivalence.Workspace figures (this branch, after round 3, merged with
origin/mainat223ac9b)Captured via
rbuild ... test --workspace --locked --no-fail-faston a disposable local merge ofthis branch into
origin/main(223ac9b), confined to my own worktree and discarded aftermeasurement:
running [0-9]+ tests?headers: 55,test result:lines: 55 (equal — no lost binary)0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out, full triple,field-anchored): 14
passed + failed + ignored + measured + filtered=1825 + 0 + 47 + 0 + 0 = 1872shadow_caster_selection.rs's own block within this same workspace run:20 passed; 0 failed; 0 ignored; 0 measured; 0 filtered outorigin/mainbaseline at223ac9b(1823 + 0 + 47 + 0 = 1870, same 55headers/results, 0 non-canonical, 14 truly-empty):
1825 - 1823 = 2, matching the file's own testcount staying at 20 (round 3 rewrote an existing test, adding none) versus
223ac9b's pre-PR file.Zero failures, zero non-canonical lines, same 55 targets, same 14 truly-empty targets throughout —
no behavioral collision against any in-flight PR merged into
223ac9b.41cca4ereported1812 + 0 + 47 + 0 + 0 = 1859. That figure is retired — this section now measures against currentorigin/main, per round-3 review instruction.)NOT verified, and not claimed
(M6/MUT-A/MUT-B/MUT-C/M10/M11/M14). It is not a claim that every conceivable mutation of
pass.rsor
shadow_caster_selection.rsis killed — only these seven, all previously catalogued, werere-run.
build_scenes_source_text_contains_no_literal_call_to_itselfcloses MUT-6's exact shape (a directrecursive
build_scene(call in the body) and discloses, in its own doc comment, that it does notclose the same coupling reached through an intermediate helper function, through
build_scene (scene - 1)with inserted whitespace, or through a function-pointer alias. Those gapsare real and are left open, not claimed closed.
least as far as
build_scene's known tail expression; it does not prove every byte between theopening and that tail was interpreted correctly by the depth counter. A comment or string literal
containing an unbalanced
{/}pair before the tail, that nets to the correct depth by the timethe tail is reached, is not ruled out and was not tested for.
splitmix64-scrambled seedsbeyond what the differential test and the coverage floors already exercise.