fix(#838): write out lost_load_zone's ZoneAssetState arms so a new in-flight variant reds the binary crate - #847
Conversation
…riant reds this file
UNFINISHED — preserved at wind-down, NOT reviewed, NOT suite-run, NO PR.
The fix and its doc comment are written. What is NOT done:
* the E0004 probe was mid-flight when the agent stopped; its temporary
fifth variant in crates/eqoxide-nav/src/zone_assets.rs has been reverted
by the orchestrator, so no probe scaffolding is in this commit
* no rustc E0004 output has been captured
* no negative control against the unmodified wildcard
* no workspace suite run
* the three include_str!("app.rs") source-text pins have not been checked
against this edit
Anyone resuming: treat every claim in the new doc comment as unverified
prose until the probe is actually run. Base is 8e1dee7; main has since
moved to 7cd9652 (#787 merged), so merge origin/main before building.
Refs #838
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQVEpaaKeXsZcW9VT2roeV
…riant"
claim with the measured E0004 probe result
The rustdoc added in the WIP commit asserted that the tests below
`lost_load_zone` "stay green under a probe variant". That was reasoned, not
run, and as written it contradicts the paragraph it sits in: with the arms
written out, a fifth variant reds this file, so the tests do not run at all
until an arm is written. Replaced with what was actually measured on this PR:
- probe variant present, explicit arms -> exactly one E0004 in the binary
crate, at `src/app.rs`'s `lost_load_zone` match
- probe variant present, `_ => None` -> no E0004 at all
Also narrowed "forces whoever adds it to classify it as in-flight or terminal"
to what E0004 really does: it forces a decision at this site, it does not make
the resulting arm correct. Same correction applied to the test's "what this
test cannot do" note.
Comment-only; no behaviour change.
|
BLOCKING FINDINGS — one blocking (a false mechanism sentence in a tracked file, plus its twin in this body), three non-blocking. The code is right: I attacked the change itself hard and could not break it. Every finding below is prose. Reviewed independently at head MUST-FIX 1 —
|
…nd the wildcard was never why Round-5 review found a reasoned-not-measured mechanism claim in the rustdoc above `lost_load_zone`. It said #826's E0004 probe "did not name this site, because the wildcard absorbed the new variant instead of erroring". That is false, and the probe that falsifies it is the same one the paragraph cites. Measured on this head. Adding a fifth in-flight variant to `ZoneAssetState` and running `cargo check -p eqoxide-nav --locked` reds `eqoxide-nav`'s own lib with six E0004s (all in zone_assets.rs) and stops there: `Checking eqoxide v` appears 0 times, so this file is never type-checked at all. A wildcard here and four explicit arms here are indistinguishable to that invocation. The wildcard's danger is real; that particular probe simply was not the thing that failed to see it. The rustdoc now names the invocation that does bite. Once the six nav arms are filled with `=> todo!()`, `cargo check -p eqoxide --locked` compiles this crate and reports exactly one E0004 in package `eqoxide`, at src/app.rs:136:11 (re-measured after this very edit shifted the line; the pushed body's 118 was WIP-era). `--all-targets` on that same `-p eqoxide` check changes neither number. Negative control run here, not inherited: with `_ => None` restored and the probe variant and filled nav arms still in place, the same command exits 0 with 0 errors and 0 warnings, `Checking eqoxide v0.1.0` present once as the reach control. Two scope corrections in the same paragraph: "exactly one E0004 in the whole crate" is now "in package `eqoxide`", and the one further E0004 that `-p eqoxide-nav --all-targets` reports at zone_assets.rs:705 (inside #826's own test) is named as a different package rather than left to contradict the count. Comment-only. `git diff -U0 | grep '^[+-]' | grep -v '^[+-][+-]'` is 27 lines and every one is `///`, `//`, or blank; the non-comment count is 0. `cargo test -p eqoxide-nav --locked --no-fail-fast` is byte-identical before and after: 2 headers / 2 result lines, 246 passed + 0 failed + 16 ignored + 0 filtered = 262 = the header total, one `Finished \`test\` profile` compile sentinel, 0 stderr errors. That suite is the one that matters here because two of its guards read `src/app.rs` as corpus. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Round 5 pushed as The blocking finding: confirmed false, and I measured the thing that falsifies itThe rustdoc said #826's E0004 probe did not name this site "because the wildcard absorbed the new
So Which invocation actually reds this fileFilling the six nav arms with
136, not 118 or 123. 118 was WIP-era. It measured 123:11 at Negative control — re-run here, not inheritedProbe variant in place, six nav arms filled, Non-blocking items"exactly one E0004 in the whole crate" — narrowed to package The
The claim in the body is now scoped to what the grep supports, and the suite run below is what DeliverablesComment-only, proved mechanically.
One Probe hygiene. Every mutation was One process note against myselfThe first attempt at the arm-filling probe used a shared anchor string that occurs four times in Still open
|
Closes #838.
lost_load_zoneis the decision function behindwatch_for_lost_load, the only detector for azone-asset loader that died without reporting. It matched on
ZoneAssetStatewith_ => None.That wildcard is correct for every state that exists today —
Idle,ReadyandFailedshouldall be left alone — which is what made it dangerous: it reads as deliberate, and it would go on
silently answering
Nonefor a fifth, in-flight variant added later. Such a state would thennever be declared lost, and an agent polling a frozen in-flight zone cannot tell "still loading, be
patient" from "nothing is coming, ever". That is the agent-honesty failure
Failedexists toprevent.
This PR writes the four arms out. One file,
src/app.rs, +41/-2, comment-heavy; no behaviourchange — every existing state answers exactly what it answered before.
Relationship to #826/#837
#826 fixed the same class of wildcard on
ZoneAssetState::collision()incrates/eqoxide-nav/.Its rustdoc scopes this site out by name, and this PR is the named follow-up rather than a
contradiction of it:
lost_load_zone"because the wildcard absorbed the new variant instead of erroring". That wasreasoned, not run, and it is false — the probe below is what falsifies it. A probe scoped to
eqoxide-navreds that crate's own lib and stops there, so the crate holdingsrc/app.rsisnever compiled and this
matchis never type-checked at all. A wildcard here and four explicit armshere are indistinguishable to that invocation. The wildcard's danger is real; that particular probe
simply was not the thing that failed to see it. The rustdoc carried the same false sentence and is
corrected in
2f617b6.Evidence: the E0004 probe
Method: add a fifth in-flight variant to
ZoneAssetState, thencargo checkthe binary crate(
check -p eqoxide --locked) and read rustc's E0004 list. Run remotely; stdout and stderr capturedseparately.
Round 1 — the probe does not reach the binary crate on its own. Adding the variant alone red
eqoxide-navwith 6 E0004s, all incrates/eqoxide-nav/src/zone_assets.rs(lines 105, 118,154, 165, 356, 471 —
tag,zone,collision,detail,usability, and the hand-writtenDebugimpl), and
eqoxidewas never compiled. Recording only this would have proved nothing aboutsrc/app.rs. Each of those six matches was given aProbeVerifying { .. } => todo!()arm so thebinary crate is actually type-checked.
Round 2 — positive control. With
eqoxide-navcompiling and the arms inlost_load_zonewritten out,
cargo check -p eqoxide --lockedreports exactly one E0004 in packageeqoxide(re-measured at2f617b6; the line number below moved because the round-5 doc edit sitsabove the function, and the previously published
118was WIP-era numbering):src/app.rs:136islost_load_zone'smatch st {. Reach control:Checking eqoxide vappearsexactly once in that run. Adding
--all-targetsto the same-p eqoxidecheck changes neithernumber (still 1 E0004, still
src/app.rs:136:11).Scope of "exactly one", stated properly. It is one per package
eqoxide, not one in theworkspace.
cargo check -p eqoxide-nav --all-targets --lockedunder the same probe reports onefurther E0004, at
crates/eqoxide-nav/src/zone_assets.rs:705:19— inside #826's own test, whichdeliberately matches the enum with no wildcard. That is a different package and is not part of the
count above. Within package
eqoxide, the other wildcards on this enum — includingterrain_meshesin
eqoxide-http— stayed silent under the same probe. They are real and out of this PR's scope.Round 3 — negative control. Re-run at round 5 rather than inherited. Same probe variant still
in place, same filled arms in
eqoxide-nav, onlylost_load_zonereverted to its pre-#838_ => Noneform:grep -c 'error\[E0004\]'on stderr: 0grep -cE '^error'on stderr: 0;grep -cE '^warning': 0Finished \dev` profile [unoptimized + debuginfo] target(s) in 3.40s`Checking eqoxide v0.1.0present exactly once in that run, so the crate really wasre-checked and not served from a stale artifact.
So the E0004 is attributable to the match form, not to the probe variant merely existing.
Revert, with the stale-artifact trap closed. Both files were copied aside with
cp -pandrestored from those copies, then
touched —cp -ppreserves mtime, so without thetouchcargo silently reuses the stale artifact and the "clean" run is a false control (
md5sumdoes nothelp: the source is right and the artifact is stale). The post-restore run shows fresh
Checking eqoxide-nav v0.1.0andChecking eqoxide v0.1.0lines, 0 errors, 0 warnings.git diff crates/eqoxide-nav/src/zone_assets.rsafter the revert: empty (wc -l= 0), andgit diff --stat origin/main..HEAD -- crates/eqoxide-nav/src/zone_assets.rson the pushed branch islikewise empty. The probe scaffolding is not in the shipped diff.
What the probe does not establish. Like #837's, this evidence is not a test and CI cannot re-run
it. It is a one-time measurement recorded here and in the rustdoc.
Why there is no lexical source-scanning guard here
#826 added
audit_collision_arms, a comment-stripping, arm-wise scan ofcollision's body, as aCI-runnable pin. This PR deliberately does not add a second one, and the residual risk is stated
rather than hidden:
ZoneAssetStatevariant redssrc/app.rswith E0004 and forces an arm to be written at this site. Measured above, bothdirections. A second guard adds nothing to that.
audit_collision_armsand itsstrip_commentsstate machine live inside#[cfg(test)] mod testsincrates/eqoxide-nav/src/zone_assets.rs(line 480), so they are notreachable from the binary crate. Adding a guard here would mean a second copy of that
scanner, in a different crate, with its own evasion surface — fix(#826): make collision() and usability() fail to compile together on a new state variant #837 needed four review rounds and
five measured evasions (comment-borne
_, a}}in a trailing comment, an#[allow(unreachable_patterns)] _, an or-pattern| _, and a same-line second arm) to get onecopy right. A second copy is a cost, not a bonus.
and writing
_ => Noneagain. That reds no build and fails no test. It is exactly the residualrisk zone_assets: collision()'s
_ => Nonewildcard can make usable_collision's documented-unreachable arm reachable (#803 follow-up) #826 chose to pin forcollisionand this PR chooses not to pin forlost_load_zone. Therustdoc says so in the body ("it does not stop someone re-introducing a wildcard later") and ends
with an explicit
Do not collapse these arms back into_``._ => Nonewildcard can make usable_collision's documented-unreachable arm reachable (#803 follow-up) #826's rustdocstates as its point (b).
The three
include_str!("app.rs")pinssrc/app.rsis read as corpus by tests in other modules, so an edit here is not bounded by thisfile. The
include_str!pins were located (git grep -n 'include_str!'), read, and checked byrunning them, not by reasoning:
src/movement.rs:3520the_zone_change_reload_block_still_forgets_the_recovery_ringif zone_needs_reload(...)block by text and asserts it containsself.controller.forget_recovery_history();src/movement.rs:3572the_frames_that_do_not_step_still_clear_the_holdif self.camera_initialized { if let Some(c) = self.collision...head, walks to its} else {, assertsself.controller.clear_hold();src/zone_in.rs:1153the_app_rs_scan_reads_a_balanced_fileapp.rsunder a comment/string-aware stripper, plusAPP_RS.len() > 50_000src/zone_in.rsthe_app_rs_call_into_this_module_is_an_unconditional_statementself.zone_in.occurs exactly once as code, un-nested, with five named argssrc/zone_in.rsthe_zone_change_blocks_ring_clear_is_not_nested_behind_a_conditionself.controller.forget_recovery_history();exactly once as code, innermost enclosing block head == the reload-block headThe comment channel is the one that has defeated pins of this shape before, and this PR is mostly
comment. Counted in the added lines of the whole branch diff:
self.zone_in.0,forget_recovery_history0,clear_hold0,zone_needs_reload0,camera_initialized0; no raw strings (r#0); braces added3
{/ 3}, all in real match patterns. So the added text introduces none of the tokens thesepins count, and all five run green.
git grep -n 'include_str!'enumerates the pins that read this file via
include_str!and nothing else. At least two furtherguards reach
src/app.rsby walking the tree, and neither would appear in that grep. Both werelocated and read at this head, with line numbers verified rather than quoted:
src/app.rscrates/eqoxide-nav/src/walker.rs:3652, insideexactly_one_production_fine_worker_is_built_in_the_tree_787(fn atwalker.rs:3542)"src/app.rs"is a named reach-control anchor: the guard asserts the file is present in its corpus, then scans that corpus forWalker::new(/LocalPlanner::spawn(constructionscrates/eqoxide-nav/src/steering.rs:1891workspace_rs_files()(rustdoc from:1885), used as the resolution corpus byevery_test_citation_in_the_four_citation_files_resolves_and_is_listed_in_a_guard(fn atsteering.rs:1726)crates/,tests/andsrc/, sosrc/app.rsis in it; only itsfn NAMEdeclarations feed the resolution setfn. Notesrc/app.rsis not incitation_corpus()(steering.rs:1874, four files), so this file's own doc comments are not scanned for citations — a test name cited in the text above would not be checkedRather than argue from those readings, the whole
eqoxide-navsuite was run before and after theround-5 edit; see below. The honest form of the claim is: complete for
include_str!-based pins,not complete for everything that reads this file — the suite run is what covers the rest.
Doc correction made on top of the WIP commit
The WIP commit (
1cfe4c9) asserted in rustdoc that the tests belowlost_load_zone"stay greenunder a probe variant". That was reasoned, not run, and it contradicts the paragraph it sits in:
with the arms written out, a fifth variant reds this file, so those tests do not run at all until an
arm is written.
ece3defreplaces it with the measured probe result above, and narrows "forceswhoever adds it to classify it as in-flight or terminal" to what E0004 really does — it forces a
decision at this site; it does not make the resulting arm correct. The same correction is applied to
the "what this test cannot do" note on
a_pending_load_with_no_live_loader_is_declared_lost.1cfe4c9's own commit message lists this work as outstanding; it is an already-pushed WIP messageand cannot be amended without a force-push (blocked here). This body supersedes it.
Suite
cargo test --workspace --locked --no-fail-fastatece3def, stdout and stderr captured separately.Finished `test` profile [unoptimized + debuginfo] target(s) in 24.46s(this is the COMPILE sentinel, not run-completion); 1
Compilingline^running [0-9]+ tests?$headers vs 55^test result:lines — equality is thelost-binary check
^running 0 tests$= 14;^test result: ok\. 0 passed; 0 failed; 0 ignored= 14).Looser patterns measured on the same log give unanchored
running 0 tests= 14,[^0-9]0 passed;= 18, bare0 passed= 21; the last two are artifacts of matching a zero in thefailed/ignored/filtered fields, not extra empty targets.
^error|panicked|Killed: 0;^warning: 0Base
ef01926measured in its own worktree runs 55/55 and 1843 + 0 + 47 + 0 = 1890. The figures areidentical, which is the expected result and is itself a check: #838 adds no test, so any delta
would have needed reconciling by name.
Not established
in a running client that was not already observable.
terrain_meshesineqoxide-httpandstatusineqoxide-navare still wildcards on this sameenum. The positive-control run confirms they stay silent under a fifth variant. They are out of
scope here, as zone_assets: collision()'s
_ => Nonewildcard can make usable_collision's documented-unreachable arm reachable (#803 follow-up) #826's rustdoc already records.Round 5
Head
2f617b6. Comment-only on top ofece3def.git diff -U0 ece3def..2f617b6 | grep '^[+-]' | grep -v '^[+-][+-]'is 27 lines; filtering out every line matching
^[+-][[:space:]]*(///|//)or^[+-][[:space:]]*$leaves 0. The whole-branch non-comment diff against
origin/mainis still exactly the sevenlines of the actual fix (the
use ... as S;import and the four arms replacing_ => None).eqoxide-navsuite, before and after, same command(
cargo test -p eqoxide-nav --locked --no-fail-fast): identical both runs —2
^running [0-9]+ tests?$headers vs 2^test result:lines,246 passed + 0 failed + 16 ignored + 0 filtered = 262 = the header total, one
Finished `test` profilecompile sentinel,^error|panicked|Killedon stderr 0.This is the suite that matters for a comment-only change to
src/app.rs, because two of itsguards read that file as corpus (table above).
cargo check -p eqoxide --lockedexits 0 with 0 errors and 0 warnings,Checking eqoxide-nav vandChecking eqoxide veach present once.git status --shortclean after every probe; the mutatedzone_assets.rswasrestored from a
cp -pcopy andtouched each time, anddiffagainst the pristine copy is empty.Still open after round 5
_ => None. Deliberate; stated in therustdoc and in the section above.