feat(pipeline): skip extraction when the source has not changed (#105) #109
@@ -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 |
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
|
||||
@@ -53,7 +53,7 @@ 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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -96,15 +96,64 @@ func parseSections(markdown string) (frontmatter string, sections []section) {
|
||||
return
|
||||
}
|
||||
|
||||
// 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 string, sections []section) string {
|
||||
var sb strings.Builder
|
||||
sb.WriteString(frontmatter)
|
||||
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()
|
||||
}
|
||||
|
||||
// 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 t := strings.TrimSpace(line); t != "" {
|
||||
seen[t] = true
|
||||
}
|
||||
}
|
||||
|
||||
var add []string
|
||||
for _, line := range strings.Split(b, "\n") {
|
||||
t := strings.TrimSpace(line)
|
||||
if t == "" || seen[t] {
|
||||
continue
|
||||
}
|
||||
seen[t] = 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
|
||||
|
||||
@@ -53,3 +53,53 @@ func TestMerge_KeepsFrontmatterFromA(t *testing.T) {
|
||||
assert.Contains(t, got.Content, "title: A")
|
||||
assert.NotContains(t, got.Content, "title: B")
|
||||
}
|
||||
|
||||
// --- 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")
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user