Co-authored-by: Mathias <mthbqv@gmail.com>
This commit was merged in pull request #132.
This commit is contained in:
@@ -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/<sourceSlug>.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)
|
||||
}
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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")
|
||||
|
||||
@@ -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.
|
||||
//
|
||||
|
||||
@@ -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, "#")).
|
||||
|
||||
Reference in New Issue
Block a user