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>

README.md

@@ -104,24 +104,6 @@ Two fields are **generator-owned** and do get refreshed on a revise:
 `last_updated` and `source_hash`. Everything else on disk wins. The boundary is
 an explicit allowlist, so a field nobody has thought about stays human-owned.
 
-### Disagreement is reported, not resolved
-
-When the fresh extraction produces a *different version* of a section that
-already exists, the on-disk version still wins — but the run now says so:
-
-```
-wiki/sources/shape-up.md: source now disagrees on section "Summary" — on-disk version kept
-```
-
-That line lands in `log.md` with the rest of the run's warnings. Without it, "the
-source is unchanged" and "the source changed its mind and we ignored it" look
-identical from the outside.
-
-This is deliberately **textual** disagreement, not semantic contradiction: cheap,
-deterministic, no model on the write path. Deciding whether a disagreement is a
-genuine contradiction stays with the Judge-gated `brain-enrich` pass, which is
-approval-gated by design. Detection here, adjudication there.
-
 ## Re-ingesting an unchanged source is skipped
 
 Ingest stores `source_hash` — sha256 of the ingested **body**, frontmatter

ingestion/internal/pipeline/pipeline.go

@@ -117,18 +117,10 @@ func buildAndWrite(rawPages []RawPage, sourceSlug, date, brainDir, source string
 			if err := os.MkdirAll(filepath.Dir(dest), 0o755); err != nil {
 				return Result{}, fmt.Errorf("mkdir for %s: %w", page.Path, err)
 			}
-			content, conflicts, err := reviseOrCreate(dest, page)
+			content, err := reviseOrCreate(dest, page)
 			if err != nil {
 				return Result{}, err
 			}
-			for _, heading := range conflicts {
-				// Reported, not resolved. The on-disk version stands; whether
-				// the disagreement is a genuine contradiction is the
-				// Judge-gated enrich pass's call, not the write path's.
-				warnings = append(warnings, fmt.Sprintf(
-					"%s: source now disagrees on section %q — on-disk version kept",
-					page.Path, heading))
-			}
 			if err := os.WriteFile(dest, []byte(content), 0o644); err != nil {
 				return Result{}, fmt.Errorf("write %s: %w", page.Path, err)
 			}
@@ -206,16 +198,15 @@ func storedSourceHash(brainDir, sourceSlug string) string {
 // its first-ingest date on a revised page. Left deliberately — the alternative
 // is this function editing frontmatter it does not own, and no lint reads that
 // field today.
-func reviseOrCreate(dest string, page wiki.Page) (string, []string, error) {
+func reviseOrCreate(dest string, page wiki.Page) (string, error) {
 	existing, err := os.ReadFile(dest)
 	switch {
 	case os.IsNotExist(err):
-		return page.Content, nil, nil
+		return page.Content, nil
 	case err != nil:
-		return "", nil, fmt.Errorf("read existing %s: %w", page.Path, err)
+		return "", fmt.Errorf("read existing %s: %w", page.Path, err)
 	}
-	merged, conflicts := wiki.MergeReport(wiki.Page{Path: page.Path, Content: string(existing)}, page)
-	return merged.Content, conflicts, nil
+	return wiki.Merge(wiki.Page{Path: page.Path, Content: string(existing)}, page).Content, nil
 }
 
 // mergeAll deduplicates pages by path, merging content from later occurrences.

ingestion/internal/pipeline/pipeline_test.go

@@ -8,7 +8,6 @@ import (
 	"net/http/httptest"
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 	"time"
 
@@ -294,34 +293,3 @@ func TestRun_FrontmatterOnlyChangeIsNotDrift(t *testing.T) {
 
 	assert.Equal(t, 1, calls, "only the body counts as the source")
 }
-
-func TestRunRaw_ReportsWhenTheSourceDisagreesWithWhatIsOnDisk(t *testing.T) {
-	// The on-disk version still wins — that is what makes unattended ingest
-	// safe. But the run must say that the source now claims something else,
-	// or a changed source is indistinguishable from an unchanged one.
-	dir := t.TempDir()
-	page := filepath.Join(dir, "wiki", "sources", "shape-up.md")
-	require.NoError(t, os.MkdirAll(filepath.Dir(page), 0o755))
-	require.NoError(t, os.WriteFile(page, []byte(`---
-title: 'Shape Up'
----
-
-## Summary
-
-Appetite is fixed, scope is variable.
-`), 0o644))
-
-	res, err := RunRaw(dir, "shape-up", []RawPage{{
-		Title: "Shape Up", Type: "source", Subtype: "book", Domain: "product",
-		Content: "## Summary\n\nAppetite is negotiable and scope is fixed.\n",
-	}}, false)
-	require.NoError(t, err)
-
-	got, err := os.ReadFile(page)
-	require.NoError(t, err)
-	assert.Contains(t, string(got), "Appetite is fixed", "on-disk version wins")
-
-	joined := strings.Join(res.Warnings, " ")
-	assert.Contains(t, joined, "Summary", "the disagreeing section must be named")
-	assert.Contains(t, joined, "shape-up", "and the page it happened on")
-}

ingestion/internal/wiki/merge.go

@@ -3,6 +3,7 @@ package wiki
 
 import (
 	"fmt"
+	"sort"
 	"strings"
 )
 
@@ -36,33 +37,10 @@ type section struct {
 // 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.
-// Merge discards any disagreement silently; prefer MergeReport where the
-// caller can act on it.
 func Merge(a, b Page) Page {
-	page, _ := MergeReport(a, b)
-	return page
-}
-
-// MergeReport is Merge, plus the headings of sections where b carried a
-// different version of a section a already had — and lost.
-//
-// That loss is the property which makes ingest safe to run unattended (#103):
-// a regenerating writer can add to a note but never rewrite it. But the
-// discarded text is also a signal, and until now it went in the bin: the source
-// now says something different about a section we already hold. Nobody could
-// tell "the source is unchanged" from "the source changed its mind and we
-// ignored it".
-//
-// Deliberately NOT a semantic contradiction check. This reports textual
-// disagreement, which is cheap, deterministic, and needs no model on the write
-// path. Deciding whether a disagreement is a genuine contradiction stays with
-// the Judge-gated enrich pass that already exists (brain-gardener#175), which
-// is approval-gated by design — detection here, adjudication there.
-func MergeReport(a, b Page) (Page, []string) {
 	fmA, preambleA, secsA := parseSections(a.Content)
 	fmB, _, secsB := parseSections(b.Content)
 
-	var conflicts []string
 	idx := make(map[string]int, len(secsA))
 	for i, s := range secsA {
 		idx[s.heading] = i
@@ -81,15 +59,10 @@ func MergeReport(a, b Page) (Page, []string) {
 			secsA[i].content = mergeBullets(sA.content, sB.content)
 		case appendSections[sB.heading]:
 			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) {
-				conflicts = append(conflicts, sB.heading)
-			}
 		}
 	}
 
-	return Page{Path: a.Path, Content: rebuildContent(refreshOwned(fmA, fmB), preambleA, secsA)}, conflicts
+	return Page{Path: a.Path, Content: rebuildContent(refreshOwned(fmA, fmB), preambleA, secsA)}
 }
 
 // generatorOwned names the frontmatter fields the ingest pipeline may refresh
@@ -109,36 +82,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
-			}
-		}
-		out = append(out, line)
+	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)
 	}
