fix(wiki): stop Merge from silently deleting on-disk content it can't parse
Review of PR #103 found a real data-loss bug: parseSections only captured text between `## ` headings. Content before the first heading — an intro paragraph, or, if the page had no headings at all, the entire body — was dropped on the floor, not carried into any section. Merge then had nothing of `a` to keep there, so the fresh extraction's sections silently became the whole output wherever that content used to be. Exactly the failure this file exists to prevent, just re-triggered by a page shape the parser didn't expect instead of by every re-ingest. Fixed by treating pre-first-heading content as a section with an empty heading. Merge's switch never matches "", so it falls through to the same on-disk-wins default every other unlisted section already gets — no new merge path, just no longer discarding this one's input. Second, separate finding from the same review: mergeBullets/appendNew deduped on the raw trimmed line, so two re-extractions of the same logical bullet with different internal whitespace (a real possibility — the extractor is an LLM, not a fixed serializer) produced a duplicate instead of a no-op, undermining the idempotency this file's own doc comments claim. Fixed by comparing a whitespace-canonicalized key (strings.Fields + single-space join) while still emitting the original line's text. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -78,21 +78,26 @@ func parseSections(markdown string) (frontmatter string, sections []section) {
|
||||
frontmatter = fmt.Sprintf("---\n%s\n---\n", strings.Join(fmLines, "\n"))
|
||||
}
|
||||
|
||||
var cur *section
|
||||
// Everything before the first `## ` heading — an intro paragraph, or, if
|
||||
// the page has no headings at all, the entire body — is ordinary content,
|
||||
// not something this parser is entitled to drop. It becomes a section
|
||||
// with an empty heading, which Merge's switch never matches, so it falls
|
||||
// through to the same "keep a's version" default every other unlisted
|
||||
// section gets (hyperguild#103 review: dropping it here meant Merge, one
|
||||
// caller up, would silently replace the whole on-disk body with the
|
||||
// fresh extraction whenever a page didn't follow the ## convention —
|
||||
// exactly the class of bug this whole file exists to prevent).
|
||||
cur := §ion{}
|
||||
for ; i < len(lines); i++ {
|
||||
line := lines[i]
|
||||
if strings.HasPrefix(line, "## ") {
|
||||
if cur != nil {
|
||||
sections = append(sections, *cur)
|
||||
}
|
||||
cur = §ion{heading: strings.TrimPrefix(line, "## ")}
|
||||
} else if cur != nil {
|
||||
} else {
|
||||
cur.content += line + "\n"
|
||||
}
|
||||
}
|
||||
if cur != nil {
|
||||
sections = append(sections, *cur)
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
@@ -109,6 +114,18 @@ func rebuildContent(frontmatter string, sections []section) string {
|
||||
sb.WriteString(frontmatter)
|
||||
for _, sec := range sections {
|
||||
body := strings.Trim(sec.content, "\n")
|
||||
if sec.heading == "" {
|
||||
// The preamble pseudo-section: content before the first ##. No
|
||||
// heading of its own to re-emit, and — unlike a named section —
|
||||
// it is usually empty (most pages start directly with a ##), so
|
||||
// an empty one must produce nothing rather than a stray blank
|
||||
// line ahead of the first real heading.
|
||||
if body == "" {
|
||||
continue
|
||||
}
|
||||
fmt.Fprintf(&sb, "\n%s\n", body)
|
||||
continue
|
||||
}
|
||||
fmt.Fprintf(&sb, "\n## %s\n\n%s\n", sec.heading, body)
|
||||
}
|
||||
return sb.String()
|
||||
@@ -131,21 +148,30 @@ func rebuildContent(frontmatter string, sections []section) string {
|
||||
// these sections — questions, updates, evolving notes — a repeat is noise
|
||||
// rather than signal, and the failure is conservative: the page stops growing
|
||||
// instead of growing wrongly.
|
||||
// canonicalLine collapses a line to its comparison key: internal whitespace
|
||||
// runs (a re-extraction's bullet spacing, tabs vs spaces, trailing spaces)
|
||||
// must not make the same logical line look new. strings.Fields already
|
||||
// trims and splits on any whitespace run, so joining it back with single
|
||||
// spaces is exactly "same words, same order, whitespace-insensitive."
|
||||
func canonicalLine(line string) string {
|
||||
return strings.Join(strings.Fields(line), " ")
|
||||
}
|
||||
|
||||
func appendNew(a, b string) string {
|
||||
seen := make(map[string]bool)
|
||||
for _, line := range strings.Split(a, "\n") {
|
||||
if t := strings.TrimSpace(line); t != "" {
|
||||
seen[t] = true
|
||||
if c := canonicalLine(line); c != "" {
|
||||
seen[c] = true
|
||||
}
|
||||
}
|
||||
|
||||
var add []string
|
||||
for _, line := range strings.Split(b, "\n") {
|
||||
t := strings.TrimSpace(line)
|
||||
if t == "" || seen[t] {
|
||||
c := canonicalLine(line)
|
||||
if c == "" || seen[c] {
|
||||
continue
|
||||
}
|
||||
seen[t] = true
|
||||
seen[c] = true
|
||||
add = append(add, line)
|
||||
}
|
||||
if len(add) == 0 {
|
||||
@@ -158,11 +184,11 @@ func mergeBullets(a, b string) string {
|
||||
seen := make(map[string]bool)
|
||||
var lines []string
|
||||
for _, line := range strings.Split(a+b, "\n") {
|
||||
trimmed := strings.TrimSpace(line)
|
||||
if trimmed == "" || seen[trimmed] {
|
||||
c := canonicalLine(line)
|
||||
if c == "" || seen[c] {
|
||||
continue
|
||||
}
|
||||
seen[trimmed] = true
|
||||
seen[c] = true
|
||||
lines = append(lines, line)
|
||||
}
|
||||
return strings.Join(lines, "\n") + "\n"
|
||||
|
||||
@@ -54,6 +54,59 @@ func TestMerge_KeepsFrontmatterFromA(t *testing.T) {
|
||||
assert.NotContains(t, got.Content, "title: B")
|
||||
}
|
||||
|
||||
// --- hyperguild#103 review: a page with content outside any `## ` section ---
|
||||
// must never lose that content on merge. parseSections used to only capture
|
||||
// text between `## ` headings — anything before the first heading (a common,
|
||||
// ordinary shape for a hand-written note: an intro paragraph before the first
|
||||
// section) or, worse, an entire page with no `## ` headings at all (a legacy
|
||||
// note, a stub, any hand-edit that doesn't follow the section convention)
|
||||
// vanished silently. Merge would then fill the page entirely from the fresh
|
||||
// extraction — the opposite of "never silently rewrite what a human put
|
||||
// there," the guarantee this whole file exists to provide.
|
||||
|
||||
func TestMerge_PreservesOnDiskContentWithNoSectionHeadings(t *testing.T) {
|
||||
a := Page{Path: "wiki/x.md", Content: "---\ntitle: X\n---\n\nJust some free text a human wrote. No section headings at all.\n"}
|
||||
b := Page{Path: "wiki/x.md", Content: "---\ntitle: X\n---\n\n## Definition\n\nFresh extraction's take.\n"}
|
||||
|
||||
got := Merge(a, b)
|
||||
assert.Contains(t, got.Content, "Just some free text a human wrote",
|
||||
"on-disk content with no ## headings must survive the merge, not be replaced wholesale by the fresh extraction")
|
||||
}
|
||||
|
||||
func TestMerge_PreservesPreambleBeforeFirstHeading(t *testing.T) {
|
||||
a := Page{Path: "wiki/x.md", Content: "---\ntitle: X\n---\n\nAn intro paragraph before the first heading.\n\n## Definition\n\nA thing.\n"}
|
||||
b := Page{Path: "wiki/x.md", Content: "---\ntitle: X\n---\n\n## Definition\n\nA thing.\n"}
|
||||
|
||||
got := Merge(a, b)
|
||||
assert.Contains(t, got.Content, "An intro paragraph before the first heading",
|
||||
"text before the first ## heading is ordinary page content, not something the parser is entitled to drop")
|
||||
}
|
||||
|
||||
// --- hyperguild#103 review: dedup must be whitespace-insensitive -----------
|
||||
// Page content is produced by an LLM per run, so byte-identical re-extraction
|
||||
// of "the same" logical line is not guaranteed — different bullet spacing or
|
||||
// trailing whitespace on an otherwise-identical line must still dedup, or
|
||||
// idempotency only holds when the extractor happens to reproduce whitespace
|
||||
// exactly, which is not a real guarantee.
|
||||
|
||||
func TestMerge_AppendSectionDedupsAcrossWhitespaceVariants(t *testing.T) {
|
||||
a := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\n## Open Questions\n\n- Does this converge?\n"}
|
||||
b := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\n## Open Questions\n\n- Does this converge? \n"}
|
||||
|
||||
got := Merge(a, b)
|
||||
assert.Equal(t, 1, strings.Count(got.Content, "Does this converge?"),
|
||||
"same logical line with different whitespace must dedup, not duplicate")
|
||||
}
|
||||
|
||||
func TestMerge_BulletSectionDedupsAcrossWhitespaceVariants(t *testing.T) {
|
||||
a := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\n## Related Concepts\n\n- [[bar|Bar]]\n"}
|
||||
b := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\n## Related Concepts\n\n- [[bar|Bar]]\n"}
|
||||
|
||||
got := Merge(a, b)
|
||||
assert.Equal(t, 1, strings.Count(got.Content, "[[bar|Bar]]"),
|
||||
"same logical bullet with different whitespace must dedup, not duplicate")
|
||||
}
|
||||
|
||||
// --- brain#21/B1: merging must be safe to run again -------------------------
|
||||
|
||||
func TestMerge_AppendSectionDoesNotDuplicateContentAlreadyPresent(t *testing.T) {
|
||||
|
||||
Reference in New Issue
Block a user