fix(wiki): surface merge conflicts as textual-diff warnings (closes #104)

Textual-diff conflict detection only (semantic contradiction detection via brain-gardener Judge deferred, confirmed by Mathias, comment 12594). Includes canonicalBlock whitespace-run normalization fix for the disagreement comparison.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Mathias <mthbqv@gmail.com>

ingestion/internal/wiki/merge.go

@@ -3,6 +3,7 @@ package wiki
 
 import (
 	"fmt"
+	"sort"
 	"strings"
 )
 
@@ -83,7 +84,7 @@ func MergeReport(a, b Page) (Page, []string) {
 			secsA[i].content = appendNew(sA.content, sB.content)
 		default:
 			// a wins — but say that b disagreed, rather than dropping it mute.
-			if strings.TrimSpace(sA.content) != strings.TrimSpace(sB.content) {
+			if canonicalBlock(sA.content) != canonicalBlock(sB.content) {
 				conflicts = append(conflicts, sB.heading)
 			}
 		}
@@ -109,36 +110,131 @@ var generatorOwned = map[string]bool{
 	"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 _, line := range strings.Split(b, "\n") {
-		key, value, found := strings.Cut(line, ":")
-		if !found || !generatorOwned[strings.TrimSpace(key)] {
-			continue
+	for _, l := range bLines {
+		if l.key != "" && generatorOwned[l.key] {
+			bVals[l.key] = l.value
 		}
-		bVals[strings.TrimSpace(key)] = strings.TrimSpace(value)
 	}
 	if len(bVals) == 0 {
 		return a
 	}
 
-	out := make([]string, 0, len(strings.Split(a, "\n")))
-	for _, line := range strings.Split(a, "\n") {
-		key, _, found := strings.Cut(line, ":")
-		k := strings.TrimSpace(key)
-		if found && generatorOwned[k] {
-			if v, ok := bVals[k]; ok {
-				out = append(out, k+": "+v)
-				delete(bVals, k)
-				continue
+	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])
 	}
-		out = append(out, line)
-	}
-	return strings.Join(out, "\n")
+	return renderFrontmatterBlock(aLines)
 }
 
 // parseSections splits markdown into its frontmatter, its preamble (content
@@ -233,6 +329,21 @@ func canonicalLine(line string) string {
 	return strings.Join(strings.Fields(line), " ")
 }
 
+// canonicalBlock applies canonicalLine per line of a multi-line section body
+// — the same whitespace-insensitivity appendNew/mergeBullets already give a
+// single line, extended to a whole section for MergeReport's disagreement
+// check (found in review, PR #129: comparing raw strings.TrimSpace only
+// tolerated leading/trailing whitespace, not a re-extraction's mid-line
+// spacing, which every other comparison in this file already tolerates).
+func canonicalBlock(block string) string {
+	lines := strings.Split(block, "\n")
+	out := make([]string, len(lines))
+	for i, l := range lines {
+		out[i] = canonicalLine(l)
+	}
+	return strings.Join(out, "\n")
+}
+
 // appendNew concatenates the lines of b that a does not already carry.
 //
 // Append sections used to concatenate wholesale, which is correct exactly once.

ingestion/internal/wiki/merge_test.go

@@ -215,6 +215,124 @@ Rewritten by the model.
 	assert.NotContains(t, got, "Renamed by the model")
 }
 
+func TestMerge_GeneratorOwnedFieldsAreAppendedWhenAPageNeverHadThemBefore(t *testing.T) {
+	// Found in review of PR #126: refreshOwned only replaced a generator-owned
+	// line that already existed in a's frontmatter — it never appended one a
+	// didn't have. Every page ingested before this feature shipped has no
+	// source_hash line at all, so on its first re-ingest after deploy, the
+	// fresh hash was silently dropped and the skip-on-unchanged feature this
+	// PR exists to build would never engage for the entire pre-existing
+	// corpus, permanently, with nothing telling anyone.
+	existing := Page{Path: "wiki/sources/x.md", Content: `---
+title: 'X'
+---
+
+## Summary
+
+Original.
+`}
+	fresh := Page{Path: "wiki/sources/x.md", Content: `---
+title: 'Renamed by the model'
+last_updated: 2026-08-22
+source_hash: bbbb
+---
+
+## Summary
+
+Rewritten by the model.
+`}
+
+	got := Merge(existing, fresh).Content
+
+	// Exact frontmatter block, not Contains: the second review round found
+	// that a substring check doesn't prove the fields landed inside a valid
+	// fence, in the right order, or that the fence itself wasn't corrupted —
+	// two independent mutations (closeIdx targeting the wrong fence, reversed
+	// sort) both survived a Contains-only version of this test.
+	want := "---\ntitle: 'X'\nlast_updated: 2026-08-22\nsource_hash: bbbb\n---\n\n## Summary\n\nOriginal.\n"
+	assert.Equal(t, want, got)
+}
+
+func TestMerge_GeneratorOwnedFieldsCreateAFrontmatterBlockWhenThePageHadNoneAtAll(t *testing.T) {
+	// Second review round on PR #126: when a has no frontmatter fence at all,
+	// the append fallback spliced raw, unfenced "key: value" lines straight
+	// into the body -- worse than the original bug, because the next
+	// parseSections call wouldn't recognize them as frontmatter (they don't
+	// start with `---`), so they'd be absorbed as ordinary preamble text and
+	// re-duplicate on every subsequent merge instead of updating in place.
+	existing := Page{Path: "wiki/sources/x.md", Content: "## Summary\n\nOriginal.\n"}
+	fresh := Page{Path: "wiki/sources/x.md", Content: `---
+last_updated: 2026-08-22
+source_hash: bbbb
+---
+
+## Summary
+
+Rewritten by the model.
+`}
+
+	got := Merge(existing, fresh).Content
+
+	want := "---\nlast_updated: 2026-08-22\nsource_hash: bbbb\n---\n\n## Summary\n\nOriginal.\n"
+	assert.Equal(t, want, got)
+}
+
+func TestMerge_GeneratorOwnedFieldsMixedReplaceAndAppend(t *testing.T) {
+	// a already has last_updated (replace in place) but not source_hash
+	// (append) -- the two code paths in the same call, together.
+	existing := Page{Path: "wiki/sources/x.md", Content: `---
+title: 'X'
+last_updated: 2026-01-01
+---
+
+## Summary
+
+Original.
+`}
+	fresh := Page{Path: "wiki/sources/x.md", Content: `---
+last_updated: 2026-08-22
+source_hash: bbbb
+---
+
+## Summary
+
+Rewritten by the model.
+`}
+
+	got := Merge(existing, fresh).Content
+
+	want := "---\ntitle: 'X'\nlast_updated: 2026-08-22\nsource_hash: bbbb\n---\n\n## Summary\n\nOriginal.\n"
+	assert.Equal(t, want, got)
+}
+
+func TestRefreshOwned_CommentLinesSurviveUnmangled(t *testing.T) {
+	// Third review round on PR #126: parseFrontmatterBlock dropped the
+	// sibling api package's comment guard (strings.HasPrefix(trimmed, "#")).
+	// Without it, a comment line containing a colon gets cut at the colon
+	// and re-rendered with collapsed whitespace instead of surviving
+	// verbatim.
+	a := "---\n#see also:   https://x/y\ntitle: 'X'\n---\n"
+	b := "---\nsource_hash: bbbb\n---\n"
+
+	got := refreshOwned(a, b)
+	assert.Equal(t, "---\n#see also:   https://x/y\ntitle: 'X'\nsource_hash: bbbb\n---\n", got)
+}
+
+func TestRefreshOwned_IndentedListItemsWithColonsAreNotMisparsedAsKeys(t *testing.T) {
+	// Same round: an indented YAML block-list item that happens to contain a
+	// colon (e.g. a title with a subtitle) is a continuation line, never a
+	// top-level key, regardless of what strings.Cut finds in it. Misparsing
+	// it as a key/value pair and re-rendering strips its indentation,
+	// de-indenting it out of the list it belongs to -- and refreshOwned
+	// re-renders this block on essentially every non-skipped ingest, so this
+	// is the normal path, not an edge case.
+	a := "---\naliases:\n  - 'Chapter 3: The Reckoning'\ntitle: 'X'\n---\n"
+	b := "---\nsource_hash: bbbb\n---\n"
+
+	got := refreshOwned(a, b)
+	assert.Equal(t, "---\naliases:\n  - 'Chapter 3: The Reckoning'\ntitle: 'X'\nsource_hash: bbbb\n---\n", got)
+}
+
 func TestMerge_ReportsSectionsWhereTheFreshVersionWasDiscarded(t *testing.T) {
 	// The asymmetry that makes ingest safe (on-disk wins) also throws away a
 	// real signal: the source now says something different about a section we
@@ -267,3 +385,32 @@ Appetite is fixed, scope is variable.
 	_, conflicts := MergeReport(page, page)
 	assert.Empty(t, conflicts, "identical content is not a disagreement")
 }
+
+func TestMerge_ReportsNothingForInternalWhitespaceVariantsOnly(t *testing.T) {
+	// Found in review of PR #129: the conflict comparison used raw
+	// strings.TrimSpace, which only tolerates LEADING/TRAILING whitespace —
+	// unlike appendNew/mergeBullets, which already canonicalize internal
+	// whitespace runs too (#103: a re-extraction's spacing isn't guaranteed
+	// byte-stable). A mid-line double-space from re-extraction would
+	// false-positive as a "disagreement" here even though every other
+	// whitespace-sensitive comparison in this file already tolerates it.
+	existing := Page{Path: "wiki/sources/x.md", Content: `---
+title: 'X'
+---
+
+## Summary
+
+Appetite  is fixed,   scope is variable.
+`}
+	fresh := Page{Path: "wiki/sources/x.md", Content: `---
+title: 'X'
+---
+
+## Summary
+
+Appetite is fixed, scope is variable.
+`}
+
+	_, conflicts := MergeReport(existing, fresh)
+	assert.Empty(t, conflicts, "internal whitespace differences alone are not a disagreement")
+}