Merge pull request #56 from fiddlerwoaroof/docs/record-cell-key-reason
CI / Test and lint (host) (push) Has been cancelled
CI / wasm32 check (browser bundle crates) (push) Has been cancelled
CI / Knowledge base consistency (push) Has been cancelled
CI / Browser harness (OPFS contract + shell) (push) Has been cancelled
CI / Browser bundle + static site (push) Has been cancelled
CI / Deploy to GitHub Pages (push) Has been cancelled

docs(core): correct the recorded reason for leaving record_cell_key's Category arm untested
This commit is contained in:
Ed Langley
2026-09-20 12:29:52 -07:00
committed by GitHub
+30
View File
@@ -434,6 +434,36 @@ Registration is guaranteed for every model category: `Workbook::new`,
`cycle_axis` does, and no-ops on an unknown category to match `set_axis`
(improvise-t8s).
### `record_cell_key` is a match, and the match is the guard (improvise-ir7l)
`GridLayout::record_cell_key(row, col)` is the one mapping from a records
position to a model cell, and it answers `Some` for `RecordCol::Measure` alone.
The other three arms are spelled out rather than swept into a `_`, and that
exhaustive match is the whole guard: a new `RecordCol` variant fails the build
until someone decides whether it names a stored cell.
**There is no test for the `Category` arm, and the reason is not that the wrong
answer would be harmless.** It would be a real fault, and a subtle one worth
naming so nobody re-derives it wrongly. `RecordCol::Category(name)` carries a
**category** name, while `RecordRow::cell_key(arg)` puts its argument under
`_Measure`. So answering `Some` there yields `coords + _Measure=<category
name>`, and `Model::formula_for` would match it whenever a `_Measure` item
happens to share a name with a regular category — projecting `CellId::Derived`
for a **coordinate column**, which is never a formula target and which the
pivot path does not have at all. The two front ends would then disagree, which
is the design-principles §5 defect this ticket exists to close. `RecordCol`
exists for exactly that collision: its doc says the enum resolves a column from
its index "so a category and a measure sharing a name stay distinguishable".
What earns the decision to leave it untested is narrower: the **reachable**
version of that fault is a new variant answered carelessly, and the exhaustive
match makes that unrepresentable rather than merely unlikely
(`doctrine(an_invariant_is_made_unrepresentable_not_validated)`). A test would
pin behaviour the type system already pins, and would not survive the arm being
renamed. Found in review, and the first recorded reason for declining it was
**wrong** — it claimed the injected behaviour was defensible by analogy with a
formula targeting a regular category, which is a different key entirely.
## `PageAxis` — what constrains, and what the user may turn (improvise-p1ym)
`GridLayout::page_coords` answers two questions that gotcha 27 says must not be