Files
hyperguild/ingestion/internal/wiki/merge.go
mathiasandClaude Sonnet 5 43f43e4621
CI / Lint / Test / Vet (pull_request) Successful in 40s
CI / Mirror to GitHub (pull_request) Skipped
fix(wiki): parseFrontmatterBlock must not treat comments or indented lines as keys
Third review round on PR #126. Two real bugs in the rewrite from the
previous commit:

1. The comment guard the sibling api package's frontmatter parser has
   (strings.HasPrefix(trimmed, "#")) was dropped in this reimplementation.
   A frontmatter comment containing a colon got cut at the colon and
   re-rendered with collapsed internal whitespace.
2. Worse: an indented YAML block-list item containing a colon (e.g. a
   title with a subtitle inside an aliases: list) was misparsed as a
   top-level key/value pair -- strings.Cut doesn't know a line is a
   continuation just because TrimSpace(key) happens to be non-empty.
   Re-rendering then stripped the line's indentation, de-indenting it
   out of the list it belonged to. refreshOwned re-renders this block on
   essentially every non-skipped ingest, so this was the normal path,
   not an edge case.

Checked whether the sibling api/frontmatter.go has the same list-item gap:
it does (same strings.Cut-then-TrimSpace shape, no indentation check) --
not a regression introduced here, a pre-existing latent defect shared
with the package this was modeled on. Filed separately rather than fixed
here (out of scope for this PR, needs its own review).

Fixed by checking indentation BEFORE calling Cut at all -- a line
starting with a space or tab is a continuation line, full stop,
regardless of what it contains -- and restoring the comment guard
verbatim from the sibling.

Flip-tested both guards independently: removing either one turns its
corresponding new test red in isolation; the other stays green,
confirming they're independent fixes, not one masking the other.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-23 07:07:21 +02:00

357 lines
12 KiB
Go

