diff --git a/ingestion/internal/pipeline/pipeline.go b/ingestion/internal/pipeline/pipeline.go index ac63c52..1007729 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) @@ -168,6 +175,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. // @@ -175,11 +192,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 208f913..5c6d048 100644 --- a/ingestion/internal/pipeline/pipeline_test.go +++ b/ingestion/internal/pipeline/pipeline_test.go @@ -277,6 +277,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 644035a..e80be52 100644 --- a/ingestion/internal/wiki/merge.go +++ b/ingestion/internal/wiki/merge.go @@ -64,23 +64,50 @@ func MergeReport(a, b Page) (Page, []string) { fmB, _, secsB := parseSections(b.Content) var conflicts []string - idx := make(map[string]int, len(secsA)) + + // Sections in secsA sharing a normalized heading (pre-existing corpus + // drift — "## Notes" and "## NOTES" both already on disk, the exact + // pattern this normalization exists to catch going forward, found in + // review of PR #130/#132) are matched to b's occurrences of that same + // heading POSITIONALLY: b's 1st occurrence of the key pairs with a's + // 1st, b's 2nd with a's 2nd, and so on. A single "last write wins" (or + // even "first write wins") map from key to one index would instead + // route every one of b's occurrences to the SAME on-disk section — + // which either freezes every duplicate but one (never receiving + // updates again) or, worse, compares mismatched pairs and reports a + // disagreement between two sections that were never supposed to be + // compared, false-positiving on the plain self-merge case a revision + // pass produces when the model echoes a page back unchanged (found in + // review of PR #132). + byKey := make(map[string][]int, len(secsA)) for i, s := range secsA { - idx[s.heading] = i + key := normalizeHeading(s.heading) + byKey[key] = append(byKey[key], i) } + cursor := make(map[string]int, len(byKey)) for _, sB := range secsB { - i, exists := idx[sB.heading] - if !exists { - idx[sB.heading] = len(secsA) + key := normalizeHeading(sB.heading) + indices := byKey[key] + pos := cursor[key] + cursor[key] = pos + 1 + + if pos >= len(indices) { + // b has more occurrences of this heading than a does (including + // the case where a has none at all) — the extra is new content, + // appended as its own section and registered so a THIRD + // occurrence of the same heading in b pairs with this one + // rather than appending yet another duplicate. + byKey[key] = append(byKey[key], len(secsA)) secsA = append(secsA, sB) continue } + i := indices[pos] 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) default: // a wins — but say that b disagreed, rather than dropping it mute. @@ -93,6 +120,34 @@ func MergeReport(a, b Page) (Page, []string) { return Page{Path: a.Path, Content: rebuildContent(refreshOwned(fmA, fmB), preambleA, secsA)}, conflicts } +// 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 4286ff3..9a8464b 100644 --- a/ingestion/internal/wiki/merge_test.go +++ b/ingestion/internal/wiki/merge_test.go @@ -305,6 +305,98 @@ 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 TestMergeReport_SelfMergeOfPreExistingDuplicateHeadingsReportsNoConflict(t *testing.T) { + // Found in review of PR #132: first-occurrence-wins routes ALL of b's + // occurrences of a normalized heading to a's FIRST occurrence, not to + // their own positional counterpart. So merging a page against itself + // (or an echoed-back copy, exactly what #106's revision prompt can + // produce) pairs b's second "## NOTES" against a's first "## Notes" — + // different content, same normalized key — and MergeReport wrongly + // reports a disagreement where none exists. Fixed by pairing + // positionally: b's Nth occurrence of a normalized heading matches a's + // Nth occurrence, not always the first. + page := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\n## Notes\n\nFirst note.\n\n## NOTES\n\nSecond note.\n"} + + _, conflicts := MergeReport(page, page) + assert.Empty(t, conflicts, "merging a page with pre-existing duplicate headings against itself must report no conflicts") +} + +func TestMergeReport_DuplicateHeadingsPairPositionallyNotAllToFirst(t *testing.T) { + // Companion case: b has genuinely different content in its second + // occurrence of the normalized heading — that must compare against a's + // second occurrence (and report if they disagree), not against a's + // first (which would either false-positive if a's first differs, or + // false-negative-mask a real disagreement in the second pair). + a := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\n## Notes\n\nFirst note.\n\n## NOTES\n\nSecond note.\n"} + b := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\n## Notes\n\nFirst note.\n\n## NOTES\n\nSecond note, revised.\n"} + + _, conflicts := MergeReport(a, b) + assert.Equal(t, []string{"NOTES"}, conflicts, + "only the second occurrence, which actually disagrees, is reported") +} + func TestRefreshOwned_CommentLinesSurviveUnmangled(t *testing.T) { // Third review round on PR #126: parseFrontmatterBlock dropped the // sibling api package's comment guard (strings.HasPrefix(trimmed, "#")).