diff --git a/.beads/issues.jsonl b/.beads/issues.jsonl index 423aa70c..f83081cc 100644 --- a/.beads/issues.jsonl +++ b/.beads/issues.jsonl @@ -46,10 +46,10 @@ {"_type":"issue","id":"improvise-9c9o","title":"A geometry effect right after a mid-batch SwitchView skips the cursor check entirely","description":"Introduced by PR #78 round 2. PASSES on pre-PR main (0947fd0) and on round-1 head (bcccfde); FAILS at d9cb16c.\n\nRound 1 compared frame hashes across a view change, which threw a cursor away spuriously. Round 2 added a 'was == self.view_name()' guard, which fixes that — but it also exempts the FIRST geometry effect after a mid-batch SwitchView from the check altogether, because there is no basis for the new view yet, so the code re-bases and skips.\n\n## Steps to Reproduce\n app.apply_effects(vec![SwitchView(\"Second\"), TransposeAxes]);\nBoth preconditions hold: the switch lands, and row_count() really changes. Yet cursor_epoch does not move and the cursor stays at (2,0) in a transposed grid.\n\n## Acceptance Criteria\n- The probe above lands as a guard.\n- The basis re-bases to the new view AT THE SWITCH rather than at the next geometry effect — i.e. the basis follows active_view changes, which SwitchView being ViewOnly currently prevents. Not a one-liner.\n\nWHY THE DIRECTION MATTERS: round 1 threw away a cursor that should have been kept; round 2 keeps one that should have been thrown away. Gotcha 35's asymmetry says under-reporting is the expensive side — a stale cursor has nothing in the stream to correct it, where an extra invalidation costs one redundant projection.\n\nReachability is the same narrow class as the bug it replaced: a . -chained : line, or any future Binding::Sequence composing the two. DrillIntoCell and EnterRecordsMode hit the pattern constantly but their target view is freshly created, so it is benign there. Merged on that basis rather than blocked.","status":"open","priority":2,"issue_type":"bug","owner":"el-github@elangley.org","created_at":"2026-09-21T05:15:07Z","created_by":"Edward Langley","updated_at":"2026-09-21T05:15:07Z","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"improvise-iot6","title":"render_csv builds GridLayout::new, so CSV export carries rows the view prunes","description":"FOURTH instance of the GridLayout::new versus with_frozen_records trap, and the first one in production that a user can see.\n\ncrates/improvise-io/src/persistence/export.rs:33 builds GridLayout::new(\u0026workbook.model, view), which does NOT apply prune_empty. So :export in the TUI and the CLI export path both write rows the grid does not draw.\n\n## Steps to Reproduce\nFixture: Region {North, Service, South}, Revenue set for North and South only, prune_empty = true. The view draws 2 rows (assert this as the precondition).\n\nrender_csv output:\n Region,Revenue\n North,1\n Service, \u003c- pruned on screen, exported anyway\n South,1\n\n## Acceptance Criteria\n- render_csv honours the view's prune_empty, via the same path grid_in now uses.\n- A test whose precondition asserts the view really prunes (a row that is actually empty, pruning left ON in the view). A fixture that turns pruning off, or has nothing to prune, is vacuous — that exact vacuity was demonstrated while writing the improvise-t6db property.\n- None of export.rs's three existing tests mentions pruning; that is why this shipped.\n\nPre-existing, not a regression from PR #78. Found by the adversarial review of #78 while enumerating every GridLayout::new call site: three are outside test modules, two of those are test helpers, and this is the third.","status":"closed","priority":2,"issue_type":"bug","owner":"el-github@elangley.org","created_at":"2026-09-21T05:14:59Z","created_by":"Edward Langley","updated_at":"2026-09-21T11:09:27Z","closed_at":"2026-09-21T11:09:27Z","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"improvise-zv8t","title":"fit_measure never tells preprocess which columns are categories, so a numeric- or date-valued category is unspellable","description":"fit_measure never tells preprocess which columns are categories, so a numeric- or date-valued category is unspellable\n\nFound reviewing improvise-wd6n's branch, and it was very nearly foreclosed in\nprose: the first draft of context/code/gp.md §11 called Artefact::NumericCategory\na permanent refusal that \"no vocabulary closes\". That is wrong, and this is the\nticket the prose would have prevented.\n\ncrates/improvise-gp/src/fit.rs passes `\u0026Overrides::default()` to `preprocess`,\nso it never tells preprocess which header columns are document categories --\nalthough the very next line does tell `Features::of`. `preprocess` then infers\nNumeric -\u003e Date -\u003e Categorical from the cell values, so:\n\n * a regular category whose items all parse as numbers (a `Year` category with\n items 2024/2025) becomes ColumnKind::Numeric, and its feature is refused as\n Artefact::NumericCategory\n * a regular category whose items are all ISO dates becomes ColumnKind::Date,\n and its features are refused as Artefact::DatePart\n\nIn both cases `Year IS 2024` is EXACTLY spellable, and the fit is refused\nanyway. Unlike the two families improvise-wd6n declines, this needs no\nformula-language change at all: `Overrides::force[col] = Forced::Categorical`\nalready exists (gp-tree/src/data.rs, consumed in preprocess's `match ov.force`)\nand is the knob. fit.rs knows which columns are categories -- it computes\n`category_columns` on the line above.\n\nThe refusal text for NumericCategory argues about `Expr::Ref` resolving to a\n_Measure item of the same name, which was never the right spelling for a\ncategory column. It also sits awkwardly beside this crate's own rule (gotcha\n37(d), gp.md §11: the check asks the document, never the name's shape) and\nbeside a_coordinate_is_spelled_as_one_even_when_an_item_shares_the_one_hots_name,\nwhich exists precisely to show the two spellings cannot collide.\n\nNeither NumericCategory nor AbsentCoordinate has a test today; they were\ninvented by the wd6n branch and only three of the five families are reached by\nits fixture.\n\n## Acceptance\n\nA fit over a view with a numeric-valued or date-valued regular category emits a\n`Cat IS Item` coordinate for it rather than being refused, demonstrated by a\ntest that fails before the fix. Whatever forcing is applied must not change how\na MEASURE column is inferred -- a numeric measure is still Numeric. If forcing\nturns out to be wrong for some reason not visible from here, say so in\ncontext/code/gp.md and correct the \"permanent refusal\" paragraph either way.\n","status":"closed","priority":2,"issue_type":"bug","owner":"el-github@elangley.org","created_at":"2026-09-21T05:05:31Z","created_by":"Edward Langley","updated_at":"2026-09-21T11:09:28Z","closed_at":"2026-09-21T11:09:28Z","dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"improvise-t6db","title":"grid_in ignored prune_empty for a non-active view, hiding a staleness bug","description":"FIXED in PR #78; filed because the FINDING is worth more than the fix and because the shape has now recurred three times.\n\nproject::grid_in built a layout for a view the host is NOT on with GridLayout::new, which does not apply prune_empty. So a client on another view was projected rows its own view config prunes. Measured on a two-view fixture where both views prune and one row is empty:\n\n Second via grid_in (GridLayout::new) 3 rows: North, Service, South\n Second via with_frozen_records (pruned) 2 rows: North, South\n\nOne line. It broke no other test, which is itself evidence nothing depended on the unpruned behaviour.\n\nWHY IT MATTERS MORE THAN ITS SIZE -- IT WAS A BUG HIDING A BUG. The adversarial review of #78 found a P1: after improvise-mcq4 narrowed the cursor fingerprint to the active view, a CellData write that prunes a row out of ANOTHER client's view stopped invalidating that client. Writing the regression test for it FAILED AT THE PRECONDITION -- the bystander's projected grid never lost a row, because grid_in never pruned it. The staleness was real and logically reachable, and completely unobservable through the projection. Fixing grid_in is what made the P1 demonstrable, and the P1 test is what found grid_in.\n\nTHIRD INSTANCE OF ONE TRAP: GridLayout::new does not prune; GridLayout::with_frozen_records does. Previously bitten by App::view_frame (improvise-ni6k, caught by a failing test) and by the reshape benchmark's geometry fixture (improvise-mcq4, caught only by measuring). A constructor whose name suggests it is the general one, and which silently omits a view setting, is worth a look on its own terms -- either new should prune, or the names should say which is which.\n\nSTILL OPEN AROUND IT, not fixed here: nothing asserts that a client on a non-active view is projected a grid matching ITS OWN view config in general. prune_empty is one setting; the rest is a guarantee by inspection rather than by test.\n","status":"open","priority":2,"issue_type":"bug","owner":"el-github@elangley.org","created_at":"2026-09-21T05:00:08Z","created_by":"claude-agent-protocol","updated_at":"2026-09-21T05:00:08Z","dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"improvise-t6db","title":"grid_in ignored prune_empty for a non-active view, hiding a staleness bug","description":"FIXED in PR #78; filed because the FINDING is worth more than the fix and because the shape has now recurred three times.\n\nproject::grid_in built a layout for a view the host is NOT on with GridLayout::new, which does not apply prune_empty. So a client on another view was projected rows its own view config prunes. Measured on a two-view fixture where both views prune and one row is empty:\n\n Second via grid_in (GridLayout::new) 3 rows: North, Service, South\n Second via with_frozen_records (pruned) 2 rows: North, South\n\nOne line. It broke no other test, which is itself evidence nothing depended on the unpruned behaviour.\n\nWHY IT MATTERS MORE THAN ITS SIZE -- IT WAS A BUG HIDING A BUG. The adversarial review of #78 found a P1: after improvise-mcq4 narrowed the cursor fingerprint to the active view, a CellData write that prunes a row out of ANOTHER client's view stopped invalidating that client. Writing the regression test for it FAILED AT THE PRECONDITION -- the bystander's projected grid never lost a row, because grid_in never pruned it. The staleness was real and logically reachable, and completely unobservable through the projection. Fixing grid_in is what made the P1 demonstrable, and the P1 test is what found grid_in.\n\nTHIRD INSTANCE OF ONE TRAP: GridLayout::new does not prune; GridLayout::with_frozen_records does. Previously bitten by App::view_frame (improvise-ni6k, caught by a failing test) and by the reshape benchmark's geometry fixture (improvise-mcq4, caught only by measuring). A constructor whose name suggests it is the general one, and which silently omits a view setting, is worth a look on its own terms -- either new should prune, or the names should say which is which.\n\nSTILL OPEN AROUND IT, not fixed here: nothing asserts that a client on a non-active view is projected a grid matching ITS OWN view config in general. prune_empty is one setting; the rest is a guarantee by inspection rather than by test.\n","status":"closed","priority":2,"issue_type":"bug","owner":"el-github@elangley.org","created_at":"2026-09-21T05:00:08Z","created_by":"claude-agent-protocol","updated_at":"2026-09-21T12:39:35Z","closed_at":"2026-09-21T12:39:35Z","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"improvise-ijql","title":"check-comment-runs.py --check-evidence silently skips the comment-run cap it is named for","description":"UPDATE 2026-09-21, after PR #83 merged: the DOCS half is done — the code-review skill's gate list and every wave brief now run both spellings. The CODE half is NOT done and this ticket is about that: scripts/check-comment-runs.py:1010 still early-returns on args.check_evidence, so the flag SELECTS rather than adds and the flagged run still says nothing about the cap it skipped.\n\nThree agents hit this independently in one campaign, which is the argument for fixing it rather than documenting around it. The reviewer's position was that CI runs all three forms (ci.yml L144/146/148) so this is documentation only; the counter is that the flagged form is the EXPENSIVE one (it needs a compiled workspace), which makes it the one a human runs alone, and a gate whose skip is silent is the defect class this repo keeps finding.\n\nTwo shapes: make --check-evidence additive (run the cap too), or make the flagged run print one line saying the cap did not run. The second is cheaper and matches what PR #83 did for --rust-tests, where the fix was to make the success line name its source.","status":"closed","priority":2,"issue_type":"bug","owner":"el-github@elangley.org","created_at":"2026-09-21T04:31:48Z","created_by":"Edward Langley","updated_at":"2026-09-21T05:23:49Z","closed_at":"2026-09-21T05:23:49Z","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"improvise-mcq4","title":"The frame fingerprint is a 4x regression on the held-key path improvise-748f just optimized","description":"MEASURED, not suspected. Found while merging PR #78 (improvise-ni6k) with PR #76 (improvise-748f), which landed first.\n\nimprovise-748f split the geometry fingerprint so that a batch which writes NO cell -- the held-key path, [/] and axis cycle and hide/show -- stays O(config) instead of walking 18,000 cells. improvise-ni6k's App::view_frame builds a GridLayout per view via with_frozen_records, and with_frozen_records applies prune_empty, and prune_empty WALKS CELLS. So for a view with prune_empty on, ni6k puts the O(cells) walk straight back onto the path 748f had just taken it off.\n\nMEASURED on the merged tree, benches/reshape_observation.rs, one CycleAxis batch, prune_empty ON, frame fingerprint disabled vs enabled:\n\n 400 cells 93.6us -\u003e 283.2us 3.0x\n 9,600 cells 1.97ms -\u003e 7.34ms 3.7x\n 18,000 cells 3.04ms -\u003e 12.41ms 4.1x\n 90,000 cells 28.5ms -\u003e 91.7ms 3.2x\n\nThe 16.7ms frame budget is crossed at 18,000 cells, where the pre-fingerprint tree sat comfortably inside it at 3.0ms.\n\nWHY 3-4x AND NOT 2x: view_frames builds every view's layout, before AND after each model effect. The bench fixture has two views, so that is four pruned layout builds against the one rebuild_layout already does.\n\nWHY THE COST WAS NOT SEEN EARLIER: ni6k's own benchmark measures a SetCell batch, where 748f's document_content walk dominates and the fingerprint reads as +2 to +5 percent. And the geometry case, when first added, used the dense_app fixture as-is -- prune_empty OFF -- where with_frozen_records does not walk cells at all and a CycleAxis batch reads 17-176us. THAT FIXTURE MEASURES THE CHEAP HALF AND CALLS IT THE COST. It is the same non-discriminating-fixture failure this repo names everywhere; the numbers above come from turning prune_empty on, and the benchmark now does.\n\nOPTIONS, none taken:\n- Fingerprint only the ACTIVE view for the cursor and fall back to the cheaper config hash (last_reshaped_views) for the client audience. Over-resets bystanders on other views, which is the safe direction per gotcha 35, and removes the per-view multiplier. Probably the right answer.\n- Memoise the pruned layout across the before/after pair.\n- Accept, on the argument that prune_empty plus a large document plus a held key is rare. That argument needs the user, not an agent.\n\nDO NOT close this by re-running the benchmark with prune_empty off.","notes":"FIXED to 1.9x, not closed: one pruned build per model effect remains.\n\nThe coordinator ruled this must be fixed before PR #78 lands, and was right -- a cost measured to four significant figures is a decision to ship, not a thing to file.\n\nWHAT WORKED, AND IT IS NOT WHAT THE TICKET PROPOSED. The ticket's first option (and my own suggestion) was to fingerprint only the ACTIVE view and fall back to the config hash for the client audience. Implemented and measured: it bought NOTHING -- 12.41ms -\u003e 12.56ms at 18,000 cells. The bench's second view is one created fresh with everything on Axis::None, so building it costs nothing. THE PER-VIEW MULTIPLIER WAS NEVER THE COST; two EXPENSIVE builds were. That is only known because it was measured after implementing rather than reasoned about before.\n\nWHAT ACTUALLY WORKED: the opening basis is free. At batch start App::layout already IS the active view's pruned layout, so opening_view_frame() hashes it instead of building a second one. frame_of() is the shared hash and view_frame() builds then calls it.\n\n one CycleAxis batch, prune_empty ON:\n before after fingerprint off\n 400 cells 283.2us 167.0us 93.6us\n 9,600 cells 7.34ms 3.879ms 1.967ms\n 18,000 cells 12.41ms 5.752ms 3.043ms\n 90,000 cells 91.69ms 56.54ms 28.52ms\n\n4.1x -\u003e 1.9x at the canonical size, back inside the 16.7ms frame budget.\n\nTwo guards on the free basis: it is gated on drill_state.is_none(), because a drill freezes the record list and the held layout is then not what view_frame would build; and it is opening-use only, because App::layout is stale mid-batch (gotcha 23).\n\nTHE NARROWING WAS KEPT anyway, because it is the honest shape -- the cursor's question is about one view. The audience half is now the config diff observe_reshape already computes free. It over-reports (safe direction) and is read only behind the epoch gate in adopt_cursor_epoch, so its imprecision cannot reach a batch that reframed nothing. a_reshape_of_one_view_leaves_a_client_on_another_view_alone still passes, so it has not widened into improvise-nynk's negative arm.\n\nBOTH SUBSUMPTION PROOFS RE-RUN AFTER THE NARROWING, since narrowing is exactly what could quietly stop covering something: #76's own two arms still pass with no per-effect reset anywhere; AddFormula/RemoveFormula and SortData are still covered; disabling the narrowed observation fails ten tests.\n\nWHAT REMAINS, AND WHY THIS STAYS OPEN: one pruned layout build per model effect. The LAST one duplicates work rebuild_layout does immediately afterwards, and eliding it means moving the check after the loop -- which is the position that clobbers a deliberate cursor placement in the same batch (the three records-mode tests that caught it the first time). A restructure that knows which effect is last, including on abort, is possible and was not attempted at this hour.\n\nThe benchmark now carries the held-key case with prune_empty ON and a comment saying not to turn it off, because that fixture is the only reason any of this was visible.\nTHE NARROWING BOUGHT ITS SPEED PARTLY WITH CORRECTNESS, and the adversarial review of PR #78 caught it. Correcting my own note above.\n\nI narrowed the frame fingerprint from every view to the active one, measured that it moved 12.41ms to 12.56ms, kept it anyway \"as the honest shape\", and reported that it bought nothing. It bought a P1: a CellData write that prunes a row out of ANOTHER client's view became invisible. view_shapes is config-only -- it hashes the prune_empty FLAG, not which rows survive -- so last_reshaped_views is empty, the one-view frame covers only the host's, and the bystander is never sent ResetCursor while its grid loses a row. Gotcha 24's permanently-stale case.\n\nTHE LESSON I WANT ON THIS TICKET: a measurement showing a change is free is not evidence that it is correct. I answered \"what does this cost\" and reported the answer as though it had also answered \"is this right\". The two questions were asked in the wrong order and only one of them was asked at all.\n\nTHE REPAIR, and the cost trade is accepted deliberately rather than hidden:\n- LayoutGeometry arm keeps the cheap one-view frame. view_mut() writes only the active view, and the structural cases (AddCategory, RemoveCategory, Add/RemoveItem) are already covered by view_shapes.\n- CellData arm restores the all-views frame, because a cell write can prune a row out of any view.\n\n Measured, my own numbers, one batch, prune_empty ON:\n held-key (CycleAxis) 18k cells 5.69ms -- UNCHANGED, 0%\n SetCell 18k cells 10.66ms -\u003e 11.26ms +5.7%\n\nThe +5.7% scales LINEARLY IN VIEW COUNT on the CellData arm. A ten-view document pays it ten times. That is the trade and it is accepted, not hidden.\n\nTHE ABSOLUTE, stated plainly because it should not be discovered later: even after the mcq4 fix, this PR roughly DOUBLES held-key latency on large pruned documents -- 3.04ms to 5.69ms at 18,000 cells -- and 56ms at 90,000 cells is 3.4 frames. The 16.7ms budget is respected at the canonical size and is not respected at the pathological one, which was also true before at 28.5ms.\n\nA SECOND BUG FOUND WHILE WRITING THE REGRESSION TEST, filed as improvise-t6db: the test's precondition failed because grid_in built a non-active view with GridLayout::new, which does not prune. The bystander's projected grid never lost a row, so the P1 was unobservable -- a bug hiding a bug.\n\nSTILL OPEN, unchanged: one pruned build per model effect. The last duplicates rebuild_layout and can only be elided by moving the check after the loop, which clobbers a deliberate cursor placement in the same batch. With the arms now split, moving the LayoutGeometry arm alone after the loop fails 2 tests; the reviewer measured 4 against the unified arm before the split.\n","status":"open","priority":2,"issue_type":"bug","owner":"el-github@elangley.org","created_at":"2026-09-21T04:20:08Z","created_by":"claude-agent-protocol","updated_at":"2026-09-21T05:00:28Z","dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"improvise-5ubs","title":"A cyclic-successor all() can silently omit a variant while its doc says it cannot","description":"Found by the adversarial review of PR #78, which probed it rather than reading the doc comment.\n\nAppMode::all() and Rejection::all() walk a total, cyclic after(). The claim in both doc comments is that a new variant cannot be omitted, because after() has no None arm so the only way to answer for a new variant is to NAME ANOTHER ONE.\n\nThat is half true and the half it gets wrong is the half that matters. ANSWERING after() FOR a new variant is forced by the compiler. POINTING AN EXISTING ARM AT IT IS NOT. A probe variant whose after() arm returns some existing variant, with nothing in the cycle pointing back to it, is simply never walked: all().len() came back 17, not 18, with the suite green.\n\nSo the consolidation replaced five grep-visible lists with one derivation whose totality is asserted in prose and checked by nothing. THE REVIEWER'S META-POINT IS THE REAL TICKET: five disagreeing AppMode lists were visible to grep; all() under-enumerating is visible to nothing. A consolidation owes a test that the surviving derivation is TOTAL, not merely that it agrees with what it replaced.\n\nWHAT IS STILL BETTER THAN BEFORE, so this is not a revert: the compiler now forces a human to look, and two of the five lists it replaced were ALREADY WRONG (both missing SavePrompt). This is a gap in a guarantee, not a regression.\n\nOPTIONS, roughly in order of cost:\n- A test that walks the cycle and asserts every variant is visited, driven by something independent of after() -- which needs an independent enumeration, and that is the problem restated.\n- A derive macro or a dependency (strum). Rejected during PR #78 only because adding a workspace dep mid-wave collides in Cargo.toml; on its own it is the honest answer and cheap.\n- Accept and document the residual precisely, replacing both doc comments, which currently overclaim.\n\nWHATEVER IS CHOSEN, FIX THE DOC COMMENTS FIRST: they presently say the omission is impossible, and a reader checking this property will believe them. An overclaiming comment is worse than none, because it stops the next person looking.","notes":"A FOURTH INSTANCE, inside the PR that found this ticket. The adversarial review of PR #78 added a fourteenth `pub struct` declaring `EffectKind::Model(ModelChange::LayoutGeometry)`, pointed at by nothing, and the whole suite stayed green.\n\nThe site is layout_geometry_effects_actually_change_document_config in ui/effect.rs. Its comment said \"a fourteenth declaring LayoutGeometry must come here and say so\". That is false for exactly the reason this ticket names: adding is compiler-forced only where an EXISTING arm must point at the new thing, and here nothing must. `assert_eq!(cases.len(), 13)` is a count, not a totality check -- it catches a case being deleted from the list, never one being absent from it.\n\nCorrected in #78 to say what it is rather than what it is not; the mechanism is unchanged and the cursor consolidation does not depend on it. Filed here rather than fixed because the honest fix is whatever 5ubs settles on, and a fourth hand-rolled attempt in a fourth place is the disease.\n\nWHAT MAKES THIS ONE WORTH RECORDING: it is the same defect as AppMode::all(), in the PR whose own consolidation work surfaced AppMode::all(). A list that is checked for LENGTH reads exactly like a list that is checked for COMPLETENESS, and both are written as an assert_eq over a number.\n","status":"open","priority":2,"issue_type":"bug","owner":"el-github@elangley.org","created_at":"2026-09-21T04:01:27Z","created_by":"claude-agent-protocol","updated_at":"2026-09-21T05:00:37Z","dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"improvise-5ubs","title":"A cyclic-successor all() can silently omit a variant while its doc says it cannot","description":"Found by the adversarial review of PR #78, which probed it rather than reading the doc comment.\n\nAppMode::all() and Rejection::all() walk a total, cyclic after(). The claim in both doc comments is that a new variant cannot be omitted, because after() has no None arm so the only way to answer for a new variant is to NAME ANOTHER ONE.\n\nThat is half true and the half it gets wrong is the half that matters. ANSWERING after() FOR a new variant is forced by the compiler. POINTING AN EXISTING ARM AT IT IS NOT. A probe variant whose after() arm returns some existing variant, with nothing in the cycle pointing back to it, is simply never walked: all().len() came back 17, not 18, with the suite green.\n\nSo the consolidation replaced five grep-visible lists with one derivation whose totality is asserted in prose and checked by nothing. THE REVIEWER'S META-POINT IS THE REAL TICKET: five disagreeing AppMode lists were visible to grep; all() under-enumerating is visible to nothing. A consolidation owes a test that the surviving derivation is TOTAL, not merely that it agrees with what it replaced.\n\nWHAT IS STILL BETTER THAN BEFORE, so this is not a revert: the compiler now forces a human to look, and two of the five lists it replaced were ALREADY WRONG (both missing SavePrompt). This is a gap in a guarantee, not a regression.\n\nOPTIONS, roughly in order of cost:\n- A test that walks the cycle and asserts every variant is visited, driven by something independent of after() -- which needs an independent enumeration, and that is the problem restated.\n- A derive macro or a dependency (strum). Rejected during PR #78 only because adding a workspace dep mid-wave collides in Cargo.toml; on its own it is the honest answer and cheap.\n- Accept and document the residual precisely, replacing both doc comments, which currently overclaim.\n\nWHATEVER IS CHOSEN, FIX THE DOC COMMENTS FIRST: they presently say the omission is impossible, and a reader checking this property will believe them. An overclaiming comment is worse than none, because it stops the next person looking.","notes":"A FOURTH INSTANCE, inside the PR that found this ticket. The adversarial review of PR #78 added a fourteenth `pub struct` declaring `EffectKind::Model(ModelChange::LayoutGeometry)`, pointed at by nothing, and the whole suite stayed green.\n\nThe site is layout_geometry_effects_actually_change_document_config in ui/effect.rs. Its comment said \"a fourteenth declaring LayoutGeometry must come here and say so\". That is false for exactly the reason this ticket names: adding is compiler-forced only where an EXISTING arm must point at the new thing, and here nothing must. `assert_eq!(cases.len(), 13)` is a count, not a totality check -- it catches a case being deleted from the list, never one being absent from it.\n\nCorrected in #78 to say what it is rather than what it is not; the mechanism is unchanged and the cursor consolidation does not depend on it. Filed here rather than fixed because the honest fix is whatever 5ubs settles on, and a fourth hand-rolled attempt in a fourth place is the disease.\n\nWHAT MAKES THIS ONE WORTH RECORDING: it is the same defect as AppMode::all(), in the PR whose own consolidation work surfaced AppMode::all(). A list that is checked for LENGTH reads exactly like a list that is checked for COMPLETENESS, and both are written as an assert_eq over a number.\n","status":"closed","priority":2,"issue_type":"bug","owner":"el-github@elangley.org","created_at":"2026-09-21T04:01:27Z","created_by":"claude-agent-protocol","updated_at":"2026-09-21T12:39:36Z","closed_at":"2026-09-21T12:39:36Z","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"improvise-349m","title":"format_md/parse_md is not idempotent when a .improv file has two Category sections with the same name","description":"format -\u003e parse -\u003e format differs, the two sides differing only in the order of '\u003e' group lines. DETERMINISTIC -- 8 failures in 8 runs on unmodified main (00f7678) -- so it is NOT gotcha 18's HashSet shape.\n\nCause: the file carries two '## Category: ||' sections with the SAME name. The second merges into the first and its group lines are appended in a position the second parse does not reproduce.\n\nReproducer, from the proptest's own minimal input:\n\n let file = \"v2025-04-09\\n# |\\\\a|\\n## View: a\\n_-: row\\ncollapsed: ||/A--\\n\\\n ## Category: ||\\n- |\\\\a\\\\a\\\\a|,|\\\\a\\\\a|,|\\\\a|,|\\\\a|\\n\u003e _---[|\\\\a|]\\n\u003e _[|\\\\a|]\\n\\\n ## Category: ||\\n- ||[||]\\n\u003e ||[||]\\n\";\n let m1 = parse_md(file).unwrap();\n let printed = format_md(\u0026m1);\n assert_eq!(printed, format_md(\u0026parse_md(\u0026printed).unwrap())); // fails\n\nWHY IT LOOKS FLAKY AND IS NOT: persistence::parse::grammar_prop_tests::generated_file_roundtrips goes red only when the grammar-walking generator happens to emit a duplicate section header, which is rare. It was observed once during PR #70's CI, the rerun passed, and the honest move was to take proptest's minimal input and try it deterministically rather than rerun and move on. That is what turned a 'flake' into a filed bug.\n\nFound by the improvise-pdvm/75ld agent on PR #70; not caused by that PR. bd search for idempotent, roundtrip, duplicate category, format_md and group order returned nothing, so it was not already filed.\n\nWorth deciding as part of the fix: is a duplicate '## Category:' header legal input at all? If it is not, the parser should refuse it and the generator should stop emitting it; if it is, the merge must produce a canonical group order. The round-trip property cannot hold without one of those answers.","status":"closed","priority":2,"issue_type":"bug","owner":"el-github@elangley.org","created_at":"2026-09-20T23:42:59Z","created_by":"orchestrator","updated_at":"2026-09-21T04:42:54Z","closed_at":"2026-09-21T04:42:54Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"improvise-v5pc","title":"restore the browser assertion that ? opens help","description":"36ade35 moved the shell suite to node and deleted harness/tests/widgets.spec.js, taking the Playwright test '? opens help, and its rows are the shared ones' with it. Nothing replaced it: no node test and no spec asserts that the help overlay is drawn, that its active tab is the shared module's page title, or that l pages it. The evidence entry in context/test-evidence/clients.md survived the deletion and went on promising the coverage for a whole release -- the improvise-5ray cost, now caught by --check-evidence, and the heading is demoted to prose rather than deleted so its two injections (dropping the paintHelp call; removing the mode != Help guard in help_rows) survive for whoever restores it. improvise-ktv1's acceptance -- a browser shows help -- is currently unasserted end to end.","status":"closed","priority":2,"issue_type":"bug","owner":"el-github@elangley.org","created_at":"2026-09-20T23:25:28Z","created_by":"claude-agent-scripts","updated_at":"2026-09-21T00:06:02Z","closed_at":"2026-09-21T00:06:02Z","close_reason":"Duplicate of improvise-78v6, already fixed and merged in PR #72. harness/tests/help.spec.js on main carries test(\"? opens help, and its rows carry the keys and the words, not empty frames\") plus a second test that reads an advertised key out of the DOM, presses it, and asserts the selection moved.\n\nWorth noting HOW this duplicate happened, because the filing agent did nothing wrong: it searched bd before filing, as AGENTS.md requires, and found nothing -- because the fix was in a sibling agent's UNMERGED PR at the time. A parallel wave makes in-flight work invisible to bd search. The cheap addition to the wave brief is to check open PRs, not only the tracker, before filing something a sibling unit might own.","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"improvise-xfsb","title":"Agents should post to GitHub under their own identity, not the owner's","description":"Today every agent comment, review and PR goes out as @fiddlerwoaroof, because gh uses his credentials. The interim is a convention -- open each comment with a line saying a model wrote it (doctrine(an_agent_posting_to_github_says_so), AGENTS.md) -- and a convention is something an agent can forget. A separate identity makes attribution structural instead.\n\nTwo routes:\n\n1. GitHub App installed on the repo. Agents mint an installation token and export GH_TOKEN. Comments render as \u003cname\u003e[bot], the same shape as copilot-pull-request-reviewer[bot]. Scoped permissions, no seat consumed, revocable independently of the owner's account, and the bot suffix is not spoofable by a human account.\n\n2. Machine user with its own PAT as GH_TOKEN. Simpler to set up, consumes a seat on some plans, and is indistinguishable from a person at a glance -- which is most of what we are trying to fix.\n\nPrefer 1. Needs the user: create the App, grant contents+pull_requests+issues write, install it, and put the app id and private key where agents can reach them.\n\nWorth checking while doing it: whether the bot can be requested as a PR reviewer, which would let review agents post as reviews rather than as comments.\n\nOnce it lands, the attribution line becomes belt-and-braces rather than the mechanism, and the doctrine should be amended to say so rather than deleted -- a reader seeing an unattributed comment from the owner's account still needs to know it might be an agent.","status":"open","priority":2,"issue_type":"task","owner":"el-github@elangley.org","created_at":"2026-09-20T23:04:19Z","created_by":"orchestrator","updated_at":"2026-09-20T23:04:19Z","dependency_count":0,"dependent_count":0,"comment_count":0} @@ -197,6 +197,7 @@ {"_type":"issue","id":"improvise-pby","title":"2.2 Write the README","description":"Replace existing README.md with new one under 250 lines. Sections in order: Title, one-sentence pitch, inline demo GIF, why this exists (Lotus Improv reference), quick start, key bindings, installation (Nix/crates.io/prebuilt - no Homebrew), codebase overview, expectations disclaimer, license. No badges, TOC, or contributing guide.","status":"closed","priority":2,"issue_type":"task","assignee":"Edward Langley","owner":"el-github@elangley.org","created_at":"2026-04-09T04:07:32Z","created_by":"Edward Langley","updated_at":"2026-07-30T23:06:31Z","closed_at":"2026-04-09T07:02:52Z","close_reason":"README written with all 10 required sections, committed. License updated to Apache-2.0 per user request.","dependency_count":0,"dependent_count":2,"comment_count":0} {"_type":"issue","id":"improvise-ihv","title":"2.1 Add Nix tooling for asciinema, VHS, and cargo-dist","description":"Modify flake.nix to add pkgs.asciinema and pkgs.vhs to dev shell nativeBuildInputs. Add cargo-dist if packaged in nixpkgs. Add runtime deps for VHS (ttyd, ffmpeg) if needed. Verify tools work via nix develop --command. Add nix run app or shell script for demo recording workflow.","status":"closed","priority":2,"issue_type":"task","assignee":"Edward Langley","owner":"el-github@elangley.org","created_at":"2026-04-09T04:07:30Z","created_by":"Edward Langley","updated_at":"2026-07-30T23:06:31Z","closed_at":"2026-04-09T07:30:19Z","close_reason":"asciinema, vhs, cargo-dist added to flake.nix dev shell and verified. cargo-dist wrapper enables 'cargo dist' subcommand. scripts/record-demo.sh added.","dependency_count":0,"dependent_count":3,"comment_count":0} {"_type":"issue","id":"improvise-yk4","title":"Phase 2: README and demo artifacts","description":"The main launch work. README is 80% of the launch, demo artifacts are 20%. Covers Nix tooling for asciinema/VHS, writing the README, creating demo GIF and asciinema casts, and verifying all artifacts.","status":"closed","priority":2,"issue_type":"feature","assignee":"spot","owner":"el-github@elangley.org","created_at":"2026-04-09T04:05:49Z","created_by":"Edward Langley","updated_at":"2026-04-14T07:47:43Z","closed_at":"2026-04-14T07:47:43Z","close_reason":"All 5 subtasks closed; artifacts verified present and tracked in git (README.md, docs/demo.gif, docs/demo.tape, docs/casts/*.cast).","dependencies":[{"issue_id":"improvise-yk4","depends_on_id":"improvise-abz","type":"blocks","created_at":"2026-04-08T23:37:37Z","created_by":"Edward Langley","metadata":"{}"},{"issue_id":"improvise-yk4","depends_on_id":"improvise-d4w","type":"blocks","created_at":"2026-04-08T23:37:36Z","created_by":"Edward Langley","metadata":"{}"},{"issue_id":"improvise-yk4","depends_on_id":"improvise-ihv","type":"blocks","created_at":"2026-04-08T23:37:34Z","created_by":"Edward Langley","metadata":"{}"},{"issue_id":"improvise-yk4","depends_on_id":"improvise-odx","type":"blocks","created_at":"2026-04-08T23:37:35Z","created_by":"Edward Langley","metadata":"{}"},{"issue_id":"improvise-yk4","depends_on_id":"improvise-pby","type":"blocks","created_at":"2026-04-08T23:37:36Z","created_by":"Edward Langley","metadata":"{}"}],"dependency_count":5,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"improvise-hsoe","title":"check-evidence cannot see a duplicate heading, and three exist","description":"Found by the Wave I agent while merging PR #92, which had accumulated TWO entries headed `### a_window_narrows_the_cells_and_neither_the_totals_nor_the_widths` — the original, describing a fixture that no longer existed and repeating a claim the adversarial review had measured false, plus the superseding one. A reader hitting the first would have taken it as current.\n\n--check-evidence CANNOT SEE THIS. It verifies that a heading names a test that runs, and a duplicate heading names one just as well. Both copies pass.\n\n## Three already on main, verified 2026-09-21\n ### a_layout_change_moves_the_layout_version_and_no_record (test-evidence/command.md)\n ### the_two_front_ends_draw_the_same_column_header (test-evidence/tui.md)\n ### the_whole_frame_is_unchanged (test-evidence/tui.md)\n\nReproduce:\n grep -rhoE '^### [A-Za-z_:][A-Za-z_0-9:]*$' context/test-evidence/*.md | sort | uniq -d\n\n## Why it matters beyond tidiness\ncontext/test-evidence records WHY a test can fail. Two entries under one heading means one of them is stale by construction — they cannot both describe the current test — and nothing indicates which. That is the same class as an evidence entry whose mechanism measurement contradicts (doctrine an_evidence_entry_that_measurement_contradicts_is_worse_than_none): a reader gets a confident wrong answer rather than no answer.\n\n## Acceptance Criteria\n- --check-evidence refuses a duplicate heading, naming the file and both line numbers. It should refuse rather than warn, per the precedent set for ambiguous leaf names in improvise-rm4s.\n- Prove it by injection: add a second copy of an existing heading and watch the checker fail by name.\n- Resolve the three above. Each needs a read to decide which copy is current, or whether they describe genuinely different aspects and want distinct names.\n- Consider whether the same check should apply ACROSS files, not only within one: a heading is keyed to a test name, and test names are unique workspace-wide once module-qualified (improvise-rm4s), so two files claiming the same test is also a defect.\n","status":"open","priority":3,"issue_type":"bug","owner":"el-github@elangley.org","created_at":"2026-09-21T12:39:28Z","created_by":"Edward Langley","updated_at":"2026-09-21T12:39:28Z","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"improvise-swre","title":"grid_of recomputes an unchanged grid every frame; memoise on the tiers that already exist","description":"Split out of improvise-jmjz on the adversarial review of PR #92, which closed jmjz on its own acceptance criterion (fuse the passes) and argued this is the separate, larger win.\n\ngrid_of recomputes the whole grid from scratch EVERY FRAME, even when nothing moved. After jmjz that is exactly one pass rather than three — 95ms at 18,000 cells, 1.32s at 90,000 — but the common case during scrolling, panel toggling and cursor movement is that the grid has not changed at all.\n\n## Why this is cheap to do here\nThe invalidation signal ALREADY EXISTS and is maintained for other reasons:\n- document_generation, layout_version, record_versions (gotcha 40, ADR 001's three tiers)\n- improvise-mcq4's view_shapes / view_frame observation\n\nA GridBody memo keyed on those takes an unchanged grid to roughly zero.\n\n## Why the remaining pass cannot be narrowed instead\nRuled out twice over, and the review measured it: totals sum EVERY row including ones a viewport scrolled past (gotcha 44), and widths are content-adaptive over every row. The benchmark's windowing ratio stayed 1.2-1.4x precisely because the surviving pass is the one a viewport cannot narrow. improvise-i5tk's decline stands.\n\n## Acceptance Criteria\n- A memo on the projected grid body keyed on the existing invalidation tiers; no NEW tier invented, because a second description of \"has the document changed\" is the disease (see the reasoning behind gotcha 39 and 45, where the trigger is the counter moving rather than a table of command variants).\n- Benchmarked: an unchanged grid across frames should approach zero, and the first frame must not regress.\n- The negative arm matters as much as the positive: a test that a grid which DID change is recomputed. A memo that never invalidates is fast and wrong, and it is the failure mode nothing will notice.\n\n## Second-order, much bigger, not this ticket\n~5us per cell at 18k is dominated by evaluate_aggregated doing a matching_values set intersection per cell (gotcha 18). An incremental aggregate index is the deeper fix. 90,000 cells at 1.32s remains unusable; that case is labelled pathological and memoisation helps it most.\n","status":"open","priority":3,"issue_type":"task","owner":"el-github@elangley.org","created_at":"2026-09-21T11:30:15Z","created_by":"Edward Langley","updated_at":"2026-09-21T11:30:15Z","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"improvise-4dpj","title":"The RecordsNormal-to-Normal arm of settle_normal_mode has zero coverage, and it is live","description":"Found by the round-2 review of PR #91, by injection.\n\nReplacing settle_normal_mode's guard with a one-way version — matching only AppMode::Normal, dropping the RecordsNormal arm — leaves THE ENTIRE WORKSPACE SUITE PASSING.\n\n## Why the crossing test does not catch it\na_batch_that_leaves_records_and_then_sets_normal_lands_in_normal drives the LAYOUT both ways, but both branches end the batch with enter-mode normal, so the mode handed to the settle is AppMode::Normal in BOTH directions. The test discriminates 'settles at all' and 'settles against the ending layout', but not 'settles from either mode'. Its intent is honest; it is half-blind.\n\n## The arm is live, not dead\nProved reachable in production:\n\n records mode -\u003e apply_effects([SetAxis { \"_Index\", Axis::None }])\n layout.is_records_mode() == false, mode was RecordsNormal\n -\u003e settle yields AppMode::Normal (passes on shipped code)\n\nSo a geometry effect that takes _Index off Row is the live path through the arm nothing tests.\n\n## Acceptance Criteria\n- One extra case in the existing loop: seed the mode as RecordsNormal and cross OUT. Two lines.\n- The injection above (one-way guard) must then fail by name.\n\nSmall irony worth keeping: the paragraph that ends 'a careful comment standing in for a test is what let the per-effect version ship' names this very test as the remedy.","status":"open","priority":3,"issue_type":"bug","owner":"el-github@elangley.org","created_at":"2026-09-21T11:29:18Z","created_by":"Edward Langley","updated_at":"2026-09-21T11:29:18Z","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"improvise-26dm","title":"The fourth length-not-completeness list: cases.len() == 13 in ui/effect.rs","description":"improvise-5ubs is closed for the two CYCLIC all() functions, by reading serde's\nderive-generated variant list out of its \"unknown variant ... expected one of\"\nmessage and diffing it against the names the cycle reached. That closes the\nAppMode and Rejection halves and the mechanism generalises to any serde enum.\n\nIt does NOT reach the FOURTH instance 5ubs records in its notes:\nlayout_geometry_effects_actually_change_document_config in\ncrates/improvise-command/src/ui/effect.rs, whose assert_eq!(cases.len(), 13) is\nover a hand-built list of EFFECTS, not of enum variants. serde has nothing to\nsay about it: an effect is a struct implementing a trait, so there is no derived\nenumeration to diff against.\n\nThe general statement stands and is the reason this is worth its own ticket:\ndoctrine(a_list_checked_for_length_reads_like_a_list_checked_for_completeness).\nA count catches a case being DELETED from the list and never one being ABSENT\nfrom it, and the two are written identically as an assert_eq over a number. The\nadversarial review of PR #78 added a fourteenth pub struct declaring\nEffectKind::Model(ModelChange::LayoutGeometry), pointed at by nothing, and the\nwhole suite stayed green.\n\nWhat would close it: an enumeration of the effects the compiler maintains. The\ncandidates are a proc-macro over the impl blocks, or inventory-style\nregistration, or reading the Debug names off a derived list the way\nevery_model_command_names_the_effect_of_the_same_name already does for the\nModelCommand mapping -- that last one is the closest existing precedent in this\nrepo and is worth looking at first.\n\nNot fixed in wave I because ui/effect.rs was another wave's ground that round,\nand because a fourth hand-rolled attempt in a fourth place is the disease 5ubs\nnames.\n","status":"open","priority":3,"issue_type":"bug","owner":"el-github@elangley.org","created_at":"2026-09-21T11:09:56Z","created_by":"Edward Langley","updated_at":"2026-09-21T11:09:56Z","dependency_count":0,"dependent_count":0,"comment_count":0} @@ -204,8 +205,8 @@ {"_type":"issue","id":"improvise-sxsf","title":"cargo test -- --list cannot name the binary, so two crates could share a full test path","description":"MEASURED 2026-09-21 (PR #88). Re-verified latent: 1444 names, zero duplicate full paths, zero duplicate (crate, path) pairs.\n\nTHE PROPOSED FIX IS DECLINED ON TWO INDEPENDENT GROUNDS, and the second is the interesting one:\n\n1. COST: 45.2s against 2.4s for the one workspace call (11 members, warm) — 18x. And --check-evidence needs the --ignored twin, so the honest figure is 22 cargo invocations.\n\n2. IT DOES NOT CLOSE THE CLASS IT NAMES. A package is not a binary. The improvise crate alone has FIVE integration test binaries beside its lib, and an integration test's top-level fn lists with NO MODULE PATH AT ALL — 60 of 1444 names are bare. So '-p improvise' prefixes all six identically and tests/a.rs::foo still cannot be told from tests/b.rs::foo.\n\nAny real fix is per-TARGET (--lib, --test \u003cname\u003e), not per-package. Whoever takes this should start there.\n\nWorth noting how this was found: the agent set out to time the per-package fix and discovered it does not work while reading the target list. The cost measurement was the excuse for reading it; the correctness finding was the payoff — an argument for measuring declines rather than asserting them.","status":"open","priority":3,"issue_type":"bug","owner":"el-github@elangley.org","created_at":"2026-09-21T04:49:16Z","created_by":"Edward Langley","updated_at":"2026-09-21T05:23:48Z","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"improvise-8shu","title":"Constrain the .improv grammar generator's strategy; improvise-io's lib binary is the run's outlier","description":"Split out of improvise-8ec3, whose second half (stop pinning one machine's number) is done. This is the first half, left open.\n\nimprovise-io's lib binary is an order of magnitude above every other binary in the run on every machine it has been measured on: 1.40-1.45s over three runs here (vs a ~0.06s median), 3.70-5.24s on the machine that filed 8ec3. The ratio is the portable signal; the seconds are not.\n\nIts slow half is the grammar-walking generator in persistence/parse.rs plus the .improv roundtrip proptests at the default 256 cases. A grammar walk with no size bound generates deeply nested documents whose cost is not uniform across cases, so a few cases dominate.\n\ndoctrine(proptest_cases) rules out the cheap fix: constrain the strategy, do NOT lower the count below 256.\n\nAcceptance: improvise-io's lib binary is no longer an order of magnitude above the run's median, with the case count still 256 and the properties still covering groups, formulas, views, hidden items and pipe-quoting edges. Report the before/after ratio, not seconds.","status":"closed","priority":3,"issue_type":"task","owner":"el-github@elangley.org","created_at":"2026-09-21T04:14:01Z","created_by":"Edward Langley","updated_at":"2026-09-21T05:15:42Z","closed_at":"2026-09-21T05:15:42Z","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"improvise-9v0j","title":"view_frame and view_shape are separated by example, not by property","description":"Found by the adversarial review of PR #78.\n\nimprovise-ni6k introduced view_frame (the drawn frame, drives the cursor) beside view_shape (the configuration, drives layout_version). They answer different questions -- AddCategory of an empty category moves view_shape and not view_frame -- and context/code/command.md section 'Two observations, not one' now records the argument.\n\nTHE GUARD IS ONE-DIRECTIONAL. Tests assert specific commands move specific counters. Nothing asserts the general relation, in either direction:\n\n- Is every reframe also a reshape? (Probably yes: a drawn-frame change should always be a config change. If so it is a property worth pinning, and its failure would mean layout_version under-reports, which per gotcha 35's asymmetry leaves a client wrong FOREVER.)\n- Is every reshape a reframe? (Definitely not -- that is the AddCategory case, and it is the whole reason there are two.)\n\nSo the first is a candidate property test and the second is a documented asymmetry. Today a change that accidentally made view_frame a superset of view_shape, or vice versa, would pass the suite.\n\nWorth doing as a proptest over a generated workbook plus a generated model command, comparing the two differing-key sets. Note the fixture trap improvise-gfvj's investigation turned up: pressing R to enter records mode fires effect::SortData first, so a records-mode fixture built through the key path has an already-sorted store and cannot demonstrate order-sensitive behaviour.","status":"open","priority":3,"issue_type":"task","owner":"el-github@elangley.org","created_at":"2026-09-21T04:01:34Z","created_by":"claude-agent-protocol","updated_at":"2026-09-21T04:01:34Z","dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"improvise-h0iv","title":"group_runs coalesces by name alone, so two runs under different headers merge","description":"Found by the adversarial review of PR #77, which landed `AxisFrame::group_runs`\nas the one home of column-group run coalescing (improvise-8qlr).\n\n`group_runs` coalesces adjacent positions by group **name**:\n\n Some((prev, span)) if prev.map(|p| p.name.as_str()) == name =\u003e *span += 1,\n\nSo two runs under two DIFFERENT headers that happen to share a name read as one\nrun, and the browser draws one `\u003cth colspan\u003e` across both. `FrameGroup` carries\n`cat` as well as `name` — precisely so a client never has to work out which\ncategory a label came from — and comparing the pair would fix it.\n\nWHY PR #77 WAS RIGHT NOT TO FIX IT, so this is a decision and not a\nrediscovery: that PR's job was to make one rule out of two. Both prior copies\n— `src/ui/grid.rs`'s `group_name != prev_group` and\n`improvise-wasm-client/src/render.rs::col_groups`' `run.label == label` —\ncoalesced by name alone, so name-only is what the consolidation had to preserve\nto stay a consolidation. Changing the rule in the same commit that moves it\nwould have meant the agreement test could no longer tell \"the two ends agree\"\nfrom \"the two ends changed together\", which is the whole evidence of the move.\n\nTHE RISK OF LEAVING IT IS SPECIFIC AND NEW: a single home reads as decided.\nThe next person to look at `group_runs` finds one rule, one call site per front\nend, and a test pinning both — and has no reason to suspect the comparison is\nnarrower than the data it is given. That is the cost of a good consolidation\nand the reason to file rather than to note.\n\nReachability today: two categories on one axis, where the inner category has\ngroups of the same name under two different outer items — e.g. `Month` grouped\ninto `Q1` nested under `Region`, giving `North/Q1` and `South/Q1` adjacent. The\nexisting fixture in `tests/grid_frame_agreement.rs` has exactly this shape but\nis saved by the ungrouped `Sep` sitting between the two `Q1` runs, which is\nalso what makes improvise-gd83's spans come out as\n`[(Q1,2), (None,1), (Q1,2), (None,1)]` rather than merged. Remove `Sep` and the\ntwo runs merge.\n\nNot obviously a bug in every case: two adjacent runs of the same group name may\nwell be one visual span a user WANTS spanned. That is the question to answer\nfirst, and it is a question about what a group header means across a nesting\nboundary — which is why this is filed rather than fixed.\n\n## Steps to Reproduce\n\n1. Build a two-level column axis whose inner category is grouped and whose\n groups repeat under each outer item, with NO ungrouped item between them.\n2. `AxisFrame::group_runs` returns one run spanning both.\n3. Both front ends draw one group header across the boundary, because both now\n ask the same (name-only) rule.\n\n## Acceptance Criteria\n\nDecide whether two adjacent same-named runs under different headers are one\nspan or two, and say why in the commit. If two, `group_runs` compares\n`(cat, name)` and the agreement-test fixture gains a case with no separating\nungrouped item — the current one cannot see this.\n","status":"open","priority":3,"issue_type":"bug","owner":"el-github@elangley.org","created_at":"2026-09-21T04:00:18Z","created_by":"agent-render-frame","updated_at":"2026-09-21T04:00:18Z","labels":["area:projection"],"dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"improvise-1wl0","title":"AxisFrame::first is serde(default), so a frame with no origin claims it leads the axis","description":"Found by the adversarial review of PR #77, which landed `AxisFrame::first`.\n\n`#[serde(default)]` on `first` forges the PERMISSIVE claim. `first: 0` is not\nan absence — it is the assertion \"this frame LEADS the axis\", and that is\nexactly what `body_from` acts on:\n\n match self.first + idx == 0 { true =\u003e \u0026self.entries[..], ... }\n\nSo an older peer's frame, or any frame whose `first` is dropped in transit,\ndeserializes as leading the axis. A scrolled browser then draws the carried\nheader ABOVE its first row — the header `body_from` exists to discard there,\nand the mirror image of the improvise-0o6x bug the field was added to fix.\n\nThis is the same argument that gave `Basis` no `Default` and no public\nconstructor (gotcha 43): a zero-valued claim that happens to be TRUE in the\ncommon case is a forgeable bypass rather than an absence. `first` is worse in\none respect — a `Basis` at least has to be constructed deliberately, while a\nserde default is applied silently by a missing field.\n\nNO SKEW SHIPS TODAY. Both ends are built from one workspace at one commit, so\nthere is no older peer. This is about the day there is one, and about the\nattribute reading as a considered compatibility decision when it was not.\n\nOptions, not yet chosen:\n\n * Drop the attribute, so a frame missing `first` fails to deserialize. Loud,\n and correct if a frame is never partially constructed. Note gotcha 37's\n observation that `#[serde(default)]` on an Option is documentation and not\n mechanism — here it is the opposite, it IS the mechanism.\n * Make the leading-ness unrepresentable-otherwise: `Option\u003cusize\u003e`, or a\n two-variant enum (`LeadsAxis` / `StartsAt(usize)`), so \"unknown\" cannot\n silently mean \"leads\".\n\nWhichever, say which in the commit, because the reason to prefer one is about\nwire compatibility and not about the type.\n\n## Steps to Reproduce\n\n1. Serialize an `AxisFrame` built for a scrolled window (`first \u003e 0`) and\n remove the `first` key from the JSON — the shape an older peer sends.\n2. Deserialize it. `first` is 0.\n3. `body_from(0)` now takes the `first + idx == 0` arm and returns the whole\n entry list, including the carried group header.\n4. The browser draws a group header above its first row, at every scroll\n position. Nothing reports anything.\n","status":"open","priority":3,"issue_type":"bug","owner":"el-github@elangley.org","created_at":"2026-09-21T04:00:14Z","created_by":"agent-render-frame","updated_at":"2026-09-21T04:00:14Z","labels":["area:projection"],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"improvise-h0iv","title":"group_runs coalesces by name alone, so two runs under different headers merge","description":"Found by the adversarial review of PR #77, which landed `AxisFrame::group_runs`\nas the one home of column-group run coalescing (improvise-8qlr).\n\n`group_runs` coalesces adjacent positions by group **name**:\n\n Some((prev, span)) if prev.map(|p| p.name.as_str()) == name =\u003e *span += 1,\n\nSo two runs under two DIFFERENT headers that happen to share a name read as one\nrun, and the browser draws one `\u003cth colspan\u003e` across both. `FrameGroup` carries\n`cat` as well as `name` — precisely so a client never has to work out which\ncategory a label came from — and comparing the pair would fix it.\n\nWHY PR #77 WAS RIGHT NOT TO FIX IT, so this is a decision and not a\nrediscovery: that PR's job was to make one rule out of two. Both prior copies\n— `src/ui/grid.rs`'s `group_name != prev_group` and\n`improvise-wasm-client/src/render.rs::col_groups`' `run.label == label` —\ncoalesced by name alone, so name-only is what the consolidation had to preserve\nto stay a consolidation. Changing the rule in the same commit that moves it\nwould have meant the agreement test could no longer tell \"the two ends agree\"\nfrom \"the two ends changed together\", which is the whole evidence of the move.\n\nTHE RISK OF LEAVING IT IS SPECIFIC AND NEW: a single home reads as decided.\nThe next person to look at `group_runs` finds one rule, one call site per front\nend, and a test pinning both — and has no reason to suspect the comparison is\nnarrower than the data it is given. That is the cost of a good consolidation\nand the reason to file rather than to note.\n\nReachability today: two categories on one axis, where the inner category has\ngroups of the same name under two different outer items — e.g. `Month` grouped\ninto `Q1` nested under `Region`, giving `North/Q1` and `South/Q1` adjacent. The\nexisting fixture in `tests/grid_frame_agreement.rs` has exactly this shape but\nis saved by the ungrouped `Sep` sitting between the two `Q1` runs, which is\nalso what makes improvise-gd83's spans come out as\n`[(Q1,2), (None,1), (Q1,2), (None,1)]` rather than merged. Remove `Sep` and the\ntwo runs merge.\n\nNot obviously a bug in every case: two adjacent runs of the same group name may\nwell be one visual span a user WANTS spanned. That is the question to answer\nfirst, and it is a question about what a group header means across a nesting\nboundary — which is why this is filed rather than fixed.\n\n## Steps to Reproduce\n\n1. Build a two-level column axis whose inner category is grouped and whose\n groups repeat under each outer item, with NO ungrouped item between them.\n2. `AxisFrame::group_runs` returns one run spanning both.\n3. Both front ends draw one group header across the boundary, because both now\n ask the same (name-only) rule.\n\n## Acceptance Criteria\n\nDecide whether two adjacent same-named runs under different headers are one\nspan or two, and say why in the commit. If two, `group_runs` compares\n`(cat, name)` and the agreement-test fixture gains a case with no separating\nungrouped item — the current one cannot see this.\n","status":"closed","priority":3,"issue_type":"bug","owner":"el-github@elangley.org","created_at":"2026-09-21T04:00:18Z","created_by":"agent-render-frame","updated_at":"2026-09-21T12:39:37Z","closed_at":"2026-09-21T12:39:37Z","labels":["area:projection"],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"improvise-1wl0","title":"AxisFrame::first is serde(default), so a frame with no origin claims it leads the axis","description":"Found by the adversarial review of PR #77, which landed `AxisFrame::first`.\n\n`#[serde(default)]` on `first` forges the PERMISSIVE claim. `first: 0` is not\nan absence — it is the assertion \"this frame LEADS the axis\", and that is\nexactly what `body_from` acts on:\n\n match self.first + idx == 0 { true =\u003e \u0026self.entries[..], ... }\n\nSo an older peer's frame, or any frame whose `first` is dropped in transit,\ndeserializes as leading the axis. A scrolled browser then draws the carried\nheader ABOVE its first row — the header `body_from` exists to discard there,\nand the mirror image of the improvise-0o6x bug the field was added to fix.\n\nThis is the same argument that gave `Basis` no `Default` and no public\nconstructor (gotcha 43): a zero-valued claim that happens to be TRUE in the\ncommon case is a forgeable bypass rather than an absence. `first` is worse in\none respect — a `Basis` at least has to be constructed deliberately, while a\nserde default is applied silently by a missing field.\n\nNO SKEW SHIPS TODAY. Both ends are built from one workspace at one commit, so\nthere is no older peer. This is about the day there is one, and about the\nattribute reading as a considered compatibility decision when it was not.\n\nOptions, not yet chosen:\n\n * Drop the attribute, so a frame missing `first` fails to deserialize. Loud,\n and correct if a frame is never partially constructed. Note gotcha 37's\n observation that `#[serde(default)]` on an Option is documentation and not\n mechanism — here it is the opposite, it IS the mechanism.\n * Make the leading-ness unrepresentable-otherwise: `Option\u003cusize\u003e`, or a\n two-variant enum (`LeadsAxis` / `StartsAt(usize)`), so \"unknown\" cannot\n silently mean \"leads\".\n\nWhichever, say which in the commit, because the reason to prefer one is about\nwire compatibility and not about the type.\n\n## Steps to Reproduce\n\n1. Serialize an `AxisFrame` built for a scrolled window (`first \u003e 0`) and\n remove the `first` key from the JSON — the shape an older peer sends.\n2. Deserialize it. `first` is 0.\n3. `body_from(0)` now takes the `first + idx == 0` arm and returns the whole\n entry list, including the carried group header.\n4. The browser draws a group header above its first row, at every scroll\n position. Nothing reports anything.\n","status":"closed","priority":3,"issue_type":"bug","owner":"el-github@elangley.org","created_at":"2026-09-21T04:00:14Z","created_by":"agent-render-frame","updated_at":"2026-09-21T12:39:36Z","closed_at":"2026-09-21T12:39:36Z","labels":["area:projection"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"improvise-gfvj","title":"A pure record reorder invalidates no cursor, and I could not build a fixture that reorders","description":"Left open by improvise-ni6k, which closed the rest of its class by observing the grid frame.\n\nni6k names SortData as its SHARPEST case: it changes neither the row count nor any label, and every position still names a different record. The frame fingerprint ni6k landed (App::view_frame, hashing GridLayout row_items/col_items) CANNOT see it, because records-mode row_items are positional (gotcha 1) -- they are 0,1,2... whatever order the store is in.\n\nI wrote the mechanism -- hashing layout.records[].coords alongside the labels -- and then REMOVED it before committing, because I could not demonstrate it catches anything. Every fixture I built came back in key order whatever order the cells were inserted in:\n\n wb.add_item(Region, North); wb.add_item(Region, East);\n set_cell(North/Revenue); set_cell(East/Revenue);\n -\u003e records mode row order is [East, North] BEFORE any SortData\n\nSo either SortData is a no-op for that shape, or something between DataStore::live_records / matching_cells / RecordRow::group is ordering records that gotcha 18 says arrive in insertion order. I did not chase which, and I am not asserting a bug in gotcha 18 on this evidence -- matching_positions sorts POSITIONS (which preserves insertion order), so the explanation is probably elsewhere and probably boring.\n\nTWO THINGS TO DO, in order:\n1. Find out what actually orders records-mode rows, and whether SortData can reorder them at all. If it cannot, this ticket closes as 'not a case' and ni6k's note should be corrected -- its sharpest example would be hypothetical.\n2. Only if it can: put the coords back into view_frame and pin it with the fixture that reproduces.\n\nWHY IT WAS NOT SHIPPED UNVERIFIED: a confident comment over a mechanism nobody has seen fire is this repo's named defect. See doctrine(check_the_checker) and the wave brief's 'a green suite is not evidence'.","notes":"REPRODUCED AND FIXED. My original conclusion was WRONG, and the way it was wrong is the useful part.\n\nI claimed I could not build a fixture where SortData reorders records. The reviewer of PR #78 reproduced it in thirty lines. I re-derived it: insert West/North/East, call sort_by_key, and both live_records() and the records-mode GridLayout reorder to East/North/West.\n\nWHY MY FIXTURE HID IT -- not the reviewer's guess (insertion already sorted), and better than that: MY FIXTURE ENTERED RECORDS MODE BY PRESSING R, and ToggleRecordsMode (command/cmd/grid.rs:476) pushes effect::SortData AS ITS FIRST EFFECT. So the store was sorted by the act of entering records mode, and the SortData I then applied was a genuine no-op. My two-record store did reorder at the DataStore level -- I checked after the review -- so the fixture, not the store, was the problem.\n\nTHE GENERAL TRAP, worth carrying: a records-mode fixture built through the key path has an already-sorted store by construction, so it cannot demonstrate anything order-sensitive. Build one by setting _Index/_Dim axes directly. That is now written into context/test-evidence/command.md under sorting_the_store_in_records_mode_invalidates_the_cursor.\n\nTHE FIX: the layout.records coords hash I removed is restored in App::view_frame, and sorting_the_store_in_records_mode_invalidates_the_cursor pins it -- asserting the row COUNT and the LABELS are both unchanged, so it is the sharp case and not a shape change wearing its name. Proved by removing the coords hash again: that test fails and NOTHING ELSE IN THE CRATE does.\n\nMY PROCESS ERROR, for the record: I concluded 'cannot reproduce' from ONE fixture without varying it, and then filed that conclusion as a finding. Removing the unverified mechanism was right; concluding the hole was not real was not. 'I could not reproduce it' and 'it does not reproduce' are different claims and I filed the second having only earned the first.\n\nClosing.","status":"closed","priority":3,"issue_type":"bug","owner":"el-github@elangley.org","created_at":"2026-09-21T03:47:15Z","created_by":"claude-agent-protocol","updated_at":"2026-09-21T04:01:44Z","closed_at":"2026-09-21T04:01:44Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"improvise-9za9","title":"Demoting an evidence heading to prose is an unguarded escape hatch","description":"check-evidence classifies a heading as a claim by SHAPE -- a bare identifier is a Rust test, a quoted string a JS test, anything else is prose. So any orphaned heading can be retired by rewriting it with a space and a dash, and nothing checks whether the prose is honest.\n\nDEMONSTRATED: appending '#### Helper -- `thing`' to an evidence file is silently unchecked. None of the 14 new --self-test evidence cases covers this, and no mechanism could: the checker cannot know whether prose is truthful.\n\nThis is disclosed rather than hidden -- the docstring names it and points at context/code/tooling.md before loosening -- and the safe direction is confirmed: prose that LOOKS like an identifier is classified as a Rust claim and fails loudly ('#### Background' reports as an orphan) rather than passing silently.\n\nThe live instance was PR #69 demoting '? opens help, and its rows are the shared ones' while PR #72 was restoring that coverage under a different name. That is the argument for treating a demotion as something a reviewer must read rather than something a gate can hold. Worth deciding whether a demotion should require a marker the checker can count, so at least the NUMBER of retired claims is visible.","status":"closed","priority":3,"issue_type":"task","owner":"el-github@elangley.org","created_at":"2026-09-21T00:11:25Z","created_by":"orchestrator","updated_at":"2026-09-21T04:49:29Z","closed_at":"2026-09-21T04:49:29Z","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"improvise-w8ih","title":"extract web/assemble.sh's stamp gate so it can be exercised in isolation","description":"Found by review of PR #69. improvise-x4wv made stamped() record every missing cache-bust stamp and put the fail in a four-line 'test -z $missing' gate after the four calls. That gate is the whole guard, and deleting it is invisible: both failures still print and assemble.sh exits 0, proved by injection on #69.\n\nThe decline of a full assemble harness stands -- it would have to fabricate web/pkg and web/worker-pkg, a second answer to 'what does the site assemble from', which improvise-vvl3 exists to prevent. But the gate alone does not need the tree: extract it (a function taking the missing list, or a tiny script) and exercise it the way scripts/node-test-test.sh and scripts/kb-test.sh exercise theirs -- empty list exits 0, non-empty exits 1 naming every entry. Wire it into CI in the same change.","status":"closed","priority":3,"issue_type":"task","owner":"el-github@elangley.org","created_at":"2026-09-21T00:01:05Z","created_by":"claude-agent-scripts","updated_at":"2026-09-21T04:49:30Z","closed_at":"2026-09-21T04:49:30Z","dependency_count":0,"dependent_count":0,"comment_count":0} @@ -249,7 +250,7 @@ {"_type":"issue","id":"improvise-rw3a","title":"Decide whether ZZ routes into the save prompt automatically","description":"The half of improvise-lae that was deliberately left open. PR #27 built the prompt (AppMode::SavePrompt, TextEntry builder, committing to SaveAs, bound Ctrl-W and reachable as `:enter-mode save-prompt`) and stopped short of making ZZ route into it.\n\nTODAY, after PR #27: ZZ on a pathless document refuses, stays in the app, and the status line names the prompt. That is already better than improvise-8tk left it -- the user is told where to go. The open question is whether being told is enough, or whether the vim reflex should just land somewhere useful.\n\nTHE OBSTACLE THE ORIGINAL TICKET NAMED, still true: CmdContext does not expose file_path, so a Cmd cannot currently decide \"is there a path\" at execute time. Adding the field is small; note improvise-2ud is separately proposing that CmdContext borrow the ModelState/ViewState slices rather than copying ~20 individual fields, which would make this free. Coordinate rather than adding a 21st copied field ahead of it.\n\nTHE CONSTRAINT PR #27 ESTABLISHED, and it must be respected by whatever lands here: the prompt is SESSION-LOCAL and was deliberately kept in the COMMAND layer rather than routed through effect::Save. Save is becoming a wire command (improvise-368h), and a prompt is addressed to precisely one session -- routing it through the effect would open a prompt on a host nobody asked, which is gotcha 38's argument for keeping AcceptWizardModel off the wire. So: a Cmd that inspects the path and emits a mode change, never an effect that decides to prompt.\n\nDECIDE ALSO what ZZ means once the prompt is open. ZZ is \"save and quit\"; if the prompt commits to SaveAs and the batch was aborted, the quit is gone. Should committing the path then quit? That is a second composition and the answer is not obviously yes -- a user who hit ZZ, got a prompt, and typed a path plausibly wants to leave, but making the prompt's Enter mode-dependent is the kind of hidden state improvise-ct5 just declined to add for `i`.\n\n## Decision\n\nOpen. This is the half of improvise-lae that was deliberately left undecided when the save prompt landed.\n\n## Rationale\n\nAfter PR #27, `ZZ` on a pathless document refuses, stays in the app, and the status line names the prompt. That is already better than improvise-8tk left it — the user is told where to go. The question is whether being told is enough, or whether the vim reflex should land somewhere useful by itself.\n\n## Alternatives Considered\n\n**Route `ZZ` into the prompt automatically.** Needs `CmdContext` to expose `file_path`, which it does not. improvise-2ud separately proposes that `CmdContext` borrow the ModelState/ViewState slices rather than copying ~20 fields, which would make this free — coordinate rather than adding a 21st copied field ahead of it.\n\n**Leave `ZZ` as it is.** The status line names `Ctrl-W`; the cost is one keystroke of discovery.\n\n**Constraint on any answer:** the prompt is SESSION-LOCAL and must stay in the command layer, never routed through `effect::Save`. Save is now a wire command (improvise-368h), and an effect that decides to prompt would open a prompt on a host nobody asked — gotcha 38's argument for keeping AcceptWizardModel off the wire.\n\n**Unanswered second question:** what `ZZ` means once the prompt is open. ZZ is save-AND-QUIT; if committing the path does not also quit, the reflex is only half honoured — but making the prompt's Enter mode-dependent is the hidden state improvise-ct5 just declined to add for `i`.\n","status":"open","priority":3,"issue_type":"decision","owner":"el-github@elangley.org","created_at":"2026-09-20T06:04:32Z","created_by":"Edward Langley","updated_at":"2026-09-20T07:45:42Z","labels":["area:tui"],"dependencies":[{"issue_id":"improvise-rw3a","depends_on_id":"improvise-2ud","type":"blocks","created_at":"2026-09-20T16:20:29Z","created_by":"Edward Langley","metadata":"{}"}],"dependency_count":1,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"improvise-g9jb","title":"The text-entry mode list is hand-written in two crates and checked against AppMode in neither","description":"Found by WU-G on PR #27 while adding AppMode::SavePrompt.\n\nThe set of text-entry modes is written down BY HAND IN TWO PLACES, in two crates:\n * improvise-command::keymap::tests::text_entry_modes_with_exits\n * improvise-wasm-client::keys::tests::text_entry_modes\n\nand NOTHING CHECKS EITHER AGAINST AppMode. So adding a text-entry variant compiles, passes, and is silently absent from both lists -- which matters more since PR #27, because the browser now refuses to ENTER a prompt it cannot commit (keys.rs::strands_the_user), and that refusal is driven off one of these hand-written lists.\n\nTHE FAILURE MODE IS THE QUIET ONE: a new prompt mode omitted from the wasm list is one a browser will happily open and then swallow the user's typing in -- doctrine(a_doorway_to_a_black_hole_is_worse_than_the_key), which PR #27 established against SavePrompt and found ExportPrompt had already.\n\nTHE FIX: derive the list once, from AppMode itself. `MinibufferConfig` is already the thing that makes a variant text-entry -- AppMode::minibuffer() is an exhaustive match returning Option\u003cMinibufferConfig\u003e (gotcha 11), so \"the text-entry modes\" is derivable rather than enumerable. A const or a function in improvise-protocol beside AppMode, with both crates reading it, replaces both lists.\n\nCHECK ONE THING BEFORE ASSUMING THE DERIVATION IS FREE: the wasm list and the command list may not be the same set for a reason -- SearchMode is hand-written on purpose and has no commit key (gotcha 11), and the browser's question (\"can this client commit here\") is not identical to the keymap's (\"does this mode take text\"). If they genuinely differ, the answer is two derived predicates over one enum, not two hand-written lists.\n\ndoctrine(a_hand_enumerated_mirror_needs_an_exhaustive_match) already covers this; this is an instance with no match behind it at all.\n","acceptance_criteria":"The set of text-entry modes is DERIVED from AppMode rather than hand-written twice. AppMode::minibuffer() is already an exhaustive match returning Option\u003cMinibufferConfig\u003e, so the set is derivable. Before assuming one derivation serves both: check whether the keymap's question ('does this mode take text') and the browser's question ('can this client commit here') are actually the same set — SearchMode is hand-written on purpose and has no commit key. If they differ, the answer is two derived predicates over one enum, never two hand-written lists.","status":"closed","priority":3,"issue_type":"task","assignee":"claude-agent-protocol","owner":"el-github@elangley.org","created_at":"2026-09-20T06:04:22Z","created_by":"Edward Langley","updated_at":"2026-09-21T03:47:03Z","started_at":"2026-09-21T03:28:20Z","closed_at":"2026-09-21T03:47:03Z","close_reason":"Closed","labels":["area:shared-frontend"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"improvise-z05r","title":"CommitExport does not bail on an empty path, and ExportCsv does not abort the batch","description":"Found by WU-G on PR #27 while fixing the save arms, and left deliberately: these are the same two defects one file over, at lower severity, and fixing them in that PR would have widened a save-focused change into an export-focused one.\n\nTWO ARMS, both mirroring what was just fixed for save:\n 1. CommitExport does not bail on an empty path, where CommitBuffer's three kinds all do (gotcha 20: buffer -\u003e trim -\u003e bail-on-empty -\u003e dirty -\u003e status is the shared spine).\n 2. ExportCsv does not set abort_effects on failure, where Save and (as of PR #27) SaveAs both do. So a `Binding::Sequence` or a `:` line composing an export with anything after it continues past a failed write.\n\nWHY IT IS LOWER SEVERITY THAN THE SAVE ARMS, and this is the honest reason it was deferred rather than a claim that it does not matter: an export failure discards nothing. improvise-8tk was a P1 because ZZ quit and the model was gone. Here the model is untouched and the user has simply not got the CSV they asked for -- so the cost is a missing file and a message, not lost work.\n\nNOTE THE SHAPE, because it is the third instance in this area: improvise-8tk fixed Save and not SaveAs; PR #27 found SaveAs; and this is the export arm of the same rule. doctrine(one_arm_of_a_symmetric_rule_is_evidence_about_that_arm_only) exists for exactly this, and the general fix may be better than the third patch -- consider whether \"an effect that writes a file and fails aborts the batch\" can be made structural rather than remembered three times.\n\nALSO CHECK, while in there: whether the export error is visible at all, given that ExportPrompt is a minibuffer mode and a status message written while a minibuffer is open is never drawn (see the sibling bead on draw_bottom_bar).\n\n## Steps to Reproduce\n\n1. `CommitExport` with an empty path: it does not bail, where `CommitBuffer`'s three kinds all do (gotcha 20's shared spine is buffer → trim → bail-on-empty → dirty → status).\n2. Compose an export with anything after it — a `Binding::Sequence`, or a `:` line on ` . ` — and make the write fail. `ExportCsv` does not set `abort_effects`, so the chain continues past the failure, where `Save` and (since PR #27) `SaveAs` both stop it.\n\nLower severity than the save arms because an export failure discards nothing: the model is untouched and the user simply has not got their CSV.\n","acceptance_criteria":"CommitExport bails on an empty path like the other three commit kinds, and ExportCsv aborts the batch on a failed write like Save and SaveAs. Prefer the structural fix over the third patch: this is the third instance of the same asymmetry, so consider whether 'an effect that writes a file and fails aborts the batch' can be made impossible to forget rather than remembered three times.","status":"closed","priority":3,"issue_type":"bug","assignee":"claude-agent-cmd","owner":"el-github@elangley.org","created_at":"2026-09-20T06:04:21Z","created_by":"Edward Langley","updated_at":"2026-09-21T04:43:49Z","started_at":"2026-09-21T03:27:54Z","closed_at":"2026-09-21T04:43:49Z","labels":["area:tui"],"dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"improvise-44ad","title":"ColWidths is redundant under the label fold, and the test that would change is gotcha 33(c)'s evidence","description":"Found while closing improvise-hkt6 (PR #20, WU-K), tried, and DELIBERATELY REVERTED rather than landed -- filed so it is a decision rather than a rediscovery.\n\nhkt6(b) obliged grid_updates to diff `totals` against the LABEL-FOLDED cache rather than against the mirror (see gotcha 33). The symmetric move for `col_widths` looks obviously right and is not free: it BREAKS `a_client_on_a_socket_is_seeded_and_gets_its_edit_back`.\n\nWHY, and it is a real property rather than a brittle assertion: folding the labels RE-FITS the widths (gotcha 33(c) -- every arm that touches labels or widths re-fits, because a renderer zips widths with labels and a length mismatch indexes off the end). So under a fold-relative diff the ColWidths that a reframe currently carries becomes REDUNDANT and is suppressed. The client is correct either way; what changes is whether the reframe still SENDS the widths.\n\nTHE REASON IT WAS NOT SPENT IN PR #20: that assertion is gotcha 33(c)'s standing evidence -- it is the test that catches \"a late ColWidths put the mismatch straight back\". Rewriting it inside a PR whose subject was three totality gaps would have destroyed the evidence for a rule while fixing something else, which is exactly the shape improvise-q0vb was declined for on PR #12. Two right changes, different costs.\n\nTHE DECISION TO MAKE:\n * Leave widths diffed against the mirror. Costs one redundant ColWidths per reframe -- the same direction Dirty::of is deliberately sloppy in (gotcha 35: over-report pays one redundant Projection, under-report leaves a client wrong forever). Defensible and arguably correct on that principle alone.\n * Or move widths to the fold and REPLACE the socket assertion with one that still pins 33(c)'s property -- that a client never holds widths of a different length from its labels -- rather than one that pins the message count. That is the better test either way, and writing it is most of the work.\n\nPREFER THE SECOND ONLY IF the replacement test is written first and shown to fail against a deliberately late ColWidths. Otherwise prefer the first and say so in gotcha 33, so the asymmetry between totals and widths is STATED rather than discovered by the next person who notices it.\n\n## Rationale\n\nimprovise-hkt6 obliged `grid_updates` to diff `totals` against the LABEL-FOLDED cache rather than the mirror, or a rename that leaves the number equal sends no `Totals` and the client blanks its own. The symmetric move for `col_widths` looks obviously right and is not free: it breaks `a_client_on_a_socket_is_seeded_and_gets_its_edit_back`.\n\nThat is a real property, not a brittle assertion. Folding the labels RE-FITS the widths (gotcha 33(c)), so under a fold-relative diff the `ColWidths` a reframe currently carries becomes redundant and is suppressed. The client is correct either way; what changes is whether the reframe still SENDS them.\n\n## Alternatives Considered\n\n**Leave widths diffed against the mirror.** Costs one redundant `ColWidths` per reframe — the same direction `Dirty::of` is deliberately sloppy in (gotcha 35: over-report pays one redundant Projection, under-report leaves a client wrong forever). Defensible on that principle alone, and then the asymmetry between totals and widths should be STATED in gotcha 33 rather than rediscovered.\n\n**Move widths to the fold and replace the socket assertion** with one that still pins 33(c)'s property — that a client never holds widths of a different length from its labels — rather than one that pins the message count. That is the better test either way, and writing it is most of the work.\n\n**Not acceptable: move widths to the fold and simply update the failing assertion.** It is gotcha 33(c)'s standing evidence, and rewriting it inside a change it is meant to guard destroys the evidence for that change — the improvise-q0vb / PR #12 shape.\n","status":"open","priority":3,"issue_type":"decision","owner":"el-github@elangley.org","created_at":"2026-09-20T05:54:56Z","created_by":"Edward Langley","updated_at":"2026-09-20T07:45:42Z","labels":["area:command"],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"improvise-44ad","title":"ColWidths is redundant under the label fold, and the test that would change is gotcha 33(c)'s evidence","description":"Found while closing improvise-hkt6 (PR #20, WU-K), tried, and DELIBERATELY REVERTED rather than landed -- filed so it is a decision rather than a rediscovery.\n\nhkt6(b) obliged grid_updates to diff `totals` against the LABEL-FOLDED cache rather than against the mirror (see gotcha 33). The symmetric move for `col_widths` looks obviously right and is not free: it BREAKS `a_client_on_a_socket_is_seeded_and_gets_its_edit_back`.\n\nWHY, and it is a real property rather than a brittle assertion: folding the labels RE-FITS the widths (gotcha 33(c) -- every arm that touches labels or widths re-fits, because a renderer zips widths with labels and a length mismatch indexes off the end). So under a fold-relative diff the ColWidths that a reframe currently carries becomes REDUNDANT and is suppressed. The client is correct either way; what changes is whether the reframe still SENDS the widths.\n\nTHE REASON IT WAS NOT SPENT IN PR #20: that assertion is gotcha 33(c)'s standing evidence -- it is the test that catches \"a late ColWidths put the mismatch straight back\". Rewriting it inside a PR whose subject was three totality gaps would have destroyed the evidence for a rule while fixing something else, which is exactly the shape improvise-q0vb was declined for on PR #12. Two right changes, different costs.\n\nTHE DECISION TO MAKE:\n * Leave widths diffed against the mirror. Costs one redundant ColWidths per reframe -- the same direction Dirty::of is deliberately sloppy in (gotcha 35: over-report pays one redundant Projection, under-report leaves a client wrong forever). Defensible and arguably correct on that principle alone.\n * Or move widths to the fold and REPLACE the socket assertion with one that still pins 33(c)'s property -- that a client never holds widths of a different length from its labels -- rather than one that pins the message count. That is the better test either way, and writing it is most of the work.\n\nPREFER THE SECOND ONLY IF the replacement test is written first and shown to fail against a deliberately late ColWidths. Otherwise prefer the first and say so in gotcha 33, so the asymmetry between totals and widths is STATED rather than discovered by the next person who notices it.\n\n## Rationale\n\nimprovise-hkt6 obliged `grid_updates` to diff `totals` against the LABEL-FOLDED cache rather than the mirror, or a rename that leaves the number equal sends no `Totals` and the client blanks its own. The symmetric move for `col_widths` looks obviously right and is not free: it breaks `a_client_on_a_socket_is_seeded_and_gets_its_edit_back`.\n\nThat is a real property, not a brittle assertion. Folding the labels RE-FITS the widths (gotcha 33(c)), so under a fold-relative diff the `ColWidths` a reframe currently carries becomes redundant and is suppressed. The client is correct either way; what changes is whether the reframe still SENDS them.\n\n## Alternatives Considered\n\n**Leave widths diffed against the mirror.** Costs one redundant `ColWidths` per reframe — the same direction `Dirty::of` is deliberately sloppy in (gotcha 35: over-report pays one redundant Projection, under-report leaves a client wrong forever). Defensible on that principle alone, and then the asymmetry between totals and widths should be STATED in gotcha 33 rather than rediscovered.\n\n**Move widths to the fold and replace the socket assertion** with one that still pins 33(c)'s property — that a client never holds widths of a different length from its labels — rather than one that pins the message count. That is the better test either way, and writing it is most of the work.\n\n**Not acceptable: move widths to the fold and simply update the failing assertion.** It is gotcha 33(c)'s standing evidence, and rewriting it inside a change it is meant to guard destroys the evidence for that change — the improvise-q0vb / PR #12 shape.\n","status":"closed","priority":3,"issue_type":"decision","owner":"el-github@elangley.org","created_at":"2026-09-20T05:54:56Z","created_by":"Edward Langley","updated_at":"2026-09-21T12:39:38Z","closed_at":"2026-09-21T12:39:38Z","labels":["area:command"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"improvise-j7v6","title":"The typed-string classification lives in two crates, and its guard test asserts by hand","description":"Named by improvise-r4lo (PR #21) while deleting the OTHER duplication, and deliberately left rather than folded in -- r4lo was about two answers on the CLIENT side, and deleting Client::set_cell/set_cell_at settled that. This is the duplication one level out, and it crosses a crate boundary.\n\nwasm-client/src/socket.rs::typed classifies a user-typed string into a CellValue. So does improvise-command's commit_regular_cell_value, on the host's own key path. Two implementations of \"what does this string mean\", in two crates, with nothing holding them together.\n\nBOTH ARE LEGITIMATE, which is why this is not simply r4lo again. The browser has no App to ask and ModelCommand::SetCell takes a CellValue, so somebody on the client side must decide; a host typing locally has an App and classifies there. The defect is that the RULE is written twice, not that it is applied in two places.\n\nTHE TEST THAT LOOKS LIKE A GUARD AND IS NOT: a_typed_string_is_classified_as_the_host_classifies_it states the obligation IN ITS NAME and does not discharge it -- it asserts three outcomes BY HAND rather than comparing against the host's function. So the two can drift and the test stays green, which is this repository's characteristic defect (doctrine(check_the_checker)): a test that passes while the thing it names is broken.\n\nTHE FIX: one classification function in improvise-protocol, which both crates already depend on, and both call sites calling it. Then the test either disappears (type identity replaces it) or becomes a real comparison. Note doctrine(a_consolidation_that_keeps_its_compatibility_test_is_unfinished) -- if the shared function lands and the hand-written three-outcome test survives beside it, the consolidation is not done.\n\nWATCH ONE THING WHILE MOVING IT: the classification must stay consistent with what the .improv format considers a number versus pipe-quoted text (gotcha 34, and format.rs::pipe_quote exists precisely so a text cell cannot read back as a number). A shared classifier that disagrees with the persistence layer would be a third answer wearing the badge of the first.\n","acceptance_criteria":"One classification function in improvise-protocol, called by socket.rs::typed and by commit_regular_cell_value. a_typed_string_is_classified_as_the_host_classifies_it either disappears (type identity replaces it) or becomes a real comparison against the shared function — if the shared function lands and the hand-written three-outcome test survives beside it, the consolidation is not finished. Check the result stays consistent with what .improv considers a number versus pipe-quoted text, or the shared classifier is a third answer wearing the badge of the first.","status":"closed","priority":3,"issue_type":"task","assignee":"claude-agent-protocol","owner":"el-github@elangley.org","created_at":"2026-09-20T05:45:29Z","created_by":"Edward Langley","updated_at":"2026-09-21T03:47:03Z","started_at":"2026-09-21T03:28:20Z","closed_at":"2026-09-21T03:47:03Z","close_reason":"Closed","labels":["area:shared-frontend"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"improvise-rucf.3","title":"Projection freshness for a source an external writer can change","description":"Child of improvise-rucf (ADR 003, live-backed models). Filed and DEFERRED 2026-09-19 alongside its parent.\n\nADR 001 governs concurrent EDITS. A live-backed model is read-only, so it has none and the basis check does not apply to it -- the guarantee is not voided, it is scoped to documents improvise owns, which is what it always meant.\n\nWHAT REMAINS IS A DIFFERENT AND SMALLER QUESTION, and this bead is it: an EXTERNAL WRITER changes what clients SEE. So a live-backed projection needs a freshness story -- polled, triggered, or explicitly stale-until-refreshed. That is projection freshness, not conflict resolution, and conflating the two is the trap.\n\nNOTE WHAT CHANGES ABOUT THE PROJECTION ARCHITECTURE: today's diff (ui/project.rs::grid_updates) compares a desired state derived from the App against each client's mirror. With a live source, \"the desired state\" involves latency -- a HashMap lookup becomes a query -- so the VIEWPORT-BOUNDED PROJECTION STOPS BEING AN OPTIMISATION AND BECOMES LOAD-BEARING. improvise-i5tk (narrow the TUI's GridCache to its viewport, benchmark first) is the related bead and its answer may be forced by this one.\n\nSHARED WITH THE SQLITE SOURCE CHILD: the RecordHash -\u003e record index becomes a cache needing invalidation rather than a structure maintained alongside writes. One invalidation story, not two.\n\nEXPLICITLY OUT OF SCOPE: anything resembling a lease, a lock, or a reservation. Those are ADR 001 layer 2 and they are about edits.\n","status":"deferred","priority":3,"issue_type":"task","owner":"el-github@elangley.org","created_at":"2026-09-20T05:31:33Z","created_by":"Edward Langley","updated_at":"2026-09-20T05:31:35Z","dependencies":[{"issue_id":"improvise-rucf.3","depends_on_id":"improvise-rucf","type":"parent-child","created_at":"2026-09-20T05:31:33Z","created_by":"Edward Langley","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"improvise-rucf.2","title":"SQLite source implementation: aggregate pushdown over a user-supplied query, native-only and feature-gated","description":"Child of improvise-rucf (ADR 003, live-backed models). Filed and DEFERRED 2026-09-19 alongside its parent.\n\nThe SQLite implementation of the source trait: aggregate pushdown against a user-supplied query.\n\nWHICH ROWS: a user-supplied SELECT, with a bare table name as sugar over it. A query is strictly more useful (joins, filters, computed columns) and no harder to consume -- a result set is a result set.\n\nTYPE MAPPING, and it is BETTER INPUT THAN CSV rather than merely different: a SQLite column carries a declared type, where import/analyzer.rs::detect_date_format currently guesses a date by pattern-matching text. INTEGER/REAL are measure candidates, TEXT a category candidate, BLOB skip. NULL is ABSENCE, not the string \"null\" (improvise-ych). Dates may be TEXT, INTEGER (epoch) or REAL (julian) -- DETECT, DO NOT ASSUME. SQLite is dynamically typed per value, so both the declared-type and the sampled path are real and the classifier should take typed input where it has it.\n\nWHERE THE BOUNDARY GOES: forbidden(improvise_core, io). The source TRAIT belongs in improvise-core; this implementation belongs in improvise-io, FEATURE-GATED AND NATIVE-ONLY. libsqlite3-sys does not build for wasm32-unknown-unknown under this repo's nix cc-wrapper, and improvise-io is in the wasm bundle, so the wasm CI job must keep passing untouched. VERIFY THE GATE, do not assume it -- the improvise-9ix agent established the wasm failure is the nix wrapper injecting -fzero-call-used-regs rather than proof that bundled SQLite cannot target wasm, so the gating protects the browser bundle rather than working around an impossibility.\n\nNO WRITES, and not as a runtime refusal: per ADR 003 writes are not in the trait a query source implements, and a source projecting no CellId::Record makes a client write unconstructible.\n\nINTERNING AND IDENTITY: DataStore interns names and ADR 002 hashes the coordinate tuple. Both still work -- coordinates come from the query -- but the RecordHash -\u003e record index that resolves a CellId becomes A CACHE WITH AN INVALIDATION STORY rather than a structure maintained alongside writes. That invalidation is shared with the freshness child; do not solve it twice.\n","status":"deferred","priority":3,"issue_type":"task","owner":"el-github@elangley.org","created_at":"2026-09-20T05:31:32Z","created_by":"Edward Langley","updated_at":"2026-09-20T05:31:34Z","dependencies":[{"issue_id":"improvise-rucf.2","depends_on_id":"improvise-rucf","type":"parent-child","created_at":"2026-09-20T05:31:32Z","created_by":"Edward Langley","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} @@ -348,7 +349,7 @@ {"_type":"issue","id":"improvise-6nsd","title":"A prune-by-write renumbers the grid without moving layout_version -- ADR 001 middle tier","description":"A cell write that prunes a row renumbers the grid without moving layout_version.\n\nMeasured 2026-09-21 while answering improvise-9v0j, by driving each ModelCommand\nthrough apply_command and reading cursor_epoch() and layout_version().\n\nEvery command in reshaping_commands() moves both counters. A ClearCell that\nprunes a row out of a pruning view moves the cursor epoch and NOT layout_version:\n\n ClearCell(prunes a row) reframe=true reshape=false\n\nThat is not a defect in the observation. improvise-mcq4 designed it that way: a\ncell write changes DATA, not the view's CONFIGURATION, and view_shape hashes\nconfiguration. So 9v0j's candidate property test (\"is every reframe also a\nreshape?\") would FAIL and writing it would be wrong. That half is answered and\nrecorded in context/code/command.md.\n\nWHAT IS OPEN, and it is an ADR question rather than a test to land quietly.\nAfter such a write the grid has renumbered while layout_version has not moved,\nso a client still claiming the OLD layout tier is accepted. The record tier\ncovers a write to the PRUNED record -- that client's picture of Service is\nstale. It says nothing about a write to a SURVIVING record whose row index moved\nunderneath. Whether that is a hole in ADR 001's middle tier, or correctly\ndelegated to the record tier because the record surrogate is position-free\n(ADR 002), is the question.\n\nNote which way the asymmetry falls if it IS a hole: gotcha 35 says\nunder-reporting leaves a client wrong forever, and layout_version under-reports\nhere by construction.\n\nReachability: needs two clients on pruning views and a cell write that empties a\nrow. The cursor invalidation half already works (mcq4), so the user-visible\nsymptom would be a basis check accepting a batch composed against a grid shape\nthat no longer exists -- not a wrong cursor.\n","status":"open","priority":4,"issue_type":"bug","owner":"el-github@elangley.org","created_at":"2026-09-21T11:09:49Z","created_by":"Edward Langley","updated_at":"2026-09-21T11:30:17Z","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"improvise-9d5k","title":"Two small wrong claims in kb-merge.pl's own comments","description":"Both found by review on PR #69, both one-line fixes, both are claims ABOUT A CHECK sitting where nothing verifies them -- which is the subject of the PR they arrived in.\n\n1. kb-test.sh case 10's comment says 'under unification conflicts sees the non-ground key match both facts and refuses the add outright, so the reverted fix fails HERE.' DEMONSTRATED false: reverting conflicts to unification leaves case 10 PASSING and fails case 17 instead. A rule now keys on its whole clause, so the ':-' term cannot unify with probe_fact(a) and conflicts never fires there. The site is guarded, by a different case, so this is a wrong explanation rather than a hole.\n\n2. replace's new refusal message says '...so it would ADD them beside the ones already there' even when the predicate has no existing clauses. Behaviour is right and fail-safe; only that clause of the sentence is wrong in that arm.","status":"closed","priority":4,"issue_type":"task","owner":"el-github@elangley.org","created_at":"2026-09-21T00:11:27Z","created_by":"orchestrator","updated_at":"2026-09-21T04:49:29Z","closed_at":"2026-09-21T04:49:29Z","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"improvise-r08u","title":"IS and WHERE differ on trailing word boundary in formula.pest","description":"improvise-4zl7 gave `IS` a trailing word-boundary lookahead: `is_expr` is compound-atomic so `Region ISLAND` no longer parses as `Is(Region, LAND)`. `^\"WHERE\"` in `where_clause` and `inline_where` still has the identical hole — `SUM(Rev WHEREAS x = y)` would read as a WHERE on a category `AS`. It is far less exposed than IS was (WHERE sits in one fixed position after an expression, where IS follows any identifier in expression position), which is why it was left alone rather than fixed in the same pass: it is pre-existing and changing it is a grammar change with its own round-trip risk.\n\n## Steps to Reproduce\n\n1. `parse_formula(\"X = SUM(Rev WHEREAS x = y)\", \"Cat\")`\n2. It parses, reading `AS` as the filter category, where it should be a syntax error.","acceptance_criteria":"Either `^\"WHERE\"` gets the same `!bare_char` lookahead `IS` has and a test asserting `WHEREAS` does not parse as a WHERE clause, or the asymmetry is declined in writing in context/code/formula-and-io.md with the reason. The grammar-walking generator proptests must still pass either way.","status":"closed","priority":4,"issue_type":"bug","owner":"el-github@elangley.org","created_at":"2026-09-20T19:16:46Z","created_by":"wave3-4zl7","updated_at":"2026-09-21T04:42:55Z","closed_at":"2026-09-21T04:42:55Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"improvise-1l9o","title":"furnished_client hand-builds a basis instead of taking the server's","description":"A nit from the adversarial review of PR #50, explicitly not worth a push at\nthe time.\n\nlockstep.rs's furnished_client builds its BasisCache by walking the client's\nown grid for Record cells and calling basis::published(0, 0, records) with\nevery version hard-coded to 0. That is a small re-implementation of\nproject.rs::basis_update's record selection, in a test whose entire purpose is\nto rule out re-implementations.\n\nIt is harmless today -- a fresh App really is at generation 0, layout 0, and\nevery record at version 0 -- which is exactly why it will stop being harmless\nsilently if any of those three stops being true.\n\nThe reviewer demonstrated the alternative while checking something else: seed\nthe ClientState from a real Sessions::connect snapshot and let the server's own\nProjection::Basis populate the cache. That removes the fabrication and makes\nthe fixture a client the server actually produced.\n\nNote the same fabrication exists in client_ready_to_edit, which predates this\nPR, so a fix should take both.","acceptance_criteria":"The lockstep fixtures get their basis from a real Sessions::connect\nsnapshot rather than constructing one. Both furnished_client and\nclient_ready_to_edit, since they share the fabrication. The tests that use\nthem must still pass unchanged -- if seeding from the server changes an\noutcome, that is a finding about the fixture and worth stopping for.","status":"open","priority":4,"issue_type":"task","owner":"el-github@elangley.org","created_at":"2026-09-20T18:58:19Z","created_by":"wave2-h-frontend","updated_at":"2026-09-20T18:58:19Z","labels":["area:shared-frontend"],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"improvise-1l9o","title":"furnished_client hand-builds a basis instead of taking the server's","description":"A nit from the adversarial review of PR #50, explicitly not worth a push at\nthe time.\n\nlockstep.rs's furnished_client builds its BasisCache by walking the client's\nown grid for Record cells and calling basis::published(0, 0, records) with\nevery version hard-coded to 0. That is a small re-implementation of\nproject.rs::basis_update's record selection, in a test whose entire purpose is\nto rule out re-implementations.\n\nIt is harmless today -- a fresh App really is at generation 0, layout 0, and\nevery record at version 0 -- which is exactly why it will stop being harmless\nsilently if any of those three stops being true.\n\nThe reviewer demonstrated the alternative while checking something else: seed\nthe ClientState from a real Sessions::connect snapshot and let the server's own\nProjection::Basis populate the cache. That removes the fabrication and makes\nthe fixture a client the server actually produced.\n\nNote the same fabrication exists in client_ready_to_edit, which predates this\nPR, so a fix should take both.","acceptance_criteria":"The lockstep fixtures get their basis from a real Sessions::connect\nsnapshot rather than constructing one. Both furnished_client and\nclient_ready_to_edit, since they share the fabrication. The tests that use\nthem must still pass unchanged -- if seeding from the server changes an\noutcome, that is a finding about the fixture and worth stopping for.","status":"closed","priority":4,"issue_type":"task","owner":"el-github@elangley.org","created_at":"2026-09-20T18:58:19Z","created_by":"wave2-h-frontend","updated_at":"2026-09-21T12:39:37Z","closed_at":"2026-09-21T12:39:37Z","labels":["area:shared-frontend"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"improvise-thpe","title":"help::compact discriminates a chord by parsing a string it formatted","description":"Finding 3 of the adversarial review of PR #50. Not live-broken and\ndeliberately not fixed in that PR.\n\nimprovise_protocol::help::compact drops chorded keys from a status-bar hint by\ntesting whether the formatted label starts with \"Ctrl-\" or \"Alt-\". The\nreviewer enumerated every format_key_label output and none begins with either,\nso there is no false positive today.\n\nThe defect is the shape, not the behaviour. keymap.rs::labels_for HAS the\nstructured fact -- it matches KeyPattern::Key(code, mods) and then throws mods\naway into a string -- and compact then reconstructs a worse version of it by\nparsing. A future modifier spelled any other way (Super-, Cmd-, a bare glyph)\nis silently not a chord, lands in the status bar, and nothing fails.\n\nThe fix is to carry the fact rather than re-derive it: either Hint keeps the\nKeyModifiers beside each label, or labels_for returns (KeyCode, KeyModifiers)\npairs and help.rs formats at the end. The second is tidier and is what\nwhich_key already does with key_label(code, *mods).\n\n## Steps to Reproduce\n1. Add a modifier to keymap.rs::key_label spelled with any prefix other than\n \"Ctrl-\" or \"Alt-\" -- \"Super-x\", say.\n2. Bind a command advertised by a mode's hint to that chord only.\n3. hint_line puts it in the status bar. Nothing fails.","acceptance_criteria":"compact stops parsing a string to learn something the keymap already\nknew: either Hint carries the KeyModifiers beside each label, or labels_for\nreturns (KeyCode, KeyModifiers) and help.rs formats last. Proved by adding a\nmodifier spelled differently and watching a test fail -- the point is that\ntoday it would not.","status":"open","priority":4,"issue_type":"task","owner":"el-github@elangley.org","created_at":"2026-09-20T18:45:36Z","created_by":"wave2-h-frontend","updated_at":"2026-09-20T18:45:36Z","labels":["area:shared-frontend"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"improvise-h074","title":"Two edges of App::resolve_view: a worse panic message, and a host-side DeleteView bypass","description":"Found by the adversarial review of PR #48 (improvise-jytm). Two small things, both\nabout what happens at the edges of the fallback improvise-jytm introduced.\n\n**The panic message got worse.** `App::resolve_view` falls back to\n`workbook.active_view` for a name the document has not got, then indexes\n`workbook.views[default]`. `Workbook::active_view()` — which several of these call\nsites used before — carries `.expect(\"active_view always names an existing view\")`.\n`IndexMap`'s `Index` impl panics with no such message. So a `Workbook` whose\n`active_view` names nothing (which only a hand-built or deserialized-from-elsewhere\nworkbook can be, since `create_view` and `delete_view` both maintain it) now fails\nwith a bare index panic where it used to name the invariant that was broken. Cheap\nfix: keep the `.expect` string.\n\n**A host-initiated `DeleteView` bypasses `adopt_deleted_views`.** That function runs\nin `Sessions::apply_model`, which is the door a *client's* command comes through. A\nhost driving `App::apply_effects` directly — through `Host::app_mut()` — deletes a\nview without any session mirror being moved off it.\n\n**Read the unreachability argument carefully, because it is weaker than it looks.**\nA first version of this ticket said `app_mut()` had one caller, in\n`improvise-ws-server/src/main.rs`, at startup, before any client had connected. That\nwas wrong, and review caught it. The one caller is\n`crates/improvise-worker-server/src/worker.rs:64`, inside `persist()` — which runs\n**after every accepted model command, with a client connected**. The bypass is still\nunreachable, but only because `persist()` happens to pass `effect::Save`, which\ndeletes no view. \"The one existing caller passes a harmless effect\" is a much thinner\nguarantee than \"nobody is connected yet\", and it is the exact shape of premise this\nrepo has been burned by: gotcha 47 records that the lost update `CheckedBatch` later\nfixed had shipped under an unreachability argument too.\n\nThe second is the interesting one to decide rather than to patch: it is the general\nshape of \"session bookkeeping that only the session door performs\". `Host::project`\nis already where every outbound path funnels, so whether the adoption belongs there\ninstead is the design question.\n\n## Steps to Reproduce\n\nThe panic message:\n\n1. Build a `Workbook` whose `active_view` names a view not in `views` (only reachable\n by constructing the struct directly — the constructors maintain the invariant).\n2. Call `App::view()`. It panics from `IndexMap`'s `Index` impl with no message,\n where `Workbook::active_view()` would have said which invariant broke.\n\nThe bypass:\n\n1. A client connects and switches to \"Second\".\n2. The host calls `host.app_mut().apply_effects(vec![Box::new(DeleteView(\"Second\"))])`\n — the same door `improvise-worker-server`'s `persist()` uses for `effect::Save`\n after every accepted model command.\n3. The session's mirror still names \"Second\". Nothing moved it and nothing told the\n client, because `Sessions::adopt_deleted_views` runs only in `apply_model`.\n","acceptance_criteria":"App::resolve_view's fallback index carries the invariant message Workbook::active_view() had. And a decision is recorded on whether session adoption belongs at the Sessions door or at Host::project — with a test for a host-initiated DeleteView if the answer moves it.","notes":"PR #76 (branch wave/command-effect-gaps) does HALF of this: App::resolve_view's fallback carries back the .expect(\"active_view always names an existing view\") message, guarded by a_default_that_names_nothing_says_which_invariant_broke in ui/app.rs. The SECOND half is untouched — whether a host-initiated DeleteView's session adoption belongs at the Sessions door or at Host::project is a decision about ui/project.rs and ui/host.rs, which were another agent's ground in this wave. Reported rather than guessed.","status":"in_progress","priority":4,"issue_type":"task","assignee":"claude-agent-cmd","owner":"el-github@elangley.org","created_at":"2026-09-20T18:33:43Z","created_by":"wave2-e-active-view","updated_at":"2026-09-21T03:42:18Z","started_at":"2026-09-21T03:27:56Z","lease_expires_at":"2026-09-21T03:32:56Z","heartbeat_at":"2026-09-21T03:27:56Z","labels":["area:active-view"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"improvise-x4wv","title":"assemble.sh's stamped guard under-reports multiple broken substitutions, and its comment reads as though it does not","description":"Found by injection during PR #47 (the landing-page inversion), by the WU-P agent.\n\nweb/assemble.sh's four 'stamped' checks exist because one grep over four files succeeds when ANY one of them matches, so three failed substitutions could still produce a successful assembly under set -e (found by Copilot on PR #24). The comment says each stamped reference 'is verified on its own and names itself when it is the one missing'.\n\nInjection result: breaking ONE substitution (a wrong sedi pattern for worker.js) produces exactly the claimed behaviour —\n\n assemble: .../app/worker.js is missing the cache-bust stamp: worker-pkg/improvise_worker_server.js?v=inj\n\nwith the app.js guard ahead of it passing and printing its match. Correct and isolated.\n\nBut 'stamped' calls exit 1 on the FIRST failure, so breaking BOTH app.js and worker.js reports app.js alone. The phrase 'when it is the one missing' is accurate for a single failure and misleading for a multiple one: a reader takes it as 'each missing stamp names itself'.\n\nThis FAILS SAFE — the build stops and a real problem is reported — so it is P4, not a correctness bug. It is filed because the prose is what a future agent will act on, and because the original guard was itself the product of a review finding about a check that passed when it should not have.\n\nDeliberately NOT fixed in PR #47: pre-existing, out of that unit, and reshaping stamped would rewrite a guard whose current form came out of the PR #24 finding. The cheap fix is probably the comment, not the code.\n\n## Steps to Reproduce\n\n1. In web/assemble.sh, corrupt the sedi patterns for BOTH app.js and worker.js.\n2. Run web/assemble.sh into a scratch dir.\n3. Only the app.js failure is reported; the worker.js one is never reached.","acceptance_criteria":"Either stamped reports every missing stamp before exiting, or its comment says plainly that it reports the FIRST failure only. Demonstrated by breaking two substitutions and seeing both named, or by the comment matching the single-report behaviour.","status":"closed","priority":4,"issue_type":"bug","assignee":"claude-agent-scripts","owner":"el-github@elangley.org","created_at":"2026-09-20T18:14:57Z","created_by":"orchestrator","updated_at":"2026-09-20T23:35:12Z","started_at":"2026-09-20T23:22:06Z","closed_at":"2026-09-20T23:35:12Z","close_reason":"fixed in PR #69, each proved by injection","dependency_count":0,"dependent_count":0,"comment_count":0} diff --git a/prolog-sessions/kb.pl b/prolog-sessions/kb.pl index ee9274bb..f5eea1ec 100644 --- a/prolog-sessions/kb.pl +++ b/prolog-sessions/kb.pl @@ -283,6 +283,10 @@ doctrine(a_guard_seeded_on_agreement_cannot_detect_drift,'lockstep.rs records co doctrine(a_substitution_is_a_defect_at_every_site_that_makes_it,'improvise-e5pf was filed as one command folded without a resolution. A sweep found the real fault is ONE SUBSTITUTION AT THREE DECISION POINTS: wherever the host asks layout.is_records_mode(), the client asks state.view.mode.is_records(). The wire command is only how the client REACHES the bad state; the substitution is why the bad state does damage, and it decides whether enriching the projection is even sufficient. Worse, the first site inverts its own failure mode: commit() is written to DECLINE a records commit it cannot compose, and with the mode wrong it does not decline -- it composes a typed_cell_command and a Basis claim against a records grid whose columns are [coords, +cat, measures, +], which is a WRONG WRITE rather than the intended black hole. When a ticket names a symptom, grep for the PROXY the symptom rests on; a proxy used once is used everywhere the same question is asked.'). doctrine(comparing_a_function_against_itself_tests_nothing,'improvise-jmjz fused three grid passes into one and guarded the width half with \`one_row.col_widths == compute_col_widths(model, &app.layout, false, 0)\`. But compute_col_widths IS grid_body with an empty window -- the same function twice -- so windowing the widths degrades BOTH SIDES IDENTICALLY (both collapse to the MIN_COL_WIDTH clamp) and the assertion cannot fail. Injecting \`if drawn { widen(..) }\` left 1476 tests passing. The fixture hid it a second way: the column header \"Revenue\" is wider than the value \"300021\", so the width assertion passed on the header alone. A check whose two sides share an implementation is a tautology wearing the shape of a comparison -- and a consolidation is exactly when one gets written, because the second implementation has just been deleted. The discriminating fixture puts the widest DATA value outside the window and makes it exceed both the header and the total (values 1, 888888888, -888888888: the total cancels to 1.0 so it cannot supply the answer).'). doctrine(a_negative_result_is_only_as_good_as_the_setup_that_produced_it,'PR #92 reported that a none_cats hazard was \"argued and unobserved at this shape\" after widening a fixture to probe it. Printing the actual axes showed Product on COLUMN in every view -- add_category defaults it there and nothing moved it -- so none_cats was {_Index, _Dim} identically everywhere and THE STATED INPUT TO THE HAZARD WAS ABSENT. The path was unprobed, reported as a negative result. This is an_injection_that_does_not_fail_names_an_untested_path applied to a SETUP rather than to an injection, and it is worse, because a recorded negative stops the next person looking. Before writing \"probed and not reproduced\", ASSERT THE PRECONDITION IN THE TEST -- print or assert the condition the hazard needs, so a fixture that drifted out of the shape fails loudly instead of reporting absence.'). +doctrine(the_elsewhere_path_is_only_as_correct_as_the_inputs_it_can_rebuild,'project::grid_in projects a view the host is NOT on. Twice now it has been wrong because it silently inherited an input computed for the host view. improvise-t6db was the first: it built with GridLayout::new, which skips prune_empty, so the other client grid never pruned -- one line to fix. improvise-ac2z is the second and is NOT local: App::rebuild_layout fills Model::formula_cache under the HOST view none_cats, and grid_in builds another view layout against that stale cache, so Total cells are keyed without the coordinate the other view puts on an axis, the lookup finds nothing, and prune_empty drops the formula columns entirely. ABSENT COLUMNS, not wrong numbers. The fix is not local because grid_in takes &App while recompute_formulas needs &mut Model. The general rule: when a function serves a context other than the one its inputs were computed for, enumerate those inputs and ask of EACH whether it was derived for the caller or for the subject -- a green suite says nothing, because every fixture where host and subject agree is vacuous for this class.'). +doctrine(a_conflicting_pr_runs_no_ci_because_it_has_no_merge_ref,'A pull_request workflow runs against refs/pull/N/merge, and GitHub cannot build that ref for a CONFLICTING pull request -- so NO RUN IS EVER CREATED and the PR simply shows zero checks. That is indistinguishable from a dropped webhook if you only look at the runs list. PR #92 sat with zero checks across two pushes and a close/reopen; the cause was that PR #91 had merged and both had appended to the same four context/ files. DIAGNOSIS ORDER: before theorising about event delivery, ask \`gh pr view N --json mergeable\` -- one call, and it answers first. The orchestrator ran a genuinely good control (push to another PR, watch a run appear in five seconds) which correctly proved delivery works repo-wide, and then drew the WRONG conclusion from a sound experiment, because the hypothesis space had two entries and the truth was a third. A control discriminates only between the hypotheses you listed.'). +doctrine(no_checks_reported_is_a_merge_state_symptom,'A pull_request-triggered workflow builds refs/pull/N/merge, and a CONFLICTING pull request HAS NO MERGE REF, so no run is ever created -- \"no checks reported\" then looks IDENTICAL to a dropped webhook. Measured on PR #92, 2026-09-21: 34 minutes of polling actions/runs?head_sha=, a close/reopen and an empty commit all produced zero runs; one git merge main produced a run in seconds. ASK gh pr view --json mergeable,mergeStateStatus FIRST -- one call answers what no amount of polling the runs API can distinguish. Both the agent and the orchestrator hypothesised a dropped event and a stalled delivery; NEITHER LISTED THE THIRD CASE, and the orchestrator control -- pushing to another PR and watching a run appear in five seconds -- is consistent with all three. A control discriminates only between the hypotheses you listed.'). +doctrine(grep_over_git_diff_is_silently_empty_under_an_external_differ,'This repo configures diff.external = difftastic, which prints a SIDE-BY-SIDE format carrying no unified-diff markers. So \`git diff | grep \"^-\"\` matches nothing REGARDLESS OF CONTENT, and an agent verifying \"nothing was dropped in the merge\" read three consecutive empty outputs as three clean bills. Verified by the orchestrator: on a file with one real deletion, git diff | grep -c \"^-\" gives 0 and git diff --no-ext-diff | grep -c \"^-\" gives 1. Use --no-ext-diff in any programmatic read of a diff, or --numstat / --name-only, which are unaffected. The general shape, and it is the sharpest form of check_the_checker: A CHECK THAT RETURNS EMPTY FOR THE WRONG REASON IS INDISTINGUISHABLE FROM ONE THAT PASSES -- and an empty result is the one outcome nobody investigates.'). work_done('improvise-8tk',bug_fix,'ZZ/:wq quit even when the save failed, silently discarding the model. Save now aborts the effect batch on failure; save_to_current_path/1 returns Result; three tests in ui/effect.rs.'). work_done('improvise-907',bug_fix,'Import wizard step machine redesign: validation_error() gate at ReviewProposals (no-category and over-limit), history-stack previous() bound to Left on every step, and Done made a real confirmation step holding the built workbook. Superseded improvise-gfq, improvise-o1c, improvise-c6j.'). work_done('improvise-6sq',wont_do,'Merging EditOrDrill and EnterEditAtCursorCmd into one EnterEdit{mode} is not safe: they differ in WHEN the cell value is read, not in a mode parameter. Added ui/app.rs::tests::i_and_o_diverge_on_an_empty_records_grid as executable evidence.').