feat(pipeline): ingest revises the existing page instead of clobbering it #103

Merged
mathias merged 3 commits from agent/b1-compile-on-ingest into main 2026-09-22 21:43:04 +00:00
5 changed files with 349 additions and 14 deletions
+27
View File
@@ -76,6 +76,33 @@ that prompted the question is answered by naming it here instead.
Full reasoning, including the reversal trigger: brain
`wiki/hyperguild/decisions/brain-mcp-stays-in-ingestion-fix-discoverability-by-name`.
## Ingest revises, it does not overwrite
Re-ingesting a source **merges into the page already on disk** rather than
replacing it. Previously the write was unconditional, so a second ingest of the
same source discarded everything accumulated since — including hand-written
sections.
The merge is deliberately asymmetric in the existing page's favour:
| On the page | In the fresh extraction | Result |
|---|---|---|
| a section | the same section | **the on-disk version wins** |
| — | a new section | added |
| a bullet list (`Sources`, `Key Claims`, …) | more bullets | unioned, no duplicates |
| an append section (`Open Questions`, `Updates`, …) | more lines | only genuinely new lines appended |
| frontmatter | frontmatter | **the on-disk frontmatter wins** |
So an extraction can add to a note but never silently rewrite what a human put
there, which is what makes it safe to run on every ingest rather than behind a
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.
## Phase 1 tools (available now)
| Tool | What it does |
+35 -1
View File
@@ -96,7 +96,11 @@ 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)
}
if err := os.WriteFile(dest, []byte(page.Content), 0o644); err != nil {
content, err := reviseOrCreate(dest, page)
if err != nil {
return Result{}, err
}
if err := os.WriteFile(dest, []byte(content), 0o644); err != nil {
return Result{}, fmt.Errorf("write %s: %w", page.Path, err)
}
}
@@ -115,6 +119,36 @@ func buildAndWrite(rawPages []RawPage, sourceSlug, date, brainDir, source string
return Result{Pages: written, Warnings: warnings}, nil
}
// 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.
//
// This is the compile half of compile-on-ingest. Before it, the write was an
// unconditional os.WriteFile, so re-ingesting a source replaced its page
// wholesale and discarded everything accumulated since — including human edits
// (hyperguild#43: "re-ingest overwrites wholesale").
//
// The merge is deliberately asymmetric in the existing page's favour:
// wiki.Merge keeps the on-disk frontmatter and the on-disk version of any
// section both sides carry, adds sections only the extraction produced, and
// unions bullet lists. So a freshly extracted page can add to a note but never
// silently rewrite what a human put there — which is what makes it safe to run
// this on every ingest rather than behind a review gate.
//
// Known limitation: because the on-disk frontmatter wins, `last_updated` keeps
// 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, error) {
existing, err := os.ReadFile(dest)
switch {
case os.IsNotExist(err):
return page.Content, nil
case err != nil:
return "", fmt.Errorf("read existing %s: %w", page.Path, err)
}
return wiki.Merge(wiki.Page{Path: page.Path, Content: string(existing)}, page).Content, nil
}
// mergeAll deduplicates pages by path, merging content from later occurrences.
func mergeAll(pages []wiki.Page) []wiki.Page {
order := make([]string, 0, len(pages))
@@ -171,3 +171,65 @@ func mustJSON(v any) string {
}
return string(b)
}
// --- B1: compile-on-ingest — revise the existing page, never clobber it -----
func TestRunRaw_MergesIntoAnExistingPageInsteadOfOverwritingIt(t *testing.T) {
// Before this, buildAndWrite did an unconditional os.WriteFile: re-ingesting
// a source replaced its page wholesale, discarding anything accumulated
// since — including human edits. hyperguild#43 recorded the symptom as
// "re-ingest overwrites wholesale".
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
A book about shaping product work.
## My Take
Read this before the next planning cycle.
`), 0o644))
_, err := RunRaw(dir, "shape-up", []RawPage{{
Title: "Shape Up", Type: "source", Subtype: "book", Domain: "product-strategy",
Content: "## Summary\n\nA book about shaping product work.\n\n## Key Claims\n\n- Appetite beats estimation.\n",
}}, false)
require.NoError(t, err)
got, err := os.ReadFile(page)
require.NoError(t, err)
s := string(got)
assert.Contains(t, s, "## My Take", "a section the extraction never saw must survive the merge")
assert.Contains(t, s, "Read this before the next planning cycle.")
assert.Contains(t, s, "Appetite beats estimation.", "newly extracted material must still land")
}
func TestRunRaw_ReIngestingUnchangedContentIsByteIdentical(t *testing.T) {
// Compile-on-ingest runs on every ingest, so "nothing changed" must produce
// no diff at all. Otherwise every re-ingest dirties the tree, brain-sync
// commits it, and the git history fills with noise that hides real edits.
dir := t.TempDir()
raw := []RawPage{{
Title: "Shape Up", Type: "source", Subtype: "book", Domain: "product-strategy",
Content: "## Summary\n\nA book about shaping product work.\n\n## Open Questions\n\n- Does appetite scale?\n",
}}
_, err := RunRaw(dir, "shape-up", raw, false)
require.NoError(t, err)
first, err := os.ReadFile(filepath.Join(dir, "wiki", "sources", "shape-up.md"))
require.NoError(t, err)
_, err = RunRaw(dir, "shape-up", raw, false)
require.NoError(t, err)
second, err := os.ReadFile(filepath.Join(dir, "wiki", "sources", "shape-up.md"))
require.NoError(t, err)
assert.Equal(t, string(first), string(second),
"re-ingesting unchanged content must leave the page byte-identical")
}
+104 -13
View File
@@ -29,12 +29,16 @@ type section struct {
}
// Merge combines two Page values with the same path.
// Frontmatter is taken from a. 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.
// 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, secsA := parseSections(a.Content)
_, secsB := parseSections(b.Content)
fmA, preambleA, secsA := parseSections(a.Content)
_, _, secsB := parseSections(b.Content)
idx := make(map[string]int, len(secsA))
for i, s := range secsA {
@@ -53,14 +57,24 @@ func Merge(a, b Page) Page {
case bulletSections[sB.heading]:
secsA[i].content = mergeBullets(sA.content, sB.content)
case appendSections[sB.heading]:
secsA[i].content = strings.TrimRight(sA.content, "\n") + "\n\n" + strings.TrimLeft(sB.content, "\n")
secsA[i].content = appendNew(sA.content, sB.content)
}
}
return Page{Path: a.Path, Content: rebuildContent(fmA, secsA)}
return Page{Path: a.Path, Content: rebuildContent(fmA, preambleA, secsA)}
}
func parseSections(markdown string) (frontmatter string, sections []section) {
// 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
@@ -78,6 +92,22 @@ func parseSections(markdown string) (frontmatter string, sections []section) {
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]
@@ -96,24 +126,85 @@ func parseSections(markdown string) (frontmatter string, sections []section) {
return
}
func rebuildContent(frontmatter string, sections []section) string {
// 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 {
fmt.Fprintf(&sb, "\n## %s\n\n%s", sec.heading, sec.content)
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") {
trimmed := strings.TrimSpace(line)
if trimmed == "" || seen[trimmed] {
c := canonicalLine(line)
if c == "" || seen[c] {
continue
}
seen[trimmed] = true
seen[c] = true
lines = append(lines, line)
}
return strings.Join(lines, "\n") + "\n"
+121
View File
@@ -53,3 +53,124 @@ func TestMerge_KeepsFrontmatterFromA(t *testing.T) {
assert.Contains(t, got.Content, "title: A")
assert.NotContains(t, got.Content, "title: B")
}
// --- hyperguild#103 review: a page with content outside any `## ` section ---
// must never lose that content on merge. parseSections used to only capture
// text between `## ` headings — anything before the first heading (a common,
// ordinary shape for a hand-written note: an intro paragraph before the first
// section) or, worse, an entire page with no `## ` headings at all (a legacy
// note, a stub, any hand-edit that doesn't follow the section convention)
// vanished silently. Merge would then fill the page entirely from the fresh
// extraction — the opposite of "never silently rewrite what a human put
// there," the guarantee this whole file exists to provide.
func TestMerge_PreservesOnDiskContentWithNoSectionHeadings(t *testing.T) {
a := Page{Path: "wiki/x.md", Content: "---\ntitle: X\n---\n\nJust some free text a human wrote. No section headings at all.\n"}
b := Page{Path: "wiki/x.md", Content: "---\ntitle: X\n---\n\n## Definition\n\nFresh extraction's take.\n"}
got := Merge(a, b)
assert.Contains(t, got.Content, "Just some free text a human wrote",
"on-disk content with no ## headings must survive the merge, not be replaced wholesale by the fresh extraction")
}
func TestMerge_DegenerateEmptyHeadingDoesNotCollideWithPreamble(t *testing.T) {
// Second review pass on #103: representing the preamble as a section with
// heading=="" collides with a literal `## ` (marker, no title text) —
// strings.TrimPrefix("## ", "## ") is also "". `a` has only a preamble
// (no degenerate heading of its own); `b` introduces a genuinely new
// degenerate `## ` section. Because both key to "", Merge's idx thinks
// b's section "already exists" in a (it's really just a's unrelated
// preamble) and never adds it — b's section vanishes from the output
// entirely, not merged, not kept, not appended.
a := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\nAn intro paragraph.\n\n## Definition\n\nA thing.\n"}
b := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\n## \n\nA genuinely new degenerate section's body.\n\n## Definition\n\nA thing.\n"}
got := Merge(a, b)
assert.Contains(t, got.Content, "An intro paragraph.", "a's own preamble must survive")
assert.Contains(t, got.Content, "A genuinely new degenerate section's body.",
"b's degenerate section is new content, not a collision with a's unrelated preamble — it must not be silently dropped")
}
func TestMerge_PreservesPreambleBeforeFirstHeading(t *testing.T) {
a := Page{Path: "wiki/x.md", Content: "---\ntitle: X\n---\n\nAn intro paragraph before the first heading.\n\n## Definition\n\nA thing.\n"}
b := Page{Path: "wiki/x.md", Content: "---\ntitle: X\n---\n\n## Definition\n\nA thing.\n"}
got := Merge(a, b)
assert.Contains(t, got.Content, "An intro paragraph before the first heading",
"text before the first ## heading is ordinary page content, not something the parser is entitled to drop")
}
// --- hyperguild#103 review: dedup must be whitespace-insensitive -----------
// Page content is produced by an LLM per run, so byte-identical re-extraction
// of "the same" logical line is not guaranteed — different bullet spacing or
// trailing whitespace on an otherwise-identical line must still dedup, or
// idempotency only holds when the extractor happens to reproduce whitespace
// exactly, which is not a real guarantee.
func TestMerge_AppendSectionDedupsAcrossWhitespaceVariants(t *testing.T) {
a := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\n## Open Questions\n\n- Does this converge?\n"}
b := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\n## Open Questions\n\n- Does this converge? \n"}
got := Merge(a, b)
assert.Equal(t, 1, strings.Count(got.Content, "Does this converge?"),
"same logical line with different whitespace must dedup, not duplicate")
}
func TestMerge_BulletSectionDedupsAcrossWhitespaceVariants(t *testing.T) {
a := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\n## Related Concepts\n\n- [[bar|Bar]]\n"}
b := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\n## Related Concepts\n\n- [[bar|Bar]]\n"}
got := Merge(a, b)
assert.Equal(t, 1, strings.Count(got.Content, "[[bar|Bar]]"),
"same logical bullet with different whitespace must dedup, not duplicate")
}
// --- brain#21/B1: merging must be safe to run again -------------------------
func TestMerge_AppendSectionDoesNotDuplicateContentAlreadyPresent(t *testing.T) {
// Compile-on-ingest merges a freshly extracted page into the one already
// on disk. Bullet sections union, so they are naturally idempotent — but
// append sections concatenate, so re-ingesting an unchanged source would
// grow "Open Questions" on every run until the page is unreadable.
page := Page{Path: "wiki/sources/x.md", Content: `---
title: X
---
## Open Questions
- Does this converge?
`}
once := Merge(page, page)
twice := Merge(once, page)
assert.Equal(t, 1, strings.Count(once.Content, "Does this converge?"),
"merging a page with itself must not duplicate an append section")
assert.Equal(t, once.Content, twice.Content,
"merge must be idempotent — a third pass may not keep growing the page")
}
func TestMerge_AppendSectionStillAccumulatesGenuinelyNewContent(t *testing.T) {
// The dedup must not turn append sections into keep-a-only sections:
// new material is exactly what compile-on-ingest exists to add.
existing := Page{Path: "wiki/sources/x.md", Content: `---
title: X
---
## Open Questions
- Does this converge?
`}
fresh := Page{Path: "wiki/sources/x.md", Content: `---
title: X
---
## Open Questions
- What happens under load?
`}
got := Merge(existing, fresh).Content
assert.Contains(t, got, "Does this converge?", "existing question must survive")
assert.Contains(t, got, "What happens under load?", "new question must be added")
}