17a582ae4d2c4188f6e1dfd3a8adf680858a95d5
1457 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
17a582ae4d |
test(gp): pin the FitOutcome::warnings order the doc comment and gp.md both state (improvise-ipqu)
`fit_measure` concatenates three warning tiers with two `splice(0..0, ..)` calls. The resulting order — the budget's notices, then the feature table's, then preprocess's — is stated in the field's doc comment and in context/code/gp.md section 14, joined into a user-facing status line, and was checked nowhere: swapping the two statements left all 74 improvise-gp tests green. No pre-existing fixture produced more than one tier, which is why nothing caught it. `all_three_warning_tiers_at_once` adds two things to the wide-category fixture: a `_Measure` item with no cell anywhere (a header column entirely missing in every row, which preprocess warns about) and a `Cost` cell at an unused coordinate (one visible record with no `Score`, which feature_table skips). Neither disturbs the budget arithmetic. The assertion is on the three positions, not on the concatenation containing them, and the helper panics on an absent tier so a tier that stops speaking fails loudly rather than making the ordering trivially true. Two rivals, each moving a different boundary, each caught only by this test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
2ccf2e5e2c |
Merge pull request #140 from fiddlerwoaroof/feat/gd28-drill-decompose
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
refactor(view): a virtual view's shape is model-side data, not an effect list (improvise-gd28) |
||
|
|
ab227cdef6 |
fix(view): restore the two predicates a mis-targeted rival-revert corrupted
Both faults were injected on purpose to show the extracted functions are on
the live path, and the revert of the second one replaced the WRONG occurrence:
`str.replace(".is_measure()", ".is_index_or_dim()", 1)` hit the first match in
the file, which is `drill_constraint`'s pre-existing predicate at line 78, and
left `records_steps`' injected one at line 137 in place. So the commit carried
two faults where it had shown one:
- `drill_constraint` stopped recognising a formula targeting `_Measure`, so the
derived measure stayed in the constraint and `matching_cells` returned no
source rows -- the defect `drill_into_formula_cell_returns_data_records`
exists to catch.
- `records_steps` put `_Index` and `_Dim` on Row and Column and then overwrote
both with `Axis::None`, while never aggregating `_Measure` away, so entering
records mode lost its spine entirely.
Found by the ChatGPT Codex review on the PR, reading the diff, and both of its
findings were exactly right.
The process failure is the part worth recording: after reverting the rivals I
re-ran clippy, fmt, --check-evidence, the node suites, the eight-crate wasm
gate and Playwright, and every one of them passed over the broken logic. The
one gate that could see it -- `cargo test --workspace` -- was measured BEFORE
the injections and never again, so the PR body's "identical, 1699 passed"
was a figure that was checkable and false. An injected fault is not reverted
until the full suite has been re-run; the targeted test passing again only
shows the targeted fault is gone.
Re-measured after this commit, on the orchestrator's machine: 1699 passed, 0
failed, 7 ignored over 38 binaries -- identical to the base at `a9cad15`, which
is what this PR claims and now actually has.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
|
||
|
|
fe8d5404e2 |
refactor(view): a virtual view's shape is model-side data, not an effect list (improvise-gd28)
`DrillIntoCell::execute` and the records-mode command each spelled out what
their view must be as a run of `effect::SetAxis { view: ViewTarget::Active }`
pushes, interleaving the model question ("what is this view") with the UI
question ("how does a TUI keypress get there"). The two spellings shared the
`_Index`/`_Dim` spine and nothing kept them in agreement.
`view::layout` now answers the first question as data: `records_spine()`,
`records_steps(model)` and `drill_steps(key, none_cats)`, each a `Vec<ViewStep>`
of `Axis`/`PageSelection` assignments naming no effect, no `App` and no front
end. `cmd/grid.rs::steps_into_active_view` is the whole of the second question
for this front end -- a browser composing `ModelCommand`s against `_Drill` will
answer it differently without touching core.
`drill_constraint`/`drill_snapshot` were already model-side and are untouched;
core.md S8 records why their order cannot be derived a second time. `DrillState`
deliberately does not move: it holds an `Rc` and its `pending_edits` are
uncommitted writes belonging to whoever staged them, so it stays on `App` where
gotcha 30's session-local-residue tag puts it.
Behaviour is unchanged and no test expectation moved, which is the strongest
evidence a pure separation can produce. Measured on the orchestrator's machine,
before and after:
cargo test --workspace 1699 passed, 0 failed, 7 ignored over 38 binaries
node web/tests 56 tests over 6 files
node web/tests/bundle 4 tests over 1 file
playwright (harness) 68 passed, 4 skipped
--check-evidence 823 entries, 301 citing tests, 83 prose headings
Two rivals injected against the extracted functions, to show they are on the
live path rather than a parallel copy: `drill_steps` emitting `Axis::Page`
instead of `Axis::Filter` fails `drilling_fixes_its_coordinates_on_filter_not_page`,
and `records_steps` filtering on `is_measure()` instead of `is_index_or_dim()`
fails `save_with_unfinished_record_is_readable`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
|
||
|
|
a9cad15827 |
Merge pull request #137 from fiddlerwoaroof/fix/f9qf-pin-the-shell
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(scripts): one shell and one environment for every checker (improvise-f9qf) |
||
|
|
1ae2d2e75a |
Merge pull request #139 from fiddlerwoaroof/fix/k844-importance-overflow
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(gp-tree): a share of a large importance overflowed f32 twice over (improvise-k844) |
||
|
|
a14955cfb4 |
docs(tooling): the seventh sh-override is a TOML array, which no shebang gate can see
Reasoning: context/code/tooling.md section 5. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
f52c2df0c7 |
fix(dist): the extra-artifacts build honours its script's shebang (improvise-f9qf)
`build = ["sh", "scripts/build-web-release.sh"]` overrides the shebang the same way the old local gate did, so the script ran under dash on the release runner while declaring bash. It is the seventh such site and the only one outside a shell file; scripts/checkers.sh cannot see a TOML array, so this one is fixed by reading rather than by a gate. The script is mode 100755, so dropping the `sh` runs it through its own `#!/usr/bin/env bash`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
327a76bb58 |
Merge pull request #138 from fiddlerwoaroof/fix/tg27-narrow-window
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(web): a narrow window must not widen the document (improvise-tg27) |
||
|
|
48a1fa31db |
docs(gp): the reason k844 was deferred was wrong, and the correction is the point
context/code/gp.md §7 said reordering importance_ranking's arithmetic "moves the bit patterns misclass_rate_and_importance_are_bit_for_bit_what_they_were pins", and filed the fix on that cost. It does not. That test pins permutation_importance's RAW scores; importance_ranking consumes them afterwards, so no path it witnesses runs through the share. Measured on the commit that actually fixes k844: tests/scoring_known_answers.rs is byte-identical and not one literal moved. A cost predicted from a test's name rather than from what it calls is how a one-line fix stays filed. The wrong prediction is kept and corrected in place rather than deleted, because that is the part a future reader needs — the fact was stated in two sections and changing it is a grep, not an edit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
b39b3fbcb5 | Merge remote-tracking branch 'origin/main' into fix/k844-importance-overflow | ||
|
|
bbfdf992cd |
fix(gp-tree): a share of a large importance overflowed f32 twice over (improvise-k844)
`importance_ranking` computed `100.0 * imp[j] / total` in f32, which associates
left: a lone importance above 3.4e36 overflowed on the multiply, before the
divide that would have brought it back into range, and the feature carrying all
of the importance was reported as `inf%`.
Dividing first — the fix the ticket's title proposes — is not enough, and that
is the half worth recording. `total` overflows too: several large importances
sum past f32::MAX, and the reassociated version then divides by `inf` and
reports a serene 0% for every feature, where the original reported NaN%. That is
improvise-pv3g's `cv_regress` shape again: an overflow that corrupts a reported
number with no panic to announce it.
So the sum and the ratio move to f64 — no sum of finite f32s can overflow it —
and only the resulting share, bounded by 100 and therefore always finite, is
narrowed back. The arithmetic stops being able to produce a non-finite share
rather than a call site checking for one afterwards; `fit_csv`'s existing clamp
stays as the bar-length bound it also is. The one remaining guard,
`!total.is_finite()`, answers only an importance that arrives non-finite: an
infinite total apportions nothing, so the ranking is empty rather than NaN.
Reachable end to end: both callers score importance in the target's own units,
so an importance is an MSE difference and grows as the square of the target.
That leaves a live window between pv3g's `standardize` refusal and this
overflow, and `a_large_measures_importance_shares_reach_the_report_finite` sits
inside it — before the fix, `fit_measure` reported `[("Cost", inf)]`.
The scoring bit patterns in tests/scoring_known_answers.rs are unchanged, which
is the evidence the fix is confined to the non-finite path: `importance_ranking`
is not on any path that file pins. gp-tree still prints nothing.
Evidence in context/test-evidence/gp.md; the argument in context/code/gp.md §8.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
|
||
|
|
76f69ce671 |
Merge branch 'main' of github.com:fiddlerwoaroof/improvise into fix/tg27-narrow-window
# Conflicts: # context/test-evidence/clients.md |
||
|
|
eefd992538 |
Merge pull request #136 from fiddlerwoaroof/fix/nxah-rollback
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(protocol): a refused frame is put back by the fold that made it (improvise-nxah) |
||
|
|
c8d715346f |
fix(web): a narrow window must not widen the document (improvise-tg27)
The report was "a narrow window clips grid columns without usable horizontal
scroll", and the ticket suspected a flex `min-width: auto` trap swallowing
`#grid`'s overflow. It was not that: `#grid` has `overflow: auto`, which
already zeroes a flex item's automatic minimum. What could not shrink was the
app bar.
Measured at
|
||
|
|
2151fe0759 | Merge gh/main into fix/f9qf-pin-the-shell | ||
|
|
253b84b2da |
fix(protocol): a refused frame is put back by the fold that made it (improvise-nxah)
`Client::key` folds a locally-resolved batch before sending it, so the page redraws on the keystroke. A refusal left those folds in place: measured against a real `Host`, a refused commit left the client one row down with its edit buffer cleared while the host's mirror of that same client was still at the origin holding the user's typing. What rolls back is `ViewState` — the thing the fold produces — never a cache. A `Command::Model` is a no-op in `reduce_view`, so the optimistic fold never touched the caches, and the bug report's own evidence (`page_coords`) cannot diverge this way: it moves only under `Projection::Grid`. What diverges is the mode, cursor, status and buffers that ride along with the model command. The rollback needed a positive acknowledgement first, which the protocol did not have: with only `Rejected` on the wire, a client holding two frames in flight cannot tell which one an answer addresses. `ServerFrame::Accepted` closes that — emitted once per accepted mutating frame, last, after everything the frame produced — the same shape as the basis-publication gap ADR 001 originally left. Two existing facts make the rule usable rather than a convention: only `Command::Model` can be refused, so a frame carrying none is answered by neither and needs no entry; and `Rejection::refused_an_edit` already separates the two `Save` refusals, so a failed write cannot consume the answer owed to an edit. A refusal drops one frame and replays the rest, because a single saved snapshot is unsound under pipelining — it would discard a later frame the host applied. This does not do improvise-3al1's force, and it does not contradict it: the 2026-09-21 wave recorded the two tickets as contradictory on the ground that a rollback would wipe the staged typing. It does the opposite — the typing frames carry no model command, are never refused, and have already settled, so rolling back the commit restores the buffer to the typing. Asserted, not inferred. Reasoning: context/code/clients.md, context/code/protocol.md §4. Evidence: context/test-evidence/clients.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
42ec30df57 |
Merge pull request #135 from fiddlerwoaroof/fix/0ygd-tab-new-measure
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(command): naming a measure inserts a column, so Tab must not advance (improvise-0ygd) |
||
|
|
cb6866b2c6 |
fix(scripts): one shell and one environment for every checker (improvise-f9qf)
`scripts/pr-reviews-test.sh` passed 19/19 locally and 2/19 in CI on the same commit, because a multi-assignment `local` is not left-to-right on bash 5 while macOS's bash 3.2 tolerates it. The obvious half of the fix is pinning the shell. The half that decides the design is not. .envrc DOES already put a nix bash 5.3.15 on PATH, which is CI's semantics -- and it is not enough. `pkgs.mkShell` exports `name`, `out`, `outputs`, `system`, `shell`, `builder`, `stdenv`, `phases` and about twenty more generically-named variables that CI does not have, so a script reading one by accident under `set -u` is SILENT in the dev shell and fatal in CI. That is exactly what happened: the broken clause read `$name`, and in the dev shell `$name` was `nix-shell-env`. Measured at 051f541^ on one machine: macOS /bin/bash 3.2 19 passed, 0 failed direnv exec . ./scripts/pr-reviews-test.sh 19 passed, 0 failed the same, with `name` unset 2 passed, 17 failed Two reasons for green, one of them the correct interpreter. A dev shell is a third environment, not a reproduction of CI. A nix shebang was measured and declined, on availability rather than speed: warm, `nix shell nixpkgs#bash --command true` is 61.8ms against an ambient 5.1ms, and `nix-shell -p bash --run true` -- the manpage form -- is 675.2ms, 132x, inside git's merge machinery for beads-merge-driver.sh. But only the `kb` job installs nix; build, test, wasm, js, harness-lint and browser have none, so a nix shebang needs an installer step on three more jobs to reach a bash those runners already ship. zsh is not a shebang change and the runner does not have it either. Both declined in context/code/tooling.md section 5. What lands instead pins interpreter and environment together, in one place: scripts/checkers.sh is the one entry point for every check_the_checker, used verbatim by CI and by the local gate. It pins a bash >= 5 (ambient first, nix only when the ambient one is older, exit 3 when neither), scrubs the environment to an allowlist, and refuses any shell shebang in scripts/ that is not `#!/usr/bin/env bash`. The pinned bash goes on the front of PATH so inner invocations get it too -- which is where the fault actually lived. The second undetected split is closed with it: the four `#!/bin/sh` and one `#!/usr/bin/env sh` scripts are now bash, and the six places that ran a script through an explicit `sh` -- overriding the callee's own shebang, which is the same disease -- now invoke it directly. Verification, injected against the real script: restoring pr-reviews' multi-assignment `local` leaves `sh scripts/pr-reviews-test.sh` at 22/22 and `direnv exec . ./scripts/pr-reviews-test.sh` at 22/22, while `./scripts/checkers.sh pr-reviews` reports 2 passed, 20 failed, exit 1 -- matching the 2/19 CI actually reported. scripts/checkers-test.sh is twelve cases, each injecting rather than asserting; the pair that carries it runs one fixture both ways, so the `set -u` case is a test of the runner and not of `set -u`. The old local gate was `for t in scripts/*-test.sh; do sh "$t"; done`, which overrides every shebang with /bin/sh -- so it ran every suite under bash 3.2 in sh-mode regardless of what the file said. It is replaced by `./scripts/checkers.sh`. Workspace suite unchanged at 1689 passing over 38 binaries. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
d5201d984b |
fix(command): naming a measure inserts a column, so Tab must not advance (improvise-0ygd)
Typing a name into the trailing `+` column of a records grid and pressing Tab created the measure and then threw the user across the grid: it added a record row nobody asked for and parked the cursor at (row + 1, 0), leaving the column the commit had just made unreachable. Enter on the same cell was correct. The cause is a stale index, not an abort and not an ordering. `AdvanceDir::Right` reads `at_bottom_right` off `CursorState::col_count`, the PRE-commit column count — `apply_effects` rebuilds the layout only after the whole batch (gotcha 23). A `+measure` commit inserts its column at the cursor's own index and pushes the `+` one to the right, exactly as `+cat` does, so after the commit the cursor is no longer at the right edge. `context/code/command.md` argued the opposite in prose: that the measure `+` has no column to its right for an index to shift into, and that both advances are already special-cased at the bottom-right edge. The first is false — the column that shifts is `+measure` itself — and the second names the bug rather than a safeguard. That paragraph is corrected here. `inserts_a_column` therefore covers both creating columns, and takes the value, because an empty name creates nothing: `created_name` is now the one home of that rule, read by `add_measure`, `add_category` and the predicate. Splitting on the buffer rather than the column is what keeps bulk entry, whose Tab-at-the-bottom- right path is reachable only from `+measure` — it is always the rightmost column — by tabbing onto the `+` with nothing typed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
22ab58f652 |
Merge pull request #133 from fiddlerwoaroof/fix/pr-reviews-inline
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(scripts): pr-reviews reads all three review surfaces |
||
|
|
a799e244af |
Merge pull request #134 from fiddlerwoaroof/fix/jz6w-keyoutcome-frame
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(protocol): a key can ask its host to save (improvise-jz6w) |
||
|
|
051f54111b |
fix(scripts): a local is not left-to-right on bash 5, and --paginate emits one array per page
Both found by ChatGPT Codex on PR #133, both invisible on macOS, both arriving as exit 2 with no output -- jq's answer to an invalid --argjson, so a shell fault at the top of the script wears jq's exit code. `local n="$1" name="$2" f="$DIR/$n.$name.json"` creates every name before running the assignments on bash 5, so $n is unbound in the third clause and set -u kills the shell. The dead command substitution then hands --argjson an empty string. Works on macOS bash 3.2: the suite passed 19/19 locally and 2/19 in CI on the same commit. Each local now gets its own statement. `gh api --paginate` emits one JSON array PER PAGE, so a comment thread past the page size is several concatenated documents that --argjson rejects the same way. Both fetch paths now flatten through one pages_to_array (`jq -s 'add // []'`), which is a no-op on a single page. The fixture path shares that helper, which is the only reason a fixture can test it: a fixture holding a single array would exercise a shape the live fetch never produces. Three cases added, and removing the helper from the fixture path fails exactly those three, the wait case with the live exit-2 signature. Suite is 22 cases. No PR in this repo has yet had enough inline comments to reach a second page, so nothing but the test will see that one before a user does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
7e6cac0885 |
fix(protocol): a key can ask its host to save (improvise-jz6w)
`ClientFrame::Save` landed with improvise-368h and `Host::handle` has answered it ever since, but `KeyOutcome` was `Local | Upstream | Ignored`, so no resolver could say "send this frame". `save` therefore sat on `the_command_names_that_still_need_the_host_are_these`, resolved to `Upstream`, and `Host::handle` answers `Upstream` with an empty vec — gotcha 47's black hole. An implemented server feature that no key could reach. `KeyOutcome::Request(Request)` closes it, with `Request::Save` the one member and `Request::frame` the one mapping to an envelope. The variant carries a narrow enum rather than a whole `ClientFrame` on purpose: a resolver handing out arbitrary frames could compose a `Viewport` (the page's fact, not the keymap's) or a `Commands` batch, which would be indistinguishable on the wire from a `Local` while skipping the local fold `Local`'s contract requires. `requested` is checked first in `step`, so no name is in two lists, and a request cannot travel inside a `Binding::Sequence` — a sequence is one frame. `save` leaves the host-needed list in the same commit, as gotcha 47 requires. The guard is end to end, in `browser_round_trip.rs`: Ctrl-S on a real `Client` yields `ClientFrame::Save`, and a real `Host` over a real `App` writes the document to storage. Two half-tests that never meet is exactly what left this gap open. Reasoning in `context/code/clients.md`; evidence and the injected rivals in `context/test-evidence/clients.md`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
0e10af005a |
docs(pr-review): a commit_id is per-comment, not per-review
Found by dogfooding the new script on PR #122, the PR that prompted it. One review submitted between 19:48 and 19:53 carries five inline comments: three name |
||
|
|
a47310d50f |
fix(scripts): pr-reviews reads all three review surfaces (improvise-xgra)
It printed review BODIES only and was blind to inline review comments -- the surface a human reviewer is most likely to use. On PR #122 the user reviewed with the body "Generally good, added comments inline." and five substantive rulings underneath it; pr-reviews printed the one-liner, and an agent reported the PR as awaiting review while the answers sat on it. pulls/{n}/comments and issues/{n}/comments join pulls/{n}/reviews, all three interleaved in time order, each inline comment carrying its path and line (falling back to original_line, which is what an outdated comment has left). The exit contract is unchanged: 0 means something exists against the current head, 1 means the wait elapsed, 4 is usage. An inline comment at the head now satisfies --wait -- it carries a commit_id and is exactly the case that failed. An issue-level comment does not: it names no commit and so makes no claim about which tree was read. The usage's exit-code warning is widened from "do not pipe into head/tail" to the real rule: nothing may follow the script in the same `;` list either, because a trailing `echo "exit=$?"` makes the status the echo's. scripts/pr-reviews-test.sh is the check_the_checker, fixture-driven and needing no network, wired into CI's kb job beside pr-watch-test.sh and release-preflight-test.sh. Five faults were injected and watched to fail; context/test-evidence/tooling.md records which case caught each. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
d672985b07 |
Merge pull request #132 from fiddlerwoaroof/docs/adr-004-rgb-ruling
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
|
||
|
|
85c273952f |
docs(adr): keep the Rgb arm — the user ruled on the one choice left open
"Keep full color support in themes" (review on
|
||
|
|
56632430db | Merge pull request #122 from fiddlerwoaroof/docs/adr-004-faces | ||
|
|
6a4c7dcd8e |
docs(adr): ADR 004 accepted — the user answered all five open questions
Status proposed -> accepted. Each answer is folded into the clauses and recorded verbatim in a new "What the user decided" section, with what it settled and where it now lives. The questions are kept: a decision whose alternatives have been deleted reads as though it was never in doubt. Two answers changed the decision rather than confirming it. REVERSE IS IN, AND MY OBJECTION TO IT WAS WRONG (Q3). The draft excluded it because "CSS, Swift and Qt would have to compute it, which makes resolution stateful". The user asked whether a class overriding custom properties could do it. It can, and working it through shows the objection was mistaken: the swap happens in the RENDERER, after resolution, reading the face's own two custom properties crossed — a re-reference, not a computation — so `resolve` stays context-free and clause 3 is untouched. The `Default` case, which is the one that would have broken it, is covered by the var() fallback. No reversed palette is needed, and neither is `--accent-ink`: a hardcoded ink token makes the same assertion about the surface that `fg(Color::Black)` does, one layer up. Swift and Qt swap two values they already hold. The portable set is six, and the twelve hand-rolled `fg(Black).bg(accent)` sites stop asserting the ink is black — a latent light-terminal bug. THE FORMAT IS JSON, AND THE DEFERRAL DISSOLVED RATHER THAN BEING MET (Q5). It was deferred on the condition that improvise grows a configuration story. Download-edit-upload is not a config file: no search path, no precedence, no absent-file policy — it is an export/import pair, the shape export_csv already has. So the larger question stays unmade. JSON costs no new dependency (serde_json is already in every crate but improvise-protocol, where it is optional); the types derive serde in the protocol and the serde_json call happens at the front ends, keeping the bundle footprint unchanged. "Editable by hand" constrains the shape: face names as keys, stable ordering, no surrogate keys. And a published face name is a compatibility surface — stated as a cost, not only a benefit. The other three. Q1 (the item-add/category-add split is an accident) is the one flagged as expensive and went the cheap way: no face name encodes it, and the default theme makes them agree — which is clause 8's ONE DECLARED EXCEPTION to the reproduces-today-exactly property, written down because the migration's whole safety argument is that it changes nothing. Q2 decomposes the thirteen DarkGray modes into five faces derived from distinctions AppMode already draws (minibuffer/panel/is_editing), with the three splits that could not be justified named rather than guessed; the user's "probably" is respected, since thirteen faces would have been a transcription. Q4 makes the second slot table optional, and a terminal mostly ignores a two-table theme because it HAS the user's palette — the tables are the browser's substitute for one. Detection declined: verified that crossterm 0.29 has OSC 52 but no OSC 11 background query. Kept deliberately: every rejected alternative; the palette-relative-vs- absolute clause Codex improved; and the honest statement that the protection is a property of the shipped theme rather than of the type. The user has not ruled on dropping `Rgb`, so it is now listed as a live open choice. Gotcha 48 applied to myself, since this touched five sections: grepped every claim before changing it, which caught "Seven clauses", the stale `--accent-ink` alternative, and two headings I had wrapped across lines. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
9d2bdd39e6 |
Merge pull request #131 from fiddlerwoaroof/fix/cli-honesty
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: the headless CLI was silent whether it worked or not, and the refusal named the wrong command |
||
|
|
ffa1a87cd9 |
Merge remote-tracking branch 'gh/main' into fix/cli-honesty
# Conflicts: # context/test-evidence/command.md |
||
|
|
69c873d3c7 |
Merge pull request #130 from fiddlerwoaroof/fix/vw47-view-name
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: a new view takes an unused name, derived once for both front ends |
||
|
|
6034a216e1 |
docs(site): publish the two new context docs, and de-number two stale counts
The `build` job failed, correctly: `scripts/build-doc-site.mjs` requires every `context/**/*.md` to be either in the SECTIONS manifest or a named omission (improvise-res9's cure), and the two files this branch added were neither. Both are now published, beside the TUI in each section. While there: `context/code/doc-site.md` asserted 'the seven `context/code/*.md` documents' when there were already twelve, and '501 entries' when the checker reports 801. Both were stale before this branch and this branch makes them staler, so they are de-numbered rather than re-asserted — a count in prose that nothing checks is a fact with no owner, and the shape that stops drifting is not having one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
42cbf0932b |
docs: name the follow-up, and stop the scratch dir looking like a ticket id
improvise-udt2 is the ticket context/code/cli.md section 3 points at: a status message carries no severity, so a refusal that does not abort cannot set a non-zero exit. It names both disproved classifiers, so the next attempt does not rediscover them. And the per-test scratch directory was named `improvise-cli-<tag>`, which matches the ticket-id shape the AGENTS.md audit greps for. An ID-shaped non-ID is worse than a dead ID, because the diagnosis is identical and the repair is not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
b8e1c138eb |
fix(command): a refusal named a command the user did not type
improvise-z05h, three bugs stacked on one surface.
`CmdRegistry::resolve` rewrites `formula` to `add-formula` *before* the arity
check runs, so `formula Product Total = Widget + Gadget` was refused with
"add-formula requires 1-2 argument(s), got 6" — a command nobody typed, and
an arity where the user needed to be told to quote.
The name now lives in the one place that knows which of the two was written:
`require_args` no longer takes a `word` and `CmdRegistry::parse` prefixes
`"{typed}: "` onto any `Err`. That removes 41 name literals from `cmd/`,
which is the argument gotcha 15 already makes about `registry.rs`. The
`AddFormulaCmd` validator states the shape rather than the arity.
`interactive` deliberately does not prefix — `dispatch` in
`command/keymap.rs` already formats `"{name}: {e}"` with the bound name
(improvise-55l5, PR #126), and doing both would double it.
All three documented spellings of `:formula` taught the unquoted form and so
were ones the parser refuses. help.rs carried the quick-start line **twice**,
on two different pages, which is gotcha 48's "changing a fact stated twice is
a grep, not an edit".
`every_ex_command_the_help_pages_show_is_one_the_parser_accepts` closes the
surface rather than the instance: `HelpLine::Keys` rows are derived from the
keymap and cannot go stale, but `Literal` and `Accent` rows are prose with
nothing checking them, and that is the whole unguarded surface. Run against
|
||
|
|
c13466f1d9 |
fix(cli): the headless commands were silent whether they worked or not
improvise-q914 and improvise-j6x8 are one bug seen twice, and the second
measured it: `run_headless_commands` applied effects and never read
`app.view_state.status_msg`, setting `exit_code` only for parse errors. A
successful command and a refused one therefore produced byte-identical
output — nothing — and exit 0 for both. `improvise script` is the same
function, so it shared the hole exactly.
Three decisions, argued in `context/code/cli.md`:
**Everything to stderr.** The tempting split — stdout for a report, stderr
for a refusal — cannot be built honestly, because `status_msg` is a bare
`String` with no severity and both candidate classifiers are disproved by
code already in the tree. "A refusal aborted" is false: `AddItem` writes its
`Err` to the status and does not abort, and that is q914's own example. "A
refusal is anything that spoke" is false too: `cmd/fit.rs` reports the seed
that reproduces a fit on **success**. Guessing between them is the adjacent
check design-principles §7 names.
**Exit 1 on a parse error or an aborted batch, and no further.** Both are
real flags. A refusal that only writes a status is now printed but still
exits 0, because nothing can classify it; the README says so plainly rather
than over-promising, and closing it needs a severity on `ViewState`, which
is a wire change reaching the browser.
**No quiet flag.** Silence is never the default and `2>/dev/null` already
exists.
Also: a run that changed nothing no longer rewrites the document. Saving
unconditionally normalised it — `1e20` to its twenty digits — which turns a
hand-edited fixture into a silent no-op. The gate accumulates
`model_state.dirty` per command rather than reading it once at the end,
because `effect::Save` clears the flag and `[add-item, w elsewhere]` would
otherwise drop the edit.
And the three clap doc strings advertised JSON, which the parser refuses.
`tests/headless_cli.rs` pins all of it at the process boundary, where an
exit code and a stream exist. Six of its twelve tests fail at
|
||
|
|
d0ec6a44a1 |
fix: a new view takes an unused name, derived once for both front ends
improvise-vw47. Create View 2 and View 3, configure View 3, delete View 2,
then ask for a new view: both resolvers spelled `format!("View {}", len + 1)`,
which is the count and not a free name. `Workbook::create_view` replaces
whatever holds the name, so View 3's format reset to defaults and no view was
added -- a symptom that looks like nothing happened.
The name is derived at two sites, `cmd/panel.rs::CreateAndSwitchView` and
`resolve.rs`'s "create-and-switch-view" arm, so fixing one would have left the
TUI and the browser naming different views. `view::unused_view_name` in
improvise-core is the one derivation both now call. It takes an iterator of
names because the browser holds no `Workbook`, only the `ViewListCache` that
`view_summaries` maps one-for-one off `workbook.views`.
`Workbook::create_view` is deliberately untouched. Its replacement semantics is
what the `_Records`/`_Drill` rebuilders need -- both reach it through
`effect::CreateView` on every entry -- so refusing or renaming there would turn
"a view was reset" into "the key does nothing". The argument is in
`context/code/core.md` section 11.
Three tests, written failing first. The helper's own cases pin that a run of
taken names is searched rather than bumped once. The `Cmd` test uses the
ticket's fixture and asserts the count is below the highest name before asking
anything, because the pre-existing sibling test's fixtures all leave the
derived name free -- which is how the bug lived under it. `lockstep.rs`
witnesses the agreement: note that `both_ends` PASSES against the unfixed
source, both ends being wrong identically, so that test carries the count and
format assertions too.
Rivals injected: fixing panel.rs alone fails lockstep on the document text;
starting the search at 1 fails the pre-existing incremental-naming test.
Baseline 1667 passed / 0 failed / 7 ignored over 37 result lines; now 1670.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
|
||
|
|
bf83294e15 |
Merge pull request #129 from fiddlerwoaroof/fix/label-and-views
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: a Label category survives a save, and a new view does not pivot it |
||
|
|
76d909ec6f |
Merge pull request #128 from fiddlerwoaroof/fix/bd9o-records-browser
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
The tile bar reaches records mode, where every key was silent (improvise-bd9o) |
||
|
|
afbe7e7d1f |
docs: record the measured generator coverage of the new kind_line rule
context/code/formula-and-io.md already says a new grammar production needs generator coverage or the proptests quietly stop describing the format. `kind_line` is an alternative of `category_entry`, so the grammar walk reaches it with no generator change — but that is precisely the claim the section exists to say must be checked rather than assumed. Probed over 500 files from generator::improv_file(): 14 carried a `kind: label` line and all 14 parsed to a CategoryKind::Label, so each 256-case proptest run exercises the production about seven times. The probe is not kept. Its rate depends on proptest's internal RNG, so as a standing assertion it would fail on a dependency bump for a reason that has nothing to do with the format — the measurement belongs in the argument, where it can be re-run and corrected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
b0739326f8 |
review: two sibling passages went stale when the new section landed
ChatGPT Codex on PR #128, correct on both, against head |
||
|
|
aa86a48af1 |
review: the "grid, not mode" clause was argued and unchecked
Adversarial review of
|
||
|
|
c80da1be36 |
fix: seat a category by its kind, not by its name
improvise-cj4u (P2). View::on_category_added took a name and asked CategoryKind::of_name, which reads Label as Regular by design. Workbook::add_label_category papered over that with a set_axis(None) straight afterwards, so views existing at that moment were right; Workbook::create_view had only names to hand and had no such correction, so every view created after a label pivoted it onto Row — and on an already-pivoted document onto Page, filtering the whole document down to one description. on_category_added now takes the CategoryKind, and the two set_axis corrections are deleted: one answer to "where does this category go" rather than two, of which only one caller knew to write the second. The signature change is deliberate — it is what made the compiler name every call site. Every site outside Workbook is test code and passes of_name(name), which preserves what those fixtures meant. Shares a theme with improvise-dy3j — a Label kind that cannot cross a boundary carrying only a name — but not a mechanism: dy3j is the file format, this is a function signature. Fixed separately for that reason. The control tests carry as much as the fix: a blanket `unseated = true` satisfies every label assertion in the repository, so the direction the bug cannot supply is asserted too, and a label is shown not to consume the Row slot the next regular category wants. Reasoning in context/code/core.md §9; evidence in context/test-evidence/core.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
d9f732c529 |
fix: persist a category's kind, so a label no longer unloads a document
improvise-dy3j (P1). CategoryKind::Label had no spelling in `.improv` at all: format_md wrote every category as a bare `## Category: Name` and build_category read every section back through add_category. The kind was dropped on the way out AND on the way in, so a round-trip equality test could not see it. A label is outside MAX_CATEGORIES = 12 by design and the import wizard scores it as zero, so twelve categories plus a label is legally constructible. On load every label came back Regular, the thirteenth was refused, and parse_md failed: any such document was permanently unloadable. Below the limit the loss was quieter — a label silently became a pivot category. Grammar first. The ticket proposed `## Category: Note [label]`, and improvise-pczv forbids it: a section header takes its name as the WHOLE line, so the suffix is indistinguishable from a category named `Note [label]`. The kind is therefore an entry inside the section, `kind: label`, spelled with a real token rather than a lookahead — the grammar-walking generator skips predicates. Absence means `regular`, so every existing file parses unchanged and a pivot category's section is byte-identical to before. Tests assert the parsed Category::kind, never the emitted line. Both rival faults were injected against the fixed tree and both fail two tests: a writer that omits the line, and a reader that discards the kind it parsed (whose file text is correct, so any text-comparing test passes it). Reasoning in context/code/formula-and-io.md; evidence in context/test-evidence/formula-and-io.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
d124f18c2e |
fix(browser): the tile bar reaches records mode, where every key was silent
`declined()` answers `toggle-records-mode` out loud and the code read as though that closed the route. It did not. The tile bar carries the virtual `_Index` and `_Dim` (`View::tileable_cats`), `set-axis-for-tile` composes locally, and records mode auto-detects from `_Index` on Row plus `_Dim` on Column — so a handful of undeclined keys land a browser in `RecordsNormal`, where `i` did nothing and said nothing. The silence had TWO sources, and the ticket predicted one. On a measure column the cell is a `CellId::Record`, so the editor opened and `Enter` died in `commit`'s `grid.records` early return. On a COORDINATE column the projected cell carries no `CellId` at all, so `edit_or_drill` short-circuited before reaching any arm — improvise-fdk1 seen from the key path, since a coordinate rename names a record the `RecordHash` surrogate cannot address. So the guard goes at the entrance of `edit_or_drill`, before the cell is looked at, rather than on the `Record` arm: a guard on the arm answers "is this cell one we cannot commit" where the question is "is this grid one we cannot commit in". It reads `grid.records` and not `mode.is_records()` for the reason `commit` already did — the grid is the observed fact and the mode settles from it. `CellId::Create` now declines instead of returning `None`, so a creating column reaching this function is a door rather than a hole. `create_column` (improvise-gcvb) was already unreachable and is now explicitly so. The regression guard walks every column of the records row and prints each cell's `CellId`, because a fixture on the measure column — the one the ticket's transcript stands on — cannot see the other two. Two rivals were injected and each fails on a different assertion; both are recorded in the evidence file. Closes improvise-bd9o. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
d46ec59984 |
Merge pull request #127 from fiddlerwoaroof/ui/header-stable
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: the app bar is reserved, not fitted, and a message is a toast (improvise-n6t8) |
||
|
|
4625593dbb |
Merge pull request #126 from fiddlerwoaroof/fix/command-layer
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(command): two P1s in the key path, and both were about silence |
||
|
|
1f222e6215 |
style: centre the view chip in its reserve, as the mode pill already is
Found by looking at the screenshots rather than by a test, which is the only instrument that reaches this: `#view-name` is now a fixed 16ch, and "Plan" left-aligned in it read as a pill that had failed to shrink rather than as a deliberately reserved slot. Centring is what makes the reserve legible as a reserve. No test moves -- the header spec measures boxes, and text alignment inside a fixed box changes none of them. That is the honest scope of the guard and not a gap in it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
87a9377de7 |
Merge gh/main (#125 decimal default) into fix/command-layer
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH # Conflicts: # context/test-evidence/command.md |