diff --git a/ingestion/internal/wiki/merge.go b/ingestion/internal/wiki/merge.go index 628e34f..a233fd1 100644 --- a/ingestion/internal/wiki/merge.go +++ b/ingestion/internal/wiki/merge.go @@ -41,9 +41,19 @@ func Merge(a, b Page) Page { fmA, preambleA, secsA := parseSections(a.Content) fmB, _, secsB := parseSections(b.Content) + // First occurrence wins on a collision — a page can already carry two + // on-disk sections that only differ by case or whitespace (the exact + // corpus drift this normalization exists to catch going forward, found + // pre-existing in review of PR #130): a plain last-write assignment + // here would silently and deterministically route every future update + // to whichever duplicate happens to sort last in the file, freezing + // the other one forever with nothing to say so. idx := make(map[string]int, len(secsA)) for i, s := range secsA { - idx[normalizeHeading(s.heading)] = i + key := normalizeHeading(s.heading) + if _, exists := idx[key]; !exists { + idx[key] = i + } } for _, sB := range secsB { diff --git a/ingestion/internal/wiki/merge_test.go b/ingestion/internal/wiki/merge_test.go index f8c2b35..762293e 100644 --- a/ingestion/internal/wiki/merge_test.go +++ b/ingestion/internal/wiki/merge_test.go @@ -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, "#")).