fix(#742): clear the residual workspace lint warnings — per instance, with a reason each; and write the _SUCCEEDS/_FAILS convention down

[djhenry]

Closes #742 — partially: 6 of the 7 measured warnings are fixed here; the 7th is in a file an open PR owns and is deferred behind it (details below). Plus the convention decision the issue asks for.

Before/after warning table

Both sides rebuilt from scratch — cargo clean first, then cargo check --workspace --all-targets --locked. Warnings are deduplicated by cargo across incremental builds, so a warm build under-reports; neither figure below is warm, and neither reuses the issue's numbers.

Before (main a48760b): 7. After (this branch): 1.

#	file:line	lint	disposition	why
1	crates/eqoxide-core/src/game_state.rs:2042:90	dead_code — field 0 of Ev::BuffUnknown	fix the wiring	The property test's alphabet lied about its own coverage. See below.
2	src/model.rs:323:34	unused_imports — MockProbe	delete	Genuine leftover of an over-broad re-export. All four consumers already bind a MockProbe by inference from MockModel::probe(); nothing ever wrote the type's name.
3	src/model.rs:454:23	dead_code — MockModel::plan_radius	delete	Genuine leftover. A builder setter no test ever called. The field stays live (run hands it to find_path_ex) with its documented 1.0 default; only the never-used override is gone.
4	crates/eqoxide-nav/src/steering.rs:823:9	dead_code — Run::x_min, Run::x_max	make it read	See below.
5	crates/eqoxide-nav/src/traversability.rs:681:8	dead_code — fn lintel_corridor	delete	Orphaned by #567. See below.
6	crates/eqoxide-nav/src/traversability.rs:726:8	dead_code — fn low_wall_corridor	delete	Orphaned by #567. See below.
7	crates/eqoxide-nav/src/walker.rs:2032:8	dead_code — fn pad_scene	DEFERRED	File owned by open PR #774. Untouched.

No allow was added at any scope — not per-item, not per-module, not per-crate. Each instance resolved to a delete or a real fix, so none was needed.

Why 7 and not the issue's 8 — reconciled by measurement, not arithmetic

The issue's 8 was measured at merge-base d977706. Two commits have moved it since, and both are checkable:

• −2: asset_sync: the asset-server login() sits outside the observed window, so a hung login reads as idle #731 / PR fix(#731): observe the asset-server login, so a hung login stops reading as an idle client (supersedes #743) #755 (644ba2c) added the narrow per-function #[allow(non_snake_case)] to both src/asset_sync.rs test names while lint hygiene: the residual workspace warnings after #730, and the _SUCCEEDS/_FAILS naming convention decision #742 was being written. git log -S"allow(non_snake_case)" -- src/asset_sync.rs returns exactly that commit. So the non_snake_case pair is already gone from main; nothing in this PR touches it.
• +1: South Qeynos → qcat (zone 45): walker stalls 6 of 8 attempts at [-534,144,-6] — the route exists and does succeed; the "no walkable floor" premise is false #673 / PR fix(#673): resync the coarse-route cursor when physics carries the character past it #727 (24b84cb) introduced Run::x_min/x_max in steering.rs. git show d977706:crates/eqoxide-nav/src/steering.rs | grep -c x_min → 0. That is why eqoxide-nav reads 4 here against the issue's 3.

8 − 2 + 1 = 7. ✓

The three that were not just noise

Ev::BuffUnknown's discarded payload (#1) — the test's vocabulary lied. The arm read:

Ev::BuffUnknown(_) => { st.buff_slot_changed(1, None, false);
                        if !ref_slots.contains(&1) { ref_unresolved.insert(1); } }

Both the state under test and the reference model hardcoded slot 1 and dropped the variant's payload, while every sibling arm (BuffOn(s), BuffOff(s), BuffKnownOther(s)) passes its slot through. The alphabet therefore could not express "an unresolvable buff in a slot other than the one the rest of the sequence acts on": a BuffUnknown(2) symbol would be byte-identical in effect to BuffUnknown(1) — silently — while reading in the source as extra coverage. The slot now comes from the event on both sides, and BuffUnknown(2) joins the alphabet (13 → 14 symbols, 2197 → 2744 orderings).

I want to be exact about what this is: LevitateState itself needed no change and none was made. This was a defect in the test's vocabulary, not in the product. It is not a #343-shaped honesty bug (see below).

lintel_corridor / low_wall_corridor (#5, #6) — orphaned by #567, and the surviving doc still pointed at them. #567 moved the tests that consume these fixtures into tests/walker_sim.rs (they step the real movement::CharacterController, which eqoxide-nav cannot depend on) but left the fixtures behind in traversability.rs, unused. The geometry in walker_sim.rs is byte-identical, so deleting the orphans moves no coverage — planner_never_routes_under_a_lintel_the_walker_collides_with and planner_never_routes_over_a_low_wall_the_walker_cant_step are alive and untouched.

The part worth flagging: the surviving test's doc comment in traversability.rs said "the lintel fixture above catches the behaviour" and "..._low_wall_... below pins the behaviour". Both cross-references have been false since #567 — they pointed at a dead copy and at a test that is no longer in the file. Both now name the tests/walker_sim.rs tests that actually pin the behaviour. That is a docs-honesty fix riding along with the lint fix, and it is the reason I did not simply delete the two functions and stop.

Run::x_min/x_max (#4) — computed, then dropped. Run's doc says the whole-run and settled extents "measure different things, and both are recorded rather than one standing in for the other" — the #727 round-5 correction that exists specifically to stop the transient and the settled band being conflated. But nothing read the whole-run pair, so the transient it protects was invisible in the output. They are now printed beside the settled band, and deliberately not asserted: the transient's width is a diagnostic, and this test's only claim is about the settled cycle. Observed output (-p eqoxide-nav --lib -- ..._a_few_frames_wide --nocapture):

settled span 1.3200   east of LANDED 0.4011   west of LANDED 0.9189   whole-run extent [-536.5245, -528.3905] (transient included)

Collision list — what I deliberately did not touch

Derived from gh pr list --state open + git diff --name-only origin/main...origin/<branch> for every open PR, then cross-checked against every live worktree's HEAD and working tree.

warning	owner	status
crates/eqoxide-nav/src/walker.rs:2032:8 — dead_code, fn pad_scene	PR #774 (fix-766-nav-local)	Deferred. Not edited.

That is the only collision. game_state.rs, model.rs, steering.rs, traversability.rs and dev-workflow.md are in no open PR's diff and in no live worktree's uncommitted changes.

For whoever picks up the deferred one after #774 merges: pad_scene is a one-line alias fn pad_scene() -> Collision { pad_scene_leaves(false) } with no callers anywhere in the tree — every consumer calls pad_scene_leaves directly. It looks like a plain delete. I did not delete it and did not compile-verify that deletion, so treat that as a reading, not a measurement.

Note on the name trap: there are two asset_sync.rs. This PR touches neither — not root src/asset_sync.rs (its convention work was already done by #755) and not crates/eqoxide-ipc/src/asset_sync.rs (owned elsewhere).

Convention decision: write it down, do not add more code

The issue offers two defensible options and asks for one. I picked write the convention down, in docs/dev-workflow.md under Testing.

The reason is that the code half is already in the tree and I would have been writing a no-op. All four instances already carry a narrow per-function #[allow(non_snake_case)]: #730/PR #738 added src/movement.rs's pair, and #731/PR #755 added src/asset_sync.rs's pair (644ba2c) while #742 was being written. The issue's "add the same narrow allow to the two new asset_sync.rs functions" was therefore already satisfied by another PR before I started — measured, not assumed: grep -n "allow(non_snake_case)" returns src/movement.rs:1694,1758 and src/asset_sync.rs:1399,1415. So "add the allow" had nothing left to do, and "write it down" was the option with content.

The written rule endorses exactly what #742 and #738's reviewer recommended, and records why rather than just what: per-function only, never module or crate scope, because a wide allow also silences every accidental non_snake_case in that scope — a typo'd fn myTest, a stray capitalised local — which is the same "the warning you meant to keep is lost with the ones you meant to suppress" failure that #723/#730 exist to close. It also records the requirement that each instance keep a comment naming the load-bearing word, since all four already do.

I did not also add code, per the issue's "pick one".

Honesty-bug triage — none of the #343 shape found

The issue asks whether any dead_code is an observable an agent can read with no live writer. No, and this is measurable rather than argued: cargo attributed all 7 warnings to lib test targets, and cargo check --workspace --all-targets reported zero warnings against any non-test target. Every one of the 7 sits inside a #[cfg(test)] module — verified per hunk: game_state.rs #[cfg(test)] at 2018, steering.rs #[cfg(test)] mod cursor_resync_tests at 690, traversability.rs #[cfg(test)] mod tests at 644, model.rs #[cfg(test)] mod mock at 330 and the #[cfg(test)] re-export. None is reachable over the HTTP API, so none can report a confident falsehood to a driving agent.

No separate issue is filed. The two "wired up wrong" instances (#1, and the stale cross-references behind #5/#6) are both test-internal, both trivial, and both in files no open PR owns, so per the brief they are fixed here rather than handed off.

Verification

Tiers 1–3 of the hierarchy; tier 4 (live run) deliberately skipped, and it is not a gap: the entire diff is #[cfg(test)] code plus one Markdown file, so there is no production behaviour to observe in a client. (Verified by locating each hunk's enclosing #[cfg(test)], as listed above. I did not verify binary identity of the release artifact — that is the one claim here I am making from inspection rather than measurement.)

Tier 3, mutation-checked (-p eqoxide-core --lib -- levitate_state_is_exactly_the_union_of_its_two_channels):

step	result
Add BuffUnknown(2) to the alphabet, arm unfixed	GREEN — and this is the finding, not a mistake. It passes because the reference model hardcoded slot 1 too, so both sides agreed and the new symbol was a silent duplicate. I predicted RED here and was wrong; measuring is what located the real defect.
Arm fixed (slot from the event, both sides)	GREEN
Mutant: state-under-test call back to buff_slot_changed(1, ...), reference left on s	RED — panicked at game_state.rs:2103: levitate answer() mismatch after [Fly(false), BuffUnknown(2), BuffOff(1)] (fly=false slots={} unresolved={2})
Restore from the copy	md5sum identical (4ed54fd1998efba92534665f1181b11f)

So the fixed arm plus the new symbol detect a slot-identity regression that neither could detect before — the coverage is real, not nominal.

Full workspace suite — cargo test --workspace --locked --no-fail-fast, both sides, cold. Five figures:

figure	main a48760b (baseline)	this branch
Finished line	`test` profile [unoptimized + debuginfo] target(s) in 49m 15s	`test` profile [unoptimized + debuginfo] target(s) in 36m 58s
test result: lines / expected	50 / 50	50 / 50
M non-canonical (visibly corrupt)	0	0
K all-zero (empty targets)	15	15
passed + failed + ignored + filtered = running total	1664 + 0 + 45 + 0 = 1709	1664 + 0 + 45 + 0 = 1709
equals sum of header Ns?	yes — 1709	yes — 1709

Header-vs-summary parity holds on both sides (50 = 50), so no binary died mid-run. Test counts are identical across the two sides, which is the expected shape: this PR adds no test and removes none — the new BuffUnknown(2) symbol lives inside an existing test, and the deleted fixtures were functions, not tests. The wall-clock difference is builder contention (load average 42–93 on a 6-core box across the two runs), not a signal.

The warning tables above were independently reproduced by these test runs, which is a second command agreeing with the first: cargo test --workspace emitted the same 7 --> locations on main and exactly 1 (walker.rs:2032) on this branch.

What I could not establish

• Whether pad_scene is truly a safe delete. Read-only reading says yes (one-line alias, no callers). Not deleted, not compiled, not tested — deferred behind PR fix(#766): retire nav_local with the goal — a zone change no longer publishes the previous zone's fine-tier verdict #774.
• Binary identity of the release artifact. I established every hunk is inside #[cfg(test)] by inspection; I did not build both sides in release and compare, so "no production behaviour changed" rests on that inspection.
• Whether the whole-run extent now printed in steering.rs is correct. It is a diagnostic and nothing asserts it; I observed one value and did not validate it against an independent measurement.
• Which specific eqoxide-nav warning the fix(#730): deny the f32-fallback regression in eqoxide-ui, allow the two deliberate non_snake_case test names #738 reviewer's "3" excluded. I resolved this by dating x_min's introduction to 24b84cb, but I did not rebuild at d977706 to confirm the reviewer's set item-by-item.
• CI toolchain pinning (.github/workflows/test.yml, dtolnay/rust-toolchain@stable, no MSRV) — explicitly out of scope, untouched, and I surfaced nothing new about it.

🤖 Generated with Claude Code

https://claude.ai/code/session_01HQVEpaaKeXsZcW9VT2roeV

[djhenry]

Independent review — PR #786 (#742 lint hygiene)

Verdict: APPROVED.

I re-derived every load-bearing claim rather than reading the PR body as fact. Everything checked
out; one small figure discrepancy noted below is benign.

What I measured (executed, not read)

1. Warning counts, both sides, cold cargo check --workspace --all-targets --locked on the
   remote builder, separate worktrees at a48760b and ea137ba:

   • main (a48760b): exactly 7 warnings, at the exact 7 file:lines in the PR table
     (game_state.rs:2042:90, model.rs:323:34, model.rs:454:23, steering.rs:823:9,
     traversability.rs:681:8, traversability.rs:726:8, walker.rs:2032:8), all attributed to
     lib test targets.
   • branch (ea137ba): exactly 1 warning — walker.rs:2032:8 pad_scene, the deferred one.
   • This is a full independent reproduction, not a re-read of the table.

2. Reconciliation arithmetic (8 → 7), re-derived myself:

   • git log -S"allow(non_snake_case)" -- src/asset_sync.rs → exactly 644ba2c; confirmed
     src/asset_sync.rs:1399,1415 already carry #[allow(non_snake_case)] on main. The −2 holds.
   • git show d977706:crates/eqoxide-nav/src/steering.rs | grep -c x_min → 0;
     git log -S"x_min" -- crates/eqoxide-nav/src/steering.rs → introduced at 24b84cb (South Qeynos → qcat (zone 45): walker stalls 6 of 8 attempts at [-534,144,-6] — the route exists and does succeed; the "no walkable floor" premise is false #673/PR
     fix(#673): resync the coarse-route cursor when physics carries the character past it #727). The +1 holds. 8 − 2 + 1 = 7, confirmed by measurement, not accepted on arithmetic alone.

3. Deletion safety (lintel_corridor/low_wall_corridor) — workspace-wide grep, not
   file-scoped: both names are gone from traversability.rs and appear only in
   tests/walker_sim.rs. Diffed the two versions byte-for-byte — identical, including the
   inline comments. The two tests that consume them
   (planner_never_routes_under_a_lintel_the_walker_collides_with,
   planner_never_routes_over_a_low_wall_the_walker_cant_step) genuinely assert the
   drift-direction property (walk the real CharacterController into the fixture, confirm it's
   blocked, then assert the planner never emits a route through it) — not just fixture reuse
   under a shared name. No coverage was lost.

4. MockModel::plan_radius deletion: confirmed the field is still written (run() at
   src/model.rs:514 still hands self.plan_radius to find_path_ex) and no remaining call site
   invokes the deleted setter (grep -rn '\.plan_radius\(' --include='*.rs' . → empty). Genuine
   dead code, safe delete.

5. The BuffUnknown mutation-check, reproduced independently (attack gfaydark platform pathing stall #4 in the brief):
   copied game_state.rs aside (md5sum 4ed54fd1998efba92534665f1181b11f, matching the PR body's
   own citation — same file), applied the asymmetric mutant (state-under-test call reverted to
   hardcoded slot 1, reference model left reading the event's real slot), ran
   -p eqoxide-core --lib -- levitate_state_is_exactly_the_union_of_its_two_channels on the
   builder. Result: RED, panicked at crates/eqoxide-core/src/game_state.rs:2103:17: ... after [Fly(false), BuffUnknown(2), BuffOff(1)] — the exact sequence and file:line quoted in the PR.
   Restored from the copy; md5sum matched the pre-mutation value again. The regression-detection
   claim is real, not narrated.

6. The citation/doc-span guards in steering.rs — reach proven by execution, not by reading
   (per the coordinator's correction mid-review): re-implemented unbalanced_doc_spans standalone
   in Python from crates/eqoxide-nav/src/steering.rs:1779-1795, gave it a positive control
   (a synthetic file with a genuinely broken two-line span plus a fenced lone backtick that must
   NOT fire) — it caught the broken span and correctly ignored the fence. Ran it against the real
   files at both revisions: zero unbalanced spans in traversability.rs and
   docs/dev-workflow.md (this PR's two doc-comment edits), and zero in all four files the actual
   guard's cited_in corpus covers (steering.rs, walker.rs, collision.rs,
   tests/walker_sim.rs), main and branch alike.

   • Real, confirmed gap, unrelated to a defect in this PR: unbalanced_doc_spans and
     doc_citations are only invoked over the 4-file cited_in corpus
     (steering.rs:1572-1576), not the 162-file resolve_in walk (verified: find crates tests src -name '*.rs' | wc -l → 162, confirming the coordinator's count). traversability.rs and
     docs/dev-workflow.md are outside cited_in, so a broken citation or reflowed span in either
     would not be caught by this guard or by CI. This PR's two new backtick citations in
     traversability.rs (to tests/walker_sim.rs::planner_never_routes_under_a_lintel_... and
     ..._over_a_low_wall_...) were verified by hand (both fn names exist, confirmed by grep) —
     not by the automated guard, because the guard cannot see that file. Worth a standing note for
     whoever next edits traversability.rs's doc comments; not a blocker here.
   • The PR's actual edit inside steering.rs (the Run::x_min/x_max "make it read" change) is
     entirely // line comments inside a test body, not //////! doc comments — confirmed by
     reading the exact prefix used, so it is structurally outside both scanners' input regardless
     of file.

7. Full workspace test suite, branch only, cold, cargo test --workspace --locked --no-fail-fast (did not re-run main's full suite — see "took on word" below):

   • 50 test result: lines, 0 FAILED.
   • Field-anchored all-zero/empty targets (0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out, all fields zero): 15 — matches the PR's figure. A naive 0 passed; 0 failed grep
     (ignoring the ignored field) gave 19, over-counting by exactly the failure mode the
     brief warned about (targets with real ignored-doctests, e.g. 1 ignored, 2 ignored,
     4 ignored, are not empty).
   • passed+failed+ignored+filtered summed via regex over all 50 lines: 1664 + 0 + 45 + 0 =
     1709. Sum of running N tests headers across all 50 targets: 1709, count 50. Parity
     holds — no binary died mid-run.
   • Running/Doc-tests occurrences counted anywhere-on-line: 37 Running + 13 Doc-tests = 50,
     reconciling exactly against the 50 test-result lines.
   • One discrepancy from the PR's claimed figures: I found 1 non-canonical (spliced)
     test result: line (floating_placement's result line got a Running tests/fog_shader.rs
     header spliced into it before its own finished in suffix), where the PR claims 0. Not a
     failure — the embedded counts (11 passed; 0 failed) are intact and included correctly in
     the 1664/1709 sums above; it's ordinary concurrent-output interleaving, the same phenomenon
     the brief's standing rules describe as expected under builder contention. Flagging the number
     itself as measured-different rather than silently rounding it to match.

Verified by inspection (not full independent re-execution)

• Every one of the 5 hunks in game_state.rs, steering.rs, traversability.rs, model.rs sits
  inside a #[cfg(test)] block — checked each file's enclosing mod line directly (game_state.rs
#[cfg(test)] pub(crate) mod tests at line 2018, steering.rs #[cfg(test)] mod cursor_resync_tests at 691, traversability.rs #[cfg(test)] mod tests at 644, model.rs
#[cfg(test)] pub(crate) use mock::MockModel / mod mock at 327–330). No production path
  changed.
• git diff --name-only origin/main...HEAD: exactly the 5 files claimed
  (game_state.rs, steering.rs, traversability.rs, docs/dev-workflow.md, model.rs).
  crates/eqoxide-nav/src/walker.rs is not in the diff — the pad_scene deferral is real.
• gh pr view 774 confirms walker.rs is in that PR's file list — the collision list's single
  entry is real, not asserted.
• git diff origin/main...HEAD | grep -E '^\+.*allow\(': the only match is prose inside
  docs/dev-workflow.md describing the convention (`#[allow(non_snake_case)]` inside backticks
  in Markdown) — no allow attribute was added to any Rust source. Confirmed by diff, not claim.
• bash scripts/check-no-local-detail.sh on the worktree: exit 0.
• docs/dev-workflow.md's new text: reads accurately (per-function scope only, states the reason —
  a wide allow also swallows accidental non_snake_case — matches the four existing sites'
  actual shape) and doesn't overclaim; it explicitly says "naming convenience with no runtime
  meaning."

Took on the author's word (not re-verified)

• Main-side full cargo test --workspace run (the 49m/37m wall-clock and the main-side 50/50 /
  1664+45=1709 figures) — I verified main's warning count independently (cargo check, exact
  match) but did not re-run main's full test suite; CI already gates main continuously and the
  branch-side full run above is independently reproduced in full. Given time budget this is the one
  tier-3 rule I did not fully double-run on both sides.
• pad_scene's deletion safety once PR fix(#766): retire nav_local with the goal — a zone change no longer publishes the previous zone's fine-tier verdict #774 merges — untouched by this PR by design; not my
  scope to verify.

No findings that block merge

Nothing here rises to a request-changes finding. The one real gap found (the citation/span guard's
4-file blind spot covering traversability.rs and docs/dev-workflow.md) is a pre-existing
property of steering.rs's guard, not something this PR introduced or should be blocked on fixing;
I hand-verified the two new citations resolve correctly in its absence.

---

Squash subject: fix(#742): clear the residual workspace lint warnings — per instance, with a reason each

Squash body: use the branch's single commit body as-is (ea137ba) — it already matches what
shipped, contains no retracted claims, and every identifier it names (BuffUnknown,
lintel_corridor, low_wall_corridor, plan_radius, MockProbe, pad_scene, x_min/x_max)
is left un-hand-wrapped and greppable. No trailer needs stripping.