diff --git a/.beads/issues.jsonl b/.beads/issues.jsonl index 1e0ef61b..b9104a29 100644 --- a/.beads/issues.jsonl +++ b/.beads/issues.jsonl @@ -1,4 +1,4 @@ -{"_type":"issue","id":"improvise-bpq2","title":"Client-side key resolution: what must the projections carry?","description":"USER RULING (2026-09-19): keys are converted into commands IN THE BROWSER; the keymap is a client-only concept the server must not know. Follows from the standing principle -- the model is shared, the UI is not, and a keymap is UI. It also buys something real: two clients may have DIFFERENT bindings over the same document, which is impossible if the server owns the keymap.\n\nThis inverts improvise-gsw conclusion (which had keys travelling upstream unresolved) and closes the keypress-door ticket. The work is now to make client-side resolution POSSIBLE.\n\nTHE PROBLEM TO SOLVE. A binding names an INTENT resolved against a CmdContext -- Model, GridLayout, cursor, yanked cell, expanded cats, mode. A Command is the RESOLVED MUTATION, e.g. SetSelected { row, col }. A thin client holds ViewState plus the widget caches. So for each binding ask: can a client holding only that state compose the Command? Where the answer is no, the fix is to ENRICH THE PROJECTION, not to move resolution to the server.\n\nKNOWN GAPS, from the gsw agent own analysis of what it could not resolve:\n * CURSOR MOVEMENT -- GridCache is indexed by position WITHIN THE VIEWPORT, so a client cannot distinguish \"last visible row\" from \"last row\" and cannot clamp. It needs the total row/col count, or an explicit \"can I move further\" fact.\n * PANEL CURSOR MOVEMENT -- move-panel-cursor walks a flattening that depends on expanded_cats. Either the cache carries the flattened list, or the client re-derives it -- and re-deriving is how two ends drift.\n * MODIFIED CHARACTERS -- KeyPattern::AnyChar matches a Char under ANY modifiers on the host. A client mirroring that is fine now that the client owns the keymap outright; the old constraint (local resolution may only ever be a strict subset) DISSOLVES once there is no host keymap to be a subset of.\n\nMETHOD: enumerate the bindings, not the commands -- the keymap is the thing being made client-side, so walk it. doctrine(binding_dump_diff) already says most bindings have no direct test, so a green suite is not evidence here either.\n\nALSO IN SCOPE: move ClientMessage/ServerMessage into improvise-protocol. The vocabulary is there and the framing that says which a JSON blob is, is not; the ws-server will want the same envelopes and two crates should not define them.\n\nBlocks improvise-1ey.","notes":"\n--- THE COMPOSABILITY LIST (improvise-q08 agent, 2026-09-19) ---\nWhich Commands a thin client holding ViewState + the five widget caches CANNOT compose. This is the real content of this ticket.\n\nHARD BLOCKERS:\n1. SetCell { key: CellKey } and ClearCell(CellKey) -- THE BIG ONE, and it blocks\n the most basic action there is. A client sees a grid of LABEL STRINGS indexed\n by viewport position. A CellKey is the full sorted coordinate tuple: every Row\n and Column category, plus every Page AND Filter selection (gotcha 27). Nothing\n in GridCache says which category a row label belongs to; nested row axes\n flatten several categories into one label; the page/filter constraints are not\n projected at all; and records mode encodes a COLUMN INDEX in _Dim (gotcha 1).\n There is no client-side derivation. NOTE THIS BLOCKS improvise-1ey DIRECTLY --\n its stated scope is \"type a number into a cell and see the value update\".\n2. ToggleGroup { category, group } and AddItemInGroup -- CategoryRow.items is a\n flat Vec\u003cString\u003e; groups, group membership and collapsed state are not\n projected. The server does the cat_tree flattening.\n3. RemoveFormula { target, target_category } -- FormulaCache is Vec\u003cString\u003e of RAW\n text. The client would have to re-parse to get target (dragging\n improvise-formula into a thin client), and target_category is simply absent, so\n a formula written [CustomCat] cannot be addressed at all.\n4. LoadModel(PathBuf) / ImportJsonHeadless -- paths name documents in the HOSTs\n Storage (gotcha 2) and nothing projects a listing, so a client cannot compose a\n path it has reason to believe exists.\n5. ApplyAndClearDrill / AcceptWizardModel -- nullary so constructible, but nothing\n projects drill or wizard state, so a client cannot know when either is\n meaningful. (Both are already off the wire per improvise-3x3u.)\n\nCOMPOSABLE BUT TRAPPED:\n* SetSelected { row, col } is ABSOLUTE; GridCache is VIEWPORT-RELATIVE. The client\n must add row_offset/col_offset from its own ViewState -- it has them, but the two\n index spaces are not distinguished by any type and getting it wrong is silent.\n* The grids TOTAL row/column count is not projected, so a client cannot clamp its\n cursor or size a scrollbar.\n\nFINE TODAY: SetAxis/CycleAxis/TransposeAxes/TogglePruneEmpty/SortData/\nCleanEmptyRecords, AddCategory/RemoveCategory, AddItem/RemoveItem/HideItem/\nShowItem/SetPageSelection WHEN DRIVEN FROM THE CATEGORY PANEL (not from the grid\ncursor -- same reason as 1), CreateView/DeleteView/SwitchView/ViewBack/ViewForward,\nAddFormula, SetNumberFormat (though the CURRENT format is in no cache, so it cannot\nbe pre-filled), and the whole view half.\n\nSUGGESTED ORDER: item 1 first and alone -- it is what 1ey needs and it is a\nprojection design question (how does a client name a cell?), not a plumbing one.\n\n--- ADR 002 ACCEPTED (user, 2026-09-19): see context/adr/002-naming-a-cell.md ---\nA projected cell carries an OPAQUE SURROGATE KEY, and its ABSENCE means the\ncell is an aggregate:\n CellDisplay { text, numeric, error, key: Option\u003cCellId\u003e }\nSome -\u003e a stored cell this client may set, named by a token it echoes back.\nNone -\u003e an aggregate: drill, do not set.\n\nWHY THE NULLABILITY IS THE POINT: a client cannot compose SetCell for an\naggregate because there is NO KEY TO PUT IN IT. That is make-invalid-states-\nunrepresentable (design-principles 3), strictly stronger than a server-side\nrejection. It also mirrors a fork the model ALREADY makes -- EditOrDrill in\ncmd/mode.rs branches on exactly this -- so the wire projects an existing\ndistinction rather than inventing one. And it follows the precedent\nCellDisplay::numeric set, whose doc says the server decides \"because it is the\nside that knows\".\n\n_INDEX WAS CONSIDERED AND REJECTED, and the reason is worth keeping: right\nontology, unsafe representation. CategoryKind::VirtualIndex is documented as\n\"Items are 0, 1, ... N where N = number of matching cells\" -- POSITIONAL over\nthe currently matching set, not an identity. _Index = \"3\" names a different\nrecord after a filter change, an insert or a delete, and a different one again\nin another view. A client echoing a stale _Index writes to the WRONG CELL,\nSILENTLY -- worse than a surrogate that fails to resolve, and the same hazard\nclass as viewport-relative-vs-absolute and the stale cursor under a renamed key.\n\nBUT KEEP WHAT IT POINTS AT: the surrogate should name the RECORD, not the cell.\nA record is the coord tuple minus _Measure (record_row_identity), which is\nalready ADR 001 unit of atomicity and already what record_versions is keyed by.\nAddress a cell as (record surrogate, measure) and ONE IDENTITY serves both\naddressing and staleness, instead of each ADR inventing its own.\n\nOPEN, for the implementer to decide with evidence: hash vs interned id (the\ncodebase interns Symbols already); how the server resolves a surrogate back --\nnote that REGENERATING FROM THE CURRENT LAYOUT means a stale surrogate simply\nfails to resolve, which IS ADR 001 stale-basis rejection at cell granularity,\nso the two compose; and what DRILL addresses with, which this does not answer.","status":"open","priority":1,"issue_type":"feature","owner":"el-github@elangley.org","created_at":"2026-09-19T23:42:49Z","created_by":"Edward Langley","updated_at":"2026-09-19T23:53:49Z","dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"improvise-bpq2","title":"Client-side key resolution: what must the projections carry?","description":"USER RULING (2026-09-19): keys are converted into commands IN THE BROWSER; the keymap is a client-only concept the server must not know. Follows from the standing principle -- the model is shared, the UI is not, and a keymap is UI. It also buys something real: two clients may have DIFFERENT bindings over the same document, which is impossible if the server owns the keymap.\n\nThis inverts improvise-gsw conclusion (which had keys travelling upstream unresolved) and closes the keypress-door ticket. The work is now to make client-side resolution POSSIBLE.\n\nTHE PROBLEM TO SOLVE. A binding names an INTENT resolved against a CmdContext -- Model, GridLayout, cursor, yanked cell, expanded cats, mode. A Command is the RESOLVED MUTATION, e.g. SetSelected { row, col }. A thin client holds ViewState plus the widget caches. So for each binding ask: can a client holding only that state compose the Command? Where the answer is no, the fix is to ENRICH THE PROJECTION, not to move resolution to the server.\n\nKNOWN GAPS, from the gsw agent own analysis of what it could not resolve:\n * CURSOR MOVEMENT -- GridCache is indexed by position WITHIN THE VIEWPORT, so a client cannot distinguish \"last visible row\" from \"last row\" and cannot clamp. It needs the total row/col count, or an explicit \"can I move further\" fact.\n * PANEL CURSOR MOVEMENT -- move-panel-cursor walks a flattening that depends on expanded_cats. Either the cache carries the flattened list, or the client re-derives it -- and re-deriving is how two ends drift.\n * MODIFIED CHARACTERS -- KeyPattern::AnyChar matches a Char under ANY modifiers on the host. A client mirroring that is fine now that the client owns the keymap outright; the old constraint (local resolution may only ever be a strict subset) DISSOLVES once there is no host keymap to be a subset of.\n\nMETHOD: enumerate the bindings, not the commands -- the keymap is the thing being made client-side, so walk it. doctrine(binding_dump_diff) already says most bindings have no direct test, so a green suite is not evidence here either.\n\nALSO IN SCOPE: move ClientMessage/ServerMessage into improvise-protocol. The vocabulary is there and the framing that says which a JSON blob is, is not; the ws-server will want the same envelopes and two crates should not define them.\n\nBlocks improvise-1ey.","notes":"\n--- THE COMPOSABILITY LIST (improvise-q08 agent, 2026-09-19) ---\nWhich Commands a thin client holding ViewState + the five widget caches CANNOT compose. This is the real content of this ticket.\n\nHARD BLOCKERS:\n1. SetCell { key: CellKey } and ClearCell(CellKey) -- THE BIG ONE, and it blocks\n the most basic action there is. A client sees a grid of LABEL STRINGS indexed\n by viewport position. A CellKey is the full sorted coordinate tuple: every Row\n and Column category, plus every Page AND Filter selection (gotcha 27). Nothing\n in GridCache says which category a row label belongs to; nested row axes\n flatten several categories into one label; the page/filter constraints are not\n projected at all; and records mode encodes a COLUMN INDEX in _Dim (gotcha 1).\n There is no client-side derivation. NOTE THIS BLOCKS improvise-1ey DIRECTLY --\n its stated scope is \"type a number into a cell and see the value update\".\n2. ToggleGroup { category, group } and AddItemInGroup -- CategoryRow.items is a\n flat Vec\u003cString\u003e; groups, group membership and collapsed state are not\n projected. The server does the cat_tree flattening.\n3. RemoveFormula { target, target_category } -- FormulaCache is Vec\u003cString\u003e of RAW\n text. The client would have to re-parse to get target (dragging\n improvise-formula into a thin client), and target_category is simply absent, so\n a formula written [CustomCat] cannot be addressed at all.\n4. LoadModel(PathBuf) / ImportJsonHeadless -- paths name documents in the HOSTs\n Storage (gotcha 2) and nothing projects a listing, so a client cannot compose a\n path it has reason to believe exists.\n5. ApplyAndClearDrill / AcceptWizardModel -- nullary so constructible, but nothing\n projects drill or wizard state, so a client cannot know when either is\n meaningful. (Both are already off the wire per improvise-3x3u.)\n\nCOMPOSABLE BUT TRAPPED:\n* SetSelected { row, col } is ABSOLUTE; GridCache is VIEWPORT-RELATIVE. The client\n must add row_offset/col_offset from its own ViewState -- it has them, but the two\n index spaces are not distinguished by any type and getting it wrong is silent.\n* The grids TOTAL row/column count is not projected, so a client cannot clamp its\n cursor or size a scrollbar.\n\nFINE TODAY: SetAxis/CycleAxis/TransposeAxes/TogglePruneEmpty/SortData/\nCleanEmptyRecords, AddCategory/RemoveCategory, AddItem/RemoveItem/HideItem/\nShowItem/SetPageSelection WHEN DRIVEN FROM THE CATEGORY PANEL (not from the grid\ncursor -- same reason as 1), CreateView/DeleteView/SwitchView/ViewBack/ViewForward,\nAddFormula, SetNumberFormat (though the CURRENT format is in no cache, so it cannot\nbe pre-filled), and the whole view half.\n\nSUGGESTED ORDER: item 1 first and alone -- it is what 1ey needs and it is a\nprojection design question (how does a client name a cell?), not a plumbing one.\n\n--- ADR 002 ACCEPTED (user, 2026-09-19): see context/adr/002-naming-a-cell.md ---\nA projected cell carries an OPAQUE SURROGATE KEY, and its ABSENCE means the\ncell is an aggregate:\n CellDisplay { text, numeric, error, key: Option\u003cCellId\u003e }\nSome -\u003e a stored cell this client may set, named by a token it echoes back.\nNone -\u003e an aggregate: drill, do not set.\n\nWHY THE NULLABILITY IS THE POINT: a client cannot compose SetCell for an\naggregate because there is NO KEY TO PUT IN IT. That is make-invalid-states-\nunrepresentable (design-principles 3), strictly stronger than a server-side\nrejection. It also mirrors a fork the model ALREADY makes -- EditOrDrill in\ncmd/mode.rs branches on exactly this -- so the wire projects an existing\ndistinction rather than inventing one. And it follows the precedent\nCellDisplay::numeric set, whose doc says the server decides \"because it is the\nside that knows\".\n\n_INDEX WAS CONSIDERED AND REJECTED, and the reason is worth keeping: right\nontology, unsafe representation. CategoryKind::VirtualIndex is documented as\n\"Items are 0, 1, ... N where N = number of matching cells\" -- POSITIONAL over\nthe currently matching set, not an identity. _Index = \"3\" names a different\nrecord after a filter change, an insert or a delete, and a different one again\nin another view. A client echoing a stale _Index writes to the WRONG CELL,\nSILENTLY -- worse than a surrogate that fails to resolve, and the same hazard\nclass as viewport-relative-vs-absolute and the stale cursor under a renamed key.\n\nBUT KEEP WHAT IT POINTS AT: the surrogate should name the RECORD, not the cell.\nA record is the coord tuple minus _Measure (record_row_identity), which is\nalready ADR 001 unit of atomicity and already what record_versions is keyed by.\nAddress a cell as (record surrogate, measure) and ONE IDENTITY serves both\naddressing and staleness, instead of each ADR inventing its own.\n\nOPEN, for the implementer to decide with evidence: hash vs interned id (the\ncodebase interns Symbols already); how the server resolves a surrogate back --\nnote that REGENERATING FROM THE CURRENT LAYOUT means a stale surrogate simply\nfails to resolve, which IS ADR 001 stale-basis rejection at cell granularity,\nso the two compose; and what DRILL addresses with, which this does not answer.\n\n--- SURROGATE DECIDED (user, 2026-09-19): hash of the normalized record, maintained as an index ---\nNOT a hash of the cell -- of the RECORD (the coord tuple minus _Measure), which\nis already ADR 001 unit of atomicity and already what record_versions is keyed\nby. A cell is (record hash, measure).\n\nWHY: it makes stale edits cheap to identify, three ways at once. The surrogate\na client already holds IS the lookup key for its own version, so record_versions\nbecomes HashMap\u003cRecordHash, u64\u003e -- fixed size, no allocation, no CellKey\nreconstruction on the hot path. A basis is a list of (record hash, version)\npairs: compact on the wire, O(1) to check. And a record that no longer exists\nsimply fails to resolve, so that whole class of staleness needs no version\ncomparison at all.\n\nIt is well-defined because CellKey ALREADY has a canonical form -- CellKey::new\nsorts by category name and equality/hashing rely on it (gotcha 5). Normalized\nmeans that form with _Measure removed; the normalization is a property the type\nguarantees, not a new convention.\n\nRESOLUTION IS A MAINTAINED INDEX, IN DataStore -- a hash is one-way, and\nregenerating from the layout would be O(visible cells) per resolve AND would\nonly cover the visible window, while a client may hold a surrogate for a record\nthat scrolled away. DataStore is the right home: it already interns every name\ninto a SymbolTable and already maintains HashMap\u003c(Symbol,Symbol), HashSet\u003cInternedKey\u003e\u003e\nfor hot-path lookups. The RecordHash index is the same shape, updated at the same\npoints, and that existing (Symbol,Symbol) index is what makes INVALIDATION\ntractable -- when a category or item disappears, the records mentioning it are\nfindable without a scan.\n\nTWO THINGS THAT FALL OUT: the index and record_versions want to be ONE structure\n(RecordHash -\u003e (record, version)) since both are keyed the same and invalidated\ntogether, giving both ADRs their answer in one lookup. And structural vs data\nstaleness separate cleanly: does not resolve = the record is GONE (reject,\nnothing to write to); resolves with a differing version = the record CHANGED\n(reject as stale, offer force). Regenerating from the layout would have\nconflated those, because a record scrolled out of view would look identical to\none that no longer exists.\n\nTHREE SILENT-BREAKAGE TRAPS, each producing a WRONG WRITE rather than an error:\n * NOT DefaultHasher / RandomState -- randomly seeded PER PROCESS, so hashes\n would differ across a restart and between processes.\n * NOT #[derive(Hash)] as the definition -- Rust guarantees no stability across\n compiler versions or platforms. Hash an explicit BYTE SERIALIZATION of the\n sorted (category, item) pairs.\n * Pick the WIDTH BY CONSEQUENCE: a collision is a silent write to the wrong\n record, which is exactly why _Index was rejected. 64 bits hits birthday risk\n near 2^32 records; 128 bits costs 8 more bytes per VISIBLE cell (~8KB on a\n 50x20 viewport). Argue it, do not default to u64 for convenience.","status":"open","priority":1,"issue_type":"feature","owner":"el-github@elangley.org","created_at":"2026-09-19T23:42:49Z","created_by":"Edward Langley","updated_at":"2026-09-19T23:56:13Z","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"improvise-c4er","title":"[bug] There is no server door for a keypress; the asker gets no commands back","description":"Found by improvise-gsw (2026-09-19) and it is THE seam the browser MVP depends on. Blocks improvise-1ey.\n\nA browser client cannot resolve keys locally -- a binding names an INTENT resolved against a CmdContext while a Command is the RESOLVED MUTATION (see the improvise-gxi close reason and gotcha 38). So the client sends the raw keypress upstream as ClientMessage::Key { code, modifiers }.\n\nTHE GAP: Sessions::apply(app, id, cmd) takes a Command. ui::reduce::reduce_key(app, key) exists but returns only PROJECTIONS -- not the view-slice commands the ASKER must fold into its own ViewState. With keys travelling unresolved the host owes the asker back the commands its own keypress produced, and nothing returns them.\n\nNOTE this is NOT Applied::Diff.broadcast, which is what the OTHER clients fold (gotcha 36). The asker needs its own, and there is nowhere for them to come from.\n\nSUGGESTED SHAPE (from the gsw agent): Sessions::apply_key(app, id, key) -\u003e Applied, with the askers own view-slice commands in the result. Derive it rather than inventing -- doctrine(derive_the_enum_from_the_effects). The keypress is already KeyCode + KeyModifiers, which Command::SetTransientPrefix puts on the wire today; note improvise_protocol::KeyEvent itself does NOT derive Serialize/Deserialize, only its two components do, which is worth fixing at the same time.\n\nAlso from gsw: ClientMessage/ServerMessage envelopes currently live in the client crate because the VOCABULARY is in improvise-protocol but the FRAMING that says which a JSON blob is, is not. Moving them is a one-file lift and the ws-server will want the same ones -- do it here rather than letting two crates define them.","status":"closed","priority":1,"issue_type":"bug","owner":"el-github@elangley.org","created_at":"2026-09-19T23:40:37Z","created_by":"Edward Langley","updated_at":"2026-09-19T23:42:47Z","closed_at":"2026-09-19T23:42:47Z","close_reason":"WRONG PREMISE -- CLOSED AND INVERTED by a user ruling (2026-09-19): keys are converted into commands IN THE BROWSER, and the keymap is a CLIENT-ONLY concept the server must not know at all.\n\nThis ticket asked for a server door for a keypress (Sessions::apply_key). There must not be one. The reasoning follows from the standing principle -- the model is shared, the UI is not -- and A KEYMAP IS UI. If the server knows the keymap, two clients cannot have different bindings, and that is a thing this architecture should permit: a browser client with browser-native keys and a TUI with vim keys, over the same document.\n\nSo the gap this ticket named does not exist. The asker never needs its own commands handed back, because the asker COMPOSED them and already knows what it sent. ClientMessage::Key should not exist; a client sends Commands only. ui::reduce::reduce_key stays, but it belongs to the native TUI -- which is a client that happens to run in-process -- not to the server client-facing surface.\n\nTHE REAL WORK THIS DISPLACES INTO: a client must hold enough PROJECTED STATE to resolve a binding into a Command. That is a requirement on what the projections carry, not a reason to move resolution serverward. Filed separately.\n\nThe one part of this ticket that survives independently: ClientMessage/ServerMessage envelopes belong in improvise-protocol rather than the client crate -- the vocabulary is there, the framing is not. Refiled with the projection work.","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"improvise-acsn","title":"[bug] A document swap over the wire leaves the client's own cursors stale; nothing tells the host a rebase is owed","description":"Wave-4 review (2026-09-19), CONFIRMED by probe. Latent -- no host wires Sessions yet -- but newly reachable from the wire.\n\nModelCommand now lets a client send LoadModel / ImportJsonHeadless / AcceptWizardModel, the WholeWorkbook commands, for the first time. The SERVER mirrors self-heal: ClientSession::project reseeds on the document-generation bump, so the server is honest. But cursors live in the CLIENT own ViewState, and that is corrected only by a Snapshot -- whose only producer, Sessions::rebase, has NO NON-TEST CALLER in the tree (grep finds one hit, a test). Applied carries only { dirty, broadcast }, neither of which tells the host a rebase is owed.\n\nProbe (two sessions, MemoryFs, Model(LoadModel) where the incoming document active view is also named Default):\n generation 0 -\u003e 1, broadcast = None\n bystander mirror cursor before project = (3, 0)\n bystander mirror cursor after project = (0, 0)\nSo the server has it at (0,0) while the bystander client keeps cursors[Default] = (3,0) from the OUTGOING document -- the gotcha-24 cursor-vs-geometry mismatch that apply_switch own doc comment says this shape exists to prevent, now arriving through another client.\n\nFIX: give Applied a way to say the document was replaced -- carry the generation, or a rebase flag -- so a host CANNOT forget. Pre-existing in kind (a keypress-driven LoadModel had the same gap), which is why the fix belongs on Applied rather than on the command.","status":"closed","priority":1,"issue_type":"bug","owner":"el-github@elangley.org","created_at":"2026-09-19T22:18:02Z","created_by":"Edward Langley","updated_at":"2026-09-19T22:24:25Z","closed_at":"2026-09-19T22:24:25Z","close_reason":"Applied went from a record to an enum: Applied::Diff { dirty, broadcast } | Applied::Rebased { snapshots }. The enum IS the mechanism -- a host that wants the Dirty must match, and matching names the case where a Snapshot is owed. Changing the shape made the compiler name all five existing read sites, which is the point. Rebased carries no Dirty and no broadcast, because each Snapshot already carries a whole view slice plus the seed reproducing the new document; a diff against the old one would describe nothing that still exists. Sessions::apply_model snapshots document_generation before and after apply_command and calls rebase itself, exactly as apply_switch adopts the accepted view itself -- rebase gains its first non-test caller.\n\nTHE TRIGGER IS THE GENERATION, NOT THE VARIANT: a LoadModel on an unreadable path leaves the document alone, and reseeding on the variant would cost a client its cursors for a FAILED load. Both arms are tested per doctrine(one_arm_of_a_symmetric_rule...): a_command_that_leaves_the_document_in_place_is_not_a_rebase and a_load_that_fails_is_not_a_rebase. Injections both directions: never-rebase fails the swap test (left 3, right 0); always-rebase fails four tests including the failed-load arm.\n\nFIXTURE TRAP WORTH KNOWING, recorded in the repo-map: the first version of the test PASSED, because the existing other_document() renames the active view Default-\u003eSheet, so apply_model broadcast a SwitchView, the client folded it, and the stale cursor was read under a key nothing had written -- right by accident. A new fixture keeps the view NAME and changes only the shape, so there is nothing to hide behind.\n\nRejected: Applied { dirty, broadcast, rebase: bool } -- the host that forgets to call rebase is the host that forgets to read the flag; carrying the generation -- same problem plus it asks the host to remember the comparison; a hook on the LoadModel effect -- Sessions is unreachable from an effect, which is why the generation bump exists at all.","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"improvise-oi9b","title":"[bug] 24 of 28 model_effects mappings are unguarded; a wrong mapping passes the whole suite","description":"Wave-4 review (2026-09-19), CONFIRMED by injection. Test gap, not a live bug -- the mapping is correct today.\n\nui/reduce.rs::model_effects maps each ModelCommand back to its effect. Injecting wrong-but-still-Model mappings left cargo test --workspace GREEN:\n ModelCommand::HideItem -\u003e effect::ShowItem PASSES\n ModelCommand::CreateView -\u003e effect::DeleteView PASSES\n ModelCommand::AddItem -\u003e effect::RemoveItem PASSES\n ModelCommand::ViewBack -\u003e effect::ViewForward fails (caught incidentally by the project.rs lockstep test)\n\nWhy: every_model_command_names_an_effect_that_declares_a_model_change cannot distinguish two effects of the SAME EffectKind, and a_model_command_lands_where_its_keypress_lands drives only three variants (TransposeAxes, TogglePruneEmpty, AddCategory). That leaves 24 variants whose mapping nothing checks -- and every inverse pair (Hide/Show, Add/Remove item, Create/Delete view, SetCell/ClearCell) is exactly the case the surviving tests cannot separate.\n\nFAILURE SCENARIO: a rename or re-order during a future refactor silently re-points a wire command at its INVERSE. A client sending Model(HideItem) un-hides instead, and because the Dirty is identical every other client is told a consistent lie.\n\nFIX: extend a_model_command_lands_where_its_keypress_lands to a table over every_model_command(), or -- cheaper and total -- assert the variant name equals the effect type name. The mapping is name-for-name by construction, so that assertion covers all 28 at once. This is the same class the author already fixed once, when a one-category fixture could not tell TransposeAxes from CycleAxis.","status":"closed","priority":1,"issue_type":"bug","owner":"el-github@elangley.org","created_at":"2026-09-19T22:18:01Z","created_by":"Edward Langley","updated_at":"2026-09-19T22:24:24Z","closed_at":"2026-09-19T22:24:24Z","close_reason":"Guarded by name, covering all 28 at once. every_model_command_names_the_effect_of_the_same_name asserts each variant returns exactly one effect whose TYPE NAME equals the variant name -- read off the effect derived Debug, since Effect is a trait object and type_name is out of reach through it. The mapping is name-for-name by construction, so a variant whose effect must be named something else is a variant that stopped being derived: it should fail there and be argued about.\n\nThe gap was reproduced first: AddItem-\u003eRemoveItem, HideItem-\u003eShowItem and CreateView-\u003eDeleteView applied SIMULTANEOUSLY left cargo test --workspace at 918 passed, 0 failed -- three wire messages doing the exact inverse of what they say, suite fully green. Each injection was then re-run individually against the new test and each fails with the names printed.\n\nThe name READER is itself checked (the_effect_name_reader_reads_all_three_debug_shapes), because the delimiter after the identifier differs per Debug shape -- nothing, ( or space-brace -- and its own injection was verified to fail.\n\nRejected extending the behavioural test to a table over all 28: most variants have no single keypress to compare against, so you would invent one or drive the effect directly, at which point it stops being an independent yardstick. The behavioural test keeps its three genuinely key-driven cases and its two-category pivoted() fixture.","dependency_count":0,"dependent_count":0,"comment_count":0} diff --git a/context/adr/002-naming-a-cell.md b/context/adr/002-naming-a-cell.md index e812e45d..0ff7aa5a 100644 --- a/context/adr/002-naming-a-cell.md +++ b/context/adr/002-naming-a-cell.md @@ -58,13 +58,46 @@ such fact. These are deliberately unresolved; the implementer should decide them with evidence. -**What the surrogate *is*.** A hash of the `CellKey` is the obvious candidate: stateless, -stable across a reconnect, nothing to keep in sync. But a hash is one-way, so the server -still needs to get back to a `CellKey` — see below — and it needs enough width that a -collision is not a silent wrong-cell write. The alternative is an interned id, for which -this codebase has precedent: `DataStore` already interns names into `Symbol` through a -`SymbolTable`. That has no collision risk and can be small, at the cost of a table with a -lifecycle. +**What the surrogate is — DECIDED: a hash of the normalized record.** Not of the cell: +the record is the coordinate tuple minus `_Measure`, which is already ADR 001's unit of +atomicity and already what `record_versions` is keyed by. A cell is then +`(record hash, measure)`. + +The reason is that it makes **stale edits cheap to identify**, and it does so three ways +at once. The surrogate a client already holds *is* the lookup key for its own version, so +`record_versions` becomes `HashMap` — fixed-size, no allocation, no +`CellKey` reconstruction on the hot path. A basis is then a list of +`(record hash, version)` pairs, which is compact on the wire and O(1) to check. And a +record that no longer exists simply **fails to resolve**, so that whole class of +staleness needs no version comparison at all. One identity serves addressing and +staleness instead of each ADR carrying its own. + +It is well-defined because `CellKey` already has a canonical form: `CellKey::new` sorts +coordinates by category name and equality and hashing rely on it (gotcha 5). "Normalized" +means that canonical sorted form with `_Measure` removed — the normalization is a +property the type already guarantees, not a new convention to maintain. + +An interned id was rejected: it would need a table with a lifetime nobody has specified, +and it buys only collision-freedom, which width buys more cheaply. + +**Three ways this silently breaks, all avoidable.** They are listed because each produces +a *wrong write* rather than an error, which is the failure mode this ADR exists to +prevent: + +- **Do not use `DefaultHasher` or anything built on `RandomState`.** `std`'s default + `HashMap` hasher is randomly seeded **per process**, so hashes would differ across a + server restart and between any two processes. Specify a fixed hash (a keyed SipHash + with a constant key, or FxHash/xxHash/blake3) and write the choice down. +- **Do not use `#[derive(Hash)]` as the definition.** Rust gives no stability guarantee + for derived `Hash` across compiler versions or platforms. Hash an **explicit byte + serialization** of the sorted `(category, item)` pairs, so the surrogate is defined by + the format rather than by whatever the compiler did. +- **Pick the width by consequence, not by convenience.** A collision is a silent write to + the wrong record — precisely why `_Index` was rejected. 64 bits reaches birthday risk + around 2³² records, which a pivot tool will not see; 128 bits costs 8 more bytes per + *visible* cell, so on a 50×20 viewport that is 8 KB a frame. The cost is small and the + failure is silent, so argue the width explicitly rather than defaulting to `u64` because + it is convenient. **`_Index` was considered and rejected — right ontology, unsafe representation.** It is the coordinate that identifies a *record*, and a record is exactly the thing that is @@ -84,14 +117,37 @@ record, not the cell.** A record is the coordinate tuple minus `_Measure` then one identity serves both addressing *and* staleness, and the two ADRs share a mechanism instead of each inventing one. -**How the server resolves a surrogate back to a `CellKey`.** Two shapes. A table -populated as it projects — simple, but it is per-session state with a lifetime nobody has -specified. Or **regenerate from the current layout on demand**: the server already has -the `GridLayout`, so it can recompute the visible keys and their surrogates and look the -incoming one up. That is attractive for a reason beyond avoiding state: **a surrogate -minted against a layout that has since changed simply fails to resolve** — which is -exactly ADR 001's stale-basis rejection, arriving at cell granularity for free. The two -designs compose rather than merely coexist. +**How the server resolves a surrogate back — DECIDED: a maintained index, in +`DataStore`.** A hash is one-way, so resolution needs a reverse map, and regenerating it +from the layout on demand would be O(visible cells) per resolve *and* would only ever +cover the visible window — a client may legitimately hold a surrogate for a record that +has scrolled out of view. + +`DataStore` is the right home because it already does exactly this. It interns every +category and item name into a `SymbolTable`, and it already maintains a secondary index +`HashMap<(Symbol, Symbol), HashSet>` for hot-path lookups. A +`RecordHash → record` index is the same shape, updated at the same points, and the +existing `(Symbol, Symbol)` index is what makes invalidation tractable: when a category or +item disappears, the records that mention it are findable without a scan. + +Two consequences worth stating, because they fall out and are easy to miss. + +**The index and `record_versions` want to be one structure.** Both are keyed by the record +hash and both are invalidated at the same moments, so `RecordHash → (record, version)` +gives ADR 001's basis check and this ADR's resolution in a single lookup. + +**Structural and data staleness separate cleanly**, which is better than the +regenerate-on-demand shape would have given: + +| incoming surrogate | means | answer | +|---|---|---| +| does not resolve | the record is **gone** — a category or item it named was removed | reject; there is nothing to write to | +| resolves, version matches | the client's basis is current | apply | +| resolves, version differs | the record **changed** since the client read it | reject as stale (ADR 001), offer force | + +Regenerating from the layout would have conflated the first two rows, because a record +scrolled out of the viewport would have looked identical to a record that no longer +exists. **What drill uses.** If `key` is `None` the client wants to *drill*, and this ADR gives it nothing to drill *with*. Whether an aggregate needs its own surrogate, or whether