8ad5c0806db558749c10d753b02aeb065ca11a29
72 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Edward Langley
|
8ad5c0806d |
feat(web): dismissable notice, a raised minibuffer, and a styled shell
Three things the user reported after looking at the deployed page, in the order they cost a visitor something. The deployment notice was ten dense lines of configuration reference — ?doc=, ?seed=, ?ws=, wss:// and reverse proxies — above the tile bar, the category tree and the grid on every visit: about 230px of an 820px window. It is now a <details>, so the collapsing is the BROWSER's (keyboard operable, and correct with app.js absent or still loading) and the only thing the page adds is remembering the answer. It starts COLLAPSED, and collapsing withholds nothing: the one fact a first visitor needs — the document lives in this tab, is saved to this browser's private storage and never leaves the machine — is the <summary>, which never collapses. What a stored value means is a decision and lives in shell.js; touching localStorage is a browser API and stays in app.js, wrapped, with every value that is not one of the two spellings read as "unanswered". The minibuffer was a 14px line in the page's bottom-left corner, up to 800px from the cell being typed into. It is now a raised bar over the grid. The CONTENT is untouched — RenderMinibuffer is built from `state.view.mode.minibuffer()`, the same semantic value the TUI draws — so this is position, weight and contrast, and there is no second derivation of what a prompt says. MinibufColor still resolves here exactly as draw.rs::minibuf_fg resolves it for the terminal. style.css stops being "legibility only": layered surfaces, one accent, a filled badge for the mode, chips for the two axis strips, one selection treatment, tabular numerals in the grid, and a focus ring on the one keyboard surface the page has. The palette is the landing page's, so the two read as one product. Asserted (harness/tests/contract.spec.js, Chromium): the notice starts collapsed and remembers, in BOTH directions across a reload; it opens from the keyboard, which the page's own keydown handler must not claim; the minibuffer is inside the viewport, OVERLAPS #grid — the assertion a static bottom bar cannot satisfy — and is larger than body text. Each was proved discriminating by injecting the fault and watching it fail. NOT asserted, and named as such in context/code/clients.md: colour, contrast, spacing, weight, the grouped and multi-level header DOM, either theme, and the localStorage-throws path. Those are argued and read. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Ed Langley
|
9eb13119ae |
Merge pull request #114 from fiddlerwoaroof/wave/s-resolve-keys
CI / Build all bundles (push) Has been cancelled
CI / Test and lint (host) (push) Has been cancelled
CI / wasm32 check (browser bundle crates) (push) Has been cancelled
CI / Browser JS unit tests (node --test) (push) Has been cancelled
CI / Knowledge base consistency (push) Has been cancelled
CI / Lint the harness crate (push) Has been cancelled
CI / Browser harness (OPFS contract + shell) (push) Has been cancelled
CI / Deploy to GitHub Pages (push) Has been cancelled
fix: five category-panel and axis defects, four of them P1 (koh6, g3no, uej8, z336, 25bf) |
||
|
Edward Langley
|
aeff017626 |
fix(panel): a singleton category's item reached the escalation anyway (Codex P1)
A ChatGPT Codex review of #114 found the cursor fix wrong for a category with exactly ONE item, and it is this PR's own argument turning against it: that is the configuration where the conditional retreat degenerates into the unconditional step-back the PR argued was worse than the bug. No sibling below, so it stepped back onto the category's own header -- one d from RemoveCategory. Reproduced on both paths first, asserting the CONSEQUENCE rather than a landing index: rebuild the post-deletion tree, seat the cursor where the command put it, and ask whether a second d composes a RemoveCategory. It did, on both. The enumeration is what makes the second fix total. CatTreeEntry has two variants and flatten_cat_tree emits a header then its items, so after deleting the item at i: below is a sibling -> the list closes up, stay; above is a sibling -> step back; NEITHER -> the category is now empty and both neighbours are Category rows, so there is no safe ADJACENT landing at all. That third case is structural, which is why the rule could not be patched in place. The landing is therefore the nearest Item row, preferring above then below -- never a header. Cost, stated rather than hidden: the cursor may cross into another category, and a held d then deletes one item per press, each with its own status line, walking upward. It never reaches RemoveCategory, which is the criterion the review asked for. Landing on the emptied category's own header was rejected even though RemoveCategory there is cheap -- an empty category has no cells, and a non-measure formula target IS an item, so no formulas. That is a harm-magnitude argument where the repo prefers the bad state unreachable. Residual, stated: if the deleted item was the only item in the whole tree there is no Item row anywhere and the cursor does not move. Every category is empty then, so no landing exists that would avoid it. Both fixtures are fenced so they cannot pass for the wrong reason. The browser one expands BOTH categories, because a first version expanded only the singleton's and the cursor fell off the shortened tree, where the delete arm answers that there is no category to delete -- green for a reason unrelated to the fix. The TUI one seats the two-item category FIRST, so the backward search (the preferred direction) finds a real landing. Injected the pre-review rule on both paths: each new test fails, and the two adjacent-case tests still pass. The singleton is the discriminating input and nothing else in the suite sees it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
56b3058136 |
docs(browser): the arguments for the page bar, the totals and the delegation
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
|
||
|
Edward Langley
|
e2417414ad |
docs: the arguments the new comments point at, and the test evidence
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 |
||
|
Edward Langley
|
a93d668e5e |
feat(browser): paint the page bar and the totals strip, and delegate the pointer
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 |
||
|
Edward Langley
|
a633613e56 |
test(browser): improvise-ociy had no test; its own review found that
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 |
||
|
Edward Langley
|
4c15f081d3 |
test(browser): improvise-i0v9's two-tab case, end to end in both engines
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 |
||
|
Edward Langley
|
12714c7527 |
feat(browser): a boot failure is a state with a reason, not silence
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
|
||
|
Edward Langley
|
3d0850c132 |
Merge main (d7bfcbb, PR #96) into wave/operandless-commands
Two textual conflicts, and one the merge resolved cleanly and wrongly. effect.rs: #96 extracted the LayoutGeometry list into layout_geometry_cases() shared by two tests, one of which compares it against the wire vocabulary; my side had added ViewTarget arguments and carried the base's assert_eq!(cases.len(), 13). Kept #96's structure with my arguments and dropped the count -- the list-versus-vocabulary check is strictly stronger and a count beside it would be a second answer to the same question. Verified the count came from the merge base, not from my side, before dropping it. test-evidence/command.md: both sides appended entries at the same point. Kept both. app.rs auto-merged CLEANLY and did not compile: improvise-4dpj's new a_batch_that_leaves_records_settles_out_of_records_normal constructs effect::SetAxis, which now carries a view operand. Fixed with ViewTarget::Active -- the host's own key path, which is what that test is. Checked rather than assumed, per the brief: - Test names diffed BOTH ways against the union of the two parents, not the count. Union 1510, merged 1509, exactly one name absent and it is the intended one: a_hosts_none_cats_leak_into_another_views_formula_columns, the ac2z characterization test my own branch replaced with a_formulas_columns_survive_a_host_that_aggregates_differently. Nothing else lost, nothing unexpected gained. - #96's the_enumeration_names_every_variant_the_derive_declares is still discriminating against the reshaped ModelCommand variants: dropping TransposeAxes from every_model_command() fails it by name. - The test-support feature stays off in shipped builds: cargo tree --edges normal --target wasm32-unknown-unknown finds no serde_json. And the finding the brief asked for, which is the opposite of a disagreement: #96's switching_views_across_records_ness_settles_against_ the_view_switched_to is the HOST-side twin of the client fix in this branch. Same batch, same crossing in both directions, and independently the same phrase in its message. Neither is redundant -- nothing in the projection stream carries the host's ViewState, so a host-side guard cannot see a client's mode or the reverse. Cross-referenced in context/test-evidence/clients.md so neither is deleted as a duplicate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
81fe546594 |
Settle the mode by observing the grid, not at each composition site
ChatGPT Codex's P1 on #98: switch_view_at composes [SwitchView(target), ChangeMode(Normal)], and changed_mode settled that against the grid the client STILL HOLDS -- the outgoing view's. Records -> pivot emitted RecordsNormal and pivot -> records emitted Normal, which is the cross-view drift this PR exists to close, by a second door. The hand-back predicted this as follow-up 3 and called it unreachable because R is a stranding decline in the browser. That is right about the TOGGLE route and wrong about this one: records-ness is auto-detected from the axes (gotcha 1), so a view switch changes it with nothing in the batch saying "records". One door was enumerated and there were two. So the remedy is general rather than a patch to switch_view_at. changed_mode is DELETED; every site emits the plain AppMode::Normal; and ClientState::fold settles after every fold against the grid it HOLDS. Route-independent, so no enumeration of composition sites is load-bearing any more -- the same shape the repo already prefers in cursor_epoch over matching on CycleAxis, document_generation over matching on LoadModel, and the dirty flag over a table of command variants. improvise_protocol::settled_mode is the one rule and all three ends call it: ClientState::fold, ClientSession::fold (the server's mirror, so the two cannot diverge in a field nobody currently reads today) and App::settle_normal_mode. App::normal_mode is gone -- it was that function's host-side duplicate, and my change orphaned it. Bidirectional coverage drives a real Host: a session connected to a records-mode host, the pair folded locally and sent as a ClientFrame::Commands, every reply delivered in arrival order. The composed ChangeMode carries the STALE answer deliberately, because composing the friendly value would let the records -> pivot direction pass under the fault. Both directions fail with settle_mode no-opped, each proved separately since the loop stops at the first. Evidence, the injections and why the first fix's unreachability argument was too narrow: context/test-evidence/clients.md, context/code/protocol.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
ab6bb41da3 |
Review of this branch: two checks that could not fail, one drift left live
changed_mode settled only Normal -> RecordsNormal, but the host's
settle_normal_mode settles BOTH members of the pair, so a RecordsNormal
over an ordinary grid stayed put on the client and became Normal on the
host. Esc out of RecordsEditing resolves through entered() to
`enter-mode records-normal`, so the mirror-image drift was reachable by
the same route the fixed one was. Now total, with the pass-through arm
and both negative rows asserted.
a_compacted_seed_folds_to_the_same_cache_as_the_whole_history left
GridCache::records at its default, so a seed() that OMITTED the arm
compacted to an equal cache and the check could not see the omission.
The history now sets it; deleting the arm from seed() fails the test.
grid_updates' records arm had no test at all: grid_of setting the field
is covered by lockstep, but a client that connects before records mode is
entered learns only through the diff.
Evidence for all three, with the injected faults:
context/test-evidence/{clients,command}.md.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
|
||
|
Edward Langley
|
618801701e |
The browser asks the grid what kind of grid it is (improvise-e5pf)
One substitution at three decision points: wherever the host asks layout.is_records_mode(), resolve.rs asked state.view.mode.is_records(). The two agree only while the mode and the layout agree, and Command::ChangeMode(AppMode::Normal) is the message that separates them -- the host settles Normal against the layout the batch ends on, reduce_view folds the variant it was handed. GridCache::records carries the fact, projected by grid_of off layout.is_records_mode() and diffed like the extent. Deriving it on the client from row_frame.cats would have been a second implementation of is_records_mode (improvise-edp's disease); deriving it from the mode is the bug. Like Extent it frames nothing, so apply does not re-fit and its batch position is not load-bearing. The client's settling has one home, resolve::changed_mode, reached by every site that can emit AppMode::Normal -- including entered(), which is how a bound `enter-mode normal` gets there. The three decision points ask state.grid.records, so a commit at a records cell declines rather than composing typed_cell_command against a grid whose columns are [coords.., +cat, measures.., +]. lockstep now DRIVES the keys: in_lockstep presses on both ends and refreshes the client from a real projection rather than from the host's slice, which is what let the old records coverage start from agreement. The reachable path is Tab (category panel) then Esc; c/f/v are Ignored in RecordsNormal and R is a stranding decline, which a first draft of the test found the hard way. Two resolve.rs fixtures put a records mode over a non-records grid -- a state no host can produce -- and had to be corrected with the fix. Evidence and both injected faults: context/test-evidence/clients.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
46c3cfa158 |
docs: two things self-review turned up
The lockstep fixtures no longer fabricate a basis, but they do still assume a cursor at the origin: published_basis asks Sessions::connect for a window at offsets zero while client_of builds its grid at the cursor's offsets. They coincide in every fixture here. Recorded rather than closed. And the keypress path got improvise-jmjz's fix for free: compute_col_widths is one walk now rather than two, so App::apply_effects' own width computation roughly halved (134ms to 68ms at 18,000 cells). That is improvise-748f's held-key path and was not the ticket's subject. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
e3fcdc8bc6 |
test(lockstep): take the fixtures' basis from the server, not from a recipe (improvise-1l9o)
furnished_client and client_ready_to_edit built their BasisCache by walking the client's own grid for Record cells and calling basis::published(0, 0, records) with every version hard-coded to zero -- a small re-implementation of project.rs::basis_update's record selection, in a file whose entire purpose is to rule out re-implementations. Both now take the Projection::Basis out of a real Sessions::connect snapshot. The reason the ticket calls it a nit is the reason it needed a test: a fresh App really is at generation 0, layout 0 and every record at 0, so the fabrication and the publication are indistinguishable on every existing fixture -- all 23 lockstep tests pass under either. So the change alone is structural and untested by construction. the_fixtures_basis_tracks_a_host_whose_layout_has_moved moves a tier first and then asserts the fixture's claim names the host's current tiers; re-fabricating the two document tiers fails it (0,0) against (0,1) and fails nothing else. Worth recording: SetNumberFormat was the first attempt and does not move layout_version, although it declares Model(LayoutGeometry). layout_version is OBSERVED from view_shapes rather than read off the declaration (improvise-pdvm), and a number format is not part of a view's shape hash. Classification is not a promise about the counter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
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 |
||
|
Ed Langley
|
1cafdad2fc |
Merge pull request #78 from fiddlerwoaroof/wave3/protocol-session
Wave 3: protocol and session — five consolidations, the cursor observed, and lltt found live |
||
|
Edward Langley
|
22bb621dd8 |
test(harness): close the stale-assemble hole, and decide the dropped assertion
improvise-669b. PR #65 single-homed HANDSHAKE_TIMEOUT_MS in web/shell.js, but the home is single in the TREE: the spec imports web/shell.js and the page loads harness/serve/app/shell.js. createShell now exposes timeoutMs() beside timesArmed(), and outlastTheWatchdog requires the running page's value to equal the one it imported. Injected with the ticket's recipe (8000 -> 100 in source, no rebuild): 5 passed / 1 failed where it was previously 6/6 green. Also in scope, the same file's stale line: HANDSHAKE_TIMEOUT_MS restates nothing and one test outlasts it, not two. improvise-qf57. The formula-row click's `#grid input` toHaveCount(0) stays dropped, and now deliberately. The reviewer's argument was that the exact `#mode` assertion subsumes it; measuring gives a better one. app.js creates a grid <input> in exactly one place, the grid cell's own mousedown, so no panel handler can reach it — and an invented formula-row activation opens the MINIBUFFER, not a grid editor. Injecting ChangeMode(formula_edit()) into click_formula_row: the toHaveCount(0) probe PASSES and the run fails on the #mode assertion below it. Restoring it would add an assertion that cannot fail, which context/test-evidence/clients.md already records as the reason a sibling assert! was removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
16042a04fb | Merge remote-tracking branch 'gh/main' into wave3/protocol-session | ||
|
Edward Langley
|
313e02f522 |
Merge #79, and fix improvise-mcq4: the frame observation costs one build, not four
Merge of `gh/main` at
|
||
|
Edward Langley
|
01d48f2ce9 |
Merge remote-tracking branch 'gh/main' into fix/ws-origin-check
# Conflicts: # context/test-evidence/clients.md |
||
|
Edward Langley
|
cead3210e5 |
fix: two Origin headers state none a policy can answer for
Review of the previous commit. `HeaderMap::get` is first-wins, and the deployment README recommends puts a reverse proxy in front of the socket — one that APPENDS `Origin` rather than setting it leaves two, and answering from the first lets the second be anything at all. No browser sends two, so two is not a state any policy can answer for and the handshake is refused, exactly as an `Origin` that is not text already was. Guarded by two_origins_state_none_this_policy_can_answer_for, proved by restoring the first-wins read, which fails that test alone across the workspace. The test also asserts the allowed origin BY ITSELF is still allowed, so the refusal is demonstrably the second header rather than the first. Second review finding: a_handshake_from_a_foreign_origin_is_refused matched "403" in a Display string, which cannot tell a refused upgrade from a dropped connection. It now matches Error::Http and asserts the status. Proved by changing forbidden() to UNAUTHORIZED, which fails it and a_refusal_is_a_forbidden_response. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
d100639348 |
fix: check Origin at the handshake, so a page cannot drive someone's session
improvise-kc7n. A websocket handshake is exempt from the same-origin policy — there is no CORS preflight — so any page a user visited could open a socket to improvise-ws-server from that user's own allowed address and read and edit every document served. An address check cannot see it: the request genuinely comes from an allowed client, and only Origin says what is driving it. Loopback is the least protected case, having no proxy in front of it at all. `src/origin.rs` is the decision and `transport::admitted` its one caller, which refuses the upgrade with a 403 rather than accepting and ignoring. The default allows pages served from loopback — which is the shipped local recipe, page on `http://localhost:8080` and socket on another loopback port — and refuses everything else; `--allow-origin` names the deployed page, which reaches the server through a TLS proxy and is not loopback. An absent Origin is allowed and decided rather than defaulted: no browser omits it, so absent is a non-browser client, and refusing those would break this crate's own tests while buying nothing that --bind does not already buy. Five edits, five guards, each reverted alone against the named test: - origin.rs permits() present arm -> a_foreign_origin_is_refused - origin.rs permits() None arm -> a_handshake_with_no_origin_is_accepted - origin.rs is_loopback() -> a_handshake_from_a_loopback_page_is_accepted - transport.rs handshake callback -> a_handshake_from_a_foreign_origin_is_refused - main.rs --allow-origin wiring -> allow_origin_widens_the_policy_and_nothing_else_does `transport::serve` takes the policy rather than defaulting one, so a host cannot forget to pass it. The argument is context/code/clients.md §Origin and the evidence context/test-evidence/clients.md; README no longer says the server never looks, and the proxy's Origin allow-list is now defence in depth. This is not authentication — it answers what page is driving a client, never who is connecting. improvise-kpnr is untouched and still open. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
a69b0bca86 |
test: the third edit of improvise-0x9u's repair, and two evidence corrections
`close` restores the DRAWN cell text rather than the seed, and reverting that one line left the whole suite green while `18400` sat where `18,400` had been on the shipped demo. Asserted after the second click, proved by that revert. improvise-o31u's desktop arm cannot fail for the reason its evidence gave: both named mechanisms were measured and neither moves it (`#cast-pivot` is 966px at 1280, so an 880px floor is inert wherever it is placed). The entry now says it is forward-looking, and credits the phone arm with the fault it actually catches. Plus three one-liners: UNCALLED's source-scan blindness, the gzip -9 proxy in the improvise-cl7y table, and why `view-back` is not one of its five siblings. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
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
|
||
|
Edward Langley
|
9e37213cd4 |
fix(docs): re-record the four casts from a script, with no shell to leak
improvise-2oxt. All four casts under docs/casts/ opened and closed on `(0) Mac:<hostname> ~gf/u/edwlan/improvise` — a real hostname and home directory on what improvise-kh8 made the site root — and `formulas.cast` and `import.cast` showed that prompt as their visible OPENING frame. drill.cast also predated the records columns entirely, showing a single `Value` column where the current build draws a column per measure plus `+cat` and `+`. The repair is structural, not careful. `docs/record-casts.py` drives `improvise` on a pty it owns and writes asciicast v2 itself, so `improvise` is the pty's ONLY child and there is no shell for a prompt to leak from — nothing to remember to clean, and nothing that comes back when somebody re-records on another machine. Three other things stopped being accidents with it: the geometry is an input (120x37, the numbers docs/index.html already spelled, so they did not change), the pacing is a per-step table, and the header carries no `env` (which held $SHELL) and no `timestamp` (the recording machine's clock). Two decisions that each cost a re-record. No `:q` is typed at the end: the player loops, so the last frame is also the frame before the first, and a minibuffer reading `:q` is what it would show — the process is signalled instead. And each scenario had to be ARRANGED so the thing it names is visible: the drill cast drilled into a cell with no records behind it, and the pivot cast pressed `c` on a category already on the column axis. Both recorded cleanly and demonstrated nothing — true frames about the wrong thing — and neither was visible from the keystroke list. That is the method worth keeping: a cast can be replayed through a small ANSI reconstruction and READ AS TEXT, so "what does frame N show" has an answer rather than a guess. It is how the `+cat`/`+` columns in the new drill.cast were confirmed present, and how both dud scenarios were caught. The guard is `landing.spec.js`, asserted over the cast FILES: no header carries `env` or `timestamp`, the geometry is the one the page configures, and no chunk contains an OSC window-title sequence — the leak's signature, emitted by a shell once per prompt with the cwd in it and by `improvise` never. Asserted over the file rather than the rendering because a player draws a prompt as ordinary text, so a screenshot assertion would have to know what the hostname had been. BOTH clauses were injected separately, because the `env` one would otherwise mask the OSC one: the old cast fails on `env`, and the old cast with `env` stripped fails on the title sequence. docs/index.html loses the `p.dated` note admitting the drill cast was out of date, which is now false. Tests: 33 harness + 3 skipped (was 28 + 3). Rust and node unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
9421eb207c |
fix(docs): the landing page's casts are legible on a phone
improvise-o31u. On a 390px viewport all four players computed a terminal font size of 4.83258px — reproduced to five digits before the fix. The player DERIVES its font size from the width it is given, and 120 columns into the ~356px inside the page's gutters gives about 3px per character. Since improvise-kh8 this page is the SITE ROOT, so that is the first thing a phone visitor sees. Under `max-width: 900px` the player is allowed to overflow INSIDE `.player` (`overflow-x: auto`, an 880px floor on its child) rather than being squeezed to fit. Measured after: 11.9457px, matching PR #47's reverted measurement. The instrument is the finding and is worth more than the CSS. `getBoundingClientRect` reports a height for a `content-visibility`-hidden child and heading text overflows its box without widening it, so neither "the element is present" nor a box measurement can see this. The COMPUTED FONT SIZE is what a reader sees, so that is what is asserted. Two guards on the trade this fix must not make: the document's own `scrollWidth - clientWidth` stays 0 — a page that scrolls sideways on a phone is worse than one unreadable section — and `#cast-pivot` must be the thing that scrolls, which separates "contained the overflow" from "removed it by shrinking something else". The desktop arm is not ceremony either: the same query written without a `max-width` bound, or with the floor on `.player` rather than its child, changes 1280px too. Tests: 4 landing tests (was 2). Rust and node unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
852e3f8659 |
fix(web): a selection driven below the fold scrolls itself into view
improvise-jq8l. `web/app.js` called `scrollIntoView` NOWHERE, so a selection moved past the bottom of `#grid` stayed there, invisible, with no way back but scrolling by hand. What the ticket did not predict: this is NOT a projection bug and NOT a `row_offset` bug, and the acceptance criteria's talk of reconciling the absolute selection against the announced window describes work `render.rs` already does. The row had been sent. `#grid` has `overflow: auto`, `ROW_PX`/`COL_PX` UNDER-estimate (a real row measures 28.6px against the assumed 26) and inline group rows are deliberately not charged for at all, so the page renders more rows than its own box can show and the overflow is the DOM's. `block: "nearest"` is the whole option: it scrolls only when the element is out of view, and it scrolls the nearest container rather than the document. The expensive half was the fixture. Neither shipped document can reach the state — `web/demo.improv` is four rows, `harness/fixtures/grouped.improv` is eight, and a probe measured BOTH at `scrollHeight === clientHeight` in a 484px box. The assertion would have passed against a page that never scrolls, which is why the spec asserts the overflow exists before it presses a key. `harness/fixtures/tall.improv` is thirty rows in three groups; measured against the unfixed page it gives 105px of overflow with the selected cell 96px below the visible bottom and `scrollTop` still 0. Tests: 25 chromium harness (was 24), Rust and node unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
628f43121d |
feat(web): the title bar is painted, a click survives an editor, and the twin door is gone
improvise-5jna, improvise-0x9u, improvise-seik — three browser-page defects with one shape: a thing that looks done from every check except running it. improvise-5jna. `doc_name`/`doc_dirty`/`doc_stored_as` were projected by z754 and painted by nothing. `render_surface.test.js` could not see it — it is a SUBSTRING SEARCH over the page's source, so a field named by a function nothing calls reads as painted, which is also how `model.minibuffer` shipped dead. So the guard is a harness assertion that loads the page and reads `#doc-name`, proved by writing it first (`element(s) not found`). The joining is `shell.js::documentTitle`, the TUI's own `name (stored_as) [+]` minus the chrome; the `[+]` is a node test, not a harness one, because this deployment persists on every accepted batch and clears the flag in the same round trip, so the marker is transient. UNPAINTED is now empty. improvise-0x9u. From an EDITING state a cell click did nothing at all: the mousedown moved focus, the input's blur ran `close`, `close` repainted, `replaceChildren` destroyed the `<td>` the mousedown landed on, and no `click` was ever dispatched. The old spec pressed Escape between the two clicks, so it only ever clicked from a non-editing page — this repo's characteristic defect. Worth recording: moving to `mousedown` is NECESSARY AND NOT SUFFICIENT, and the first attempt failed the same assertion with the same symptom. Removing a focused `<input>` fires `blur` SYNCHRONOUSLY, so `close` rebuilt the table from inside itself and detached the `<td>` again. `close` therefore no longer repaints, the `blur` arm checks `closed` first, and the handler `preventDefault()`s so no focus change happens at all — otherwise the default action focuses `#grid` after `input.focus()` and blurs the new editor. `close` restores the DRAWN text, since nothing repaints behind it any more. improvise-seik. `Client::viewport` was a wasm-bindgen door with no caller, kept for a reason `web/app.js` falsified in the same wave. It is one letter from `set_viewport` and fails SILENTLY — identical frame, forgotten window, every cursor key an upstream black hole. Deleted, along with the exported `init()` free function that duplicated `new Client()`. The guard is not the deletion: `wasm_surface.test.js` now reads the generated `.d.ts` and requires every declared method to be one `app.js` calls or to be named in `UNCALLED` with its reason. Proved by injecting a spare `static viewport(...)` into the built `.d.ts` and watching it fail by name. Tests: 1405 Rust (unchanged), 27 node + 4 bundle (was 26 + 3), 24 chromium harness (was 23). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
9976486d69 |
docs: the help entry no longer contradicts itself
Found by review on #75. Renaming the heading to the test PR #72 restored made check-evidence green, and left the body three paragraphs down still saying "The test no longer exists" and "nothing replaced the assertion" -- one entry making mutually exclusive claims about whether the coverage exists. The checker could not see it, and that is not a gap in the checker: it validates heading-to-test and says nothing about the prose beneath, which is what a human reads. A mechanical check licenses exactly the claim it makes and no neighbouring one. The paragraph now records the gap as history -- gone for a release, invisible because the checker then read only ### headings and only Rust names, restored by #72 with improvise-igxp's stronger assertions -- and points at help.spec.js's own entry rather than promising or denying coverage itself. doctrine(a_checker_that_reads_headings_cannot_see_a_contradicting_body): a rename is a read of the whole entry, not an edit of its first line. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
3d34b32085 |
Merge remote-tracking branch 'gh/docs/help-evidence-rename'
# Conflicts: # prolog-sessions/kb.pl |
||
|
Ed Langley
|
279bf3a848 |
Merge pull request #71 from fiddlerwoaroof/wave/browser-keys
fix(web): an advertised key is never silent, a formula is visible, and +cat commits |
||
|
Edward Langley
|
5fa554d712 |
docs: the help evidence heading names a test that exists again
PR #61 deleted the help spec; PR #69 demoted its evidence heading to prose as Retired; PR #72 restored the coverage under a DIFFERENT name. So main briefly carried a heading whose claim was false in both directions -- the prose said retired while the test was back, and restoring the heading verbatim would have named a test that does not exist. Renamed to #72's actual test. check-evidence: 535 entries, all resolve; 162 tests cite the file and all have one. Worth noting the sequencing, because it is the interesting part: #69 is the PR that TAUGHT the checker to see #### headings and JS test names at all, so it is simultaneously the thing that would catch this and the thing whose demotion kept the merged tree green. The demotion was correct when written and stale by the time it merged. Plus doctrine(a_message_assertion_is_a_uniqueness_claim_about_the_whole_vocabulary): asserting WHICH refusal fired is only as strong as that message being unique across every refusal the code emits, so enumerate the whole vocabulary rather than the line you are looking at -- and where no message can discriminate, assert the outcome, and guard the backstop separately. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Ed Langley
|
fe25952aed |
Merge pull request #69 from fiddlerwoaroof/wave/tooling-gates
fix(tooling): six checks that did not check (vz54, 5ray, lyyc, x4wv, 2f97, dj1l) |
||
|
Edward Langley
|
be01339039 |
Merge gh/main (4e48a6e, PRs #70 and #72) into wave/browser-keys
Two doc conflicts, both pure appends at the same place, both resolved by keeping BOTH sides: - `context/code/clients.md`'s boundary table: #72 added the help-overlay row where this branch added the key→screen row. Six became seven, not six with one of them replaced. - `context/test-evidence/clients.md`: both branches appended entries. `README.md`, `harness/build.sh`, `harness/playwright.config.js` and `.gitignore` are byte-identical to main's — this branch never touched them, so #72's measured engine-coverage claims are intact rather than restored stale. The check that matters here is NOT the count. #72 added three spec files while this branch added one, so a total would have masked a loss. Diffed harness test NAMES across the merge instead: 16 before, 23 after, **nothing lost**. 1398 + main = 1405 Rust passing; harness 28 passed + 3 skipped; node 26. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Ed Langley
|
4e48a6ecb4 |
Merge pull request #70 from fiddlerwoaroof/wave/effect-classification
fix(command): a counter is bumped on an observed change, not a declared intent |
||
|
Edward Langley
|
4aa4e88ce4 |
fix(protocol): Ignored is a black hole too, and a decline cannot shadow working code
PR #71 review, blocking finding — and it was demonstrated rather than argued. `KeyOutcome` has TWO silent variants and the new guard watched one. It asserted `resolve(..) == Upstream(event)`, but `KeyOutcome::of(vec![])` returns `Ignored`, which puts nothing on the screen either. Degrading only the three declines no browser spec presses — add-record-row, delete-formula-at-cursor, view-back — to `Step::Declined(Vec::new())` left 1397 Rust, 19 harness and 26 node tests passing. Three of the six commands this change exists to make audible could go completely silent with the whole evidence base green. The generalisation is the part to keep. `KeyOutcome::of` turning an empty `Local` into an honest `Ignored` is good design and also a LAUNDERING step: every wrong answer that composes nothing arrives wearing the variant that is usually innocent. A guard's discriminator has to be STATE — did anything the user can see change — never the shape of the return value. That is why the `+cat` injection failed in `both_ends`, which compares documents and cursors, and the decline injection passed everywhere, where nothing compared a screen. Widening it costs ten entries, all one boundary case: `move-panel-cursor` mirrors a `Cmd` that returns nothing when the cursor does not move. `furnished` now carries three of everything with all three panel cursors seated at index 1, so the residue is zero rather than parked in the named list. Seating them at 0 and calling the `Up` results expected would have hidden the real defect in the same list as the boundary case. `nothing_declined_is_also_composed` closes the other direction, which was a doc comment. `step` reaches `declined` first, so an arm in `composed` for a declined name is unreachable — a future ticket implementing `view-back` would leave the browser saying "not available yet" over working code with nothing noticing. It is derived from `declined` itself over every `(name, args)` bound in every mode, parents and prefixes and sequence steps included, so a seventh decline needs no edit here. It found a real overlap on its first run: `enter-mode` is answered by both. Rather than move the check, both halves now hold the rule — `composed` returns `None` for a stranded mode — so removing one fails towards `Upstream` rather than towards a dead room. `Keymap::parent()` added so a test can walk the fallback chain. 1397 → 1398 Rust; harness 19 + 3 skipped; node 26. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
14757a753f |
fix(command): the observation goes in the loop, or Save cannot clear it
Review of PR #70 found a regression the 1400-test suite did not name. `observe_reshape` ran AFTER the effect loop and re-raised model_state.dirty from the document_config comparison, while `Save` clears it INSIDE the loop. So gotcha 45's ordering property -- "Save clears it in loop order, so a batch that edits and then saves ends clean" -- survived for CellData/WholeWorkbook and was broken for LayoutGeometry. [SetNumberFormat, Save] -> reported a successful save, document dirty [SetCell, Save] -> fine on both Reachable from `:format ,.2f . w` and from any Binding::Sequence pairing a geometry effect with a save. The TUI draws "Saved to ..." while :q refuses; the worker, whose persist trigger IS the flag, pays a whole-document OPFS rewrite. The exact failure class this PR set out to close, one level up. The fix is position, not logic: the comparison is taken after each LayoutGeometry effect against the value held after the previous one. `layout_version` stays after the loop, which is not the same hazard -- nothing clears it, and an aborted batch's effects never ran, so an end comparison sees only what did. Neither arm of that is tested; filed. Two more review findings: a_panel_command_writes_that_panel_and_no_other asserted THROUGH the mapping it tests -- zeroing via panel_open_mut and reading via panel_open, the same four accessors. A consistent permutation of all four is a self-consistent bijection and passed. It now reads the three field pairs by name once per panel, and the consistent permutation fails it. It had been catching only a non-injective mapping, which is precisely the injection the evidence recorded. And improvise-56ek's claim of "no second copy left to agree with" was false when written: resolve.rs carried a fourth copy of the same Panel -> ViewState match, in the same crate on the same type. Deleted; its three call sites go through ViewState::panel_open. The claim is corrected rather than overstated -- registry.rs maps Panel to CmdContext fields, a different type, and the Panel -> (cursor, length) pairs are a different derivation and lockstep's subject. 1401 tests pass. Both new guards proved by injection: reinstating the after-loop comparison fails the ordering test by name with 454 others passing; a consistent Formula<->Category permutation across all four accessors fails the panel test, where before it did not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
d1ce2ecbc3 |
fix(web): an advertised key is never silent, a formula is visible, and +cat commits
improvise-rmvx, improvise-fdt6, improvise-gcvb — three faces of the same
thing: a browser offering something and then doing nothing.
rmvx. `the_command_names_that_still_need_the_host_are_these` was built with
`in_mode(AppMode::Normal)`, so it only ever asked about Normal-mode bindings:
`add-record-row` (RecordsNormal) and `delete-formula-at-cursor` (FormulaPanel)
were outside its reach entirely and could never have failed it. A named list
that silently scopes itself to one mode is a guard that looks broader than it
is. `every_advertised_command_answers_in_its_own_mode` crosses
`help::advertised` — command NAMES, so no hint string is tokenised — against
`resolve` in every mode, over a fixture furnished enough that an `Upstream` can
only mean an unhandled name. It found a seventh instance the ticket had not
predicted: `view-back` is advertised in RecordsNormal.
Six names now decline out loud, each with its own true subject, out of one
`declined` table; `stranding` returns the subject rather than a bool, so `:`
says the command line is unavailable instead of doing nothing. A declining step
takes its whole `Binding::Sequence` with it, which is what keeps `o` ("add a
record row, then edit it") from opening an editor on a row nobody created.
fdt6. `commit-buffer formula` already resolved; what did not exist was any way
to SEE the typing. `RenderModel::editing` is scoped to the two cell-editing
modes and reads the "edit" buffer by name, so FormulaEdit, CategoryAdd, ItemAdd
and the three prompts opened and swallowed every character. `RenderModel::
minibuffer` is `AppMode::minibuffer()` plus the buffer that config names, read
through `improvise_protocol::read_buffer` so gotcha 11 keeps its one home, and
`web/app.js` paints it.
gcvb. The records-mode early return was broader than its reason. A
`CellId::Create` commits the typed NAME as one existing `ModelCommand`, reads
no `record_col` and writes no cell, so the return narrows rather than
disappears. `+cat` must NOT advance: an inserted column renumbers every index
to its right and the advance was computed from the pre-commit layout, so a
client that advanced anyway would land one column off, silently. Both halves
proved by injection against `CommitAndAdvance` in `tests/lockstep.rs`.
Every new guard was proved by injecting the fault it names. The one worth
repeating: `web/tests/render_surface.test.js` went green over a page that
painted nothing, because the string `model.minibuffer` was inside a function
nothing called. The static check is a completeness check, never an execution
check — `harness/tests/keys.spec.js` caught it in Chromium on the first run.
1394 → 1397 Rust tests; harness 17 → 19 passed, 3 skipped; node 26.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
|
||
|
Edward Langley
|
f5198c9142 |
test(harness): gate the grouped reads on a painted head
Self-review under the code-review skill: the three grouped tests read the head and the row labels in one shot, which cannot wait, while the grid is painted from the first projection and reframed by the viewport the page then announces. openGrouped now gates on the head's shape, its first group and the first row's innermost label with retrying assertions. 15/15 under --repeat-each=3, and all five injections re-run afterwards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
f9580a33af |
docs: what the harness now reaches, and the evidence for each assertion
The engine-coverage and measurement passages in context/code/clients.md, the boundary table (four boundaries become six), and README's caveat -- which named the grouped and multi-level DOM as never executed, and it now is. A fact stated in more than one place is a grep, not an edit (gotcha 48). context/test-evidence/clients.md records, per test, what fault was injected to prove it can fail: the blanked help keys, the five grid-frame injections, the 404'd casts and the dead front-door link, and the mis-wired item-row click -- plus the two-command test-name diff that found that last one. Playwright: 26 passed + 3 skipped, 34.4s / 33.7s (was 17 + 3, 15.1s); the landing page's cast playback is most of the difference. node --test web/tests 26, cargo test --workspace 1394, both unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
Edward Langley
|
beef2587ad |
fix(command): a counter is bumped on an observed change, not a declared intent
`EffectKind` says what an effect *means* to do. `layout_version` and
`model_state.dirty` were believing it, and failed in both directions.
improvise-pdvm: AddItem/AddItemInGroup/RemoveItem declare CellData and
reshape the grid. Since improvise-9mo9 they call reset_cursor, so
cursor_epoch moved while layout_version did not -- one event moving one of
two counters that answer the same question, and a client told its cursor is
invalid while its layout basis tier still reads current.
improvise-75ld: a refused AddCategory (MAX_CATEGORIES) and a *duplicate* one
both bumped layout_version and raised dirty. The duplicate is the reachable
silent case -- Model::add_category returns Ok for a name it already has, so
it does not even abort -- and it refused every other client's next batch with
Rejection::LayoutMoved and made the worker rewrite the whole OPFS document.
Both are answered the way record_versions and cursor_epoch already are:
observe. Two fingerprints are taken around a batch that contains any
EffectKind::Model(_).
view_shape one view's DRAWN shape (axes, effective items minus this
view's hidden ones, collapsed groups with something drawn
under them, page selection, prune_empty). Per view name, so
it answers both "did anything move" and "which views moved".
document_config everything a LayoutGeometry effect can write that the
file would carry. Raises dirty. A different hash on purpose:
a number format and a hidden ghost are bytes on disk and not
shape.
No `Effect::kind()` declaration changed, so Dirty::of (gotcha 35) and
record_versions (gotcha 40) are untouched. What moved is which reader
believes the declaration: CellData and WholeWorkbook still raise dirty off
it, in an exhaustive match so a fourth ModelChange must answer for itself.
improvise-nynk: Sessions::adopt_cursor_epoch reset every mirror and returned
one Broadcast::ToAll, which improvise-jytm made wrong -- a reshape of view A
threw away the cursor of a client on view B. The trigger stays the epoch
moving (gotcha 24); only the audience narrows, to the observed reshaped-view
set, as one Broadcast::To per affected client. An empty set means "not
known", never "nobody", and cursor_invalidated is a named function with both
arms tested so that fail-safe cannot be tidied away.
improvise-56ek: SetPanelOpen and SetPanelCursor matched a Panel to a field
twice, in two crates. One home now -- ViewState::panel_open_mut /
panel_cursor_mut and their read halves -- and no compatibility test between
them, because there is no second copy to agree with.
1400 tests pass (baseline 1394: +7 guards, -1 replaced). Every guard proved
by injecting the fault it names; the injections and what each one broke are
in context/test-evidence/.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
|
||
|
Edward Langley
|
2eb46122d3 |
fix(tooling): --check-evidence sees #### headings, JS tests, and the reverse direction (improvise-5ray, improvise-lyyc)
Two blind spots with one consequence: an evidence file could promise a guard
that was not there, and a test could point at prose that did not exist.
5ray: headings were collected with `startswith("### ")` and resolved against
Rust fn names only. context/test-evidence/clients.md uses `#### "name"` for
every browser test precisely BECAUSE a `###` naming a Playwright test would
have been reported as an orphan -- so the convention that kept the checker
quiet was the same one that made it blind. Now `###` and `####` alike, decided
by SHAPE: a bare identifier is a Rust test, a quoted string is a JS test
(harness/tests and web/tests), anything else is prose naming nothing.
lyyc: the check was one-directional. A test whose doc comment names
context/test-evidence must now appear there. Deliberately only that narrower
obligation -- most tests have no evidence and should not be forced to. Neither
direction can see a STALE entry; that stays discipline.
Each half found a real instance on its first run. A `#### "? opens help..."`
heading outlived the test
|
||
|
Edward Langley
|
ec03f950b3 | Merge remote-tracking branch 'gh/main' into fix/web-panel-clicks | ||
|
Edward Langley
|
27580e4d64 |
docs(tests): a repair needs its own probe, because a repair can look like a fix
The two-step mutation recipe recorded alongside this finds a DISARMED
test. It says nothing about whether the repair you then write is real.
That gap is not hypothetical: the first repair of
panel_commands_address_the_named_panel_only folded [open:false,
open:true], read as a fix, and was still vacuous.
The probe that settles it is a ONE-DIRECTIONAL fault -- make the writer
`if open { set true }`, so the flag can be raised but never lowered. A
vacuous repair cannot see it, because a test that only ever asserts the
`true` direction is satisfied by a writer that only ever writes `true`.
Injected into both copies at once it fails set_panel_open_and_cursor AND
panel_commands_address_the_named_panel_only, each in its own copy --
which is what establishes both repairs are genuine rather than merely
differently spelled. Verified here rather than taken on report.
Prefer it to re-reading the diff: the diff contained `open: false` in
the version that was vacuous, which is exactly why reading it passed.
Docs only -- crates/, src/, web/ and harness/tests/ are untouched.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
|
||
 Edward Langley
|
89f67afd4c |
fix(harness): make a vacuous watchdog denial fail, and single-home the window
PR #65's review found this PR had planted a fresh instance of the defect it exists to repair. Both blocking findings fixed, each demonstrated. FINDING 1. "a peer that cannot be reached is named, and the watchdog stays out of it" could not see the property its comment claimed. `shell.arm` runs only after `await transport.ready`, which never resolves on a socket that never opens — so the watchdog is never armed there and denying "sent no document" denies an unreachable event. Reverting PR #21's routing left that clause passing. The aggravating half: `context/test-evidence/clients.md` already said so, and this PR EDITED that paragraph — inserting "runFor(10s) advances the watchdog" immediately above the sentence explaining it is never armed. Not text the PR left alone; text the PR walked past mid-sentence. The clause is gone and the test keeps the naming property it really guards: a `close` on a socket that never opened must not read as "disconnected". And the shape cannot return — `outlastTheWatchdog` now asks the page `timesArmed() > 0` before jumping, so a watchdog denial re-added to that path FAILS BY NAME. It asks *ever armed* rather than *armed now* on purpose: by the time the disconnect test jumps, a correct page has already disarmed, which is the property, so the obvious spelling would have denied the passing case. Building the guard is what surfaced that distinction; reading had not. FINDING 2. The spec kept its own `HANDSHAKE_TIMEOUT_MS` beside a comment saying a silent disagreement "would pass for the wrong reason". It did: the spec at 1000 against the page's 8000 made the suite green over the reverted routing, because the jump landed inside the window. The constant is now exported from `web/shell.js` and imported by both, so the disagreement is unconstructible. Shrinking it in its one home still catches the routing revert — that is the demonstration the guard did not go quiet. Filed, not fixed: improvise-78v6 (#61 also deleted the help-overlay spec and nothing replaced it — a PR about a dropped guard that did not diff the dropping PR's test names) and improvise-5ray (--check-evidence collects only `### ` and resolves only Rust fns, so every browser entry's `####` is invisible; appending one that names nothing exits 0, which is how the dangling heading cleared the gate for a release). Verified on the merged tree: node 26/26, bundle 3/3, Playwright 17 passed + 3 skipped, cargo test --workspace 1378 passed, comment-runs clean, --check-evidence 501 resolving, all cited ids resolve, no .beads diff. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
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 |
||
|
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> |
||
|
Edward Langley
|
57a25807af |
Merge gh/main (e8562f0) into fix/web-panel-clicks
PR #61 landed while this branch was open. It deleted `harness/tests/pointer.spec.js` and `harness/tests/widgets.spec.js`, split `web/app.js` into app/shell/serial, and added a Node suite under `web/tests/`. Two conflicts, both resolved deliberately rather than by reflex. **`harness/tests/pointer.spec.js` — modify/delete, resolved as DELETE.** The habitual "keep both sides" would resurrect a file #61 removed on purpose. `contract.spec.js` states its own scope as the RenderModel→DOM boundary and the pointer's half of the JS→wasm one, so that is where these assertions belong. Merged in, rather than restored: - the category sidebar is open at launch and `C` still hides it; - a first tile click selects and does NOT touch the document, a second cycles; - the view row and formula row clicks improvise-iioi added, which `contract.spec.js` had no entry for at all. **`context/test-evidence/clients.md` — content, resolved as BOTH SIDES.** The two `pointer.spec.js` subsections are retargeted at the file that exists, with a note saying why restoring the deleted one would have been wrong. The two older `#### … the file's own scope` sections keep their history and gain a forward pointer, because a reader following them would otherwise hunt for a file that is gone. **`web/tests/bundle/wasm_surface.test.js` now names `click_formula` and `click_view`.** That test is an exact `deepEqual` over the page's `client.*` calls and its comment says a new call "has to come and say so". It did: it failed on the merge, which is the mechanism working. It could not have been satisfied earlier — it did not exist when this branch was cut. Two review findings fixed on top, neither a live bug: - **The tile guarantee was overclaimed.** `T` leaves `tile_cat_idx` at 0, so a click on the LEFTMOST tile cycles on the first gesture. "A click never mutates on its own" and "a second click cycles" are both false against that probe. The behaviour is right — a click does what `Enter` does there — so the prose now states the implemented rule, and `t_then_a_click_on_the_leftmost_tile_cycles_it_immediately` names the case and asserts the neighbouring tile answers the same gesture differently. - **`switch_view_at`'s `Option` had no guard on the keyboard side.** Removing the `unwrap_or_default()` makes `Enter` in an empty view panel `Upstream` — gotcha 47's black hole — and passed the whole workspace. That would make membership of the host-needed list state-dependent, which is the one property that list denies. One test closes it. Gates on the merged tree: 1391 Rust tests, 26 + 3 Node tests, 16 Playwright (3 WebKit OPFS skips), clippy -D warnings, fmt, harness fmt + wasm32 clippy, eight wasm crates, comment runs and 513 evidence entries clean. `.beads/` matches gh/main exactly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Edward Langley
|
9863d96eef |
fix(harness): restore the close-routing guard PR #61 dropped, on a faked clock
Adversarial review of PR #61 found one real coverage loss, and it is the defect class that PR's own body names: an assertion that survives the deletion of the thing it was about. `shell.spec.js` lost "a socket that opened and then dropped reads as disconnected, and stays that way" and both MutationObserver negative assertions. Reverting PR #21's routing half in `web/app.js` — the socket `error` and `close` arms calling `setShell` instead of `fail` — left the node suite 26/26 and Playwright 13/13 green. `shell.test.js` cannot cover it, and why is the whole boundary argument: "a loud failure DISARMS the watchdog" tests `createShell` in isolation and cannot see whether `app.js` ROUTES the close event through `fail()`. That routing is the half gotcha 48 says keeps getting lost; this was the third time, after two earlier mis-confirmations that simulated the `opened` flag and not the routing. Both tests are back under their original names, so the two `####` headings in context/test-evidence/clients.md name live tests again. They cost ~75ms each rather than ~9s, because `page.clock` fakes the PAGE's timers: the page, the socket, the listeners and the `setTimeout` call are all real and only the passage of time is simulated. That is not a weakening, and the injections are the proof in both directions — if `runFor` did not advance the page's timers, a broken routing could not surface the watchdog's message and they would pass: - both arms reverted to `setShell` -> "…and stays that way" fails - close arm only -> "…and stays that way" fails - close arm drops the `opened` flag -> "…the watchdog stays out of it" fails Three injections, three outcomes, none of them the unit test. The helper guards its own premise, because it turned out not to: `runFor` without `clock.install()` does nothing AND throws nothing, so a test that forgot the install asserted "the watchdog never spoke" against a watchdog that never ran — 6/6 green. `outlastTheWatchdog` now requires the page's own `Date.now()` to have moved, and that omission fails by name. Also: `render_surface.test.js` matched `model.<field>` over source INCLUDING comments, so un-painting a field and mentioning it in a comment passed — demonstrated against the pre-fix version, then fixed and re-demonstrated. And the PR's speed claim is corrected where it is documented. The eleven deleted per-widget tests cost 3.3s between them against 3.4s for the three contract tests replacing them: they were free. Essentially all of 36.9s -> 15.5s was two real-timer waits, one of them this guard. The wait was the cost, not the test. Filed, not fixed: improvise-igxp (the help overlay's rows are asserted as present, never as content — inherited verbatim from the test contract.spec.js replaced) and improvise-4ktt (the js job's bundle-freedom is a shell-glob convention, not a gate). The `####`-vs-`###` blind spot in --check-evidence, which is why the dangling heading survived the gate, is a comment on improvise-r0qa. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |