diff --git a/README.md b/README.md index 609410d..1c1d545 100644 --- a/README.md +++ b/README.md @@ -100,8 +100,21 @@ review gate. Re-ingesting unchanged content is **byte-identical** — no diff, so brain-sync does not commit noise that hides real edits. -Known limitation: because the on-disk frontmatter wins, `last_updated` keeps its -first-ingest date on a revised page. +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. + +## Re-ingesting an unchanged source is skipped + +Ingest stores `source_hash` — sha256 of the ingested **body**, frontmatter +excluded — on the source page. A later ingest of the same source compares it +first and skips extraction entirely when it matches, reporting the skip rather +than doing it silently. + +Body-only is deliberate: a re-crawl that stamps a new fetch date has not changed +the source, and treating that as drift would re-extract the 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. ## Phase 1 tools (available now) diff --git a/ingestion/internal/pipeline/build.go b/ingestion/internal/pipeline/build.go index c59ba8e..4ff005a 100644 --- a/ingestion/internal/pipeline/build.go +++ b/ingestion/internal/pipeline/build.go @@ -76,6 +76,9 @@ func buildFrontmatter(rp RawPage, date string) string { } fmt.Fprintf(&sb, "date_ingested: %s\n", date) fmt.Fprintf(&sb, "last_updated: %s\n", date) + if rp.SourceHash != "" { + fmt.Fprintf(&sb, "source_hash: %s\n", rp.SourceHash) + } if rp.Source != "" { fmt.Fprintf(&sb, "source: %s\n", yamlScalar(rp.Source)) } diff --git a/ingestion/internal/pipeline/parse.go b/ingestion/internal/pipeline/parse.go index 6b2afc5..43615e1 100644 --- a/ingestion/internal/pipeline/parse.go +++ b/ingestion/internal/pipeline/parse.go @@ -20,9 +20,13 @@ type RawPage struct { // ingested content's own frontmatter (see parseContentFrontmatter) — never // set by the LLM. json:"-" keeps them immune to same-named keys the LLM // might emit. Only meaningful for Type == "source". - Source string `json:"-"` - Author string `json:"-"` - Published string `json:"-"` + // SourceHash is sha256 of the ingested BODY (frontmatter excluded), stamped + // so the next ingest of the same source can tell "unchanged" from "edited" + // without re-running extraction (#105). + SourceHash string `json:"-"` + Source string `json:"-"` + Author string `json:"-"` + Published string `json:"-"` } // ParseRawPages parses LLM output as a JSON array of RawPage objects. @@ -112,6 +116,11 @@ type sourceMeta struct { Source string Author string Published string + // BodyHash is sha256 of the content with any leading frontmatter removed. + // Body-only 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 (#105). + BodyHash string } // parseContentFrontmatter extracts source/author/published from a leading @@ -154,6 +163,7 @@ func applySourceMeta(pages []RawPage, meta sourceMeta) { if pages[i].Type != "source" { continue } + pages[i].SourceHash = meta.BodyHash pages[i].Source = meta.Source pages[i].Author = meta.Author pages[i].Published = meta.Published diff --git a/ingestion/internal/pipeline/pipeline.go b/ingestion/internal/pipeline/pipeline.go index 48b3aea..f7c3487 100644 --- a/ingestion/internal/pipeline/pipeline.go +++ b/ingestion/internal/pipeline/pipeline.go @@ -3,6 +3,8 @@ package pipeline import ( "context" + "crypto/sha256" + "encoding/hex" "fmt" "os" "path/filepath" @@ -26,6 +28,11 @@ type Config struct { type Result struct { Pages []string // relative paths written (or would-be written in dry-run) Warnings []string + // Skipped reports that the source was byte-identical to the last ingest, + // so no extraction ran and nothing was written. Reported rather than + // silent: a run that does nothing must say so, or "the pipeline is fine" + // and "the pipeline stopped working" look the same (#105, #107). + Skipped bool } // Run ingests content and writes structured wiki pages to brainDir/wiki/. @@ -43,6 +50,18 @@ func Run(ctx context.Context, cfg Config, brainDir, content, source string, dryR sourceSlug := wiki.Slug(source) date := time.Now().UTC().Format("2006-01-02") + + // Drift check BEFORE any extraction: an unchanged source costs a full LLM + // pass over every chunk to produce a result that merges to nothing (#105). + // The merge is already idempotent (#103), so this is cost and signal, not + // correctness — which is why it is safe for it to be an early return. + bodyHash := hashBody(content) + if !dryRun && bodyHash != "" && bodyHash == storedSourceHash(brainDir, sourceSlug) { + return Result{Skipped: true, Warnings: []string{ + fmt.Sprintf("source %q unchanged since last ingest (hash %s) — extraction skipped", source, bodyHash[:12]), + }}, nil + } + chunks := Chunk(content, cfg.ChunkSize) var allRaw []RawPage @@ -59,7 +78,9 @@ func Run(ctx context.Context, cfg Config, brainDir, content, source string, dryR allWarnings = append(allWarnings, warnings...) } - applySourceMeta(allRaw, parseContentFrontmatter(content)) + meta := parseContentFrontmatter(content) + meta.BodyHash = bodyHash + applySourceMeta(allRaw, meta) return buildAndWrite(allRaw, sourceSlug, date, brainDir, source, inventory, allWarnings, dryRun) } @@ -119,6 +140,45 @@ func buildAndWrite(rawPages []RawPage, sourceSlug, date, brainDir, source string return Result{Pages: written, Warnings: warnings}, nil } +// hashBody returns sha256 of content with any leading frontmatter block +// removed, or "" for empty content. +// +// Body-only 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 corpus every time +// a crawler ran (#105). +func hashBody(content string) string { + body := content + if strings.HasPrefix(body, "---\n") { + if _, rest, found := strings.Cut(body[4:], "\n---"); found { + body = strings.TrimPrefix(rest, "\n") + } + } + if strings.TrimSpace(body) == "" { + return "" + } + sum := sha256.Sum256([]byte(body)) + return hex.EncodeToString(sum[:]) +} + +// storedSourceHash reads source_hash from the source page a previous ingest +// wrote, or "" when there is no such page or no such field. +// +// Absence always means "extract". That is the fail-safe direction: a missed +// skip costs one extraction, while a wrong skip loses an update silently — and +// silence is the failure mode this repo keeps paying for. +func storedSourceHash(brainDir, sourceSlug string) string { + raw, err := os.ReadFile(filepath.Join(brainDir, "wiki", "sources", sourceSlug+".md")) + if err != nil { + return "" + } + for _, line := range strings.Split(string(raw), "\n") { + if key, value, found := strings.Cut(line, ":"); found && strings.TrimSpace(key) == "source_hash" { + return strings.TrimSpace(value) + } + } + return "" +} + // reviseOrCreate returns the bytes to write for one extracted page: merged into // whatever is already on disk, or the page as-is when the path is new. // diff --git a/ingestion/internal/pipeline/pipeline_test.go b/ingestion/internal/pipeline/pipeline_test.go index b62301c..d208fe3 100644 --- a/ingestion/internal/pipeline/pipeline_test.go +++ b/ingestion/internal/pipeline/pipeline_test.go @@ -233,3 +233,63 @@ func TestRunRaw_ReIngestingUnchangedContentIsByteIdentical(t *testing.T) { assert.Equal(t, string(first), string(second), "re-ingesting unchanged content must leave the page byte-identical") } + +// --- #105: source drift — an unchanged re-ingest must not re-extract -------- + +func TestRun_SkipsExtractionWhenTheSourceIsUnchanged(t *testing.T) { + // #103 made a re-ingest harmless (byte-identical merge). It is still not + // free: the full LLM extraction runs over every chunk to produce a result + // that merges to nothing. This is the cost half. + dir := t.TempDir() + var calls int + cfg := Config{Complete: func(_ context.Context, _, _ string) (string, error) { + calls++ + return `[{"title":"Shape Up","type":"source","subtype":"book","domain":"product","content":"## Summary\n\nA book.\n"}]`, nil + }} + + content := "---\nsource: https://example.com/shape-up\n---\n\nThe body of the article.\n" + + first, err := Run(context.Background(), cfg, dir, content, "shape-up", false) + require.NoError(t, err) + require.Equal(t, 1, calls, "first ingest must extract") + assert.False(t, first.Skipped) + + second, err := Run(context.Background(), cfg, dir, content, "shape-up", false) + require.NoError(t, err) + assert.Equal(t, 1, calls, "unchanged content must not be extracted again") + assert.True(t, second.Skipped, "the skip must be reported, not silent") +} + +func TestRun_ReExtractsWhenTheSourceBodyChanged(t *testing.T) { + dir := t.TempDir() + var calls int + cfg := Config{Complete: func(_ context.Context, _, _ string) (string, error) { + calls++ + return `[{"title":"Shape Up","type":"source","subtype":"book","domain":"product","content":"## Summary\n\nA book.\n"}]`, nil + }} + + _, err := Run(context.Background(), cfg, dir, "---\nsource: x\n---\n\nversion one\n", "shape-up", false) + require.NoError(t, err) + _, err = Run(context.Background(), cfg, dir, "---\nsource: x\n---\n\nversion two — the source was edited\n", "shape-up", false) + require.NoError(t, err) + + assert.Equal(t, 2, calls, "changed content must be re-extracted") +} + +func TestRun_FrontmatterOnlyChangeIsNotDrift(t *testing.T) { + // The hash covers the body, so re-fetching a page whose metadata moved + // (a re-crawl stamping a new date) is not treated as the source changing. + dir := t.TempDir() + var calls int + cfg := Config{Complete: func(_ context.Context, _, _ string) (string, error) { + calls++ + return `[{"title":"Shape Up","type":"source","subtype":"book","domain":"product","content":"## Summary\n\nA book.\n"}]`, nil + }} + + _, err := Run(context.Background(), cfg, dir, "---\nsource: x\nfetched: monday\n---\n\nsame body\n", "shape-up", false) + require.NoError(t, err) + _, err = Run(context.Background(), cfg, dir, "---\nsource: x\nfetched: tuesday\n---\n\nsame body\n", "shape-up", false) + require.NoError(t, err) + + assert.Equal(t, 1, calls, "only the body counts as the source") +} diff --git a/ingestion/internal/wiki/merge.go b/ingestion/internal/wiki/merge.go index 8c161ea..20ca53f 100644 --- a/ingestion/internal/wiki/merge.go +++ b/ingestion/internal/wiki/merge.go @@ -3,6 +3,7 @@ package wiki import ( "fmt" + "sort" "strings" ) @@ -38,7 +39,7 @@ type section struct { // not present in a are appended. func Merge(a, b Page) Page { fmA, preambleA, secsA := parseSections(a.Content) - _, _, secsB := parseSections(b.Content) + fmB, _, secsB := parseSections(b.Content) idx := make(map[string]int, len(secsA)) for i, s := range secsA { @@ -61,7 +62,151 @@ func Merge(a, b Page) Page { } } - return Page{Path: a.Path, Content: rebuildContent(fmA, preambleA, secsA)} + 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 diff --git a/ingestion/internal/wiki/merge_test.go b/ingestion/internal/wiki/merge_test.go index e1bdcde..5bd962d 100644 --- a/ingestion/internal/wiki/merge_test.go +++ b/ingestion/internal/wiki/merge_test.go @@ -174,3 +174,161 @@ title: X assert.Contains(t, got, "Does this converge?", "existing question must survive") assert.Contains(t, got, "What happens under load?", "new question must be added") } + +func TestMerge_GeneratorOwnedFrontmatterFieldsAreRefreshed(t *testing.T) { + // #103 made on-disk frontmatter win wholesale, which is what protects + // human-set fields from a regenerating writer. The cost was that fields the + // generator legitimately owns — when it last ran, what it last saw — froze + // at their first-ingest values. An explicit owned-set is the boundary: + // named fields the generator may refresh, everything else human-owned. + existing := Page{Path: "wiki/sources/x.md", Content: `--- +title: 'X' +last_updated: 2026-01-01 +source_hash: aaaa +half_life: durable +claim_provenance: asserted +--- + +## 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 + + assert.Contains(t, got, "last_updated: 2026-08-22", "generator owns last_updated") + assert.Contains(t, got, "source_hash: bbbb", "generator owns source_hash") + assert.Contains(t, got, "title: 'X'", "a human-set title must survive regeneration") + assert.Contains(t, got, "half_life: durable", "the claim envelope must survive regeneration") + assert.Contains(t, got, "claim_provenance: asserted") + assert.Contains(t, got, "Original.", "an existing section still wins over the model's rewrite") + 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) +}