Skip to content

docs(#822): three tracked files told an API reader to read a pos_up key the API does not serve - #841

Merged
djhenry merged 1 commit into
mainfrom
fix-822-posup
Aug 1, 2026
Merged

docs(#822): three tracked files told an API reader to read a pos_up key the API does not serve#841
djhenry merged 1 commit into
mainfrom
fix-822-posup

Conversation

@djhenry

@djhenry djhenry commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Fixes the agent-honesty defect in #822: tracked prose telling an API reader to look up a pos_up
key that no body this API serves contains.

Verified at source, not transcribed from the issue. get_debug hand-builds its player object
and emits "pos": [player.pos_east, player.pos_north, player.pos_up] (crates/eqoxide-http/src/observe.rs:982).
PlayerState derives Serialize but no handler serialises it whole — the same fact #801/#810
already recorded on afloat_stall — so its field names never reach the wire. Corroborated by the
mutation run below, whose failure message printed the endpoint's real 55-key set: it contains pos
and no pos_east/pos_north/pos_up.

Two line numbers in #822 have drifted since it was filed, and one attribution was off; both stated
here rather than silently worked around:

  • the serializer is at observe.rs:982, not :926;
  • pos_east/pos_north/pos_up at crates/eqoxide-http/src/lib.rs:137-139 are PlayerState
    fields, not ControllerView's. ControllerView lives in eqoxide-ipc and has no pos_up field
    at all (grep -rn pos_up returns no hit in that crate). The substance of the issue is unchanged —
    they are internal field names either way.

Concept-grep inventory

Per #822: grep the concept, not one identifier. The concept searched was "prose that tells a
reader of the HTTP API where to read the vertical/up coordinate"
. Searches run over the whole
worktree (*.rs, *.md, *.py, *.sh, *.toml): pos_up; pos_east|pos_north;
pos\[[012]\]; player\.pos|pos|pos array|\[east; and, scoped to docs/,
\bpos\b|position|height|vertical|up coordinate|z-?coord|altitude intersected with
read|agent|key|field|report|serve|expose|check.

Every hit, and the ruling on each:

Site What it is Ruling
docs/http-api.md:316 levitating table: "pos_up is a height the character will not fall from" CHANGED — site 1 of the issue
crates/eqoxide-http/src/lib.rs:198 rustdoc on PlayerState::levitating: "an agent that reads pos_up…" CHANGED — site 2 of the issue
crates/eqoxide-core/src/coord.rs:29 WIRE_Z_OFFSET datum paragraph: "every agent-facing field (/player pos_up, /observe, /observe/entities)" CHANGED — a third site the issue did not name. Found only by the concept grep. It asserts the same nonexistent key and a /player endpoint that does not exist: the routers are nested at crates/eqoxide-http/src/lib.rs:1140-1155 and there is no /player namespace among them.
crates/eqoxide-http/src/lib.rs:137-139 PlayerState field declarations left alone — the internal names themselves
crates/eqoxide-http/src/lib.rs:278-288 from_game_state construction (pos_up: gs.player_z) left alone — internal construction
crates/eqoxide-http/src/lib.rs:339 comment: hold is "as fresh as pos_east/north/up" left alone — a comment about the fields' freshness, addressed to a reader of this struct, not to an API consumer
crates/eqoxide-http/src/lib.rs:667 PlayerAfloatStallView doc: "served as the single pos array … NOT as pos_east/pos_north/pos_up keys" left alone — correct, this is #810's corrective sentence
crates/eqoxide-http/src/lib.rs:689 anchor_* doc: "there are no pos_east/pos_north/pos_up keys in any body this API serves" left alone — correct, #810 round-2
crates/eqoxide-http/src/observe.rs:982 the serializer itself left alone — the code under discussion
crates/eqoxide-http/src/observe.rs:2019-2021 zone_exits: comment + let pos = [...] local left alone — internal read for a geometry test
crates/eqoxide-http/src/lib.rs:1233,1241-1246 pos_up_reports_foot_not_wire test left alone — asserts the internal field carries the FOOT datum
docs/http-api.md:435 anchor_* row: "(Position is served as that one array; there are no pos_east/pos_north/pos_up keys…)" left alone — correct, #810's sentence; the one #822 says line 316 contradicted
docs/http-api.md:36 endpoint table: "pos [east,north,up]" left alone — already correct
docs/http-api.md:404, :940, :943-944, :1330 prose about player.pos / pos moving, provisional, frozen left alone — all name pos, none names a pos_* key
crates/eqoxide-http/src/move_api.rs:50 "matching /v1/observe/debug pos: east = +server_x" left alone — already correct
crates/eqoxide-nav/src/diagnostics.rs:123,412,462 [east, north, up] / [east, north, floor_z] array docs left alone — array shapes, no key names (and eqoxide-nav is off-limits this wave)
crates/eqoxide-core/src/afloat.rs:312,328, physics.rs:113-169, game_state.rs:984,1410,2891,2950, zone_assets.rs:443, assets/src/lib.rs:543, ui/windows/map.rs:98,191, renderer/*, walker.rs:235,1950, interact.rs:191, name_match.rs:53, persist.rs:38, docs/architecture.md:69-70, docs/collision-system.md:92,136, docs/autonomous-play.md:181, docs/plan-standalone-gm-client.md:192,272 internal pos locals, mesh/GLB axis-order notes, gs.player_z caveats left alone — none addresses an HTTP-API reader; several are in files another agent holds

No other site was found that tells an API reader to read a pos_* key. That is the result of
the searches listed above, not a claim that no such sentence can exist anywhere in the repo.


Exact before/after

1. docs/http-api.md:316

Before:

| `true`  | Levitating. `pos_up` is a height the character will **not** fall from, and the controller applies no gravity. |

After:

| `true`  | Levitating. `player.pos[2]` — the up component of the served position array `[east, north, up]` — is a height the character will **not** fall from, and the controller applies no gravity. |

2. crates/eqoxide-http/src/lib.rs:198 (rustdoc on PlayerState::levitating)

Before:

/// Exposed because it changes what movement commands do: an agent that reads `pos_up` while
/// levitating is reading a height it will NOT fall from. No `skip_serializing_if`: the key is
/// ALWAYS present so an absent-key can never be misread as "known false".

After:

/// Exposed because it changes what movement commands do: an agent that reads the up component of
/// the served position array (`player.pos[2]` on `GET /v1/observe/debug`) while levitating is
/// reading a height it will NOT fall from. The `pos_up` field beside this one is the internal
/// name that value is built from, not a response key — position is served as the one `pos` array
/// (#822). No `skip_serializing_if`: the key is
/// ALWAYS present so an absent-key can never be misread as "known false".

#822 offered a fork here ("fix the referent or make it explicit that the name is internal"). Both
are taken: the sentence is about what an agent reads, so it names player.pos[2], and it also
says outright that the adjacent field is internal — so the next reader of this struct does not have
to re-derive which of the two things the sentence meant.

3. crates/eqoxide-core/src/coord.rs:29 (the third site)

Before:

/// ingest in `packet_handler`), and every agent-facing field (`/player` `pos_up`, `/observe`,
/// `/observe/entities`) — is FOOT. The `± WIRE_Z_OFFSET` conversion happens ONLY at the packet edge.

After:

/// ingest in `packet_handler`), and the agent-facing position readings (`player.pos` on
/// `GET /v1/observe/debug`, the positions in `GET /v1/observe/entities`) — is FOOT. Those two are
/// named because they are the position readings this datum rule was written for, not as a claim
/// that they are the only agent-facing values it covers. (There is no `/player` endpoint and no
/// `pos_up` response key — `pos_up` is the internal `PlayerState` field the served `pos` array's
/// third element is built from, #822.) The `± WIRE_Z_OFFSET` conversion happens ONLY at the packet
/// edge.

The old sentence also carried an "every agent-facing field … is FOOT" completeness claim that
nothing here establishes. Since the clause was being rewritten anyway, it is replaced by a named,
checkable pair plus an explicit statement that the pair is not exhaustive, rather than re-asserting
a universal.


Mutation checks

The prose fix has no test behind it, and this was MEASURED, not assumed. All three wording edits
were reverted to their pre-#822 text (the new test kept) and the full workspace suite was re-run:

headers/results passed failed ignored filtered exit
fixed tree 55 / 55 1832 0 47 0 0
all three wordings reverted 55 / 55 1832 0 47 0 0 — still green

So reverting any of the three sentences goes unnoticed by the suite. Said plainly rather than
implied. A doc-key guard of the kind #822 floats ("check the doc's claimed response keys all exist
in a served body") would close that gap for the whole class; it is not attempted here.

Note for whoever tries it: crates/eqoxide-net/src/action_loop.rs:4688 does
include_str!("../../../docs/http-api.md"), so a markdown-only edit to that file is not inert —
it recompiles eqoxide-net.
The only assertion that parses DOC reads the | \idle` |row of the nav_state table; the edit above is in thelevitating` table, so that test is untouched.

What IS mutation-checked is the new test, position_is_served_as_one_pos_array_with_no_pos_up_key_822
in crates/eqoxide-http/src/observe.rs. It pins the wire fact the corrected sentences assert, so
a future change that adds a pos_* key or renames pos goes red and the prose stops being true only
by luck. It drives the real router through debug_json, not serde_json::to_value(&player_state)
the dead end #810 round 1 found. Fixture uses three distinct values with a negative up
([812.5, 43.0, -119.75]), so a transposed or flattened axis cannot pass.

Both mutations run on the remote builder, -p eqoxide-http --lib --locked --no-fail-fast, file
restored from an md5sum-verified copy between runs. Control: 269 passed; 0 failed.

  1. Rename the "pos" key in get_debug to "pos_up"268 passed; 1 failed, at the array
    assertion. Its message printed the endpoint's real key set, which is where the "no pos_* key"
    claim in this PR comes from rather than from reading the source twice.
  2. Keep "pos" and additionally player.insert("pos_up".into(), json!(-119.75))268 passed;
    1 failed
    , at the contains_key loop. This is the half that only the negative assertion covers;
    without it the test would pass a body that served both.

Recorded so the next person does not re-run it: adding a fourth key inside the json! literal does
not compile — that object is already at serde_json's macro recursion limit (error: recursion limit reached while expanding $crate::json_internal!), which is why mutation 2 goes through
player.insert. That limit is also why get_debug hand-builds and patches the object at all, which
is the root reason PlayerState's field names never reach the wire.


Full workspace run — five-figure log standard

rbuild <worktree> test --workspace --locked --no-fail-fast, stdout and stderr captured to
separate files.

  • Compile sentinel (stderr, COMPILE only — not run completion):
    Finished `test` profile [unoptimized + debuginfo] target(s) in 53m 15s
  • Run completion confirmed by the two things that actually prove it: the wrapper's captured
    exit status 0, and a complete final test result: line
    (test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s).
    There is no fetched line to check on a test invocation — rbuild only rsyncs a binary back for
    build/b.
  • Lost-binary check — running [0-9]+ tests? headers vs test result: lines: 55 / 55, EQUAL.
    (Regex allows the singular running 1 test.)
  • Non-canonical test result: lines: 0. Every one matches
    ^test result: (ok|FAILED)\. N passed; N failed; N ignored; N measured; N filtered out; finished in Ns$.
  • Targets with nothing to run: 14, anchored on the full triple 0 passed; 0 failed; 0 ignored.
  • Arithmetic: 1832 passed + 0 failed + 47 ignored + 0 filtered = 1879 = the running N
    header sum (1879). Balances.
  • Positive control that MY edits were built:
    test observe::tests::position_is_served_as_one_pos_array_with_no_pos_up_key_822 ... ok appears in
    the log. A stale-tree build could not print it.

bash scripts/check-no-local-detail.sh → exit 0, "no forbidden patterns in tracked files".


NOT verified, and not claimed

  • No live client was run. This change alters three sentences and adds one unit test; there is no
    runtime behaviour to observe. The new test exercises the real router in-process, which is the
    ceiling here.
  • The prose is not machine-checked. Measured above: reverting all three sentences leaves the
    workspace suite green. The new test guards the wire fact, never the wording.
  • "No other site exists" is not claimed. The inventory is the result of the specific searches
    listed; a sentence phrased in words none of those patterns match would not have been found.
  • The FOOT-datum claim in coord.rs is inherited, not re-verified here. This PR corrected the
    endpoint and key names in that sentence; whether /v1/observe/entities positions are FOOT was
    not independently re-measured in this change.
  • docs/http-api.md was not audited end-to-end for other claimed-but-unserved keys. That is the
    class-wide check Two docs tell an agent to read pos_up; the API serves pos as an array and has no such key #822 suggests and explicitly does not require.
  • Timings are wall-clock from one run each, on a shared builder — not benchmarks.

Closes or addresses?

Closes #822. Its acceptance has two bullets and one instruction:

  • "Neither sentence tells an API reader to read a key that is not served" — met, and a third such
    sentence found by the concept grep is fixed too.
  • "Mutation-check whatever you add: revert the wording and confirm something goes red, or state
    plainly that the fix is prose with no test behind it"
    — the disjunction's second branch is taken,
    measured rather than asserted, and stated in its own section above.
  • The remaining bullet — a check that every documented response key exists in a served body — is
    marked in the issue as "worth considering, but not required to close this". It is not done
    here. If it should block, say so and this becomes Addresses #822; a follow-up issue for that
    class-wide guard would be the cleaner home for it either way.

🤖 Generated with Claude Code

https://claude.ai/code/session_01HQVEpaaKeXsZcW9VT2roeV

…`pos_up` key that is not served

Position is served as one array — `"pos": [east, north, up]`. `pos_east`/`pos_north`/`pos_up`
are `PlayerState` field names that `get_debug` folds into that array; no body this API serves
has a `pos_*` key. Three sentences told an agent to read one anyway:

* `docs/http-api.md` levitating table — now names `player.pos[2]`.
* `PlayerState::levitating` rustdoc — the sentence is about what an AGENT reads, so it now
  names `player.pos[2]` and says plainly that the `pos_up` field beside it is internal.
* `eqoxide_core::coord::WIRE_Z_OFFSET` datum paragraph — found by grepping the CONCEPT, not
  the identifier. It named a `/player` endpoint that does not exist alongside `pos_up`, and
  claimed "every agent-facing field"; both are gone.

Adds `position_is_served_as_one_pos_array_with_no_pos_up_key_822`, which pins the wire fact
those sentences now assert (through the real router, not a hand-serialised struct). The prose
itself has no test behind it — measured, see the PR body.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQVEpaaKeXsZcW9VT2roeV
@djhenry

djhenry commented Aug 1, 2026

Copy link
Copy Markdown
Owner Author

Independent adversarial review — PR #841

I did not write this PR. I re-verified its central claims against source directly, ran an
independent concept-grep sweep with my own predicate (not the author's), and mutation-checked
the new test at the call site on the remote builder. Findings below.

1. The three replacement sentences — checked TRUE at source, not just "less false"

  • crates/eqoxide-http/src/observe.rs:982"pos": [player.pos_east, player.pos_north, player.pos_up].
    Confirmed at that exact line. get_debug's player object emits pos and no pos_east/
    pos_north/pos_up keys.
  • Router nesting — crates/eqoxide-http/src/lib.rs:1140-1155 really is the 16-entry
    .nest("/v1/<group>", ...) list (observe, quests, group, guild, move, trainer,
    pet, combat, interact, merchant, inventory, chat, events, social, camera,
    lifecycle). No /player namespace exists anywhere in it — confirms the coord.rs fix's
    claim that the old "/player pos_up" reference named a nonexistent endpoint.
    observe::router() (observe.rs:180) has .route("/debug", get(get_debug)), so
    GET /v1/observe/debug (the path all three new sentences use) is correct.
  • docs/http-api.md:316 new text (player.pos[2] — up component of [east, north, up]) — TRUE.
  • crates/eqoxide-http/src/lib.rs:198 new rustdoc (agent reads player.pos[2] on
    GET /v1/observe/debug; pos_up beside it is the internal field, not a response key) — TRUE.
    One stylistic imprecision, not a factual error (see NON-BLOCKING below).
  • crates/eqoxide-core/src/coord.rs:29-34 new text — TRUE, including the walked-back "every
    agent-facing field" → named, non-exhaustive pair. Verified GET /v1/observe/entities
    (observe.rs:1797, get_entities) serves a bare [x,y,z] map / labeled variant, never a
    pos_up-shaped key — consistent with what the sentence now claims.

2. Central wire fact — independently reproduced, not just read

debug_json (test helper, observe.rs:2095) drives the real router() through a full
axum::Router::oneshot HTTP request, not a hand-serialized struct — ruling out the #810-round-1
dead end the PR describes. Confirmed by reading it directly.

3. Independent concept-grep sweep (my own predicate, not the author's)

Ran grep -rln "pos_up\|pos_east\|pos_north" (all tracked file types, whole worktree, excluding
target/) — deliberately a different search shape than the author's five-pattern/table approach,
per the standing #799 concern that reusing a scanner reproduces its blind spots. Result: exactly
the four files the PR touches or discusses
(coord.rs, lib.rs, observe.rs,
docs/http-api.md). No hit in scripts/, todo.md, README.md, or any other crate. This
corroborates — via an independently-run sweep — the author's "no other site found" result. It does
not prove no synonym-phrased site exists (author doesn't claim that either), and neither swept
non-.md/.rs/tracked-script prose describing the concept in words that don't contain these three
identifiers.

4. Spot-checked "left alone" rulings — all correct

Checked the ones most likely to matter (agent-facing docs / public rustdoc):
lib.rs:667-670 (PlayerAfloatStallView freshness doc), lib.rs:689-692 (anchor_* doc),
docs/http-api.md:435 (anchor_* table row), docs/http-api.md:36 (pos [east,north,up] table
cell), docs/http-api.md:404/940/943-944/1330 (all reference player.pos, none a pos_* key),
observe.rs:2019-2021 (internal zone_exits local, not served under that name),
lib.rs:1236-1249 (pos_up_reports_foot_not_wire — an internal-field test, doesn't claim pos_up
is a response key). All rulings hold. Also confirmed anchor_east/anchor_north/anchor_up are
real served keys (observe.rs:1252, player.insert("afloat_stall", json!(player_afloat_stall))
serializes PlayerAfloatStallView whole via its Serialize derive) — a different field family from
pos_*, so this is not a contradiction of the "no pos_* key" claim.

5. Mutation evidence — spot-checked at the call site, RED confirmed

Backed up observe.rs (cp -p, recorded md5sum f302307f...), edited observe.rs:982 to rename
the served key "pos""pos_up" (the exact mutation the PR claims to have run), and ran it for
real:

rbuild <worktree> test -p eqoxide-http --lib --locked --no-fail-fast \
  -- position_is_served_as_one_pos_array_with_no_pos_up_key_822

Result — RED, as claimed:

test observe::tests::position_is_served_as_one_pos_array_with_no_pos_up_key_822 ... FAILED
assertion `left == right` failed: position must be served as the one array [east, north, up].
Keys served: [..., "pos_up", ...]   (no "pos" key present)
  left: None
 right: Some(Array [Number(812.5), Number(43.0), Number(-119.75)])
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 268 filtered out

This matches the PR's description almost exactly, including that the failure message prints the
real served key set. File was restored from the cp -p backup and the restore verified by
md5sum match (f302307f... before mutation = f302307f... after restore) before finishing.

Mutation 2 (add a second pos_up key alongside pos) was NOT independently re-run. The
builder was under extreme contention during this review (uptime showed load average 88 on a
6-core box
, ~14 other users' jobs running concurrently) and mutation 1 alone took 34m 50s
compile time for a single targeted -p eqoxide-http --lib build. Re-running a second mutation
under the same load was not a reasonable use of review time once one call-site mutation had
already confirmed the RED behavior the class of test claims. This is an honest gap, not a
"probably fine": mutation 2 is taken on the author's word, not independently measured here.

The test does pin the claim the docs make (one pos array, no pos_east/pos_north/pos_up
keys) rather than something merely adjacent to it — confirmed by reading the assertions directly.

6. Closes vs Addresses

Read issue #822 directly. Its own acceptance criteria say the class-wide "every documented key
exists in a served body" check is "worth considering, but not required to close this." The PR's
choice to keep Closes #822 and not implement that check is consistent with the issue's own
bar. Agree: non-blocking, good candidate for a follow-up issue, not a blocker for this PR.

7. DOC / include_str! coupling claim — verified

crates/eqoxide-net/src/action_loop.rs:4688 has exactly one include_str! of
docs/http-api.md, inside the_documented_idle_reasons_are_exactly_the_ones_the_code_can_publish_725.
It only reads the | \idle` |row of thenav_statetable. This PR's only markdown edit is in thelevitating table (:316`), a different row entirely — confirmed not coupled to that test.

8. Merge-order check (PRs #836, #840)

Both also touch docs/http-api.md (and #836 also touches observe.rs). Diffed both against
main: #836's observe.rs hunk is a comment ~60 lines from get_debug's nav_local section, far
from line 982; its docs/http-api.md hunk is around the nav_local topic (~line 1024). #840
touches docs/http-api.md around lines ~612 and ~778, and action_loop.rs. No region overlaps
with #841's touched lines
(:316 levitating table, observe.rs:982, lib.rs:198, coord.rs:29).
No semantic conflict found; an ordinary same-file textual merge (non-overlapping hunks) is possible
but not a semantic hazard.


MUST-FIX (blocking)

None found. Every replacement sentence checked true at source; the central wire fact was
independently reproduced through the real router; the "left alone" rulings spot-checked all hold;
my own independently-chosen concept-grep sweep turned up no additional site; and the mutation
claim I could afford to re-run (call-site, not body-wrap) came back RED exactly as described.

NON-BLOCKING

  1. crates/eqoxide-http/src/lib.rs:200 — "The pos_up field beside this one is the internal
    name..." pos_up is declared at lib.rs:139; levitating (this doc) is at lib.rs:204 — 65
    lines apart, not textually adjacent in the struct. "Beside" is defensible read loosely as "in
    this same struct," and the substantive claim (internal field, not a response key) is correct,
    but a tighter phrasing ("elsewhere in this struct" / "the PlayerState::pos_up field") would
    remove the ambiguity for a future reader without re-deriving what "beside" meant.
  2. Class-wide doc-key guard (checking every documented response key exists in a served body) —
    the PR and issue Two docs tell an agent to read pos_up; the API serves pos as an array and has no such key #822 both correctly scope this out as a follow-up. Worth filing as its own
    issue given this is at least the third instance of this specific defect class (pos_up sites,
    fix(#801): publish the trapped-swimmer stall to the agent, not to the log #810's anchor_*/hold round, player.hold is never served by any endpoint — the field is maintained, tested, and invisible to every agent (#724) #817's held_secs).
  3. Mutation 2 not independently re-verified (see above) — taken on the author's word only,
    under a measured, disclosed builder-load constraint. If a future reviewer has spare builder
    headroom, re-running "add a pos_up key alongside pos" and confirming it still fails on the
    contains_key loop would close this gap.

Could not establish either way

  • Whether a synonym-phrased sentence (using words other than pos_up/pos_east/pos_north)
    tells an agent to read the up coordinate incorrectly somewhere in the tree. Both the author's
    sweep and my independent one are literal-identifier searches; neither is a semantic/concept
    search over free-text prose. Neither this PR nor this review claims exhaustiveness on that axis.
  • The inherited FOOT-datum claim for /v1/observe/entities (that coord.rs now names) — the PR
    states plainly this was not re-measured in this change, and I did not independently re-measure
    it either; only that the code path (entity_positions(), converted wire→foot at ingest per
    existing packet_handler logic) is structurally consistent with the claim.

Verdict: APPROVE

The prose is true at every checked site, the central wire fact was independently reproduced (not
just re-read), the concept-grep sweep held up under a second, independently-chosen predicate, and
the one mutation I could afford to run under a heavily contended builder came back RED exactly as
claimed. The two non-blocking items are minor and do not gate this PR.

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