1236 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
Edward Langley	c73df4b5a1	kb: a pipe discards the exit code you meant to read ... Fourth occurrence in one campaign, and this one was in a tool I wrote TO BE the reliable answer. scripts/pr-reviews --wait correctly exited 1 on a timeout, and my invocation ended in '| tail -20; echo', so the harness reported 0 — a review window that had EXPIRED read as a review that had ARRIVED, which is the dangerous direction. The earlier three: pr-watch reading tr's status through a pipe; a probe reporting EXIT=0 from a checker that had exited 1; and PIPESTATUS in zsh, where the array is lowercase pipestatus — the two shells differ, which is its own trap. The script's usage now says not to pipe it. Closes improvise-711n (,.2f yielded zero decimals — the spelling our own docs show) and improvise-x1ns (the preflight undercount ChatGPT Codex found on a PR I had already merged unread). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 10:56:36 -07:00
Edward Langley	60bb3c56f1	Merge gh/main (PR #97) into orchestrator main	2026-09-21 10:56:33 -07:00
Ed Langley	f5df19553e	Merge pull request #97 from fiddlerwoaroof/wave/format-and-fit ... Wave M: number-format precision (711n), preflight under-count (x1ns), e8no declined	2026-09-21 10:55:57 -07:00
Edward Langley	0466da173d	kb: an unreachability claim is only as wide as the routes enumerated ... Three from Wave L's Codex round. The first is the campaign's cleanest statement of a recurring failure. PR #98 predicted its own composition-time mode drift and declared it unreachable because the toggle route (R) is a stranding decline in the browser. True of that route. A VIEW SWITCH reached the same drift, because records-ness is auto-detected from the axes so a switch changes it with nothing in the batch saying 'records'. The enumeration of routes is the part nothing checks, so an unreachability argument is evidence about the routes you thought of and nothing else. The remedy is the thing worth copying: close the CLASS by observation rather than patch the route. resolve::changed_mode is deleted; ClientState::fold settles against the grid it holds, so no enumeration of composition sites is load-bearing. Read-time derivation was rejected for having the same window and for moving the enumeration to readers, where a missed reader is silent. Same shape the repo already prefers: cursor_epoch over matching on CycleAxis, document_generation over matching on LoadModel. And a test-construction trap found by the author in its own bidirectional test: a precondition computed from the same tuple element as the assertion cannot be relaxed one iteration at a time, so the test fails for the wrong reason — which reads exactly like the bug. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 10:49:35 -07:00
Edward Langley	e7c63e9530	kb+docs: strengthen the check, do not settle for correcting the prose ... Three facts from Wave N's Codex round. The first is the one that generalises. improvise-26dm's round 1 found its OWN defect — the right-hand side of a completeness diff was a second hand-written list, not the derived enumeration its prose claimed — and shipped an honest correction of the prose. Codex asked for the fix instead and was right: the disclosure was accurate and still the weaker artefact, because the fix was four lines of plumbing. Correcting prose is the fallback, not the deliverable. Two traps, both of which caught someone: cfg(test) and pub(crate) are different walls, and only one is a visibility keyword. The follow-up said 'pub(crate) -> pub' — filed by the agent that wrote it and repeated by me when I authorised 'one token' — because both of us read the item line and not the attribute above it. And cargo tree shows DEV edges by default, so asking whether a new feature leaked serde_json into the wasm bundle reports it PRESENT. --edges normal reports 0. I verified both readings myself. Same class as the difftastic grep trap: a command that looks like it answers the question, answering a neighbouring one, in the direction that reads as a finding. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 10:44:05 -07:00
Edward Langley	23c81977eb	kb: an invariant generalised from a fix generalises the fix ... Three facts from Wave M's Codex round, and the middle one is about the kind of mistake that is hard to see from inside. Having repaired the categorical under-count, the agent proposed a never-under-counts invariant WHILE THE TREE ALREADY HELD A COUNTER-EXAMPLE — the missing Date arm, which Codex then found. An invariant written immediately after a fix tends to state what the fix established rather than what the code guarantees, because the evidence in front of you is the defect you just closed. The question that would have caught it is about the OTHER function: 'what else does the thing I approximate do that I have not mirrored?' The agent retracted it unprompted, before it was ever recorded. Its structural fix is the right shape: encoded_width now matches Forced with NO WILDCARD, so a fourth variant is a compile error rather than a silent categorical fallback — which is exactly how Date was missed. And the reason both bugs were invisible to a green suite: an advisory bound over another function is a duplicated derivation, and nothing compared the two at all. A width asserted against a constant passes at every stage while the approximation drifts. The only honest test is to run the thing it approximates and compare against what it actually produced. Noted while filing: scripts/kb refused the first draft with dangling_citation, because the doctrine named an invariant that was retracted and never recorded. The same class as a dead ticket id, caught by the tool. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 10:41:14 -07:00
Edward Langley	67905d9440	scripts: pr-reviews, because twenty PRs merged with their reviews unread ... Prints a PR's reviews and says whether each targets the CURRENT head or an earlier one, because that changes what you do: a finding against an old commit may be fixed or still live, and both have happened this week. --wait blocks until a review lands against the head. Observed latency is ten to fifteen minutes after a push, so a merge four minutes later races the review that was about to arrive. When it times out that is a decision to merge unreviewed, not a clean bill. The detection rule is placement, not authorship: Codex posts under the owner's account, the same one our agents use, and ENDS with its attribution where ours OPEN with theirs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 10:40:38 -07:00
Edward Langley	c2bc939608	improvise-x1ns (2nd): the preflight counts a date measure's parts ... P1 from ChatGPT Codex on #97, live at f128087 — in the change that fixed the first instance of this same defect. `preprocess` infers Numeric -> Date -> Categorical, and `encoded_width` had no date arm at all. A date string does not parse as f64, so a measure column holding dates fell through to the categorical count. `build_date` emits epoch_day/year/month/day/dow, and seven parts once any cell carries a time, so a column with ONE distinct timestamp promised 1 and materialised 5. Against a budget of 64 that is an under-count of four on a single column, in the direction that spends the allocation the preflight exists to prevent. The fix is structural rather than another remembered branch, because two instances one branch apart is the tell. `inferred_kind` reproduces `preprocess`'s own match arm for arm and returns a `Forced`; `encoded_width` matches that with NO wildcard, so a fourth variant is a compile error here instead of silently taking the categorical arm — which is exactly how Date was missed. The .missing flag moves out of the arms, since every branch owes it on the same terms. The fixture carries a time case because of the RIVAL FIX, not the bug: "a date column is 5" and "5, or 7 when any cell has a time" both repair the plain-date failure and differ only on a column containing a time. Injected, `date_parts` hard-coded to 5, the plain-date case still PASSES and the time case fails at "promised 8 ... materialised 10". Injecting the bug proves the test sees the bug; injecting the rival proves it sees the rule. Both arms compare against `data.feature_names.len()`, never against a width computed a second way. Also recorded in gp.md: the "must never under-count" line was written as a proposed kb.pl invariant while the tree already held this counter-example. An invariant generalised from the defect just fixed generalises the fix, not the code; the question that would have caught it is "what else does `preprocess` do that I have not mirrored", which is about the other function and not the bug. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 10:35:55 -07:00
Edward Langley	95657dc90b	kb: inject the rival fix, and a preflight inherits every branch it guards ... Both from the first use of the read-the-reviews rule, which found a P1 Codex review on each open PR. The preflight one is the sharper. encoded_width guards preprocess, which dispatches Numeric -> Date -> Categorical, and it mirrored two of three. improvise-x1ns was the categorical arm undercounting the (missing) one-hot; Codex then found the DATE arm missing entirely — a date column takes the categorical fallback because '2024-01-15' does not parse as f64, so a column holding one distinct date promises 1 where build_date materialises 5, or 7 with a time. Two findings, one shape, one inference branch apart. A preflight is a second implementation of the thing it guards, so it inherits every branch of it, and the branch nobody mirrored is the one nobody was thinking about. Note the second finding contradicts an invariant proposed in the same hand-back that fixed the first: the counter-example was already in the tree. And the Wave M agent's own best insight: check_the_checker proves a test sees the BUG, not that it encodes the RULE. Two implementations both repair ',.2f' and differ only on '.2f3', so the discriminating injection is the other plausible fix rather than the original fault. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 10:34:12 -07:00
Ed Langley	730f39a514	Merge pull request #95 from fiddlerwoaroof/docs/read-the-reviews ... docs: read the reviews before merging; authorship will not identify them	2026-09-21 10:33:57 -07:00
Edward Langley	267769a497	Merge commit 'ea206bf' into wave/format-and-fit	2026-09-21 10:33:45 -07:00
Edward Langley	f128087999	review: correct six prose claims, four of them false ... An adversarial review of this branch found that the prose was worse than the diff — including, twice, in the exact genre the branch was congratulating itself for catching. Recorded rather than quietly rewritten, because that is the shape. Must-fix, all verified before correcting: - `fit.rs`'s comment on the exact check still read "this is the check that enforces the bound in both directions of a drift" — verbatim the sentence gp.md §14 now spends 25 lines refuting. Gotcha 48's third clause exactly: a fact stated twice is changed by grep, not by edit. I refuted the copy in the prose and left the copy in the code. - "false for eight months" was fabricated. `encoded_width` landed 2026-09-19, the sentence 2026-09-20; the repository itself is six months old. A paragraph whose point is that unchecked cost claims are checkable closed on one. - "NumericCategory was the one artefact family with no coverage" is false: it was one of three, and `ValueOfAMeasure` and `AbsentCoordinate` still have none. Corrected in both files that said it, and the remaining gap is named. The decline's stated reason was wrong, which matters most because a decline is the artefact nothing checks. It claimed one-hot is the only encoding of a category whose splits are spellable; `is_expr` is a `primary`, so an ordinal split IS spellable as a weighted indicator sum. Verified by running `parse_formula`, not by re-reading the grammar, and now pinned by `an_ordinal_split_on_a_category_is_spellable_as_an_indicator_sum` so the paragraph cannot go quietly wrong again. e8no stays declined on grounds that survive: a category's items carry insertion order rather than magnitude, and the emitted formula spells one IS per item either way, so the width is not saved. Also weakened two overclaims — "marking it a Label yields the same fit" (false: the feature count moves the RNG draw sequence, gotcha 37b) and "asserted rather than argued" (the dichotomy is argued from spell()'s exhaustive match; the test asserts one instance) — narrowed "the only coordinate predicate is is_expr" to what is true of it, corrected an uncheckable test count, named which two of the three consumers of parse_number_format are actually guarded, and moved an evidence block that had been inserted under the wrong module heading. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 10:26:27 -07:00
Edward Langley	aa06a9db84	improvise-711n: pin that a multibyte format string cannot split a char boundary ... `format:` is `rest_of_line` in `improv.pest`, so the format string can hold any text. `leading_digits` slices at the byte index `str::find` returns, which is a char boundary by construction — but that is an argument, and this is the test. It also pins that a non-ASCII digit (U+0662) names no precision rather than one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 10:19:14 -07:00
Edward Langley	e045f9871b	improvise-x1ns: the preflight counts the (missing) one-hot; improvise-e8no declined ... improvise-x1ns (found by ChatGPT Codex on PR #90, live on main at ce895ff). `encoded_width` computed `has_missing` and used it in the numeric arm only, while `build_categorical` maps every absent cell to MISSING_CAT and includes it in `distinct(&mapped)`. A forced category with N distinct present values and one record lacking that coordinate promised N features and materialised N+1. That under-reports in the direction that costs. The preflight runs before `preprocess`; the exact check on `feature_names.len()` runs after, so it bounds the error message and not the allocation the preflight exists to prevent. gp.md §14 claimed drift was "bounded in both directions by the exact check" — wrong, and wrong against the argument two paragraphs above it in the same section. That sentence is corrected rather than softened; the previous wave read this shape and reasoned it away with it. The clamp stays outside the addition, so an entirely-missing column — which `preprocess` skips — is still counted as one and the fix moves only its own case. improvise-e8no is DECLINED, on evidence rather than on appetite. `formula.pest`'s only coordinate predicate is `is_expr`, a single-value equality: no IN, no ordering over items, and a coordinate reaches an `Expr` only through IS. So the predicates a tree may state about a category are exactly its items, one at a time, and one-hot is the only encoding whose splits are spellable — the width is the size of the vocabulary, not an implementation's appetite. Ordinal encoding is already the classified refusal `Artefact::NumericCategory`, and hashing has no document name at all. The pre-zv8t "fit that succeeded" therefore either never split on the column, in which case Label is exactly equivalent and in budget, or was refused at storage time: no fit is lost that was both storable and used the column. Asserted by a_split_on_a_numeric_category_read_as_a_number_cannot_be_stored, which is also the first coverage NumericCategory has had. Both refusal messages said "a category with a distinct value per record becomes one column per record", which diagnoses the runaway shape and not e8no's. They now say a category becomes one column per distinct item, whether its items read as numbers or not — that clause being the whole of what zv8t changed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 10:18:38 -07:00
Edward Langley	ea206bf8de	docs: read the reviews before merging; authorship will not identify them ... I merged twenty PRs this campaign without reading their REVIEWS. CI green plus a dispatched adversarial agent felt like the whole gate and was not. Two ChatGPT Codex reviews were sitting on merged PRs; both were correct. improvise-x1ns (P2, filed, live on main): encoded_width's categorical branch counts only distinct PRESENT values, while build_categorical maps every missing cell to MISSING_CAT and emits an additional one-hot. A forced category with N present values plus one missing coordinate passes the preflight as N and materializes N+1 — caught only by the later exact check, which is the allocation the preflight exists to prevent. The keeper from that review: 'bounded by the later exact check' is not a defence, because the exact check runs AFTER the allocation. It bounds the error message, not the cost. Our own review had seen the shape and reasoned it away on exactly that ground. The PR #91 review described round 1 and round 2 fixed it — verified with a probe rather than assumed, both directions. But the regression it asked for does not exist, so improvise-4dpj now carries it. Three things make a Codex review easy to miss, all now in AGENTS.md: it arrives as a review rather than a comment; it posts under the owner's account, the same one our agents use, so authorship cannot distinguish it — the tell is the TRAILING '_Automated review by ChatGPT Codex_' where ours OPEN with '_Posted by Claude_'; and it may target an earlier head, so a finding can be already fixed or still live and you must check which. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 10:15:30 -07:00
Edward Langley	3775d4ea1a	improvise-711n: a trailing conversion letter no longer erases the precision ... `parse_number_format` read the precision as `fmt[i+1..].parse::<u8>()` over everything after the last `.`, so `,.2f` — the printf spelling the repo-map's own `.improv` example carries — failed to parse and fell through to zero decimals, silently, in the TUI, the browser projection and CSV export. The precision is now the leading digit run after the last `.`, with any conversion letter ignored. The spellings that are legal are stated once, in `context/code/core.md`, along with why the trailing `f` is accepted rather than refused (the grammar takes the format line as an opaque `rest_of_line`, so refusing it rejects documents this repo already writes) and why the reading is printf's rather than Excel's (`.00` is the spelling the two disagree on). `export_csv_quotes_commas_per_rfc4180` asserted the value `starts_with("1,200")` against a `,.2f` fixture, so it could not tell two decimals from none. It is now an exact equality, and reverting the fix fails it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 10:14:27 -07:00
Ed Langley	ce895ff560	Merge pull request #94 from fiddlerwoaroof/docs/ext-diff-and-ci ... kb: a check that returns empty for the wrong reason passes	2026-09-21 10:09:13 -07:00
Edward Langley	d72d7558d3	kb: a check that returns empty for the wrong reason passes ... Two doctrines, both about checks that cannot fail, both found this round. This repo configures diff.external = difftastic, which prints side-by-side output carrying no unified-diff markers — so 'git diff | grep "^-"' matches nothing regardless of content. An agent verifying that a merge dropped nothing read three consecutive empty outputs as three clean bills. I reproduced it: on a file with one real deletion, git diff | grep -c '^-' gives 0 and --no-ext-diff gives 1. Use --no-ext-diff, or --numstat/--name-only which are unaffected. That is the sharpest form of check_the_checker in the campaign: an empty result is the one outcome nobody investigates. And the CI diagnosis, recorded with my own error in it. A pull_request workflow builds refs/pull/N/merge; a CONFLICTING PR has none, so no run is created and it looks identical to a dropped webhook. 34 minutes of polling, a close/reopen and an empty commit produced nothing; one merge produced a run in seconds. Ask gh pr view --json mergeable first. Filed improvise-hsoe: --check-evidence cannot see a duplicate heading, and three exist on main — verified. Both copies pass, because a duplicate names a running test just as well, and one of them is stale by construction. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 05:41:31 -07:00
Edward Langley	950c16bceb	Merge gh/main (PR #92) into orchestrator main	2026-09-21 05:39:08 -07:00
Ed Langley	be0be40b5a	Merge pull request #92 from fiddlerwoaroof/wave/protocol-render-gaps ... wave I: protocol render and basis gaps (t6db, 5ubs, 1wl0, h0iv, jmjz, 1l9o, 44ad, 9v0j)	2026-09-21 05:38:40 -07:00
Edward Langley	d41c8b8e6f	Merge main (1a9f45a) into wave/protocol-render-gaps ... #91 and #93 landed while this wave was in flight and appended to the same four context/ files. That conflict is why CI never ran: a pull_request workflow builds refs/pull/N/merge, and a CONFLICTING PR has no merge ref, so no run is ever created. Neither the close/reopen nor the empty commit could have helped. Resolution: keep both sides. Only context/test-evidence/protocol.md conflicted textually; the other three auto-merged. Verified with --no-ext-diff that nothing of this branch was dropped -- the only removals are main's own rewrites, of the frame-basis paragraph in context/code/command.md and of the minibuffer-exit entry (renamed _to_plain_normal -> _to_records_normal) in test-evidence/command.md. NOTE the first three "nothing was dropped" checks were WORTHLESS and read as clean: an external diff tool is configured, so `git diff | grep '^-'` was grepping a side-by-side format for unified-diff markers and matched nothing whatever the content. --no-ext-diff is what actually answers the question. A check that returns empty for the wrong reason is indistinguishable from one that passes. Fixed while re-reading, and pre-dating the merge: this branch had TWO entries headed a_window_narrows_the_cells_and_neither_the_totals_nor_the_widths -- the original, describing a fixture that no longer exists and repeating the claim the PR #92 review measured false, and the superseding one. Merged into a single entry whose subject is what each of the three fixtures was blind to. The evidence checker cannot see this: it verifies that a heading names a real test, and a duplicate heading names one just as well. Merging gh/main (1a9f45a) rather than local main (1ec43a7): local main carries an unpushed orchestrator kb.pl commit, and the PR's merge ref is computed against what is on GitHub. Gates on the merged tree: 1486 passing, 0 failing; clippy -D warnings, fmt, wasm32, both comment-check spellings and kb check all clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 05:33:07 -07:00
Edward Langley	1ec43a7aa0	kb: a conflicting PR runs no CI, because it has no merge ref ... I diagnosed PR #92's missing CI as a dropped webhook and was wrong. A pull_request workflow runs against refs/pull/N/merge, and GitHub cannot build that ref for a CONFLICTING PR — so no run is ever created and the PR shows zero checks, which looks exactly like a dropped event. #91 had merged and both PRs appended to the same four context/ files. The control I ran was sound and the conclusion was not: pushing to another PR produced a run in five seconds, which correctly proved delivery works repo-wide. I read that as 'therefore the event was dropped', when the truth was that no event was ever generated. A control discriminates only between the hypotheses you listed, and ours had two entries. Diagnosis order, now written down: before theorising about delivery, ask gh pr view N --json mergeable. One call, and it answers first. The agent was right to refuse to guess and right that its evidence could not separate its two hypotheses. Neither of us listed the third. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 05:30:02 -07:00
Ed Langley	1a9f45a438	Merge pull request #93 from fiddlerwoaroof/docs/wave-j-facts ... docs+kb: the map pointed at the wrong file, and that manufactured a false claim	2026-09-21 05:29:30 -07:00
Edward Langley	8acce9dfa8	ci: nudge — the pull_request event for 902a77c was never delivered ... No workflow run exists for 902a77c, pushed at 11:36:42Z and confirmed as the PR head. A close/reopen did not help. Delivery is not stalled repo-wide: a push to docs/wave-j-facts at 12:11:54Z produced a pull_request CI run at 12:11:59Z, five seconds later. So the event for this SHA was dropped individually, and re-triggering is correct. This moves the head, so the adversarial review at 46c3cfa/902a77c must be re-confirmed against the new SHA. The tree is identical to 902a77c. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 05:12:53 -07:00
Edward Langley	99ec32d6f3	kb: comparing a function against itself tests nothing ... Two facts from the adversarial review of PR #92, both about checks that cannot fail. improvise-jmjz guarded its width half with col_widths == compute_col_widths, but compute_col_widths IS grid_body with an empty window — the same function twice — so windowing degrades both sides identically and the assertion cannot fail. Injecting the fault left 1476 tests passing. A consolidation is exactly when such a check gets written, because the second implementation has just been deleted. And a negative result is only as good as the setup that produced it: the none_cats hazard was reported 'argued and unobserved' from a fixture whose stated condition was absent, because add_category defaults a category to Column and nothing moved it. Setting the axis explicitly failed the property on the first run and found improvise-ac2z. Filed from the same review: improvise-711n (parse_number_format yields zero decimals for the ,.2f spelling our own docs show, P2) and improvise-swre (memoise the grid across frames). improvise-jmjz closed on its own criterion; improvise-6nsd lowered to P4 with the severity note. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 05:11:52 -07:00
Edward Langley	902a77c174	review: two corrections from the #92 adversarial review, and a P2 they found ... 1. THE JMJZ WIDTH ASSERTION COULD NOT SEE A WINDOWED WIDTH, and the evidence said it could. Injecting `if drawn { widen(..) }` -- widths from the window only -- left 1476 passing, 0 failing. Two reasons, the second structural: the measure was named `Revenue` (7 chars) against a value of `300021` (6), so the assertion passed on the header alone; and comparing grid_body's widths against compute_col_widths is INCAPABLE of testing windowing, because compute_col_widths IS grid_body with an empty window. Windowing degrades both sides identically. The same function twice is not a check, and it reads exactly like one. Fixture is now the reviewer's shape -- a total that cancels and a one-character header -- reordered once more so BOTH assertions discriminate: with the large values last, the windowed row and the cancelled total were both 1.0, so the totals assertion went blind to the row-window fault while the widths assertion happened to catch it. Large values first: for ri in rows.clone() -> totals [888888888.0] against [1.0] if drawn { widen(..) } -> widths [10] against [11] 10 rather than 5 is a smaller gap and a better witness: the missing character is the minus sign of a row outside the window. 2. THE T6DB none_cats PARAGRAPH REPORTED A NEGATIVE RESULT FROM A SETUP THAT LACKED THE CONDITION -- and correcting it found a P2. The fixture was said to put Product on Axis::None in one view and on Column in another. It did not: add_category defaults to Column and nothing moved it, so all three views had none_cats {_Index, _Dim} and the stated input to the hazard was absent. Setting it explicitly failed the property on the first run, and not on a cell value: Second loses BOTH Total columns when the host stands on Default. rebuild_layout fills Model::formula_cache for the HOST's view, and grid_in on the elsewhere path builds another view's layout against that stale cache -- the Total cells are keyed without a Product coordinate, the other view asks for one, finds nothing, and prune_empty drops the columns. Filed improvise-ac2z (P2). Not local: grid_in takes &App and recompute_formulas needs &mut Model. Guarded by a_hosts_none_cats_leak_into_another_views_formula_columns, a characterization test whose message says that failing means the bug is fixed. The property skips pairs whose none_cats differ and asserts compared > 0, so a future fixture cannot skip everything and pass. 3. Smaller, all taken. "Nothing else fails" on the t6db injection was wrong: it fails three tests, including the pre-existing prune-invalidation one -- the #78 fix had more coverage than it was credited with. And the 5ubs should_panic named one thing and exercised another: pointed at a struct it reached only the split_once branch, and all three panics shared one literal so `expected` could not discriminate. The parse is now a `parse_expected` seam fed hand-written mutated messages, each refusal with its own wording, plus a happy path. 1470 -> 1475 passing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 04:36:42 -07:00
Edward Langley	20322ada6f	Merge gh/main (PR #91) into orchestrator main	2026-09-21 04:29:27 -07:00
Ed Langley	ea59126a97	Merge pull request #91 from fiddlerwoaroof/wave/command-tui-gaps ... Wave J: command/TUI gaps — records-mode minibuffer exits, the cursor basis at a mid-batch switch, chord detection	2026-09-21 04:28:46 -07:00
Edward Langley	40ce4bcbee	docs: the map pointed at the wrong file, and that manufactured a false claim ... Root cause of PR #91's blocker 1, and it was mine. The row 'Change what a browser resolves without the host' named improvise-wasm-client/src/keys.rs. That file is 187 lines holding only BrowserKey — a KeyboardEvent flattened to an improvise key. The resolve function and KeyOutcome live in improvise-protocol/src/resolve.rs and are shared by both front ends. So the author consulted the map, opened the file it named, grepped it honestly, and wrote a true-and-irrelevant negative claim into context/ — the one place design-principles §6 says a reason must not be wrong, because nothing can find it later. A stale pointer does not merely fail to help. It manufactures confident false claims, because the reader has done exactly the work the map told them to do. The whole wasm-client inventory block was stale with it: keys.rs listed at 1597 lines against an actual 187 with a description of what resolve.rs does, render.rs 669 against 888, socket.rs 444 against 289, lockstep 606 against 1126 — and state.rs listed at all, when ClientState moved to improvise-protocol/src/client.rs. My earlier line-count refresh missed every one of them for the reason I documented at the time: the wasm-client/ shorthand prefix resolves to no path, so a path-walking refresh skips the block silently. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 04:25:30 -07:00
Edward Langley	d9d456c890	kb: serde's derive is the one enumeration the compiler maintains ... improvise-5ubs is solved, and not the way anyone proposed. Closing a length-not-completeness check needs an enumeration INDEPENDENT of the derivation, and stable Rust has none for an enum carrying data: variant_count is nightly, strum::EnumIter cannot construct a variant holding a MinibufferConfig. Serde can — the Deserialize derive builds the complete variant-name list from the enum definition and publishes it in its unknown-variant message. variants.rs reads it back and diffs it against what the cycle reached. The dependency on serde's message format is made loud rather than silent: it panics naming the message if the split fails and asserts the parsed list has more than one name, so a format change cannot degrade into a vacuous pass. Verified by reading it rather than taking the report. Also: a consolidation's frozen behaviour is a date, not a decision. A consolidation must preserve what it moves or its agreement test cannot tell 'both ends agree' from 'both ends changed together' — so the rule is frozen for the duration of the move, and afterwards a single home reads as decided. improvise-8qlr froze name-only group coalescing; the test still pinning it justified itself with 'because that is what src/ui/grid.rs uses', a clause 8qlr had already made false. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 04:19:41 -07:00
Edward Langley	a10e95eb5f	fix(mode): settle Normal/RecordsNormal once, after the rebuild (review of #91) ... Two blockers from the adversarial review. BLOCKER 2, a new wrong state this PR introduced: resolving `AppMode::Normal` per-effect reads the batch's OPENING layout (gotcha 23), so a batch that CROSSES — `:toggle-records-mode . enter-mode normal` — settled against the grid it started on and left `RecordsNormal` over a pivot grid. Base was correct there. The fix moves the resolution to one place, `App::settle_normal_mode` called after `rebuild_layout`, where there is no crossing case to get wrong. That is strictly simpler than what it replaces: `ChangeMode` is an ordinary `simple_effect!` again, `ReturnToNormalUnlessMoved` and `CloseWizard` write plain `Normal`, and the per-edit coverage question the review filed as finding 3 is gone because there is one edit. Removing its call site fails exactly three tests, workspace-wide. And the prose defended the staleness with an example that does not bear the weight: `:toggle-records-mode` works, but because `ToggleRecordsMode` and `ViewBack` overwrite the answer in both directions, so it exercises nothing. Replaced by a test that drives the crossing both ways. BLOCKER 1, a false claim in `context/`: "keys.rs emits no ChangeMode" is true and irrelevant — keys.rs holds only `BrowserKey`. The live browser key resolver is `improvise_protocol::resolve`, which emits `Command::ChangeMode(Normal)` in five places, and `reduce.rs` folds it with no resolution. The browser front end still has the sihr bug; it is LIVE, not latent, and it compounds because the client's only records-ness fact is the mode it just lost. Named honestly, with improvise-e5pf cited. Still not closeable here — a client holds a `GridCache`, not a `GridLayout`. Also the two corrections: the thpe test's doc comment no longer claims a discrimination it does not have, and the 2ud deferral notes that `registry.rs`'s `visible_cols: 8` is production, not a test helper. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 04:18:36 -07:00
Edward Langley	fe816c9734	kb: a mitigation that names the wrong file is not a mitigation ... PR #91 deferred a wire-level gap because 'keys.rs emits no ChangeMode at all'. True, and irrelevant: keys.rs holds only BrowserKey, and the live browser resolver is improvise_protocol::resolve, which emits ChangeMode(AppMode::Normal) at five sites. The browser front end kept the very bug the PR fixed for the TUI, and the suite was green over a divergence the PR created. I copied that false clause straight into improvise-e5pf when I filed it. Corrected, and raised to P2: it is live, not latent. The rule: before writing 'X does not do Y' as the reason a gap may be deferred, grep for Y rather than for X. The claim that matters is about the behaviour, and the file you happen to have open is not the only place it lives. Also records that improvise-sihr's prose defended its staleness with an example that works for a different reason — toggle-records-mode overwrites normal_mode()'s answer in both directions, so the staleness is neutralised there rather than exercised, and the case that would have tested it regressed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 04:15:16 -07:00
Edward Langley	94b9408425	Merge remote-tracking branch 'gh/main' into wave/command-tui-gaps	2026-09-21 04:15:01 -07:00
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	2026-09-21 04:13:57 -07:00
Edward Langley	a449149998	docs: cite the two follow-up beads filed for this wave ... improvise-6nsd (a prune-by-write renumbers the grid without moving layout_version) and improvise-26dm (the fourth length-not-completeness list, in ui/effect.rs). Both read back after bd create. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 04:13:16 -07:00
Edward Langley	27399b6a28	kb+docs: prose that calls a change free is a claim, and it is checkable ... Wave K's own prose said improvise-zv8t cost nothing — "no document that fitted before stops fitting" — reasoning that a tree splitting on a numeric category was refused as an artefact anyway. False: the refusal is conditioned on used_features, so a fit that never split on the column SUCCEEDED, and the wide columns are exactly the ones a fit has least reason to split on. Measured with a throwaway probe on both sides. The sentence sat one paragraph below the "permanent refusal" claim the previous wave had just corrected, in the same section, green on either reading. So prose_that_calls_something_permanent_forecloses_the_ticket has a sibling. A cost claim is as checkable as a permanence claim and is checked the same way: construct the document the claim says does not exist. Filed improvise-e8no for the cost, with a note not to close it by making the forcing conditional on cardinality — that is inference by another name. The branch's own adversarial round found a bug the branch introduced: forcing by column NAME while budgeting by column INDEX, and a document may legally name a category and a measure the same thing. Repaired as a data dependency rather than a moved line, so the injection is a compile error. GridLayout::new has now caused FOUR bugs. improvise-iot6 was the last production instance — every remaining call site is test code, which I verified rather than took on report: two of them sit inside cfg(test) blocks well below the module head and neither was named in the hand-back. Suite re-measured here: 1464 over 35 binaries, 1 ignored. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 04:10:12 -07:00
Edward Langley	ae1570b508	docs(command): every reframe is NOT also a reshape, measured (improvise-9v0j) ... 9v0j asks for the general relation between view_frame and view_shape, which are separated by example and never stated as a property. One direction is the known asymmetry -- AddCategory of an empty category reshapes without reframing. The other was open, and the ticket guessed "probably yes, and if not, layout_version under-reports, which per gotcha 35 leaves a client wrong forever". Probed by driving each ModelCommand through apply_command and reading cursor_epoch() and layout_version(). Every command in reshaping_commands() moves both; a ClearCell that prunes a row out of a pruning view moves the cursor epoch and NOT layout_version. That is improvise-mcq4 working as designed -- a cell write changes data, not the view's configuration, and view_shape hashes configuration. So the proposed property test would FAIL and writing it would be wrong. What remains open is narrower and stated as a question rather than answered: the grid has renumbered while layout_version has not moved, so a client still claiming the old layout tier is accepted. The record tier covers a write to the pruned record; it says nothing about a write to a surviving one whose row index moved. Whether that is a hole in ADR 001's middle tier or correctly delegated is an ADR question. No code change: view_frame/view_shape live in ui/app.rs, which is another wave's ground this round. Also recorded: the probe needed the right fixture to say anything. Against app_with_two_pruning_views, seven of the twelve commands are no-ops because that document has no Product category, and a table of false/false rows reads exactly like a finding. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 04:09:27 -07:00
Edward Langley	822547a332	perf(project): one walk of the grid instead of three (improvise-jmjz) ... grid_of evaluated the whole grid roughly three times per frame: compute_col_widths walked every cell for its text, then every cell again for the column totals it must size, and grid_of then evaluated the window a third time. Every result was discarded at the end of the frame. ui/viewport.rs::grid_body is now the one walk. It evaluates each cell once and derives all three answers from that value -- widen the column, add to the column's sum, and if the position is inside the requested window keep the CellValue for cell_display to dress. compute_col_widths survives as grid_body with an empty window, because the keypress path wants widths and no cells, and two width rules would be the disease improvise-edp names. MEASURED, orchestrator's machine, cargo bench --bench grid_projection, before and after: typical (400 cells) 1.840ms -> 0.687ms large (9,600) 172.6ms -> 50.0ms 18k (gotcha 41) 294.4ms -> 95.2ms pathological (90,000) 4.481s -> 1.320s About 3x, which is the number of passes removed -- the shape the acceptance criterion asked for rather than a speedup argued from the structure. Three things the ticket did not predict. The benchmark's col_widths column now measures something ELSE. It read 45-47% and now reads 64-82%, which looks like a regression and is the opposite: compute_col_widths IS the fused walk, so its share rose while the frame fell by two thirds. A ratio whose denominator moved is not comparable across the change, and the benchmark now says so. The windowing ratio did not improve, and that is correct: the pass that remains is exactly the one a viewport cannot narrow. improvise-i5tk's decline stands and the two findings are now cleanly separated. It is not solved, only three times cheaper -- 18k cells is still 95ms, 5.7 frames. The next lever is caching ACROSS frames. Guard: a_window_narrows_the_cells_and_neither_the_totals_nor_the_widths. Fusing three consumers into one walk creates a hazard the separate passes did not have, and the natural "optimisation" (iterate rows rather than 0..row_count) makes a column total the sum of the visible rows, silently. Watched fail with left: [1.0] against right: [300021.0]. All 1469 pre-existing tests unchanged, which is also the evidence the outputs did not move. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 04:07:51 -07:00
Edward Langley	8c47377edf	Merge gh/main (PR #90) into orchestrator main	2026-09-21 04:07:17 -07:00
Ed Langley	63a5c13af5	Merge pull request #90 from fiddlerwoaroof/wave/io-gp-correctness ... fix(io,gp): CSV export ignores prune_empty; a numeric-valued category is unspellable	2026-09-21 04:07:07 -07:00
Edward Langley	3e3ebec25a	kb+docs: an injection that does not fail names an untested path ... Five facts from Wave J. The one worth reading is the first: check_the_checker is written as "inject the fault and watch it fail", and the case where it PASSES is the more informative result, because it is evidence about the test rather than about the code. On improvise-sihr, forcing ReturnToNormalUnlessMoved alone back to Normal left the characterization test green — which is how the untested path was found. An empty colon line takes ExecuteCommand's is_empty branch and settles through ChangeMode, so the effect settling every colon line that actually runs had no test. There are two tests rather than one because of it. Also records why ChangeMode could hide the bug at all: it was a simple_effect!, which declares a one-field newtype writing one place, so there was nowhere for a rule to live and the exit mode had to be a constant in the keymap and in each commit command. Gotcha 16 already warns the macro's kind argument must be mandatory; this is a second cost of the same shape. And that improvise-9c9o's ticket pre-diagnosed its own fix as a protocol change, and was wrong — the basis is App's own bookkeeping, so it was achieved by moving where the re-base happens. A pre-diagnosis is a hypothesis wearing the authority of a decision. Inventory: three protocol files were absent entirely — help.rs (925), client.rs (255) and resolve.rs, which at 2477 lines and 46 tests is the largest file in the crate. Filed improvise-e5pf: Command::ChangeMode(Normal) is on the wire and reduce_view folds it without the records resolution. Latent — keys.rs emits no ChangeMode and a view-half command never reaches the host's slice, both verified rather than assumed — and closeable only by enriching the projection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 04:06:44 -07:00
Edward Langley	d42253ef38	review(gp,io): the zv8t forcing defeated its own budget, and two narrower fixes ... Adversarial review round on the branch. One real finding, two prose/comment corrections, one fixture widened. F2 (the real one, and introduced by zv8t). `as_categories` forces by column NAME; `encoded_width` counts by column INDEX. A document may legally name a regular category and a raw measure the same thing — the repo already ships that fixture in `a_category_and_a_measure_of_the_same_name_are_refused` — and the measure column was then forced categorical by the name match while the preflight still counted it numeric by index. Measured at 40 records: preflight 41, actual 80. `no_ambiguous_column_names` ran AFTER `preprocess`, so the expansion the preflight exists to prevent was materialised and only then refused; at the default max_records that is a 20k-row one-hot matrix built before the error. That is `doctrine(guard_the_cost_not_the_result)` defeated by an inconsistency this ticket introduced — not the bounded approximation that `doctrine(a_budget_check_is_advisory_or_exact_and_must_say_which)` licenses. The repair is a data dependency rather than a moved line: `as_categories` runs the check and returns `Result<Overrides>`, so the Overrides `preprocess` needs cannot exist until the header has been accepted (`doctrine(an_invariant_is_made_unrepresentable_not_validated)`). The regression guard is therefore the SIGNATURE and the injection is a compile error — swapping the two statements back gives E0425, verified. What it does not cover is stated in the evidence rather than implied: a deliberate deletion of the check from `as_categories` is still possible; the accidental reorder is not. F5. The `encoded_width` comment claimed the preflight "must count it the same way". It does not, quite: `build_categorical` counts MISSING_CAT as a distinct value and the preflight does not, so it under-counts by one per category with an absent coordinate. Bounded by the exact check and pre-existing for genuinely categorical columns — but the comment asserted an identity that does not hold. F4. The evidence for the two spelling tests said that on the unfixed tree the text "could only have been `Expr::Ref(\"Year\")`". There is no text on the unfixed tree at all — `fitted_formula` refuses before emitting. The argument for the assertion survives; the stated reason was a guess about a branch that does not run, and an evidence file is the one place that should be exact. And the iot6 fixture: `prune_empty` prunes BOTH axes and the test asserted rows only. `Feb` is now the empty column, the precondition is a pair on each layout, and the CSV header is asserted to carry `Jan` alone. 1464 passing, up from 1463. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 04:02:02 -07:00
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	2026-09-21 04:01:08 -07:00
Edward Langley	c186909de6	docs: the review's two findings on this branch ... `Command::ChangeMode(AppMode::Normal)` is on the wire and `reduce_view` folds it without the resolution, so a client that emitted it would keep improvise-sihr's behaviour. Not live — `keys.rs` emits no `ChangeMode`, and `apply_command` never folds a view-half command into the host's own slice — and not closeable here: a client holds a `GridCache`, not a `GridLayout`, so records-ness is not a fact it has. Named as a gap, with the shape that would close it. And the improvise-9c9o re-base costs one extra `view_frame` per batch that both switches and moves geometry, which the prose now states rather than leaving for the next person to measure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 03:59:44 -07:00
Edward Langley	d8df9196f5	decide(protocol): keep ColWidths diffed against the mirror, for a different reason (improvise-44ad) ... The move -- diff col_widths against the label-folded cache, as hkt6 already obliged for totals -- is DECLINED, and the two things learned are worth more than the change would have been. THE TICKET'S STATED COST DOES NOT EXIST. It says the move breaks a_client_on_a_socket_is_seeded_and_gets_its_edit_back, and that that assertion is gotcha 33(c)'s standing evidence, so rewriting it inside a change 33(c) guards is the improvise-q0vb / PR #12 shape. Measured by deleting the re-fit from fit_to_labels: three existing unit tests fail and the socket test is not one of them. What it actually asserts is that a reframe CARRIED widths -- a statement about the producer's diff, not about the consumer's re-fit. 33(c) is triply guarded elsewhere. THE REAL REASON IS COUPLING. The widths a label-fold invents are each column's own label width: an approximation the consumer applies until the authority catches up. Diffing the producer's output against it makes what the server sends depend on a fallback rule defined in the client -- invisible today, silent the day a second client implements the fold. The redundant ColWidths per reframe buys out of that, and over-sending is the direction Dirty::of is deliberately sloppy in. Landed anyway: the half the ticket calls "the better test either way". a_client_never_holds_widths_of_a_different_length_from_its_labels asserts the invariant 33(c) is really about, at EVERY step of a fold that widens before its widths arrive and narrows before they shrink -- not merely at the end, which is where a message-count assertion looks. Watched fail at step 0 with the re-fit deleted. The totals/col_widths asymmetry is now stated rather than discovered: totals is fold-relative because hkt6 found a real hole; col_widths has none, the re-fit being a safe fallback rather than a stale value. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 03:59:03 -07:00
Edward Langley	e50473df71	docs(gp): fitted_formula takes Features, not Overrides ... Second self-review finding in the same paragraph, and the same class: §11 said "`crate::dataset` and `fitted_formula` take the caller's `Overrides`". Only the first does. `fitted_formula` takes a `Features`, which `Features::of` read off whatever `Dataset` it was handed — the Overrides are two steps upstream. The conclusion was right and the mechanism was wrong, which is the kind of sentence that survives review because the reader checks the conclusion. Rewritten to name the actual chain. Also verified while checking it, and worth recording because it looked like a drift and is not: the two test call sites still passing `Overrides::default()` to `crate::dataset` describe the SAME dataset production now builds. In `preprocess` the `present.is_empty()` drop happens BEFORE `ov.force` is consulted, so forcing an entirely-missing column categorical changes nothing — `an_empty_category_before_a_measure_does_not_break_the_role_check`'s precondition is as strong as it reads. The other site's fixture has only Label categories, so `category_columns` is 0 and the two Overrides are literally equal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 03:58:43 -07:00
Edward Langley	877d3e193b	refactor(help): a chord is recognised by its modifiers, not its spelling (improvise-thpe) ... `compact` tested whether a label it had just formatted began with "Ctrl-" or "Alt-". No false positive today — the reviewer enumerated every `format_key_label` output — so this is the shape, not the behaviour. `Hint::keys` is now `(KeyCode, KeyModifiers)` and `compact` takes the pairs, so the string is not in scope to parse; `Hint::labels` and the help pages format last through the one spelling in `keymap::key_label`. The regression guard is the signature, as it is for `Checked`. `Keymap::labels_for` — the "throw the modifiers away" step the ticket names — has no caller left and is deleted: a consolidation that keeps its old path keeps the defect available. The rule also got wider and total over the bitset: a chord is any modifier beyond shift. Injected a temporary `KeyModifiers::SUPER` spelled `Super-x` and ran it — the old filter keeps it, `compact` drops it. Binding dump byte-identical across the change (4406 lines). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 03:57:32 -07:00
Edward Langley	c4bf74608f	fix(protocol): a group run is opened by a header, not by a name (improvise-h0iv) ... group_runs coalesced adjacent positions by group NAME, so two runs under two different headers sharing a name read as one and both front ends drew a single span across the boundary. THE TICKET'S PROPOSED FIX DOES NOT WORK, and that is worth more than the fix. h0iv suggested comparing the (cat, name) pair, FrameGroup carrying cat for exactly that reason. But its own reachability case is ONE category -- Month grouped into Q1 nested under Region, giving North/Q1 and South/Q1 adjacent -- so both headers are (Month, Q1), equal as values, and the pair separates nothing. A test pins that so the next reader does not re-propose it. What does separate them is that they are two distinct FrameEntry::Group entries, which groups() already borrows from self.entries: std::ptr::eq on the header is the literal statement of "the same header opened both". THE PRODUCT QUESTION, decided here and reversible in one comparison. Whether two adjacent same-named runs are one span a user wants spanned is a matter of taste, which is why #77 filed rather than fixed. What settled it is not the span but the indicator: the existing wasm-client test merged Month/Q1 (expanded) with Fiscal/Q1 (COLLAPSED) and its own message said the merged span is drawn with the first one's indicator -- a collapsed group rendered as expanded, with no affordance saying otherwise. A span boundary is taste; a wrong collapse glyph is a wrong fact. That test's justification was also spent: it said the rule must be name-only "because that is what src/ui/grid.rs uses", and since improvise-8qlr grid.rs calls group_run_heads. There is no second copy left to agree with, which is the general point -- a consolidation's frozen behaviour reads as decided afterwards, and nothing says it was frozen only for the duration of the move. The agreement fixture gains the case it could not see: two_level_rows minus the ungrouped Sep, so the two Q1 runs abut, end to end through a real GridLayout and both renderers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 03:57:07 -07:00
Edward Langley	e6e39a03ff	review(gp): the zv8t prose claimed a cost it has, and the cost is measurable ... Self-review of 99cfb7f, of the prose rather than the diff. §11 said the forcing cost nothing — "no document that fitted before stops fitting" — reasoning that a tree splitting on a numeric category was refused as an artefact anyway. That is false. The refusal is conditioned on `used_features`, so a fit that simply never split on the numeric category SUCCEEDED, and the wide categories are exactly the ones a fit has least reason to split on. Measured with a throwaway #[ignore] probe on both sides of the fix — 100 records, a 100-item numeric `Year` beside a `Cost` that predicts `Score` on its own: pre-fix: FIT LANDS -> SUM(IF(Cost < 48.0150032043457, 970.3.., 3684.7..)) post-fix: REFUSED -> would encode 101 features from 100 records So the sentence one paragraph below improvise-wd6n's corrected "permanent refusal" was the same failure mode: a claim nothing in the suite could check, green either way. Noting it because the pattern, not the sentence, is the finding — `doctrine(prose_that_calls_something_permanent_forecloses_the_ticket)` has a sibling in prose that calls something free. The trade is still right, for a reason that is NOT "both refuse": the old success was accidental, since `Year` was encoded as a continuous number over 1900–1999 that the fit could only have used by producing a formula it would then be refused for. What is genuinely lost is the case where the user wanted neither — a wide numeric category left alone while the fit uses the measures. Filed as improvise-e8no (P3), with the acceptance criterion that the fix is a budget that can decline A COLUMN rather than the whole table, and an explicit "do not close this by making the forcing conditional on cardinality", which would be inference by another name. `a_wide_numeric_category_now_refuses_a_fit_that_used_to_ignore_it` pins both halves. Its second half is what keeps it from being a bare characterization: it marks `Year` a Label and asserts the fit then lands on `Cost`, because advice in an error message is a promise and this is the half worth asserting. Note for the record: 99cfb7f's own message carries the false claim and cannot be corrected. Per design-principles §6, the correction lives in context/ and this says so — the message is superseded by §11 and by the test-evidence entry. 1463 passing, up from 1462. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 03:56:47 -07:00
Edward Langley	8e9842b89f	fix(cursor): the frame basis follows active_view, not the next geometry effect (improvise-9c9o) ... PR #78 round 2's `was == self.view_name()` guard fixed a spurious invalidation and introduced its mirror: the first geometry effect after a mid-batch `SwitchView` skipped the check entirely, because there was no basis for the new view yet, so the code re-based and skipped. Under-reporting, which gotcha 35 says is the expensive side — a stale cursor has nothing in the stream to correct it. The re-base now happens at the switch, in the loop, as soon as the name moves, so by the time the geometry arm runs the name and the basis agree by construction and the guard is gone as unreachable. `SwitchView` stays `ViewOnly`: the basis is `App`'s own bookkeeping and none of it is on the wire, so this is not the protocol change the ticket feared. Also deletes a fixture line in the sibling test that was a complete no-op — `views["Second"].clone_into(&mut views["Default"].clone())` clones into a temporary that is dropped — and replaces it with the assertion it was pretending to be. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH	2026-09-21 03:53:37 -07:00