Compare commits
4
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
09aeaa91f5 | ||
|
|
9abe4369f2 | ||
|
|
60a8801a1d | ||
|
|
54237af7ad |
@@ -76,6 +76,33 @@ that prompted the question is answered by naming it here instead.
|
||||
Full reasoning, including the reversal trigger: brain
|
||||
`wiki/hyperguild/decisions/brain-mcp-stays-in-ingestion-fix-discoverability-by-name`.
|
||||
|
||||
## Ingest revises, it does not overwrite
|
||||
|
||||
Re-ingesting a source **merges into the page already on disk** rather than
|
||||
replacing it. Previously the write was unconditional, so a second ingest of the
|
||||
same source discarded everything accumulated since — including hand-written
|
||||
sections.
|
||||
|
||||
The merge is deliberately asymmetric in the existing page's favour:
|
||||
|
||||
| On the page | In the fresh extraction | Result |
|
||||
|---|---|---|
|
||||
| a section | the same section | **the on-disk version wins** |
|
||||
| — | a new section | added |
|
||||
| a bullet list (`Sources`, `Key Claims`, …) | more bullets | unioned, no duplicates |
|
||||
| an append section (`Open Questions`, `Updates`, …) | more lines | only genuinely new lines appended |
|
||||
| frontmatter | frontmatter | **the on-disk frontmatter wins** |
|
||||
|
||||
So an extraction can add to a note but never silently rewrite what a human put
|
||||
there, which is what makes it safe to run on every ingest rather than behind a
|
||||
review gate.
|
||||
|
||||
Re-ingesting unchanged content is **byte-identical** — no diff, so brain-sync
|
||||
does not commit noise that hides real edits.
|
||||
|
||||
Known limitation: because the on-disk frontmatter wins, `last_updated` keeps its
|
||||
first-ingest date on a revised page.
|
||||
|
||||
## Phase 1 tools (available now)
|
||||
|
||||
| Tool | What it does |
|
||||
|
||||
@@ -12,6 +12,7 @@ import (
|
||||
"time"
|
||||
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/brain"
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/claim"
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/extract"
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/pipeline"
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/search"
|
||||
@@ -152,15 +153,24 @@ func writeHallNote(brainDir string, opts WriteNoteOptions) (string, error) {
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
if err := os.MkdirAll(filepath.Dir(dest), 0o755); err != nil {
|
||||
return "", fmt.Errorf("create hall dir: %w", err)
|
||||
}
|
||||
|
||||
existingFields, body := splitFrontmatter(opts.Content)
|
||||
existingByKey := make(map[string]frontmatterField, len(existingFields))
|
||||
for _, f := range existingFields {
|
||||
existingByKey[f.key] = f
|
||||
}
|
||||
|
||||
// Validate the claim-property envelope BEFORE creating the directory, so a
|
||||
// rejected write leaves nothing behind — not even an empty hall dir
|
||||
// (brain#21). Fail closed: a value outside the enum is worse than an absent
|
||||
// one, because a lint skips what it cannot parse and the note then reads as
|
||||
// classified while nothing acts on it.
|
||||
if err := claim.ValidateEnvelope(scalarValues(existingFields)); err != nil {
|
||||
return "", fmt.Errorf("claim envelope: %w", err)
|
||||
}
|
||||
|
||||
if err := os.MkdirAll(filepath.Dir(dest), 0o755); err != nil {
|
||||
return "", fmt.Errorf("create hall dir: %w", err)
|
||||
}
|
||||
emitted := make(map[string]bool, 6)
|
||||
|
||||
var fm strings.Builder
|
||||
@@ -218,6 +228,25 @@ func writeHallNote(brainDir string, opts WriteNoteOptions) (string, error) {
|
||||
return filepath.ToSlash(rel), nil
|
||||
}
|
||||
|
||||
// scalarValues flattens frontmatter fields to key -> scalar value for
|
||||
// validation. Multi-line values (lists, nested blocks) yield the text after
|
||||
// the colon on the first line, which is empty for a list — correct here,
|
||||
// since every claim-property field is a scalar enum.
|
||||
func scalarValues(fields []frontmatterField) map[string]string {
|
||||
out := make(map[string]string, len(fields))
|
||||
for _, f := range fields {
|
||||
if len(f.lines) == 0 {
|
||||
continue
|
||||
}
|
||||
_, value, found := strings.Cut(f.lines[0], ":")
|
||||
if !found {
|
||||
continue
|
||||
}
|
||||
out[f.key] = strings.TrimSpace(value)
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// frontmatterField is one top-level YAML key from a frontmatter block,
|
||||
// along with its raw line and any indented continuation lines (e.g. a
|
||||
// bulleted list value spanning multiple lines).
|
||||
|
||||
@@ -440,3 +440,60 @@ func TestIngestPath_Directory(t *testing.T) {
|
||||
require.True(t, ok, "pages must be an array")
|
||||
assert.NotEmpty(t, pagesSlice)
|
||||
}
|
||||
|
||||
// --- brain#21: claim-property envelope on the write path -------------------
|
||||
|
||||
func TestWriteNote_PreservesClaimEnvelopeVerbatim(t *testing.T) {
|
||||
// writeHallNote already passes unknown keys through; this pins that
|
||||
// behaviour for the envelope specifically, so a future refactor of the
|
||||
// merge loop cannot silently drop it.
|
||||
dir := t.TempDir()
|
||||
|
||||
rel, err := api.WriteNote(dir, api.WriteNoteOptions{
|
||||
Content: "---\nhalf_life: durable\napplies: universal\nclaim_provenance: asserted\n---\n\nA durable claim.\n",
|
||||
Wing: "homelab",
|
||||
Hall: "decisions",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
got, err := os.ReadFile(filepath.Join(dir, filepath.FromSlash(rel)))
|
||||
require.NoError(t, err)
|
||||
assert.Contains(t, string(got), "half_life: durable")
|
||||
assert.Contains(t, string(got), "applies: universal")
|
||||
assert.Contains(t, string(got), "claim_provenance: asserted")
|
||||
}
|
||||
|
||||
func TestWriteNote_RejectsInvalidClaimEnvelopeAndWritesNothing(t *testing.T) {
|
||||
// Fail closed: a value outside the enum must not land. A lint skips what
|
||||
// it cannot parse, so an unvalidated write produces a note that reads as
|
||||
// classified while nothing acts on it.
|
||||
dir := t.TempDir()
|
||||
|
||||
_, err := api.WriteNote(dir, api.WriteNoteOptions{
|
||||
Content: "---\nhalf_life: forever\nclaim_provenance: asserted\n---\n\nBody.\n",
|
||||
Filename: "bad-envelope",
|
||||
Wing: "homelab",
|
||||
Hall: "facts",
|
||||
})
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "half_life")
|
||||
|
||||
_, statErr := os.Stat(filepath.Join(dir, "wiki/homelab/facts/bad-envelope.md"))
|
||||
assert.True(t, os.IsNotExist(statErr), "a rejected write must leave no file behind")
|
||||
}
|
||||
|
||||
func TestWriteNote_RejectsClaimFieldWithoutProvenance(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
|
||||
_, err := api.WriteNote(dir, api.WriteNoteOptions{
|
||||
Content: "---\napplies: homelab\n---\n\nBody.\n",
|
||||
Filename: "no-provenance",
|
||||
Wing: "homelab",
|
||||
Hall: "facts",
|
||||
})
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "claim_provenance")
|
||||
|
||||
_, statErr := os.Stat(filepath.Join(dir, "wiki/homelab/facts/no-provenance.md"))
|
||||
assert.True(t, os.IsNotExist(statErr))
|
||||
}
|
||||
|
||||
@@ -0,0 +1,98 @@
|
||||
// Package claim validates the claim-property frontmatter that says when and
|
||||
// where a note's claim holds, and who decided that (brain#21).
|
||||
//
|
||||
// Two families were designed together and land separately. This is the
|
||||
// applicability envelope:
|
||||
//
|
||||
// half_life: volatile | seasonal | durable — how fast it decays
|
||||
// applies: local | homelab | universal — how widely it holds
|
||||
// claim_provenance: asserted | inferred — who set the above
|
||||
//
|
||||
// The strength family (confidence, contested) follows once the ingest-time
|
||||
// contradiction detector exists to populate it.
|
||||
//
|
||||
// Why the envelope is validated rather than merely accepted: a value outside
|
||||
// the enum is worse than an absent one. A lint skips what it cannot parse, so
|
||||
// the note reads as classified while nothing acts on it — the field becomes
|
||||
// decoration that suppresses the very review it was added to trigger.
|
||||
//
|
||||
// Why `applies` and not `scope`: `scope:` has been in use since brain#5 for
|
||||
// glossary terms (`homelab-core`, `project:<repo>`) — a disjoint value space
|
||||
// on the same key. The shipped schema keeps the name; this one takes another.
|
||||
package claim
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// Field names, exported so writers and lints agree on the spelling.
|
||||
const (
|
||||
FieldHalfLife = "half_life"
|
||||
FieldApplies = "applies"
|
||||
FieldProvenance = "claim_provenance"
|
||||
)
|
||||
|
||||
var (
|
||||
halfLives = []string{"volatile", "seasonal", "durable"}
|
||||
appliesTo = []string{"local", "homelab", "universal"}
|
||||
provenances = []string{"asserted", "inferred"}
|
||||
)
|
||||
|
||||
// ValidateEnvelope checks the envelope fields of one note's frontmatter.
|
||||
//
|
||||
// Absent fields are valid: the envelope is opt-in for new writes, and a
|
||||
// corpus-wide backfill is deliberately out of scope (brain#21) — inferring
|
||||
// 1113 notes' properties in bulk has the same failure mode as the extraction
|
||||
// pass that produced 28 wrong wikilinks out of 67.
|
||||
func ValidateEnvelope(fm map[string]string) error {
|
||||
halfLife, hasHalfLife := present(fm, FieldHalfLife)
|
||||
applies, hasApplies := present(fm, FieldApplies)
|
||||
prov, hasProv := present(fm, FieldProvenance)
|
||||
|
||||
if hasHalfLife {
|
||||
if err := oneOf(FieldHalfLife, halfLife, halfLives); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
if hasApplies {
|
||||
if err := oneOf(FieldApplies, applies, appliesTo); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
if hasProv {
|
||||
if err := oneOf(FieldProvenance, prov, provenances); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
// The field and its provenance travel together or not at all. A value
|
||||
// whose author is unknown cannot be trusted differently from one Mathias
|
||||
// asserted, which is how an inferred guess hardens into accepted fact.
|
||||
switch {
|
||||
case (hasHalfLife || hasApplies) && !hasProv:
|
||||
return fmt.Errorf("%s is required when %s or %s is set",
|
||||
FieldProvenance, FieldHalfLife, FieldApplies)
|
||||
case hasProv && !hasHalfLife && !hasApplies:
|
||||
return fmt.Errorf("%s is set but neither %s nor %s is",
|
||||
FieldProvenance, FieldHalfLife, FieldApplies)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// present reports a trimmed field value and whether it was set to anything
|
||||
// non-empty. A key written with an empty value is treated as absent rather
|
||||
// than as an error: that is a caller emitting a placeholder, not asserting.
|
||||
func present(fm map[string]string, key string) (string, bool) {
|
||||
v := strings.TrimSpace(fm[key])
|
||||
return v, v != ""
|
||||
}
|
||||
|
||||
func oneOf(field, value string, allowed []string) error {
|
||||
for _, a := range allowed {
|
||||
if a == value {
|
||||
return nil
|
||||
}
|
||||
}
|
||||
return fmt.Errorf("%s: %q is not one of %s", field, value, strings.Join(allowed, ", "))
|
||||
}
|
||||
@@ -0,0 +1,89 @@
|
||||
// Package claim_test covers the claim-property envelope (brain#21).
|
||||
package claim_test
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/claim"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestValidateEnvelope(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
fm map[string]string
|
||||
wantErr string
|
||||
}{
|
||||
{
|
||||
name: "absent is fine — the fields are opt-in on new writes",
|
||||
fm: map[string]string{"wing": "homelab", "hall": "facts"},
|
||||
},
|
||||
{
|
||||
name: "complete envelope with provenance",
|
||||
fm: map[string]string{
|
||||
"half_life": "durable", "applies": "universal", "claim_provenance": "asserted",
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "inferred provenance is equally valid — it just must be stated",
|
||||
fm: map[string]string{
|
||||
"half_life": "volatile", "applies": "local", "claim_provenance": "inferred",
|
||||
},
|
||||
},
|
||||
{
|
||||
// A value nobody can act on is worse than no value: a lint
|
||||
// silently skips it, so the note looks classified and isn't.
|
||||
name: "unknown half_life value",
|
||||
fm: map[string]string{"half_life": "forever", "claim_provenance": "asserted"},
|
||||
wantErr: `half_life: "forever" is not one of volatile, seasonal, durable`,
|
||||
},
|
||||
{
|
||||
name: "unknown applies value",
|
||||
fm: map[string]string{"applies": "galaxy", "claim_provenance": "asserted"},
|
||||
wantErr: `applies: "galaxy" is not one of local, homelab, universal`,
|
||||
},
|
||||
{
|
||||
// brain#21: a value a model inferred must never be
|
||||
// indistinguishable from one Mathias asserted. Writing the field
|
||||
// without saying who set it is exactly that laundering.
|
||||
name: "envelope field without provenance",
|
||||
fm: map[string]string{"half_life": "durable"},
|
||||
wantErr: "claim_provenance is required when half_life or applies is set",
|
||||
},
|
||||
{
|
||||
name: "applies without provenance",
|
||||
fm: map[string]string{"applies": "homelab"},
|
||||
wantErr: "claim_provenance is required when half_life or applies is set",
|
||||
},
|
||||
{
|
||||
name: "unknown provenance value",
|
||||
fm: map[string]string{"half_life": "durable", "claim_provenance": "probably"},
|
||||
wantErr: `claim_provenance: "probably" is not one of asserted, inferred`,
|
||||
},
|
||||
{
|
||||
// Provenance alone says who set fields that aren't there.
|
||||
name: "provenance without any envelope field",
|
||||
fm: map[string]string{"claim_provenance": "asserted"},
|
||||
wantErr: "claim_provenance is set but neither half_life nor applies is",
|
||||
},
|
||||
{
|
||||
// The glossary wing has used scope: homelab-core since #5. The
|
||||
// envelope deliberately does NOT claim that key (brain#21).
|
||||
name: "glossary scope is untouched",
|
||||
fm: map[string]string{"scope": "homelab-core", "term": "koala"},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
err := claim.ValidateEnvelope(tt.fm)
|
||||
if tt.wantErr == "" {
|
||||
assert.NoError(t, err)
|
||||
return
|
||||
}
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), tt.wantErr)
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -96,7 +96,11 @@ func buildAndWrite(rawPages []RawPage, sourceSlug, date, brainDir, source string
|
||||
if err := os.MkdirAll(filepath.Dir(dest), 0o755); err != nil {
|
||||
return Result{}, fmt.Errorf("mkdir for %s: %w", page.Path, err)
|
||||
}
|
||||
if err := os.WriteFile(dest, []byte(page.Content), 0o644); err != nil {
|
||||
content, err := reviseOrCreate(dest, page)
|
||||
if err != nil {
|
||||
return Result{}, err
|
||||
}
|
||||
if err := os.WriteFile(dest, []byte(content), 0o644); err != nil {
|
||||
return Result{}, fmt.Errorf("write %s: %w", page.Path, err)
|
||||
}
|
||||
}
|
||||
@@ -115,6 +119,36 @@ func buildAndWrite(rawPages []RawPage, sourceSlug, date, brainDir, source string
|
||||
return Result{Pages: written, Warnings: warnings}, nil
|
||||
}
|
||||
|
||||
// reviseOrCreate returns the bytes to write for one extracted page: merged into
|
||||
// whatever is already on disk, or the page as-is when the path is new.
|
||||
//
|
||||
// This is the compile half of compile-on-ingest. Before it, the write was an
|
||||
// unconditional os.WriteFile, so re-ingesting a source replaced its page
|
||||
// wholesale and discarded everything accumulated since — including human edits
|
||||
// (hyperguild#43: "re-ingest overwrites wholesale").
|
||||
//
|
||||
// The merge is deliberately asymmetric in the existing page's favour:
|
||||
// wiki.Merge keeps the on-disk frontmatter and the on-disk version of any
|
||||
// section both sides carry, adds sections only the extraction produced, and
|
||||
// unions bullet lists. So a freshly extracted page can add to a note but never
|
||||
// silently rewrite what a human put there — which is what makes it safe to run
|
||||
// this on every ingest rather than behind a review gate.
|
||||
//
|
||||
// Known limitation: because the on-disk frontmatter wins, `last_updated` keeps
|
||||
// its first-ingest date on a revised page. Left deliberately — the alternative
|
||||
// is this function editing frontmatter it does not own, and no lint reads that
|
||||
// field today.
|
||||
func reviseOrCreate(dest string, page wiki.Page) (string, error) {
|
||||
existing, err := os.ReadFile(dest)
|
||||
switch {
|
||||
case os.IsNotExist(err):
|
||||
return page.Content, nil
|
||||
case err != nil:
|
||||
return "", fmt.Errorf("read existing %s: %w", page.Path, err)
|
||||
}
|
||||
return wiki.Merge(wiki.Page{Path: page.Path, Content: string(existing)}, page).Content, nil
|
||||
}
|
||||
|
||||
// mergeAll deduplicates pages by path, merging content from later occurrences.
|
||||
func mergeAll(pages []wiki.Page) []wiki.Page {
|
||||
order := make([]string, 0, len(pages))
|
||||
|
||||
@@ -171,3 +171,65 @@ func mustJSON(v any) string {
|
||||
}
|
||||
return string(b)
|
||||
}
|
||||
|
||||
// --- B1: compile-on-ingest — revise the existing page, never clobber it -----
|
||||
|
||||
func TestRunRaw_MergesIntoAnExistingPageInsteadOfOverwritingIt(t *testing.T) {
|
||||
// Before this, buildAndWrite did an unconditional os.WriteFile: re-ingesting
|
||||
// a source replaced its page wholesale, discarding anything accumulated
|
||||
// since — including human edits. hyperguild#43 recorded the symptom as
|
||||
// "re-ingest overwrites wholesale".
|
||||
dir := t.TempDir()
|
||||
page := filepath.Join(dir, "wiki", "sources", "shape-up.md")
|
||||
require.NoError(t, os.MkdirAll(filepath.Dir(page), 0o755))
|
||||
require.NoError(t, os.WriteFile(page, []byte(`---
|
||||
title: 'Shape Up'
|
||||
---
|
||||
|
||||
## Summary
|
||||
|
||||
A book about shaping product work.
|
||||
|
||||
## My Take
|
||||
|
||||
Read this before the next planning cycle.
|
||||
`), 0o644))
|
||||
|
||||
_, err := RunRaw(dir, "shape-up", []RawPage{{
|
||||
Title: "Shape Up", Type: "source", Subtype: "book", Domain: "product-strategy",
|
||||
Content: "## Summary\n\nA book about shaping product work.\n\n## Key Claims\n\n- Appetite beats estimation.\n",
|
||||
}}, false)
|
||||
require.NoError(t, err)
|
||||
|
||||
got, err := os.ReadFile(page)
|
||||
require.NoError(t, err)
|
||||
s := string(got)
|
||||
|
||||
assert.Contains(t, s, "## My Take", "a section the extraction never saw must survive the merge")
|
||||
assert.Contains(t, s, "Read this before the next planning cycle.")
|
||||
assert.Contains(t, s, "Appetite beats estimation.", "newly extracted material must still land")
|
||||
}
|
||||
|
||||
func TestRunRaw_ReIngestingUnchangedContentIsByteIdentical(t *testing.T) {
|
||||
// Compile-on-ingest runs on every ingest, so "nothing changed" must produce
|
||||
// no diff at all. Otherwise every re-ingest dirties the tree, brain-sync
|
||||
// commits it, and the git history fills with noise that hides real edits.
|
||||
dir := t.TempDir()
|
||||
raw := []RawPage{{
|
||||
Title: "Shape Up", Type: "source", Subtype: "book", Domain: "product-strategy",
|
||||
Content: "## Summary\n\nA book about shaping product work.\n\n## Open Questions\n\n- Does appetite scale?\n",
|
||||
}}
|
||||
|
||||
_, err := RunRaw(dir, "shape-up", raw, false)
|
||||
require.NoError(t, err)
|
||||
first, err := os.ReadFile(filepath.Join(dir, "wiki", "sources", "shape-up.md"))
|
||||
require.NoError(t, err)
|
||||
|
||||
_, err = RunRaw(dir, "shape-up", raw, false)
|
||||
require.NoError(t, err)
|
||||
second, err := os.ReadFile(filepath.Join(dir, "wiki", "sources", "shape-up.md"))
|
||||
require.NoError(t, err)
|
||||
|
||||
assert.Equal(t, string(first), string(second),
|
||||
"re-ingesting unchanged content must leave the page byte-identical")
|
||||
}
|
||||
|
||||
@@ -29,12 +29,16 @@ type section struct {
|
||||
}
|
||||
|
||||
// Merge combines two Page values with the same path.
|
||||
// Frontmatter is taken from a. 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.
|
||||
// Frontmatter is taken from a. The preamble (content before the first ##
|
||||
// heading) is taken from a — same "on-disk wins" default every unlisted
|
||||
// named section gets, kept as a separate field rather than a section with
|
||||
// an empty heading so it can never collide with one (see parseSections).
|
||||
// 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.
|
||||
func Merge(a, b Page) Page {
|
||||
fmA, secsA := parseSections(a.Content)
|
||||
_, secsB := parseSections(b.Content)
|
||||
fmA, preambleA, secsA := parseSections(a.Content)
|
||||
_, _, secsB := parseSections(b.Content)
|
||||
|
||||
idx := make(map[string]int, len(secsA))
|
||||
for i, s := range secsA {
|
||||
@@ -53,14 +57,24 @@ func Merge(a, b Page) Page {
|
||||
case bulletSections[sB.heading]:
|
||||
secsA[i].content = mergeBullets(sA.content, sB.content)
|
||||
case appendSections[sB.heading]:
|
||||
secsA[i].content = strings.TrimRight(sA.content, "\n") + "\n\n" + strings.TrimLeft(sB.content, "\n")
|
||||
secsA[i].content = appendNew(sA.content, sB.content)
|
||||
}
|
||||
}
|
||||
|
||||
return Page{Path: a.Path, Content: rebuildContent(fmA, secsA)}
|
||||
return Page{Path: a.Path, Content: rebuildContent(fmA, preambleA, secsA)}
|
||||
}
|
||||
|
||||
func parseSections(markdown string) (frontmatter string, sections []section) {
|
||||
// parseSections splits markdown into its frontmatter, its preamble (content
|
||||
// before the first `## ` heading), and its `## `-headed sections.
|
||||
//
|
||||
// The preamble is returned separately rather than as a section with an
|
||||
// empty heading — a section keyed by heading=="" would collide with a
|
||||
// literal degenerate `## ` (marker, no title text; strings.TrimPrefix also
|
||||
// yields ""), which is a distinct, addressable section a page can genuinely
|
||||
// have, not the same thing as "no heading yet." Two entries sharing one key
|
||||
// in Merge's idx map means one of them silently loses lookup visibility —
|
||||
// found in review as real, reproducible data loss, not a hypothetical.
|
||||
func parseSections(markdown string) (frontmatter, preamble string, sections []section) {
|
||||
lines := strings.Split(markdown, "\n")
|
||||
i := 0
|
||||
|
||||
@@ -78,6 +92,22 @@ func parseSections(markdown string) (frontmatter string, sections []section) {
|
||||
frontmatter = fmt.Sprintf("---\n%s\n---\n", strings.Join(fmLines, "\n"))
|
||||
}
|
||||
|
||||
// Everything before the first `## ` heading — an intro paragraph, or, if
|
||||
// the page has no headings at all, the entire body — is ordinary content,
|
||||
// not something this parser is entitled to drop (hyperguild#103 review:
|
||||
// dropping it meant Merge, one caller up, would silently replace the
|
||||
// whole on-disk body with the fresh extraction whenever a page didn't
|
||||
// follow the ## convention — exactly the class of bug this file exists
|
||||
// to prevent).
|
||||
var preambleLines []string
|
||||
for ; i < len(lines); i++ {
|
||||
if strings.HasPrefix(lines[i], "## ") {
|
||||
break
|
||||
}
|
||||
preambleLines = append(preambleLines, lines[i])
|
||||
}
|
||||
preamble = strings.Join(preambleLines, "\n")
|
||||
|
||||
var cur *section
|
||||
for ; i < len(lines); i++ {
|
||||
line := lines[i]
|
||||
@@ -96,24 +126,85 @@ func parseSections(markdown string) (frontmatter string, sections []section) {
|
||||
return
|
||||
}
|
||||
|
||||
func rebuildContent(frontmatter string, sections []section) string {
|
||||
// rebuildContent serialises sections back into a page.
|
||||
//
|
||||
// Each section body is normalised to exactly one trailing newline and no
|
||||
// leading blank line. Without that, a section keeps whatever blank lines it was
|
||||
// parsed with and this function adds its own, so every merge pass grows the page
|
||||
// by a blank line — invisible in a diff viewer, fatal to byte-stability. Compile-on-ingest
|
||||
// merges on every run, so "merging changes nothing when nothing changed" has to
|
||||
// hold at the byte level, not just the content level.
|
||||
func rebuildContent(frontmatter, preamble string, sections []section) string {
|
||||
var sb strings.Builder
|
||||
sb.WriteString(frontmatter)
|
||||
if body := strings.Trim(preamble, "\n"); body != "" {
|
||||
fmt.Fprintf(&sb, "\n%s\n", body)
|
||||
}
|
||||
for _, sec := range sections {
|
||||
fmt.Fprintf(&sb, "\n## %s\n\n%s", sec.heading, sec.content)
|
||||
body := strings.Trim(sec.content, "\n")
|
||||
fmt.Fprintf(&sb, "\n## %s\n\n%s\n", sec.heading, body)
|
||||
}
|
||||
return sb.String()
|
||||
}
|
||||
|
||||
// canonicalLine collapses a line to its comparison key: internal whitespace
|
||||
// runs (a re-extraction's bullet spacing, tabs vs spaces, trailing spaces)
|
||||
// must not make the same logical line look new. strings.Fields already
|
||||
// trims and splits on any whitespace run, so joining it back with single
|
||||
// spaces is exactly "same words, same order, whitespace-insensitive."
|
||||
func canonicalLine(line string) string {
|
||||
return strings.Join(strings.Fields(line), " ")
|
||||
}
|
||||
|
||||
// appendNew concatenates the lines of b that a does not already carry.
|
||||
//
|
||||
// Append sections used to concatenate wholesale, which is correct exactly once.
|
||||
// Compile-on-ingest merges a freshly extracted page into the one already on
|
||||
// disk, so an unchanged source re-ingested would grow "Open Questions" and
|
||||
// "Updates" on every run until the page is unreadable — and the drift guard
|
||||
// that would otherwise catch it (hashing the source body) does not exist yet,
|
||||
// so idempotency has to hold here rather than upstream.
|
||||
//
|
||||
// Line-level rather than block-level: the common case is a re-ingest carrying
|
||||
// one genuinely new line alongside several it already has. A block-level
|
||||
// "skip if identical" would duplicate the old lines every time that happened.
|
||||
//
|
||||
// The cost is that a line repeated verbatim on purpose collapses to one. For
|
||||
// these sections — questions, updates, evolving notes — a repeat is noise
|
||||
// rather than signal, and the failure is conservative: the page stops growing
|
||||
// instead of growing wrongly.
|
||||
func appendNew(a, b string) string {
|
||||
seen := make(map[string]bool)
|
||||
for _, line := range strings.Split(a, "\n") {
|
||||
if c := canonicalLine(line); c != "" {
|
||||
seen[c] = true
|
||||
}
|
||||
}
|
||||
|
||||
var add []string
|
||||
for _, line := range strings.Split(b, "\n") {
|
||||
c := canonicalLine(line)
|
||||
if c == "" || seen[c] {
|
||||
continue
|
||||
}
|
||||
seen[c] = true
|
||||
add = append(add, line)
|
||||
}
|
||||
if len(add) == 0 {
|
||||
return a // nothing new — leave the section byte-identical
|
||||
}
|
||||
return strings.TrimRight(a, "\n") + "\n\n" + strings.Join(add, "\n") + "\n"
|
||||
}
|
||||
|
||||
func mergeBullets(a, b string) string {
|
||||
seen := make(map[string]bool)
|
||||
var lines []string
|
||||
for _, line := range strings.Split(a+b, "\n") {
|
||||
trimmed := strings.TrimSpace(line)
|
||||
if trimmed == "" || seen[trimmed] {
|
||||
c := canonicalLine(line)
|
||||
if c == "" || seen[c] {
|
||||
continue
|
||||
}
|
||||
seen[trimmed] = true
|
||||
seen[c] = true
|
||||
lines = append(lines, line)
|
||||
}
|
||||
return strings.Join(lines, "\n") + "\n"
|
||||
|
||||
@@ -53,3 +53,124 @@ func TestMerge_KeepsFrontmatterFromA(t *testing.T) {
|
||||
assert.Contains(t, got.Content, "title: A")
|
||||
assert.NotContains(t, got.Content, "title: B")
|
||||
}
|
||||
|
||||
// --- hyperguild#103 review: a page with content outside any `## ` section ---
|
||||
// must never lose that content on merge. parseSections used to only capture
|
||||
// text between `## ` headings — anything before the first heading (a common,
|
||||
// ordinary shape for a hand-written note: an intro paragraph before the first
|
||||
// section) or, worse, an entire page with no `## ` headings at all (a legacy
|
||||
// note, a stub, any hand-edit that doesn't follow the section convention)
|
||||
// vanished silently. Merge would then fill the page entirely from the fresh
|
||||
// extraction — the opposite of "never silently rewrite what a human put
|
||||
// there," the guarantee this whole file exists to provide.
|
||||
|
||||
func TestMerge_PreservesOnDiskContentWithNoSectionHeadings(t *testing.T) {
|
||||
a := Page{Path: "wiki/x.md", Content: "---\ntitle: X\n---\n\nJust some free text a human wrote. No section headings at all.\n"}
|
||||
b := Page{Path: "wiki/x.md", Content: "---\ntitle: X\n---\n\n## Definition\n\nFresh extraction's take.\n"}
|
||||
|
||||
got := Merge(a, b)
|
||||
assert.Contains(t, got.Content, "Just some free text a human wrote",
|
||||
"on-disk content with no ## headings must survive the merge, not be replaced wholesale by the fresh extraction")
|
||||
}
|
||||
|
||||
func TestMerge_DegenerateEmptyHeadingDoesNotCollideWithPreamble(t *testing.T) {
|
||||
// Second review pass on #103: representing the preamble as a section with
|
||||
// heading=="" collides with a literal `## ` (marker, no title text) —
|
||||
// strings.TrimPrefix("## ", "## ") is also "". `a` has only a preamble
|
||||
// (no degenerate heading of its own); `b` introduces a genuinely new
|
||||
// degenerate `## ` section. Because both key to "", Merge's idx thinks
|
||||
// b's section "already exists" in a (it's really just a's unrelated
|
||||
// preamble) and never adds it — b's section vanishes from the output
|
||||
// entirely, not merged, not kept, not appended.
|
||||
a := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\nAn intro paragraph.\n\n## Definition\n\nA thing.\n"}
|
||||
b := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\n## \n\nA genuinely new degenerate section's body.\n\n## Definition\n\nA thing.\n"}
|
||||
|
||||
got := Merge(a, b)
|
||||
assert.Contains(t, got.Content, "An intro paragraph.", "a's own preamble must survive")
|
||||
assert.Contains(t, got.Content, "A genuinely new degenerate section's body.",
|
||||
"b's degenerate section is new content, not a collision with a's unrelated preamble — it must not be silently dropped")
|
||||
}
|
||||
|
||||
func TestMerge_PreservesPreambleBeforeFirstHeading(t *testing.T) {
|
||||
a := Page{Path: "wiki/x.md", Content: "---\ntitle: X\n---\n\nAn intro paragraph before the first heading.\n\n## Definition\n\nA thing.\n"}
|
||||
b := Page{Path: "wiki/x.md", Content: "---\ntitle: X\n---\n\n## Definition\n\nA thing.\n"}
|
||||
|
||||
got := Merge(a, b)
|
||||
assert.Contains(t, got.Content, "An intro paragraph before the first heading",
|
||||
"text before the first ## heading is ordinary page content, not something the parser is entitled to drop")
|
||||
}
|
||||
|
||||
// --- hyperguild#103 review: dedup must be whitespace-insensitive -----------
|
||||
// Page content is produced by an LLM per run, so byte-identical re-extraction
|
||||
// of "the same" logical line is not guaranteed — different bullet spacing or
|
||||
// trailing whitespace on an otherwise-identical line must still dedup, or
|
||||
// idempotency only holds when the extractor happens to reproduce whitespace
|
||||
// exactly, which is not a real guarantee.
|
||||
|
||||
func TestMerge_AppendSectionDedupsAcrossWhitespaceVariants(t *testing.T) {
|
||||
a := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\n## Open Questions\n\n- Does this converge?\n"}
|
||||
b := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\n## Open Questions\n\n- Does this converge? \n"}
|
||||
|
||||
got := Merge(a, b)
|
||||
assert.Equal(t, 1, strings.Count(got.Content, "Does this converge?"),
|
||||
"same logical line with different whitespace must dedup, not duplicate")
|
||||
}
|
||||
|
||||
func TestMerge_BulletSectionDedupsAcrossWhitespaceVariants(t *testing.T) {
|
||||
a := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\n## Related Concepts\n\n- [[bar|Bar]]\n"}
|
||||
b := Page{Path: "p.md", Content: "---\ntitle: X\n---\n\n## Related Concepts\n\n- [[bar|Bar]]\n"}
|
||||
|
||||
got := Merge(a, b)
|
||||
assert.Equal(t, 1, strings.Count(got.Content, "[[bar|Bar]]"),
|
||||
"same logical bullet with different whitespace must dedup, not duplicate")
|
||||
}
|
||||
|
||||
// --- brain#21/B1: merging must be safe to run again -------------------------
|
||||
|
||||
func TestMerge_AppendSectionDoesNotDuplicateContentAlreadyPresent(t *testing.T) {
|
||||
// Compile-on-ingest merges a freshly extracted page into the one already
|
||||
// on disk. Bullet sections union, so they are naturally idempotent — but
|
||||
// append sections concatenate, so re-ingesting an unchanged source would
|
||||
// grow "Open Questions" on every run until the page is unreadable.
|
||||
page := Page{Path: "wiki/sources/x.md", Content: `---
|
||||
title: X
|
||||
---
|
||||
|
||||
## Open Questions
|
||||
|
||||
- Does this converge?
|
||||
`}
|
||||
|
||||
once := Merge(page, page)
|
||||
twice := Merge(once, page)
|
||||
|
||||
assert.Equal(t, 1, strings.Count(once.Content, "Does this converge?"),
|
||||
"merging a page with itself must not duplicate an append section")
|
||||
assert.Equal(t, once.Content, twice.Content,
|
||||
"merge must be idempotent — a third pass may not keep growing the page")
|
||||
}
|
||||
|
||||
func TestMerge_AppendSectionStillAccumulatesGenuinelyNewContent(t *testing.T) {
|
||||
// The dedup must not turn append sections into keep-a-only sections:
|
||||
// new material is exactly what compile-on-ingest exists to add.
|
||||
existing := Page{Path: "wiki/sources/x.md", Content: `---
|
||||
title: X
|
||||
---
|
||||
|
||||
## Open Questions
|
||||
|
||||
- Does this converge?
|
||||
`}
|
||||
fresh := Page{Path: "wiki/sources/x.md", Content: `---
|
||||
title: X
|
||||
---
|
||||
|
||||
## Open Questions
|
||||
|
||||
- What happens under load?
|
||||
`}
|
||||
|
||||
got := Merge(existing, fresh).Content
|
||||
assert.Contains(t, got, "Does this converge?", "existing question must survive")
|
||||
assert.Contains(t, got, "What happens under load?", "new question must be added")
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user