review: the surviving mutant, and four claims that were not true

From the independent review of #23. One fixture line, four comments, and a
set of doc guards it offered as optional.

**The one windowing clause with no guard.** Deleting `pending.clear()` — keep
only the nearest header *above* the window — left the whole workspace green.
`a_window_opening_inside_a_group_names_it_without_painting_it` used a fixture
with exactly one group above the window, so it could not tell "the nearest
comes" from "every one comes". Correctness is untouched either way
(`group_of` still finds the nearest, `body_from` still starts at the first
item); it is a *cost* property, O(the groups a deep scroll crossed), and
example-based correctness tests are blind to those. The test now opens inside
`app_with_consecutive_headers`, which has two headers above the window, and
is renamed `a_window_opening_inside_a_group_names_only_the_nearest`. The
mutant dies: `["[Hidden]", "[Late]", "East"]` against `["[Late]", "East"]`.

**`grouped_model()` claimed coverage it does not have.** Its doc advertised
"multi-level row headers"; `Region` is alone on Row and `Month` alone on
Column, so both axes are single-level and `the_whole_frame_is_unchanged` —
this PR's only byte-exact evidence — exercises no multi-level suppression at
all, which is the whole subject of the change. The fixture is **not** touched:
widening it would destroy the before/after comparison, the same trade as
PR #12. The doc now says what it covers and names what it does not, and
points at `tests/grid_frame_agreement.rs` for the multi-level half.

**`RenderRow::labels()` said a group header has a label of its own**; it
returns `&[]`. Corrected, with the reason the empty slice is right (the label
lives in the `Group` variant) and the consequence for a caller that means
"every label on screen".

**The corrected expectation I wrote down was arithmetically wrong**, which is
worse than writing none — whoever picks up improvise-gd83 would have believed
it. `_Measure` is the *outer* level, so the axis is two `Q1` headers, one per
measure, with the ungrouped `Sep` inside each run rather than after both.
Membership-correct spans are four runs, `[(Q1,2),(None,1),(Q1,2),(None,1)]`,
not two. Now a named constant with the frame dumped beside it, and an
`assert_ne!` that starts failing the day gd83 lands.

**"The only thing the user can click" is not true in a browser.** `web/app.js`
puts no listener on a group `<th>` and `toggle-group-under-cursor` is still in
`the_command_names_that_still_need_the_host_are_these`, so `z` is `Upstream`
into an empty vec — a black hole, not a slow path. The rule is unchanged and
still right: the user must *see* the collapsed group. Three comments now say
seen rather than clicked, and name what would have to land first.

**Taken from the optional list**, because deleting `same_prefix` for this
reason and leaving its four siblings unguarded is inconsistent: `item`,
`label`, `shows_level` and `group_of` have zero production callers and now
each carry a one-line "not for a draw loop — see `header_labels`/`groups`/
`shares_ancestry`". `shows_level` says out loud that it materializes every
item per call and is deliberately *not* made cheaper, since a fast
per-position form is an invitation to the loop `header_labels` exists to
replace.

**Test count corrected.** 1296 passing, 0 failing, 2 ignored, under
`--no-fail-fast`. My 1331 was a miscount and is fully explained: 1296 + 2
ignored + the 33 `test result:` lines my `^test [a-z]` regex also matched.
Slowest binary 1.44s here, sum of binaries 2.31s.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Edward Langley
2026-09-20 00:28:38 -07:00
parent 81faf597ce
commit aab9b0fb21
5 changed files with 109 additions and 22 deletions
+23 -2
View File
@@ -329,8 +329,29 @@ fn the_two_front_ends_draw_the_same_column_header() {
answering Q1 (see `AxisFrame::group_of`). That is `GridLayout`'s \
rule and the TUI paints the same span; it is reproduced here rather \
than corrected, because correcting it changes what the TUI draws. \
When it is fixed this line becomes [(Some(\"Q1\"), 4), (None, 2)] \
and this is the test that says so."
See `THE_MEMBERSHIP_CORRECT_SPANS` for what improvise-gd83 makes \
this line say."
);
// Spelled out rather than guessed, because a wrong future expectation is
// worse than none: whoever picks up improvise-gd83 will read this and
// believe it. The frame here is NOT one Q1 run — `_Measure` is the outer
// level, so the column axis is:
//
// col_cats = ["_Measure", "Month"]
// Group(Q1) [Revenue,Jan] [Revenue,Feb] [Revenue,Sep]
// Group(Q1) [Cost,Jan] [Cost,Feb] [Cost,Sep]
//
// Two Q1 headers, one per measure, and the ungrouped `Sep` sits *inside*
// each run rather than after both. So membership-correct spans are four
// runs, not two — and they do not coalesce across the second header,
// because `Sep` breaks the run between them.
const THE_MEMBERSHIP_CORRECT_SPANS: [(Option<&str>, usize); 4] =
[(Some("Q1"), 2), (None, 1), (Some("Q1"), 2), (None, 1)];
assert_ne!(
named.as_slice(),
THE_MEMBERSHIP_CORRECT_SPANS.as_slice(),
"when this stops differing, improvise-gd83 is fixed and the \
assertion above should become the constant"
);
// And the group line the TUI paints names the same group.