diff --git a/ingestion/internal/pipeline/pipeline.go b/ingestion/internal/pipeline/pipeline.go index f7c3487..bdf8cf6 100644 --- a/ingestion/internal/pipeline/pipeline.go +++ b/ingestion/internal/pipeline/pipeline.go @@ -62,13 +62,20 @@ func Run(ctx context.Context, cfg Config, brainDir, content, source string, dryR }}, nil } + // The source's own page path is deterministic (sourceSlug, computed above) + // even before extraction runs, unlike a concept/entity page's path, which + // only exists once the model has produced a title. So page-aware + // extraction (#106) is scoped to the source page: it's the one page every + // chunk's prompt can be given in advance. + existingPage := readSourcePage(brainDir, sourceSlug) + chunks := Chunk(content, cfg.ChunkSize) var allRaw []RawPage var allWarnings []string for _, chunk := range chunks { - userPrompt := BuildPrompt(schema, source, chunk, inventory) + userPrompt := BuildPrompt(schema, source, chunk, inventory, existingPage) output, err := cfg.Complete(ctx, systemPrompt, userPrompt) if err != nil { return Result{}, fmt.Errorf("LLM call: %w", err) @@ -160,6 +167,16 @@ func hashBody(content string) string { return hex.EncodeToString(sum[:]) } +// readSourcePage returns the full on-disk content of the source page at +// wiki/sources/.md, or "" when it doesn't exist yet. +func readSourcePage(brainDir, sourceSlug string) string { + raw, err := os.ReadFile(filepath.Join(brainDir, "wiki", "sources", sourceSlug+".md")) + if err != nil { + return "" + } + return string(raw) +} + // storedSourceHash reads source_hash from the source page a previous ingest // wrote, or "" when there is no such page or no such field. // @@ -167,11 +184,7 @@ func hashBody(content string) string { // 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") { + for _, line := range strings.Split(readSourcePage(brainDir, sourceSlug), "\n") { if key, value, found := strings.Cut(line, ":"); found && strings.TrimSpace(key) == "source_hash" { return strings.TrimSpace(value) } diff --git a/ingestion/internal/pipeline/pipeline_test.go b/ingestion/internal/pipeline/pipeline_test.go index d208fe3..25796a0 100644 --- a/ingestion/internal/pipeline/pipeline_test.go +++ b/ingestion/internal/pipeline/pipeline_test.go @@ -276,6 +276,104 @@ func TestRun_ReExtractsWhenTheSourceBodyChanged(t *testing.T) { assert.Equal(t, 2, calls, "changed content must be re-extracted") } +// --- #106: page-aware extraction — the model sees the page it is revising -- + +func TestRun_ExistingSourcePageIsIncludedInTheExtractionPrompt(t *testing.T) { + // #103 made the merge asymmetric (on-disk wins); #106 is the other half — + // the extraction itself gets to see what's already there instead of + // producing a page blind and letting the merge reconcile it afterwards. + // This only proves the existing page reaches the prompt; the byte-identical + // guarantee for an echoed-back revision is a separate test below. + dir := t.TempDir() + for _, sub := range []string{"wiki/concepts", "wiki/entities", "wiki/sources"} { + require.NoError(t, os.MkdirAll(filepath.Join(dir, sub), 0o755)) + } + + seeded := "---\ntitle: 'Shape Up'\n---\n\n## Summary\n\nA book about shaping product work.\n\n## Key Claims\n\n- Appetite beats estimation.\n" + page := filepath.Join(dir, "wiki", "sources", "shape-up.md") + require.NoError(t, os.WriteFile(page, []byte(seeded), 0o644)) + + var gotPrompt string + cfg := Config{Complete: func(_ context.Context, _, user string) (string, error) { + gotPrompt = user + return mustJSON([]RawPage{{ + Title: "Shape Up", Type: "source", Subtype: "book", + Content: "## Summary\n\nA book about shaping product work.\n", + }}), nil + }} + + // Deliberately does not mention "Appetite beats estimation." — that phrase + // can only reach the prompt via the existing on-disk page, not the source + // content, so its presence proves the page was threaded through. + content := "A short editorial update to the book's listing. No new claims here." + _, err := Run(context.Background(), cfg, dir, content, "shape-up", false) + require.NoError(t, err) + + assert.Contains(t, gotPrompt, "Appetite beats estimation.", + "the existing page's content must be visible to the extraction prompt") + assert.Contains(t, gotPrompt, "revis", "the prompt must ask for a revision, not a fresh page") +} + +func TestRun_NewSourceHasNoExistingPageInThePrompt(t *testing.T) { + // The other side of the same behavior: a source with no page on disk yet + // must not get an "existing page" section injected into its prompt at all. + dir := t.TempDir() + for _, sub := range []string{"wiki/concepts", "wiki/entities", "wiki/sources"} { + require.NoError(t, os.MkdirAll(filepath.Join(dir, sub), 0o755)) + } + + var gotPrompt string + cfg := Config{Complete: func(_ context.Context, _, user string) (string, error) { + gotPrompt = user + return mustJSON([]RawPage{{ + Title: "New Thing", Type: "source", Subtype: "article", + Content: "## Summary\n\nBrand new.\n", + }}), nil + }} + + _, err := Run(context.Background(), cfg, dir, "Brand new content.", "new-thing", false) + require.NoError(t, err) + + assert.NotContains(t, gotPrompt, "Current page", + "a source with no page on disk yet must not get a revision section in its prompt") +} + +func TestRun_RevisionEchoingExistingPageStaysByteIdentical(t *testing.T) { + // The issue's explicit risk: a model given the current page will sometimes + // echo it back verbatim. That must not duplicate content or otherwise + // perturb the page — the deterministic merge is still the backstop. + dir := t.TempDir() + for _, sub := range []string{"wiki/concepts", "wiki/entities", "wiki/sources"} { + require.NoError(t, os.MkdirAll(filepath.Join(dir, sub), 0o755)) + } + + seeded := "---\ntitle: 'Shape Up'\n---\n\n## Summary\n\nA book about shaping product work.\n\n## Key Claims\n\n- Appetite beats estimation.\n" + page := filepath.Join(dir, "wiki", "sources", "shape-up.md") + require.NoError(t, os.WriteFile(page, []byte(seeded), 0o644)) + + cfg := Config{Complete: func(_ context.Context, _, _ string) (string, error) { + return mustJSON([]RawPage{{ + Title: "Shape Up", Type: "source", Subtype: "book", + Content: "## Summary\n\nA book about shaping product work.\n\n## Key Claims\n\n- Appetite beats estimation.\n", + }}), nil + }} + + content := "A short editorial update to the book's listing. No new claims here." + result, err := Run(context.Background(), cfg, dir, content, "shape-up", false) + require.NoError(t, err) + require.False(t, result.Skipped) + + got, err := os.ReadFile(page) + require.NoError(t, err) + + want := "---\ntitle: 'Shape Up'\nlast_updated: " + time.Now().UTC().Format("2006-01-02") + + "\nsource_hash: " + hashBody(content) + "\n---\n\n" + + "## Summary\n\nA book about shaping product work.\n\n" + + "## Key Claims\n\n- Appetite beats estimation.\n" + assert.Equal(t, want, string(got), + "a revision pass that echoes the existing page back must not duplicate or alter its content") +} + 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. diff --git a/ingestion/internal/pipeline/prompt.go b/ingestion/internal/pipeline/prompt.go index 00b65c7..bdfa2e5 100644 --- a/ingestion/internal/pipeline/prompt.go +++ b/ingestion/internal/pipeline/prompt.go @@ -22,8 +22,18 @@ Each element must have exactly these fields: Wikilinks in content: [[Display Name]] — just the display name, no slug, no pipe separator. Only link to pages listed in the inventory or pages you are creating in this response.` -// BuildPrompt constructs the user prompt for a single chunk. -func BuildPrompt(schema, source, content string, inventory map[wiki.PageType][]wiki.Entry) string { +// BuildPrompt constructs the user prompt for a single chunk. existingPage is +// the full current content (frontmatter + body) of the source's own wiki page +// when one already exists on disk, or "" for a source being ingested for the +// first time. When non-empty, the model is asked to revise that page rather +// than write a fresh one — see hyperguild#106. The deterministic merge in +// wiki.Merge remains the backstop regardless of what the model returns: an +// on-disk section the model doesn't touch survives untouched, and a section +// it does touch only takes effect where Merge's strategy allows the fresh +// side to change anything (bullet union, append) — never for an ordinary +// section, so the model proposing a revision is never how a page gets +// overwritten. +func BuildPrompt(schema, source, content string, inventory map[wiki.PageType][]wiki.Entry, existingPage string) string { var sb strings.Builder fmt.Fprintf(&sb, "Today's date is %s.\n\n", time.Now().UTC().Format("2006-01-02")) @@ -49,6 +59,18 @@ func BuildPrompt(schema, source, content string, inventory map[wiki.PageType][]w sb.WriteString("\n") } + if existingPage != "" { + sb.WriteString("## Current page for this source — revise it, don't restart\n\n") + sb.WriteString("This source has already been ingested and has the wiki page shown below.\n") + sb.WriteString("You are revising that page, not writing a fresh one:\n") + sb.WriteString(" - Add sections or claims the source below supports that this page doesn't have yet.\n") + sb.WriteString(" - Strengthen a claim where the source gives more detail, evidence, or nuance than what's already there.\n") + sb.WriteString(" - Leave unchanged whatever the source doesn't affect — reuse the existing wording verbatim rather than paraphrasing it, so an unchanged claim is recognized as unchanged.\n") + sb.WriteString(" - A merge step keeps this page's on-disk content for any section you don't return, so it's fine to omit a section you aren't revising.\n\n") + sb.WriteString(existingPage) + sb.WriteString("\n\n") + } + sb.WriteString("## Non-negotiable rules\n\n") sb.WriteString("1. Output ONLY a valid JSON array — no prose, no fences.\n") sb.WriteString("2. Fields: title, type, subtype (if applicable), domain (if applicable), content.\n") diff --git a/ingestion/internal/wiki/merge.go b/ingestion/internal/wiki/merge.go index 20ca53f..a233fd1 100644 --- a/ingestion/internal/wiki/merge.go +++ b/ingestion/internal/wiki/merge.go @@ -41,23 +41,34 @@ func Merge(a, b Page) Page { fmA, preambleA, secsA := parseSections(a.Content) fmB, _, secsB := parseSections(b.Content) + // First occurrence wins on a collision — a page can already carry two + // on-disk sections that only differ by case or whitespace (the exact + // corpus drift this normalization exists to catch going forward, found + // pre-existing in review of PR #130): a plain last-write assignment + // here would silently and deterministically route every future update + // to whichever duplicate happens to sort last in the file, freezing + // the other one forever with nothing to say so. idx := make(map[string]int, len(secsA)) for i, s := range secsA { - idx[s.heading] = i + key := normalizeHeading(s.heading) + if _, exists := idx[key]; !exists { + idx[key] = i + } } for _, sB := range secsB { - i, exists := idx[sB.heading] + key := normalizeHeading(sB.heading) + i, exists := idx[key] if !exists { - idx[sB.heading] = len(secsA) + idx[key] = len(secsA) secsA = append(secsA, sB) continue } sA := secsA[i] switch { - case bulletSections[sB.heading]: + case bulletSectionsNorm[key]: secsA[i].content = mergeBullets(sA.content, sB.content) - case appendSections[sB.heading]: + case appendSectionsNorm[key]: secsA[i].content = appendNew(sA.content, sB.content) } } @@ -65,6 +76,34 @@ func Merge(a, b Page) Page { return Page{Path: a.Path, Content: rebuildContent(refreshOwned(fmA, fmB), preambleA, secsA)} } +// normalizeHeading returns the key Merge uses to decide "is this the same +// section" — trimmed and case-folded, so "Key Claims" and "Key claims" match +// (hyperguild#106: heading casing drift must not fork a section). It is used +// only for that comparison; the on-disk section's original heading text +// (secsA[i].heading, untouched here) is what actually gets written out, so a +// human's heading casing is never silently rewritten by a merge. +func normalizeHeading(h string) string { + return strings.ToLower(strings.TrimSpace(h)) +} + +// bulletSectionsNorm and appendSectionsNorm are bulletSections/appendSections +// keyed by normalizeHeading instead of exact text, so a section's merge +// strategy (union bullets, append) is chosen by the same normalized identity +// Merge uses to match it — otherwise a heading that matched an on-disk +// section (case-insensitively) could still fail the exact-string strategy +// lookup and silently fall through to "keep a's version," dropping b's +// content instead of unioning or appending it. +var bulletSectionsNorm = normalizeSet(bulletSections) +var appendSectionsNorm = normalizeSet(appendSections) + +func normalizeSet(names map[string]bool) map[string]bool { + out := make(map[string]bool, len(names)) + for k := range names { + out[normalizeHeading(k)] = true + } + return out +} + // generatorOwned names the frontmatter fields the ingest pipeline may refresh // on a page it is revising. Everything else on disk wins. // diff --git a/ingestion/internal/wiki/merge_test.go b/ingestion/internal/wiki/merge_test.go index 5bd962d..762293e 100644 --- a/ingestion/internal/wiki/merge_test.go +++ b/ingestion/internal/wiki/merge_test.go @@ -305,6 +305,68 @@ Rewritten by the model. assert.Equal(t, want, got) } +// --- hyperguild#106: heading casing drift must not fork a section ---------- +// Page-aware extraction asks the model to revise the page it's given, but +// nothing guarantees the model reproduces a heading's exact on-disk casing +// ("Key Claims" vs "Key claims"). Without normalizing the match key, that +// drift silently creates a near-duplicate section instead of updating the +// original — the failure mode #106 exists to close. + +func TestMerge_HeadingCaseDriftMatchesSameSection(t *testing.T) { + a := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\n## Key Claims\n\n- Appetite beats estimation.\n"} + b := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\n## Key claims\n\n- New pattern holds under load.\n"} + + got := Merge(a, b).Content + + assert.Equal(t, 1, strings.Count(got, "## Key"), + "casing drift in the heading must not create a second section") + assert.Contains(t, got, "Appetite beats estimation.") + assert.Contains(t, got, "New pattern holds under load.", + "the differently-cased section's content must still be unioned in") + assert.Contains(t, got, "## Key Claims", + "the on-disk heading's original casing must be preserved in the output") + assert.NotContains(t, got, "## Key claims", + "the fresh extraction's heading casing must not overwrite the on-disk display text") +} + +func TestMerge_HeadingWhitespaceDriftMatchesSameSection(t *testing.T) { + a := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\n## Open Questions \n\n- Does appetite scale?\n"} + b := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\n## Open Questions\n\n- What about remote teams?\n"} + + got := Merge(a, b).Content + + assert.Equal(t, 1, strings.Count(got, "## Open Questions"), + "surrounding whitespace drift in the heading must not create a second section") + assert.Contains(t, got, "Does appetite scale?") + assert.Contains(t, got, "What about remote teams?") +} + +func TestMerge_PreExistingCaseDuplicateHeadingsRouteToTheFirstOccurrence(t *testing.T) { + // Found in review of PR #130: normalizeHeading's idx map is built by + // iterating secsA with idx[normalizeHeading(s.heading)] = i -- last + // write wins. Before heading normalization existed, two on-disk + // sections differing only by case (real, pre-existing corpus drift -- + // the exact pattern #106 itself cites) never collided, since idx keyed + // on the exact string. After normalization they collide, and every + // fresh update silently, deterministically routes to whichever + // duplicate happens to appear LAST in the file -- the first one is + // frozen forever, never receiving updates again, with nothing to say + // so. A Contains-only check doesn't catch this: nothing is literally + // deleted, a bullet just lands under the wrong of two still-present + // headings. Assert it lands under the FIRST occurrence instead — + // deterministic and matches "earliest is canonical," same reasoning + // as every other on-disk-wins default in this file. + a := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\n## Key Claims\n\n- claim one\n\n## KEY CLAIMS\n\n- claim two\n"} + b := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\n## Key Claims\n\n- claim one\n- fresh claim\n"} + + got := Merge(a, b).Content + + assert.Contains(t, got, "## Key Claims\n\n- claim one\n- fresh claim\n", + "the fresh bullet must land in the first-occurrence section, not silently route to the last duplicate") + assert.Contains(t, got, "## KEY CLAIMS\n\n- claim two\n", + "the second duplicate is untouched by this update, not merged into or overwritten") +} + func TestRefreshOwned_CommentLinesSurviveUnmangled(t *testing.T) { // Third review round on PR #126: parseFrontmatterBlock dropped the // sibling api package's comment guard (strings.HasPrefix(trimmed, "#")).