Commit Graph

43 Commits

Author SHA1 Message Date
Edward Langley d9b10905d0 improvise-2bac: context docs, evidence, fmt
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
2026-09-22 20:58:34 -07:00
Edward Langley afba3fa0a6 WIP improvise-2bac: interactive arm is parse-then-contextualize
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
2026-09-22 20:50:48 -07:00
Edward Langley b855faf3c5 fix(cli): an explicit coordinate still beats the cursor (improvise-6uva)
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
2026-09-22 19:07:33 -07:00
Edward Langley 64bcb821da fix(cli): a script's command is built against the cursor it will run against (improvise-6uva)
`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
2026-09-22 18:59:08 -07:00
Edward Langley 42cbf0932b docs: name the follow-up, and stop the scratch dir looking like a ticket id
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
2026-09-22 08:20:48 -07:00
Edward Langley c13466f1d9 fix(cli): the headless commands were silent whether they worked or not
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
2026-09-22 08:18:17 -07:00
Edward Langley d00cea2477 fix: show decimals by default, and let a formula name its own format
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
2026-09-21 23:33:52 -07:00
Ed Langley 7b8d7375af Merge pull request #108 from fiddlerwoaroof/wave/j-persistence
wave/j: .improv persistence — eight beads, and the format changed
2026-09-21 16:23:57 -07:00
Ed Langley c91aee154e Merge pull request #107 from fiddlerwoaroof/wave/i-import
Wave I: five silent import defects (jc95, 5hfh, l1f5, jcfs, 2cw9)
2026-09-21 16:23:46 -07:00
Ed Langley 8d682bec1c Merge pull request #106 from fiddlerwoaroof/wave/l-prune-layout
Wave L: prune_empty drops collapsed headers and GroupEnds; column width overflows before clamping
2026-09-21 16:03:46 -07:00
Edward Langley 7509417d0b test(import): three review findings — a rival fix, a missing injection, an order test with one history
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
2026-09-21 15:15:04 -07:00
Edward Langley c84707261c fix(io): five .improv persistence defects, grammar first
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
2026-09-21 14:46:15 -07:00
Edward Langley dcc0addfb4 test(import): un-ignore five reproductions and record what each is evidence of
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
2026-09-21 14:43:19 -07:00
Edward Langley 20030cac0d fix(core): four evaluator and display bugs, and one exhaustive match
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
2026-09-21 14:42:19 -07:00
Edward Langley 09e43c1fcc fix(command,protocol): cap a column width while it is still usize
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
2026-09-21 14:42:08 -07:00
Edward Langley a36b5a63f7 test: expand overwrite warning patterns in reproduction test
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)
2026-09-21 14:15:58 -07:00
Edward Langley bc9ecaf527 test: improve assertions in bug hunt reproductions
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)
2026-09-21 14:08:28 -07:00
Edward Langley 0d13ea5f51 test: ignore known bug reproductions by default
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)
2026-09-21 13:59:38 -07:00
Edward Langley 8c84fd95ea test: preserve bug-hunt reproductions
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)
2026-09-21 13:55:03 -07:00
Edward Langley c4bf74608f fix(protocol): a group run is opened by a header, not by a name (improvise-h0iv)
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
2026-09-21 03:57:07 -07:00
Edward Langley 5e957e1f58 Merge remote-tracking branch 'gh/main' into wave3/protocol-session 2026-09-20 21:06:20 -07:00
Edward Langley 6f98bfd995 Review round 1: green CI, the SortData hole reopened, the paging regression removed
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
2026-09-20 21:03:40 -07:00
Edward Langley 8f3a9768b7 fix: group membership, leading headers and one run rule on AxisFrame
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
2026-09-20 20:40:57 -07:00
Edward Langley bc77e68f40 One classifier for a typed string, one exhaustive walk over Rejection
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
2026-09-20 20:36:51 -07:00
Edward Langley ac6f17288c docs(tests): record how to know a launch default has stopped disarming tests
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
2026-09-20 14:24:08 -07:00
Edward Langley 1922beca06 fix(tests): a launch default disarms every test that asserts what it supplies
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>
2026-09-20 14:12:44 -07:00
Edward Langley 0f14b2a39e fix(protocol): a formula-owned cell projects CellId::Derived, not Record
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>
2026-09-20 10:59:59 -07:00
Edward Langley f7d377b010 refactor(tests): cap the integration tests' comments
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>
2026-09-20 09:51:38 -07:00
Edward Langley aab9b0fb21 review: the surviving mutant, and four claims that were not true
From the independent review of #23. One fixture line, four comments, and a
set of doc guards it offered as optional.

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

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

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

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

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

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

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

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-20 00:28:38 -07:00
Edward Langley 22360d46e5 Merge main into wave/a-grid-frame
Two conflicts, both expected.

**`invalidate_relabelled` (improvise-hkt6, WU-K).** Both sides kept: the
signature and the item-tuple comparison are this branch's — `old` is an
`AxisFrame` now, and a group header appearing or collapsing must not blank
cells it did not rename — and the totals blanking and its whole argument are
main's. `grid_updates` still diffs the totals against the *framed* mirror,
which is what makes a renamed column whose total did not change send one;
`a_relabelled_column_drops_its_total`,
`a_relabelled_row_leaves_the_totals_alone` and
`a_renamed_column_whose_total_did_not_change_still_gets_its_total_back` all
pass against the new frame.

**`lib.rs`'s composition test.** Main's deletion taken: 18189c1 moved
`client_showing`, `published_basis` and
`a_client_composes_a_write_from_the_id_it_was_given` into `socket.rs` when
it consolidated `set_cell_at` and `typed`.

Main's new `GridUpdate::RowLabels`/`ColLabels` call sites are converted, and
five doc comments naming the retired spellings with them — a fact stated in
several places is a grep, not an edit.

1327 passing, 0 failing. clippy/fmt clean, wasm gate clean, both bundles
build.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-20 00:10:15 -07:00
Edward Langley d9a4790d4d fix: an empty group name is a name; and stop reserving the group rows
Copilot's seventh pass. One applied, one applied by *withdrawing* the
previous fix rather than patching it again.

**`RenderColGroup::label` is an `Option<String>`.** `""` was the
ungrouped-run sentinel, but an empty string is a legal group *name* —
`Category::add_item_in_group` accepts one and the TUI draws its indicator
like any other — so a real group serialized identically to no group at all
and the page dropped its indicator on a truthiness test. `None` is now the
run in no group, and `app.js` checks for null.

**Inline group rows are no longer reserved in the viewport, and the reason
is written down.** Copilot was right twice in a row here, about two
opposite mistakes. Counting the current group rows is a feedback loop (a
smaller window crosses fewer groups and asks for a larger one). Keeping the
maximum is a ratchet: one group header per data row stores an oversized cost
on the first oversized projection and clamps the grid to a single row
*permanently*. Both were on this branch.

The third option — invent a density heuristic — is the one this repo warns
against, in a file no test here can execute. So the reservation keeps only
what depends on the axes and not on the window: the header lines, the group
line, the row-header width. The residue is an over-request bounded by the
groups in view, and the off-screen-selection gap it leaves is neither new
nor specific to groups — nothing in this page calls `scrollIntoView` at all.
Filed rather than guessed at.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-19 23:45:08 -07:00
Ed Langley 391599476b Merge pull request #27 from fiddlerwoaroof/wave/g-command-layer
WU-G: the keymap and command layer (improvise-th7m, lae, ct5, 2zq; git/aaw declined with evidence; 0hb deferred with its design note).

Includes the ejection fix the 2zq ruling called for — a virtual on an axis is a configuration, not a vacancy — which also closed a live bug where adding a second regular category evicted _Measure from Column and made every measure vanish from the demo's own view.
2026-09-19 23:41:59 -07:00
Edward Langley 6a929c1d21 feat(records): a +cat column, so a schema can be built without leaving the grid
improvise-2zq step 2, on the user's ruling. Step 1 made this coherent by
stopping `on_category_added` from ejecting the user; this adds the column.

Typing a name into `+cat` creates that category, and because
`GridLayout::records_columns` filters on `kind.is_regular()` and never
looks at items or at an axis, the coordinate column appears immediately
and is typeable on the next keystroke.

**The two questions the ruling left open, and the answers.**

*Where the column goes.* Not a second trailing column, and not one `+`
disambiguated by a prefix. A prefix would put a sigil into the name space
— and a category and a measure may legitimately share a name, which is
the exact ambiguity `_Dim` already carries a column *index* rather than a
label to avoid (gotcha 1). A key was rejected too: the name still has to
be typed somewhere, and a minibuffer lands in the records-exit class bug
this PR characterizes but does not fix.

So each `+` sits adjacent to what it adds to — `[coords…, +cat,
measures…, +]`. Position tells the two apart instead of a label, which is
the clutter the ticket was worried about. It costs a mid-grid column
index, and the consequence worth naming is that a **blank** records grid
now opens with the cursor on `+cat` rather than `+`: a record needs
coordinates before it needs measures, so that order is defensible, but it
does change what the first keystroke in an empty grid does.

*Whether committing also sets the coordinate on the current row.* It does
not. It is symmetric with the measure `+`, which creates an empty column
and writes no value; and setting it would move **every** measure cell in
the row onto the new coordinate (gotcha 1) — a data migration across N
cells attached to the same Enter that made a schema change, in an app
with no undo. The follow-up edit is one column away.

**The refusal is visible, which took a change of order.** `MAX_CATEGORIES`
is reachable here rather than theoretical. `AddCategory` now aborts the
batch on refusal — same reason a failed save does (gotcha 21): what
follows assumes the category exists, and the trailing `EnterEditAtCursor`
would re-open the minibuffer *over* the message, since `draw_bottom_bar`
renders the minibuffer instead of the status line. And
`CommitAndAdvance` now emits its mode change **before** the commit rather
than after, so an abort cannot eat it. That is the third time this PR has
applied one rule — tidy-up goes where an abort cannot reach it — and the
comment at that site had already described the pattern for the *advance*
failing; the commit failing was simply not considered. The limit itself
is still checked in exactly one place, `Model::add_category`.

Evidence. Three new tests drive real keys:
`the_new_category_column_creates_a_category_and_its_column` (the column
is usable immediately), `..._writes_no_coordinate` (the deliberate
answer to question 2, asserted over every row), and
`..._refuses_past_the_limit_where_the_user_can_see_it`, which asserts the
mode has **no minibuffer** — proved discriminating by swapping the commit
back in front of the mode change and watching it report
"Maximum of 12 categories reached" from inside `RecordsEditing`, where
nothing would have drawn it.

Four existing characterization tests were updated knowingly, not
incidentally: the shared records fixture's column list (which accounted
for 8 of the 9 initial failures on its own), the core layout and
persistence smoke expectations, and the blank-model bootstrap, which now
has to step to the `+` column and so records the cursor-position
consequence above.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-19 23:10:33 -07:00
Edward Langley ed201e83de test: the column header check is exact, not a contains
Copilot's second pass, and correct: the check was one-directional. It
filtered out the labels the browser blanked and then asked whether each
surviving one appeared *somewhere* on the TUI's line — so a `col_levels`
that over-suppressed was skipped by the filter, and a label painted under
the wrong column still matched.

It now cuts the painted line into its columns by the cache's own
`col_widths` past the row header — the same geometry the widget lays them
out with — and compares the whole vector.

Both faults Copilot named were injected and watched to fail: blanking a
label the TUI paints, and reversing the column order within a level.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-19 22:58:54 -07:00
Edward Langley 7c4b90c537 fix: four Copilot findings on the grid frame
All four were right; two more were right and are declined on the threads.

**A frame's pre-window headers were O(offset).** `axis_frame` accumulated
every group header above the window and flushed the run at the first
in-window item, so a deep scroll past many groups serialized headers that
`body_from` discards on arrival. Only the *nearest* one can ever be answered
with, so only that one is kept.

**`web/app.js` captured the row counter, not its value.** `r` is one binding
shared by every click-handler closure, so after the loop every editable cell
would have called `edit` with the last data row. The group-aware counter was
right; capturing it was not. It is a bug this PR introduced — the old code
took `r` as a `forEach` parameter, which is per-call.

**The agreement suite drove one arm.** It only read `rows`, and its fixture
had no grouped column axis, so `col_levels` and `col_groups` could regress
behind a green suite
(`doctrine(one_arm_of_a_symmetric_rule_is_evidence_about_that_arm_only)`).
The fixture now groups two of three months under `Q1` and leaves `Sep`
outside, with `_Measure` giving a second column level, and
`the_two_front_ends_draw_the_same_column_header` checks the levels against
the painted header lines and the spans against the column count. One span
per column, and a dropped level suppression, were each injected and watched
to fail.

**The group branch did not compare the label.** It asserted only that the
painted text began with an indicator, so any group name passed. It now
compares the whole painted header against the browser's glyph-plus-label.

The column test also *demonstrated* the finding this PR declines: `Q1` spans
all six columns including the two under the ungrouped `Sep`, because
`expand_category` emits a header on entering a group and no boundary on
leaving one. That is `GridLayout`'s rule and the TUI paints the same span.
It is asserted as it is, with the defect and the corrected expectation
written next to it, and stated on `AxisFrame::group_of`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-19 22:52:18 -07:00
Edward Langley 239f1f0b80 test: the shipped-document walk must not skip a gzipped one
Self-review of improvise-odbc's own guard. The walk filtered on
`Path::extension() == "improv"`, and `extension()` of `demo.improv.gz` is
`gz` — so a gzipped shipped document would have been skipped silently, which
is exactly the "exercised by nothing" hole the test exists to close, put back
one line inside the fix for it.

Matched on the full name instead. Proved both arms: a valid `.improv.gz`
dropped into `examples/` passes, and a truncated one fails. (The truncation
also exercises the `with_context` added two commits ago, from the other side.)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-19 22:48:12 -07:00
Edward Langley 7013fbc398 test: the two front ends draw the same grid frame
The check gotcha 44's divergence never had. Each front end is driven the way
it is really driven — the widget into a ratatui buffer, the browser client
through a `Snapshot` of that same cache's `seed()` — and they are compared on
what they *draw*, not on what they were handed; comparing the caches would be
vacuous, since it is one cache.

Injected both faults and watched them fail before keeping it:
`labels: vec![rows.item(r).join("/")]` in the client's `render_model`, and the
same join in the widget's row header. Each names the row and both spellings.

`improvise-wasm-client` becomes a dev-dependency of the root crate for this.
It builds natively — that is how its own tests already run — and being a dev
dependency the terminal binary does not grow a wasm client.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-19 22:43:52 -07:00
Edward Langley 301a74821a feat(protocol): the grid's frame on the wire, as AxisFrame
`GridCache::row_labels`/`col_labels` were `Vec<String>` built by
`GridLayout::row_label`, which slash-joins a multi-level tuple — the exact
rendering `two_row_categories_produce_cross_product_labels` asserts must not
appear. So the protocol carried a form of the labels the TUI itself
considered wrong, and `improvise-wasm-client/src/render.rs` drew precisely
that form: the browser showed `Food/Alice` where the TUI showed two header
columns, with no groups and no collapse indicators (gotcha 44).

Both fields become one `AxisFrame` per axis, carrying the five facts gotcha
44 enumerates: the categories (hence the header levels), the label *tuples*
and the inline group headers, the group a position sits in, and each group's
collapse state. `record_col` stays off — session-local, keys pending drill
edits, correct to keep off.

It is one type and not a `row_levels` beside `row_labels`, because two
answers to "what are the labels" is the disease (improvise-edp). The rules
that used to be written twice are methods on it and are now called from both
front ends: `shows_level` (repeat suppression, improvise-jjx's fix),
`group_of`, `body_from`, and `header_widths` — the row-header width rule,
which existed twice already, inline in `render_grid` and again in
`viewport::compute_row_header_width` (improvise-1125).

`GridWidget` loses its `&View`: nothing in it reads the view any more.

Evidence. `the_whole_frame_is_unchanged` — the exact-buffer characterization
over a grid with row groups, column groups, a page axis, multi-level headers
and a total row — passes byte for byte, so the TUI is unchanged. The new
`tests/grid_frame_agreement.rs` drives the widget into a ratatui buffer and
the browser client through a `Snapshot` of the *same* cache, and compares
what each draws; re-introducing the slash-join on either side was injected
and watched to fail before the test was kept.

`tests/web_demo_model.rs::the_web_demo_has_one_category_per_visible_axis` is
deleted: it guarded the gap this closes, and a consolidation that keeps its
compatibility test is unfinished.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-19 22:43:47 -07:00
Edward Langley 1c2361b45a fix(persistence): delete bank-info.improv, and pin why it could not be read
improvise-odbc named two readings of the parse failure at
`- JORDAN MARTINEZ` and said to decide by asking whether improvise can
WRITE that line today. It cannot: `item_list`'s `name` is
`pipe_quoted | bare_name`, `bare_name` is `[A-Za-z_][A-Za-z0-9_-]*`, and
`quote_name` mirrors it character for character, so a spaced item name is
emitted `|JORDAN MARTINEZ|`. So the reader and the writer agree and the
round-trip property is not incomplete — the file was stale.

It was stale by hand, not by drift. c48a5cd migrated it to v2025-04-09 by
editing it in place: the `Date` items were pipe-quoted and the `Entity`
ones were not, and every line of its `## Data` section names a category
`Measure` where the file declares `_Measure`. It has not been loadable
since April, and nothing loaded it, so nothing said so. Deleted rather
than regenerated: there is no generator for it — it is hand-written fake
bank data — so "regenerating" would mean inventing a canonical form and
checking in 1200 lines whose only property is that they parse, which the
generator-driven proptests already establish over a far wider input space.

