1. THE JMJZ WIDTH ASSERTION COULD NOT SEE A WINDOWED WIDTH, and the evidence
said it could. Injecting `if drawn { widen(..) }` -- widths from the window only
-- left 1476 passing, 0 failing. Two reasons, the second structural: the measure
was named `Revenue` (7 chars) against a value of `300021` (6), so the assertion
passed on the header alone; and comparing grid_body's widths against
compute_col_widths is INCAPABLE of testing windowing, because compute_col_widths
IS grid_body with an empty window. Windowing degrades both sides identically.
The same function twice is not a check, and it reads exactly like one.
Fixture is now the reviewer's shape -- a total that cancels and a one-character
header -- reordered once more so BOTH assertions discriminate: with the large
values last, the windowed row and the cancelled total were both 1.0, so the
totals assertion went blind to the row-window fault while the widths assertion
happened to catch it. Large values first:
for ri in rows.clone() -> totals [888888888.0] against [1.0]
if drawn { widen(..) } -> widths [10] against [11]
10 rather than 5 is a smaller gap and a better witness: the missing character is
the minus sign of a row outside the window.
2. THE T6DB none_cats PARAGRAPH REPORTED A NEGATIVE RESULT FROM A SETUP THAT
LACKED THE CONDITION -- and correcting it found a P2. The fixture was said to put
Product on Axis::None in one view and on Column in another. It did not:
add_category defaults to Column and nothing moved it, so all three views had
none_cats {_Index, _Dim} and the stated input to the hazard was absent.
Setting it explicitly failed the property on the first run, and not on a cell
value: Second loses BOTH Total columns when the host stands on Default.
rebuild_layout fills Model::formula_cache for the HOST's view, and grid_in on the
elsewhere path builds another view's layout against that stale cache -- the Total
cells are keyed without a Product coordinate, the other view asks for one, finds
nothing, and prune_empty drops the columns. Filed improvise-ac2z (P2). Not local:
grid_in takes &App and recompute_formulas needs &mut Model.
Guarded by a_hosts_none_cats_leak_into_another_views_formula_columns, a
characterization test whose message says that failing means the bug is fixed. The
property skips pairs whose none_cats differ and asserts compared > 0, so a future
fixture cannot skip everything and pass.
3. Smaller, all taken. "Nothing else fails" on the t6db injection was wrong: it
fails three tests, including the pre-existing prune-invalidation one -- the #78
fix had more coverage than it was credited with. And the 5ubs should_panic named
one thing and exercised another: pointed at a struct it reached only the
split_once branch, and all three panics shared one literal so `expected` could not
discriminate. The parse is now a `parse_expected` seam fed hand-written mutated
messages, each refusal with its own wording, plus a happy path.
1470 -> 1475 passing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
The lockstep fixtures no longer fabricate a basis, but they do still assume a
cursor at the origin: published_basis asks Sessions::connect for a window at
offsets zero while client_of builds its grid at the cursor's offsets. They
coincide in every fixture here. Recorded rather than closed.
And the keypress path got improvise-jmjz's fix for free: compute_col_widths is
one walk now rather than two, so App::apply_effects' own width computation
roughly halved (134ms to 68ms at 18,000 cells). That is improvise-748f's
held-key path and was not the ticket's subject.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
improvise-6nsd (a prune-by-write renumbers the grid without moving
layout_version) and improvise-26dm (the fourth length-not-completeness list, in
ui/effect.rs). Both read back after bd create.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
9v0j asks for the general relation between view_frame and view_shape, which are
separated by example and never stated as a property. One direction is the known
asymmetry -- AddCategory of an empty category reshapes without reframing. The
other was open, and the ticket guessed "probably yes, and if not, layout_version
under-reports, which per gotcha 35 leaves a client wrong forever".
Probed by driving each ModelCommand through apply_command and reading
cursor_epoch() and layout_version(). Every command in reshaping_commands() moves
both; a ClearCell that prunes a row out of a pruning view moves the cursor epoch
and NOT layout_version. That is improvise-mcq4 working as designed -- a cell
write changes data, not the view's configuration, and view_shape hashes
configuration. So the proposed property test would FAIL and writing it would be
wrong.
What remains open is narrower and stated as a question rather than answered: the
grid has renumbered while layout_version has not moved, so a client still
claiming the old layout tier is accepted. The record tier covers a write to the
pruned record; it says nothing about a write to a surviving one whose row index
moved. Whether that is a hole in ADR 001's middle tier or correctly delegated is
an ADR question.
No code change: view_frame/view_shape live in ui/app.rs, which is another wave's
ground this round.
Also recorded: the probe needed the right fixture to say anything. Against
app_with_two_pruning_views, seven of the twelve commands are no-ops because that
document has no Product category, and a table of false/false rows reads exactly
like a finding.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
grid_of evaluated the whole grid roughly three times per frame:
compute_col_widths walked every cell for its text, then every cell again for the
column totals it must size, and grid_of then evaluated the window a third time.
Every result was discarded at the end of the frame.
ui/viewport.rs::grid_body is now the one walk. It evaluates each cell once and
derives all three answers from that value -- widen the column, add to the
column's sum, and if the position is inside the requested window keep the
CellValue for cell_display to dress. compute_col_widths survives as grid_body
with an empty window, because the keypress path wants widths and no cells, and
two width rules would be the disease improvise-edp names.
MEASURED, orchestrator's machine, cargo bench --bench grid_projection, before
and after:
typical (400 cells) 1.840ms -> 0.687ms
large (9,600) 172.6ms -> 50.0ms
18k (gotcha 41) 294.4ms -> 95.2ms
pathological (90,000) 4.481s -> 1.320s
About 3x, which is the number of passes removed -- the shape the acceptance
criterion asked for rather than a speedup argued from the structure.
Three things the ticket did not predict.
The benchmark's col_widths column now measures something ELSE. It read 45-47%
and now reads 64-82%, which looks like a regression and is the opposite:
compute_col_widths IS the fused walk, so its share rose while the frame fell by
two thirds. A ratio whose denominator moved is not comparable across the change,
and the benchmark now says so.
The windowing ratio did not improve, and that is correct: the pass that remains
is exactly the one a viewport cannot narrow. improvise-i5tk's decline stands and
the two findings are now cleanly separated.
It is not solved, only three times cheaper -- 18k cells is still 95ms, 5.7
frames. The next lever is caching ACROSS frames.
Guard: a_window_narrows_the_cells_and_neither_the_totals_nor_the_widths. Fusing
three consumers into one walk creates a hazard the separate passes did not have,
and the natural "optimisation" (iterate rows rather than 0..row_count) makes a
column total the sum of the visible rows, silently. Watched fail with
left: [1.0] against right: [300021.0]. All 1469 pre-existing tests unchanged,
which is also the evidence the outputs did not move.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
furnished_client and client_ready_to_edit built their BasisCache by walking the
client's own grid for Record cells and calling basis::published(0, 0, records)
with every version hard-coded to zero -- a small re-implementation of
project.rs::basis_update's record selection, in a file whose entire purpose is
to rule out re-implementations. Both now take the Projection::Basis out of a
real Sessions::connect snapshot.
The reason the ticket calls it a nit is the reason it needed a test: a fresh App
really is at generation 0, layout 0 and every record at 0, so the fabrication
and the publication are indistinguishable on every existing fixture -- all 23
lockstep tests pass under either. So the change alone is structural and
untested by construction. the_fixtures_basis_tracks_a_host_whose_layout_has_moved
moves a tier first and then asserts the fixture's claim names the host's current
tiers; re-fabricating the two document tiers fails it (0,0) against (0,1) and
fails nothing else.
Worth recording: SetNumberFormat was the first attempt and does not move
layout_version, although it declares Model(LayoutGeometry). layout_version is
OBSERVED from view_shapes rather than read off the declaration (improvise-pdvm),
and a number format is not part of a view's shape hash. Classification is not a
promise about the counter.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
The move -- diff col_widths against the label-folded cache, as hkt6 already
obliged for totals -- is DECLINED, and the two things learned are worth more
than the change would have been.
THE TICKET'S STATED COST DOES NOT EXIST. It says the move breaks
a_client_on_a_socket_is_seeded_and_gets_its_edit_back, and that that assertion
is gotcha 33(c)'s standing evidence, so rewriting it inside a change 33(c)
guards is the improvise-q0vb / PR #12 shape. Measured by deleting the re-fit
from fit_to_labels: three existing unit tests fail and the socket test is not
one of them. What it actually asserts is that a reframe CARRIED widths -- a
statement about the producer's diff, not about the consumer's re-fit. 33(c) is
triply guarded elsewhere.
THE REAL REASON IS COUPLING. The widths a label-fold invents are each column's
own label width: an approximation the consumer applies until the authority
catches up. Diffing the producer's output against it makes what the server sends
depend on a fallback rule defined in the client -- invisible today, silent the
day a second client implements the fold. The redundant ColWidths per reframe
buys out of that, and over-sending is the direction Dirty::of is deliberately
sloppy in.
Landed anyway: the half the ticket calls "the better test either way".
a_client_never_holds_widths_of_a_different_length_from_its_labels asserts the
invariant 33(c) is really about, at EVERY step of a fold that widens before its
widths arrive and narrows before they shrink -- not merely at the end, which is
where a message-count assertion looks. Watched fail at step 0 with the re-fit
deleted.
The totals/col_widths asymmetry is now stated rather than discovered: totals is
fold-relative because hkt6 found a real hole; col_widths has none, the re-fit
being a safe fallback rather than a stale value.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
group_runs coalesced adjacent positions by group NAME, so two runs under two
different headers sharing a name read as one and both front ends drew a single
span across the boundary.
THE TICKET'S PROPOSED FIX DOES NOT WORK, and that is worth more than the fix.
h0iv suggested comparing the (cat, name) pair, FrameGroup carrying cat for
exactly that reason. But its own reachability case is ONE category -- Month
grouped into Q1 nested under Region, giving North/Q1 and South/Q1 adjacent --
so both headers are (Month, Q1), equal as values, and the pair separates
nothing. A test pins that so the next reader does not re-propose it. What does
separate them is that they are two distinct FrameEntry::Group entries, which
groups() already borrows from self.entries: std::ptr::eq on the header is the
literal statement of "the same header opened both".
THE PRODUCT QUESTION, decided here and reversible in one comparison. Whether
two adjacent same-named runs are one span a user wants spanned is a matter of
taste, which is why #77 filed rather than fixed. What settled it is not the
span but the indicator: the existing wasm-client test merged Month/Q1
(expanded) with Fiscal/Q1 (COLLAPSED) and its own message said the merged span
is drawn with the first one's indicator -- a collapsed group rendered as
expanded, with no affordance saying otherwise. A span boundary is taste; a
wrong collapse glyph is a wrong fact.
That test's justification was also spent: it said the rule must be name-only
"because that is what src/ui/grid.rs uses", and since improvise-8qlr grid.rs
calls group_run_heads. There is no second copy left to agree with, which is the
general point -- a consolidation's frozen behaviour reads as decided afterwards,
and nothing says it was frozen only for the duration of the move.
The agreement fixture gains the case it could not see: two_level_rows minus the
ungrouped Sep, so the two Q1 runs abut, end to end through a real GridLayout and
both renderers.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
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
AppMode::all() and Rejection::all() walk a total cyclic after(), and both doc
comments claimed a new variant could not be omitted. Half true, wrong half:
answering after() FOR a new variant is compiler-forced, pointing an existing
arm AT it is not, so a variant nothing names is never walked -- measured by
the PR #78 reviewer as all().len() == 17 against 18 variants, suite green.
Closing it needs an enumeration independent of after(). Stable Rust has none
for an enum with data -- variant_count is nightly, a derive macro is a new
proc-macro crate, and strum's EnumIter cannot construct AppMode::ItemAdd or
Editing at all. serde can: the Deserialize derive already builds the complete
variant-name list from the enum definition and publishes it in its
"unknown variant ... expected one of ..." message. variants.rs reads that back
and diffs it against the names the cycle reached, each taken off the variant's
own serialization rather than off Debug.
The helper is defensive because an unparsed message would return an empty list,
and an empty list passes every cycle; a should_panic test points it at a struct
and asserts the message, not merely that it panics.
Both overclaiming doc comments replaced. The fourth instance of this shape --
assert_eq!(cases.len(), 13) in ui/effect.rs -- is over a hand-built list of
effects rather than enum variants, so serde has nothing to say about it; left
filed rather than bodged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
PR #78 fixed grid_in to build a non-active view with with_frozen_records
rather than GridLayout::new, which skips prune_empty, and landed with no
test of its own. This is the general property: which view the host stands
on is not an input to any client's grid.
The fixture is the load-bearing part. The reviewer's first formulation made
the two views differ by turning prune_empty OFF in the second, which passed
on the buggy tree -- with nothing to prune, both constructors agree. So the
fixture gives one region no cell at all, leaves all three views pruning, and
asserts every view really is two rows before comparing anything.
Widened while writing it: an aggregate formula, so the views also differ in
none_cats, which grid_in likewise does not rebuild for the elsewhere path.
That asymmetry is argued and NOT observed -- see context/test-evidence.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
improvise-864a diagnosed over all 100 push-to-main runs rather than the three
the ticket named — and the three-run framing was mine, a sampling artefact
that made a two-day, sixteen-failure pattern look like a concurrency race
between queued deploys. 12/12 artifact-not-found failures had another run's
SUCCESSFUL deploy complete inside their own upload-to-deploy window, against
2/56 successes.
The decisive observation was a negative one already sitting in a log: a run
seeing its own harness-serve and wasm-bundles but not its own github-pages.
Same run, same token, same call, same retention — which kills flakiness,
eventual consistency, retention, token scope and a v3/v4 mismatch together.
One clause of the ticket's own correction is falsified: PR #46's guard aimed
at the right quantity for the wrong reason. It still must not ship, because
the association is not a mechanism and one run survived a foreign deploy in
its own window.
improvise-ijql closed additive. The author argued against the precedent I
pointed it at and was right: a provenance line qualifies an answer the run
gave, a disclaimer qualifies one it did not, and exit 0 is what CI branches
on. The rule is now stated over the exit code.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Gotcha 24 rewritten. Since improvise-ni6k the cursor is invalidated by
OBSERVING the grid's frame in apply_effects, inside the loop; no effect calls
reset_cursor at all, where this paragraph said thirteen did two days ago and
two before that. Total over EffectKind::Model(_) by construction rather than
by a maintained list — but totality is conditional on classification, and
nothing makes classification total.
Records the two-arm asymmetry and why it exists: narrowing the CellData arm
to one view measured as free and silently lost a multi-client invalidation.
The absolute is stated beside the ratio.
Two beads filed from the re-review, neither blocking the merge:
improvise-iot6 (P2) — render_csv builds GridLayout::new, so CSV export writes
rows the view prunes. FOURTH instance of that trap and the first in
production a user can see; reachable from :export and the CLI, and none of
export.rs's three tests mentions pruning, which is why it has been shipping.
improvise-9c9o (P2) — a geometry effect immediately after a mid-batch
SwitchView skips the check entirely. Passes on pre-PR main and on round 1,
fails at the merged head. Under-reporting, which gotcha 35 says is the
expensive direction.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Two findings from running /code-review on this branch.
The dispatch self-test's dirty arm asserted the run failed and named `f.rs`,
which is adjacent to the question -- a failure for any other reason naming that
file would have satisfied it. It now requires the cap's own message as well,
so the assertion names the question it means.
And .claude/skills/code-review/SKILL.md still told reviewers the flag "selects
rather than adds, so --check-evidence never runs the comment cap", which this
branch makes false. Grepped for every copy of the claim; that was the only one
outside context/. A doc that describes the script it gates is code, and leaving
it is the gotcha-48 failure exactly.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
No code change; the ticket stays open, now with numbers instead of an estimate.
Still latent, re-verified 2026-09-21: 1444 test names, zero duplicate full
paths, zero duplicate (crate, path) pairs.
The proposed fix -- `--list` per package with the crate name prefixed -- is
declined on two independent grounds. It costs 45.2s against 2.4s for the one
workspace call (11 members, warm), and --check-evidence needs the --ignored
twin, so the honest figure is 22 cargo invocations for a case nothing can reach.
And it does not close the class it names: a package is not a binary. `improvise`
carries five integration test binaries beside its lib, and an integration test's
top-level fn lists with no module path at all -- 60 of the 1444 names are bare
-- so `-p improvise` prefixes all six identically and tests/a.rs::foo still
cannot be told from tests/b.rs::foo. Any fix worth having is per-target
(--lib, --test <name>), not per-package.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
No code fix. The ticket asked that any fix first establish why two of three
green uploads had no artifact at deploy time, and the answer is neither of the
two directions it offered.
Measured over all 100 push-to-main CI runs from 2026-09-19T22:02Z to
2026-09-21T05:02Z, via the run/job/step/log APIs -- job metadata and logs
outlive artifacts, which expire in a day and are all 404 now, so this is
reproducible where the ticket's own artifact-count table no longer is:
12 artifact-not-found failures -- 12 (100%) had another run's SUCCESSFUL
deploy complete inside their own upload->deploy window
56 successes -- 2 (3.6%) did
4 other failures -- 0; all four are the unrelated "Create Pages
site failed: Resource not accessible by integration", pre-improvise-3gy
Direction: the oldest run of the 17:47Z batch failed at 17:47:23Z, before the
next deploy in the batch started (17:47:31Z) and before any Pages deployment
existed at all, since deploy-pages fetches artifact metadata BEFORE creating a
deployment. So it cannot have been caused by its own deploy, by supersession or
by a concurrent one. The artifact was already gone.
Scoped by artifact NAME, not by run: run 35560726456 finalized harness-serve,
wasm-bundles and github-pages seconds apart, and 2m21s later its own lookup
reported "Found 2 artifact(s)" with no github-pages. Same run, same token, same
call, same retention -- which rules out service flakiness, eventual consistency,
retention, token scope and a v3/v4 backend mismatch in one observation, since
any of them would have taken the siblings too.
One clause of the ticket's own correction is falsified: 52507a6 did not fail
"anyway" -- 5f871f7's deploy completed four seconds before its own began. Fewer
successful deploys is the right lever, so PR #46 aimed at the right quantity for
the wrong reason. It still should not ship: the association is not a mechanism
and the mechanism is not deterministic (5f871f7 had a foreign deploy inside its
own window and survived), so shipping it would be exactly
doctrine(measure_the_obvious_fix_before_reporting_it_as_the_fix).
Also corrects the pages-deploy comment in ci.yml, which stated the superseded-run
mechanism this evidence refutes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
scripts/check-comment-runs.py early-returned on args.check_evidence, so the
flagged run answered the evidence question and said nothing at all about the
comment-run cap. The flagged form is the expensive one -- it needs a compiled
workspace -- which makes it the one a human runs alone; three agents hit this
independently in one campaign, and in one case a local gate went green while
CI went red on a three-line doc comment (improvise-ijql).
The cap moves into check_cap(), and --check-evidence now runs it first and
returns the worse of the two verdicts. The ticket's cheaper alternative -- keep
the early return and print a line saying the cap did not run -- is declined and
the reason is recorded: a provenance line qualifies an answer the run gave,
where a disclaimer qualifies one it did not, while still exiting 0. Exit 0 is
what CI branches on and what a human reads as clean, so the rule is stated over
the exit code: a tree with a long comment run must not pass any invocation that
can exit 0.
The guard is _dispatch_self_test, which runs the script as a subprocess against
a temporary tree -- the only level at which this is visible, since every unit in
the file was correct and the defect was in main()'s dispatch. Both arms were
demonstrated by injection: restoring the early return fails
"DISPATCH: --check-evidence: exited 0 over a four-line comment run", and forcing
the cap to always report a violation fails the clean arm for both invocations,
which is what stops an always-fail from satisfying the first.
Measured: the cap adds 0.29s to a 6.5s --check-evidence run.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
The ticket blamed an unbounded grammar walk. Instrumenting the halves
separately: load_grammar() x1000 = 2.50s against a 15.7ms walk, so the walk
was 0.6% of the cost and improv_file() was re-parsing improv.pest once per
generated case. A OnceLock took improvise-io's lib from 1.42s to 0.21s with
the strategy's input space unchanged; constraining it would have moved 15.7ms
and shipped as the fix.
Re-measured here rather than taking the hand-back's figure, which differed:
1445 tests over 34 binaries, longest single binary 0.6-1.0s across two runs on
one machine. That spread, on one machine in one sitting, is its own argument
for reading your own run.
Also corrects the repo map's bug-fix workflow line, which said to document a
bug in the test's doc comment — the two-line comment cap forbids exactly that
and design-principles §7 says the opposite. A fact stated twice, changed by an
edit rather than a grep.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Review findings on this branch, all three in the gp commit.
The prose was the worst of them: §11 called Artefact::NumericCategory a
permanent refusal that "no vocabulary closes". It is not. fit_measure hands
preprocess an Overrides::default(), so it never says which header columns are
categories — although the next line tells Features::of — and preprocess then
infers Numeric -> Date -> Categorical from the values. A `Year` category with
items 2024/2025 becomes a numeric column and is refused, with `Year IS 2024`
spellable all along. Forced::Categorical is the existing knob. Filed as
improvise-zv8t. Prose that calls something permanent is what stops the ticket
being filed, which makes this the branch's own defect class one level up.
MissingFlag's words said "that measure", but its arm is not conditioned on
is_category and dataset.rs writes "" for an absent coordinate, so a .missing
flag over a category is reachable — the same "declined in words naming
something they are not" the commit was written to fix. Now "that column".
ValueOfAMeasure's tail likewise covered only the present case.
The test module claimed "each artefact family" while its fixture reaches three
of five; named for what it reaches. Re-injected the collapsed-reason fault
after the reword: still fails, so the negative clauses still bite.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Merge of `gh/main` at 8bb1d53 (#83), plus the four findings from the adversarial
review of #78. The P1 is the one that matters, and it is mine.
1 (P1). improvise-mcq4 narrowed the cursor's frame fingerprint from every view
to the active one. I measured that the narrowing moved 12.41ms to 12.56ms, kept
it "as the honest shape", and reported that it bought nothing. It bought a
multi-client invalidation: a `CellData` write that prunes a row out of ANOTHER
client's view became invisible, because `view_shapes` hashes the `prune_empty`
FLAG rather than which rows survive. Gotcha 24's permanently-stale case.
**A measurement showing a change is free is not evidence that it is correct.**
I asked what it cost and reported the answer as though it had also answered
whether it was right.
The arms are now split by the thing that differs: `LayoutGeometry` keeps the
cheap one-view frame (`view_mut()` writes only the active view, and the
structural cases are already covered by `view_shapes`); `CellData` restores the
all-views frame. Measured here, not cited: **0%** on the held-key path, **+5.7%**
on an 18k two-view `SetCell`, scaling linearly in view count. Trade accepted and
recorded on improvise-mcq4 along with the absolute — this PR still roughly
doubles held-key latency on large pruned documents.
**Writing that regression test found the bug that was hiding it.** The
precondition failed: `project::grid_in` built a non-active view with
`GridLayout::new`, which does not apply `prune_empty`, so a bystander was
projected three rows where its own config gives two. Its grid never lost a row,
so the staleness was unobservable — a bug hiding a bug, and the THIRD instance of
the `new`-versus-`with_frozen_records` trap. Fixed; broke no other test. Filed as
improvise-t6db because the finding outlives the one-line fix.
2 (P2). `frame_basis` was taken once at batch open and compared across a
`SwitchView`, which is `ViewOnly` and so did not re-base it — the next model
effect hashed the new view against the old view's value. It is keyed by view name
now and re-bases when the name moves.
3 (P2). The `drill_state.is_none()` guard on the free opening basis was correct,
load-bearing, and tested by nothing — reverting it left all tests green. It has
exactly one test now, and removing the guard fails that test and nothing else.
4 (P3). `assert_eq!(cases.len(), 13)` claimed a totality it does not have; the
reviewer added a fourteenth effect and the suite stayed green. The comment now
says it is a count. Filed under improvise-5ubs as a fourth instance — inside the
PR that found improvise-5ubs.
THREE CORRECTIONS TO MY OWN REPORTING, all understatements:
- Disabling the observation fails **13** tests, not the ten I claimed (the review
said 12; the P1 probe adds one).
- Moving the check after the loop: with the arms now split, moving the
`LayoutGeometry` arm alone fails 2; the review measured 4 against the unified
arm, and both are more than the three I named.
- `4c55341`'s subject said "Merge #81" and it was single-parent; the real merge is
`ec236da`. Subject corrected by amend.
1456 tests. Both spellings of the comment-run script run separately. Test function
names diffed by name across the merge including `harness/`: nothing lost.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
improvise-669b. PR #65 single-homed HANDSHAKE_TIMEOUT_MS in web/shell.js, but
the home is single in the TREE: the spec imports web/shell.js and the page
loads harness/serve/app/shell.js. createShell now exposes timeoutMs() beside
timesArmed(), and outlastTheWatchdog requires the running page's value to equal
the one it imported. Injected with the ticket's recipe (8000 -> 100 in source,
no rebuild): 5 passed / 1 failed where it was previously 6/6 green.
Also in scope, the same file's stale line: HANDSHAKE_TIMEOUT_MS restates
nothing and one test outlasts it, not two.
improvise-qf57. The formula-row click's `#grid input` toHaveCount(0) stays
dropped, and now deliberately. The reviewer's argument was that the exact
`#mode` assertion subsumes it; measuring gives a better one. app.js creates a
grid <input> in exactly one place, the grid cell's own mousedown, so no panel
handler can reach it — and an invented formula-row activation opens the
MINIBUFFER, not a grid editor. Injecting ChangeMode(formula_edit()) into
click_formula_row: the toHaveCount(0) probe PASSES and the run fails on the
#mode assertion below it. Restoring it would add an assertion that cannot
fail, which context/test-evidence/clients.md already records as the reason a
sibling assert! was removed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Merge of `gh/main` at 0947fd0. No conflicts: improvise-5jm5's `Aggregation`
enum reaches `ui/project.rs` at three lines (one import, two call sites) and
none of them is one this branch touches. Re-checked here rather than trusted,
since `project.rs` was being reworked for improvise-mcq4 after that check was
made; the mcq4 code is in `ui/app.rs` and does not overlap.
Two things beyond the merge.
**The benchmark fixture now says not to simplify it.** `prune_empty` is the
only thing that makes the frame fingerprint walk cells, so a geometry case with
it off measures the cheap half — 17–176µs against the real 5.75ms — and reports
that as the cost. It is the reason improvise-mcq4 was nearly missed, and it is
the first thing a later reader would "tidy". The comment names the file that
carries the argument.
**`context/code/command.md` carries the fixed numbers and, more usefully, the
fact that the obvious fix bought nothing.** Narrowing the fingerprint from every
view to the active one was the ticket's first option and my own suggestion, and
it moved 12.41ms to 12.56ms, because a second view is typically fresh with
everything on `Axis::None` and costs nothing to build. That is written down,
because the next person to look at this will have the same idea.
Gate note: `--check-evidence` does not run the comment-run cap — they are two
modes of one script. Both are run separately here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Both from the adversarial review of PR #78. The first is the sharpest finding
of the campaign: improvise-mcq4's narrowing had already been measured as
buying nothing (12.41ms to 12.56ms) and was kept only as 'the honest shape' —
and it turned out to cost a multi-client invalidation no test named. A change
with no measured benefit carried an unmeasured cost.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Facts from PR #83, including two that replace text I had not yet applied —
the earlier phrasing said evidence names are "leaf names", which the merged
work makes wrong.
The finding worth carrying is one the review did not contain and the author
found while fixing it: NEITHER HALF SUFFICES. Refusing an ambiguous leaf does
not survive a rename — rename one of a colliding pair and the leaf is unique
again and resolves silently to the survivor. Qualification is the fix; the
refusal is only what forces someone to write it. Verified here independently:
un-qualifying the cited heading exits 1 naming both candidates, restoring it
exits 0.
improvise-ijql is NOT closed. The docs half is done, but line 1010 still
early-returns on the flag, so --check-evidence still selects rather than adds
and still says nothing about the cap it skipped. Recorded on the ticket with
the two candidate shapes.
Filed improvise-sxsf: `cargo test -- --list` does not name the binary, so two
crates sharing a module path remain indistinguishable. Not reachable today,
not guarded, and "verified latent" ages the moment anyone adds a crate.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
improvise-wd6n. Spelling::Artefact was one variant standing for five distinct
families, the last five reached through a `_ =>` wildcard in spell(). The one
refusal they fed said the tree "splits on a col.missing flag or a date part",
so three of the five were declined in words naming something they are not.
Artefact now names which family, spell() is exhaustive (a sixth FeatureRole is
a compile error rather than a sixth thing that message misdescribes), and the
refusal gives one clause per family.
The two families the ticket asks about are declined in writing in
context/code/gp.md §11, separately, because each needs different vocabulary:
col.missing needs a word for absence that reaches apply_agg and eval_agg, and
the date parts need a date type improvise does not have anywhere. Nothing new
is emitted, so improvise-4zl7's route-then-aggregate rule is untouched.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
improvise-8shu. improvise-io's lib binary was the run's outlier at 1.42s
against a 0.01s median. The ticket's diagnosis was an unbounded grammar walk
generating deep documents; measuring the two halves separately falsified it.
load_grammar() x1000 was 2.50s and the walk x1000 was 15.7ms — improv_file()
called load_grammar() inside its prop_map, so pest_meta re-parsed the grammar
once per generated case.
The strategy's input space is unchanged. The grammar is read once through a
OnceLock, and a guard counts parses rather than calls so it is independent of
which test in the binary runs first. Case counts go 500 -> 256, the default
design-principles §7 prescribes and never below it.
1.42s -> 0.21s; improvise-io is no longer the run's longest binary.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
improvise-ahdf. Gotchas 36 and 43 named Sessions::apply as the production
door. It is not pub, it delegates to apply_checked after Checked::against,
and its only call sites are tests — Host calls apply_checked. Verified by
reading the signatures rather than the prose: apply at project.rs:878 has no
pub, apply_checked at :887 does. design-principles.md §5 already said
apply_checked, so the repo map was the stale copy of a fact stated twice.
Two of the four remaining mentions were live claims and are corrected; one is
historical ("demonstrated against ... before removal") and stands, and one is
the new sentence saying apply is not pub.
Also closes 26 tickets whose fixes are in merged main, each verified against
the code or against an evidence entry rather than against a PR body. Left
open deliberately: h074 (reported half-done), s17o (a decision in #78's
ground), mcq4 (fixed to 1.9x, residual documented), lltt (P1, detector only).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Both blockers from the adversarial review of #83, both in the mode this PR
rewrote, both producing a pass while the gate did not do what its prose said.
1. cargo_test_names reduced every listed path to its last segment. Six leaf
names collide in this workspace and two are cited in evidence, so a heading was
satisfiable by a test in another module answering another question -- renaming
ui::project::tests::a_load_that_fails_is_not_a_rebase, the one its entry
describes, left the checker at exit 0 with the ui::host namesake answering for
it. Paths are kept whole; a bare leaf resolves only when unique, and an
ambiguous one is REFUSED with both candidates named. _RUST_NAME accepts `::`, so
a qualified heading is a claim rather than prose, and a citation is matched on
the leaf so qualifying a heading cannot orphan its own test. The two colliding
entries are qualified. Neither half suffices alone: rename one of a colliding
pair and the leaf becomes unique again, which is why qualification is the fix
and the refusal is what forces it.
2. --rust-tests called cargo_test_names with one argument, so the --ignored
subtraction never ran, and it printed a byte-identical success line. It now
requires --rust-tests-ignored (one alone exits 2) and the line names its source.
Also, from the same review: a missing context/test-evidence/ exited 0 while 170
citations pointed at nothing (early return, now gone); the doc-site "runs last"
check accepted a trailing `echo` that merely NAMED the generator, so it asks for
an invocation; bundle_freedom gains an arm for a transitive `import "../app.js"`,
which reaches the bundle while naming nothing forbidden; and two prose overclaims
are corrected -- the prose-heading count is a convenience rather than a guard
(git diff is the mechanism), and the ratio rule says "above the run MEDIAN",
which is defensible where "above the others" is false against second place.
scripts/check-ci-graph.py makes claim 6's rule a gate: pages-deploy must `needs`
every other job in ci.yml. The rule was stated in the last commit and a
zz-new-check: job added during review was still ungated with nothing noticing.
README now states the rule and lists no jobs, the second copy being what went
stale twice.
Every fault re-injected and observed; figures in context/test-evidence/tooling.md
rather than only here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
improvise-mcq4's obvious narrowing — fingerprint the active view rather than
every view — was the ticket's first option, the orchestrator's suggestion and
the author's plan, and moved 12.41ms to 12.56ms. Nothing. The per-view
multiplier was never the cost; two expensive builds were, and the bench's
second view was a fresh one costing nothing to build. It would have shipped as
the fix had the author not re-measured after implementing. A fix three people
agree is obvious has had its plausibility checked and not its effect.
And the merge note: when two branches replace one mechanism with two different
ones, the dangerous file is the one that does NOT conflict. app.rs conflicted
loudly and was resolved by hand; effect.rs auto-merged clean and left both
cursor mechanisms live, 1435 tests green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Gotcha 49 records improvise-kc7n: a websocket handshake is exempt from the
same-origin policy, so no network control can see cross-site hijacking — an
IP allow-list, a private network and loopback binding all answer "may this
client reach the endpoint" and the honest answer is yes. Loopback is the
least protected case, not the safest. Records why all three arms are decided
rather than defaulted, and that none of it is authentication.
The File Inventory line counts were audited: 26 of 61 resolvable entries had
drifted past 10%, several by a third. Refreshed, and relabelled as a dated
snapshot with the method, because false precision in a carefully-checked
document is worse than an obvious estimate. Two traps found while doing it,
both recorded: a path-walking refresh resolves a bare name like lib.rs
against the first matching crate and rewrites the wrong number, and the
ws-server/ entries use a shorthand prefix no such walk resolves at all.
The code-review skill's gate list now runs BOTH spellings of
check-comment-runs.py. The flag selects rather than adds, so --check-evidence
never runs the comment cap and does not say so; three separate agents hit it
in one campaign (improvise-ijql).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Merge of `gh/main` at 23f6def (#79, #80, #82) together with the fix for the
regression this branch measured and filed. One conflict, in
`context/test-evidence/clients.md`, where both sides appended a section: both
kept.
improvise-mcq4. The observation was building every view's pruned layout, before
and after every model effect, and `with_frozen_records` applies `prune_empty`,
which walks cells. Two changes, and the first one is the one that mattered — not
the one that looked obvious:
- **Narrowing to the active view bought nothing.** It was the coordinator's
suggestion and mine, and measuring it is what showed why: the bench's second
view is a fresh one with everything on `Axis::None`, so it costs nothing to
build. The per-view multiplier was never the cost; two *expensive* builds were.
Kept anyway — `active_view_frame` is the honest shape, and the audience half is
now the config diff `observe_reshape` already computes for free, which
over-reports in the safe direction and is read only behind the epoch gate.
- **The opening basis is free.** At batch start `self.layout` already IS the
active view's pruned layout, so `opening_view_frame` hashes it instead of
building a second one. `frame_of` is the shared hash; `view_frame` builds then
calls it. Guarded on `drill_state.is_none()`, because a drill freezes the record
list and the held layout is then not what `view_frame` would build. Opening use
only — `self.layout` is stale mid-batch (gotcha 23).
MEASURED, one `CycleAxis` batch, `prune_empty` ON, against the same fixtures:
before fix after fix baseline
400 cells 283.2us 167.0us 93.6us
9,600 cells 7.34ms 3.879ms 1.967ms
18,000 cells 12.41ms 5.752ms 3.043ms
90,000 cells 91.69ms 56.54ms 28.52ms
4.1x down to 1.9x at the canonical size, and back inside the 16.7ms frame budget
where it had been outside it. What remains is one pruned build per model effect,
which is inherent to observing a pruned frame; the last one duplicates
`rebuild_layout`'s work and could be elided only by moving the check after it,
which is the position that clobbers a deliberate cursor placement in the same
batch. Not done, and improvise-mcq4 carries the residual.
BOTH CONSTRAINTS RE-RUN AFTER THE CHANGE, because narrowing is exactly what could
quietly stop covering things:
- Subsumption: #76's own `a_page_flip_under_prune_empty_invalidates_the_cursor`
and its negative twin both still pass with no per-effect reset anywhere.
- Breadth: `AddFormula`/`RemoveFormula` and `SortData` still covered.
- Disabling the narrowed observation fails ten tests, so it still does the work.
- `a_reshape_of_one_view_leaves_a_client_on_another_view_alone` still passes, so
the cheap audience set has not widened into nynk's negative arm.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
CI's `check-comment-runs.py` runs with no flags and the local gate in my brief
runs it with `--check-evidence`, which reports the evidence audit and not the
run cap — so a three-line clap doc comment passed locally and failed in CI.
Run both spellings.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Edward Langley