Review of PR #132 found a real bug in the first-occurrence-wins fix:
routing every one of b's occurrences of a normalized heading to a's
single first occurrence means a self-merge (or a revision pass echoing
a page back unchanged, exactly what #106 can produce) pairs b's SECOND
"## NOTES" against a's FIRST "## Notes" -- different content, same
normalized key -- and MergeReport wrongly reports a disagreement where
none exists. Inert in #130 (fed the vestigial Merge wrapper, which
discards conflicts) and only became visible once correctly wired into
MergeReport, the function production actually calls.
Fixed by pairing occurrences positionally: a's on-disk sections sharing
a normalized heading are collected into an ordered list per key: b's
1st occurrence of that key pairs with a's 1st, b's 2nd with a's 2nd,
and so on. b having MORE occurrences than a appends the extra as new
content and registers it, so a third occurrence in b pairs with that
newly appended one rather than creating yet another duplicate.
Flip-tested: forcing every pairing back to position 0 (the prior fix's
actual behavior) turns the new self-merge test red; the positional
version stays green, and the original PR #130 regression test (fresh
content must route to the first occurrence when b has only one
occurrence) still passes unchanged -- the two fixes aren't in tension,
positional pairing subsumes first-occurrence-wins as the pos==0 case.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Review of PR #130 found a real bug in the new normalizeHeading matching:
idx was built with idx[normalizeHeading(s.heading)] = i, plain map
assignment while iterating secsA — last write wins. A page that already
carries two on-disk sections differing only by case or whitespace (real,
pre-existing corpus drift, the exact pattern #106 itself cites) never
collided in idx before this normalization existed, since it keyed on the
exact string. After normalization they collide, and every future update
silently, deterministically routes to whichever duplicate happens to
sort last in the file — the first one is frozen forever, never
receiving an update again, with nothing to say so. Same bug class
parseSections's own doc comment already warns about (two keys sharing
one map slot means one loses lookup visibility), reintroduced here.
Fixed by keeping only the first occurrence's index when a collision
happens while building idx — deterministic, and matches "earliest is
canonical," the same reasoning behind every other on-disk-wins default
in this file.
New test reproduces the exact review finding with a positional
assertion (the fresh bullet must land adjacent to the first
occurrence's own content, not the second) — a Contains-only version of
this test would have passed even with the bug present, since nothing is
literally deleted, a bullet just silently routes to the wrong of two
still-present sections.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Page-aware extraction (previous commit) asks the model to revise the
page it's given, but nothing guarantees it reproduces a heading's
exact on-disk text. Before this, Merge matched sections by exact
string equality, so "Key Claims" vs "Key claims" (or trailing
whitespace) silently created a parallel section instead of updating
the original — the drift #106 calls out by name.
Merge's idx map and the bulletSections/appendSections strategy lookup
now key on normalizeHeading (trim + lowercase), so casing/whitespace
drift matches the same section. The on-disk heading's original text
(secsA[i].heading) is untouched and is what gets written out — the
normalized form is used only to decide "is this the same section,"
never to rewrite a human's heading casing.
Touches wiki/merge.go, which is explicitly out of casual-edit scope
per today's review history on this file: justified because heading
normalization is #106's own checklist item and has to hook into
exactly this matching logic — there's nowhere else to put it. Scope
kept minimal: the two lookup sites only, no other behavior in this
file changed. bulletSectionsNorm/appendSectionsNorm exist because
matching a section by its normalized key but then classifying its
merge strategy by exact-string lookup would silently fall through to
"keep a's version" for a case-drifted bullet/append heading — dropping
b's content instead of unioning or appending it.
Closes hyperguild#106.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Textual-diff conflict detection only (semantic contradiction detection via brain-gardener Judge deferred, confirmed by Mathias, comment 12594). Includes canonicalBlock whitespace-run normalization fix for the disagreement comparison.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Mathias <mthbqv@gmail.com>
fix(wiki): give the preamble its own field instead of overloading heading=="" (#103)
Round 3 review: verified structurally that the preamble is no longer in the sections slice or the idx map, so it cannot collide with a degenerate `## ` heading. Flip-tested by reverting to the heading=="" model and confirming TestMerge_DegenerateEmptyHeadingDoesNotCollideWithPreamble goes red. Adversarial pass (multiple degenerate headings, frontmatter-only page, unicode around the ## boundary, 3x self-merge idempotency with preamble+degenerate) all held. CI green on 09aeaa9 (run 7820).
Co-authored-by: Mathias <mthbqv@gmail.com>
mathiasandClaude Sonnet 5