From bf935d89bf6574dd4297dc5bc5a1076b93b2a486 Mon Sep 17 00:00:00 2001 From: Mathias Date: Wed, 23 Sep 2026 07:28:11 +0200 Subject: [PATCH] fix(wiki): normalise heading casing/whitespace before matching in Merge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- ingestion/internal/wiki/merge.go | 39 +++++++++++++++++++++++---- ingestion/internal/wiki/merge_test.go | 36 +++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 5 deletions(-) diff --git a/ingestion/internal/wiki/merge.go b/ingestion/internal/wiki/merge.go index 644035a..f41b53b 100644 --- a/ingestion/internal/wiki/merge.go +++ b/ingestion/internal/wiki/merge.go @@ -66,21 +66,22 @@ func MergeReport(a, b Page) (Page, []string) { var conflicts []string idx := make(map[string]int, len(secsA)) for i, s := range secsA { - idx[s.heading] = i + idx[normalizeHeading(s.heading)] = i } for _, sB := range secsB { - i, exists := idx[sB.heading] + key := normalizeHeading(sB.heading) + i, exists := idx[key] if !exists { - idx[sB.heading] = len(secsA) + idx[key] = len(secsA) secsA = append(secsA, sB) continue } sA := secsA[i] switch { - case bulletSections[sB.heading]: + case bulletSectionsNorm[key]: secsA[i].content = mergeBullets(sA.content, sB.content) - case appendSections[sB.heading]: + case appendSectionsNorm[key]: secsA[i].content = appendNew(sA.content, sB.content) default: // a wins — but say that b disagreed, rather than dropping it mute. @@ -93,6 +94,34 @@ func MergeReport(a, b Page) (Page, []string) { return Page{Path: a.Path, Content: rebuildContent(refreshOwned(fmA, fmB), preambleA, secsA)}, conflicts } +// normalizeHeading returns the key Merge uses to decide "is this the same +// section" — trimmed and case-folded, so "Key Claims" and "Key claims" match +// (hyperguild#106: heading casing drift must not fork a section). It is used +// only for that comparison; the on-disk section's original heading text +// (secsA[i].heading, untouched here) is what actually gets written out, so a +// human's heading casing is never silently rewritten by a merge. +func normalizeHeading(h string) string { + return strings.ToLower(strings.TrimSpace(h)) +} + +// bulletSectionsNorm and appendSectionsNorm are bulletSections/appendSections +// keyed by normalizeHeading instead of exact text, so a section's merge +// strategy (union bullets, append) is chosen by the same normalized identity +// Merge uses to match it — otherwise a heading that matched an on-disk +// section (case-insensitively) could still fail the exact-string strategy +// lookup and silently fall through to "keep a's version," dropping b's +// content instead of unioning or appending it. +var bulletSectionsNorm = normalizeSet(bulletSections) +var appendSectionsNorm = normalizeSet(appendSections) + +func normalizeSet(names map[string]bool) map[string]bool { + out := make(map[string]bool, len(names)) + for k := range names { + out[normalizeHeading(k)] = true + } + return out +} + // generatorOwned names the frontmatter fields the ingest pipeline may refresh // on a page it is revising. Everything else on disk wins. // diff --git a/ingestion/internal/wiki/merge_test.go b/ingestion/internal/wiki/merge_test.go index 4286ff3..8386815 100644 --- a/ingestion/internal/wiki/merge_test.go +++ b/ingestion/internal/wiki/merge_test.go @@ -305,6 +305,42 @@ Rewritten by the model. assert.Equal(t, want, got) } +// --- hyperguild#106: heading casing drift must not fork a section ---------- +// Page-aware extraction asks the model to revise the page it's given, but +// nothing guarantees the model reproduces a heading's exact on-disk casing +// ("Key Claims" vs "Key claims"). Without normalizing the match key, that +// drift silently creates a near-duplicate section instead of updating the +// original — the failure mode #106 exists to close. + +func TestMerge_HeadingCaseDriftMatchesSameSection(t *testing.T) { + a := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\n## Key Claims\n\n- Appetite beats estimation.\n"} + b := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\n## Key claims\n\n- New pattern holds under load.\n"} + + got := Merge(a, b).Content + + assert.Equal(t, 1, strings.Count(got, "## Key"), + "casing drift in the heading must not create a second section") + assert.Contains(t, got, "Appetite beats estimation.") + assert.Contains(t, got, "New pattern holds under load.", + "the differently-cased section's content must still be unioned in") + assert.Contains(t, got, "## Key Claims", + "the on-disk heading's original casing must be preserved in the output") + assert.NotContains(t, got, "## Key claims", + "the fresh extraction's heading casing must not overwrite the on-disk display text") +} + +func TestMerge_HeadingWhitespaceDriftMatchesSameSection(t *testing.T) { + a := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\n## Open Questions \n\n- Does appetite scale?\n"} + b := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\n## Open Questions\n\n- What about remote teams?\n"} + + got := Merge(a, b).Content + + assert.Equal(t, 1, strings.Count(got, "## Open Questions"), + "surrounding whitespace drift in the heading must not create a second section") + assert.Contains(t, got, "Does appetite scale?") + assert.Contains(t, got, "What about remote teams?") +} + func TestRefreshOwned_CommentLinesSurviveUnmangled(t *testing.T) { // Third review round on PR #126: parseFrontmatterBlock dropped the // sibling api package's comment guard (strings.HasPrefix(trimmed, "#")).