What is kept is the evidence and the gap the incident actually exposed:

- `an_item_name_the_item_line_cannot_carry_round_trips` puts the file's own
  item names through `format_md`/`parse_md` and asserts the spaced ones come
  back quoted and the bare-legal one stays bare.
- `a_bare_item_name_with_a_space_is_rejected` asserts the reader really does
  refuse the unquoted line, with the quoted form parsing as the precondition.
  Without it the first test would be consistent with the reader accepting
  both spellings, which is not what happened.
- `tests/shipped_documents_load.rs` closes "a sample document exercised by
  nothing": it walks the two directories `.gitignore` un-ignores and loads
  every `.improv` in them. A directory walk, not a list of names, because a
  list is a second place to remember. Proved discriminating by dropping the
  offending line into `examples/` and watching it fail.

Unrelated, reported not fixed: `foo.improv` in the repo root has no version
line at all and is equally unloadable.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-19 22:36:12 -07:00
Edward Langley a550201fae feat(web): the page spawns the worker, opens a demo, and reads a key
Three pieces of the standalone deployment, all in web/.

improvise-djm — app.js picks its peer instead of assuming a socket. The
wasm bundle is the same artifact either way (improvise-xks6), so the
difference is confined to one factory: `?ws=` selects the websocket, its
absence spawns `new Worker('./worker.js?doc=&seed=', {type:'module'})`.
The websocket path is KEPT, because improvise-ws-server is how a
TUI-hosted document is shared and nothing else can reach it; the presence
of the parameter is the whole selection mechanism, since a second knob
naming the same fact is a second thing to get wrong.

