fix(wiki): normalise heading casing/whitespace before matching in Merge
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>
This commit is contained in:
@@ -66,21 +66,22 @@ func MergeReport(a, b Page) (Page, []string) {
|
|||||||
var conflicts []string
|
var conflicts []string
|
||||||
idx := make(map[string]int, len(secsA))
|
idx := make(map[string]int, len(secsA))
|
||||||
for i, s := range secsA {
|
for i, s := range secsA {
|
||||||
idx[s.heading] = i
|
idx[normalizeHeading(s.heading)] = i
|
||||||
}
|
}
|
||||||
|
|
||||||
for _, sB := range secsB {
|
for _, sB := range secsB {
|
||||||
i, exists := idx[sB.heading]
|
key := normalizeHeading(sB.heading)
|
||||||
|
i, exists := idx[key]
|
||||||
if !exists {
|
if !exists {
|
||||||
idx[sB.heading] = len(secsA)
|
idx[key] = len(secsA)
|
||||||
secsA = append(secsA, sB)
|
secsA = append(secsA, sB)
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
sA := secsA[i]
|
sA := secsA[i]
|
||||||
switch {
|
switch {
|
||||||
case bulletSections[sB.heading]:
|
case bulletSectionsNorm[key]:
|
||||||
secsA[i].content = mergeBullets(sA.content, sB.content)
|
secsA[i].content = mergeBullets(sA.content, sB.content)
|
||||||
case appendSections[sB.heading]:
|
case appendSectionsNorm[key]:
|
||||||
secsA[i].content = appendNew(sA.content, sB.content)
|
secsA[i].content = appendNew(sA.content, sB.content)
|
||||||
default:
|
default:
|
||||||
// a wins — but say that b disagreed, rather than dropping it mute.
|
// 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
|
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
|
// generatorOwned names the frontmatter fields the ingest pipeline may refresh
|
||||||
// on a page it is revising. Everything else on disk wins.
|
// on a page it is revising. Everything else on disk wins.
|
||||||
//
|
//
|
||||||
|
|||||||
@@ -305,6 +305,42 @@ Rewritten by the model.
|
|||||||
assert.Equal(t, want, got)
|
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) {
|
func TestRefreshOwned_CommentLinesSurviveUnmangled(t *testing.T) {
|
||||||
// Third review round on PR #126: parseFrontmatterBlock dropped the
|
// Third review round on PR #126: parseFrontmatterBlock dropped the
|
||||||
// sibling api package's comment guard (strings.HasPrefix(trimmed, "#")).
|
// sibling api package's comment guard (strings.HasPrefix(trimmed, "#")).
|
||||||
|
|||||||
Reference in New Issue
Block a user