diff --git a/ingestion/internal/wiki/merge.go b/ingestion/internal/wiki/merge.go index 89fe3c4..8c161ea 100644 --- a/ingestion/internal/wiki/merge.go +++ b/ingestion/internal/wiki/merge.go @@ -29,12 +29,16 @@ type section struct { } // Merge combines two Page values with the same path. -// Frontmatter is taken from a. Sections are merged by strategy: -// bullet sections union unique lines, append sections concatenate, -// all others keep a's version. Sections in b not present in a are appended. +// Frontmatter is taken from a. The preamble (content before the first ## +// heading) is taken from a — same "on-disk wins" default every unlisted +// named section gets, kept as a separate field rather than a section with +// an empty heading so it can never collide with one (see parseSections). +// Sections are merged by strategy: bullet sections union unique lines, +// append sections concatenate, all others keep a's version. Sections in b +// not present in a are appended. func Merge(a, b Page) Page { - fmA, secsA := parseSections(a.Content) - _, secsB := parseSections(b.Content) + fmA, preambleA, secsA := parseSections(a.Content) + _, _, secsB := parseSections(b.Content) idx := make(map[string]int, len(secsA)) for i, s := range secsA { @@ -57,10 +61,20 @@ func Merge(a, b Page) Page { } } - return Page{Path: a.Path, Content: rebuildContent(fmA, secsA)} + return Page{Path: a.Path, Content: rebuildContent(fmA, preambleA, secsA)} } -func parseSections(markdown string) (frontmatter string, sections []section) { +// parseSections splits markdown into its frontmatter, its preamble (content +// before the first `## ` heading), and its `## `-headed sections. +// +// The preamble is returned separately rather than as a section with an +// empty heading — a section keyed by heading=="" would collide with a +// literal degenerate `## ` (marker, no title text; strings.TrimPrefix also +// yields ""), which is a distinct, addressable section a page can genuinely +// have, not the same thing as "no heading yet." Two entries sharing one key +// in Merge's idx map means one of them silently loses lookup visibility — +// found in review as real, reproducible data loss, not a hypothetical. +func parseSections(markdown string) (frontmatter, preamble string, sections []section) { lines := strings.Split(markdown, "\n") i := 0 @@ -80,24 +94,35 @@ func parseSections(markdown string) (frontmatter string, sections []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{} + // not something this parser is entitled to drop (hyperguild#103 review: + // dropping it 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 file exists + // to prevent). + var preambleLines []string + for ; i < len(lines); i++ { + if strings.HasPrefix(lines[i], "## ") { + break + } + preambleLines = append(preambleLines, lines[i]) + } + preamble = strings.Join(preambleLines, "\n") + + var cur *section for ; i < len(lines); i++ { line := lines[i] if strings.HasPrefix(line, "## ") { - sections = append(sections, *cur) + if cur != nil { + sections = append(sections, *cur) + } cur = §ion{heading: strings.TrimPrefix(line, "## ")} - } else { + } else if cur != nil { cur.content += line + "\n" } } - sections = append(sections, *cur) + if cur != nil { + sections = append(sections, *cur) + } return } @@ -109,28 +134,28 @@ func parseSections(markdown string) (frontmatter string, sections []section) { // by a blank line — invisible in a diff viewer, fatal to byte-stability. Compile-on-ingest // merges on every run, so "merging changes nothing when nothing changed" has to // hold at the byte level, not just the content level. -func rebuildContent(frontmatter string, sections []section) string { +func rebuildContent(frontmatter, preamble string, sections []section) string { var sb strings.Builder sb.WriteString(frontmatter) + if body := strings.Trim(preamble, "\n"); body != "" { + fmt.Fprintf(&sb, "\n%s\n", body) + } 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() } +// 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), " ") +} + // appendNew concatenates the lines of b that a does not already carry. // // Append sections used to concatenate wholesale, which is correct exactly once. @@ -148,15 +173,6 @@ 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") { diff --git a/ingestion/internal/wiki/merge_test.go b/ingestion/internal/wiki/merge_test.go index 6985d0b..e1bdcde 100644 --- a/ingestion/internal/wiki/merge_test.go +++ b/ingestion/internal/wiki/merge_test.go @@ -73,6 +73,24 @@ func TestMerge_PreservesOnDiskContentWithNoSectionHeadings(t *testing.T) { "on-disk content with no ## headings must survive the merge, not be replaced wholesale by the fresh extraction") } +func TestMerge_DegenerateEmptyHeadingDoesNotCollideWithPreamble(t *testing.T) { + // Second review pass on #103: representing the preamble as a section with + // heading=="" collides with a literal `## ` (marker, no title text) — + // strings.TrimPrefix("## ", "## ") is also "". `a` has only a preamble + // (no degenerate heading of its own); `b` introduces a genuinely new + // degenerate `## ` section. Because both key to "", Merge's idx thinks + // b's section "already exists" in a (it's really just a's unrelated + // preamble) and never adds it — b's section vanishes from the output + // entirely, not merged, not kept, not appended. + a := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\nAn intro paragraph.\n\n## Definition\n\nA thing.\n"} + b := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\n## \n\nA genuinely new degenerate section's body.\n\n## Definition\n\nA thing.\n"} + + got := Merge(a, b) + assert.Contains(t, got.Content, "An intro paragraph.", "a's own preamble must survive") + assert.Contains(t, got.Content, "A genuinely new degenerate section's body.", + "b's degenerate section is new content, not a collision with a's unrelated preamble — it must not be silently dropped") +} + 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"}