fix(wiki): first-occurrence wins when heading normalization collides pre-existing duplicates
CI / Lint / Test / Vet (pull_request) Successful in 34s
CI / Mirror to GitHub (pull_request) Skipped

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>
This commit is contained in:
2026-09-23 07:38:43 +02:00
co-authored by Claude Sonnet 5
parent bf935d89bf
commit 921bea8738
2 changed files with 38 additions and 1 deletions
+26
View File
@@ -341,6 +341,32 @@ func TestMerge_HeadingWhitespaceDriftMatchesSameSection(t *testing.T) {
assert.Contains(t, got, "What about remote teams?")
}
func TestMerge_PreExistingCaseDuplicateHeadingsRouteToTheFirstOccurrence(t *testing.T) {
// Found in review of PR #130: normalizeHeading's idx map is built by
// iterating secsA with idx[normalizeHeading(s.heading)] = i -- last
// write wins. Before heading normalization existed, two on-disk
// sections differing only by case (real, pre-existing corpus drift --
// the exact pattern #106 itself cites) never collided, since idx keyed
// on the exact string. After normalization they collide, and every
// fresh update silently, deterministically routes to whichever
// duplicate happens to appear LAST in the file -- the first one is
// frozen forever, never receiving updates again, with nothing to say
// so. A Contains-only check doesn't catch this: nothing is literally
// deleted, a bullet just lands under the wrong of two still-present
// headings. Assert it lands under the FIRST occurrence instead —
// deterministic and matches "earliest is canonical," same reasoning
// as every other on-disk-wins default in this file.
a := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\n## Key Claims\n\n- claim one\n\n## KEY CLAIMS\n\n- claim two\n"}
b := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\n## Key Claims\n\n- claim one\n- fresh claim\n"}
got := Merge(a, b).Content
assert.Contains(t, got, "## Key Claims\n\n- claim one\n- fresh claim\n",
"the fresh bullet must land in the first-occurrence section, not silently route to the last duplicate")
assert.Contains(t, got, "## KEY CLAIMS\n\n- claim two\n",
"the second duplicate is untouched by this update, not merged into or overwritten")
}
func TestRefreshOwned_CommentLinesSurviveUnmangled(t *testing.T) {
// Third review round on PR #126: parseFrontmatterBlock dropped the
// sibling api package's comment guard (strings.HasPrefix(trimmed, "#")).