From 2b086b205ad99346c146530b38ba224f4d6964a2 Mon Sep 17 00:00:00 2001 From: Edward Langley Date: Mon, 21 Sep 2026 13:49:58 -0700 Subject: [PATCH] test: parse_duplicate_categories could not fail for the merge it names Found by an external Grok Bot adversarial review (improvise-jkxr), verified by injection rather than taken on report. The test parses two `## Category: Type` sections -- `- A` then `- B` -- and asserted only `!item_names.is_empty()`. push_category concatenates on a duplicate section name, and a merge keeping EITHER half alone still leaves the list non-empty, so only a total wipe could ever have failed it. Fault injected: `existing.items.extend(items)` -> `existing.items = items`, so a second section replaces the first. The old assertion PASSED; the new one fails with left ["B"], right ["A", "B"]. Pins order as well as membership -- insertion order is load-bearing (gotcha 18) -- matching the standard its neighbour already holds groups to. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH --- context/test-evidence/formula-and-io.md | 22 ++++++++++++++++++++ crates/improvise-io/src/persistence/parse.rs | 2 +- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/context/test-evidence/formula-and-io.md b/context/test-evidence/formula-and-io.md index bcfaed99..4fe396c3 100644 --- a/context/test-evidence/formula-and-io.md +++ b/context/test-evidence/formula-and-io.md @@ -777,3 +777,25 @@ fixture had a single `Month`, so it demonstrated the row half and left the column half asserted by nothing -- the same defect one axis over. `Feb` is now the empty column, the precondition is a pair on each layout ((2, 1) drawn against (3, 2) unpruned), and the CSV header is asserted to carry `Jan` alone. + +### persistence::parse::parser_edge_cases::parse_duplicate_categories + +Found by an external Grok Bot adversarial review (improvise-jkxr), and the +finding was verified by injection rather than taken on report. + +The test parses two `## Category: Type` sections — `- A` then `- B` — and +asserted only `!item_names.is_empty()`. That cannot fail for the defect the +test's name and placement imply. `ParsedFile::push_category` concatenates on a +duplicate section name (`invariant(duplicate_category_sections_merge_in_pass_one)`), +and a merge that kept *either* half alone still leaves the list non-empty. + +**Fault injected:** `existing.items.extend(items)` → `existing.items = items`, +so a second section *replaces* the first instead of merging. The old assertion +**passed**. The strengthened assertion fails with `left: ["B"], right: ["A", "B"]`. +Only a total wipe of both sections could ever have failed the original. + +The assertion is now `assert_eq!(item_names, ["A", "B"])`, which pins order as +well as membership — insertion order is load-bearing here (gotcha 18), and the +neighbouring `two_sections_naming_one_category_round_trip_to_a_stable_group_order` +already holds groups to that standard. This test was the item-list half of the +same invariant, held to a weaker one. diff --git a/crates/improvise-io/src/persistence/parse.rs b/crates/improvise-io/src/persistence/parse.rs index c5408227..46fb303e 100644 --- a/crates/improvise-io/src/persistence/parse.rs +++ b/crates/improvise-io/src/persistence/parse.rs @@ -1098,7 +1098,7 @@ mod parser_edge_cases { let m = parse_md(text).unwrap(); let cat = m.model.category("Type").unwrap(); let item_names: Vec<&str> = cat.items.values().map(|i| i.name.as_str()).collect(); - assert!(!item_names.is_empty()); + assert_eq!(item_names, ["A", "B"], "the merge dropped a section's items"); } #[test]