When source records collide on every coordinate, the import wizard now asks:
sum them, keep them separate with a generated occurrence category, or go
back. Collisions are found before the build by the same placement pass the
build uses, and are carried to the TUI as structured data (WizardCache) and
listed in full, scrollable. At the 12-category cap, "keep separate" is shown
disabled with its reason and remedy.
Every wizard warning and message now wraps to the panel instead of being
truncated, and long lists scroll.
Headless paths no longer sum silently: `import --no-wizard` takes
`--on-duplicate refuse|sum|separate` (default refuse), and `:import-json`
refuses and points at `:import`.
Reasoning: context/code/formula-and-io.md, "Colliding records are a
question, not a default"; context/code/tui.md; context/code/cli.md §11.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
Every write the merged PRs added now names a view, chosen deliberately:
- The keyboard commit names OpenedCell::view, captured when the editor
opened, as EditTarget does for the mouse (context/code/clients.md).
- RenameRecordCoordinate names no view. Its record must hold cells, so it
resolves in the store alone: NamedRecord::Stored, as does the rename
destination records_at_stake adds. named_records replaces cells_named as
the exhaustive list records_named is read off.
- #252/#265's non_item_refusal moves into Grids::unwritable. The strict view
lookup (a missing view is RecordGone) and the Grids memo are kept.
- Test fixtures name a view their document has: Default for host() and
host_on_a_fold(), the records view for the rename pair, and the viewing
client's own view in browser_round_trip.
Evidence files: both sides kept, every heading of both parents verified
present. Test names: the merge holds every test of both parents.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
Conflicts:
- cmd/core.rs committable_cell_key: took main's form, which asks
`uneditable` (#277), and gave `uneditable` a declining NonItem arm.
That keeps this branch's refusal of a commit on a non-item cell.
- resolve.rs reopened_at (auto-merged, non-exhaustive after the merge):
a declining NonItem arm, the sentence `i` already says.
The #277 review asked for both arms to decline, never to open.
- tests/shipped_documents_load.rs: kept both, this branch's
every_record_a_shipped_document_projects_is_writable and main's git()
helper.
- context/test-evidence/{command,core}.md: both sides appended at EOF; kept
both, the branch's first.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
GIT_CONFIG_GLOBAL=/dev/null and GIT_CONFIG_NOSYSTEM=1 in run_git were
witnessed by a config excludesFile, which vwjj's -c
core.excludesFile=/dev/null outranks. So dropping either line passed
every test from vwjj onwards.
The hostile child's config, read as both GIT_CONFIG_GLOBAL and
GIT_CONFIG_SYSTEM, now also sets core.sharedRepository = nonsense.
Measured on git 2.55: git init and git add die on it, git ls-files
does not read it (so the walk, which does not null the user's config,
is unaffected), and run_git passes no -c for it. init.defaultObjectFormat
and init.defaultRefFormat were tried first; they only warn.
Dropping either env line is now red. Every other rival in the combined
test is unchanged, and there is still one child per test.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
tests/shipped_documents_load.rs re-ran itself in 18 child processes, one
per hostile setting, and each child spawns several gits. So the binary
took 2.1-3.3s against design-principles 7's 2s-per-binary tripwire.
The six per-setting tests (53zz x2, vwjj, pr3i, a0i3 and 2iws/vav0) are
now one test, a_hostile_git_environment_changes_nothing. It has two
children: the gzip shape test and every_shipped_document_loads. Each
carries every setting at once:
- the template dir;
- the global and system config (with trace2 targets);
- XDG_CONFIG_HOME;
- GIT_DIR and GIT_INDEX_FILE;
- the five pathspec and attribute settings;
- the six trace variables.
Afterwards the test asserts the outer index is still empty.
Each setting breaks the walk on its own, so a rival that drops one
defence leaves that setting live, and the child fails. Every rival that
was red against the per-setting tests is red against the combined one.
The rivals that were green before are green now:
- ICASE;
- the trace2 names under the 0 override;
- the GIT_CONFIG_GLOBAL and NOSYSTEM nulls, whose excludes witness vwjj's
-c override masks. That last pair is filed as improvise-3j70.
Measured on an Apple M4 Max, three workspace runs each: this binary went
from 2.10-3.31s (load 37-39) to 0.57-0.91s (load 30-33). It is now
below the next-largest binary in every run.
The evidence sections keep their reasoning, under prose headings.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
The review of #276 found that an exact list of the three names the test
set passed it. The child now sets all six variables that write to their
target on git 2.55: GIT_TRACE, _SETUP, _PERFORMANCE, 2, 2_EVENT and
2_PERF. An exact three-name list leaves GIT_TRACE_SETUP behind and
fails.
vav0: a trace2.*Target in user config is not an environment variable,
so removal cannot reach it, and the walk's git does not null the user's
config. untraced_git now also sets GIT_TRACE2, GIT_TRACE2_EVENT and
GIT_TRACE2_PERF to 0, which outranks the config. The child adds a
GIT_CONFIG_GLOBAL whose [trace2] section sends all three targets to
/dev/stdout.
Rivals, each of which fails:
- no strip;
- the exact three names;
- the prefix minus GIT_TRACE, minus _SETUP, or minus _PERFORMANCE;
- no 0 override.
The evidence entry moves from the web_demo_model section to the
shipped_documents_load one.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
GIT_TRACE, GIT_TRACE2_EVENT and GIT_TRACE_PERFORMANCE are process-wide,
so --local-env-vars lists none of them. Pointed at /dev/stdout, git
wrote trace lines into the ls-files -z output, and the walk read them
as paths. git() now builds on untraced_git(), which removes every
variable starting with GIT_TRACE: a prefix, because the family is
open-ended. The rev-parse --local-env-vars call goes through it too,
since its stdout is parsed as well.
the_walk_ignores_a_git_trace_sent_to_stdout reruns the gzip shape test
and every_shipped_document_loads in children with all three set to
/dev/stdout. It failed at #269's head. Rivals, each of which fails it:
- no strip;
- the exact name only;
- keep GIT_TRACE_PERFORMANCE;
- keep GIT_TRACE2_EVENT.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
GIT_LITERAL/GLOB/NOGLOB/ICASE_PATHSPECS and GIT_ATTR_SOURCE are
process-wide, so --local-env-vars names none of them. Remove them in
git() beside the local-vars loop.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
improvise-pr3i, found reviewing #255. git init copies GIT_TEMPLATE_DIR
into the new .git. That variable is not in --local-env-vars, and it
bypasses config, so a template whose info/exclude holds *.gz still broke
a_gzipped_document_is_walked at git add. scratch_repo now runs
`git init -q --template=`, which copies no template at all, including a
distribution's compiled-in default. Stripping the variable would still
copy that default.
Also: rerun_with_env's callers print the child's stderr as well as its
stdout (told), because the child's panic goes to stderr.
Stacked on #255 (fix/vwjj-default-excludes).
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
improvise-vwjj, found reviewing #248. GIT_CONFIG_GLOBAL=/dev/null empties
the config files. It does not stop git falling back to
$XDG_CONFIG_HOME/git/ignore (or ~/.config/git/ignore) when
core.excludesFile is unset, and git/attributes has the same kind of
default. So a *.gz line in the user's default ignore file still failed
a_gzipped_document_is_walked.
run_git now passes -c core.excludesFile=/dev/null and
-c core.attributesFile=/dev/null. A -c argument outranks every config
file and every default path.
The new test re-runs the gzip shape test under an XDG_CONFIG_HOME whose
git/ignore holds *.gz, and again with a git/attributes line that breaks
git add. Each half was red on its own.
Stacked on #248 (fix/53zz-scratch-repo-env).
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
improvise-53zz. The scratch-repo shape tests inherited the caller's git
environment. A hook in a linked worktree exports GIT_DIR (and
GIT_INDEX_FILE), which override -C, so `git -C <scratch> add` staged the
scratch files into the outer checkout's index. The user's global
excludesFile applied too, so a `*.gz` line there broke the gzip test.
One helper, git(root), now removes every name that
`git rev-parse --local-env-vars` prints. run_git also sets
GIT_CONFIG_GLOBAL=/dev/null and GIT_CONFIG_NOSYSTEM=1.
Two new tests re-run a shape test in a child process of this binary with
the variables set. Both were red before the fix. Five rivals are recorded
in context/test-evidence/tui.md.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
improvise-6yvg reproduced. A JSON import in improvise-command and a legacy
JSON .improv load in improvise-io stored 0.24815714285714285 as
0.24815714285714283 in each crate's own test build. The root binary's
`improvise import` used the same lossy parser. Now float_roundtrip is on every
normal serde_json dependency in the workspace: root, improvise-command,
improvise-io, and the optional test-support dependency of improvise-protocol.
improvise-dz8c: the hand-listed cargo-tree check over three frame-parsing
crates is replaced by tests/json_numbers.rs. It reads
`cargo metadata --no-deps` and names every package, including target-specific
entries, whose normal serde_json dependency lacks the feature. The rule and why
it beats "parses a frame", the build graph, and enabling the feature once in
the protocol crate are in context/code/protocol.md.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
The headless exit code read `last_batch_aborted()`, which answers "did a chain
stop", not "did the request happen" — wrong both ways. `commit-cell-edit` at
bottom-right lands its write, saves, and then `EnterAdvance`'s `AbortChain`
exited 1 (c5ip); `add-item` into a missing category refuses without aborting
and exited 0 (udt2).
`App` gains a per-batch `batch_refused`, reset beside `batch_aborted` and
written only by `App::refuse(msg)`, which sets the status and the flag
together; commands reach it through `effect::refuse`. `main.rs` exits 1 on
`last_batch_refused()` only. Not a severity on `status_msg`: that would be
portable ViewState and would go stale across its ~60 writers.
Strays into ui/app.rs (four small additions) — flagged in the hand-back.
Unconverted refusals are filed as improvise-i3o5; headless `q` as improvise-ep5o.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
`run_headless_commands` read `crossterm::terminal::size()`, and the page size
decides where `page-scroll` lands, so one script wrote a different record in a
different window — and CI (no tty, 80x24 default) disagreed with a shell. Pin
it to 80x24 in `HEADLESS_EXTENT`. The test drives two sizes through LINES/
COLUMNS with the child detached by `setsid`, and asserts the same record.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
Found by the ChatGPT Codex review of PR #151 at head 64bcb82, and correct.
`clear-cell`, `yank`, `paste` and `drill-into-cell` are registered with
`register`, and their INTERACTIVE arms took `|_args, ctx|`: they discarded the
arguments and used `ctx.cell_key()`. That was harmless while a script only ever
reached the pure arm, which reads the coordinates. The moment the headless loop
re-parsed interactively, `clear-cell Region/East` passed the syntax gate and
then cleared whatever the cursor was sitting on — the origin, `Region/North`.
So this ticket's own lesson has a third instance, inside its own fix: two
construction arms that mean different things demote whichever caller reaches the
weaker one, and here the "weaker" one was the arm that had the context. The four
arms now go through `key_from_args_or_cursor` — explicit coordinates win, the
cursor is the fallback, which is what the keymap path wants anyway since a
binding carries no arguments.
`an_explicit_coordinate_still_beats_the_cursor` writes three cells and clears
the one the cursor is NOT on, so it separates "cleared the named cell" from
"cleared the cursor cell". Restoring the arg-discarding arm fails it.
Evidence: context/test-evidence/cli.md.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
`improvise cmd` and `improvise script` resolved every command through
`CmdRegistry::parse` — the pure arm, which builds `CursorState::default()`: row
0, col 0, and a grid of zero rows and zero columns. The interactive arm, which
builds `CursorState::from_ctx(ctx)`, was never reached from a script. So every
`Move` and `enter-advance` snapshotted a zeroed cursor at PARSE time, clamped to
(0, 0) and emitted `SetSelected(0, 0)` — worse than a no-op, since it RESETS the
cursor. Nothing could position it, so every cursor-reading command was pinned to
the origin.
Reproduced at a9cad15 on a document with Region on row carrying North, South,
East: `move-selection 2 0 . hide-selected-row-item` and a bare
`hide-selected-row-item` both wrote `hidden: Region/North`. After the fix the
first writes `hidden: Region/East`.
The repair is `parse::segments` + `parse::parse_segment_interactive`, with the
headless loop re-parsing per segment, because command n must see command n-1's
cursor. Line atomicity is preserved by keeping the existing whole-line
`parse_line` as a syntax gate, so a line that parsed before still parses and the
only behavioural change is which cursor the commands see.
Two things the ticket did not predict. The TUI's `:` line had the identical
defect — `ExecuteCommand` also used the pure arm — and is fixed the same way.
And doing that surfaced a second instance of the same shape one level down:
`register_pure` built an interactive arm that refused EMPTY args while its own
pure arm accepted them, so `w` with no path parsed one way and refused the
other, breaking `improvise cmd 'w'` and `:w` the moment either re-parsed
interactively. Both arms are now `parse`.
Evidence: context/test-evidence/cli.md; reasoning: context/code/cli.md section 7.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
improvise-udt2 is the ticket context/code/cli.md section 3 points at: a status
message carries no severity, so a refusal that does not abort cannot set a
non-zero exit. It names both disproved classifiers, so the next attempt does
not rediscover them.
And the per-test scratch directory was named `improvise-cli-<tag>`, which
matches the ticket-id shape the AGENTS.md audit greps for. An ID-shaped
non-ID is worse than a dead ID, because the diagnosis is identical and the
repair is not.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
improvise-q914 and improvise-j6x8 are one bug seen twice, and the second
measured it: `run_headless_commands` applied effects and never read
`app.view_state.status_msg`, setting `exit_code` only for parse errors. A
successful command and a refused one therefore produced byte-identical
output — nothing — and exit 0 for both. `improvise script` is the same
function, so it shared the hole exactly.
Three decisions, argued in `context/code/cli.md`:
**Everything to stderr.** The tempting split — stdout for a report, stderr
for a refusal — cannot be built honestly, because `status_msg` is a bare
`String` with no severity and both candidate classifiers are disproved by
code already in the tree. "A refusal aborted" is false: `AddItem` writes its
`Err` to the status and does not abort, and that is q914's own example. "A
refusal is anything that spoke" is false too: `cmd/fit.rs` reports the seed
that reproduces a fit on **success**. Guessing between them is the adjacent
check design-principles §7 names.
**Exit 1 on a parse error or an aborted batch, and no further.** Both are
real flags. A refusal that only writes a status is now printed but still
exits 0, because nothing can classify it; the README says so plainly rather
than over-promising, and closing it needs a severity on `ViewState`, which
is a wire change reaching the browser.
**No quiet flag.** Silence is never the default and `2>/dev/null` already
exists.
Also: a run that changed nothing no longer rewrites the document. Saving
unconditionally normalised it — `1e20` to its twenty digits — which turns a
hand-edited fixture into a silent no-op. The gate accumulates
`model_state.dirty` per command rather than reading it once at the end,
because `effect::Save` clears the flag and `[add-item, w elsewhere]` would
otherwise drop the edit.
And the three clap doc strings advertised JSON, which the parser refuses.
`tests/headless_cli.rs` pins all of it at the process boundary, where an
exit code and a stream exist. Six of its twelve tests fail at bf83294. The
rival worth injecting — print on the happy path, stay silent on the
refusal — fails five of them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
improvise-e0ex. `Margin = Profit / Revenue` drew as `1`: `View::new` seeded
`,.0` — zero decimals — so a 0.66 ratio rounded half-away-from-zero. The cause
is the format, not the arithmetic; exact decimal math would not change the
rendering by one character, so nothing here touches the evaluator and gotcha 8
is unchanged.
Two halves, per the user's ruling:
* `View::new` seeds `,.2`. Revenue draws `18,400.00` and Margin `0.66`. The
noise on whole numbers is the cheaper error — a currency column with two
redundant zeroes is legible, a ratio with none is wrong — and a format cannot
be guessed from a value.
* A formula may carry its own format, which wins for its target; a raw measure
keeps the view's. `formula.pest` gains
`format_clause = ${ ^"FORMAT" ~ sp+ ~ format_spec }`, with `sp+` for `WHERE`'s
reason (a predicate is invisible to the grammar-walking generator) and
`FORMAT` added to `quote::KEYWORDS`.
`improv.pest` is deliberately unchanged: `formula_raw` already carries the
`## Formulas` line verbatim, so the format rides in the formula text with
nothing to add — and the generator, which reads `improv.pest`, sees no new
spelling to violate.
Resolution has one home. `FormatPlan::of(model, view)` replaces the
`(comma, decimals)` pair every render path threaded — `GridLayout::display_text`,
`viewport::grid_body`/`compute_col_widths`, `project::grid_in` and
`persistence/export.rs::render_csv` — rather than being added beside it, which
would be two answers to "what format is this cell". It is asked of the cell's
`CellKey`, so a cell aggregating over `_Measure` keeps the view's format by
construction. `GridBody::col_formats` carries the answer to the totals strip,
which is formatted on its own path.
Persistence is untouched except for metadata: `format_f64` is display,
`persistence/format.rs::format_number` writes round-trip precision, and the two
have no call between them. Newly created views write `format: ,.2`; existing
documents are unaffected, because `format_md` has always written the line
explicitly. `web/demo.improv` moves to `,.2` on both views.
The trap this sets is gotcha 34's: `format_md` writes `Formula::raw`, so a
`FORMAT` suffix round-trips whether or not the walker parsed it. The round-trip
test therefore asserts the parsed field — injecting a walker that drops the
clause fails that one test and nothing else in `improvise-io`, 242 tests
including the whole characterization suite and the grammar properties.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Self-review with the code-review skill, against kb.pl's doctrine. All three are
tests that passed while the thing they named was not pinned.
F1. `doctrine(inject_the_rival_fix_not_only_the_bug)`. The jcfs test used one
twelve-value fixture, and `take(100)` — raising the cap rather than removing it
— passed the ENTIRE suite. That pins a behaviour, not the rule: the rule is that
detection does not depend on how many values there are, and any fixed fixture
pins one side of some cap. It now runs at 11 and 500 leading values, so every
`take(N)` fails one arm. Renamed to say what it asserts.
F2. `doctrine(an_injection_that_does_not_fail_names_an_untested_path)`.
`an_all_null_column_is_a_label` was never proved able to fail. Under the l1f5
fault it PASSES — an all-null column was Label before the fix too — so its
neighbours' injection says nothing about it. Injecting its own fault (dropping
the `!present.is_empty()` conjunct) fails it alone. The rival fix for l1f5 was
also injected: `is_number() || is_null()` over the full slice repairs Revenue
and leaves Region broken, which is why both `all_` bindings had to move and
`a_blank_value_does_not_demote_a_string_column` is the separating input.
F3. `doctrine(an_order_test_needs_two_histories)`. The cell-order test used one
insertion history five times, which cannot tell "listed its own order" from
"emitted one canonical order that matches this fixture". Same six regions now
imported forwards and backwards, each asserting its own. Reaching the second
direction needed the loop re-ordered under the fault, since an injection that
fails at iteration 1 leaves iteration 2 untested.
Also: `duplicate_csv_headers_not_silent` asserted its two values by map
ITERATION ORDER; now by key, which is also what it means.
Four controls were verified able to fail rather than assumed:
an_ordinary_float_is_still_a_number (float branch deleted),
distinct_headers_are_not_renamed_and_warn_nothing (rename every header),
an_all_null_column_is_a_label (F2), and the long-row control.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
improvise-pnf5 (P1). A cell keyed by the empty CellKey is a legal model state
-- what AddRecordRow leaves while a records row is unfinished -- and format_md
wrote it as " = 0" while coord_list required at least one coordinate. Saving
in records mode therefore overwrote a good document with one the application
could not reopen. Given a spelling rather than discarded, because the ticket
is explicit that legitimate data must not be dropped: data_coords makes the
coordinate list optional and the scalar cell is written "= 42". Unambiguous
under PEG, since bare_name cannot start with "=".
improvise-m4rv. Workbook::new seeds a Default view and on_category_added
fills it, so a file naming only Plan parsed into two views -- the phantom at
index 0, and appended to the user's document on any save. parse_md now drops
Default when the file named at least one view and none was Default. A file
naming no view, or naming Default itself, is untouched.
improvise-3yv3. format_md collected every ungrouped item and emitted the
whole list before any grouped one, so an interleaved category came back
reordered -- and grid axis ordering consumes item insertion order (gotcha 18).
Items are now written in insertion order, a run of bare items sharing a line.
improvise-qu4a. View::prune_empty never reached disk. The line is spelled
with a value rather than by its presence, so the grammar generator can emit
either; absent means false, the compatible reading.
improvise-ppru. Model::measure_agg was lost by the roundtrip. An Aggregation
section carries it, sorted -- measure_agg is a HashMap and hash order is not
an order (gotcha 18), so two saves of one document must be made to agree.
improvise-4lgj. The three wildcard arms in the walker silently skipped any
grammar rule they did not handle, so extending improv.pest caused silent data
loss rather than a loud mismatch. They now bail, and a new exhaustive match
over Rule names the walker that reads each one -- a new rule stops that test
compiling, which is design-principles section 2's "the compiler finds every
call site" applied to a pest-derived enum. It earned its keep immediately by
naming Rule::section, which the first draft had missed.
That arm is also what makes the change safe: the grammar-walking generator
reaches both new rules -- proved by injecting a bail into each and watching
generated_file_parses fail -- so an unwalked rule is now a test failure.
Un-ignores the five reproductions in tests/bug_hunt_reproductions.rs.
improvise-nvq4 did NOT reproduce and is kept as a passing control.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
The five reproductions for improvise-jc95, 5hfh, l1f5, 2cw9 and the long-row
half of k8i now run. Removing `#[ignore]` is part of the fix: an ignored
reproduction is an open bug, and the module doc now says so rather than claiming
the file is report-only.
Three of the five asserted warning PROSE against a module that emitted no
warnings at all, so they specified wording with no convention to match — and
`"overwritten"` does not contain `"overwrite"`, so the most natural wording
would have failed its own assertion. Each now asserts the OBSERVABLE outcome
first and the prose only as a companion:
- duplicate_csv_headers_not_silent: three fields with values 1, 2, 3 in order.
- repeated_import_coordinates_not_silently_overwritten: exactly one cell at 30.0.
- long_csv_row_is_not_silently_dropped: no row discarded, plus a warning that
counts the affected rows and names the widest, because the values here really
are still dropped and there is no observable outcome to assert instead.
Every fix was verified by injecting its own fault and watching the test fail:
the `unwrap_or(0)` restored, `disambiguate_headers` short-circuited, the
long-row counter disabled, both `all_` bindings restored to the full slice,
`take(10)` restored, the accumulate arm changed back to last-wins, and the cell
write order sorted. Each named test failed; each control did not.
Evidence in context/test-evidence/formula-and-io.md, keyed by test name,
including the correction to `the_detected_date_format_does_not_depend_on_the_hash_seed`
— its fixture had made the ten-sample truncation a specification.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
improvise-s08t: eval_agg narrowed its partial key only where
find_item_category resolved, so SUM(Typo) left the key unconstrained and
aggregated every numeric measure matching the context. An unknown name now
errors ref:{name} before the raw fast path.
improvise-s2w3: the same fast path assumed a bare Ref names a STORED cell. A
formula target has no raw cell behind it (improvise-nrs), so SUM(Double)
summed nothing and answered 0 where SUM(Double + 0) folded and answered 20.
core.md §13's defence of the specialisation holds — what failed is its
premise — so eval_agg asks formula_for, the one home of "is this cell
derived", and a derived ref falls into the fold.
improvise-7iww: recompute_formulas stopped at MAX_EVAL_DEPTH and left the last
iteration in formula_cache, so a circular formula read Error("circular")
directly and Number(26) through the cached path — which is the DISPLAYED path,
since default views put the virtual categories on Axis::None. Cells still
moving when the bound runs out are now marked circular, which is the same
bound evaluate_depth already enforces.
improvise-vyrx: eval_bool_cached wildcarded BinOp where eval_bool lists its
arms. Verified by adding a BinOp::Mod: four sites now fail the build where
three did before.
improvise-pikk: round_half_away scaled by 10^decimals before truncating, so
format_f64(1e307, false, 2) rendered "inf" for a finite number.
improvise-f86q: add_category checked MAX_CATEGORIES before contains_key, so
re-adding an existing name at the limit was refused with a false message — and
per gotcha 21 aborted the rest of the batch.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
improvise-yae4. grid_body narrowed every measured width to u16 and only then
added the one-character gap and clamped to MAX_COL_WIDTH, so the arithmetic ran
at full width before the cap that makes it safe. A stored 65535-column text
value panicked at `w + 1` in debug; in release the sum wrapped to 0 and the
column was drawn at MIN_COL_WIDTH — the narrowest column, for the widest value.
A 65536-column value never reached the addition: `as u16` truncated it to 0
first, so it wrapped in debug builds too, silently.
Widths are now capped while still usize, in the one place every measurement
enters (`widen`), which is why header_level_seed folds through it rather than
keeping a second `as u16`. The answer is unchanged for every input that did not
overflow: (min(w,32) + 1).clamp(5,32) and (w + 1).clamp(5,32) agree everywhere,
both saturating at 32 for any w >= 31.
improvise_protocol::header_level_widths carried the identical defect on the
row-header axis — a 65535-column item label — and is fixed the same way. It is
the one home for that rule (gotcha 44), so the row header and the column widths
cannot end up with two answers.
wide_cell_clamps_without_overflow is un-ignored and extended to the 65536 arm
and to an ordinary 20-column value. It asserts `widths == vec![expect]` rather
than `all(<= 32)`: the length pins that the fixture really produces ONE column,
and the ordinary arm pins that the single column is the one carrying the value —
a fixture in which no column was wide would read MIN_COL_WIDTH there and pass a
`<= 32` check. Both injected faults are recorded in
context/test-evidence/core.md.
Not fixed, and out of this ticket's scope: compute_visible_cols accumulates
`acc + w` in u16, reachable only from a terminal ~65535 columns wide.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Expand the warning pattern matching to include various forms of the
overwrite warning, such as 'overwritten', 'overwriting', and 'overwrote',
while still ensuring the affected coordinates are present.
Co-Authored-By: fiddlerwoaroof/git-smart-commit (unsloth/gemma-4-26B-A4B-it-qat-GGUF:UD-Q4_K_XL)
Refine assertions in reproduction tests to be more specific and ensure they
correctly validate expected error messages and warnings.
This includes improvements to:
- duplicate_csv_headers_not_silent
- repeated_import_coordinates_not_silently_overwritten
- long_csv_row_is_not_silently_dropped
- wide_cell_clamps_without_overflow
Co-Authored-By: fiddlerwoaroof/git-smart-commit (unsloth/gemma-4-26B-A4B-it-qat-GGUF:UD-Q4_K_XL)
These tests reproduce known bugs and are marked with #[ignore] to prevent
them from failing standard test runs.
They can be run explicitly using 'cargo test --test bug_hunt_reproductions
-- --ignored --nocapture'.
Co-Authored-By: fiddlerwoaroof/git-smart-commit (unsloth/gemma-4-26B-A4B-it-qat-GGUF:UD-Q4_K_XL)
Preserves a collection of test cases designed to reproduce specific bugs in
the `improvise` and `improvise_io` libraries. This includes issues related
to:
- CSV parsing
- Non-finite values
- Circular formulas
- UI overflow from large strings
Twenty tests in this file are intentionally designed to fail to reproduce
known unfixed bugs, while two control tests are expected to pass.
Co-Authored-By: fiddlerwoaroof/git-smart-commit (unsloth/gemma-4-26B-A4B-it-qat-GGUF:UD-Q4_K_XL)
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
Four findings from the adversarial review of #78. Two blocked; all four are
real and one of them was a straight error on my part.
1. CI WAS RED AND I NEVER RAN THE CHECKS. Nine comment runs over the
two-line cap, all mine, all introduced this PR; both checks exit 0 at
8a559b2 and run in under a second. Behind it `--check-evidence` also
failed, with two orphan pairs that are the direct product of my own
renames: the consolidation renamed two tests and left their evidence
naming the old names. All thirteen fixed. The renamed entries are
rewritten rather than relabelled — each now records what the rename was
FOR, which is the thing a relabel would have thrown away.
3. MY SortData CONCLUSION WAS WRONG, and the mechanism I removed was
needed. I reported "could not build a fixture where SortData reorders".
It reorders trivially. The cause was not the reviewer's guess (already
sorted insertion) but better: ToggleRecordsMode pushes effect::SortData
as its FIRST effect, so a records-mode fixture built by pressing R has
an already-sorted store BY CONSTRUCTION and cannot demonstrate anything
order-sensitive. My store was fine all along.
The `layout.records` coords hash is restored, and
`sorting_the_store_in_records_mode_invalidates_the_cursor` pins it with
a fixture that sets the axes directly. It asserts the row COUNT and the
LABELS are both unchanged, so it is the sharp case rather than a shape
change wearing its name. Removing the hash again fails that test and
NOTHING ELSE IN THE WORKSPACE.
The process error is the part worth keeping: I concluded "cannot
reproduce" from ONE fixture without varying it, then filed that
conclusion as a finding. "I could not reproduce it" and "it does not
reproduce" are different claims and I filed the second having earned
only the first.
4. THE page_selections COMPONENT IS DELETED. With prune_empty off a page
flip renumbers nothing and the cursor was still sent home — PR #76's own
ruling is that this is worse than the hole improvise-7kvw names. My
comment defended it with a basis argument for a cursor decision, which
is how it got written. `paging_with_nothing_pruned_invalidates_no_cursor`
is the negative arm that was missing; restoring the three lines fails it
and nothing else, and the positive arm passes either way because
`with_frozen_records` already applies pruning — which is exactly why the
negative arm had to exist.
ALSO: context/code/command.md gains "Two observations, not one". My ni6k
reasoning lived only in a commit message, which design-principles.md §6
explicitly forbids — "a reason recorded only there is a reason nothing can
ever find wrong". `view_frame`'s doc pointed at a section that did not
exist; now it resolves. The stale "since improvise-9mo9 they call
reset_cursor" sentence is corrected: no effect calls it any more.
Finding 2 (the #77 merge conflict over AxisEntry::GroupEnd) is NOT in this
commit — #77 lands first and I merge main afterwards, handling the variant
deliberately rather than letting it fall into a `=> {}` that would make a
frame change invisible to the cursor.
Filed, not fixed: improvise-5ubs (a cyclic-successor all() can omit a
variant while its doc says it cannot — the reviewer's meta-finding, that a
consolidation owes a test its surviving derivation is TOTAL) and
improvise-9v0j (view_frame vs view_shape separated by example, not by
property). improvise-gfvj closed with the correction.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
improvise-gd83, improvise-0o6x and improvise-8qlr. One commit because they are
one change to `AxisFrame`/`AxisEntry` — a variant, a field and a method — and
all three move `tests/grid_frame_agreement.rs`, whose two ends must be changed
together or the test stops comparing anything.
improvise-gd83 — AxisEntry carries a group-end boundary
-------------------------------------------------------
`expand_category` emitted a header on ENTERING a group and no boundary on
leaving one, so `group_of` answered with the nearest preceding header rather
than membership, and the TUI painted that span. Copilot was right on PR #23;
this is the filing gotcha 44 said the fix needed.
`AxisEntry::GroupEnd` is the boundary, carried onto the wire as
`FrameEntry::GroupEnd`. Two arms, and a fix can close one and miss the other:
* within one expansion — Jan[Q1], Feb[Q1], Sep — a `Some -> None` transition;
* across expansions — `cross_product` expands the inner category once per
prefix and each call starts with `last_group = None`, so Food/Bob[Family]
is followed directly by Clothing/Carol with no transition to hook onto.
That needs a `GroupEnd` at the END of an expansion whose last item was
grouped.
`GroupEnd` is NOT emitted between two groups: the next header already ends the
previous run, and emitting one there moved `data_row_to_visual_skips_headers`
and its column twin for no gain. Those two are untouched, which is the evidence
the boundary is as narrow as it claims. A trailing marker is trimmed, since the
axis itself ends the last run. Nothing draws a `GroupEnd` and it consumes no
data index.
`THE_MEMBERSHIP_CORRECT_SPANS` and its `assert_ne!` tripwire are gone, and the
value they predicted is now asserted element for element — it was right to the
element, which is worth recording.
improvise-0o6x — a leading group header is drawn
------------------------------------------------
`body_from(idx)` started at the first ITEM, so at the top of an axis a leading
`FrameEntry::Group` was dropped on the way to the screen, in both front ends. A
collapsed group first on its axis therefore had its rows hidden and its header
nowhere: no affordance to get them back.
It is a boundary condition as the ticket suspected, but the boundary is not
`idx == 0`. The windowing rule carries the nearest header ABOVE a scrolled
window for `group_of` and `body_from` discards it — correctly — and a frame
cannot tell that from the top of the axis using `idx` alone, because the
browser's frame is already windowed and always passes `0`. So `AxisFrame`
gained `first`, the absolute data index of its first item, and the rule is
`first + idx == 0`. Without it the browser would draw an extra header row at
every scrolled position that the TUI does not.
RE-PINNED DELIBERATELY, the improvise-q0vb acceptance shape: `GROUPED_FRAME` in
`src/ui/grid.rs` moves by exactly one line — the leading `▼ Co…` now draws
above `East` and everything below shifts down one. The group line, the column
headers, the separator, the data rows and the totals strip are byte-identical,
which is the evidence this is the boundary condition and not a reshuffle.
`harness/tests/grouped.spec.js` asserted TWO group rows against a fixture with
three, naming this ticket on the line and saying the count becoming 3 would be
the record of when it was fixed. It is 3, verified in Chromium. Three
assertions in that file took "the first tbody row" to be a data row; a
`firstDataRow` helper now skips the header, which is the same re-pin.
improvise-8qlr — the run rule lives once
-----------------------------------------
Column-group run coalescing was derived in `src/ui/grid.rs` and again in
`improvise-wasm-client/src/render.rs::col_groups`, held in step by a comment
saying so. `AxisFrame::group_runs`/`group_run_heads` is the one home and both
ends call it. Runs still coalesce by NAME, which is what both ends did before
and is preserved rather than quietly changed.
The TUI keeps one divergence, stated: it labels its first VISIBLE column even
mid-run, or a scrolled grid draws an unlabelled group line.
The agreement test's TUI arm was `group_line.contains("▼ Q1")` — the unpinned
arm the ticket names. It now compares the painted group line cell for cell
against spans derived from the browser's own runs. The test is fixed, not
deleted.
Injected faults, each failing by name
-------------------------------------
* gd83: delete either `GroupEnd` emission — the two new layout tests fail
with the leaked group, and the column-header agreement test fails with them.
* 0o6x: force `body_from`'s match to `false` — `the_whole_frame_is_unchanged`
and the new leading-header agreement test fail.
* 8qlr: the ticket's own recipe, forcing the TUI to emit the group label on
every column. `cargo test -p improvise` was GREEN before this commit and
now fails with ["▼ Q1","▼ Q1","",...] against ["▼ Q1","","",...].
What the ticket did not predict
-------------------------------
A collapsed leading group has no item to carry its header into a window, so it
reaches the frame only through `axis_frame`'s `pending` queue. The first
version of the windowing change had `GroupEnd` CLEAR `pending` above the
window, which deleted exactly that header — the affordance 0o6x exists to
restore. `GroupEnd` is now queued like a header and clears nothing. Found by
driving the new agreement test in both collapse states; the uncollapsed arm
alone passes against the broken version.
1411 Rust tests pass (baseline 1405, +6). Playwright 28 passed + 3 skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
improvise-j7v6: the rule turning a typed string into a `CellValue` was
written twice — `resolve::typed` (already moved into improvise-protocol
since the ticket was filed) and `commit_regular_cell_value`'s
if/else-if/else on the host's key path. The host now calls
`improvise_protocol::typed`, which is `pub` for that reason.
Per doctrine(a_consolidation_that_keeps_its_compatibility_test_is_unfinished),
`a_typed_string_is_classified_as_the_host_classifies_it` did not survive
as a comparison: type identity replaced its claim. It is re-scoped to the
thing this crate still owes — the FRAME the string composes — and renamed
`a_typed_string_composes_the_frame_that_carries_it`.
The ticket's flagged risk (gotcha 34: a shared classifier that disagrees
with the persistence layer is a third answer) is now asserted rather than
argued: tests/typed_string_agreement.rs round-trips what `typed` calls
text and what it calls a number through format_md/parse_md. Proved
discriminating: dropping `pipe_quote` from the writer fails it.
improvise-8uj8: `every_rejection_message_says_the_edit_did_not_land`
drove a hand-written five of seven `Rejection` variants while claiming
the fact held "over all of them". `Rejection::all` (the same cyclic walk
`AppMode::all` uses) and `Rejection::refused_an_edit` (exhaustive) now
drive it, and its claim is re-scoped to what is true: a rejection that
refused an EDIT says nothing was applied, and the two Save answers —
which refused no edit — say the document was not written. Renamed
`every_rejection_message_says_what_it_refused`.
Proved both ways: wording SaveFailed as "hmm" fails the test, which the
old list could not have seen; adding a Rejection variant fails the build
at basis.rs:101 and :126.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
The question PR #60 left open was not which tests the launch default
disarmed, but HOW TO KNOW THE CLASS IS CLOSED. Grep cannot answer it: it
finds sites that NAME the flag, and the hazard is a test that merely
starts from a default-built state and asserts something downstream,
mentioning nothing. Finding the three disarmed tests by grep was luck.
Mutation answers it, because it does not care what a test mentions.
Recorded in context/test-evidence/clients.md: mutate every site that
writes, propagates or reads the value to the constant the default
supplies; sites where nothing fails are suspects; then RE-RUN EACH
SUSPECT WITH THE DEFAULT INVERTED. If it now fails, the test was
vacuous -- the default disarmed it, fix the fixture. If it still fails
nothing, it was simply unguarded -- pre-existing, file it.
That second step is the whole method. Without it the two species are
indistinguishable (both look like "nothing failed"), and it is what
established that the vacuity class is closed at TWO rather than three.
It found two further silent sites, neither caused by the ruling:
- App::cmd_context propagates the flag and nothing tests it. Stuck
true, `C` on a CLOSED panel decides to close it -- the panel becomes
unopenable by its own key -- with the workspace green. Invisible
because the command test helper builds the value it then asserts on.
Filed as improvise-qsmp; control run proves it predates the ruling.
- render.rs propagates it into RenderModel. Stuck true is improvise-wsaw
reintroduced one layer up, and RUST SEES NOTHING -- only the Chromium
harness catches it. A Rust-only run of the `test` job cannot see a
reintroduced wsaw, which is the sharpest argument on file for the
browser job being able to fail the build.
Also marks the_terminal_constructor_inherits_the_shared_default_rather_than_setting_it
PROSPECTIVE: with three panels its two siblings already cover every
Panel::ALL member, so it cannot fail alone today. It earns its place the
way gotcha 12 describes Panel::ALL -- a fourth panel reaches it from one
entry -- and saying so stops a green tick being read as present-tense
evidence.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
USER RULING (2026-09-20): the category sidebar is the launch default in the
TUI as well as the browser. `improvise model.improv` opening with the
sidebar showing is intended, not collateral. The behaviour is unchanged;
what changes here is that it is now a stated, tested decision.
Three tests asserted `category_panel_open` was TRUE after doing something
to make it true. The default now supplies that, so all three passed
without the code under them working.
**`set_panel_open_and_cursor` is the serious one.** With it vacuous,
`effect::SetPanelOpen`'s Category arm could be replaced by a no-op — `C`
doing nothing in the terminal — and the whole workspace stayed green at
1391 tests. Now closes the panel first and asserts it closed, so the
effect has to move the flag.
**`panel_commands_address_the_named_panel_only` is the subtle one, and my
first repair of it was ALSO vacuous.** Folding `[open:false, open:true]`
and asserting the final state is true still passes against a reducer that
folds nothing: the default already supplies `true`, so neither command
needs to land. Only asserting the FALSE direction discriminates. Caught by
injection reporting NONE failing after the "fix" — it would otherwise have
shipped looking repaired.
The rule this leaves, now in `context/code/clients.md`: **assert the
direction the default cannot supply.**
`tests/tui_launch_state.rs` gives the ruling a guard on the front end it
newly changes — the TUI's launch frame had no test at all. Not a rendered
frame: §7 says not to test `Widget::render` and the repo has no
`TestBackend`, so the target is `App::new`. One of the three walks
`Panel::ALL` comparing `App::new` against `ViewState::default()`, which is
what would catch a TUI-only launch state — the divergence wsaw closed.
**The view-row click test was vacuous for the property it is named for.**
`Default` is index 0 and holds `view_panel_cursor`, so clicking it made
`index` and the cursor the same number. Injecting the cursor-vs-index
fault into `click_view_row` passed the contract suite 3/3 — the exact
fault the ticket, the evidence and the Rust test exist for was invisible
to the browser half. Now switches to Default by keyboard, then clicks
Plan, and restores the two assertions the relocation had dropped.
Inherited from the deleted `pointer.spec.js`, not introduced here.
Filed rather than fixed: improvise-56ek (`SetPanelOpen` maps a Panel to a
field twice, in two crates — each copy needed repairing independently, and
the harness passing against a no-op'd effect is correct rather than a gap)
and improvise-qf57 (the formula row's dropped `toHaveCount(0)`).
improvise-4ibz is closed by the ruling.
Every claim here proved by injection under `--no-fail-fast`, which is the
method note: `cargo test --workspace` is fail-fast and understates blast
radius.
Gates: 1394 Rust tests, 26 + 3 Node, 16 Playwright (3 WebKit OPFS skips),
clippy -D warnings, fmt, eight wasm crates, comment runs, 516 evidence
entries. `.beads/` untouched.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
improvise-ir7l. project::cell_id decided Record vs Aggregate purely on whether
a regular category sat on Axis::None; it never asked whether the coordinate
named a formula target. So a formula cell was a Record, render.rs::editable
said true, and a write to it landed, was stored, and was then shadowed forever
by formula_cache — not a revert, a permanent silent shadow, on the demo's
front page.
Rather than a second "but not really" flag beside CellId (the two-answers
shape ADR 002 exists to forbid), a formula-owned cell now projects a different
kind of identity. Derived carries the formula's target and no surrogate, so
the write is unconstructible rather than refused.
The predicate is Model::formula_for(&CellKey), one home, which already existed
three times open-coded inside model/types.rs. The TUI lands in the same change:
EditOrDrill calls the same predicate and refuses, or the two front ends
diverge.
Two orderings are load-bearing and tested: Derived is asked before the record
branch (the coordinate is stored), and after the aggregate branch (gotcha 10 —
an aggregating formula cell is still drillable).
Two tests were passing while their names were false for the Profit column and
are renamed and two-sided now.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
grid_frame_agreement.rs is the strongest test in the last wave and its
comments were the record of six injected faults; all of it is now in
context/test-evidence/tui.md, keyed by test name.
THE_MEMBERSHIP_CORRECT_SPANS and its assert_ne! are UNTOUCHED. Its comment
now says what it is — a deliberately wrong constant and a live tripwire —
and points at the derivation rather than carrying it.
shipped_documents_load.rs keeps its two load-bearing claims in the file
(directory walk, full-name match) because both sit directly over the code
that would otherwise be "simplified" back into the bug.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
Edward Langley