diff --git a/ingestion/internal/wiki/merge.go b/ingestion/internal/wiki/merge.go index dea8b45..89fe3c4 100644 --- a/ingestion/internal/wiki/merge.go +++ b/ingestion/internal/wiki/merge.go @@ -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) - } + 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) - } + 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" diff --git a/ingestion/internal/wiki/merge_test.go b/ingestion/internal/wiki/merge_test.go index 042d804..6985d0b 100644 --- a/ingestion/internal/wiki/merge_test.go +++ b/ingestion/internal/wiki/merge_test.go @@ -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) {