Compare commits

..
Author SHA1 Message Date
mathias 9345a0a867 feat(pipeline): report when a re-ingest disagrees with the page on disk (#104)
CI / Lint / Test / Vet (pull_request) Successful in 34s
CI / Mirror to GitHub (pull_request) Skipped
The asymmetry that makes unattended ingest safe (#103) also discarded a
real signal. When a fresh extraction produced a different version of a
section already on disk, the on-disk version won and the model's version
went in the bin without a word — so "the source is unchanged" and "the
source changed its mind and we ignored it" were indistinguishable.

wiki.MergeReport now returns the headings where the incoming version
disagreed and lost, and the ingest run reports them as warnings, which
AppendLog already writes into log.md.

This is the simplified answer to contradiction flagging at ingest. The
issue proposed calling brain-gardener's Judge from the ingest path, which
needs either a shared Go module across two repos or an HTTP service that
does not exist — and puts an LLM call on the write path, where a timeout
must neither fail the ingest nor silently mark nothing as contested.

None of that is necessary to get the signal. The merge already computes
the disagreement; it was throwing it away. Surfacing it costs nothing,
needs no model, no cue-word matcher, no cross-repo dependency, and is
deterministic.

Deliberately textual disagreement, not semantic contradiction. Deciding
whether a disagreement is a genuine contradiction stays with the
Judge-gated enrich pass that already runs hourly and is approval-gated by
design (brain-gardener#175). Detection here, adjudication there.

Merge is kept as a thin wrapper over MergeReport so existing callers are
untouched.

Refs #104, #103, brain-gardener#175, brain#21.
2026-09-23 06:51:49 +02:00
mathias a5a328fea5 feat(pipeline): skip extraction when the source has not changed (#105)
CI / Lint / Test / Vet (pull_request) Successful in 34s
CI / Mirror to GitHub (pull_request) Skipped
Ingest now stamps source_hash — sha256 of the ingested body, frontmatter
excluded — onto the source page, and compares it before doing any work. An
unchanged source returns Skipped with no LLM call at all.

This is cost and signal, not correctness: #103 already made a re-ingest
byte-identical, so a repeated run was harmless but not free — a full
extraction over every chunk to produce a result that merges to nothing.
Building this first would have put idempotency in the guard rather than in
the write, which is the wrong dependency direction.

Body-only hashing on purpose. A re-crawl that stamps a new fetch date has
not changed the source, and treating that as drift would re-extract the
whole corpus every time a crawler ran. A missing or unreadable hash always
means "extract": a missed skip costs one extraction, a wrong skip loses an
update silently, and silence is the failure mode this repo keeps paying
for.

The skip is reported (Result.Skipped plus a warning naming the hash), not
silent — a run that does nothing must say so, or "the pipeline is fine" and
"the pipeline stopped working" look identical (#107).

Storing the hash required settling who owns which frontmatter field. #103
made the on-disk block win wholesale, which protects human-set metadata
from a regenerating writer but froze fields the generator legitimately
owns. wiki.Merge now refreshes an explicit generatorOwned allowlist —
last_updated and source_hash — and keeps everything else on disk. An
allowlist rather than a carve-out, so a field nobody has thought about
stays human-owned by default. That also retires the last_updated
limitation #103 shipped with.

Refs #105, #103, #107, brain#21.
2026-09-22 23:45:25 +02:00
2 changed files with 19 additions and 277 deletions
+19 -130
View File
@@ -3,7 +3,6 @@ package wiki
import (
"fmt"
"sort"
"strings"
)
@@ -84,7 +83,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 canonicalBlock(sA.content) != canonicalBlock(sB.content) {
if strings.TrimSpace(sA.content) != strings.TrimSpace(sB.content) {
conflicts = append(conflicts, sB.heading)
}
}
@@ -110,131 +109,36 @@ 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 _, l := range bLines {
if l.key != "" && generatorOwned[l.key] {
bVals[l.key] = l.value
for _, line := range strings.Split(b, "\n") {
key, value, found := strings.Cut(line, ":")
if !found || !generatorOwned[strings.TrimSpace(key)] {
continue
}
bVals[strings.TrimSpace(key)] = strings.TrimSpace(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)
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)
}
sort.Strings(keys)
for _, k := range keys {
aLines = setLine(aLines, k, bVals[k])
}
return renderFrontmatterBlock(aLines)
return strings.Join(out, "\n")
}
// parseSections splits markdown into its frontmatter, its preamble (content
@@ -329,21 +233,6 @@ 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.
-147
View File
@@ -215,124 +215,6 @@ 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
@@ -385,32 +267,3 @@ 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")
}