Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
9345a0a867 | ||
|
|
a5a328fea5 |
@@ -104,6 +104,24 @@ Two fields are **generator-owned** and do get refreshed on a revise:
|
||||
`last_updated` and `source_hash`. Everything else on disk wins. The boundary is
|
||||
an explicit allowlist, so a field nobody has thought about stays human-owned.
|
||||
|
||||
### Disagreement is reported, not resolved
|
||||
|
||||
When the fresh extraction produces a *different version* of a section that
|
||||
already exists, the on-disk version still wins — but the run now says so:
|
||||
|
||||
```
|
||||
wiki/sources/shape-up.md: source now disagrees on section "Summary" — on-disk version kept
|
||||
```
|
||||
|
||||
That line lands in `log.md` with the rest of the run's warnings. Without it, "the
|
||||
source is unchanged" and "the source changed its mind and we ignored it" look
|
||||
identical from the outside.
|
||||
|
||||
This is deliberately **textual** disagreement, not semantic contradiction: cheap,
|
||||
deterministic, no model on the write path. Deciding whether a disagreement is a
|
||||
genuine contradiction stays with the Judge-gated `brain-enrich` pass, which is
|
||||
approval-gated by design. Detection here, adjudication there.
|
||||
|
||||
## Re-ingesting an unchanged source is skipped
|
||||
|
||||
Ingest stores `source_hash` — sha256 of the ingested **body**, frontmatter
|
||||
|
||||
@@ -62,20 +62,13 @@ 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, existingPage)
|
||||
userPrompt := BuildPrompt(schema, source, chunk, inventory)
|
||||
output, err := cfg.Complete(ctx, systemPrompt, userPrompt)
|
||||
if err != nil {
|
||||
return Result{}, fmt.Errorf("LLM call: %w", err)
|
||||
@@ -124,10 +117,18 @@ 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)
|
||||
}
|
||||
content, err := reviseOrCreate(dest, page)
|
||||
content, conflicts, err := reviseOrCreate(dest, page)
|
||||
if err != nil {
|
||||
return Result{}, err
|
||||
}
|
||||
for _, heading := range conflicts {
|
||||
// Reported, not resolved. The on-disk version stands; whether
|
||||
// the disagreement is a genuine contradiction is the
|
||||
// Judge-gated enrich pass's call, not the write path's.
|
||||
warnings = append(warnings, fmt.Sprintf(
|
||||
"%s: source now disagrees on section %q — on-disk version kept",
|
||||
page.Path, heading))
|
||||
}
|
||||
if err := os.WriteFile(dest, []byte(content), 0o644); err != nil {
|
||||
return Result{}, fmt.Errorf("write %s: %w", page.Path, err)
|
||||
}
|
||||
@@ -167,16 +168,6 @@ 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.
|
||||
//
|
||||
@@ -184,7 +175,11 @@ func readSourcePage(brainDir, sourceSlug 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 {
|
||||
for _, line := range strings.Split(readSourcePage(brainDir, sourceSlug), "\n") {
|
||||
raw, err := os.ReadFile(filepath.Join(brainDir, "wiki", "sources", sourceSlug+".md"))
|
||||
if err != nil {
|
||||
return ""
|
||||
}
|
||||
for _, line := range strings.Split(string(raw), "\n") {
|
||||
if key, value, found := strings.Cut(line, ":"); found && strings.TrimSpace(key) == "source_hash" {
|
||||
return strings.TrimSpace(value)
|
||||
}
|
||||
@@ -211,15 +206,16 @@ func storedSourceHash(brainDir, sourceSlug string) string {
|
||||
// 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) {
|
||||
func reviseOrCreate(dest string, page wiki.Page) (string, []string, error) {
|
||||
existing, err := os.ReadFile(dest)
|
||||
switch {
|
||||
case os.IsNotExist(err):
|
||||
return page.Content, nil
|
||||
return page.Content, nil, nil
|
||||
case err != nil:
|
||||
return "", fmt.Errorf("read existing %s: %w", page.Path, err)
|
||||
return "", nil, fmt.Errorf("read existing %s: %w", page.Path, err)
|
||||
}
|
||||
return wiki.Merge(wiki.Page{Path: page.Path, Content: string(existing)}, page).Content, nil
|
||||
merged, conflicts := wiki.MergeReport(wiki.Page{Path: page.Path, Content: string(existing)}, page)
|
||||
return merged.Content, conflicts, nil
|
||||
}
|
||||
|
||||
// mergeAll deduplicates pages by path, merging content from later occurrences.
|
||||
|
||||
@@ -8,6 +8,7 @@ import (
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -276,104 +277,6 @@ 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.
|
||||
@@ -391,3 +294,34 @@ func TestRun_FrontmatterOnlyChangeIsNotDrift(t *testing.T) {
|
||||
|
||||
assert.Equal(t, 1, calls, "only the body counts as the source")
|
||||
}
|
||||
|
||||
func TestRunRaw_ReportsWhenTheSourceDisagreesWithWhatIsOnDisk(t *testing.T) {
|
||||
// The on-disk version still wins — that is what makes unattended ingest
|
||||
// safe. But the run must say that the source now claims something else,
|
||||
// or a changed source is indistinguishable from an unchanged one.
|
||||
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
|
||||
|
||||
Appetite is fixed, scope is variable.
|
||||
`), 0o644))
|
||||
|
||||
res, err := RunRaw(dir, "shape-up", []RawPage{{
|
||||
Title: "Shape Up", Type: "source", Subtype: "book", Domain: "product",
|
||||
Content: "## Summary\n\nAppetite is negotiable and scope is fixed.\n",
|
||||
}}, false)
|
||||
require.NoError(t, err)
|
||||
|
||||
got, err := os.ReadFile(page)
|
||||
require.NoError(t, err)
|
||||
assert.Contains(t, string(got), "Appetite is fixed", "on-disk version wins")
|
||||
|
||||
joined := strings.Join(res.Warnings, " ")
|
||||
assert.Contains(t, joined, "Summary", "the disagreeing section must be named")
|
||||
assert.Contains(t, joined, "shape-up", "and the page it happened on")
|
||||
}
|
||||
|
||||
@@ -22,18 +22,8 @@ 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. 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 {
|
||||
// BuildPrompt constructs the user prompt for a single chunk.
|
||||
func BuildPrompt(schema, source, content string, inventory map[wiki.PageType][]wiki.Entry) string {
|
||||
var sb strings.Builder
|
||||
|
||||
fmt.Fprintf(&sb, "Today's date is %s.\n\n", time.Now().UTC().Format("2006-01-02"))
|
||||
@@ -59,18 +49,6 @@ 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")
|
||||
|
||||
@@ -3,7 +3,6 @@ package wiki
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"sort"
|
||||
"strings"
|
||||
)
|
||||
|
||||
@@ -37,71 +36,60 @@ type section struct {
|
||||
// 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.
|
||||
// Merge discards any disagreement silently; prefer MergeReport where the
|
||||
// caller can act on it.
|
||||
func Merge(a, b Page) Page {
|
||||
page, _ := MergeReport(a, b)
|
||||
return page
|
||||
}
|
||||
|
||||
// MergeReport is Merge, plus the headings of sections where b carried a
|
||||
// different version of a section a already had — and lost.
|
||||
//
|
||||
// That loss is the property which makes ingest safe to run unattended (#103):
|
||||
// a regenerating writer can add to a note but never rewrite it. But the
|
||||
// discarded text is also a signal, and until now it went in the bin: the source
|
||||
// now says something different about a section we already hold. Nobody could
|
||||
// tell "the source is unchanged" from "the source changed its mind and we
|
||||
// ignored it".
|
||||
//
|
||||
// Deliberately NOT a semantic contradiction check. This reports textual
|
||||
// disagreement, which is cheap, deterministic, and needs no model on the write
|
||||
// path. Deciding whether a disagreement is a genuine contradiction stays with
|
||||
// the Judge-gated enrich pass that already exists (brain-gardener#175), which
|
||||
// is approval-gated by design — detection here, adjudication there.
|
||||
func MergeReport(a, b Page) (Page, []string) {
|
||||
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.
|
||||
var conflicts []string
|
||||
idx := make(map[string]int, len(secsA))
|
||||
for i, s := range secsA {
|
||||
key := normalizeHeading(s.heading)
|
||||
if _, exists := idx[key]; !exists {
|
||||
idx[key] = i
|
||||
}
|
||||
idx[s.heading] = i
|
||||
}
|
||||
|
||||
for _, sB := range secsB {
|
||||
key := normalizeHeading(sB.heading)
|
||||
i, exists := idx[key]
|
||||
i, exists := idx[sB.heading]
|
||||
if !exists {
|
||||
idx[key] = len(secsA)
|
||||
idx[sB.heading] = len(secsA)
|
||||
secsA = append(secsA, sB)
|
||||
continue
|
||||
}
|
||||
sA := secsA[i]
|
||||
switch {
|
||||
case bulletSectionsNorm[key]:
|
||||
case bulletSections[sB.heading]:
|
||||
secsA[i].content = mergeBullets(sA.content, sB.content)
|
||||
case appendSectionsNorm[key]:
|
||||
case appendSections[sB.heading]:
|
||||
secsA[i].content = appendNew(sA.content, sB.content)
|
||||
default:
|
||||
// a wins — but say that b disagreed, rather than dropping it mute.
|
||||
if strings.TrimSpace(sA.content) != strings.TrimSpace(sB.content) {
|
||||
conflicts = append(conflicts, sB.heading)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
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
|
||||
return Page{Path: a.Path, Content: rebuildContent(refreshOwned(fmA, fmB), preambleA, secsA)}, conflicts
|
||||
}
|
||||
|
||||
// generatorOwned names the frontmatter fields the ingest pipeline may refresh
|
||||
@@ -121,131 +109,36 @@ var generatorOwned = map[string]bool{
|
||||
"source_hash": true,
|
||||
}
|
||||
|
||||
// fmLine is one line of a frontmatter block: a parsed key/value pair, or a
|
||||
// raw line (blank, comment, or anything else that isn't a simple "key:
|
||||
// value") kept verbatim. Same shape as api.frontmatter's fmLine — this
|
||||
// package doesn't import that one (it's unexported there, and a's/b's
|
||||
// frontmatter here always arrives as a string, not a parsed struct), so the
|
||||
// small, already-proven set()-appends-if-absent behavior is reproduced
|
||||
// locally rather than string-index surgery reinvented per call site.
|
||||
type fmLine struct {
|
||||
key, value, raw string
|
||||
}
|
||||
|
||||
// parseFrontmatterBlock parses a "---\n...\n---\n" fenced block (the shape
|
||||
// parseSections produces) into an ordered line list. A block with no
|
||||
// opening/closing fence — including the empty string, a page with no
|
||||
// frontmatter at all — returns nil, ok=false: there is nothing to update in
|
||||
// place, only something to potentially create.
|
||||
func parseFrontmatterBlock(block string) (lines []fmLine, ok bool) {
|
||||
if !strings.HasPrefix(block, "---\n") {
|
||||
return nil, false
|
||||
}
|
||||
rest := block[len("---\n"):]
|
||||
end := strings.Index(rest, "\n---\n")
|
||||
if end < 0 {
|
||||
return nil, false
|
||||
}
|
||||
for _, line := range strings.Split(rest[:end], "\n") {
|
||||
// An indented line is a continuation (a YAML block-list item, a
|
||||
// wrapped value) never a top-level key, regardless of whether it
|
||||
// happens to contain a colon — " - 'Chapter 3: The Reckoning'"
|
||||
// is not a key named "- 'Chapter 3". Checked before Cut so a
|
||||
// colon inside one never reaches the key/value path at all.
|
||||
if line != "" && (line[0] == ' ' || line[0] == '\t') {
|
||||
lines = append(lines, fmLine{raw: line})
|
||||
continue
|
||||
}
|
||||
key, val, found := strings.Cut(line, ":")
|
||||
key = strings.TrimSpace(key)
|
||||
if !found || key == "" || strings.HasPrefix(strings.TrimSpace(line), "#") {
|
||||
lines = append(lines, fmLine{raw: line})
|
||||
continue
|
||||
}
|
||||
lines = append(lines, fmLine{key: key, value: strings.TrimSpace(val)})
|
||||
}
|
||||
return lines, true
|
||||
}
|
||||
|
||||
// setLine updates key's value in place if lines already has it, or appends
|
||||
// a new entry if it doesn't — the one behavior the original refreshOwned
|
||||
// got wrong (PR #126 review, round 2: it only ever updated in place).
|
||||
func setLine(lines []fmLine, key, value string) []fmLine {
|
||||
for i := range lines {
|
||||
if lines[i].key == key {
|
||||
lines[i].value = value
|
||||
return lines
|
||||
}
|
||||
}
|
||||
return append(lines, fmLine{key: key, value: value})
|
||||
}
|
||||
|
||||
func renderFrontmatterBlock(lines []fmLine) string {
|
||||
if len(lines) == 0 {
|
||||
return ""
|
||||
}
|
||||
var sb strings.Builder
|
||||
sb.WriteString("---\n")
|
||||
for _, l := range lines {
|
||||
switch {
|
||||
case l.key == "":
|
||||
sb.WriteString(l.raw)
|
||||
case l.value == "":
|
||||
// A key with nothing after the colon (the YAML block-list shape:
|
||||
// "aliases:" followed by indented "- item" lines on their own
|
||||
// raw lines) must stay bare — "key: " with a trailing space
|
||||
// is a different, if invisible, byte sequence, and this file's
|
||||
// whole purpose is byte-stability across an unrelated merge.
|
||||
sb.WriteString(l.key)
|
||||
sb.WriteByte(':')
|
||||
default:
|
||||
sb.WriteString(l.key)
|
||||
sb.WriteString(": ")
|
||||
sb.WriteString(l.value)
|
||||
}
|
||||
sb.WriteByte('\n')
|
||||
}
|
||||
sb.WriteString("---\n")
|
||||
return sb.String()
|
||||
}
|
||||
|
||||
// refreshOwned returns a's frontmatter with the generator-owned fields taken
|
||||
// from b where b supplies them. Fields absent from b keep a's value, so a
|
||||
// generator that stops emitting a field cannot blank it.
|
||||
//
|
||||
// A field b supplies that a doesn't have yet — every page ingested before
|
||||
// this mechanism existed has no source_hash line at all — is appended, not
|
||||
// dropped (round 1 of review: dropping it silently meant source_hash never
|
||||
// populated for the pre-existing corpus). If a has no frontmatter fence at
|
||||
// all, one is created to hold the new fields rather than splicing raw
|
||||
// "key: value" text into the body (round 2: that corrupted the page and the
|
||||
// fields would duplicate on every subsequent merge, since unfenced text
|
||||
// isn't recognized as frontmatter by parseSections on the next pass).
|
||||
func refreshOwned(a, b string) string {
|
||||
bLines, _ := parseFrontmatterBlock(b)
|
||||
bVals := make(map[string]string)
|
||||
for _, l := range bLines {
|
||||
if l.key != "" && generatorOwned[l.key] {
|
||||
bVals[l.key] = l.value
|
||||
for _, line := range strings.Split(b, "\n") {
|
||||
key, value, found := strings.Cut(line, ":")
|
||||
if !found || !generatorOwned[strings.TrimSpace(key)] {
|
||||
continue
|
||||
}
|
||||
bVals[strings.TrimSpace(key)] = strings.TrimSpace(value)
|
||||
}
|
||||
if len(bVals) == 0 {
|
||||
return a
|
||||
}
|
||||
|
||||
aLines, _ := parseFrontmatterBlock(a) // ok==false (no fence) -> aLines nil, same as "start fresh"
|
||||
|
||||
// Sorted for a deterministic, byte-stable result when more than one
|
||||
// field is appended in the same call — map iteration order isn't.
|
||||
keys := make([]string, 0, len(bVals))
|
||||
for k := range bVals {
|
||||
keys = append(keys, k)
|
||||
out := make([]string, 0, len(strings.Split(a, "\n")))
|
||||
for _, line := range strings.Split(a, "\n") {
|
||||
key, _, found := strings.Cut(line, ":")
|
||||
k := strings.TrimSpace(key)
|
||||
if found && generatorOwned[k] {
|
||||
if v, ok := bVals[k]; ok {
|
||||
out = append(out, k+": "+v)
|
||||
delete(bVals, k)
|
||||
continue
|
||||
}
|
||||
}
|
||||
out = append(out, line)
|
||||
}
|
||||
sort.Strings(keys)
|
||||
for _, k := range keys {
|
||||
aLines = setLine(aLines, k, bVals[k])
|
||||
}
|
||||
return renderFrontmatterBlock(aLines)
|
||||
return strings.Join(out, "\n")
|
||||
}
|
||||
|
||||
// parseSections splits markdown into its frontmatter, its preamble (content
|
||||
|
||||
@@ -215,182 +215,55 @@ Rewritten by the model.
|
||||
assert.NotContains(t, got, "Renamed by the model")
|
||||
}
|
||||
|
||||
func TestMerge_GeneratorOwnedFieldsAreAppendedWhenAPageNeverHadThemBefore(t *testing.T) {
|
||||
// Found in review of PR #126: refreshOwned only replaced a generator-owned
|
||||
// line that already existed in a's frontmatter — it never appended one a
|
||||
// didn't have. Every page ingested before this feature shipped has no
|
||||
// source_hash line at all, so on its first re-ingest after deploy, the
|
||||
// fresh hash was silently dropped and the skip-on-unchanged feature this
|
||||
// PR exists to build would never engage for the entire pre-existing
|
||||
// corpus, permanently, with nothing telling anyone.
|
||||
func TestMerge_ReportsSectionsWhereTheFreshVersionWasDiscarded(t *testing.T) {
|
||||
// The asymmetry that makes ingest safe (on-disk wins) also throws away a
|
||||
// real signal: the source now says something different about a section we
|
||||
// already have. Today that disagreement is silently discarded, so nobody
|
||||
// can tell "the source is unchanged" from "the source changed its mind and
|
||||
// we ignored it".
|
||||
existing := Page{Path: "wiki/sources/x.md", Content: `---
|
||||
title: 'X'
|
||||
---
|
||||
|
||||
## Summary
|
||||
|
||||
Original.
|
||||
Appetite is fixed, scope is variable.
|
||||
|
||||
## Method
|
||||
|
||||
Six-week cycles.
|
||||
`}
|
||||
fresh := Page{Path: "wiki/sources/x.md", Content: `---
|
||||
title: 'Renamed by the model'
|
||||
last_updated: 2026-08-22
|
||||
source_hash: bbbb
|
||||
---
|
||||
|
||||
## Summary
|
||||
|
||||
Rewritten by the model.
|
||||
`}
|
||||
|
||||
got := Merge(existing, fresh).Content
|
||||
|
||||
// Exact frontmatter block, not Contains: the second review round found
|
||||
// that a substring check doesn't prove the fields landed inside a valid
|
||||
// fence, in the right order, or that the fence itself wasn't corrupted —
|
||||
// two independent mutations (closeIdx targeting the wrong fence, reversed
|
||||
// sort) both survived a Contains-only version of this test.
|
||||
want := "---\ntitle: 'X'\nlast_updated: 2026-08-22\nsource_hash: bbbb\n---\n\n## Summary\n\nOriginal.\n"
|
||||
assert.Equal(t, want, got)
|
||||
}
|
||||
|
||||
func TestMerge_GeneratorOwnedFieldsCreateAFrontmatterBlockWhenThePageHadNoneAtAll(t *testing.T) {
|
||||
// Second review round on PR #126: when a has no frontmatter fence at all,
|
||||
// the append fallback spliced raw, unfenced "key: value" lines straight
|
||||
// into the body -- worse than the original bug, because the next
|
||||
// parseSections call wouldn't recognize them as frontmatter (they don't
|
||||
// start with `---`), so they'd be absorbed as ordinary preamble text and
|
||||
// re-duplicate on every subsequent merge instead of updating in place.
|
||||
existing := Page{Path: "wiki/sources/x.md", Content: "## Summary\n\nOriginal.\n"}
|
||||
fresh := Page{Path: "wiki/sources/x.md", Content: `---
|
||||
last_updated: 2026-08-22
|
||||
source_hash: bbbb
|
||||
---
|
||||
|
||||
## Summary
|
||||
|
||||
Rewritten by the model.
|
||||
`}
|
||||
|
||||
got := Merge(existing, fresh).Content
|
||||
|
||||
want := "---\nlast_updated: 2026-08-22\nsource_hash: bbbb\n---\n\n## Summary\n\nOriginal.\n"
|
||||
assert.Equal(t, want, got)
|
||||
}
|
||||
|
||||
func TestMerge_GeneratorOwnedFieldsMixedReplaceAndAppend(t *testing.T) {
|
||||
// a already has last_updated (replace in place) but not source_hash
|
||||
// (append) -- the two code paths in the same call, together.
|
||||
existing := Page{Path: "wiki/sources/x.md", Content: `---
|
||||
title: 'X'
|
||||
last_updated: 2026-01-01
|
||||
---
|
||||
|
||||
## Summary
|
||||
|
||||
Original.
|
||||
Appetite is negotiable and scope is fixed.
|
||||
|
||||
## Method
|
||||
|
||||
Six-week cycles.
|
||||
`}
|
||||
fresh := Page{Path: "wiki/sources/x.md", Content: `---
|
||||
last_updated: 2026-08-22
|
||||
source_hash: bbbb
|
||||
|
||||
got, conflicts := MergeReport(existing, fresh)
|
||||
|
||||
assert.Contains(t, got.Content, "Appetite is fixed", "on-disk version still wins")
|
||||
assert.NotContains(t, got.Content, "negotiable")
|
||||
assert.Equal(t, []string{"Summary"}, conflicts,
|
||||
"only the section that actually disagreed is reported")
|
||||
}
|
||||
|
||||
func TestMerge_ReportsNothingWhenTheFreshVersionAgrees(t *testing.T) {
|
||||
page := Page{Path: "wiki/sources/x.md", Content: `---
|
||||
title: 'X'
|
||||
---
|
||||
|
||||
## Summary
|
||||
|
||||
Rewritten by the model.
|
||||
Appetite is fixed, scope is variable.
|
||||
`}
|
||||
|
||||
got := Merge(existing, fresh).Content
|
||||
|
||||
want := "---\ntitle: 'X'\nlast_updated: 2026-08-22\nsource_hash: bbbb\n---\n\n## Summary\n\nOriginal.\n"
|
||||
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, "#")).
|
||||
// Without it, a comment line containing a colon gets cut at the colon
|
||||
// and re-rendered with collapsed whitespace instead of surviving
|
||||
// verbatim.
|
||||
a := "---\n#see also: https://x/y\ntitle: 'X'\n---\n"
|
||||
b := "---\nsource_hash: bbbb\n---\n"
|
||||
|
||||
got := refreshOwned(a, b)
|
||||
assert.Equal(t, "---\n#see also: https://x/y\ntitle: 'X'\nsource_hash: bbbb\n---\n", got)
|
||||
}
|
||||
|
||||
func TestRefreshOwned_IndentedListItemsWithColonsAreNotMisparsedAsKeys(t *testing.T) {
|
||||
// Same round: an indented YAML block-list item that happens to contain a
|
||||
// colon (e.g. a title with a subtitle) is a continuation line, never a
|
||||
// top-level key, regardless of what strings.Cut finds in it. Misparsing
|
||||
// it as a key/value pair and re-rendering strips its indentation,
|
||||
// de-indenting it out of the list it belongs to -- and refreshOwned
|
||||
// re-renders this block on essentially every non-skipped ingest, so this
|
||||
// is the normal path, not an edge case.
|
||||
a := "---\naliases:\n - 'Chapter 3: The Reckoning'\ntitle: 'X'\n---\n"
|
||||
b := "---\nsource_hash: bbbb\n---\n"
|
||||
|
||||
got := refreshOwned(a, b)
|
||||
assert.Equal(t, "---\naliases:\n - 'Chapter 3: The Reckoning'\ntitle: 'X'\nsource_hash: bbbb\n---\n", got)
|
||||
_, conflicts := MergeReport(page, page)
|
||||
assert.Empty(t, conflicts, "identical content is not a disagreement")
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user