docs(adr): put answers 35 and 36 on their true head and mark Q23's premises (improvise-cq1q.1)
Merge the eighth and ninth rounds (all four comments are on 4aed8c07),
quote the anchored line, drop the lean toward option (c), add pending-Q23
markers, and record the review's non-blocking points as flagged text.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
committed by
Edward Langley
parent
5385bb36bc
commit
769f7b251f
@@ -8,13 +8,13 @@ user approved revision 8 on PR #210 (owner comment 5824468104, 2026-09-25 00:17:
|
||||
**Revision 9 is a proposed amendment, not yet approved.** It folds in the user's
|
||||
answers on the implementation tickets (answers 30–32), the user's two inline
|
||||
comments on its head `4aed8c07` and a clarification in chat (answers 33 and 34),
|
||||
its two inline comments on `e9659903` (answers 35 and 36), and clarifies text the reviews found unclear or stale (improvise-cq1q.1). Answer 33
|
||||
its two later inline comments, also on `4aed8c07` (answers 35 and 36), and clarifies text the reviews found unclear or stale (improvise-cq1q.1). Answer 33
|
||||
reverses the matching order that answer 31 recorded, and answer 34 overrides the
|
||||
ADR's own reading of "space". Answers 35 and 36 make measures the normal formula
|
||||
target and give them an id/display split, which reopens Q23. It asks three new
|
||||
questions, Q23 (how a formula-target measure carries its id, and how a formula line
|
||||
spells its target), Q24 and Q25 (two details of slugify's whitespace rule), and
|
||||
flags nine places where it states a reading of its own that is not a user answer,
|
||||
flags eleven places where it states a reading of its own that is not a user answer,
|
||||
listed under Q23. ·
|
||||
**Decided by:** the user; see "What the user decided" ·
|
||||
**Ticket:** improvise-sboz · **Unblocks:** improvise-fz77 ·
|
||||
@@ -376,12 +376,16 @@ answers 31 and 33 differ, answer 33 governs.
|
||||
|
||||
Clause 12 is rewritten to match.
|
||||
|
||||
### Eighth round: answers on revision 9
|
||||
### Eighth round: answers on revision 9 (head `4aed8c07`)
|
||||
|
||||
The user answered revision 9 (`context/adr/007-stable-identities.md` at `4aed8c07`)
|
||||
with two inline comments on PR #326, posted 2026-10-04 02:48Z and 02:49Z, and then
|
||||
clarified the second one in chat (not on the PR, relayed to the steward). Each is
|
||||
mapped to the line it is attached to, and quoted verbatim.
|
||||
with four inline comments on PR #326: two posted 2026-10-04 02:48Z and 02:49Z, and
|
||||
two posted 2026-10-05 20:22:52Z and 20:23:40Z. The GitHub API gives all four the
|
||||
original commit `4aed8c07`. **All four are one round**: the user wrote the last two
|
||||
before `e9659903`, which folded in the first two, existed, so they had not seen
|
||||
answers 33 and 34 written up. The second comment was also clarified in chat (not on
|
||||
the PR, relayed to the steward). Each is mapped to the line it was attached to on
|
||||
`4aed8c07`, and quoted verbatim.
|
||||
|
||||
**33. The order of answer 31.** On line 356, the sentence under answer 31, "A typed
|
||||
name is matched against displays first, then against ids":
|
||||
@@ -413,16 +417,14 @@ This **overrides the ADR's own reading** (revision 9's, which was not a user ans
|
||||
4 is **the ADR's own reading of "whitespace"**, flagged there. Answers 18 and 21
|
||||
keep their wording, which this reads as "space, meaning whitespace".
|
||||
|
||||
### Ninth round: answers on revision 9's head `e9659903`
|
||||
|
||||
The user posted two inline comments on PR #326 at 2026-10-05 20:22:52Z and 20:23:40Z,
|
||||
both on the line holding Q23's option (c). Each is quoted verbatim.
|
||||
|
||||
**35. Measures get the id/display split too.**
|
||||
**35. Measures get the id/display split too.** On line 1091, the end of Q23's
|
||||
option (c), "Always the target's id, giving a `_Measure` target an id of its own that is
|
||||
distinct from the formula's. This reverses clause 1's 'its handle is its formula's
|
||||
`FormulaId`'":
|
||||
|
||||
> "Measures should have ID/display split too. Measures are the normal case for formulas, anyways: something like `profit = revenue - cost`"
|
||||
|
||||
**36. Dimensions are rarely formula targets.**
|
||||
**36. Dimensions are rarely formula targets.** Also on line 1091, 48 seconds later:
|
||||
|
||||
> "Dimensions are mainly in formulas when you're calculating some kind of derived dimension, but usually dimensions should be relatively static and not interesting in formulas."
|
||||
|
||||
@@ -440,8 +442,10 @@ things in earlier text meet them, and none is resolved here:
|
||||
an id and a display. A measure that exists only because a formula targets it has
|
||||
its display on the `Formula` and its id in the `FormulaId`. Whether that is the
|
||||
split the user wants, or whether the target should be an item, is Q23.
|
||||
- **Answer 3** ("every reference is an id") sits badly with revision 9's line for the
|
||||
normal case, where the target position holds a bare display.
|
||||
- **Answer 3** ("every reference is an id") is about references. Clause 5 calls a
|
||||
formula line a declaration, so the bare display in its target position is a
|
||||
declaration's display and not a reference. Whether the normal case should still
|
||||
spell an id there is part of Q23.
|
||||
- **Answers 5 and 19** picture the measure case as a display plus one id
|
||||
(`Profit(3)`, `Profit (profit)`), which fits an id/display split.
|
||||
- **Answer 30** and the ADR's worked example (`Tax` in `CustomCat`) are about the
|
||||
@@ -552,7 +556,8 @@ Following answers 1 to 3:
|
||||
user answer), where `ItemHandle` is clause 6's
|
||||
`{Item(ItemId), FormulaTarget(FormulaId)}`. Outside `_Measure` every coordinate is
|
||||
an `Item`. A `_Measure` formula cell's coordinate is `FormulaTarget`, because
|
||||
that target has no item. The key stays sorted, now **by category id**, so gotcha
|
||||
that target has no item (**pending Q23**: option (c) gives it an item, and this
|
||||
reading falls). The key 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. Answer 22 makes every id unique across kinds,
|
||||
so an id's text alone names one thing; the tag is there so that the types
|
||||
@@ -938,7 +943,8 @@ Following answer 3, `RecordHash` hashes the explicit serialization of the sorted
|
||||
over different strings. The text is enough: answer 22 makes ids unique across
|
||||
kinds, so no `Item` and `FormulaTarget` can spell alike, and the hash stays
|
||||
canonical. A record is the key minus `_measure`, so a `FormulaTarget` coordinate
|
||||
is never part of a record in the first place.
|
||||
is never part of a record in the first place. **Pending Q23**: under option (c) no
|
||||
`FormulaTarget` exists, and this paragraph reduces to ids over `ItemId`s.
|
||||
- **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`
|
||||
@@ -1013,7 +1019,8 @@ Following answers 15 and 25:
|
||||
1. **Remove legacy JSON** (improvise-tt22). Done, in PR #224.
|
||||
2. **Types:** the id newtypes over `Symbol`, the `display` fields, `FormulaId`,
|
||||
`ViewId`, and `ItemHandle`, which the re-key (step 3) needs for `CellKey` before
|
||||
clause 6's step (5) uses it in projections.
|
||||
clause 6's step (5) uses it in projections (**pending Q23**: under option (c)
|
||||
there is no `ItemHandle`).
|
||||
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 literal (clause 10), the grammar in clause 5, the
|
||||
@@ -1238,27 +1245,49 @@ dimension case, so the two cases put different kinds of thing in one position.
|
||||
collide with it comes out bare: `Profit` → `profit`, not `profit-<n>`.
|
||||
`- Profit(profit) = revenue - cost [_measure]`, and a dimension target is
|
||||
`- tax(tax-1) = … [customcat]`. References to the measure are `profit`. This is
|
||||
the smallest change, and matches the pictures in answers 5 and 19. Whether it
|
||||
*is* the split the user asks for is the thing to decide. Its cost is that the
|
||||
normal case writes a bare display in a position that holds an id for the rare one,
|
||||
which sits against answer 3.
|
||||
the smallest change. Its cost is that the normal case writes a display in a
|
||||
position that holds an id for the rare one. Under (a) a measure does have an id
|
||||
and a display; whether that is the split the user asks for is the thing to decide.
|
||||
- **(b) Always a display, then ids in tags.** Measure: `- Profit(profit) = … [_measure]`.
|
||||
Dimension: `- Tax(tax-1)(tax) = … [customcat]`. The target position never mixes
|
||||
kinds. A dimension line gains a second tag.
|
||||
- **(c) A formula-target measure is a real `_Measure` item**, like a dimension target
|
||||
under answer 30: its own `ItemId` and display, and a separate `FormulaId`
|
||||
(`profit`, then `profit-<n>`). The target position is the item id in every case:
|
||||
`- profit(profit-1) = revenue - cost [_measure]`. This is the reading closest to
|
||||
"measures have an id/display split too". It removes `ItemHandle::FormulaTarget`,
|
||||
`- profit(profit-1) = revenue - cost [_measure]`. It removes `ItemHandle::FormulaTarget`,
|
||||
returns `CellKey` to `(CategoryId, ItemId)`, and ends the `CellKey` typing problem
|
||||
Codex raised on this PR. It reverses clause 1's "no item of its own" and revision
|
||||
9's readings in clauses 1, 2, 5 and 8. It also needs the item and its display
|
||||
written somewhere: today only non-formula `_Measure` items persist, and formula
|
||||
targets are rebuilt from `## Formulas`.
|
||||
|
||||
Answers 5 and 19 picture the measure case as `Profit(3)` and `Profit (profit)`. Both
|
||||
pictures fit (a), (b) and (c): each gives the measure a display and an id, and under
|
||||
(c) the `profit` in answer 19's picture is the item id.
|
||||
|
||||
**No recommendation is made until the user chooses.** Revision 9 recommended (a),
|
||||
written when the dimension case was the one in view. (c) is the literal reading of
|
||||
answer 35 and (a) is the minimal change.
|
||||
written when the dimension case was the one in view. Any ranking of how well an option
|
||||
fits answer 35 would be the ADR's own judgement and not the user's, since (a) and (b)
|
||||
also give a measure an id and a display. (a) is the smallest change.
|
||||
|
||||
Four further points the choice must settle, all the ADR's own reading and none a
|
||||
user answer:
|
||||
- **A formula targeting a stored measure of the same name.** `measure_item_names`
|
||||
merges them today, so a stored `Profit` column and a formula `Profit = …` are one
|
||||
measure. Under (a) and (b) that one column has two ids (the item's `profit` and the
|
||||
formula's `profit-<n>`), and the ADR must say which one keys and references use.
|
||||
Under (c) it is the natural case, and the formula attaches to the existing item.
|
||||
- **Removing a formula under (c).** `RemoveFormula` drops the formula and leaves its
|
||||
`_Measure` item, as it leaves a dimension target's item today (answer 30). The
|
||||
measure column then has no formula and possibly no data.
|
||||
- **The `[_measure]` tag.** Today's writer omits the tag for a `_Measure` formula
|
||||
(`- {raw}`), and the user's own example, `profit = revenue - cost`, has none. The
|
||||
examples above write it only to show the category. Whether the tag is omitted for
|
||||
`_measure` is not decided here.
|
||||
- **Other ADRs.** ADR 008 (PR #308) anchors the cursor by item identity, which under
|
||||
(a) or (b) a formula-measure column does not have, so it would need `ItemHandle`.
|
||||
ADR 009 (PR #309) relies on `find_item_category`'s `_Measure` fallback for formula
|
||||
targets, which (c) would remove. Neither ADR is edited here.
|
||||
|
||||
Two consequences, both the ADR's own reading, for whichever is chosen:
|
||||
- **References to a formula target** are the id text of its handle: under (a) and
|
||||
@@ -1271,24 +1300,31 @@ Two consequences, both the ADR's own reading, for whichever is chosen:
|
||||
|
||||
**Q24. Does `a_ b` slug to `a__b`?** Answer 34 collapses a run of whitespace to one
|
||||
`_`, and the ADR applies the steps as written, so an underscore the display already
|
||||
had is not merged with the one a run makes. The user did not say.
|
||||
had is not merged with the one a run makes. The user did not say. The author's
|
||||
lean, not the user's, is (a): it is what the user's wording ("consecutive spaces")
|
||||
and PR #318 give.
|
||||
- **(a) Keep it.** `a_ b` → `a__b`, and a deliberate `a__b` stays.
|
||||
- **(b) Collapse any run of whitespace and `_` to one `_`.** `a_ b` → `a_b`, and
|
||||
`a__b` → `a_b`. That changes ids for displays that hold a double underscore.
|
||||
|
||||
A neighbouring edge: step 2 runs before step 3 strips non-alphabet characters, so
|
||||
`Time \u{200B} Period` (a zero-width space between two spaces) slugs to
|
||||
`time__period`, two runs and not one.
|
||||
|
||||
**Q25. Does a trailing run slug to a trailing `_`?** Step 4 removes leading `_` and
|
||||
nothing removes trailing ones, so `East ` → `east_`. The user did not say.
|
||||
nothing removes trailing ones, so `East ` → `east_`. The user did not say. The
|
||||
author's lean, not the user's, is (a).
|
||||
- **(a) Keep it.** It is what PR #318 does.
|
||||
- **(b) Remove trailing `_` as step 4 removes leading `_`.** `East ` → `east`, and
|
||||
also `East_` → `east`.
|
||||
- **(c) Trim whitespace before step 2.** `East ` → `east`, and `East_` stays `east_`.
|
||||
|
||||
Revision 9 states a reading of its own, which is not a user answer, in each of these
|
||||
nine places (three of them hold two readings). Each is flagged where it appears, and
|
||||
eleven places (three of them hold two readings). Each is flagged where it appears, and
|
||||
approving the PR approves them:
|
||||
- the virtual views' reserved ids `_records` and `_drill` (clause 1);
|
||||
- the `_Index`/`_Dim` coordinates are outside the id namespace (clause 1);
|
||||
- a `CellKey` coordinate is `(CategoryId, ItemHandle)` (clause 2);
|
||||
- a `CellKey` coordinate is `(CategoryId, ItemHandle)` (clause 2; **pending Q23**);
|
||||
- `collapsed:` is `(CategoryId, group name)` (clause 2);
|
||||
- "whitespace" means the Unicode `White_Space` property (replacing the U+0020 reading,
|
||||
by answer 34), and a slug of only `-` is a legal id (clause 4);
|
||||
@@ -1298,7 +1334,10 @@ approving the PR approves them:
|
||||
(clause 5; **reopened by answers 35 and 36, Q23**);
|
||||
- CheckedBatch's created-record exemption is removed (clause 6);
|
||||
- a display equal to another thing's id counts as ambiguous in the formula panel
|
||||
(clause 6).
|
||||
(clause 6);
|
||||
- an id is matched exactly, with no case folding (clause 12);
|
||||
- the display stage of the typed-name resolver goes once autocomplete exists
|
||||
(clause 12).
|
||||
|
||||
## Consequences
|
||||
|
||||
|
||||
Reference in New Issue
Block a user