// ingestion/internal/wiki/merge.go
package wiki
import (
"fmt"
"sort"
"strings"
)
var bulletSections = map[string]bool{
"Related Concepts": true,
"Related Entities": true,
"Sources": true,
"Key Claims": true,
"Entities Mentioned": true,
"Concepts Introduced or Reinforced": true,
"Chapters": true,
}
var appendSections = map[string]bool{
"Evolving Notes": true,
"Updates": true,
"Open Questions Raised": true,
"Open Questions": true,
}
type section struct {
heading string
content string
}
// Merge combines two Page values with the same path.
// 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, preambleA, secsA := parseSections(a.Content)
fmB, _, secsB := parseSections(b.Content)
idx := make(map[string]int, len(secsA))
for i, s := range secsA {
idx[s.heading] = i
}
for _, sB := range secsB {
i, exists := idx[sB.heading]
if !exists {
idx[sB.heading] = len(secsA)
secsA = append(secsA, sB)
continue
}
sA := secsA[i]
switch {
case bulletSections[sB.heading]:
secsA[i].content = mergeBullets(sA.content, sB.content)
case appendSections[sB.heading]:
secsA[i].content = appendNew(sA.content, sB.content)
}
}
return Page{Path: a.Path, Content: rebuildContent(refreshOwned(fmA, fmB), preambleA, secsA)}
}
// generatorOwned names the frontmatter fields the ingest pipeline may refresh
// on a page it is revising. Everything else on disk wins.
//
// #103 made the on-disk frontmatter win wholesale, which is what stops a
// regenerating writer from overwriting human-set metadata — a title someone
// fixed, or the claim-property envelope (brain#21). The cost was that fields
// the generator legitimately owns froze at their first-ingest values: when it
// last ran, and what it last saw.
//
// An explicit allowlist rather than a carve-out: the ownership boundary is
// nameable, and a field nobody has thought about stays human-owned by default,
// which is the safe direction.
var generatorOwned = map[string]bool{
"last_updated": true,
"source_hash": true,
}
// fmLine is one line of a frontmatter block: a parsed key/value pair, or a
// raw line (blank, comment, or anything else that isn't a simple "key:
// value") kept verbatim. Same shape as api.frontmatter's fmLine — this
// package doesn't import that one (it's unexported there, and a's/b's
// frontmatter here always arrives as a string, not a parsed struct), so the
// small, already-proven set()-appends-if-absent behavior is reproduced
// locally rather than string-index surgery reinvented per call site.
type fmLine struct {
key, value, raw string
}
// parseFrontmatterBlock parses a "---\n...\n---\n" fenced block (the shape
// parseSections produces) into an ordered line list. A block with no
// opening/closing fence — including the empty string, a page with no
// frontmatter at all — returns nil, ok=false: there is nothing to update in
// place, only something to potentially create.
func parseFrontmatterBlock(block string) (lines []fmLine, ok bool) {
if !strings.HasPrefix(block, "---\n") {
return nil, false
}
rest := block[len("---\n"):]
end := strings.Index(rest, "\n---\n")
if end < 0 {
return nil, false
}
for _, line := range strings.Split(rest[:end], "\n") {
// An indented line is a continuation (a YAML block-list item, a
// wrapped value) never a top-level key, regardless of whether it
// happens to contain a colon — " - 'Chapter 3: The Reckoning'"
// is not a key named "- 'Chapter 3". Checked before Cut so a
// colon inside one never reaches the key/value path at all.
if line != "" && (line[0] == ' ' || line[0] == '\t') {
lines = append(lines, fmLine{raw: line})
continue
}
key, val, found := strings.Cut(line, ":")
key = strings.TrimSpace(key)
if !found || key == "" || strings.HasPrefix(strings.TrimSpace(line), "#") {
lines = append(lines, fmLine{raw: line})
continue
}
lines = append(lines, fmLine{key: key, value: strings.TrimSpace(val)})
}
return lines, true
}
// setLine updates key's value in place if lines already has it, or appends
// a new entry if it doesn't — the one behavior the original refreshOwned
// got wrong (PR #126 review, round 2: it only ever updated in place).
func setLine(lines []fmLine, key, value string) []fmLine {
for i := range lines {
if lines[i].key == key {
lines[i].value = value
return lines
}
}
return append(lines, fmLine{key: key, value: value})
}
func renderFrontmatterBlock(lines []fmLine) string {
if len(lines) == 0 {
return ""
}
var sb strings.Builder
sb.WriteString("---\n")
for _, l := range lines {
switch {
case l.key == "":
sb.WriteString(l.raw)
case l.value == "":
// A key with nothing after the colon (the YAML block-list shape:
// "aliases:" followed by indented "- item" lines on their own
// raw lines) must stay bare — "key: " with a trailing space
// is a different, if invisible, byte sequence, and this file's
// whole purpose is byte-stability across an unrelated merge.
sb.WriteString(l.key)
sb.WriteByte(':')
default:
sb.WriteString(l.key)
sb.WriteString(": ")
sb.WriteString(l.value)
}
sb.WriteByte('\n')
}
sb.WriteString("---\n")
return sb.String()
}
// refreshOwned returns a's frontmatter with the generator-owned fields taken
// from b where b supplies them. Fields absent from b keep a's value, so a
// generator that stops emitting a field cannot blank it.
//
// A field b supplies that a doesn't have yet — every page ingested before
// this mechanism existed has no source_hash line at all — is appended, not
// dropped (round 1 of review: dropping it silently meant source_hash never
// populated for the pre-existing corpus). If a has no frontmatter fence at
// all, one is created to hold the new fields rather than splicing raw
// "key: value" text into the body (round 2: that corrupted the page and the
// fields would duplicate on every subsequent merge, since unfenced text
// isn't recognized as frontmatter by parseSections on the next pass).
func refreshOwned(a, b string) string {
bLines, _ := parseFrontmatterBlock(b)
bVals := make(map[string]string)
for _, l := range bLines {
if l.key != "" && generatorOwned[l.key] {
bVals[l.key] = l.value
}
}
if len(bVals) == 0 {
return a
}
aLines, _ := parseFrontmatterBlock(a) // ok==false (no fence) -> aLines nil, same as "start fresh"
// Sorted for a deterministic, byte-stable result when more than one
// field is appended in the same call — map iteration order isn't.
keys := make([]string, 0, len(bVals))
for k := range bVals {
keys = append(keys, k)
}
sort.Strings(keys)
for _, k := range keys {
aLines = setLine(aLines, k, bVals[k])
}
return renderFrontmatterBlock(aLines)
}
// 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
if i < len(lines) && strings.TrimSpace(lines[i]) == "---" {
i++
var fmLines []string
for i < len(lines) {
if strings.TrimSpace(lines[i]) == "---" {
i++
break
}
fmLines = append(fmLines, lines[i])
i++
}
frontmatter = fmt.Sprintf("---\n%s\n---\n", strings.Join(fmLines, "\n"))
}
// 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 (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, "## ") {
if cur != nil {
sections = append(sections, *cur)
}
cur = &section{heading: strings.TrimPrefix(line, "## ")}
} else if cur != nil {
cur.content += line + "\n"
}
}
if cur != nil {
sections = append(sections, *cur)
}
return
}
// rebuildContent serialises sections back into a page.
//
// Each section body is normalised to exactly one trailing newline and no
// leading blank line. Without that, a section keeps whatever blank lines it was
// parsed with and this function adds its own, so every merge pass grows the page
// 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, 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")
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.
// Compile-on-ingest merges a freshly extracted page into the one already on
// disk, so an unchanged source re-ingested would grow "Open Questions" and
// "Updates" on every run until the page is unreadable — and the drift guard
// that would otherwise catch it (hashing the source body) does not exist yet,
// so idempotency has to hold here rather than upstream.
//
// Line-level rather than block-level: the common case is a re-ingest carrying
// one genuinely new line alongside several it already has. A block-level
// "skip if identical" would duplicate the old lines every time that happened.
//
// The cost is that a line repeated verbatim on purpose collapses to one. For
// 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.
func appendNew(a, b string) string {
seen := make(map[string]bool)
for _, line := range strings.Split(a, "\n") {
if c := canonicalLine(line); c != "" {
seen[c] = true
}
}
var add []string
for _, line := range strings.Split(b, "\n") {
c := canonicalLine(line)
if c == "" || seen[c] {
continue
}
seen[c] = true
add = append(add, line)
}
if len(add) == 0 {
return a // nothing new — leave the section byte-identical
}
return strings.TrimRight(a, "\n") + "\n\n" + strings.Join(add, "\n") + "\n"
}
func mergeBullets(a, b string) string {
seen := make(map[string]bool)
var lines []string
for _, line := range strings.Split(a+b, "\n") {
c := canonicalLine(line)
if c == "" || seen[c] {
continue
}
seen[c] = true
lines = append(lines, line)
}
return strings.Join(lines, "\n") + "\n"
}