-	return strings.Join(out, "\n")
+	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

ingestion/internal/wiki/merge_test.go

@@ -215,55 +215,120 @@ Rewritten by the model.
 	assert.NotContains(t, got, "Renamed by the model")
 }
 
-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
-	// already have. Today that disagreement is silently discarded, so nobody
-	// can tell "the source is unchanged" from "the source changed its mind and
-	// we ignored it".
+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
 
-Appetite is fixed, scope is variable.
-
-## Method
-
-Six-week cycles.
+Original.
 `}
 	fresh := Page{Path: "wiki/sources/x.md", Content: `---
-title: 'X'
+title: 'Renamed by the model'
+last_updated: 2026-08-22
+source_hash: bbbb
 ---
 
 ## Summary
 
-Appetite is negotiable and scope is fixed.
-
-## Method
-
-Six-week cycles.
+Rewritten by the model.
 `}
 
-	got, conflicts := MergeReport(existing, fresh)
+	got := Merge(existing, fresh).Content
 
-	assert.Contains(t, got.Content, "Appetite is fixed", "on-disk version still wins")
-	assert.NotContains(t, got.Content, "negotiable")
-	assert.Equal(t, []string{"Summary"}, conflicts,
-		"only the section that actually disagreed is reported")
+	// 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_ReportsNothingWhenTheFreshVersionAgrees(t *testing.T) {
-	page := Page{Path: "wiki/sources/x.md", Content: `---
-title: 'X'
+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
 
-Appetite is fixed, scope is variable.
+Rewritten by the model.
 `}
 
-	_, conflicts := MergeReport(page, page)
-	assert.Empty(t, conflicts, "identical content is not a disagreement")
+	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)
 }