diff --git a/context/adr/007-stable-identities.md b/context/adr/007-stable-identities.md index 2001d740..612fbeb6 100644 --- a/context/adr/007-stable-identities.md +++ b/context/adr/007-stable-identities.md @@ -1,15 +1,16 @@ # ADR 007 — Stable identities for categories, items, formulas and views -**Status:** proposed (2026-09-23), revised after adversarial review (2026-09-23) · -**Decided by:** pending — the user answers the open questions on the PR · -**Ticket:** improvise-sboz · **Unblocks:** improvise-km71, improvise-fz77, improvise-fdk1 · -**Builds on:** ADR 001, ADR 002 +**Status:** proposed (2026-09-23). Revision 2 folds in the user's answers; several +questions are still open (listed under "Still open"). This ADR is not accepted until the +user says it is. · +**Decided by:** the user, in part; see "What the user decided" · +**Ticket:** improvise-sboz · **Unblocks:** improvise-km71, improvise-fz77, +improvise-fdk1 · **Builds on:** ADR 001, ADR 002 · **Depends on:** improvise-tt22 +(remove legacy JSON) -**Numbering.** This ADR was first filed as 005. ADR 005 is already claimed by -improvise-l2b8 (exact decimal arithmetic) and ADR 006 by improvise-ilvl (everything is a -command). Neither exists as a file yet, but both numbers are taken, so this ADR is 007. -The review tickets filed against the first draft still say "ADR 005" in their titles; -they mean this document. +**Numbering.** The ADR was first filed as 005, but improvise-l2b8 already claims ADR 005 +(exact decimal arithmetic) and improvise-ilvl claims 006. The review tickets filed +against the first draft still say "ADR 005" in their titles; they mean this document. **Direction from** the user, 2026-09-23, in three messages: @@ -23,592 +24,526 @@ they mean this document. > "Maybe something like `View: Default(default)` and the standard > `View: |Default (default)|(default)` for quoting." -The second message supersedes the "even if only transient" in the first: ids are -**persisted**. The third settles two things this ADR would otherwise ask. **Views carry -ids**, and the id is written **in parentheses immediately after the name**. +## What the user decided -**What the revision changed.** An adversarial review of the first draft -(improvise-sboz's discovered-from tickets) found one open question resting on a false -premise and two resting on incomplete ones. It also found defects in the grammar, in the -wire clauses and in the projections. -- **Q3's premise was false** (improvise-80rm). The CSV importer writes headers like - `Region(code)` today. Q3 is restated with its real options. -- **Q1's costs were incomplete** (improvise-65kf, improvise-gkyo): string ids break legacy - JSON loading and `Rejection`'s `Copy`, and "max id + 1" is undefined for words. -- **The draft decided id reuse after a reload silently** (improvise-8tk5). It is now - Q10. -- **Who assigns ids** was a decision the draft made without saying so (improvise-gkyo). - It is now Q11. -- The grammar, wire and projection defects (improvise-rxz9, improvise-gkyo, - improvise-i0n7) are fixed in the clauses below. Each fix names its ticket. +The user answered the **first draft** (`context/adr/005-stable-identities.md` at +`9d97664c`) with inline comments on PR #210, and then posted one comment on the thread. +Each answer is quoted verbatim below, with where it was attached and what it overturned. +**Five of the first draft's positions were reversed.** The rest of this document is +rewritten to match. -The open questions keep their numbers so that answers already written against them -still line up. Questions 10 and 11 are new. +**1. Storage keys by id; the human name becomes a display field.** On first-draft +line 92, the clause "`CellKey`, `DataStore`, views and formula text **stay keyed by +name**": + +> "Shouldn't it be easy to replace this to key by ID? There's no type change so, at worst, +> it would be renaming field access. We could also add the name as something like +> `display` and reserve `name` or whatever it is currently for the id." + +**2. The point of ids is that keys use them.** On first-draft line 97, "ids do not make a +rename cheap. They make it *expressible*": + +> "This is a lot lower value if CellKeys, etc. don't refer to the new id" + +**3. Every reference uses the id.** On first-draft line 275, the heading of clause 8, +"ADR 002: `RecordHash` stays defined over names": + +> "No, anything referring to something with an id should refer to the id as the stable +> identity to avoid glitches." + +This answers Q7 by reversing it, and reverses clause 5's "references carry names" too. + +**4. Q1, the id's internal type.** On first-draft line 469, the recommendation to wrap "a +small string": + +> "We probably can reuse symbols as the internal type with interning?" + +Ids are strings, interned through the existing `SymbolTable`. + +**5. Q2, formula id placement.** On first-draft line 473, recommendation F1 +(`- Profit(3) = Revenue - Cost`): + +> "Yes, before the equal to the right of the name" + +**6. Q3, header syntax.** On first-draft line 480, the claim that "a header with a space +before its parenthesis (`Sales (copy)`) is unaffected": + +> "Space before the open paren is optional: document this edge case that might break +> existing docs." + +**7. Q4, data keys.** On first-draft line 485, the recommendation "no — names only": + +> "In Data, we should prefer ids that are guaranteed unique to names that might not be." + +**8. Import generates ids; legacy JSON goes.** Thread comment, 2026-09-23 06:51: + +> "For CSV or JSON import, the dimensions are to be treated as the human friendly name and +> never parsed for IDs: during import, ids should be generated by a slugify algorithm that +> strips non-alphanumeric characters and replaces spaces with hyphens(collapsing runs). +> The only supported native file format is .improv, cleanup any legacy JSON format." + +Legacy JSON removal is its own implementation ticket, **improvise-tt22**. This ADR +depends on it and does not implement it. It also dissolves improvise-65kf: with no legacy +JSON to load, there are no numeric JSON ids to accept. + +**The answers change the ADR's centre of gravity.** The first draft put an id *beside* +the name as an index. The user's answers make the **id the key** and the name a +**display** string. So a rename becomes cheap: it changes one display field and moves no +key, no surrogate and no basis tier. The first draft had called that impossible. ## Context **Today the name is the identity.** `Model::categories` is an `IndexMap` keyed by name. `CellKey` and `DataStore`'s interned symbols are names. Views, axis maps, page selections, hidden and collapsed sets, and formula text all refer by name. The user's -first message is about three consequences of that. +first message names what that costs: - **An edit cannot be told apart from a delete followed by an add.** No rename operation - exists anywhere in the workspace; `grep 'fn rename'` finds nothing. The only way to - rename a category or item is to remove it and add another, which loses every cell under - it (improvise-fz77). A formula is effectively identified by - `(target, target_category)`, and `Model::add_formula` *upserts* on that pair. So "edit - this formula" and "replace it with a different formula for the same target" are the - same call, and "change its target" is a remove plus an add (improvise-km71). + exists (`grep 'fn rename'` finds nothing). A rename is remove plus add, which loses + every cell (improvise-fz77). A formula is effectively identified by + `(target, target_category)`, and `Model::add_formula` *upserts* on that pair + (improvise-km71). - **An edit whose target was concurrently replaced cannot be told apart from an edit of - the replacement.** Suppose client A removes `Region` and adds a new `Region`. Client B's - in-flight command naming `Region` then lands on something B never saw, because the - wire's only handle is the spelling. -- **A records-mode coordinate rename has no wire form** (improvise-fdk1). The record has - a surrogate (ADR 002). The category whose coordinate changes is named only by spelling, - and `CellId::Coordinate` carries no operand at all. + the replacement.** +- **A records-mode coordinate rename has no wire form** (improvise-fdk1). -**What already exists, and who reads it.** `Category.id: CategoryId` and -`Item.id: ItemId` are `usize` *aliases*, not types. They are assigned from -`Model::next_category_id`, where virtual categories take 0, 1 and 2, and from each -category's private `next_item_id`. Both counters only ever increase within a session. -Measured at `4b880846`: - -- `Model::add_category`, `add_label_category` and `Category::add_item` (on its - idempotent path) return the id, and `Workbook::add_category` passes it through. -- **No production code reads the value.** The only reader in the workspace is the test - `re_adding_an_existing_category_at_the_limit_creates_nothing_and_succeeds`. -- **Neither field has ever been written to `.improv`.** But `Model`, `Category` and - `Item` derive `Serialize`/`Deserialize`. So **legacy JSON already carries `id`, - `next_item_id` and `next_category_id` as JSON numbers**, and nothing validates them. - That matters for migration (clause 7, Q1). -- `Formula` has no id, and neither does `View`. +**What already exists.** `Category.id: CategoryId` and `Item.id: ItemId` are `usize` +aliases, assigned from monotone counters. **No production code reads either value**; +the only reader is one test. Neither has ever been written to `.improv`. `Formula` and +`View` have no id. ## Decision -**Categories, items, formulas and views each carry a stable identity. An id says -*which thing*; a name says *what it is called*.** +**Categories, items, formulas and views each have an id, and the id is their key.** A +category or item's current `name` field *becomes* the id (the user's suggestion), and the +human name moves to a new `display` field. +- **Every reference is an id.** That covers `CellKey`s, axis maps, page selections, + hidden and collapsed sets, formula references, data keys, and wire operands that name + an existing thing. +- **Display names are for people.** They appear on screen, are typed into minibuffers, + and are resolved to ids at the input edge. -- **Operands:** an operand that names *an existing thing* is an id. The one operand that - names *the thing being created* is a name. -- **Files:** ids are persisted in `.improv` on declarations only. They are optional on - read and always written. - -The decision has nine clauses. +Ten clauses. ### 1. What carries an identity -| kind | carries an id | scope of uniqueness | wire address | +| kind | id | unique within | wire address | |---|---|---|---| | category | yes | the document | `CategoryId` | | item | yes | its category | `(CategoryId, ItemId)` | | formula | yes | the document | `FormulaId` | -| view | yes, answered by the user's example | the document | `ViewId` | -| record | already has one: `RecordHash` (ADR 002) | the document | unchanged | -| group | **no**, see question 5 | | | -| the model itself | no: there is one per document, and `document_generation` already names it | | | +| view | yes (the user's example) | the document | `ViewId` | +| record | `RecordHash`, now over ids (clause 8) | the document | unchanged shape | +| group | **still open** (Q5) | | | -**Item ids are scoped to their category**, because an item is never addressed without -its category: it appears as `(category, item)` in a `CellKey`, a hidden set and a page -selection. The existing per-category `next_item_id` already works this way. +**Virtual categories keep their reserved names as their ids**: `_Index`, `_Dim` and +`_Measure`. A `_Measure` formula target has no item of its own. Its handle is its +formula's `FormulaId`, which is why the category panel's `_Measure` rows need +`ItemHandle::{Item(ItemId), FormulaTarget(FormulaId)}` (clause 6). -**Virtual categories keep fixed ids by kind** (0, 1 and 2 today) and **never write -one**, because their names are reserved. +### 2. Storage keys by id; `display` is the only place a human name lives -**A `_Measure` formula target has no item of its own and so no `ItemId`** (gotcha 7). -Its handle is its formula's `FormulaId`. That is what makes the category panel's -`_Measure` rows need a two-kind handle (clause 6, improvise-i0n7). +Following answers 1 to 3: +- **Categories and views:** `Model::categories` and `Workbook::views` are keyed by id, + and each `Category`, `Item` and `View` gains a `display: String`. +- **Cells and derived surrogates:** a `CellKey` is a sorted list of + `(CategoryId, ItemId)` pairs. It stays sorted, now **by category id**, so gotcha 5's + canonical form is kept with a different sort key. `DataStore` interns ids exactly as it + interns names today. +- **View configuration:** the axis maps, page selections, and hidden and collapsed sets + are keyed by ids. +- **Formulas:** a formula's references are ids (clause 5, and Q14). -### 2. An id is an index beside the name, never a replacement for it +**What a rename now costs:** one `display` write. It moves no `CellKey`, no `RecordHash` +and no `layout_version`, so an edit a client composed before the rename still lands after +it. That is the "avoid glitches" the user's rule is for. The grid's labels are display +strings, so the rename's effect must still dirty the grid and the panels +(`Dirty::of` gotcha 35), but no basis tier moves. -`CellKey`, `DataStore`, views and formula text **stay keyed by name**. Re-keying storage -by id would change every hot path, the `CellKey` canonical form (gotcha 5) and the -surrogate definition (clause 8). None of the three consumers needs that. +**What the migration costs, stated honestly.** Every API that takes a name today +(`Model::category(name)`, `Workbook::add_item(cat, item)`, `CellKey::new` callers, the +formula evaluator's `Expr::Ref` lookup, `View` methods) takes an id instead. The +display→id resolution moves to the places a human types, such as the minibuffer commits, +the formula parser's reference resolution, and import. The change is mechanical but wide, +and it is the bulk of the implementation. The user judged it cheap ("at worst, it would +be renaming field access"), and for the field that is right. The call sites are the +cost. -**Ids do not make a rename cheap. They make it *expressible*.** A category rename still -rewrites every `CellKey` that names it, every view's axis map, the page selections, the -hidden and collapsed sets, and every formula that refers to it by spelling. The id buys -two things: the operation is **addressed** to *the* category the client read, and it is -a different operation from a delete followed by an add, on the wire and in the history. +**Display names stay unique within their scope**, as names are today. So a human-typed +name resolves to exactly one id, and a rename to a display name already in use is +refused. -Looking something up by id can be a scan for categories, formulas and views, since there -are at most 12 regular categories plus 3 virtual ones. Items need a maintained -`ItemId → name` index per category. +### 3. Ids are interned symbols -### 3. Lifetime +Following answer 4, an id is a **string**, and in memory it is a `Symbol` from the +existing `SymbolTable`: copyable, with integer comparison. Two consequences. -- **Within a session an id is never reused.** Each counter is a high-water mark: - removing item 5 does not free 5 for the next item. So a stale id fails to resolve; it - never resolves to something else. ADR 002 says the same of `_Index`: a stale one - "would write to the **wrong cell, silently** — strictly worse than a surrogate that - fails to resolve". -- **Across a save and a reload**, whether an id may be reused **is open question 10**. - The first draft decided "no high-water mark in the file" without saying so. That lets - a reload reuse ids: delete `B(4)`, save, reload, add `C`, and `C` gets `(4)` - (improvise-8tk5). This defeats the out-of-session identity that is the only reason to - persist ids at all. -- **In-flight commands across a reload.** Anything composed against a previous document - is rejected by ADR 001's first tier (`Rejection::DocumentReplaced`), whatever ids it - names. Note that `document_generation` restarts at 0 in a new host process, so this - holds per process. A client never survives a host restart: it reconnects and is - re-seeded. -- **A rebase** (`Sessions::rebase`) seeds a fresh `Snapshot`, and its projections carry - the current ids (clause 6). -- **Assignment is deterministic**, in file order, never in hash order (gotcha 18). Views - need one extra piece of care: `Workbook::new` seeds a `Default` view, and - `drop_unnamed_default` deletes it later. Ids for views must be assigned *after* that - deletion, or the seed consumes an id. +- **A `Symbol` never crosses the wire or reaches the file.** It is an index into one + process's table. The wire and `.improv` carry the id's **text**, and each end interns + it on arrival. +- **`Rejection` stops being `Copy`**, because `Rejection::Gone` (clause 7) carries an id's + text. `basis.rs`'s `SaveFailed` carries no message *because* `Rejection` was `Copy`, so + that reason lapses. Whether `SaveFailed` now gains its message is the implementer's + call. -### 4. Ids are types, not aliases +### 4. How an id is made: slugify, one rule everywhere -`CategoryId`, `ItemId`, `FormulaId` and `ViewId` become **newtypes**, so a `CategoryId` -cannot be passed where an `ItemId` is wanted. `design-principles.md` §3 already claims -this benefit for the aliases, and an alias does not deliver it. +Following answer 8, an id is generated from the display name by **slugify**: strip +non-alphanumeric characters, replace spaces with hyphens, and collapse runs. The user +stated this for CSV and JSON import. **This ADR proposes the same rule for every other +creation too**: interactive creation, and assigning ids to an id-less `.improv` file at +load. That way a thing's id never depends on how it was made. Whether to extend it that +far is Q13(a). -What a newtype wraps is open question 1. The answer is not free either way. Integers -keep `Rejection: Copy` and load legacy JSON's numeric ids with -`#[serde(transparent)]`. Words cost both of those (improvise-65kf). +The user's rule leaves details open. They are gathered in **Q13**, with a +recommendation for each: +- **case:** keep it or fold it; +- **`_` and `-`:** whether they are "non-alphanumeric"; +- **non-ASCII letters;** +- **the empty slug:** what id a name made only of stripped characters gets. -### 5. Persistence: declarations carry ids, references carry names +Whatever Q13 settles, three rules are needed and are decided here: -**An id appears in the file only where the thing is *declared*:** category and view -section headers, item lines and formula lines. **Every reference keeps its name:** axis -lines, `hidden:`/`collapsed:`, page selections, `## Aggregation`, `Initial View:`, the -data keys and names inside formula text. +- **A slug collision gets a suffix.** Within the id's scope (clause 1), the first thing in + creation or file order keeps the slug, and later ones get `-2`, `-3` and so on, the + lowest free number. Assignment is deterministic, never in hash order (gotcha 18). +- **A hand-written id always wins over a generated one.** Load is two-pass: first every + explicit id in the file is collected, then slugs are generated for the rest, suffixed + past anything already taken. +- **Within a session an id is never reused.** Removing `east` and later adding `East` + gives `east-2` while `east` is retired in memory. Whether that retirement survives a + save and reload is Q10. -**No id in the file is ever *referred to*, so no id can dangle.** That is why a -hand-edited file with a duplicate or unknown id can always be repaired by reassigning, -without touching anything else in the file. It also keeps `## Data`, the bulk of every -document, byte-identical to today (question 4). +### 5. Persistence: ids on declarations *and* references -**The grammar.** One new token, pressed against a name with no space, as the user's -example spells it. It is spelled with **explicit ranges**, as `bare_name` is, because the -grammar-walking generator's `Gen::emit` does not know the builtin `ASCII_ALPHANUMERIC` -and emits nothing for it. Written with that builtin, 271 of 20,000 generated files were -rejected; with ranges, none were (improvise-rxz9). +Following answers 3 and 7, the file now carries ids everywhere: +- **Declarations** (section headers, item lines, formula lines) carry the display name + and the id. +- **Every reference** carries the id: axis lines, `hidden:` and `collapsed:`, page + selections, `## Aggregation`, `Initial View:`, `## Data` keys, and names inside formula + text (Q14). + +**Id alphabet.** An id is spelled in slug characters only. Whatever Q13 settles, those +are a subset of `('A'..'Z' | 'a'..'z' | '0'..'9' | "_" | "-")`, **written with ranges**, +because the generator drops `ASCII_ALPHANUMERIC` (improvise-rxz9). So an id never needs +pipe quoting, in a declaration or a reference. ```pest -// An id, adjacent to the name it belongs to: `Default(1)`, `|Time Period|(4)`. -// Compound-atomic: no whitespace inside it or before it. -id_tag = ${ "(" ~ id_token ~ ")" } -id_token = @{ ('0'..'9')+ } // if Q1 is answered "integers" -// id_token = @{ ('A'..'Z' | 'a'..'z' | '0'..'9' | "_" | "-")+ } // if Q1 is "words" -``` +id_token = @{ ('A'..'Z' | 'a'..'z' | '0'..'9' | "_" | "-")+ } +// The space is optional (answer 6). +id_tag = ${ " "* ~ "(" ~ id_token ~ ")" } -```pest -category_section = { "## Category: " ~ header_decl ~ NEWLINE ~ blank_lines ~ category_entry* } -view_section = { "## View: " ~ header_decl ~ NEWLINE ~ blank_lines ~ view_entry* } - -// A declaring header: a name, optionally followed by its id. `model_name` and -// `initial_view` keep `header_name`: the model has no id, and `Initial View:` is a reference. -header_decl = { pipe_quoted ~ id_tag? ~ &NEWLINE | bare_name ~ id_tag ~ &NEWLINE | line_name } - -item_list = { "- " ~ item_decl ~ ("," ~ " "* ~ item_decl)* ~ NEWLINE ~ blank_lines } +// Declarations: display name, then its id. +header_decl = { pipe_quoted ~ id_tag ~ &NEWLINE | header_display ~ id_tag ~ &NEWLINE | line_name } +header_display = @{ (!(id_tag ~ NEWLINE) ~ !NEWLINE ~ ANY)+ } item_decl = { name ~ id_tag? } grouped_item = { "- " ~ name ~ id_tag? ~ "[" ~ name ~ "]" ~ NEWLINE ~ blank_lines } + +// References: an id, never a display name. +id_ref = { id_token } ``` -How `header_decl` treats files written before ids existed is **open question 3**. The -CSV importer already writes headers that `header_decl` would misread. +`model_name` has no id, so it keeps `header_name`. `Initial View:` becomes a reference +and takes an `id_ref`. A group reference (`[Coastal]`) stays a name while groups have no +id (Q5). -**Formula lines** are the one hard placement. They are set out under **"The syntax, -tested"**, and are question 2. +**`header_display` is guarded only by a lookahead**, which the generator skips (gotcha +34). So the generator may emit a display ending in something that later parses as an id. +Round-trip property 3 still holds, because the first parse settles the split. But +property 1 must be stated over the parsed result, not over the generated text. -**Round-trip properties** the implementation must pin: +**The file version changes.** A file written after this change starts with a new version +line. +- **Old files carry no ids**, and in them every reference is a display name. So the + loader must know which regime it is reading, and the version line is how. +- **In an old file**, a reference resolves against display names, and ids are assigned + by slugify. +- **In a new file**, a reference is an id. -1. **A file without ids still parses** and gets ids assigned at load. -2. **Saving writes every declaration's id**, so an id-less file is rectified on its - first save. -3. **Then it is stable**: `format(parse(format(parse(x)))) == format(parse(x))`. -4. **Ids a file already carries survive verbatim.** +Under Q13's recommendation, slugify keeps case, `_` and `-`, so every **bare** name +slugifies to itself. Most of an old document's `## Data` therefore reads identically in +the new format. Pipe-quoted names are the ones that change. -**Rule for the generator.** For each property, a generated input must be able to fail -it. The grammar-walking generator **cannot exercise formula ids at all**: it never emits -`=`, and `walk_formulas_section` silently drops any formula line without one (36 of -2,000 generated files had formula lines, and every one was dropped). So properties 1–4 -are **vacuous for formulas** unless formula lines get their own strategy, one that -emits a valid `target = expr` with and without an id. That strategy is part of this -ADR's persistence work, not an optional extra (improvise-rxz9). +**Round-trip properties:** +1. an id-less (old-version) file loads, gets ids, and is written in the new version on + its first save; +2. a new-version file loads with every id kept verbatim; +3. `format(parse(format(parse(x)))) == format(parse(x))`. -**Assignment is two-pass, and the ordering is load-bearing.** First collect every -*explicit* id in the file. Then assign the missing ones in file order, skipping any id -already taken. A single pass would give `- A, B(1)` the id 1 for `A`, and then `B`'s -hand-written 1 would be the one lost. +A **formula-line generator strategy** is required. The grammar walker emits no `=` and +the walker drops such lines, so without its own strategy every formula property is +vacuous (improvise-rxz9). -**Duplicate or colliding ids** in a hand-edited file are repaired: the first -declaration keeps the id, every later duplicate is reassigned with a **load warning**, -and the next save rectifies the file (question 9). An empty tag (`Default()`) and a tag -followed by trailing whitespace (`Default(1) `) are also warned about. They fall through -to `line_name` and would otherwise silently become part of the name. +**A duplicate or unknown id in a hand-edited file can no longer be repaired silently.** +The first draft's repair argument rested on "no id is ever referred to", and answer 3 +reversed that. Reassigning a duplicate would silently change what its references point +at. So Q8 is restated below, and the recommendation changes to **refusing the load** with +an error naming the id. -**Legacy JSON stays read-only.** It goes through **the same rectifying pass** as a text -load: explicit ids are kept, duplicates reassigned and missing ids assigned. -- It carries `id`, `next_item_id` and `next_category_id` as JSON **numbers**. With - integer ids (Q1), they load as they are. -- With word ids, a custom `Deserialize` that accepts a number is required. - `#[serde(transparent)]` over a string rejects `"id": 3` (improvise-65kf). +**Legacy JSON is removed** (improvise-tt22), so it needs no migration here. -**The import wizard, CSV import and JSON import** create things through the same -counters as interactive creation, so ids are assigned as each thing is created. **CSV -export writes no ids.** +**Import** (CSV, JSON) never parses ids out of dimension names (answer 8). A header +`Region(code)` is the display name `Region(code)`, and its id is its slug. The writer +pipe-quotes any display name that would otherwise read as display plus id: +`## Category: |Region(code)|(Regioncode)`. -**The coverage match.** `grammar_coverage::walker_of` is an exhaustive match over -`Rule`, so each new rule fails the build until it names its walker (improvise-4lgj). The -generator **skips predicates** (gotcha 34), so no rule here relies on a lookahead as its -*only* guard. +### 6. The wire: address by id, create by display name, learn ids from projections -### 6. The wire: address by id, create by name, learn ids from projections +- **Every operand naming an existing thing is an id.** The one operand naming the thing + being created is a display name, and the host slugifies it: + - `AddCategory(display)` and `CreateView(display)`; + - `AddItem { category: CategoryId, display }`, + `AddItemInGroup { category: CategoryId, display, group }` and + `AddFormula { target_category: CategoryId, raw }`. +- **New operations are born with ids:** + - `RenameCategory { category, to: display }`; + - `RenameItem { category, item, to: display }`; + - `EditFormula { formula, raw }`; + - fdk1's coordinate rename; + - the removes (`RemoveCategory`, `RemoveItem`, `RemoveFormula`, `DeleteView`). +- **Older variants** (`SetAxis`, `SetPageSelection`, `HideItem`, …) **must also move to + ids**. Their operands are references, and a reference is now an id. They may move one + whole variant at a time. +- **Projections carry id and display together:** + - `CategoryRow { id, display, items: Vec<(ItemHandle, String)>, axis }`; + - `FormulaCache` rows `{ id, text }`, where the text is rendered with display names + for people; + - `ViewSummary { id, display, … }`. -- **The rule, stated per operand** (corrected; improvise-gkyo): every operand that names - an existing thing is an id, and only the operand that names the thing being created is - a name. - - `AddCategory(String)` and `CreateView(String)` are unchanged. - - `AddItem { category: CategoryId, item: String }`, - `AddItemInGroup { category: CategoryId, item: String, group: String }` and - `AddFormula { target_category: CategoryId, raw }`: the first draft kept each parent - as a name, which contradicted this rule. A group is a name because groups carry no - id (Q5). -- **New operations are born with ids:** `RenameCategory { category: CategoryId, to }`, - `RenameItem { category: CategoryId, item: ItemId, to }`, - `EditFormula { formula: FormulaId, raw }`, and fdk1's coordinate rename. Removes move - to ids too: `RemoveCategory`, `RemoveItem`, `RemoveFormula` and `DeleteView`. -- **The older view- and category-scoped variants** (`SetAxis`, `SetPageSelection`, - `HideItem`, …) **migrate one whole variant at a time**, as consumers touch them. - Mixing name-addressed and id-addressed variants is acceptable because each variant is - internally consistent. -- **Who assigns an id is open question 11.** The first draft assumed the host assigns - every id and never said so. The assumption matters because - `create-and-switch-view` already sends `[CreateView(name), SwitchView(name)]`, a frame - that addresses what it has just created. Under host-assigned ids that client cannot - know the id to address. -- **How a client learns ids: the panel projections carry them.** - - `ViewSummary` gains `id: ViewId`, and `FormulaCache` becomes rows of - `{ id: FormulaId, text }`. - - `CategoryRow` gains `id: CategoryId`, and its items become - `(ItemHandle, String)` pairs: + `AxisFrame` labels stay display strings. They gain ids when a consumer needs them. +- **`ViewState` keys by id**: the cursors, `expanded_cats`, and the view history + (`view_back_stack` / `view_forward_stack`). Answer 1 makes this natural rather than a + migration of its own. A view or category rename then orphans nothing. - ```rust - pub enum ItemHandle { Item(ItemId), FormulaTarget(FormulaId) } - ``` +### 7. ADR 001: an id that no longer resolves is gone - This is ADR 002's clause 1 again: a row whose permission differs gets a *different - kind* of identity (improvise-i0n7). `_Measure`'s rows come from - `effective_item_names`, which includes formula targets. Those have no `ItemId`, so a - single `ItemId` field could not be filled for them. Yet `delete_at_cat_cursor` - composes `RemoveItem` on those rows today. With the enum, deleting a formula-target - row composes `RemoveFormula` instead, and the wrong operation cannot be constructed. - - **`AxisFrame` does not change yet**, and **`CellId::Derived` keeps carrying the - target's *name*.** Both gain ids when a consumer needs them, through the same - projection discipline (gotcha 44). -- **`ViewState` is keyed by name in three places, and each must move to ids before the - matching rename lands** (improvise-i0n7): - - `cursors` and the view history (`view_back_stack` / `view_forward_stack`, whose - `ViewFrame.view_name`) are keyed by **view** name. A view rename orphans them. - - `expanded_cats` is keyed by **category** name, so a category rename collapses that - category on every client. That makes it **part of fz77**, not a later concern. +A command naming an id that does not resolve is rejected before anything is applied, as +`Rejection::Gone(Named)`: +- `Named` is one of `Category`, `Item`, `Formula` or `View`, and carries the id's text. +- It is **structural**: force never creates structure. - `ViewState` is per-client state, so the host cannot rewrite these keys on a rename. - They must already be ids. +The check has two halves, like the record tier: +- **Before the frame:** an exhaustive `ids_named` feeds `Checked::against`. +- **Within the frame:** an **id arm in `broken_chain`** refuses + `[RemoveFormula(f), EditFormula(f)]` (improvise-gkyo). -### 7. ADR 001: an id that no longer resolves is *gone*, and force never creates structure +Per-entity versions are still not added (Q6). -A command naming an id that does not resolve is rejected **before anything is applied**, -just as a `RecordHash` that does not resolve is (`Rejection::RecordGone`). The rejection -is new and carries which kind of thing was gone: +### 8. ADR 002: `RecordHash` is now defined over ids -```rust -Rejection::Gone(Named) // Named = Category(CategoryId) | Item(CategoryId, ItemId) - // | Formula(FormulaId) | View(ViewId) -``` +Following answer 3, `RecordHash` hashes the explicit serialization of the sorted +`(CategoryId, ItemId)` text pairs, the same function over different strings. +- **ADR 002's decision is unchanged in substance.** It still hashes an explicit + serialization of a normalized record, with a fixed key and a width argued for. +- **Its prose will be out of date once this lands.** ADR 002 says `CellKey::new` + "sorts coordinates by category name", and a record is spelled in names. Both become + ids. ADR 002 is **not edited here**. Updating that wording belongs to the + implementation PR. -It is **structural**: there is nothing to write to, so there is nothing to force. An -edit of a formula another client deleted is refused as gone. It is **never** applied to -a new formula that happens to have the same target. +**What changes in behaviour:** a display rename no longer changes any `RecordHash` or +bumps `layout_version`, because `view_shape` hashes ids too. So a client's surrogate +survives a rename, and its in-flight edit still lands. A **coordinate** change, as in +fdk1 or a records-mode move, still produces a different record and a different hash, as +it should: it *is* a different record. -**The check has two halves, like the record tier's** (improvise-gkyo): +### 9. Headers that change meaning: the edge case answer 6 asks to document -- **Before the frame:** `Checked::against` / `CheckedBatch::against` ask which ids each - command names, through an **exhaustive `ids_named`**. That is the twin of - `records_named`, so a new id-addressing variant fails the build until it says what it - names. -- **Within the frame:** `broken_chain`, the `ChainBroken` re-check, walks only - `records_named` today. So `[RemoveFormula(3), EditFormula(3)]` passes the pre-frame - check, and the edit then finds nothing. It needs an **id arm**: an id removed by an - earlier command in the same frame is refused as a broken chain, exactly as a record - is. +Under answer 6, `header_decl` reads a trailing `(token)`, with or without a space before +it, as the id. In a **new-version** file the writer never produces an ambiguous header, +because it pipe-quotes any display that would split. So the edge cases below arise only +when a **new-version** file is hand-written, or when an **old-version** file is read +with the new header rule. Whether old files get that rule is **Q12**. -**Integer ids keep `Rejection: Copy`.** `basis.rs` relies on that: `SaveFailed` carries -no message *because* `Rejection` is `Copy`. Word ids would take `Copy` away (Q1). - -**Not added here: per-entity versions.** Two clients editing *the same* formula still -race, and the last writer wins, as `AddFormula` does today. An id is exactly the key a -`formula_versions` tier would need (question 6). - -### 8. ADR 002: `RecordHash` stays defined over names - -`RecordHash` is a SipHash over an explicit serialization of the `(category, item)` -**name** pairs (`model/record.rs`). This ADR does not change that. - -A rename therefore changes every `RecordHash` that names it. A client holding a -surrogate from before the rename gets `RecordGone`. More often it gets `LayoutMoved` -first: a rename relabels the grid, `view_shape` hashes the names, and -`observe_reshape` bumps `layout_version` (gotcha 40). That is correct behaviour, not a -cost. Hashing ids instead was deferred (question 7). - -### 9. Migration - -1. **Types** (no behaviour change): the newtypes, `FormulaId`, `ViewId` and their - counters. -2. **Persistence:** the grammar change, the version handling Q3 chooses, the two-pass - rectifying load, the writer, the high-water mark Q10 chooses, and the round-trip - properties with a formula-line strategy. -3. **Projections:** ids on `CategoryRow` (with `ItemHandle`), on `FormulaCache` rows and - on `ViewSummary`. -4. **Vocabulary**, per consumer: the addressing variants, `Rejection::Gone`, `ids_named` - and `broken_chain`'s id arm. The removes and the create variants' parent operands - move to ids at the same step. -5. **Opportunistic:** the older view- and category-scoped variants, as they are touched. - -## The syntax, tested against the grammar - -The user's form is `Name(id)`, and `|quoted name|(id)` when the name needs quoting. - -**Section headers, and the importer that already writes `Name(token)`.** A header takes -its name as the *whole line* (improvise-pczv, gotcha 34). `header_decl` recognises an id -only when it is pressed against a name token: - -| line | today | under `header_decl` | +| header line | whole-line reading (today) | under `header_decl` | |---|---|---| -| `## View: Default(1)` | name `Default(1)` | name `Default`, id `1` | -| `## View: \|Default (default)\|(1)` | name `\|Default (default)\|(1)`, pipes literal | name `Default (default)`, id `1` | -| `## Category: Time Period` | `Time Period` | `Time Period`, no id; one is assigned at load | -| `## View: Sales (copy)` | `Sales (copy)` | **unchanged**: a space before `(` means no name token touches it | -| `## Category: Region(code)` | `Region(code)` | **changed**: name `Region`, id `code` | +| `## View: Default(default)` | `Default(default)` | display `Default`, id `default` | +| `## View: \|Default (default)\|(default)` | pipes literal | display `Default (default)`, id `default` | +| `## View: Sales (copy)` | `Sales (copy)` | **display `Sales`, id `copy`** | +| `## Category: Region(code)` | `Region(code)` | **display `Region`, id `code`** | +| `## Category: Budget (2024)` | `Budget (2024)` | **display `Budget`, id `2024`** | +| `## Category: Time Period` | `Time Period` | unchanged; id `Time-Period` generated | -**The first draft called the last row "a spelling nobody is known to use", and that was -false** (improvise-80rm). Run `improvise import` on a CSV whose headers are -`Size(cm),Region(code),Amount`: it saves `## Category: Region(code)` and -`## Category: Size(cm)` **today**. Under `header_decl` those load as categories `Region` -and `Size`. Every *reference* still spells `Region(code)` (clause 5), so the axis lines -and every `## Data` key name a category that no longer exists. Today's `parse_md` -accepts that reading silently: the axis lines are dropped, every cell is keyed under an -undeclared category, and the grid draws nothing. **The cost is silent loss of an -imported document's data, not a rename.** +**The documents that change meaning** are those with a view or category header ending in +a parenthesised run of letters, digits, `_` or `-`. **The app writes such headers +itself**: `improvise import` on a CSV with headers like `Region(code)` or `Size(cm)` +saves `## Category: Region(code)` today (improvise-80rm). Any view someone named +`Sales (copy)` does the same. -With integer ids (Q1) the collision narrows to headers ending in a parenthesised -*number*, such as `Year(2024)` or `Size(12)`. It does not go away. So Q3 has to choose a -mechanism; it cannot be accepted as a rare edge. Its options are set out in the question -itself. +**What it costs, if old files get the new rule:** +- The category is renamed on load: display `Region`, id `code`. +- Every reference in that old file spells `Region(code)`, which resolves to neither the + display nor the id. +- So the axis lines and every data key for that category do not resolve. +- Today's loader would silently drop the axis lines and orphan the data, so nothing is + drawn. -**Items: no compatibility cost.** Examples: `- North(1), South(2)`, `- |North, East|(3)`, -and grouped `- Coastal_East(7)[Coastal]`. A `(` after an item name is a **parse error -today**, so no existing file changes meaning. Groups (`> Coastal[Region]`) carry no id -(question 5). +**What a user would have to do with such a file** is pipe-quote the header before opening +it (`## Category: |Region(code)|`). That fixes it, because a pipe-quoted header without +an id reads as the whole name. -**Formulas: the one real placement problem.** A formula's name is its **target**. The -improv grammar never parses the target: it hands `formula_raw` to `parse_formula` as -opaque text. A trailing id is ambiguous, because a formula can end in `)` (`SUM(Revenue)`, -`(2)`), so it is rejected. Three placements remain: +Silent data loss on load is a real hazard, so this ADR does not decide it. It is **Q12**. -- **F1, after the target:** `- Profit(3) = Revenue - Cost`. The grammar gains - `formula_decl = { name ~ id_tag ~ formula_rest }`, with `formula_raw` kept as the - id-less fallback. It is unambiguous: `Profit(3)` is not a legal target today, so no - existing line changes meaning. It costs more than the first draft said. - - `Formula` exposes **no span** for its target. - - `raw` may begin with whitespace. - - So the writer must either get the target's span from `parse_formula`, or re-spell - the target with `quote_name` and keep only the text after the first unquoted `=`. +### 10. Migration - Either is a small, contained change. **Recommended.** -- **F2, before the formula:** `- (3) Profit = Revenue - Cost`. Unambiguous and cheapest, - but it breaks "id after the name". -- **F3, after the category suffix:** `- Profit = Revenue - Cost [_Measure](3)`. - **Rejected**, because it reads as `_Measure`'s id. - -Whichever placement is chosen, the id is a **file** construct. `parse_formula`, the -formula minibuffer and `Formula::raw` never see it. - -**Data keys keep names** (question 4). +1. **Remove legacy JSON** (improvise-tt22). +2. **Types:** the id newtypes over `Symbol`, the `display` fields, and `FormulaId` and + `ViewId`. +3. **Re-key storage by id** (clause 2). This is the wide, mechanical step, with + display→id resolution moved to every input edge. +4. **Persistence:** the new version line, the grammar in clause 5, the two-pass loader, + slugify, the writer, and the round-trip properties with a formula-line strategy. +5. **Projections and `ViewState`** (clause 6). +6. **Vocabulary:** the id-addressed variants, `Rejection::Gone`, `ids_named` and + `broken_chain`'s id arm. ## What this means for you ### improvise-km71 — editing a formula -- The formula panel's rows carry `FormulaId` (clause 6). Commit sends - `EditFormula { formula: FormulaId, raw }`, never `AddFormula`. The formula is replaced - **in place**, keeping its position and its id. -- **"If the edit changes the target, is it the same formula?" Yes.** Identity is the id. - If another formula already owns the new target, the edit is **refused**. It must not - fall through to `add_formula`'s upsert, which would silently replace that other - formula. +- **The operation.** The formula panel's rows carry `FormulaId`, and the row text is + rendered with display names. Commit sends `EditFormula { formula, raw }`. + - The user types `raw` with display names, and the host resolves them to ids. + - The formula is replaced **in place**, keeping its position and id. +- **A changed target is the same formula.** Identity is the id. If another formula + already owns the new target, the edit is refused. It must not fall through to + `add_formula`'s upsert. - **A target change can leave an item behind.** A non-`_Measure` target is a real item, - because `add_formula` creates it. An `EditFormula` that moves the target leaves the old - item behind. Decide whether it is removed (only if nothing else references it) or kept, - and test the direction you choose. -- Another client's deletion gives you `Rejection::Gone(Formula(id))`. Within one frame it - gives the broken-chain refusal (clause 7). -- Esc is yours to decide. The seed text is `Formula::raw`, which never contains the - file's id. -- **Don't** land a version keyed on `(target, target_category)` and migrate later. That - is the name-identity this ADR retires. + which `add_formula` creates. Decide whether the old one is removed when nothing else + references it, and test the direction you choose. +- **Rejections.** Another client's deletion gives `Rejection::Gone(Formula(id))`. A + deletion earlier in the same frame gives the broken-chain refusal. ### improvise-fz77 — renaming a category or item -- Two operations: `RenameCategory { category: CategoryId, to }` and - `RenameItem { category: CategoryId, item: ItemId, to }`. The panel rows carry both ids. -- **The rename is still a rewrite of every name reference** (clause 2). That covers: - - `CellKey`s; - - axis maps, page selections, and hidden and collapsed sets; - - `measure_agg` keys; - - group membership; - - **formula text**. - - Rewrite formula text through the formula tokenizer's identifier spans and - `improvise_formula::quote` (gotcha 34). **Never use string replacement**: `Revenue` - inside `|Net Revenue|` is not a reference to `Revenue`. -- **`ViewState::expanded_cats` must be keyed by `CategoryId` first** (clause 6). - Otherwise every rename collapses the renamed category on every client, and the host - cannot fix that, because the map is per-client state. -- A rename to a name that already exists in the same scope is **refused**, because it - would merge two things. -- Expect every `RecordHash` under a renamed name to change (clause 8). Test that the - *data* moved, not that a surrogate survived. +- **The operations.** `RenameCategory { category, to }` and + `RenameItem { category, item, to }`. The panel rows carry the ids. +- **It is now cheap** (answers 1 and 2): one `display` write. No `CellKey`, axis map, + page selection, hidden or collapsed set, or formula text is rewritten, because all of + them hold ids. No `RecordHash` or `layout_version` moves, so in-flight edits survive. +- **What it still owes:** + - **Uniqueness.** Refuse a display name already used in the same scope. + - **Dirtiness.** Mark the effect so the grid labels and panels are re-projected + (gotcha 35). The labels are display strings. + - **Tests.** Assert that the *labels* changed and the *surrogates did not*. That is + the property this design exists for. +- **It depends on step 3 of the migration**, the re-keying. Until then there is no cheap + rename to build on. ### improvise-fdk1 — renaming a records coordinate - The operation is - `RenameRecordCoordinate { record: RecordHash, category: CategoryId, item: String }`. - - The record is addressed by its surrogate and the category by its id. - - The **new item** is a **name**, because typing a coordinate value can create the - item. That is the create-by-name half of clause 6, and it is what - `RecordRewrite::of` already does: it adds the item, then moves every cell in the row. -- Map the operation onto `RecordRewrite`, the one home the drill and the plain records - path already share (improvise-uo2p). Don't write a third copy. -- `CellId::Coordinate` must grow an operand, `{ record, category: CategoryId }`, so a - client can echo it. That is an ADR 002 extension of the same shape as `Create`. -- fdk1's other blocker is that a browser cannot enter records editing, so nothing - produces the command. This ADR does not touch that. + `RenameRecordCoordinate { record: RecordHash, category: CategoryId, to: ItemRef }`, + where `ItemRef` is `Existing(ItemId)` or `New(display)`, the create-by-display half of + clause 6. +- Unlike a display rename, this **changes a coordinate**, so the record's `CellKey`s and + `RecordHash` change. It is a different record. +- Map it onto `RecordRewrite`, which the drill and the plain records path already share + (improvise-uo2p). +- `CellId::Coordinate` grows `{ record, category: CategoryId }`. +- The producer blocker (a browser cannot enter records editing) is untouched. -## Alternatives rejected +## Questions as asked, and where each now stands -- **Transient ids, never written.** The user's second message overruled this. A - transient id would be enough on the wire, but persisting ids is what gives identity - **outside a session**: a rename shows as a rename in `git diff`, and a future reference - can survive a restart. Q10 decides whether that identity actually holds across a - reload. -- **Re-keying storage by id.** See clause 2. -- **Ids on references as well as declarations.** See question 4. It would also destroy - clause 5's key property: once references carry ids, a duplicate id *can* dangle. -- **A trailing id on formula lines.** Ambiguous; see "The syntax, tested". +The first draft asked Q1–Q9, and revision 1 added Q10 and Q11. Each original question +and recommendation is kept below, with its status. -## Open questions +| # | question | my recommendation | status | +|---|---|---|---| +| 1 | Integer or word ids? | first draft: words; revision 1: integers only | **answered**: interned symbols (answer 4), generated by slugify (answer 8). Words. | +| 2 | Formula id placement | F1, `- Profit(3) = …` | **answered: F1** (answer 5) | +| 3 | Headers in old files | first draft: accept the rare break; revision 1: gate on version | **answered in part**: the space is optional (answer 6). Whether old files get the rule is still open (Q12). | +| 4 | Do `## Data` keys carry ids? | no | **answered: yes, ids** (answer 7), reversing the recommendation | +| 5 | Group ids? | not now | open | +| 6 | Per-entity versions? | not in this ADR | open | +| 7 | `RecordHash` over ids? | no, keep names | **answered: over ids** (answer 3), reversing the recommendation | +| 8 | Duplicate id: repair or refuse? | repair | open, **restated below** | +| 9 | Reading of "opt-in" | read optional, write always | open | +| 10 | Id reuse after a reload | persist a high-water mark | open, **restated below for slug ids** | +| 11 | Who assigns ids? | the host | open, **restated below** | -Each question is genuinely the user's to decide. Each has a recommendation and the -argument for it. Answer inline on the ADR line or on the PR thread. The numbers are -stable across this revision; 10 and 11 are new. +## Still open -1. **What does an id look like: an integer, or a word?** Your example `Default(default)` - uses a word. **The first draft recommended words and understated their cost.** - - Words cost three things (improvise-65kf, improvise-gkyo): - - **legacy JSON** carries numeric ids, which `#[serde(transparent)]` over a string - rejects, so a hand-written `Deserialize` is needed; - - **`Rejection` loses `Copy`**, which `basis.rs` relies on; - - **"max id + 1" is undefined for words**, so the counter has to ignore them and - any allocation scheme becomes two rules. - - Integers cost only the ability to hand-write a memorable id. - - **Recommendation: integers only.** A hand-written word id is renumbered on load - with a warning, and the next save writes the number. `Default(default)` would be - written `Default(1)`. -2. **Where does a formula's id go?** **Recommendation: F1, `- Profit(3) = Revenue - Cost`.** - It is the only placement that follows your "id after the name" rule and is - unambiguous. Its real cost, a target span or a re-spelled target, is stated under "The - syntax, tested". F2 (`- (3) Profit = …`) is cheaper. -3. **How are headers read in files written before ids existed?** *(Restated: the first - draft's premise was false.)* The CSV importer writes `## Category: Region(code)` today - (improvise-80rm). `header_decl` would read it as category `Region` with id `code`, and - the document's data would silently disappear from the grid. With integer ids the - collision narrows to headers like `Year(2024)`, but it remains. The real options: - - **(a) Gate on the file's version line (recommended).** - - A file saved after this lands starts with a new version line. - - A file with the old line `v2025-04-09` is read exactly as today: a header is the - whole line, and ids are assigned at load. - - The writer always emits the new line, and pipe-quotes any header name that - `header_decl` would split. That is a fifth `quote_header` case, so `Region(code)` - saves as `|Region(code)|(3)`. - - This keeps your syntax exactly and changes no existing file's meaning. It costs one - branch in the walker: for an old-version file, the header's matched text is taken - whole. The generator must emit both versions. - - **(b) Make the header id unambiguous by construction.** Examples: a separate - `id: 3` line under the header, like the existing `kind:` line, or a sigil such as - `Default(#3)`. The separate line is fully unambiguous in category sections. In - view sections it collides with an axis line for a category named `id` (`id: row`), - so it needs a different keyword there. A sigil only narrows the collision. Both - depart from your syntax. -4. **Do `## Data` keys carry ids?** **Recommendation: no, names only.** - - Data is the bulk of every document, and ids there would roughly double every key. - - Ids there would create references that can dangle, which clause 5's repair argument - depends on never happening. 5. **Do groups get ids?** **Recommendation: not now.** Nothing addresses a group on the - wire yet. -6. **Should an edit detect a concurrent change to the same entity** (a per-entity - version)? **Recommendation: not in this ADR.** The id is the key such a tier would - need. -7. **Should `RecordHash` be computed over ids?** **Recommendation: no, keep names** - (clause 8). -8. **A duplicate id in a hand-edited file: repair or refuse?** **Recommendation: repair - with a load warning** (clause 5). -9. **"Opt-in": confirm the reading.** *(Incomplete in the first draft.)* The reading is: - *reading* is optional (a file without ids is legal forever), and *writing* is not - (every save writes ids). **That reading only delivers identity across saves if Q10 is - answered "persist the counter".** Without that, ids survive a save but can be reused - after the next reload. -10. **May an id be reused after a save and reload?** *(New; the first draft decided this - silently.)* With no counter in the file, the next id after a reload is "max seen + - 1". So delete `B(4)`, save, reload, add `C`, and `C` gets `(4)` (improvise-8tk5). A - `git diff` then reads `B(4)`→`C(4)` as a rename, and any outside reference to `4` now - names `C`. Two branches that each add a category also collide on merge, and the - repair (question 8) re-identifies one of them. - - **(a) Persist a high-water mark per kind (recommended).** One `## Ids` line per - document holds the category, formula and view counters, and each category section - holds its `next:` item counter. Ids are then never reused within a document's - history. Merges still collide, but a collision is detected and repaired with a - warning. - - **(b) Collision-resistant tokens** (for example 8–13 random base32 characters). - These never collide, even across a merge. But every item line carries noise, a - token is a word (Q1's costs), and tokens assigned at load must still be - deterministic (gotcha 18). - - **(c) Accept reuse.** What the first draft did silently. It gives up the reason - for persisting ids. -11. **Who assigns an id?** *(New; the first draft assumed the host.)* - `create-and-switch-view` already sends `[CreateView(name), SwitchView(name)]`, a - frame that addresses what it creates (improvise-gkyo). - - **(a) The host assigns every id (recommended).** A frame cannot address by id - something it creates. A "create then act" compound becomes one command, for - example `CreateView { name, switch: true }`. `SwitchView` can stay name-addressed - in the meantime, because it only moves this client's own `ViewState`. - - **(b) Clients propose ids.** A frame can then address what it creates, but ids - must then be collision-resistant across clients (Q10(b)), and the host must refuse - a proposed id that is already taken. + wire. The cost of deferring is that group references (`[Coastal]`, `collapsed:`) stay + display names, the one exception to "every reference is an id". +6. **Per-entity versions?** **Recommendation: not in this ADR.** +8. **A duplicate or unknown id in a hand-edited file: repair or refuse?** *(Restated.)* + References now carry ids, so reassigning a duplicate silently re-points every + reference to it. **Recommendation: refuse the load**, with an error naming the id and + both declarations. +9. **"Opt-in": confirm the reading.** **Recommendation:** reading an id-less file is + supported forever, as an old-version file whose ids are assigned by slugify, and every + save writes the new version with ids. +10. **May an id be reused after a save and reload?** *(Restated for slug ids.)* Slug ids + are only reused when a removed thing's name comes back: remove `East`, save, reload, + add `East`, and it gets `East` again. + - **(a)** Persist the retired ids in the file, so the new `East` becomes `East-2`. + The list grows forever. + - **(b) Accept reuse across a reload (recommended).** Within a session, retirement + still prevents reuse. An in-flight command from before the reload is refused by the + generation tier. The one remaining risk is an outside reference that outlives the + file, and none exists today. + - **(c)** No retirement even within a session. That brings back in-session glitches. +11. **Who assigns ids?** *(Restated.)* Ids are now derived from names, so a client + *could* compute one. `create-and-switch-view` sends `[CreateView, SwitchView]`, a + frame that addresses what it creates. + - **(a) The host assigns (recommended).** A create-then-act compound becomes one + command, for example `CreateView { display, switch: true }`. The host alone + resolves collisions and suffixes. + - **(b)** The client proposes the slug and the host refuses one that is taken. The + frame can then address what it creates, but a collision becomes a refusal the user + sees. +12. **Do old-version files get the new header rule?** *(New; the hazard answer 6 asked to + document.)* See clause 9. + - **(a) No (recommended).** An old-version file is read as it always was: a header is + the whole line. That is safe because no file written before this change can contain + an id, so any `(token)` in its headers is part of the name. The optional-space rule + applies to new-version files, where the writer quotes every ambiguous display. + Nothing that exists today changes meaning. + - **(b) Yes.** Headers like `Region(code)` and `Sales (copy)` split on load, and the + data under them silently disappears unless the user pipe-quotes the header first. + If this is chosen, the loader should at least **refuse** such a file with an error + naming the header, rather than orphaning data silently. +13. **Slugify details.** *(New; the rule as stated leaves these open.)* + - **(a) Where slugify applies.** The recommendation is *all* id generation: + interactive creation and old-file load as well as import. That gives one rule, and + bare names slugify to themselves. + - **(b) Case.** Recommendation: **keep it**, so `East` stays `East` and old data keys + read the same. + - **(c) `_` and `-`.** Recommendation: **keep both.** They are already legal in bare + names, so every existing bare name is its own slug. The literal "strip + non-alphanumeric" would turn `Coastal_East` into `CoastalEast` and `Q1-2024` into + `Q12024`. + - **(d) Non-ASCII letters.** Recommendation: **strip them** (ASCII only), so an id + never needs quoting. A name made only of such letters falls to (e). + - **(e) The empty slug.** Recommendation: `-` (for example `category-1` or + `item-4`), with the lowest free `n`. +14. **Formula text in the file: ids or display names?** *(New; a consequence of answer 3 + that is visible in the file.)* Answer 3 says every reference is an id, so a formula + line would be written `- Profit(Profit) = Revenue - Cost` over ids. With Q13's + recommendations the ids equal the bare display names, so most formulas read the same. + A display rename changes the UI rendering and not the file. The cost is that + `Formula::raw` is no longer stored verbatim: the file gets a normalized rendering over + ids, and the UI a rendering over display names. **Recommendation: ids in the file**, + per answer 3. Otherwise every rename must rewrite formula text, which is exactly the + glitch answer 3 is avoiding. ## Consequences **Good.** -- Edit, rename and replace become distinct, addressable operations. -- A delete or remove is refused as *gone* instead of hitting a namesake, both before a - frame runs and within it. -- km71, fz77 and fdk1 share one vocabulary. -- The unused `id` fields acquire a job and become real types. -- `## Data` does not change. +- Edit, rename and replace are distinct, addressed operations. +- A rename is one display write: it moves no key, surrogate or basis tier, and in-flight + edits survive it. +- `## Data` keys are unique by construction. +- The format loses a legacy branch (improvise-tt22). **Bad.** -- A one-time visible diff on every document's first save: ids appear, and some headers - become pipe-quoted. -- A new file version (if Q3(a) is chosen). -- The wire vocabulary is mixed, names and ids, until the older variants are migrated. -- A rename is exactly as expensive as before. +- Re-keying storage is wide: every name-taking API moves to ids, and display→id + resolution moves to the input edges. +- A new file version. +- `Rejection` loses `Copy`. +- `Formula::raw` is no longer verbatim (if Q14 holds). +- ADR 002's prose needs a wording update when this lands. -**Not addressed.** -- Per-entity versions (Q6) and group ids (Q5). -- Ids in `AxisFrame` and `CellId::Derived`. -- Moving `ViewState`'s name-keyed maps to ids. That is required before the matching - rename lands (clause 6); for `expanded_cats`, that means before fz77. +**Not addressed.** Per-entity versions (Q6), group ids (Q5), and ids in `AxisFrame` and +`CellId::Derived`.