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:
@@ -43,21 +43,22 @@ func Merge(a, b Page) Page {
|
||||
|
||||
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)
|
||||
}
|
||||
}
|
||||
@@ -65,6 +66,34 @@ func Merge(a, b Page) Page {
|
||||
return Page{Path: a.Path, Content: rebuildContent(refreshOwned(fmA, fmB), preambleA, secsA)}
|
||||
}
|
||||
|
||||
// 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.
|
||||
//
|
||||
|
||||
@@ -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, "#")).
|
||||
|
||||
Reference in New Issue
Block a user