fix(protocol): a frame that lost its origin must not claim to lead the axis (improvise-1wl0)

AxisFrame::first carried #[serde(default)], so a frame whose first was dropped
in transit deserialized as 0 -- not an absence but the claim "this frame leads
the axis", which is precisely what body_from acts on. A scrolled browser would
draw the carried group header above its first row, silently.

Dropping the attribute rather than reaching for Option<usize> or a two-variant
enum, and the reason is about the wire rather than the type: there is exactly
one production producer, reading window.start, so a frame is never partially
constructed and "unknown" is not a state an honest peer can be in. Making it
representable would only oblige body_from to answer the same question one level
down. A missing field is now a loud parse failure at the transport.

Same argument that gave Basis no Default (gotcha 43). The Default derive still
yields first: 0 and that is not the same hole -- a default AxisFrame has no
entries, so the claim is vacuous either way.

Test written first and watched fail, printing the forged first: 0. It asserts
the key was present before removal, that the mutated value is still a
well-formed AxisFrame minus one field, and that the error kind is "missing
field" -- the three ways this check could otherwise pass for the wrong reason.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
This commit is contained in:
Edward Langley
2026-09-21 03:53:15 -07:00
parent 4e5f9f3ab8
commit 9b81324d6a
3 changed files with 85 additions and 8 deletions
+23 -5
View File
@@ -836,11 +836,29 @@ The TUI builds over the whole grid so `first` is always `0` there, and the
browser's comes from the projected window. A frame built by hand in a test must
set it, which is why it is a plain field rather than derived.
It carries `#[serde(default)]`, and **that is a defect rather than a decision**
(improvise-1wl0): `first: 0` is not an absence but the claim "this frame leads
the axis", which is the claim `body_from` acts on — so a frame that loses the
field deserializes as leading. No skew ships today, both ends being one
workspace at one commit. Same argument as `Basis` having no `Default`.
It carried `#[serde(default)]`, which was a defect rather than a decision and
is now **gone** (improvise-1wl0): `first: 0` is not an absence but the claim
"this frame leads the axis", which is the claim `body_from` acts on — so a
frame that lost the field deserialized as leading, and a scrolled browser drew
the carried header above its first row. Same argument as `Basis` having no
`Default`: a zero-valued claim that happens to be true in the common case is a
forgeable bypass rather than an absence.
**Dropping the attribute was preferred to `Option<usize>` or a two-variant
enum, and the reason is about the wire rather than the type.** There is exactly
one production producer (`project.rs::axis_frame`, which reads `window.start`),
so a frame is never partially constructed and "unknown" is not a state any
honest peer can be in — making it representable would oblige `body_from` to
answer for it, which is the same question one level down. A missing field is
now a loud parse failure at the transport, where a wire-format skew belongs.
No skew ships today, both ends being one workspace at one commit; this is about
the day there is one, and about the attribute reading as a considered
compatibility decision when it was not.
The `Default` derive still yields `first: 0`, and that is not the same hole: an
`AxisFrame::default()` has no entries, so `body_from` returns an empty slice on
either arm and the claim is vacuous. The hazard was specifically a *populated*
frame acquiring the claim by omission.
**`group_runs` / `group_run_heads` are the coalescing rule's one home.** The TUI
and `render.rs::col_groups` each coalesced adjacent columns sharing a group,
+30
View File
@@ -498,3 +498,33 @@ at `Basis`, a struct, produces a message with no `expected one of` clause; the
helper must panic rather than return. Asserting the panic *message* and not
merely that it panics, because a helper that panicked for some other reason
would satisfy a bare `should_panic`.
### render::tests::a_frame_that_lost_its_origin_does_not_claim_to_lead_the_axis
improvise-1wl0, found by the adversarial review of PR #77. `AxisFrame::first`
carried `#[serde(default)]`, so a frame whose `first` was dropped in transit
deserialized as `0` — which is not an absence but the claim that the frame
*leads the axis*, and exactly what `body_from` acts on:
`match self.first + idx == 0`. A scrolled browser would then draw the carried
group header above its first row, at every scroll position, silently: the
mirror image of the improvise-0o6x bug the field was added to fix.
The test serializes a frame built for a scrolled window (`first: 3`), removes
the key, and asserts the refusal. Three things make it discriminating rather
than a check that something went wrong:
- It asserts the key **was there** before removing it, so a rename of the field
turns this into a failure rather than a vacuous pass.
- It asserts the mutated value is **still a well-formed AxisFrame minus one
field** (`cats` and `entries` present), so the refusal cannot be about broken
JSON — the check-the-checker hole gotcha 42 records from
`the_retired_spellings_do_not_parse`.
- It asserts the **error kind** is `missing field`, not merely that an error
occurred. A `first` that had been retyped would fail differently and this
would notice.
Run against the pre-fix tree 2026-09-21: it fails at the `expect_err`, printing
`AxisFrame { .., first: 0, .. }` — the forged claim itself. The fix is the
removal of one attribute; the whole workspace stays green (1464), which is also
the evidence that nothing was relying on the permissive parse.
+32 -3
View File
@@ -178,9 +178,8 @@ pub struct AxisFrame {
/// The categories on this axis, outermost first. Empty is a real layout
/// (one nameless row or column), not a gap — hence [`AxisFrame::levels`].
pub cats: Vec<String>,
/// The **absolute** data index of this frame's first item. Zero means the
/// frame leads the axis, which is what [`AxisFrame::body_from`] asks.
#[serde(default)]
/// The **absolute** data index of this frame's first item. Zero is a claim
/// and not an absence, so it is NOT `serde(default)` — §3a, improvise-1wl0.
pub first: usize,
pub entries: Vec<FrameEntry>,
}
@@ -1013,6 +1012,36 @@ mod tests {
}
}
/// improvise-1wl0: a dropped `first` must not deserialize as the claim that
/// this frame LEADS the axis. See `context/test-evidence/protocol.md`.
#[test]
fn a_frame_that_lost_its_origin_does_not_claim_to_lead_the_axis() {
let scrolled = AxisFrame {
first: 3,
..two_level_frame()
};
let mut json: serde_json::Value =
serde_json::to_value(&scrolled).expect("an AxisFrame serializes");
assert!(
json.as_object_mut()
.expect("an AxisFrame is a JSON object")
.remove("first")
.is_some(),
"the field must have been there, or this proves nothing"
);
assert!(
json.get("cats").is_some() && json.get("entries").is_some(),
"the mutated value must still be a well-formed AxisFrame minus one \
field, or the refusal below is about broken JSON: {json}"
);
let err = serde_json::from_value::<AxisFrame>(json)
.expect_err("a frame with no origin states no claim, so it is refused");
assert!(
err.to_string().contains("missing field"),
"it must fail as a MISSING FIELD, not as a type error: {err}"
);
}
/// Bug improvise-jjx: the first RENDERED entry always shows its tuple.
/// See `context/test-evidence/protocol.md`.
#[test]