User ruling. An agent proposed that a browser write through with ordinary
SetCell commands rather than staging, since those are already basis-checked.
The substitution does not hold: the basis check is anti-staleness coordination
between clients and answers whether a picture is CURRENT; staging is an
editing-session abstraction for one user and answers whether they want to KEEP
what they typed. Write-through would silently remove the ability to abandon a
drill -- invisible to any test that asks only whether an edit lands.
The convergence that makes it cheap: clause 4 is delivered, so a staged commit
is a batch of SetCells taking its atomicity from the transaction that already
exists.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Twelve facts across four PRs. The ones that generalise furthest:
a_completeness_check_closed_over_the_omitting_artefact_is_blind -- the parity
guard asked whether every RenderModel field is painted, and its domain was
RenderModel, the very thing that can omit a field. It reported a clean bill for
a whole wave while totals and page_coords were painted by nothing.
an_escape_hatch_is_where_a_new_check_goes_vacuous -- both bugs in a new guard
were in its exemptions, never its check. '&state.grid' is a substring of
'&state.grid.row_frame'.
a_cursor_rule_copies_between_panels_only_if_the_list_is_homogeneous -- the
obvious repair for uej8 is worse than the bug.
a_ticket_can_be_one_level_too_high_and_its_fix_unimplementable -- g3no's stated
fix could not work on the browser side twice over.
deliberate_slack_can_become_load_bearing_with_nobody_editing_it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
It claimed ViewBack/ViewForward were excluded from the wire as session-local
residue, like ApplyAndClearDrill and AcceptWizardModel. But view_back_stack IS
in ViewState -- the portable slice -- so by the view half's own derivation rule
they were always Commands and were simply MISSING.
The cost was a live divergence: effect::SwitchView pushes the leaving frame
while reduce_view's arm set active_view and nothing else, so no client ever had
a back history. That, not the drill commit the declined() comment blames, is why
view-back was dead in a browser.
Found by the agent on improvise-gd28 (PR #115) while checking the derivation for
COMPLETENESS -- it had only ever been checked for correctness.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Every `context/code/...` pointer added by this branch now names prose that
exists. protocol.md §2 carries why `ViewBack`/`ViewForward` CLOSE a gap in the
view half's derivation rather than widening it, why the fold and the effect had
diverged, and why a guard was written and then deleted. command.md carries why
`forget_vanished_views` and `adopt_deleted_views` do not subsume each other.
A pointer to prose that does not exist reads as a promise the explanation does.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Found reviewing the commit before this one, and the defect is that commit's
own: giving clients a back stack gave them dead frames.
`effect::DeleteView` moved `active_view` off the doomed view and left both
stacks alone. That was already wrong for the TUI -- `<` could reach a frame
naming a view the document had lost -- and invisible only because no client
had a stack at all.
Worse, the guard the previous commit added made it a loop. `apply_step`
corrected a dead landing by folding `SwitchView(default)`, which pushes the
dead view straight back; `<` then popped it, was corrected, and pushed it
again forever.
`ViewState::forget_view` is the one home, called by `effect::DeleteView` for
the host and reached by `Command::ForgetView` for every client. Which views
vanished is OBSERVED by diffing the name set across `apply_command`, for the
reason `adopt_cursor_epoch` watches a counter rather than a command variant.
`Sessions::apply_step` is DELETED. The pruning makes a dead frame
unrepresentable, so the guard had no reachable input, and a guard whose fault
cannot be injected is indistinguishable from one that always passes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
The code-review skill run against this branch, and the findings are about my
own tests rather than the code.
1. The formula sweep's predicate is (target_category, target), and nothing
discriminated the first half: a sweep on `target` alone passed every test
I had written. Two categories may legally hold a target of the same name,
so removing Metric/Profit must not take _Measure/Profit. Proved by
injecting `retain(|f| f.target != item_name)` and watching it fail.
2. Both `cycling_an_axis_...` tests read the status off `last()`, which a
resolver emitting the status and NOT the mutation also satisfies. Both now
assert the mutation is first.
3. Two things the fix leaves that nothing guards, both filed and both written
down rather than left in a PR body. improvise-nwgk:
doctrine(a_status_line_composed_beside_a_mutation_lies_when_the_mutation_no_ops)
asks a fallible mutation to RETURN whether it acted, and remove_item still
returns unit -- the reachable lie is gone, a wire RemoveItem naming nothing
is not. improvise-en4s: Dirty::of(CellData) reporting formulas: true was
slack over-reporting and is now load-bearing, because a delete can take a
formula out of the document; narrowing that arm would strand every client's
formula panel forever.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Self-review findings, applied.
The word `Total` is now spelled twice — `src/ui/grid.rs` and
`web/shell.js::gridPlan` — which fails design-principles §5's own test: would a
third front end write this again? Yes. It is kept local on the `indicator()`
precedent (the collapse glyph is spelled in both ends while the FACT is shared)
rather than built into a protocol string table for one word, and it is now
NAMED as debt at both the call site and in `context/code/clients.md` instead of
being left for the next reader to find.
`frameCostOf` now counts the totals strip, and that is a change to a rule
`context/code/clients.md` already states — so the section says why the strip is
reserved where the inline group rows deliberately are not: it is present
exactly once, never interleaved with data, so counting it is neither the
feedback loop nor the ratchet that argument rules out. The page bar needs no
rule at all, being outside the box `extentOf` measures.
The evidence file claimed three injections into `dispatch.test.js` that had not
been run. They have been now, and the claim is rewritten to match what actually
failed rather than what was planned — the third was `navigable: true`
unconditionally, not the `position !== ""` reading the text described.
`doctrine(an_injected_fault_must_be_the_fault_you_named)` applies to the
description as much as to the fault.
Injections run and read, all against the served copy or the source:
gridPlan takes the table row index -> both round-trip tests fail
whole() drops its empty-string guard -> both "means nothing" tests fail
navigable: true -> the Filter test alone fails
grid bound to `click` -> 4 Playwright tests fail, including
the pre-existing improvise-0x9u guard
paintPages never called -> the page bar assertion fails
the tfoot block never runs -> the totals assertion fails
page_coords deleted from render_model -> the Rust page-bar test fails
totals_strip returns Vec::new() -> the Rust totals test fails, and the
new cache-parity guard reports it
PageAxis::cycle replaced by items.is_some() -> the cycle index test fails
Two residuals named rather than discovered later: the `Total` spelling above,
and the page bar being display-only — clicking an entry does nothing, because
composing a page change from a click needs a `click_page` on the wasm surface
that does not exist.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Self-review under the code-review skill, two findings against this branch.
**doctrine(binding_dump_diff) had not been run**, and it is the doctrine
aimed at exactly this change: a green suite is not evidence for a keymap
refactor. Diffed: 84 added lookups, 0 removed, 0 CHANGED -- no binding
anywhere answers differently. 40 of the 84 are not a binding change at all
but the widened probe space hitting ImportWizard's catch-all on F(3)..=F(12).
`:` produced zero rows, because every mode that now inherits it already bound
it locally: the duplication was real and consistent, which is also why nothing
in the suite could have caught the two spines drifting apart.
**The fallback arm was vacuous.** `shadowing_answer` returned `None` for the
six non-shadowing modes, so the test asserted only that a global key resolved
to SOME plain command -- "is this key bound", where it means "does this key
reach the global". It now answers with the global's own command name, read off
the global rather than restated, leaving Help's `?` as the only hand-written
expectation because it is the only decision.
Proved by injecting the rival fix rather than only the bug: giving TileSelect
`Arc::new(global_keymap())` -- its own equal-but-separate copy -- leaves this
test GREEN, since every key resolves to exactly the right command. Only
`every_modes_keymap_chain_ends_at_the_same_global` fails, which is the
clearest statement of why that one compares by pointer.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
context/code/core.md gets the two model rules (why removing a name removes
the formulas targeting it, and why a required virtual category refuses) and
the axis rule (why leaving the constraining axes forgets the page selection,
and why the predicate is filters_data rather than 'is it Page').
context/code/clients.md gets improvise-koh6's decision in full -- including
why the state-dependence rule is about Upstream and not about outcomes --
plus the cursor rule and the shared axis sentence.
Evidence entries are module-qualified where they must be: three leaf names
are answered by a test in both resolve.rs and panel.rs, and one in both
model/types.rs and workbook.rs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Two user rulings, 2026-09-21: the state drill manipulates (the view stack)
is local to the client, and a drill is an ephemeral view. The first one is
what this lands; the second is scoped in the hand-back.
`effect::SwitchView` pushes the view it leaves onto `view_back_stack` and
clears the forward stack. `reduce_view`'s arm set `active_view` and nothing
else -- so a connected client's own history was empty however many views it
visited, and `view-back` had no stack to pop. Both write the same portable
field, and the fold is meant to be the mirror's copy of the effect.
`reduce_view` now mirrors all three effects line for line, and `ViewBack` /
`ViewForward` join the view half of `Command`. They carry no basis and dirty
nothing: the stack is in `ViewState`, so this is UI, not document.
The case a client-local stack creates is a frame naming a view somebody else
deleted. `Sessions::apply_step` guards the landing exactly as `apply_switch`
guards a switch, correcting the asker alone.
Also: `drill_constraint` and `drill_snapshot` get one home in
`improvise-core::view`, where the addressed-drill work will share them with
`DrillIntoCell` rather than freezing a second record order (gotcha 18).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
improvise-p7lh: `GridCache::page_coords` and `GridCache::totals` were projected
to every session and folded correctly by every client, and NO front end read
either. The shipped front page — `web/demo.improv` is `Quarter: page, Q1` —
drew a grid filtered to Q1 with nothing saying so, and `[`/`]` resolve locally
off that same `page_coords`, so a user could page through quarters watching
every number change with no way to tell which one they were looking at.
`RenderModel` gains `page_coords` and `totals`, fed straight off `state.grid`.
The Page/Filter split is asked of `PageAxis::cycle` rather than re-derived from
`items.is_some()`, which disagrees with it on an empty item list (gotcha 27).
The totals are RELAYED, never summed in the client: the sum runs over every row
including ones a viewport scrolled past (gotcha 44).
`web/tests/render_surface.test.js` gains the direction that would have caught
this when improvise-n10 added the fields. The existing guard is closed under
what `RenderModel` chose to expose, so a cache field that never reaches it is
invisible to it; the new one asks that every `pub` field of every widget cache
is read somewhere the browser runs, or is named as debt with a reason.
`grid.col_widths` is the one entry, pointing at improvise-pcje.
improvise-b8ot: the five clickable surfaces moved from one listener per node,
reallocated inside the render loops on every repaint, to one handler per stable
delegation root — the user's ruling. The operands are carried on the node as
data attributes and read back by a pure `cellAction`/`indexAction` in
`web/shell.js`, so the handler is a LOOKUP rather than a second derivation of
which data row was clicked: `gridPlan` counts data rows because a group header
is not one, and walking the table back up would disagree the moment one appears.
The editor's own keydown/blur stay direct — one transient node, one at a time,
and `blur` does not bubble.
All four documented behaviours a naive delegation loses are preserved and each
is now asserted in `harness/tests/contract.spec.js`: mousedown rather than click
(asserted by dispatching mousedown ALONE), `preventDefault` and only over a cell,
the conditionally skipped repaint (asserted by the editor's survival), and
closing a live editor before composing the click.
Gates: 1598 passed / 0 failed / 7 ignored over 37 binaries (baseline 1595);
node --test 47 over 6 files (baseline 36 over 5); Playwright 47 passed /
4 skipped (baseline 43 / 4); clippy, fmt and --check-evidence clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
`every_bound_command_name_is_registered` had two blind spots, both found by
injection rather than by reading, and both inside the region improvise-636d
works in.
Its walk matched `Some(Binding::Prefix(_)) | None => {}`, so it never
descended into a prefix sub-keymap: everything under `g`, `y` and `Z` was
unchecked in every mode, and twice over for RecordsNormal, which reaches
Normal's three through `Keymaps::prefix_keymap`. And its key space was a
second copy of `tests_support::probe_keys_pub`, identical but for the two
F-keys the public one carries, so a name bound on any function key was
invisible here while `dump_every_binding` probed it.
Two unregistered names -- one under `g`, one on `F(5)` -- left the workspace
byte-identical at 1595/0/7. The positive control separating "untested path"
from "broken test" is the same bogus name on `Char('~')`, which fails.
The walk is now `reachable_names`, recursing into `Binding::Prefix` and
terminating because `prefix_keymaps_are_flat` says the nesting is one deep.
`probe_keys` DELEGATES to `probe_keys_pub` rather than restating it, which is
the part that matters: two lists of "the keys a test probes" is the same
disease as two keymap spines, and the F-key gap was the drift between them.
`probe_keys_pub` now covers F(1)..=F(12) rather than the two that happen to be
bound, since the point is to see a binding nobody expected.
Acceptance is the two injections failing SEPARATELY -- a fix for one need not
close the other -- and both do.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
improvise-koh6 (P1, data loss): `N` is advertised on the browser's own help
overlay and resolved `Upstream`, which is a black hole -- the DOM was
byte-identical and the user's next keystrokes landed in NORMAL, where `i`
opens an editor pre-filled with the cell under the cursor. Measured: 18,400
became the text 18400on. `search-or-category-add` is now answered in
`composed` in BOTH arms -- the composable one builds the two commands the
browser already has, the other declines out loud. So the host-needed list
stays a property of the NAME, which is what
`enter_in_an_empty_view_panel_resolves_locally_rather_than_upstream` forbids
making state-dependent; the OUTCOME varying with state is what
`cycle-axis-at-cursor` already does two arms above.
improvise-g3no (P1): `Model::remove_item` did not drop the formulas targeting
the item, so a `_Measure` row synthesised from a formula (gotcha 7) was
re-synthesised the moment it was deleted and both front ends reported
success. `remove_category` has always swept its formulas; the item level now
does too, which is the missing half of that symmetry rather than a new rule.
The optimistic status both front ends compose is now true, so no ordering
change is needed.
improvise-uej8 (P1): deleting a category's last item left the cursor on the
NEXT category's header, one `d` from a `LayoutGeometry` change that rewrites
every view's axis map. Both front ends now retreat one row when the row
below is not a sibling item -- never unconditionally, or holding `d` would
walk backwards. The formula and view panels do not have this shape: their
lists are homogeneous, so a cursor that does not move cannot escalate.
Also guards the three required virtual categories at the model boundary,
which is where `d` at the panel's default cursor 0 was deleting `_Dim`.
improvise-z336 (P2): `View::set_axis` never touched `page_selections`, so
cycling a category off Page merely HID its filter and cycling back silently
reinstated it. It now forgets the selection unless the new axis still
constrains -- `Axis::filters_data()`, Page OR Filter, because gotcha 27's
trap is answering this with `categories_on(Page)` and dropping the
selection a drill put on `Filter`.
improvise-25bf (P3): `cycle-axis-at-cursor` told the user nothing where the
tile route said "Product -> None". Both routes now go through one home,
`viewmodel::axis_change_status`. This also answers z336's secondary defect:
the stale "Filter: X = Y" persisted precisely because the key that moved the
axis wrote no status over it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
`?` resolved in 2 of 17 modes: bound once on Normal and reached by
RecordsNormal through the single inheritance edge. It was missing from all
three panels, TileSelect, SearchMode, ImportWizard and Help itself.
Meanwhile `bind_panel_toggles` had two call sites and `:` was written into
two spines while `?` was in one — nothing kept the spines in agreement.
`global_keymap()` binds `?`, `:` and the three panel toggles, and all
seventeen modes are built `Keymap::with_parent(global)`. The answer to "which
modes must not inherit" is **none**: `lookup_local` is exhausted before the
parent, so the eight text-entry modes and SearchMode shadow every global key
with their own `AnyChar`, and ImportWizard with `Any`. Only Help writes an
override, and it is a decision — `?` closes the overlay.
The global binds Char keys only, because `AnyChar` is what does the
shadowing; F1 therefore stays on Normal although it runs the same command.
`TextEntry::with_parent` is folded into `new`, which now takes the parent
every text-entry mode has.
Four guards: the chain ends at one global (Arc::ptr_eq, not by value), the
global is Char-only and prefix-free, no mode binds a toggle of its own, and
every global key resolves in every mode and types where it must. Each was
proved by injecting its fault; evidence in context/test-evidence/protocol.md
records the two cases where a neighbouring test stayed green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
ChatGPT Codex on PR #111, correct. improvise-pv3g's claim is that a finite
input no longer aborts the process, and the two LIBRARY paths honoured the
refusal while examples/fit_csv.rs turned it straight back into a panic with
`expect`. A user with a 1e20 column still got a panic -- at a different site
than before, but still a panic, from the entrypoint a person actually runs.
Demonstrated first: a temporary test drove `run_fit` over an 8-row CSV at
1e20/4e20 and aborted at fit_csv.rs:399. `run_fit` now returns
`Result<(), String>` and `main` prints the refusal and exits 1. Gotcha 37(a)
read the right way round: the library returns data, the example owns every
message, and that includes the refusals. `fit_and_report` is the split that
makes it testable without a temp file, and the example carries `test = true`
so its tests run under `cargo test --workspace`.
A SECOND abort on the same path, found because the first "just under the
overflow" fixture used 1e18 and blew up somewhere else: at that scale the map
is finite, the fit lands, and `report_importance` dies with a capacity overflow
in `"#".repeat((share / 5.0).round() as usize)`. The share is `inf` because
`importance_ranking` computes `100.0 * imp[j] / total` -- multiplication first,
so an importance over f32::MAX/100 overflows before the division that would
have returned 100.0 (measured: imp=[3.7499995e36], total the same, share=inf).
The bar width is clamped and the arithmetic is filed as improvise-k844 rather
than fixed here: reordering it moves the bit patterns
`misclass_rate_and_importance_are_bit_for_bit_what_they_were` pins, and this
PR's headline evidence is that none moved. AGENTS.md's own rule -- a finding
can be correct and still be wrong to fix in that PR.
Three guards, each failing under its own injection: restoring the panic,
refusing every map, and removing the clamp.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
The field doc named two sources and an order that the improvise-e8no splice
changed. Third review finding: a doc comment stating an order is a claim, and
this one was one commit stale.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Self-review round on wave/o-gp-fit, three findings.
1. The e8no evidence claimed the dropped-column fit "lands on the measured
pre-fix champion again". Printed it: the threshold and the high leaf are the
probe's, the LOW LEAF IS NOT (1090.28 against 970.3), because pre-zv8t the
column was present as one inferred numeric feature and the draw sequence
differed. Exactly the unchecked-cost-claim shape the same file was written
to record. The claim that survives is the internal one, which the test
asserts: ignoring the column and marking it a Label give the same champion.
2. `a_target_just_under_the_overflow_still_fits` looped over `leaf_numbers`
without asserting the walk found any, so an emitter change that stopped
reaching the leaves would have made it pass vacuously.
3. `unaffordable_categories` drops by NAME while budgeting by INDEX, which is
`doctrine(forcing_by_name_while_budgeting_by_index_defeats_the_budget)`. It
is safe by the same DATA DEPENDENCY that doctrine's own repair used --
`overrides` does not exist until `as_categories` returned Ok -- and gp.md now
says that rather than "a `?` puts it there". Also recorded: an over-count in
the advisory preflight used to cost a needless refusal and now costs a
needlessly dropped column.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
improvise-pv3g (P1). `fit-measure` panicked in `number_text` with a NaN leaf on
an ordinary document whose target values were large but finite: 8 records at
1e20/4e20. Every input was finite and `fit_measure` checked exactly that; the
overflow was derived. `standardize` sums (t - mean)^2 in f32, 8 * (1.5e20)^2
exceeds f32::MAX, `std` came back inf, every normalized target became +/-0, and
`denormalize` computed 0.0 * inf + mean = NaN two crates away from the
arithmetic that caused it.
`Scaling` has private fields and one constructor that refuses a non-finite part,
so `standardize` returns `Option<(rows, Scaling)>` and `denormalize` cannot be
handed a poisoned map at all. `cv_regress` pools nothing for a fold it cannot
standardize -- the silent half, which produced R2 numbers with no panic to
announce them. The arithmetic is untouched: every bit pattern in
`scoring_known_answers` is unchanged.
The ticket's sibling path is corrected in `context/code/gp.md` SS7: a finite
target with an infinite mean gives +inf variance, never NaN, so the
`NaN.max(1e-6)` laundering it describes needs an infinite input the existing
guard already refuses. Measured by injection.
improvise-e8no (P3). The width preflight refused the whole table when one
category was too wide, so a document that fitted before zv8t stopped fitting.
`encoded_width` became `encoded_widths` and `unaffordable_categories` drops
category columns widest-first into `Overrides::ignore` until the rest fits,
reporting each as a `Dataset::warnings` notice -- data, never printed. The
encoding decision is untouched: a category is still never inferred numeric and
the forcing is still unconditional. `context/code/gp.md` SS11's "declined"
verdict is superseded there, with the reasoning under it kept.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Integration review of 455e8614 found clause 4 delivered for the document
and the mirrors and NOT for `App::view_state`.
F1. `restore_document` said `view_state` was "deliberately untouched",
citing improvise-yqpd's note that rolling back `status_msg` would undo
an AddCategory refusal's explanation along with the damage. **That note
is about an apply_effects-level rollback, which is not what landed.** At
the FRAME level the asker receives ServerFrame::Rejected(reason) — the
explanation is delivered on the wire, addressed to the client that earned
it — so the host's status line has no explanatory duty for a client's
frame. The host's own key path is untouched and still aborts without
undoing, exactly as gotcha 21 describes.
Measured consequences, all now closed: the TUI host's cursor still moved
and cursor_epoch was still bumped; a rolled-back LoadModel left
`view_state.active_view` naming a view of the DISCARDED document; and
`status_msg` kept the rejected frame's sentence, so a frame that did
nothing claimed it had loaded a file.
The fixture had to be renamed before any of that was visible.
`host_over_a_loadable_document` built its incoming workbook with
`Workbook::new`, whose default view is `Default` — the same name the
outgoing document uses — so a ghost active_view was indistinguishable
from a correct one. Gotcha 39's trap one level up. Checked first that
neither user of the fixture depends on the names matching, and that
gotcha 39's own same-name fixture lives in project.rs and is untouched.
Restoring the whole slice cannot clobber a TUI user's in-progress typing:
`Host::commands` is synchronous, so snapshot and restore bracket only the
frame's own writes.
F3. `batch_aborted` was left true after a rolled-back frame — inert, but
the same shape as the stale-flag bug the previous commit fixed. `App`'s
doc comment names four more "last batch" fields and calls
last_reshaped_views "the shape batch_aborted takes"; restore cleared
three of the four and snapshotted none. All six now round-trip, which is
why DocumentSnapshot is renamed AppSnapshot: it is no longer only the
document.
F2 is NOT closed and is stated as scope in the evidence and the PR body:
broken_chain asks records_named, so a frame can still apply partially and
silently through a command that names no record.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Integration review, cross-branch against #108. My asymmetry argument was right
-- a &str dispatch can only have GRAMMAR_INVARIANT, a Rule dispatch can have a
compile error -- and I then applied it to the two &str sites and NOT to the
Rule site where the stronger technique was available. build_primary kept its
unlabelled `else`.
Demonstrated, not argued. With `pct_literal = { "#" ~ identifier }` first in
`primary`: the tree built clean, all 85 crate tests passed including my three
new 492x guards, and parse_formula("X = #Revenue") returned Ref("Revenue") --
the `#` discarded, the formula silently wrong. The injection must be
NON-ATOMIC: an atomic rule is caught by accident in first_inner's own
GRAMMAR_INVARIANT and would have reported the code safe.
Sweeping for the shape found three more in the same file, two genuinely silent:
build_formula's if-chain-in-a-loop dropped an unhandled part (its comment said
"Rule::EOI and any silent rules are ignored"; EOI is now an ARM, so ignoring is
a decision), filter_value_to_string returned unprocessed text, and build_unary
was caught only incidentally downstream. All four now match over Rule with an
or-pattern over the rules that cannot appear, so ANY new rule in formula.pest
is a build failure at all four until someone decides what it means. Verified by
injecting three more probe rules: every site names all three.
The greppable invariant: `== Rule::` now appears nowhere in parser.rs.
each_primary_alternative_builds_its_own_expr is the anti-clamp guard -- a
mechanical if/else-to-match conversion can transpose two arms and still
compile, since the compiler checks that every rule is NAMED, never that it is
named against the right body. Proved by swapping the string and pipe_quoted
arms: it fails and nothing else in the suite does.
Also corrects context/code/formula-and-io.md, which still called the final
`else` arms "unlabelled-but-safe" on the grounds that COUNT was the only
remaining agg_func and >= the only remaining cmp_op. Two of those three clauses
were made false by my own earlier commit. Each was true OF THE GRAMMAR AS IT
STOOD -- a snapshot silently re-read as an invariant, which is the whole defect
in one sentence.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Integration review of #106. Three findings; two applied, one does not reproduce.
1. AxisFrame::group_of had NO coverage of its GroupEnd arm. Replacing it with
`FrameEntry::GroupEnd => None` left all 133 improvise-protocol lib tests
passing — only tests/grid_frame_agreement.rs in the root crate caught it,
and that file does no pruning. So improvise-gd83's property reached the
protocol by construction through project.rs and never by assertion: a
derivation living in two crates with one of them unchecked, which is what
gotcha 44 and design-principles §5 exist to prevent.
Two tests in render.rs over the exact frames prune_axis_items now produces
— the re-opened run closed by a GroupEnd, and the collapsed header with no
item under it. The second also asserts collapsed: true travels, because a
header arriving without the flag is there and inert, which is lhi4 again
one layer out. The injection now fails both where it failed nothing.
This is the landed half of u6gc's AC3. The other half — that
prune_axis_items -> axis_frame -> group_of really composes to these frames
— still needs project.rs and stays filed rather than landed.
2. a_collapsed_groups_header_survives_pruning passed with prune_empty disabled
entirely: its fixture funded every item, so nothing was ever pruned and the
name claimed more than the test could see. It now carries an unfunded Mar,
so turning pruning off leaves DataItem(["Mar"]) in the asserted sequence and
the test fails. Same vacuity class as the any() -> whole-sequence fix in
4075d66.
3. NOT APPLIED — "no fault separates the collapsed test from the GroupEnd
test" does not reproduce. Re-measured all three faults over all four tests:
inverting close_pending_run's condition to `had_item` fails the collapsed
test while the GroupEnd test PASSES, because a re-opened run is closed by
the GroupEnd arm whatever the header condition says. The evidence now
carries the full 3x4 matrix rather than prose, so the claim is checkable
instead of assertable, and every column is distinct.
Also moves wide_cell_clamps_without_overflow's evidence from core.md to
protocol.md. improvise-yae4 touches viewport.rs and render.rs and does not
touch improvise-core at all, so core.md was the wrong home; it was put there
by hand and the checker cannot see a misfiled entry — it asks whether a
heading names a real test, not whether it sits in the right file.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
`harness/opfs-contract` carries its own [workspace] table, so it is not a
member of the root workspace -- deliberately, since it names
improvise_io::opfs and a member would break `cargo test --workspace`. The
consequence is that `cargo fmt --all -- --check` passes over it without
looking, so every gate I ran was honest and green while the file I had
edited was never visited.
It needs its own gate, as CI runs it:
cd harness/opfs-contract
cargo fmt -p opfs-contract -- --check
cargo clippy --target wasm32-unknown-unknown --all-targets -- -D warnings
The gap is bounded: grepping for crates carrying their own [workspace]
table finds exactly this one and the root.
And a fmt failure is the FIRST step of that job, so the clippy behind it
never ran and nobody knew whether it was clean. It is -- verified locally
at this head, no warnings.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
The send guard was written, argued in a comment and never run, which
AGENTS.md forbids: a fix needs a test that failed before it. Proven
against the pre-fix silent drop -- status stays "disconnected" and the
edit vanishes.
The first version of this test was VACUOUS and failed for the wrong
reason. It pressed ArrowDown, which on a client with no snapshot composes
no frame at all, so transport.send was never reached and the guard under
test never ran. Enter is forwarded upstream and composes a frame
regardless of client state. doctrine(an_injected_fault_must_be_the_fault
_you_named) seen from the other side: the TEST must reach the path.
The closing assertion keeps improvise-taxx intact -- the drop must still
be named first. A loud send may add "your edit did not go", never replace
"disconnected" with something vaguer.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Both were in the code that had just landed, and both are the repo's
characteristic defect: a check adjacent to the thing it names.
1. `broken_chain` carried a create-exemption copied from the pre-frame
check, and the hand-back recorded that arm as "correctness-preserving but
unobservable" because removing it failed no test. Wrong in both halves.
`[AddItem(West), HideItem(West), SetCell(West)]` observed rejected=false
landed=false — the frame reported ACCEPTED with the write silently gone,
which is exactly the partial-and-silent failure clause 4 exists to
eliminate, surviving inside clause 4. `resolve_cell` is gated by
`resolve_record`, so a non-resolving record is precisely the condition
under which `model_effects` returns an empty vec; and SetCell/ClearCell
are the only variants `records_named` names, so refusing is strictly
better than dropping. The exemption belongs at CHECK time — where a
record the frame will create legitimately does not exist yet — and
nowhere else. kb.pl's the_create_exemption_is_a_property_not_a_list is
unchanged: it describes `stale_records_in_a_frame`, which keeps it.
2. `batch_aborted` is reset ONLY inside `apply_effects` (gotcha 21), and
the frame loop read `last_batch_aborted()` after every command. A command
running no effects — every view command, and SwitchView, which
`apply_checked` routes past `apply_command` — leaves the flag where some
earlier batch left it. So a TUI `:w /bad/path` made the next frame a
browser sent refuse itself. Guarded per command, with a mixed-frame arm
because frame-level gating alone does not reach `[SetSelected, SetCell]`.
The same Command::Model-presence predicate now spares a frame of pure
cursor keys both the Workbook snapshot and the Sessions clone — the
browser's common path, one frame per keypress.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Self-review by fault injection, not reading. Three findings, one a real bug
this branch would otherwise have shipped and two vacuous tests of my own.
1. A REAL BUG THIS BRANCH CREATED THE DURABILITY OF. Model::remove_item left
measure_agg holding an entry for a measure it had just deleted -- exactly
improvise-qx1z's defect one layer down, in the same branch that fixed the
View half and did not look at the Model half. Harmless while improvise-ppru
was unfixed, because nothing persisted the map; persisting it turned a
stale entry into a permanent one, so the document grows an Aggregation line
naming a measure it does not have and re-adding that measure brings back an
aggregation nobody asked for. A fix that introduces a format for state that
outlives what it names is how a latent bug becomes a permanent one, so the
sweep ships with the writer.
2. A VACUOUS CONTROL. a_file_naming_no_view_keeps_the_seeded_default PASSED
under the over-broad "always drop Default" it exists to catch, because
delete_view refuses the LAST view -- a guard one level away from the
condition under test. It answered "does delete_view refuse the last view"
where it meant "does drop_unnamed_default decline to fire". Split in two:
a discriminating case naming Default AND a sibling, where the wrong
condition really deletes, and the legacy no-views case kept separate and
marked as the weaker statement it is.
3. A VACUOUS NEW TEST. a_category_named_prune_is_not_read_as_the_prune_flag
passed under the rejected presence-style grammar that swallows "prune: row"
whole, because the fixture used Row and on_category_added RE-DERIVES Row for
the first regular category on reload -- the axis matched by reinvention, not
by being read. Now uses Page plus a page selection, neither of which any
default supplies.
Also verified and unchanged: the generator reaches all three new grammar
branches, including the scalar data line, each proved by a separate injection;
awkward measure names (|A: B|, |sum|, true) round-trip through the Aggregation
section; and improvise-nvq4 still does not reproduce.
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
Self-review under the code-review skill, attacking my own three prune tests by
trying to build a wrong prune_axis_items that passes all of them.
The gap it found: all three fixtures put a group in an ALL-or-NOTHING state —
fully collapsed, fully pruned, or intact — so between them they said nothing
about the mixed case, which is the one a user meets first. A group where some
items prune and some survive now has its own test.
a_collapsed_groups_header_survives_pruning now asserts the whole row_items
sequence instead of any(|e| is the Q1 header). An any() cannot see a header
emitted in the wrong position or an extra entry beside it.
The attack itself found no defect, and what it established is worth more than
if it had. Rewriting run_had_item to mean "some item was REJECTED" rather than
"there was an item" is a no-op with the whole workspace green: the header is
pushed eagerly by the FIRST SURVIVING item, so pending_header is already None
before any rejection can be recorded, and the flag is only ever read in runs
where nothing survived — where the two readings coincide. The eager push is
what makes the collapsed/pruned distinction robust rather than delicate.
Also recorded, because it bounds what the new assertion may claim: two
consecutive GroupEnds are not reachable, since expand_category never emits
them, so "no stray GroupEnd" is a property of the writer and not something any
fixture can falsify. The commit that introduced the run_open tracking claimed
it as a virtue; it is one, but it is argued and not tested.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Self-review under the code-review skill, two findings, both narrow.
format.rs: the pikk test asserted a finite number never renders as inf and
that half-away rounding still happens, but said nothing about a genuine
f64::INFINITY. Proved the two assertions ask different questions by clamping
round_half_away to a bare passthrough: the rounding assertion fails (2.5 reads
"2"), the infinity one does not.
test-evidence: named the loop arm that looks reachable and is not -- a pass
computing None where the cache holds Some would spin to the bound and now be
marked circular. The only None evaluate_formula_cell can produce is a WHERE
filter that does not match falling through to aggregate_raw over no data, and
aggregate_raw reads self.data rather than the cache while the formulas do not
change during a recompute, so a Some-to-None transition cannot occur. Also
recorded the free cross-check between two of the fixes: the s08t test asserts
ref:Typo AFTER recompute_formulas, so a formula erroring identically every pass
must be seen to converge rather than be overwritten with circular.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
The P1's headline scenario was still argued rather than run: the read-only
path's components were verified (the harness probe, the Rust unit tests)
but the two-tabs-one-document flow it was reported as was not.
Two tabs in one context. The assertion that matters is that the second
one PAINTS -- boot succeeds without a handle -- then that it says
read-only and names another tab, then that the reason still stands past
the 8s window rather than being replaced by a watchdog blaming the
browser. `timesArmed() > 0` first, or that denial is vacuous.
Chromium and Firefox, because the exclusivity semantics are the engine's
and the two word the refusal completely differently.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
improvise-1wft and the five beads it blocked were one failure: nothing
carried a boot failure's REASON from the worker to the page. The channel
is one rule -- a protocol frame crosses the port as a STRING, a shell
notice as an OBJECT -- so it needs no protocol change and the
discriminator is total.
A boot failure is a STATE, not a rejected promise. A document another
context holds the exclusive handle to now OPENS read-only: measured first
(harness probe, Chromium + Firefox), getFile() reads the bytes while the
handle is held, with the second-handle refusal asserted so the probe
cannot pass vacuously.
- improvise-1wft the channel: Notice/NoticeKind, boot_notice, take_notice
- improvise-i0v9 read-only boot over vfs::ReadOnly + OpfsFs::read_unattached
- improvise-86nw start() rejecting for any reason now posts BootFailed
- improvise-9cma boot::document_name -- and a/b/c.improv was ACCEPTED as
c.improv, not rejected, which is worse than the report
- improvise-iobc vfs::Origin: a seed error is a notice only when the
document really had to be created without it
- improvise-vep8 the watchdog arms BEFORE transport.ready; its message
stopped claiming the connection was accepted
- improvise-7gip DedicatedWorkerGlobalScope (needs a web-sys feature)
- improvise-zre1 a persist that fails raises a notice; the trigger is the
dirty flag surviving, observed rather than predicted
- improvise-7gf8 the render breaker trips once instead of retrying forever
- improvise-ociy non-OPEN sends are loud; a viewport announcement is not
- improvise-y01k the three ws-server drops are logged
- improvise-r4nf serde failures throw instead of reading as "nothing to send"
Every notice routes through shell.fail, so gotcha 48's disarm half is not
re-broken. Two comments in shell.spec.js stated the opposite of the new
arming order and were corrected rather than left.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
improvise-el9h. `ClientSession::fold` settled `view.mode` against the grid
that session was LAST projected, and `project_diff` then assigned
`self.grid = desired` and never settled again. The client has the other
shape — `ClientState::fold` settles on every fold, `Command::Project`
included — so for `[SwitchView(records-shaped view), ChangeMode(Normal)]`
the mirror kept `Normal` while the client ended on `RecordsNormal`.
The test came first, as the ticket demands, and failed exactly that way.
`settle_mode` is now called from `fold` and at the end of `project_diff`
once the grid has moved, which also covers the three doors that write the
mirror with bare `reduce_view`: whatever they leave, the next projection
settles.
The fixture's own precondition earned its place on the first run — a view
built with `View::new` carries no axis entry for `_Index`/`_Dim`, so
`is_records_mode` can never fire on one, and without the assertion the
test would have compared two `Normal`s and passed against the bug.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Keyed by test name per design-principles section 7, recording for each one the
bug it reproduced, the fault injected to watch it fail, and -- where a test has
a neighbour that looks like it -- the input that separates the two.
Three entries are about that separation rather than about a bug:
- a_view_that_does_not_prune_writes_no_prune_line is the direction the default
supplies, so a writer emitting "prune: true" unconditionally passes the real
test and fails this one.
- an_interleaved_category_keeps_its_item_order explains why the ticket's own
two-item reproduction cannot discriminate: with one item of each kind, both
the right and the wrong order are a single bare run followed by a single
grouped one.
- slash_in_hidden_name_survives_roundtrip records that improvise-nvq4 does NOT
reproduce, and is kept as a passing control so the claim cannot be re-filed
without someone first watching it pass.
core.md is touched for the improvise-qx1z entries, which are View-level and
belong there rather than in formula-and-io.md.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
CROSSES TWO BOUNDARIES OUTSIDE THIS AGENT'S GROUND, deliberately and reported:
web/demo.improv and harness/tests/contract.spec.js. Dropping the invented
Default view takes the shipped demo from two views to one, and the spec
asserted the phantom in two places -- improvise-m4rv's acceptance criteria
name the spec directly.
The load-bearing half is the improvise-iioi click test, which needs the view
panel to hold two rows so that a click can name a DIFFERENT row than the
cursor does; with one view both readings give the same answer and the test
stops discriminating. The phantom was supplying that second row by accident.
So the demo gains a real second view, Season -- a genuine alternate pivot
(Quarter by measure, paged by Product) rather than an auto-arrangement. Plan
stays first and stays Initial View, so the cursor still starts at index 0 and
tests/web_demo_model.rs is unaffected: every visible cell of the ACTIVE view
is still a Record (gotcha 46). Verified by cargo.
The spec's axis assertion at index 0 moves from the phantom's "R:Product
C:Quarter" to Plan's own "R:Product P:Quarter", derived from
viewmodel::view_axis_summary rather than guessed.
NOT VERIFIED: harness/ has no node_modules and web/pkg is unbuilt here, so
Playwright could not be run. These edits are mechanical substitutions and
need a browser-job run before they are believed.
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
improvise-qx1z, two arms of one asymmetry.
Arm 2: View::on_category_removed cleared category_axes, page_selections and
hidden_items but not collapsed_groups — the other three lines being right
there is what makes it an omission. Re-adding the category brought the group
back pre-collapsed.
Arm 1: Model::remove_item touches no view at all, so hidden_items kept the
removed name. Re-adding the item put it in the model and the category panel
while leaving it absent from the grid with no indication why. The sweep is
View::on_item_removed, called from Workbook::remove_item over every view —
Workbook is already the stated home of cross-view ops (gotcha 6). It also
answers the third question the ticket asked to check while there: a page
selection naming the removed item goes too.
Both reachable from shipped keys. Each test fails under an injected fault,
and each fixture carries a second item or view so that "forget this item"
is separated from "forget this category".
Also lifts the "Default" view name to view::DEFAULT_VIEW, which parse_md
needs in order to tell a seeded view from one a file asked for.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Edward Langley