improvise-bck — web/demo.improv, built against an assertion rather than
by flipping an axis. examples/demo.improv leaves Customer/Date on None,
so every cell is an Aggregate and nothing is editable. The new demo puts
exactly one category on each visible axis (Product on row, _Measure on
column, Quarter on page) and carries Profit = Revenue - Cost, so a
visitor sees the formula move beside the cell they typed in, with no
keystroke and no scrolling. Two tests hold it: tests/web_demo_model.rs
projects the whole grid through grid_of and asserts every CellId is
Record (ADR 002 — that is what `editable` means), and post_message.rs
drives the shipped bytes through the real seed -> VFS -> open -> Snapshot
chain. The first was proved discriminating by pointing it at
examples/demo.improv, where it names the aggregate at (0,0).

One category per axis is asserted too: GridCache slash-joins a
multi-level tuple (gotcha 44), so a second category on an axis puts that
known gap on the front page.

improvise-hwpi — all four. A keydown listener on window feeds
Client::on_key_event, so the classifier is reachable at last; a frame
that will not fold now writes to #status instead of only the console; the
socket has an error listener, so "not running" no longer reads like
"still connecting"; and close() has a re-entry guard. Plus one the ticket
could not have predicted: a worker whose start() rejects is caught inside
worker.js and fires no `error` on the main thread, so the same silence
was about to be rebuilt in the new path — a first-frame watchdog covers
both deployments, because "the peer never answered" is the one condition
that means the same thing on each.

Verified: cargo test --workspace 1159 passing (1155 + 4), clippy clean,
fmt clean, actionlint clean, ./web/build.sh end to end (345 KB / 123 KB
gzipped client, 1.33 MB / 498 KB gzipped worker), every call present in
the generated .d.ts with the right arity, every relative reference in the
assembled site resolving. NOT verified: there is no browser here. No line
of JavaScript in this commit has ever run.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EoW5BB87ZZdm96SvBTWawH
2026-09-19 19:28:27 -07:00
Edward Langley f47decffdd test: make the records smoke test self-contained (improvise-c5v)
It read a .improv fixture, but *.improv is gitignored, so the fixture never
made it into the commit and the test would have failed on a clean checkout.
It now builds the model in-process — CSV, import, save, load — which also
covers more of the path than the fixture did.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V69PGQfMJuGRmtSWpfVAqn
2026-08-29 12:31:06 -07:00
Edward Langley 22964f99be feat: wide-format records mode, one column per measure (improvise-c5v)
Records mode showed long format: a row per cell, a _Measure column naming
which measure it was, and a synthetic "Value" column holding the number.
Entering data meant sitting on "Value", and editing the measure column
renamed a coordinate rather than a value.

Wide format instead: a row is one coordinate tuple — the cells sharing every
coordinate but _Measure — and each raw measure gets its own editable column.
Two measures on one coordinate are now ONE row.

The case matrix came from a Prolog model rather than a read-through, which
surfaced the load-bearing consequence: a row now groups several cells, so
editing a category column must move all of them. The long-format code moved
only the cell under the cursor, which here would leave half a row's measures
under the old coordinate and half under the new. Both the plain-records and
staged-drill paths implement it.

_Dim in the synthetic key now carries the column index rather than a label,
because a category and a measure item may share a name and a label could not
say which column an edit landed on. GridLayout::record_col resolves it to a
RecordCol, and drill pending edits are keyed by (row, RecordCol) — which also
frees ApplyAndClearDrill from depending on the layout still being the drill
layout.

A blank model has no measures, so wide format alone would leave nowhere to
type a number and break the onboarding path. A trailing "+" column creates a
measure by name; being a schema change, it applies immediately rather than
staging inside a drill.

Formula targets are not columns — they are computed, so there is nothing to
type into them.

prune_empty's records branch went with it: the function early-returns in
records mode, so that tail could never run. Filed as improvise-2yk in case
the early return is itself the bug.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V69PGQfMJuGRmtSWpfVAqn
2026-08-29 12:29:31 -07:00
Ed L eae00522e2 Initial implementation of Improvise TUI
Multi-dimensional data modeling terminal application with:
- Core data model: categories, items, groups, sparse cell store
- Formula system: recursive-descent parser, named formulas, WHERE clauses
- View system: Row/Column/Page axes, tile-based pivot, page slicing
- JSON import wizard (interactive TUI + headless auto-mode)
- Command layer: all mutations via typed Command enum for headless replay
- TUI: Ratatui grid, tile bar, formula/category/view panels, help overlay
- Persistence: .improv (JSON), .improv.gz (gzip), CSV export, autosave
- Static binary via x86_64-unknown-linux-musl + nix flake devShell
- Headless mode: --cmd '{"op":"..."}' and --script file.jsonl

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-03-20 21:11:55 -07:00