Files
improvise/site/docs/adr/002-naming-a-cell.html
T
Edward Langley 787e354b9b doc-site: name context/code/pr-review.md as an omission
The `build` job fails on main and on every branch merged from it:

  doc-site: context/code/pr-review.md is neither published nor a named omission

NOT a merge loss -- scripts/build-doc-site.mjs in this branch is
byte-identical to main's, and neither lists the file. PR #96 added
context/code/pr-review.md without a manifest line, which is exactly the
gate improvise-res9 added: a doc added without a line publishes nothing
and says nothing, so the generator refuses.

Omitted rather than published, and the line is cloud-agents.md's rather
than tooling.md's. tooling.md explains checkers a reader of the code will
meet; pr-review.md is about THIS REPOSITORY'S PROCESS -- where a Codex
review appears, how to tell it from our own agents' comments, why absence
is not approval. Nobody visiting a website is about to read a PR thread.

Both homes updated, because an omission is restated beside the manifest
AND in doc-site.md section 2, and the prose there counted them ("Neither
is referenced...") so it went stale at three.

This strays into scripts/, which is Wave N's ground. Flagged in the
hand-back; it is a one-line data entry either way if the orchestrator
would rather publish it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
2026-09-21 11:12:00 -07:00

83 lines
19 KiB
HTML
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
<!doctype html>
<html lang="en">
<head>
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width, initial-scale=1" />
<title>ADR 002 — Naming a cell — improvise docs</title>
<link rel="stylesheet" href="../style.css" />
</head>
<body>
<div class="shell">
<nav class="side"><a class="brand" href="../index.html">improvise docs</a><input type="checkbox" id="navtoggle" class="navtoggle" /><label class="navsummary" for="navtoggle">Pages</label><div class="navlinks"><ul><li><a href="../../index.html">← Project home</a></li><li><a href="../../app/">Browser demo</a></li><li><a href="https://github.com/fiddlerwoaroof/improvise">Source on GitHub</a></li></ul><h3>Start here</h3><ul><li><a href="../design-notes.html">Design notes</a></li><li><a href="../design-principles.html">Design principles</a></li><li><a href="../repo-map.html">Repository map</a></li></ul><h3>Decision records</h3><ul><li><a href="../adr/001-concurrent-edits.html">ADR 001 — Concurrent edits</a></li><li><a href="../adr/002-naming-a-cell.html" class="here">ADR 002 — Naming a cell</a></li><li><a href="../adr/003-live-backed-models.html">ADR 003 — Live-backed models</a></li></ul><h3>The arguments, by area</h3><ul><li><a href="../code/core.html">improvise-core</a></li><li><a href="../code/formula-and-io.html">improvise-formula and improvise-io</a></li><li><a href="../code/protocol.html">improvise-protocol</a></li><li><a href="../code/command.html">improvise-command</a></li><li><a href="../code/clients.html">The clients</a></li><li><a href="../code/tui.html">The TUI</a></li><li><a href="../code/gp.html">gp-tree and improvise-gp</a></li><li><a href="../code/kb-tool.html">The kb tool</a></li><li><a href="../code/tooling.html">The repository's own checkers</a></li></ul><h3>Test evidence</h3><ul><li><a href="../test-evidence/core.html">improvise-core</a></li><li><a href="../test-evidence/formula-and-io.html">improvise-formula and improvise-io</a></li><li><a href="../test-evidence/protocol.html">improvise-protocol</a></li><li><a href="../test-evidence/command.html">improvise-command</a></li><li><a href="../test-evidence/clients.html">The clients</a></li><li><a href="../test-evidence/tui.html">The TUI</a></li><li><a href="../test-evidence/gp.html">gp-tree and improvise-gp</a></li><li><a href="../test-evidence/tooling.html">The repository's own gates</a></li></ul><h3>About this site</h3><ul><li><a href="../doc-site.html">About this site</a></li></ul><h3>Reference</h3><ul><li><a href="../tickets.html">Ticket index</a></li></ul></div></nav>
<main>
<p class="src">Source: <a href="https://github.com/fiddlerwoaroof/improvise/blob/main/context/adr/002-naming-a-cell.md">context/adr/002-naming-a-cell.md</a></p><details class="toc" open><summary>On this page</summary><ul><li class="l1"><a href="#adr-002-how-a-client-names-a-cell">ADR 002 — How a client names a cell</a></li><li class="l2"><a href="#context">Context</a></li><li class="l2"><a href="#decision">Decision</a></li><li class="l2"><a href="#decisions-made-along-the-way-and-the-traps-in-them">Decisions made along the way, and the traps in them</a></li><li class="l2"><a href="#consequences">Consequences</a></li><li class="l2"><a href="#extension-2026-09-20-two-more-kinds-by-the-same-reasoning">Extension (2026-09-20) — two more kinds, by the same reasoning</a></li></ul></details><h1 id="adr-002-how-a-client-names-a-cell">ADR 002 — How a client names a cell</h1>
<p><strong>Status:</strong> accepted (2026-09-19), extended (2026-09-20) · <strong>Decided by:</strong> the user · <strong>Blocks:</strong> <a class="tid" href="../tickets.html#improvise-1ey" title="Browser frontend MVP: end-to-end working demo">improvise-1ey</a> · <strong>Extended by:</strong> <a class="tid" href="../tickets.html#improvise-ir7l" title="[bug] A formula cell is offered as editable and silently reverts">improvise-ir7l</a>, <a class="tid" href="../tickets.html#improvise-poia" title="The records-mode + column has no projected identity">improvise-poia</a></p>
<h2 id="context"><a class="anchor" href="#context">#</a>Context</h2>
<p><code>improvise-q08</code> established that a thin client <strong>cannot compose <code>SetCell</code></strong>, and it is the gap that blocks the browser MVP, whose stated scope is &quot;type a number into a cell and see the value update&quot;.</p>
<p>A client sees a grid of <em>label strings</em> indexed by viewport position. A <code>CellKey</code> is the full sorted coordinate tuple — every Row and Column category, <strong>plus every Page and Filter selection</strong> (gotcha 27). Nothing in <code>GridCache</code> says which category a row label belongs to; nested row axes flatten several categories into one label; page and filter constraints are not projected at all; and records mode encodes a <em>column index</em> in <code>_Dim</code> (gotcha 1). There is no client-side derivation, and projecting the coordinate space would make every client learn conventions the server exists to own.</p>
<h2 id="decision"><a class="anchor" href="#decision">#</a>Decision</h2>
<p><strong>Every projected cell carries an opaque surrogate identity, and the <em>kind</em> of identity says what may be done to it.</strong></p>
<pre><code>pub struct CellDisplay {
pub text: String,
pub numeric: bool,
pub error: bool,
pub id: CellId,
}
pub enum CellId {
/// A stored cell. `SetCell` takes this and nothing else.
Record(RecordHash),
/// An aggregate over stored cells. `Drill` takes this and nothing else.
Aggregate(AggregateHash),
}</code></pre>
<p>An aggregate&#39;s surrogate is a hash of its <strong>coordinate constraint</strong> — the row and column coordinates plus the page and filter selections that define which stored cells it sums. One hash per cell, the same width and the same index machinery as a record&#39;s, and the server resolves it back and runs <code>drill_records</code> when the client asks.</p>
<p>Three things follow, and each is the reason to prefer this over alternatives.</p>
<p><strong>1. Both operations are unrepresentable-if-wrong, not checked.</strong> A client cannot compose <code>SetCell</code> for an aggregate, <em>and</em> cannot compose <code>Drill</code> for a stored cell, because neither carries an id of the other&#39;s type. That is design-principles §3 — make invalid states unrepresentable — and it is strictly stronger than a server-side rejection, which would be a runtime answer to a question the type can settle.</p>
<p>An earlier draft used <code>Option&lt;CellId&gt;</code>, absence meaning &quot;aggregate&quot;. The enum is better for a reason worth keeping: <strong>an aggregate is not the absence of an identity, it is a different kind of identity.</strong> Modelling it as <code>None</code> made <code>SetCell</code> safe and left <code>Drill</code> with nothing to address — which is precisely how drill addressing came to be an open question. Two variants close it by construction.</p>
<p><strong>2. The client never learns the coordinate space.</strong> No category names, no page/filter constraints, no <code>_Dim</code> column-index convention. The surrogate is a token to echo, not a structure to understand — so gotchas 1 and 27 stay knowledge the server owns, and a second client implementation cannot get them subtly wrong.</p>
<p><strong>3. It mirrors a fork that already exists.</strong> <code>EditOrDrill</code> (<code>cmd/mode.rs</code>) already branches on exactly this in the TUI: a stored cell is edited, an aggregate is drilled. The wire is not inventing a distinction, it is <strong>projecting one the model already makes</strong> — the same discipline as <code>decision(improvise-cqi, command_set_derived_not_invented)</code>.</p>
<p>It also follows the precedent <code>CellDisplay::numeric</code> set, whose doc already says the server decides &quot;because it is the side that knows which it was&quot;. Settability is another such fact.</p>
<h2 id="decisions-made-along-the-way-and-the-traps-in-them"><a class="anchor" href="#decisions-made-along-the-way-and-the-traps-in-them">#</a>Decisions made along the way, and the traps in them</h2>
<p>Most of what started as open questions here got answered. They are kept with their reasoning because each rejected alternative is a thing someone will propose again.</p>
<p><strong>What the surrogate is — DECIDED: a hash of the normalized record.</strong> Not of the cell: the record is the coordinate tuple minus <code>_Measure</code>, which is already ADR 001&#39;s unit of atomicity and already what <code>record_versions</code> is keyed by. A cell is then <code>(record hash, measure)</code>.</p>
<p>The reason is that it makes <strong>stale edits cheap to identify</strong>, and it does so three ways at once. The surrogate a client already holds <em>is</em> the lookup key for its own version, so <code>record_versions</code> becomes <code>HashMap&lt;RecordHash, u64&gt;</code> — fixed-size, no allocation, no <code>CellKey</code> reconstruction on the hot path. A basis is then a list of <code>(record hash, version)</code> pairs, which is compact on the wire and O(1) to check. And a record that no longer exists simply <strong>fails to resolve</strong>, 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.</p>
<p>It is well-defined because <code>CellKey</code> already has a canonical form: <code>CellKey::new</code> sorts coordinates by category name and equality and hashing rely on it (gotcha 5). &quot;Normalized&quot; means that canonical sorted form with <code>_Measure</code> removed — the normalization is a property the type already guarantees, not a new convention to maintain.</p>
<p>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.</p>
<p><strong>Three ways this silently breaks, all avoidable.</strong> They are listed because each produces a <em>wrong write</em> rather than an error, which is the failure mode this ADR exists to prevent:</p>
<ul>
<li><strong>Do not use <code>DefaultHasher</code> or anything built on <code>RandomState</code>.</strong> <code>std</code>&#39;s default <code>HashMap</code> hasher is randomly seeded <strong>per process</strong>, 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.</li>
<li><strong>Do not use <code>#[derive(Hash)]</code> as the definition.</strong> Rust gives no stability guarantee for derived <code>Hash</code> across compiler versions or platforms. Hash an <strong>explicit byte serialization</strong> of the sorted <code>(category, item)</code> pairs, so the surrogate is defined by the format rather than by whatever the compiler did.</li>
<li><strong>Pick the width by consequence, not by convenience.</strong> A collision is a silent write to the wrong record — precisely why <code>_Index</code> was rejected. 64 bits reaches birthday risk around 2³² records, which a pivot tool will not see; 128 bits costs 8 more bytes per <em>visible</em> 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 <code>u64</code> because it is convenient.</li>
</ul>
<p><strong><code>_Index</code> was considered and rejected — right ontology, unsafe representation.</strong> It is the coordinate that identifies a <em>record</em>, and a record is exactly the thing that is settable rather than aggregated, so the instinct is sound. But <code>CategoryKind::VirtualIndex</code> is documented as <em>&quot;Items are <code>&quot;0&quot;</code>, <code>&quot;1&quot;</code>, … N where N = number of matching cells&quot;</em> — it is <strong>positional over the currently matching set</strong>, not an identity. So <code>_Index = &quot;3&quot;</code> names a different record after a filter change, an insert, or a delete, and names a different one again in another view. A client echoing a stale <code>_Index</code> back would write to the <strong>wrong cell, silently</strong> — strictly worse than a surrogate that fails to resolve, and the same hazard class as the viewport-relative-vs-absolute trap and the stale cursor under a renamed key.</p>
<p>What <code>_Index</code> is <em>pointing at</em> is worth keeping, though: <strong>the surrogate should name the record, not the cell.</strong> A record is the coordinate tuple minus <code>_Measure</code> (<code>record_row_identity</code>), which is already ADR 001&#39;s unit of atomicity and already what <code>record_versions</code> is keyed by. If a cell is addressed as <code>(record surrogate, measure)</code>, then one identity serves both addressing <em>and</em> staleness, and the two ADRs share a mechanism instead of each inventing one.</p>
<p><strong>How the server resolves a surrogate back — DECIDED: a maintained index, in <code>DataStore</code>.</strong> 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 <em>and</em> would only ever cover the visible window — a client may legitimately hold a surrogate for a record that has scrolled out of view.</p>
<p><code>DataStore</code> is the right home because it already does exactly this. It interns every category and item name into a <code>SymbolTable</code>, and it already maintains a secondary index <code>HashMap&lt;(Symbol, Symbol), HashSet&lt;InternedKey&gt;&gt;</code> for hot-path lookups. A <code>RecordHash → record</code> index is the same shape, updated at the same points, and the existing <code>(Symbol, Symbol)</code> index is what makes invalidation tractable: when a category or item disappears, the records that mention it are findable without a scan.</p>
<p>Two consequences worth stating, because they fall out and are easy to miss.</p>
<p><strong>The index and <code>record_versions</code> want to be one structure.</strong> Both are keyed by the record hash and both are invalidated at the same moments, so <code>RecordHash → (record, version)</code> gives ADR 001&#39;s basis check and this ADR&#39;s resolution in a single lookup.</p>
<p><strong>Structural and data staleness separate cleanly</strong>, which is better than the regenerate-on-demand shape would have given:</p>
<table><thead><tr><th>incoming surrogate</th><th>means</th><th>answer</th></tr></thead><tbody><tr><td>does not resolve</td><td>the record is <strong>gone</strong> — a category or item it named was removed</td><td>reject; there is nothing to write to</td></tr><tr><td>resolves, version matches</td><td>the client&#39;s basis is current</td><td>apply</td></tr><tr><td>resolves, version differs</td><td>the record <strong>changed</strong> since the client read it</td><td>reject as stale (ADR 001), offer force</td></tr></tbody></table>
<p>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.</p>
<p><strong>Whether a forged or stale surrogate is an error or a rejection.</strong> Under the regenerate-from-layout shape it is simply unresolvable, which suggests the same <code>Applied</code> rejection path ADR 001 defines rather than a distinct error.</p>
<h2 id="consequences"><a class="anchor" href="#consequences">#</a>Consequences</h2>
<p><strong>Good.</strong> Unblocks <code>improvise-1ey</code>. Keeps the client thin. Makes an invalid <code>SetCell</code> unconstructible rather than rejected. Composes with ADR 001 instead of duplicating it. Projects a distinction the model already makes, so there is one source of truth for what is settable.</p>
<p><strong>Bad.</strong> <code>CellDisplay</code> grows a field, and it is on the hot path — every visible cell carries it, so the surrogate&#39;s width is a wire-size decision, not only a collision one. A client holding a surrogate across a layout change holds something that will fail; that is correct, but the UI must handle it as a normal outcome rather than an error (the same obligation ADR 001 already places on rejection).</p>
<p><strong>Not addressed.</strong> The other composability blockers <code>improvise-q08</code> listed — <code>ToggleGroup</code>, <code>AddItemInGroup</code>, <code>RemoveFormula</code>, <code>LoadModel</code> — which are separate projection gaps and are not solved by a cell surrogate.</p>
<h2 id="extension-2026-09-20-two-more-kinds-by-the-same-reasoning"><a class="anchor" href="#extension-2026-09-20-two-more-kinds-by-the-same-reasoning">#</a>Extension (2026-09-20) — two more kinds, by the same reasoning</h2>
<p>The decision above is unchanged. <code>CellId</code> has since grown two variants, both arrived at by applying clause 1 rather than departing from it: a cell whose permission differs gets a <strong>different kind of identity</strong>, so the wrong operation stays unconstructible instead of being refused at runtime.</p>
<pre><code>pub enum CellId {
Record(RecordCell),
Aggregate(AggregateHash),
/// A cell a formula owns (improvise-ir7l).
Derived { target: String },
/// A records-mode creating column (improvise-poia).
Create(CreateKind),
}</code></pre>
<p><strong><code>Derived</code></strong> — a formula target is a real stored coordinate, so the original two-variant enum called it a <code>Record</code> and every client offered an editor over it. The write <em>landed</em>, and was then shadowed forever by <code>formula_cache</code>. It carries the formula&#39;s target and <strong>no surrogate</strong>: there is nothing a client could spend on a <code>SetCell</code>, for the same reason <code>Aggregate</code> carries an <code>AggregateHash</code>.</p>
<p><strong><code>Create</code></strong> — the records-mode <code>+</code> and <code>+cat</code> columns projected <code>None</code>, which this ADR defines as &quot;no projected cell&quot;. That conflation is the one clause 1&#39;s rejected <code>Option&lt;CellId&gt;</code> draft was about, arriving from the other side: not an aggregate mistaken for an absence, but a column the user <em>can</em> type into mistaken for one. It carries only the kind, because what the user types into a creating column is a <strong>name</strong>, not a value — the operation is <code>ModelCommand::AddItem</code> / <code>AddCategory</code>, which already existed, so no new command was needed and the record under the cursor is not part of it.</p>
<p>What the original two clauses cost, and still cost, is unchanged: a client learns no coordinate space, and an invalid operation is unconstructible rather than rejected. What the extension adds is that the enum is now <strong>four</strong> answers to &quot;what may be done here&quot;, so <code>None</code> finally means only what this ADR always said it meant — no projected cell at all.</p>
</main>
</div>
</body>
</html>