Skip to content

fix(#936, #924): correct false doc reasoning, off-by-one, add P_C vacuity guard - #1129

Merged
djhenry merged 3 commits into
mainfrom
worktree-fix-936
Sep 16, 2026
Merged

djhenry merged 3 commits into
mainfrom
worktree-fix-936

Conversation

@djhenry

@djhenry djhenry commented Sep 16, 2026

Copy link
Copy Markdown
Owner

Summary

Closes the live remainder of #936 (three prose items) and #924 (the shared vacuity-guard item), both follow-ups from the #920 round-2 review. Item 3 of #936 and #924 are the same defect — closing both here, per the audit comment on #936.

  • docs/http-api.md — the sentence "there was never a Some(..)" justified a true observable (0 held frames / 300 in a solvable zone) with a controller property that doesn't actually hold: a reviewer probe produced a Some(..) → null transition by raising a hold then swapping to solvable geometry. Replaced the false causal clause with the real invariant — app.rs only swaps self.collision at two sites, both passing through a None window that clears the hold, pinned by the_frames_that_do_not_step_still_clear_the_hold.
  • Off-by-one: GROUND_DEPTH (200) probes from foot + GROUND_ORIGIN (1), so the true reach below the feet is 199 u, not 200. Fixed at all 4 remaining literal sites (docs/http-api.md, eqoxide-http/src/lib.rs ×3); a 5th site in movement.rs already formats the value dynamically and needed no change.
  • Compressed three stale test-pass-count figures (154, 154, 158) in movement.rs doc comments — pre-existing, unrelated to fix(#845): give embedded_no_recovery an exit — a zone-wide standing-place search at the one absorbing arm #920's diff, and already stale against the current ~1145-test suite. Dropped rather than re-derived: the load-bearing fact is which named test now pins each call site, not what the suite's size happened to be at review time. Re-deriving them would just go stale again on the next test addition.
  • P_C in the #845 property family is the one counted antecedent with no vacuity guard #924: added P_C's missing vacuity guard (mobility_cases >= 10) alongside the #845 property family's other three counted-antecedent guards — the one gap fix(#845): give embedded_no_recovery an exit — a zone-wide standing-place search at the one absorbing arm #920's own disclosure flagged (P_D silently passing at 0/0 was the same shape).

Test plan

  • rbuild . test --workspace --locked — all green, 0 failed
  • cargo clippy --workspace --all-targets --all-features -- -D warnings — clean

🤖 Generated with Claude Code

https://claude.ai/code/session_01CWrs7cDd1NAJmaLscBjgcj

djhenry and others added 3 commits September 16, 2026 16:21
…uity guard

Follow-ups from the #920 round-2 review, items 1/2/4 of #936 plus #924:

- docs/http-api.md: the claim "there was never a Some(..)" for the
  embedded_no_recovery hold is false as a controller property — a reviewer
  probe produced exactly that transition by raising a hold before swapping
  to solvable geometry. Replace the false causal clause with the real
  invariant: app.rs only swaps self.collision at two sites, both passing
  through a None window that clears the hold (pinned by
  the_frames_that_do_not_step_still_clear_the_hold).
- "200 u below its feet" was off by one: GROUND_DEPTH=200 probes from
  foot+GROUND_ORIGIN(1), so the true reach below the feet is 199 u. Fixed
  across all 4 remaining sites (docs/http-api.md, eqoxide-http/src/lib.rs).
- Compressed three stale test-pass-count figures (154, 154, 158) in
  movement.rs doc comments — pre-existing, unrelated to #920's diff, and
  already stale against the current ~1145-test suite. Dropped rather than
  re-derived, since the load-bearing fact is which named test now pins
  each call site, not what the suite's size happened to be at review time.
- #924: added P_C's missing vacuity guard (mobility_cases >= 10) alongside
  the family's other three counted-antecedent guards, closing the one gap
  #920 itself flagged as the same shape as P_D's undetected 0/0 pass.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CWrs7cDd1NAJmaLscBjgcj
…ot prose literals

The earlier off-by-one fix ("200 u" -> "199 u") replaced one bare numeric
literal with another, leaving the same silent-drift hazard in place. Add
GROUND_REACH_BELOW_FEET = GROUND_DEPTH - GROUND_ORIGIN as the actual
feet-relative reach (this also caught two more live 200u/199u bugs in
game_state.rs and zone_in.rs that the earlier sed-based fix had missed).
Reference it via intra-doc links where possible, and name RESCUE_RADII in
the remaining 512u prose mentions across docs/http-api.md and eqoxide-http.

Since the affected `detail` fields are &'static str literals (not doc
comments), intra-doc links can't check them. Added a pin test in
src/movement.rs -- the only crate that can see GROUND_REACH_BELOW_FEET,
RESCUE_RADII, and eqoxide_http's public views all at once -- asserting the
docs/http-api.md row and both PlayerHoldView/PlayerRelocationView detail
strings cite the real constant-derived values.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CWrs7cDd1NAJmaLscBjgcj
A subagent review of the prior fix caught that PlayerHoldView and
PlayerRelocationView's &'static str `detail` literals — the actual
HTTP-served prose, not just the doc comments — still cited "512 u" as
a bare literal instead of naming RESCUE_RADII, unlike every other site
touched by the magic-numbers cleanup. The pin test already asserted
the number was correct but didn't check that it was named.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant