From 35adf6675708bcdf96495ec40bb70cedaccaf66b Mon Sep 17 00:00:00 2001 From: Edward Langley Date: Sun, 20 Sep 2026 21:09:49 -0700 Subject: [PATCH] docs: the duplicate-section merge and the WHERE boundary, with their evidence context/code/formula-and-io.md records why the category merge happens in pass 1 rather than pass 2, and why the WHERE boundary is sp+ rather than a lookahead -- the grammar-walking generator cannot honour a predicate, which is the part that generalises. context/test-evidence records the fault injected for each test, including the one that made the WHERE test carry two negative shapes: compound-atomicity alone already refuses the ticket's own reproducer, so a test built only on it passes over a rule still spelled sp*. improvise-349m improvise-r08u Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH --- context/code/formula-and-io.md | 34 ++++++++++++++++++ context/test-evidence/formula-and-io.md | 46 ++++++++++++++++++++++--- 2 files changed, 76 insertions(+), 4 deletions(-) diff --git a/context/code/formula-and-io.md b/context/code/formula-and-io.md index 36375ee8..a511f3a2 100644 --- a/context/code/formula-and-io.md +++ b/context/code/formula-and-io.md @@ -92,6 +92,28 @@ round-tripped from the day it exists. What the *evaluator* does with it, and why a fitted tree is always wrapped in a `SUM`, is `context/code/core.md` §13. +### `WHERE` carries the same boundary, and the mechanism has to be generatable + +`where_clause` and `inline_where` are compound-atomic with `sp+` after the +keyword, exactly as `is_expr` is (improvise-r08u). Non-atomic, pest auto-skipped +`WHITESPACE` after `^"WHERE"`, so `SUM(Rev WHEREAS x = y)` read as a `WHERE` on +a category `AS` — a silently wrong filter where it should have been a syntax +error, the same class of defect `IS` had. + +Two things decided the shape, and the second is the one worth carrying. A +negative lookahead `!bare_char` with `sp*` would have been *strictly* more +permissive — it keeps `WHERE|Region|=East`, which the old rule accepted — but +**the grammar-walking generator cannot honour a predicate**: it walks +`formula.pest` through `pest_meta` and emits `WHEREa=""`, text the grammar then +refuses, so `generated_formula_parses` goes red. A rule whose only guard is a +lookahead is a rule the generator can violate. `sp+` is generatable, matches +`IS`, and costs exactly one accepted spelling: a category may no longer abut the +keyword, so `WHERE |Region|` needs its space. + +Note compound-atomicity *alone* closes the ticket's own reproducer, because +`AS x` cannot be one identifier once implicit whitespace is gone. `sp+` is what +closes the residue, `WHEREAS=y`, and the negative test names both. + ## A formula target is unquoted, exactly as a reference is `parse_formula` strips the pipes off the target the same way it does off an @@ -158,6 +180,18 @@ format order-independent — a `## View:` may name a category defined further down. `PCategory::items` is a `Vec` in file order because `IndexMap` insertion order is load-bearing (gotcha 18), so pass 2 must replay the list as written. +Two `## Category:` sections naming one category are **one** category, and the +merge is a concatenation performed in pass 1 (`ParsedFile::push_category`, +improvise-349m). Merging in pass 2 — building each section in turn — was not +idempotent. A group is created either by the item that names it or by a `>` +line, and `build_category` creates its own section's item-groups *before* its +own `>` lines; so the first section's `>` groups landed ahead of the second +section's item-groups, while `format_md` writes one section with all the items +before all the `>` lines. The reparse then built the other order, and the two +`format_md` outputs differed. Concatenating first makes the built order the +written order by construction. Refusing a duplicate header was the alternative +and would have made a legal hand-written file an error for no gain. + ### A section header takes its name as the whole line, totally `header_name = { pipe_quoted ~ &NEWLINE | line_name }` (improvise-pczv). The diff --git a/context/test-evidence/formula-and-io.md b/context/test-evidence/formula-and-io.md index d16a5963..6d44d887 100644 --- a/context/test-evidence/formula-and-io.md +++ b/context/test-evidence/formula-and-io.md @@ -203,6 +203,23 @@ backend from quietly normalising or re-encoding what it is handed. ## `improvise-io/src/persistence/parse.rs` +### two_sections_naming_one_category_round_trip_to_a_stable_group_order + +improvise-349m: `format_md(parse_md(text))` was not idempotent when a file +carried two `## Category:` sections with the same name — the two outputs +differed only in the order of the `>` group lines. It looked like a flake +because only `grammar_prop_tests::generated_file_roundtrips` saw it, and only +when the generator happened to emit a duplicate header; taking proptest's +minimal input and running it deterministically is what turned it into a bug. + +The fixture is the smallest shape that shows it: a first section defining a +group by a `>` line only, and a second whose *item* names a different group that +also gets a `>` line. Both groups must have parents, or `format_md` writes only +one `>` line and the differing internal order is invisible in the text — which +is the trap when shrinking this fixture further. Fault injected: reverting +`ParsedFile::push_category` to a plain `Vec::push` fails the assertion, and the +ticket's own minimal input was checked against the fix as well. + ### parse_md_formula_category_suffix_without_space Bug improvise-zgb (b). The formulas walker located the `[Category]` suffix with @@ -646,7 +663,28 @@ reopens the hole; that substitution is what this test was finally proved against. The negative arm is paired with a positive one, or "reject everything" would pass. -`^"WHERE"` has the identical hole and was deliberately **not** fixed here: -pre-existing, far less exposed (it sits in one fixed position after an -expression, where `IS` follows any identifier in expression position), and a -grammar change with its own round-trip risk. Filed as improvise-r08u. +`^"WHERE"` had the identical hole. It is closed now — see below. + +### where_does_not_swallow_the_start_of_a_longer_word + +improvise-r08u, the `WHERE` half of the same defect. `where_clause` and +`inline_where` were ordinary rules, so pest auto-inserted `WHITESPACE` after +`^"WHERE"` and `SUM(Rev WHEREAS x = y)` parsed as a `WHERE` on a category `AS`: +a silently wrong filter where it should have been a syntax error. + +The test carries **two** negative shapes because the repair has two independent +halves, and this was found by injecting each separately. Making the rules +compound-atomic alone already refuses `WHEREAS x = y` — once implicit whitespace +is gone, `AS x` cannot be one identifier — so a test built only on the ticket's +reproducer passes over a rule still spelled `sp*`, and `WHEREAS=y` is what that +rule still reads as a filter. Faults injected: the original non-atomic rule +(fails on the first assertion) and `sp+` relaxed to `sp*` (fails on the second). +The positive arms are there so "refuse every WHERE" cannot pass. + +### where_requires_a_space_after_the_keyword + +The one spelling `sp+` costs, stated rather than discovered later: +`WHERE|Region Name|=|East Coast|` used to parse and now does not. `IS` has +required its space since improvise-4zl7, so this is the two keywords agreeing. +The paired positive arm — the same text with the space — is what stops it +degrading into "any WHERE with a quoted category fails".