From 8342de5c49e9d2c4e5f0a2f388f342b44beabb36 Mon Sep 17 00:00:00 2001 From: Mathias Date: Tue, 20 Oct 2026 20:05:29 +0200 Subject: [PATCH 01/14] =?UTF-8?q?feat(wing):=20Router=20=E2=80=94=20rules-?= =?UTF-8?q?only=20placement:=20deliberate=20subfolder,=20else=20default=20?= =?UTF-8?q?wing=20(#208)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No model. A subfolder/frontmatter wing must be a plain name of an existing wing and then places; conflicting signals and client-marker text park; everything else goes to the default wing. Wings are read from disk per decision. Mutation-checked. Co-Authored-By: Claude Sonnet 5.5 --- ingestion/internal/wing/resolve.go | 1 + ingestion/internal/wing/router.go | 70 +++++++++++++++++++++ ingestion/internal/wing/router_test.go | 87 ++++++++++++++++++++++++++ 3 files changed, 158 insertions(+) create mode 100644 ingestion/internal/wing/router.go create mode 100644 ingestion/internal/wing/router_test.go diff --git a/ingestion/internal/wing/resolve.go b/ingestion/internal/wing/resolve.go index 646ed19..f109f07 100644 --- a/ingestion/internal/wing/resolve.go +++ b/ingestion/internal/wing/resolve.go @@ -20,6 +20,7 @@ type Source string const ( Explicit Source = "explicit" // subfolder or frontmatter named the wing + Default Source = "default" // no signal; the configured default wing Classified Source = "classified" // classifier + scorer agreed, confident Exception Source = "exception" // needs a human ) diff --git a/ingestion/internal/wing/router.go b/ingestion/internal/wing/router.go new file mode 100644 index 0000000..1ea5001 --- /dev/null +++ b/ingestion/internal/wing/router.go @@ -0,0 +1,70 @@ +package wing + +import ( + "context" + "fmt" + "regexp" + "slices" + "strings" +) + +// Placer decides where a raw note goes. Router is the production +// implementation; the interface lets the watcher stay ignorant of how. +type Placer interface { + Resolve(ctx context.Context, n Note) Decision +} + +// Router places a note with rules only, no model (hyperguild#208): +// +// 1. A subfolder or frontmatter `wing:` is the owner's deliberate choice. It +// must name a plain, EXISTING wing (a typo never creates one) and then it +// places, whatever the wing is: extraction runs on the local model, and it +// is the owner who dropped the note there. Two signals that disagree park. +// 2. With no signal, a note whose text matches ClientBlock is parked; any +// other note goes to Default, the one wing that is created on first use. +// +// Wings are read from disk on every decision, so a wing created while the +// server runs is usable at once. +type Router struct { + BrainDir string + Default string + ClientBlock *regexp.Regexp // nil = no content check +} + +// Resolve implements Placer. It never returns an error: anything it cannot +// decide safely is an Exception with the reason spelled out. +func (r Router) Resolve(_ context.Context, n Note) Decision { + dir, fm := norm(n.Dir), norm(n.FrontmatterWing) + switch { + case dir != "" && fm != "" && dir != fm: + return exception("", 0, fmt.Sprintf("subfolder %q and frontmatter wing %q disagree", dir, fm)) + case dir != "": + return r.explicit(dir, "subfolder") + case fm != "": + return r.explicit(fm, "frontmatter") + } + if r.ClientBlock != nil && r.ClientBlock.MatchString(n.Text) { + return exception("", 0, "text matches the client-content marker; not defaulted into a shared wing") + } + if r.Default == "" || !plainWingName(r.Default) { + return exception("", 0, "no valid default wing configured") + } + return Decision{Wing: r.Default, Source: Default, Confidence: 1, Reason: "no wing named; default wing"} +} + +func (r Router) explicit(w, via string) Decision { + if !plainWingName(w) { + return exception("", 0, fmt.Sprintf("%s names %q, which is not a plain wing name", via, w)) + } + if w == r.Default && plainWingName(w) { + return Decision{Wing: w, Source: Explicit, Confidence: 1, Reason: "named by " + via} + } + known, err := KnownWings(r.BrainDir) + if err != nil { + return exception(w, 0, fmt.Sprintf("cannot list wings: %v", err)) + } + if !slices.Contains(known, w) { + return exception(w, 0, fmt.Sprintf("%s names %q: not an existing wing (new wings are never created from a folder name)", via, strings.TrimSpace(w))) + } + return Decision{Wing: w, Source: Explicit, Confidence: 1, Reason: "named by " + via} +} diff --git a/ingestion/internal/wing/router_test.go b/ingestion/internal/wing/router_test.go new file mode 100644 index 0000000..6c2cd63 --- /dev/null +++ b/ingestion/internal/wing/router_test.go @@ -0,0 +1,87 @@ +package wing_test + +import ( + "context" + "regexp" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/mathiasbq/hyperguild/ingestion/internal/wing" +) + +func routerBrain(t *testing.T) string { + dir := t.TempDir() + writePage(t, dir, "wiki/homelab/facts/a.md", "x") + writePage(t, dir, "wiki/rikatillsammans/facts/b.md", "x") + return dir +} + +func TestRouter_Decisions(t *testing.T) { + tests := []struct { + name string + note wing.Note + want wing.Source + wantWing string + }{ + {"no signal goes to the default wing", wing.Note{Text: "a talk about agents"}, wing.Default, "web-clip"}, + {"a subfolder naming an existing wing places there", wing.Note{Dir: "homelab", Text: "x"}, wing.Explicit, "homelab"}, + {"a deliberate subfolder into a client wing is the owner's call and is placed", wing.Note{Dir: "rikatillsammans", Text: "x"}, wing.Explicit, "rikatillsammans"}, + {"frontmatter wing places like a subfolder", wing.Note{FrontmatterWing: "homelab", Text: "x"}, wing.Explicit, "homelab"}, + {"subfolder is case-insensitive", wing.Note{Dir: "HomeLab", Text: "x"}, wing.Explicit, "homelab"}, + {"a typo never creates a wing", wing.Note{Dir: "hoemlab", Text: "x"}, wing.Exception, "hoemlab"}, + {"conflicting signals are parked", wing.Note{Dir: "homelab", FrontmatterWing: "rikatillsammans", Text: "x"}, wing.Exception, ""}, + {"the default wing may be named explicitly even before it exists", wing.Note{Dir: "web-clip", Text: "x"}, wing.Explicit, "web-clip"}, + {"client marker with no signal is parked, not defaulted", wing.Note{Text: "notes from the ACME-CLIENT kickoff"}, wing.Exception, ""}, + {"client marker does not block a deliberate subfolder", wing.Note{Dir: "homelab", Text: "ACME-CLIENT"}, wing.Explicit, "homelab"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := wing.Router{BrainDir: routerBrain(t), Default: "web-clip", ClientBlock: regexp.MustCompile(`(?i)acme-client`)} + d := r.Resolve(context.Background(), tt.note) + assert.Equal(t, tt.want, d.Source) + assert.Equal(t, tt.wantWing, d.Wing) + assert.NotEmpty(t, d.Reason) + }) + } +} + +func TestRouter_ReadsTheWingsFreshOnEveryDecision(t *testing.T) { + dir := routerBrain(t) + r := wing.Router{BrainDir: dir, Default: "web-clip"} + assert.Equal(t, wing.Exception, r.Resolve(context.Background(), wing.Note{Dir: "newwing", Text: "x"}).Source) + + writePage(t, dir, "wiki/newwing/facts/c.md", "x") // a wing created after startup + assert.Equal(t, wing.Explicit, r.Resolve(context.Background(), wing.Note{Dir: "newwing", Text: "x"}).Source, + "a wing made while the server runs is usable without a restart") +} + +func TestRouter_AnUnreadableBrainParksInsteadOfGuessing(t *testing.T) { + r := wing.Router{BrainDir: t.TempDir() + "/missing", Default: "web-clip"} + d := r.Resolve(context.Background(), wing.Note{Dir: "homelab", Text: "x"}) + assert.Equal(t, wing.Exception, d.Source) +} + +func TestRouter_WithoutADefaultAnUnsignalledNoteIsParked(t *testing.T) { + r := wing.Router{BrainDir: routerBrain(t)} + assert.Equal(t, wing.Exception, r.Resolve(context.Background(), wing.Note{Text: "x"}).Source) +} + +func TestRouter_ADefaultThatIsNotAPlainNameIsRefused(t *testing.T) { + r := wing.Router{BrainDir: routerBrain(t), Default: "../escape"} + assert.Equal(t, wing.Exception, r.Resolve(context.Background(), wing.Note{Text: "x"}).Source) +} + +func TestRouter_ANameThatIsNotPlainIsRefusedAsSuch(t *testing.T) { + for _, bad := range []string{"../x", "a/b", "has space", "dot.ted"} { + t.Run(bad, func(t *testing.T) { + // Default equals the bad name: the "named explicitly" shortcut for the + // default wing must not wave a malformed name through. + r := wing.Router{BrainDir: routerBrain(t), Default: bad} + d := r.Resolve(context.Background(), wing.Note{Dir: bad, Text: "x"}) + assert.Equal(t, wing.Exception, d.Source) + assert.Empty(t, d.Wing, "a malformed name is never echoed as a proposal") + assert.Contains(t, d.Reason, "not a plain wing name") + }) + } +} -- 2.54.0 From ce5a5268e716e305cb124205ad197581e8b043c7 Mon Sep 17 00:00:00 2001 From: Mathias Date: Tue, 20 Oct 2026 20:08:30 +0200 Subject: [PATCH 02/14] feat(pipeline): per-wing sources, shared concepts/entities, one slug one page (#208) With Config.Wing a source goes to wiki//sources/ (wing/hall frontmatter); concepts and entities stay in the shared wiki/concepts and wiki/entities, so an entity cited from two wings is ONE page with both sources (the per-wing facts/ layout duplicated it). A source slug already used anywhere but its own wing's sources/ is refused (slug is the graph identity). wiki.FindSlug added. Mutation-checked: collision guard, shared inventory, per-wing source path. Co-Authored-By: Claude Sonnet 5.5 --- ingestion/internal/pipeline/build.go | 35 ++--- ingestion/internal/pipeline/pipeline.go | 7 + ingestion/internal/pipeline/resolve.go | 13 +- .../internal/pipeline/wing_layout_test.go | 112 ++++++++-------- ingestion/internal/wiki/findslug_test.go | 52 ++++++++ ingestion/internal/wiki/inventory.go | 123 ++++++++++-------- 6 files changed, 205 insertions(+), 137 deletions(-) create mode 100644 ingestion/internal/wiki/findslug_test.go diff --git a/ingestion/internal/pipeline/build.go b/ingestion/internal/pipeline/build.go index c45fdac..5479db5 100644 --- a/ingestion/internal/pipeline/build.go +++ b/ingestion/internal/pipeline/build.go @@ -17,9 +17,11 @@ func BuildPages(rawPages []RawPage, sourceSlug, date string) ([]wiki.Page, []str return BuildPagesIn("", rawPages, sourceSlug, date) } -// BuildPagesIn is BuildPages for a wing: sources go to wiki//sources/, -// concepts and entities to wiki//facts/ with `kind:` recording which. -// An empty wing is the retired wiki/{sources,concepts,entities}/ layout. +// BuildPagesIn is BuildPages for a wing: a source goes to wiki//sources/ +// with wing/hall frontmatter. Concepts and entities stay in the shared +// wiki/concepts/ and wiki/entities/, since their slug is the graph's global +// identity and they are cited from many wings. An empty wing is the retired +// layout for sources too. func BuildPagesIn(wing string, rawPages []RawPage, sourceSlug, date string) ([]wiki.Page, []string) { out := make([]wiki.Page, 0, len(rawPages)) var warnings []string @@ -66,20 +68,13 @@ func buildPage(rp RawPage, sourceSlug, date, wing string) wiki.Page { dir = "wiki/" + rp.Type } - hall := "" - if wing != "" { - switch rp.Type { - case "source": - hall = "sources" - default: - hall = "facts" - } - dir = "wiki/" + wing + "/" + hall + if wing != "" && rp.Type == "source" { + dir = "wiki/" + wing + "/sources" } path := dir + "/" + slug + ".md" fm := buildFrontmatter(rp, date) - if wing != "" { - fm = addWingFrontmatter(fm, wing, hall, rp.Type) + if wing != "" && rp.Type == "source" { + fm = addWingFrontmatter(fm, wing, "sources") } return wiki.Page{ @@ -142,14 +137,10 @@ func buildFrontmatter(rp RawPage, date string) string { return sb.String() } -// addWingFrontmatter inserts wing/hall (and kind for facts pages) right after -// the opening fence of an already-built frontmatter block. -func addWingFrontmatter(fm, wing, hall, rpType string) string { - extra := "wing: " + wing + "\nhall: " + hall + "\n" - if hall == "facts" { - extra += "kind: " + rpType + "\n" - } - return strings.Replace(fm, "---\n", "---\n"+extra, 1) +// addWingFrontmatter inserts wing/hall right after the opening fence of an +// already-built frontmatter block. +func addWingFrontmatter(fm, wing, hall string) string { + return strings.Replace(fm, "---\n", "---\nwing: "+wing+"\nhall: "+hall+"\n", 1) } func yamlScalar(s string) string { diff --git a/ingestion/internal/pipeline/pipeline.go b/ingestion/internal/pipeline/pipeline.go index 9269f3a..cd4ec03 100644 --- a/ingestion/internal/pipeline/pipeline.go +++ b/ingestion/internal/pipeline/pipeline.go @@ -52,6 +52,13 @@ func Run(ctx context.Context, cfg Config, brainDir, content, source string, dryR var inventory map[wiki.PageType][]wiki.Entry var err error if cfg.Wing != "" { + // A source slug is a page's graph identity: refuse to take over a slug + // that lives anywhere but this wing's own sources/. + if existing, found, ferr := wiki.FindSlug(brainDir, wiki.Slug(source)); ferr != nil { + return Result{}, fmt.Errorf("check source slug: %w", ferr) + } else if own := "wiki/" + cfg.Wing + "/sources/" + wiki.Slug(source) + ".md"; found && existing != own { + return Result{}, fmt.Errorf("source slug %q is already used by %s", wiki.Slug(source), existing) + } inventory, err = wiki.LoadWingInventory(brainDir, cfg.Wing) } else { inventory, err = wiki.LoadInventory(brainDir) diff --git a/ingestion/internal/pipeline/resolve.go b/ingestion/internal/pipeline/resolve.go index e098ca1..21e04e3 100644 --- a/ingestion/internal/pipeline/resolve.go +++ b/ingestion/internal/pipeline/resolve.go @@ -56,17 +56,12 @@ func normalizeTitle(s string) string { } // pageTypeOf is the page's type: from its directory in the retired layout, or -// from `kind:` for a wing-layout facts page (concepts and entities share facts/). +// "sources" for wiki//sources/.md. Concepts and entities are always +// in their shared directories. func pageTypeOf(p wiki.Page) wiki.PageType { parts := strings.Split(filepath.ToSlash(p.Path), "/") - if len(parts) >= 4 && parts[0] == "wiki" { - switch parts[2] { - case "sources": - return wiki.PageTypeSource - case "facts": - pt, _ := wiki.KindFromContent(p.Content) - return pt - } + if len(parts) == 4 && parts[0] == "wiki" && parts[2] == "sources" { + return wiki.PageTypeSource } return pageTypeFromPath(p.Path) } diff --git a/ingestion/internal/pipeline/wing_layout_test.go b/ingestion/internal/pipeline/wing_layout_test.go index 0da280b..5f5b240 100644 --- a/ingestion/internal/pipeline/wing_layout_test.go +++ b/ingestion/internal/pipeline/wing_layout_test.go @@ -20,12 +20,14 @@ func wingCfg(wing string, pages ...RawPage) Config { } } -func sourceAndEntity() []RawPage { +func sourceEntityConcept() []RawPage { return []RawPage{ {Title: "Dark Factory Talk", Type: "source", Subtype: "video", Domain: "ai", - Content: "## Summary\n\nA talk.\n\n## Entities Mentioned\n\n[[Jev]]\n"}, + Content: "## Summary\n\nA talk.\n\n## Entities Mentioned\n\n[[Jev]]\n\n## Concepts Introduced or Reinforced\n\n[[Dark Factory]]\n"}, {Title: "Jev", Type: "entity", Subtype: "tool", Domain: "ai", Content: "## Definition\n\nA decision model.\n\n## Sources\n"}, + {Title: "Dark Factory", Type: "concept", Domain: "ai", + Content: "## Definition\n\nA fully automated build line.\n\n## Sources\n"}, } } @@ -41,73 +43,91 @@ func read(t *testing.T, dir, rel string) string { return string(b) } -func TestRun_WithAWingWritesIntoThatWingsHalls(t *testing.T) { +// With a wing, the SOURCE belongs to the wing; concepts and entities are the +// shared cross-wing layer (wiki/concepts, wiki/entities) because their slug is +// the graph's global identity and they are cited from many wings. +func TestRun_WithAWingTheSourceIsPerWingAndConceptsEntitiesAreShared(t *testing.T) { dir := t.TempDir() - _, err := Run(context.Background(), wingCfg("radar", sourceAndEntity()...), dir, "body of the talk", "dark-factory-talk", false) + _, err := Run(context.Background(), wingCfg("web-clip", sourceEntityConcept()...), dir, "body of the talk", "dark-factory-talk", false) require.NoError(t, err) - src := read(t, dir, "wiki/radar/sources/dark-factory-talk.md") - assert.Contains(t, src, "wing: radar") + src := read(t, dir, "wiki/web-clip/sources/dark-factory-talk.md") + assert.Contains(t, src, "wing: web-clip") assert.Contains(t, src, "hall: sources") - - ent := read(t, dir, "wiki/radar/facts/jev.md") - assert.Contains(t, ent, "wing: radar") - assert.Contains(t, ent, "hall: facts") - assert.Contains(t, ent, "kind: entity") + assert.True(t, exists(dir, "wiki/entities/jev.md")) + assert.True(t, exists(dir, "wiki/concepts/dark-factory.md")) + assert.NotContains(t, read(t, dir, "wiki/entities/jev.md"), "wing:", "a shared page belongs to no single wing") + assert.False(t, exists(dir, "wiki/web-clip/facts"), "no per-wing copy of a shared page") + assert.False(t, exists(dir, "wiki/sources"), "the retired source layout is not used") } -func TestRun_WithAWingDoesNotTouchTheRetiredLayout(t *testing.T) { +func TestRun_WithAWingTheSharedPageLinksBackToTheSource(t *testing.T) { dir := t.TempDir() - _, err := Run(context.Background(), wingCfg("radar", sourceAndEntity()...), dir, "body of the talk", "dark-factory-talk", false) + _, err := Run(context.Background(), wingCfg("web-clip", sourceEntityConcept()...), dir, "body", "dark-factory-talk", false) require.NoError(t, err) - for _, retired := range []string{"wiki/sources", "wiki/concepts", "wiki/entities"} { - assert.False(t, exists(dir, retired), "%s is the retired layout and must not be written", retired) - } + assert.Contains(t, read(t, dir, "wiki/entities/jev.md"), "[[dark-factory-talk|Dark Factory Talk]]") } -func TestRun_WithAWingTheSourceLinksBackFromTheEntity(t *testing.T) { +func TestRun_TheSameEntityCitedFromTwoWingsIsOnePageWithBothSources(t *testing.T) { dir := t.TempDir() - _, err := Run(context.Background(), wingCfg("radar", sourceAndEntity()...), dir, "body of the talk", "dark-factory-talk", false) - require.NoError(t, err) - assert.Contains(t, read(t, dir, "wiki/radar/facts/jev.md"), "[[dark-factory-talk|Dark Factory Talk]]") -} - -func TestRun_WithAWingASecondSourceReusesTheSameEntityPage(t *testing.T) { - dir := t.TempDir() - _, err := Run(context.Background(), wingCfg("radar", sourceAndEntity()...), dir, "first body", "talk-one", false) + _, err := Run(context.Background(), wingCfg("web-clip", sourceEntityConcept()...), dir, "first body", "talk-one", false) require.NoError(t, err) + // A wing must already exist to be named; create it the way a note would. + require.NoError(t, os.MkdirAll(filepath.Join(dir, "wiki/homelab/facts"), 0o755)) again := []RawPage{ {Title: "Second Talk", Type: "source", Subtype: "video", Content: "## Summary\n\nMore.\n\n## Entities Mentioned\n\n[[The Jev]]\n"}, {Title: "The Jev", Type: "entity", Subtype: "tool", Content: "## Definition\n\nStill a decision model.\n\n## Sources\n"}, } - _, err = Run(context.Background(), wingCfg("radar", again...), dir, "second body", "talk-two", false) + _, err = Run(context.Background(), wingCfg("homelab", again...), dir, "second body", "talk-two", false) require.NoError(t, err) - files, err := filepath.Glob(filepath.Join(dir, "wiki/radar/facts/*.md")) + files, err := filepath.Glob(filepath.Join(dir, "wiki/entities/*.md")) require.NoError(t, err) - assert.Len(t, files, 1, "an entity matched by title (leading article ignored) is merged, not duplicated") - assert.Contains(t, read(t, dir, "wiki/radar/facts/jev.md"), "talk-two") + assert.Len(t, files, 1, "one entity page, matched by title with the leading article ignored") + jev := read(t, dir, "wiki/entities/jev.md") + assert.Contains(t, jev, "talk-one") + assert.Contains(t, jev, "talk-two") + assert.False(t, exists(dir, "wiki/homelab/facts/jev.md"), "no second copy in the second wing") } -func TestRun_WithAWingNeverMergesIntoAnotherWingsPage(t *testing.T) { +// A source slug is the graph's identity too: a second wing must not take over +// the slug of a source that lives elsewhere. +func TestRun_ASourceSlugAlreadyUsedInAnotherWingIsRefused(t *testing.T) { dir := t.TempDir() - other := "---\ntitle: Jev\nwing: client-x\nhall: facts\nkind: entity\n---\n\n## Definition\n\nPrivate to another wing.\n" - require.NoError(t, os.MkdirAll(filepath.Join(dir, "wiki/client-x/facts"), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(dir, "wiki/client-x/facts/jev.md"), []byte(other), 0o644)) - - _, err := Run(context.Background(), wingCfg("radar", sourceAndEntity()...), dir, "body", "dark-factory-talk", false) + _, err := Run(context.Background(), wingCfg("web-clip", sourceEntityConcept()...), dir, "body", "talk", false) require.NoError(t, err) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "wiki/homelab/facts"), 0o755)) - assert.Equal(t, other, read(t, dir, "wiki/client-x/facts/jev.md"), "another wing's page is never read into or rewritten") - assert.True(t, exists(dir, "wiki/radar/facts/jev.md")) + _, err = Run(context.Background(), wingCfg("homelab", sourceEntityConcept()...), dir, "different body", "talk", false) + require.Error(t, err) + assert.Contains(t, err.Error(), "already used") + assert.False(t, exists(dir, "wiki/homelab/sources/talk.md")) +} + +func TestRun_ASourceSlugAlreadyInTheRetiredLayoutIsRefused(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "wiki/sources"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "wiki/sources/talk.md"), []byte("---\ntitle: Talk\n---\nold"), 0o644)) + + _, err := Run(context.Background(), wingCfg("web-clip", sourceEntityConcept()...), dir, "body", "talk", false) + require.Error(t, err) + assert.Contains(t, err.Error(), "already used") +} + +func TestRun_ReIngestingTheSameSourceIntoItsOwnWingIsNotACollision(t *testing.T) { + dir := t.TempDir() + _, err := Run(context.Background(), wingCfg("web-clip", sourceEntityConcept()...), dir, "body one", "talk", false) + require.NoError(t, err) + _, err = Run(context.Background(), wingCfg("web-clip", sourceEntityConcept()...), dir, "body two, edited", "talk", false) + require.NoError(t, err) } func TestRun_WithAWingRejectsANameThatIsNotAPlainWing(t *testing.T) { for _, bad := range []string{"../escape", "a/b", "Radar", "has space", "."} { t.Run(bad, func(t *testing.T) { dir := t.TempDir() - _, err := Run(context.Background(), wingCfg(bad, sourceAndEntity()...), dir, "body", "talk", false) + _, err := Run(context.Background(), wingCfg(bad, sourceEntityConcept()...), dir, "body", "talk", false) require.Error(t, err) assert.Len(t, listAll(t, dir), 1, "nothing is written for a rejected wing (only the temp dir itself exists)") }) @@ -120,19 +140,3 @@ func listAll(t *testing.T, dir string) []string { _ = filepath.WalkDir(dir, func(p string, _ os.DirEntry, _ error) error { out = append(out, p); return nil }) return out } - -// The dangerous path is the back-reference: a source that links [[Jev]] without -// proposing a Jev page makes the pipeline go and edit an existing Jev page. That -// lookup must be confined to the target wing. -func TestRun_WithAWingASourceLinkingToAnotherWingsEntityDoesNotEditIt(t *testing.T) { - dir := t.TempDir() - other := "---\ntitle: Jev\nwing: client-x\nhall: facts\nkind: entity\n---\n\n## Definition\n\nPrivate to another wing.\n\n## Sources\n" - require.NoError(t, os.MkdirAll(filepath.Join(dir, "wiki/client-x/facts"), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(dir, "wiki/client-x/facts/jev.md"), []byte(other), 0o644)) - - onlySource := sourceAndEntity()[:1] - _, err := Run(context.Background(), wingCfg("radar", onlySource...), dir, "body", "dark-factory-talk", false) - require.NoError(t, err) - - assert.Equal(t, other, read(t, dir, "wiki/client-x/facts/jev.md"), "no back-reference may be written into another wing") -} diff --git a/ingestion/internal/wiki/findslug_test.go b/ingestion/internal/wiki/findslug_test.go new file mode 100644 index 0000000..bb89795 --- /dev/null +++ b/ingestion/internal/wiki/findslug_test.go @@ -0,0 +1,52 @@ +package wiki + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func put(t *testing.T, dir, rel string) { + t.Helper() + full := filepath.Join(dir, rel) + require.NoError(t, os.MkdirAll(filepath.Dir(full), 0o755)) + require.NoError(t, os.WriteFile(full, []byte("---\ntitle: x\n---\n"), 0o644)) +} + +func TestFindSlug(t *testing.T) { + dir := t.TempDir() + put(t, dir, "wiki/sources/legacy-src.md") + put(t, dir, "wiki/concepts/a-concept.md") + put(t, dir, "wiki/entities/an-entity.md") + put(t, dir, "wiki/homelab/failures/in-a-hall.md") + put(t, dir, "wiki/web-clip/sources/a-clip.md") + + tests := []struct { + slug string + want string + }{ + {"legacy-src", "wiki/sources/legacy-src.md"}, + {"a-concept", "wiki/concepts/a-concept.md"}, + {"an-entity", "wiki/entities/an-entity.md"}, + {"in-a-hall", "wiki/homelab/failures/in-a-hall.md"}, + {"a-clip", "wiki/web-clip/sources/a-clip.md"}, + {"nothing-here", ""}, + } + for _, tt := range tests { + t.Run(tt.slug, func(t *testing.T) { + got, found, err := FindSlug(dir, tt.slug) + require.NoError(t, err) + assert.Equal(t, tt.want != "", found) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestFindSlug_NoWikiDirIsNotAFinding(t *testing.T) { + _, found, err := FindSlug(t.TempDir(), "x") + require.NoError(t, err) + assert.False(t, found) +} diff --git a/ingestion/internal/wiki/inventory.go b/ingestion/internal/wiki/inventory.go index c9e3d4e..b87f5f8 100644 --- a/ingestion/internal/wiki/inventory.go +++ b/ingestion/internal/wiki/inventory.go @@ -97,78 +97,97 @@ func readFrontmatter(bfs *brainfs.Brain, rel, fallbackSlug string) (title string return } -// LoadWingInventory returns the pages of ONE wing, grouped by type. Sources -// come from wiki//sources/; concepts and entities live together in -// wiki//facts/ and are told apart by their `kind:` frontmatter. A facts -// page without a kind (a hand-written note) is left out, so extraction can -// never merge into something it did not create. Other wings are never read. +// LoadWingInventory returns what an ingest into one wing can match against: +// that wing's sources (wiki//sources/) and the SHARED concepts and +// entities (wiki/concepts/, wiki/entities/), which belong to no single wing +// because their slug is the graph's global identity. Other wings' sources are +// not read. func LoadWingInventory(brainDir, wing string) (map[PageType][]Entry, error) { - result := map[PageType][]Entry{PageTypeConcept: {}, PageTypeEntity: {}, PageTypeSource: {}} + result, err := LoadInventory(brainDir) + if err != nil { + return nil, err + } + result[PageTypeSource] = []Entry{} // the retired source layout is not this wing's bfs, err := brainfs.Open(brainDir) if err != nil { return nil, fmt.Errorf("open brain: %w", err) } defer func() { _ = bfs.Close() }() - for _, hall := range []string{"sources", "facts"} { - dir := "wiki/" + wing + "/" + hall - names, err := bfs.ReadDirRegular(dir) - if errors.Is(err, fs.ErrNotExist) { + dir := "wiki/" + wing + "/sources" + names, err := bfs.ReadDirRegular(dir) + if errors.Is(err, fs.ErrNotExist) { + return result, nil + } + if err != nil { + return nil, fmt.Errorf("read dir %s: %w", dir, err) + } + for _, name := range names { + if !strings.HasSuffix(name, ".md") { continue } - if err != nil { - return nil, fmt.Errorf("read dir %s: %w", dir, err) - } - for _, name := range names { - if !strings.HasSuffix(name, ".md") { - continue - } - rel := dir + "/" + name - slug := strings.TrimSuffix(name, ".md") - pt := PageTypeSource - if hall == "facts" { - var ok bool - if pt, ok = kindOf(bfs, rel); !ok { - continue - } - } - title, aliases := readFrontmatter(bfs, rel, slug) - result[pt] = append(result[pt], Entry{Slug: slug, Title: title, Aliases: aliases, Type: pt, Path: rel}) - } + rel := dir + "/" + name + slug := strings.TrimSuffix(name, ".md") + title, aliases := readFrontmatter(bfs, rel, slug) + result[PageTypeSource] = append(result[PageTypeSource], Entry{Slug: slug, Title: title, Aliases: aliases, Type: PageTypeSource, Path: rel}) } return result, nil } -// kindOf reads `kind:` from a facts page's frontmatter. -func kindOf(bfs *brainfs.Brain, rel string) (PageType, bool) { - content, err := bfs.ReadRegular(rel) +// FindSlug returns the brain-relative path of an existing page whose basename is +// slug, searching the retired dirs (wiki/{sources,concepts,entities}/) and every +// wing hall (wiki///). The slug is a page's graph identity, so two +// pages must not share one. +func FindSlug(brainDir, slug string) (string, bool, error) { + bfs, err := brainfs.Open(brainDir) if err != nil { - return "", false + return "", false, fmt.Errorf("open brain: %w", err) } - return KindFromContent(string(content)) -} - -// KindFromContent maps frontmatter `kind: concept|entity` to a PageType. -func KindFromContent(content string) (PageType, bool) { - inFM := false - for _, line := range strings.Split(content, "\n") { - if strings.TrimSpace(line) == "---" { - if inFM { - break + defer func() { _ = bfs.Close() }() + file := slug + ".md" + has := func(dir string) (bool, error) { + names, err := bfs.ReadDirRegular(dir) + if errors.Is(err, fs.ErrNotExist) { + return false, nil + } + if err != nil { + return false, err + } + for _, n := range names { + if n == file { + return true, nil } - inFM = true + } + return false, nil + } + for _, d := range []string{"wiki/sources", "wiki/concepts", "wiki/entities"} { + if ok, err := has(d); err != nil { + return "", false, err + } else if ok { + return d + "/" + file, true, nil + } + } + wings, err := bfs.ReadDirDirs("wiki") + if errors.Is(err, fs.ErrNotExist) { + return "", false, nil + } + if err != nil { + return "", false, err + } + for _, w := range wings { + if w == "sources" || w == "concepts" || w == "entities" { continue } - if !inFM { + halls, err := bfs.ReadDirDirs("wiki/" + w) + if err != nil { continue } - if key, val, ok := strings.Cut(line, ":"); ok && strings.TrimSpace(key) == "kind" { - switch strings.Trim(strings.TrimSpace(val), `"'`) { - case "concept": - return PageTypeConcept, true - case "entity": - return PageTypeEntity, true + for _, h := range halls { + if ok, err := has("wiki/" + w + "/" + h); err != nil { + return "", false, err + } else if ok { + return "wiki/" + w + "/" + h + "/" + file, true, nil } } } - return "", false + return "", false, nil } -- 2.54.0 From 750dc93bdb6409687b5a62e7467ebda6c64abf79 Mon Sep 17 00:00:00 2001 From: Mathias Date: Tue, 20 Oct 2026 20:09:19 +0200 Subject: [PATCH 03/14] feat(watcher): Placer interface; acceptance tests for the sprint goal (#209) A clip dropped in raw/ with no other action lands in web-clip, its entity and concept are the shared pages, raw/ is empty and nothing waits for a human; client-marked text and a typo'd wing name park; a deliberate subfolder places. Real pipeline and file layout, model stubbed. Co-Authored-By: Claude Sonnet 5.5 --- ingestion/internal/watcher/acceptance_test.go | 116 ++++++++++++++++++ ingestion/internal/watcher/watcher.go | 10 +- .../internal/watcher/watcher_wing_test.go | 11 +- 3 files changed, 132 insertions(+), 5 deletions(-) create mode 100644 ingestion/internal/watcher/acceptance_test.go diff --git a/ingestion/internal/watcher/acceptance_test.go b/ingestion/internal/watcher/acceptance_test.go new file mode 100644 index 0000000..2cfa999 --- /dev/null +++ b/ingestion/internal/watcher/acceptance_test.go @@ -0,0 +1,116 @@ +package watcher + +import ( + "context" + "fmt" + "os" + "path/filepath" + "regexp" + "strings" + "sync/atomic" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/mathiasbq/hyperguild/ingestion/internal/pipeline" + "github.com/mathiasbq/hyperguild/ingestion/internal/wing" +) + +// These tests are the sprint's goal (hyperguild#209) stated as behaviour, with +// the real pipeline and real file layout and only the model stubbed: a person +// drops a clip into raw/ and does nothing else. + +func acceptanceCfg(t *testing.T) (Config, string, *atomic.Int32) { + t.Helper() + brainDir := t.TempDir() + var calls atomic.Int32 + // Each call extracts a source with its own title, as real clips have: two + // sources with one title are deliberately merged into one page by the + // pipeline's title resolution. + pagesFor := func(n int32) []pipeline.RawPage { + return []pipeline.RawPage{ + {Title: fmt.Sprintf("Dark Factory Talk %d", n), Type: "source", Subtype: "video", Domain: "ai", + Content: "## Summary\n\nA talk.\n\n## Entities Mentioned\n\n[[Jev]]\n\n## Concepts Introduced or Reinforced\n\n[[Dark Factory]]\n"}, + {Title: "Jev", Type: "entity", Subtype: "tool", Content: "## Definition\n\nA decision model.\n\n## Sources\n"}, + {Title: "Dark Factory", Type: "concept", Content: "## Definition\n\nA fully automated build line.\n\n## Sources\n"}, + } + } + return Config{ + BrainDir: brainDir, + Resolver: wing.Router{BrainDir: brainDir, Default: "web-clip", ClientBlock: regexp.MustCompile(`(?i)acme-client`)}, + Pipeline: pipeline.Config{ + Schema: "# Schema", + Complete: func(context.Context, string, string) (string, error) { + return mustJSONPages(pagesFor(calls.Add(1))), nil + }, + }, + }, brainDir, &calls +} + +// topLevel lists the regular files directly in raw/: the inbox. +func topLevel(t *testing.T, brainDir string) []string { + t.Helper() + entries, err := os.ReadDir(filepath.Join(brainDir, "raw")) + require.NoError(t, err) + var out []string + for _, e := range entries { + if e.Type().IsRegular() { + out = append(out, e.Name()) + } + } + return out +} + +func TestAcceptance_ADroppedClipLandsInWebClipAndTheInboxIsEmpty(t *testing.T) { + cfg, brainDir, calls := acceptanceCfg(t) + put(t, brainDir, "dark-factory-talk.md", "Transcript of a talk about dark factories.") + + require.Empty(t, processDir(context.Background(), cfg, "2026-10-20")) + + assert.True(t, has(brainDir, "wiki/web-clip/sources/dark-factory-talk.md"), "the clip is a page in web-clip") + assert.True(t, has(brainDir, "wiki/entities/jev.md"), "its entity is in the shared layer") + assert.True(t, has(brainDir, "wiki/concepts/dark-factory.md"), "its concept is in the shared layer") + assert.Empty(t, topLevel(t, brainDir), "raw/ holds nothing unprocessed") + assert.False(t, has(brainDir, "raw/needs-review"), "and nothing waits for a human") + assert.Equal(t, int32(1), calls.Load(), "one extraction, no classification step") +} + +func TestAcceptance_TheSameEntityFromTwoClipsIsOnePage(t *testing.T) { + cfg, brainDir, _ := acceptanceCfg(t) + put(t, brainDir, "talk-one.md", "First talk.") + require.Empty(t, processDir(context.Background(), cfg, "2026-10-20")) + put(t, brainDir, "talk-two.md", "Second talk, different words.") + require.Empty(t, processDir(context.Background(), cfg, "2026-10-20")) + + files, err := filepath.Glob(filepath.Join(brainDir, "wiki", "entities", "*.md")) + require.NoError(t, err) + assert.Len(t, files, 1) + b, _ := os.ReadFile(filepath.Join(brainDir, "wiki/entities/jev.md")) + assert.True(t, strings.Contains(string(b), "talk-one") && strings.Contains(string(b), "talk-two"), "both sources are cited") +} + +func TestAcceptance_AClipThatLooksLikeClientMaterialIsParkedNotFiled(t *testing.T) { + cfg, brainDir, calls := acceptanceCfg(t) + put(t, brainDir, "kickoff.md", "Notes from the ACME-CLIENT kickoff call.") + + require.Empty(t, processDir(context.Background(), cfg, "2026-10-20")) + + assert.Zero(t, calls.Load(), "not even extracted") + assert.False(t, has(brainDir, "wiki/web-clip"), "never filed into a shared wing") + assert.True(t, has(brainDir, "raw/needs-review/kickoff.md"), "waits for the owner") + assert.Empty(t, topLevel(t, brainDir)) +} + +func TestAcceptance_ASubfolderNamingAnExistingWingPlacesThereAndATypoParks(t *testing.T) { + cfg, brainDir, _ := acceptanceCfg(t) + require.NoError(t, os.MkdirAll(filepath.Join(brainDir, "wiki", "homelab", "facts"), 0o755)) + put(t, brainDir, "homelab/ops-note.md", "A note about the cluster.") + put(t, brainDir, "hoemlab/typo-note.md", "A misfiled note.") + + require.Empty(t, processDir(context.Background(), cfg, "2026-10-20")) + + assert.True(t, has(brainDir, "wiki/homelab/sources/ops-note.md")) + assert.False(t, has(brainDir, "wiki/hoemlab"), "a typo never creates a wing") + assert.True(t, has(brainDir, "raw/needs-review/typo-note.md")) +} diff --git a/ingestion/internal/watcher/watcher.go b/ingestion/internal/watcher/watcher.go index 14c6280..61ea04f 100644 --- a/ingestion/internal/watcher/watcher.go +++ b/ingestion/internal/watcher/watcher.go @@ -31,10 +31,12 @@ type Config struct { // memory (hyperguild#198). 0 means defaultMaxNoteBytes. MaxNoteBytes int64 - // Resolver, when set, decides each note's wing (hyperguild#205): explicit - // subfolder/frontmatter, else classifier + scorer. A note it cannot place - // is parked in raw/needs-review/ for a human. Nil keeps the retired layout. - Resolver *wing.Resolver + // Resolver, when set, decides each note's wing (hyperguild#205, #208): a + // deliberate subfolder/frontmatter wing, else the default wing. A note it + // cannot place is parked in raw/needs-review/ for a human. Nil keeps the + // retired layout. Set it only to a non-nil value: a nil pointer inside the + // interface is not nil. + Resolver wing.Placer } // defaultMaxNoteBytes bounds a single note: extraction and the archive copy both diff --git a/ingestion/internal/watcher/watcher_wing_test.go b/ingestion/internal/watcher/watcher_wing_test.go index 9778ab9..8a17f4c 100644 --- a/ingestion/internal/watcher/watcher_wing_test.go +++ b/ingestion/internal/watcher/watcher_wing_test.go @@ -2,6 +2,7 @@ package watcher import ( "context" + "encoding/json" "errors" "os" "path/filepath" @@ -41,7 +42,7 @@ func wingResolver(c wing.Classifier, s wing.Scorer) *wing.Resolver { } } -func wingTestCfg(t *testing.T, r *wing.Resolver, llmCalls *atomic.Int32) (Config, string) { +func wingTestCfg(t *testing.T, r wing.Placer, llmCalls *atomic.Int32) (Config, string) { t.Helper() brainDir := setupBrainDir(t) return Config{ @@ -329,3 +330,11 @@ func TestWithProposal_ANonFenceDashLineDoesNotCloseTheBlock(t *testing.T) { assert.Contains(t, strings.Join(lines[:closing], "\n"), "proposal_reason:") assert.NotContains(t, strings.Join(lines[:closing], "\n"), "foo: bar", "the unfenced note is body, not part of the block") } + +func mustJSONPages(pages []pipeline.RawPage) string { + b, err := json.Marshal(pages) + if err != nil { + panic(err) + } + return string(b) +} -- 2.54.0 From 409b85096b0b1d96cb9eeeaa62e1cff734cf273e Mon Sep 17 00:00:00 2001 From: Mathias Date: Tue, 20 Oct 2026 20:14:47 +0200 Subject: [PATCH 04/14] feat(api): brain_promote and brain_reject act on notes parked in raw/needs-review/ (#208) brain_pending lists a parked note under its bare filename, but promote/reject only read raw/, so the documented approval path did not exist. They now fall back to raw/needs-review/ (top level wins on a name clash; a symlinked directory is refused) and promote drops the resolver's proposal keys. Mutation-checked. Co-Authored-By: Claude Sonnet 5.5 --- ingestion/internal/api/frontmatter.go | 12 +++ ingestion/internal/api/promote.go | 43 ++++++++--- ingestion/internal/api/promote_review_test.go | 75 +++++++++++++++++++ 3 files changed, 120 insertions(+), 10 deletions(-) create mode 100644 ingestion/internal/api/promote_review_test.go diff --git a/ingestion/internal/api/frontmatter.go b/ingestion/internal/api/frontmatter.go index f0614b4..3cfc7d7 100644 --- a/ingestion/internal/api/frontmatter.go +++ b/ingestion/internal/api/frontmatter.go @@ -83,6 +83,18 @@ func (f *frontmatter) get(key string) string { return "" } +// drop removes every `key: value` line for key (nothing else), so a +// machine-written key does not travel with a note into the wiki. +func (f *frontmatter) drop(key string) { + kept := f.lines[:0] + for _, l := range f.lines { + if l.key != key { + kept = append(kept, l) + } + } + f.lines = kept +} + // set overrides the value for an existing key in place, or appends a new // `key: value` line when the key is absent. func (f *frontmatter) set(key, value string) { diff --git a/ingestion/internal/api/promote.go b/ingestion/internal/api/promote.go index 756641a..6a1a966 100644 --- a/ingestion/internal/api/promote.go +++ b/ingestion/internal/api/promote.go @@ -143,11 +143,11 @@ func PromoteNote(brainDir string, opts PromoteOptions) (string, error) { return "", err } - src := filepath.Join(brainDir, "raw", base) - raw, err := readRawSource(brainDir, base) + raw, srcRel, err := readRawSource(brainDir, base) if err != nil { return "", err } + src := filepath.Join(brainDir, filepath.FromSlash(srcRel)) // Collision: never silently overwrite an existing promoted note. if _, statErr := os.Stat(dest); statErr == nil { @@ -156,6 +156,11 @@ func PromoteNote(brainDir string, opts PromoteOptions) (string, error) { } fm, body := parseFrontmatter(string(raw)) + // The resolver's proposal described where the note was going to be placed; + // once a human has placed it, it is noise (and `wing` is set below). + for _, k := range []string{"proposed_wing", "proposal_confidence", "proposal_reason"} { + fm.drop(k) + } now := time.Now().UTC().Format(time.RFC3339) fm.set("wing", brain.Sanitise(opts.Wing)) fm.set("hall", opts.Hall) @@ -215,11 +220,11 @@ func RejectNote(brainDir string, opts RejectOptions) (string, error) { return "", fmt.Errorf("invalid filename %q", opts.Filename) } - src := filepath.Join(brainDir, "raw", base) - raw, err := readRawSource(brainDir, base) + raw, srcRel, err := readRawSource(brainDir, base) if err != nil { return "", err } + src := filepath.Join(brainDir, filepath.FromSlash(srcRel)) dest := filepath.Join(brainDir, "raw", "rejected", base) if _, statErr := os.Stat(dest); statErr == nil { @@ -258,25 +263,43 @@ func excerpt(s string, n int) string { // readRawSource reads a pending note for promote/reject through the raw root, // refusing symlinks and a symlinked raw/ (hyperguild#153). -func readRawSource(brainDir, base string) ([]byte, error) { +func readRawSource(brainDir, base string) ([]byte, string, error) { root, _, err := OpenRawRoot(brainDir) if err != nil { if errors.Is(err, os.ErrNotExist) { - return nil, fmt.Errorf("pending note %q does not exist in raw/", base) + return nil, "", fmt.Errorf("pending note %q does not exist in raw/", base) } - return nil, fmt.Errorf("read source: %w", err) + return nil, "", fmt.Errorf("read source: %w", err) } defer func() { _ = root.Close() }() raw, _, err := readRawNote(root, base) + if err == nil { + return raw, "raw/" + base, nil + } + if !errors.Is(err, os.ErrNotExist) { + return nil, "", fmt.Errorf("read source: %w", err) + } + // brain_pending also lists notes parked in raw/needs-review/ under their + // bare filename (hyperguild#208); the top level wins on a name clash. The + // sub-root refuses a symlinked directory like OpenRawRoot does. + sub, serr := root.OpenRoot(parkedDir) + if serr != nil { + return nil, "", fmt.Errorf("pending note %q does not exist in raw/", base) + } + defer func() { _ = sub.Close() }() + raw, _, err = readRawNote(sub, base) if err != nil { if errors.Is(err, os.ErrNotExist) { - return nil, fmt.Errorf("pending note %q does not exist in raw/", base) + return nil, "", fmt.Errorf("pending note %q does not exist in raw/", base) } - return nil, fmt.Errorf("read source: %w", err) + return nil, "", fmt.Errorf("read source: %w", err) } - return raw, nil + return raw, "raw/" + parkedDir + "/" + base, nil } +// parkedDir is where the watcher parks notes it could not place. +const parkedDir = "needs-review" + // unquoteScalar undoes the Go-style double quoting the watcher writes around a // free-text value (strconv.Quote keeps it on one line); a bare value is // returned as is. diff --git a/ingestion/internal/api/promote_review_test.go b/ingestion/internal/api/promote_review_test.go new file mode 100644 index 0000000..37e7df5 --- /dev/null +++ b/ingestion/internal/api/promote_review_test.go @@ -0,0 +1,75 @@ +package api + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const parkedNote = "---\nproposed_wing: radar\nproposal_confidence: 0.40\nproposal_reason: \"text matches the client-content marker\"\ncreated_at: 2026-10-19T08:00:00Z\n---\nBody of the parked note.\n" + +// brain_pending lists a parked note under its bare filename, so promote and +// reject must be able to act on exactly that name (#208: they could not). +func TestPromoteNote_ActsOnANoteParkedInNeedsReview(t *testing.T) { + dir := t.TempDir() + writeParked(t, dir, "unsure.md", parkedNote) + + rel, err := PromoteNote(dir, PromoteOptions{Filename: "unsure.md", Wing: "homelab", Hall: "facts"}) + require.NoError(t, err) + assert.Equal(t, "wiki/homelab/facts/unsure.md", rel) + + got, err := os.ReadFile(filepath.Join(dir, "wiki/homelab/facts/unsure.md")) + require.NoError(t, err) + assert.Contains(t, string(got), "Body of the parked note.") + assert.Contains(t, string(got), "wing: homelab") + assert.NotContains(t, string(got), "proposed_wing", "the resolver's proposal is not part of the promoted note") + assert.NotContains(t, string(got), "proposal_reason") + + _, statErr := os.Stat(filepath.Join(dir, "raw", "needs-review", "unsure.md")) + assert.ErrorIs(t, statErr, os.ErrNotExist, "the parked note is consumed") + pending, err := ListPending(dir) + require.NoError(t, err) + assert.Empty(t, pending) +} + +func TestRejectNote_ActsOnANoteParkedInNeedsReview(t *testing.T) { + dir := t.TempDir() + writeParked(t, dir, "unsure.md", parkedNote) + + rel, err := RejectNote(dir, RejectOptions{Filename: "unsure.md", Reason: "not worth keeping"}) + require.NoError(t, err) + assert.Equal(t, "raw/rejected/unsure.md", rel) + _, statErr := os.Stat(filepath.Join(dir, "raw", "needs-review", "unsure.md")) + assert.ErrorIs(t, statErr, os.ErrNotExist) + pending, _ := ListPending(dir) + assert.Empty(t, pending, "a rejected note is not re-listed") +} + +func TestPromoteNote_ATopLevelNoteWinsOverAParkedOneOfTheSameName(t *testing.T) { + dir := t.TempDir() + writeRaw(t, dir, "same.md", "---\ncreated_at: 2026-10-20T08:00:00Z\n---\nTop-level body.\n") + writeParked(t, dir, "same.md", parkedNote) + + _, err := PromoteNote(dir, PromoteOptions{Filename: "same.md", Wing: "homelab", Hall: "facts"}) + require.NoError(t, err) + got, _ := os.ReadFile(filepath.Join(dir, "wiki/homelab/facts/same.md")) + assert.Contains(t, string(got), "Top-level body.") + _, statErr := os.Stat(filepath.Join(dir, "raw", "needs-review", "same.md")) + assert.NoError(t, statErr, "the parked note of the same name is left alone") +} + +func TestPromoteNote_ASymlinkedNeedsReviewIsNotRead(t *testing.T) { + dir := t.TempDir() + outside := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(outside, "secret.md"), []byte("outside"), 0o644)) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "raw"), 0o755)) + require.NoError(t, os.Symlink(outside, filepath.Join(dir, "raw", "needs-review"))) + + _, err := PromoteNote(dir, PromoteOptions{Filename: "secret.md", Wing: "homelab", Hall: "facts"}) + assert.Error(t, err) + _, statErr := os.Stat(filepath.Join(dir, "wiki/homelab/facts/secret.md")) + assert.ErrorIs(t, statErr, os.ErrNotExist) +} -- 2.54.0 From 8988c291cab0a1fc285381bbe12b5faab69df60e Mon Sep 17 00:00:00 2001 From: Mathias Date: Tue, 20 Oct 2026 20:18:39 +0200 Subject: [PATCH 05/14] feat(server): INGEST_WING_ROUTING wires the Router; remove the classifier path (#208) Removes the LLM classifier, the substring 'BM25' scorer, the agreement gate, the backtest and the old migrate/wingmigrate tooling: measured at 2 of 55 pages placed (#208), and the migration was built for the per-wing facts layout this branch replaces. wing now holds the decision types, the Router and KnownWings. INGEST_WING_ROUTING=1 builds the Router (default wing INGEST_DEFAULT_WING=web-clip, client pattern from INGEST_CLIENT_BLOCK or CLAUDE_INGEST_CLIENT_BLOCK); a bad default or pattern is fatal at startup. Watcher tests rewritten for the Router, including a deliberate subfolder into any existing wing, typo'd folders parked, forged-key and edit-while-placing cases. Co-Authored-By: Claude Sonnet 5.5 --- ingestion/cmd/server/main.go | 43 +-- ingestion/cmd/server/main_test.go | 20 - ingestion/cmd/wingbacktest/main.go | 147 -------- ingestion/cmd/wingmigrate/main.go | 92 ----- ingestion/internal/migrate/migrate.go | 309 --------------- ingestion/internal/migrate/migrate_test.go | 243 ------------ .../internal/watcher/watcher_wing_test.go | 352 ++++++++---------- ingestion/internal/wing/backtest.go | 43 --- ingestion/internal/wing/backtest_test.go | 38 -- ingestion/internal/wing/bm25scorer.go | 85 ----- ingestion/internal/wing/bm25scorer_test.go | 61 --- ingestion/internal/wing/configured.go | 32 -- ingestion/internal/wing/configured_test.go | 47 --- ingestion/internal/wing/helpers_test.go | 16 + ingestion/internal/wing/llmclassifier.go | 99 ----- ingestion/internal/wing/llmclassifier_test.go | 67 ---- ingestion/internal/wing/resolve.go | 156 +------- ingestion/internal/wing/resolve_test.go | 112 ------ ingestion/internal/wing/router.go | 26 +- ingestion/internal/wing/router_test.go | 27 ++ 20 files changed, 263 insertions(+), 1752 deletions(-) delete mode 100644 ingestion/cmd/wingbacktest/main.go delete mode 100644 ingestion/cmd/wingmigrate/main.go delete mode 100644 ingestion/internal/migrate/migrate.go delete mode 100644 ingestion/internal/migrate/migrate_test.go delete mode 100644 ingestion/internal/wing/backtest.go delete mode 100644 ingestion/internal/wing/backtest_test.go delete mode 100644 ingestion/internal/wing/bm25scorer.go delete mode 100644 ingestion/internal/wing/bm25scorer_test.go delete mode 100644 ingestion/internal/wing/configured.go delete mode 100644 ingestion/internal/wing/configured_test.go create mode 100644 ingestion/internal/wing/helpers_test.go delete mode 100644 ingestion/internal/wing/llmclassifier.go delete mode 100644 ingestion/internal/wing/llmclassifier_test.go delete mode 100644 ingestion/internal/wing/resolve_test.go diff --git a/ingestion/cmd/server/main.go b/ingestion/cmd/server/main.go index 997dcea..07d26e7 100644 --- a/ingestion/cmd/server/main.go +++ b/ingestion/cmd/server/main.go @@ -150,17 +150,6 @@ func envInt(key string, fallback int) int { return fallback } -// envFloat reads a confidence-style value in [0,1]. Anything unparseable or -// outside the range falls back: a threshold of 0 would auto-place everything. -func envFloat(key string, fallback float64) float64 { - if v := os.Getenv(key); v != "" { - if f, err := strconv.ParseFloat(v, 64); err == nil && f >= 0 && f <= 1 { - return f - } - } - return fallback -} - // buildNotifier returns the ntfy notifier, or a nil interface when // BRAIN_NTFY_URL is unset. A nil *interface* (not a typed-nil) is required so // DegradingSink/Reconcile and the governance observer skip it cleanly. @@ -406,26 +395,34 @@ func main() { } if watchInterval > 0 { - // INGEST_WING_RESOLVE=1 routes raw/ notes into wiki// by - // subfolder, frontmatter or classifier+scorer, and parks the rest in - // raw/needs-review/ (hyperguild#205). Off = the retired layout. Set - // explicitly, a failure to build the resolver is fatal: silently - // falling back would write into the layout this flag exists to leave. - var wingResolver *wing.Resolver - if os.Getenv("INGEST_WING_RESOLVE") == "1" { - var rerr error - wingResolver, rerr = wing.NewConfiguredResolver(brainDir, llmClient.Complete, envFloat("INGEST_WING_THRESHOLD", 0.9)) + // INGEST_WING_ROUTING=1 files raw/ notes by rule (hyperguild#208): a + // deliberate raw// folder or `wing:` frontmatter, else the default + // wing (INGEST_DEFAULT_WING, "web-clip"); a note whose text matches the + // client-content pattern (INGEST_CLIENT_BLOCK, else + // CLAUDE_INGEST_CLIENT_BLOCK) or that names an unknown wing is parked in + // raw/needs-review/. Off = the retired layout. Set explicitly, a bad + // default or pattern is fatal: silently falling back would write into + // the layout this flag exists to leave. The interface stays nil when off + // (a nil *Router inside it would not be nil). + var placer wing.Placer + if os.Getenv("INGEST_WING_ROUTING") == "1" { + pattern := os.Getenv("INGEST_CLIENT_BLOCK") + if pattern == "" { + pattern = os.Getenv("CLAUDE_INGEST_CLIENT_BLOCK") + } + router, rerr := wing.NewRouter(brainDir, envOr("INGEST_DEFAULT_WING", "web-clip"), pattern) if rerr != nil { - logger.Error("build wing resolver", "err", rerr) + logger.Error("build wing router", "err", rerr) os.Exit(1) } - logger.Info("wing resolver enabled", "wings", len(wingResolver.Known), "threshold", wingResolver.Threshold) + placer = router + logger.Info("wing routing enabled", "default_wing", router.Default, "client_filter", router.ClientBlock != nil) } watcher.Start(ctx, watcher.Config{ BrainDir: brainDir, Interval: time.Duration(watchInterval) * time.Second, Pipeline: pipelineCfg, - Resolver: wingResolver, + Resolver: placer, }) } diff --git a/ingestion/cmd/server/main_test.go b/ingestion/cmd/server/main_test.go index af50176..b52f0c7 100644 --- a/ingestion/cmd/server/main_test.go +++ b/ingestion/cmd/server/main_test.go @@ -100,23 +100,3 @@ func TestClaudeSinkDoesNotWriteThroughASymlinkedArchiveDir(t *testing.T) { }) } } - -func TestEnvFloat(t *testing.T) { - tests := []struct { - name string - set string - want float64 - }{ - {"unset falls back", "", 0.9}, - {"valid value", "0.75", 0.75}, - {"garbage falls back rather than becoming 0", "high", 0.9}, - {"out of range is rejected, not clamped silently into auto-placing everything", "-1", 0.9}, - {"above one is rejected", "1.5", 0.9}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Setenv("TEST_ENV_FLOAT", tt.set) - assert.InDelta(t, tt.want, envFloat("TEST_ENV_FLOAT", 0.9), 1e-9) - }) - } -} diff --git a/ingestion/cmd/wingbacktest/main.go b/ingestion/cmd/wingbacktest/main.go deleted file mode 100644 index cab4638..0000000 --- a/ingestion/cmd/wingbacktest/main.go +++ /dev/null @@ -1,147 +0,0 @@ -// Command wingbacktest measures how well the wing resolver would have placed -// pages whose wing a human already decided (hyperguild#205). -// -// It reads pages from the wings given in -wings, hides the wing, asks the -// local classifier and the BM25 scorer, and reports coverage/precision per -// confidence threshold. Page text is sent to -llm-url, which MUST be a local -// model (it is not checked here); sample only wings you would allow. -package main - -import ( - "context" - "flag" - "fmt" - "io/fs" - "log/slog" - "math/rand" - "os" - "path/filepath" - "sort" - "strings" - "time" - - "github.com/mathiasbq/hyperguild/ingestion/internal/llm" - "github.com/mathiasbq/hyperguild/ingestion/internal/wing" -) - -func main() { - brain := flag.String("brain", "/var/lib/supervisor/brain", "brain directory (read only)") - wingsFlag := flag.String("wings", "", "comma-separated wings to sample pages FROM (must be wings you would allow)") - perWing := flag.Int("per-wing", 8, "pages sampled per wing") - llmURL := flag.String("llm-url", "http://10.0.1.20:31234/v1", "OpenAI-compatible LOCAL endpoint") - model := flag.String("model", "qwen35-9b-fast", "model name") - seed := flag.Int64("seed", 1, "sampling seed") - flag.Parse() - - if err := run(*brain, strings.Split(*wingsFlag, ","), *perWing, *llmURL, *model, *seed); err != nil { - slog.Error("wingbacktest", "err", err) - os.Exit(1) - } -} - -func run(brain string, sampleWings []string, perWing int, llmURL, model string, seed int64) error { - known, err := wing.KnownWings(brain) - if err != nil { - return err - } - client := llm.New(llmURL, "", model, 3*time.Minute) - classifier := wing.NewLLMClassifier(client.Complete) - rng := rand.New(rand.NewSource(seed)) - - var samples []wing.Sample - perWingSamples := map[string][]wing.Sample{} - for _, w := range sampleWings { - w = strings.TrimSpace(w) - if w == "" { - continue - } - pages, err := listPages(brain, w) - if err != nil { - return err - } - rng.Shuffle(len(pages), func(i, j int) { pages[i], pages[j] = pages[j], pages[i] }) - if len(pages) > perWing { - pages = pages[:perWing] - } - for _, rel := range pages { - text, err := readBody(brain, rel) - if err != nil || len(text) < 200 { - continue - } - sugg, cerr := classifier.Classify(context.Background(), text, known) - if cerr != nil { - slog.Warn("classify failed", "page", rel, "err", cerr) - } - top, _ := wing.TopWing(wing.NewBM25Scorer(brain, known, rel).Score(text)) - s := wing.Sample{Truth: w, Suggested: strings.ToLower(sugg.Wing), Confidence: sugg.Confidence, ScorerTop: top} - slog.Info("sample", "page", rel, "truth", w, "classifier", s.Suggested, "conf", s.Confidence, "scorer", top, "reason", sugg.Reason) - samples = append(samples, s) - perWingSamples[w] = append(perWingSamples[w], s) - } - } - report(samples, perWingSamples) - return nil -} - -func report(samples []wing.Sample, per map[string][]wing.Sample) { - fmt.Printf("# wing backtest\n\nn=%d pages. Classifier+scorer must agree; threshold applies to classifier confidence.\n\n", len(samples)) - fmt.Println("| threshold | auto | coverage | precision |") - fmt.Println("|---|---|---|---|") - for _, t := range []float64{0.5, 0.6, 0.7, 0.8, 0.9, 0.95} { - r := wing.Evaluate(samples, t) - fmt.Printf("| %.2f | %d/%d | %.0f%% | %.0f%% |\n", t, r.Auto, r.N, 100*r.Coverage, 100*r.Precision) - } - fmt.Println("\n## per wing (threshold 0.80)\n\n| wing | n | auto | precision |\n|---|---|---|---|") - names := make([]string, 0, len(per)) - for w := range per { - names = append(names, w) - } - sort.Strings(names) - for _, w := range names { - r := wing.Evaluate(per[w], 0.8) - fmt.Printf("| %s | %d | %d | %.0f%% |\n", w, r.N, r.Auto, 100*r.Precision) - } - fmt.Println("\n## wrong auto-placements at 0.80") - fmt.Println() - for _, s := range samples { - if s.Suggested != "" && s.Confidence >= 0.8 && s.ScorerTop == s.Suggested && s.Suggested != s.Truth { - fmt.Printf("- truth=%s -> both said %s (conf %.2f)\n", s.Truth, s.Suggested, s.Confidence) - } - } -} - -func listPages(brain, w string) ([]string, error) { - var out []string - root := filepath.Join(brain, "wiki", w) - err := filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { - if err != nil || d.IsDir() || !d.Type().IsRegular() || !strings.HasSuffix(p, ".md") || d.Name() == "_index.md" { - return err - } - rel, rerr := filepath.Rel(brain, p) - if rerr != nil { - return rerr - } - out = append(out, filepath.ToSlash(rel)) - return nil - }) - if err != nil { - return nil, fmt.Errorf("walk %s: %w", w, err) - } - return out, nil -} - -// readBody returns the page text without YAML frontmatter, so the wing label -// (and anything else in frontmatter) cannot leak into the classification. -func readBody(brain, rel string) (string, error) { - b, err := os.ReadFile(filepath.Join(brain, filepath.FromSlash(rel))) - if err != nil { - return "", fmt.Errorf("read %s: %w", rel, err) - } - s := string(b) - if strings.HasPrefix(s, "---\n") { - if end := strings.Index(s[4:], "\n---"); end >= 0 { - s = s[4+end+4:] - } - } - return strings.TrimSpace(s), nil -} diff --git a/ingestion/cmd/wingmigrate/main.go b/ingestion/cmd/wingmigrate/main.go deleted file mode 100644 index eb0707d..0000000 --- a/ingestion/cmd/wingmigrate/main.go +++ /dev/null @@ -1,92 +0,0 @@ -// Command wingmigrate moves pages out of the retired wiki/{sources,concepts, -// entities}/ layout into wiki// (hyperguild#205 S5). -// -// Default is a dry run that prints the plan. -apply performs the Auto items. -// Page text goes to -llm-url, which MUST be a local model. -package main - -import ( - "context" - "flag" - "fmt" - "log/slog" - "os" - "strings" - "time" - - "github.com/mathiasbq/hyperguild/ingestion/internal/llm" - "github.com/mathiasbq/hyperguild/ingestion/internal/migrate" - "github.com/mathiasbq/hyperguild/ingestion/internal/wing" -) - -func main() { - brain := flag.String("brain", "/var/lib/supervisor/brain", "brain directory") - llmURL := flag.String("llm-url", "http://10.0.1.20:31234/v1", "OpenAI-compatible LOCAL endpoint") - model := flag.String("model", "qwen35-9b-fast", "model name") - threshold := flag.Float64("threshold", 0.9, "classifier confidence threshold") - apply := flag.Bool("apply", false, "perform the Auto moves (default: dry run)") - what := flag.String("what-if-eligible", "", "dry run only: comma-separated wings to treat as eligible, to see what tagging them would allow") - flag.Parse() - - if *apply && *what != "" { - slog.Error("-what-if-eligible is a dry-run aid and cannot be combined with -apply: eligibility is a sovereignty decision made in classification.yaml") - os.Exit(2) - } - if err := run(*brain, *llmURL, *model, *threshold, *apply, *what); err != nil { - slog.Error("wingmigrate", "err", err) - os.Exit(1) - } -} - -func run(brain, llmURL, model string, threshold float64, apply bool, whatIf string) error { - client := llm.New(llmURL, "", model, 3*time.Minute) - base, err := wing.NewConfiguredResolver(brain, client.Complete, threshold) - if err != nil { - return err - } - if whatIf != "" { - allowed := map[string]bool{} - for _, w := range strings.Split(whatIf, ",") { - allowed[strings.TrimSpace(w)] = true - } - base.Eligible = func(w string) bool { return allowed[w] } - } - rf := func(exclude string) *wing.Resolver { - r := *base - r.Scorer = wing.NewBM25Scorer(brain, base.Known, exclude) - return &r - } - items, err := migrate.BuildPlan(context.Background(), brain, rf) - if err != nil { - return err - } - report(items) - if !apply { - fmt.Println("\n_dry run: nothing was changed_") - return nil - } - n, err := migrate.Apply(brain, items) - fmt.Printf("\napplied %d moves\n", n) - return err -} - -func report(items []migrate.Item) { - auto, exc := 0, 0 - for _, it := range items { - if it.Status == migrate.Auto { - auto++ - } else { - exc++ - } - } - fmt.Printf("# wing migration plan\n\n%d pages: %d auto, %d exceptions\n\n", len(items), auto, exc) - fmt.Println("| status | kind | from | to / proposal | how | why |") - fmt.Println("|---|---|---|---|---|---|") - for _, it := range items { - dest := it.To - if dest == "" { - dest = it.Wing - } - fmt.Printf("| %s | %s | %s | %s | %s | %s |\n", it.Status, it.Kind, it.From, dest, it.Method, strings.ReplaceAll(it.Reason, "|", "/")) - } -} diff --git a/ingestion/internal/migrate/migrate.go b/ingestion/internal/migrate/migrate.go deleted file mode 100644 index d92001b..0000000 --- a/ingestion/internal/migrate/migrate.go +++ /dev/null @@ -1,309 +0,0 @@ -// Package migrate moves pages out of the retired wiki/{sources,concepts, -// entities}/ layout into wiki//{sources,facts}/ (hyperguild#205). -// -// It is a two-step tool on purpose. BuildPlan only reads and decides; Apply -// performs exactly the items the plan marked Auto and nothing else. Sources are -// placed by the wing Resolver. Concepts and entities have no wing of their own: -// they inherit the strict-majority wing of the sources that cite them, which is -// deterministic and needs no model. Whatever cannot be placed safely is an -// Exception, left where it is for a human. -package migrate - -import ( - "context" - "errors" - "fmt" - "io/fs" - "path" - "regexp" - "sort" - "strings" - - "github.com/mathiasbq/hyperguild/ingestion/internal/brain" - "github.com/mathiasbq/hyperguild/ingestion/internal/brainfs" - "github.com/mathiasbq/hyperguild/ingestion/internal/wing" -) - -// Status is whether the plan moves a page by itself. -type Status string - -const ( - Auto Status = "auto" - Exception Status = "exception" -) - -// Item is one planned move. -type Item struct { - From string // brain-relative, e.g. wiki/entities/jev.md - To string // brain-relative; empty for an Exception without a proposal - Slug string - Kind string // source | concept | entity - Wing string // decided wing, or the proposal for an Exception - Status Status - Method string // resolver | inherited | junk | blocked - Confidence float64 - Reason string -} - -// ResolverFor returns the Resolver to use for one page. It takes the page's -// own path so a scorer can exclude the page from its evidence. -type ResolverFor func(excludePath string) *wing.Resolver - -var ( - linkSlugRE = regexp.MustCompile(`\[\[([^|\]]+)`) - legacy = []struct{ dir, kind string }{ - {"wiki/sources", "source"}, - {"wiki/concepts", "concept"}, - {"wiki/entities", "entity"}, - } -) - -// BuildPlan reads the legacy layout and decides, without writing anything. -func BuildPlan(ctx context.Context, brainDir string, rf ResolverFor) ([]Item, error) { - bfs, err := brainfs.Open(brainDir) - if err != nil { - return nil, fmt.Errorf("open brain: %w", err) - } - defer func() { _ = bfs.Close() }() - - taken, err := slugsInWings(bfs) - if err != nil { - return nil, err - } - - var items []Item - decided := map[string]Item{} // source slug -> its decision - for _, l := range legacy { - names, err := bfs.ReadDirRegular(l.dir) - if errors.Is(err, fs.ErrNotExist) { - continue - } - if err != nil { - return nil, fmt.Errorf("read %s: %w", l.dir, err) - } - for _, name := range names { - if !strings.HasSuffix(name, ".md") || name == "_index.md" { - continue - } - rel := l.dir + "/" + name - slug := strings.TrimSuffix(name, ".md") - it := Item{From: rel, Slug: slug, Kind: l.kind} - if strings.HasPrefix(name, "tunnel-candidates-") { - it.Status, it.Method, it.Reason = Exception, "junk", "auto-tunnel review log ingested as a source; not a source" - items = append(items, it) - continue - } - content, err := bfs.ReadRegular(rel) - if err != nil { - it.Status, it.Method, it.Reason = Exception, "blocked", fmt.Sprintf("unreadable: %v", err) - items = append(items, it) - continue - } - if l.kind == "source" { - d := rf(rel).Resolve(ctx, wing.Note{Text: string(content), FrontmatterWing: frontmatterValue(string(content), "wing")}) - it.Method, it.Wing, it.Confidence, it.Reason = "resolver", d.Wing, d.Confidence, d.Reason - if d.Source == wing.Exception { - it.Status = Exception - } else { - it.Status, it.To = Auto, "wiki/"+d.Wing+"/sources/"+name - } - decided[slug] = it - } - items = append(items, it) - } - } - - // Second pass: concepts and entities inherit from their sources. - for i, it := range items { - if it.Kind == "source" || it.Method == "junk" || it.Method == "blocked" { - continue - } - content, err := bfs.ReadRegular(it.From) - if err != nil { - continue - } - it.Method = "inherited" - w, reason := inheritedWing(string(content), decided) - if w == "" { - it.Status, it.Reason = Exception, reason - } else { - it.Status, it.Wing, it.To = Auto, w, "wiki/"+w+"/facts/"+it.Slug+".md" - it.Reason = fmt.Sprintf("strict majority of its sources are in %q", w) - } - items[i] = it - } - - blockCollisions(bfs, items, taken) - sort.SliceStable(items, func(i, j int) bool { return items[i].From < items[j].From }) - return items, nil -} - -// inheritedWing returns the wing held by a strict majority of the legacy -// sources cited in the page's "## Sources" section, or "" with the reason. -func inheritedWing(content string, decided map[string]Item) (string, string) { - section := sourcesSection(content) - if section == "" { - return "", "page has no Sources section to inherit a wing from" - } - total := 0 - votes := map[string]int{} - for _, m := range linkSlugRE.FindAllStringSubmatch(section, -1) { - d, ok := decided[strings.TrimSpace(m[1])] - if !ok { - continue // not a legacy source (already migrated, or not a source) - } - total++ - if d.Status == Auto { - votes[d.Wing]++ - } - } - if total == 0 { - return "", "none of its cited sources are in the legacy layout" - } - for w, n := range votes { - if n*2 > total { - return w, "" - } - } - return "", fmt.Sprintf("its %d cited sources disagree on a wing or were not placed", total) -} - -// blockCollisions turns an Auto item into an Exception when its destination -// exists, its slug is already used in some wing (graph identity is the slug -// alone), or two planned items would land on the same path. -func blockCollisions(bfs *brainfs.Brain, items []Item, taken map[string]string) { - planned := map[string]int{} - for _, it := range items { - if it.Status == Auto { - planned[it.To]++ - } - } - for i, it := range items { - if it.Status != Auto { - continue - } - switch { - case planned[it.To] > 1: - items[i].Status, items[i].Reason = Exception, fmt.Sprintf("another legacy page would land on %s", it.To) - case exists(bfs, it.To): - items[i].Status, items[i].Reason = Exception, fmt.Sprintf("%s already exists", it.To) - default: - if other, ok := taken[it.Slug]; ok { - items[i].Status, items[i].Reason = Exception, fmt.Sprintf("slug %q is already used by %s", it.Slug, other) - } - } - } -} - -func exists(bfs *brainfs.Brain, rel string) bool { - _, err := bfs.Lstat(rel) - return err == nil -} - -// slugsInWings maps every slug under wiki/// to its path. -func slugsInWings(bfs *brainfs.Brain) (map[string]string, error) { - out := map[string]string{} - wings, err := bfs.ReadDirDirs("wiki") - if errors.Is(err, fs.ErrNotExist) { - return out, nil - } - if err != nil { - return nil, fmt.Errorf("list wiki: %w", err) - } - for _, w := range wings { - if w == "sources" || w == "concepts" || w == "entities" { - continue - } - halls, err := bfs.ReadDirDirs("wiki/" + w) - if err != nil { - continue - } - for _, h := range halls { - names, err := bfs.ReadDirRegular("wiki/" + w + "/" + h) - if err != nil { - continue - } - for _, n := range names { - if strings.HasSuffix(n, ".md") && n != "_index.md" { - out[strings.TrimSuffix(n, ".md")] = "wiki/" + w + "/" + h + "/" + n - } - } - } - } - return out, nil -} - -// Apply performs the Auto items, one at a time, and stops at the first -// failure. Each move writes the destination with O_EXCL, then removes the -// source, so a failure never loses a page. Exceptions are not touched. -func Apply(brainDir string, items []Item) (int, error) { - bfs, err := brainfs.Open(brainDir) - if err != nil { - return 0, fmt.Errorf("open brain: %w", err) - } - defer func() { _ = bfs.Close() }() - - n := 0 - touched := map[string]bool{} - for _, it := range items { - if it.Status != Auto { - continue - } - content, err := bfs.ReadRegular(it.From) - if err != nil { - return n, fmt.Errorf("read %s: %w", it.From, err) - } - hall := path.Base(path.Dir(it.To)) - if err := bfs.CreateRegular(it.To, []byte(stamp(string(content), it.Wing, hall, it.Kind))); err != nil { - return n, fmt.Errorf("write %s: %w", it.To, err) - } - if err := bfs.Remove(it.From); err != nil { - return n, fmt.Errorf("moved to %s but removing %s failed: %w", it.To, it.From, err) - } - touched[it.Wing] = true - n++ - } - for w := range touched { - _ = brain.BuildWingIndex(brainDir, w) // best-effort upkeep; the pages are already moved - } - return n, nil -} - -// stamp inserts wing/hall (and kind for facts) after the opening fence, leaving -// every other byte of the page alone. A page without frontmatter gets a block. -func stamp(content, w, hall, kind string) string { - extra := "wing: " + w + "\nhall: " + hall + "\n" - if hall == "facts" { - extra += "kind: " + kind + "\n" - } - if strings.HasPrefix(content, "---\n") { - return "---\n" + extra + content[4:] - } - return "---\n" + extra + "---\n" + content -} - -// sourcesSection returns the text under a "## Sources" heading up to the next -// "## " heading. -func sourcesSection(content string) string { - _, rest, ok := strings.Cut(content, "\n## Sources") - if !ok { - return "" - } - if i := strings.Index(rest, "\n## "); i >= 0 { - rest = rest[:i] - } - return rest -} - -func frontmatterValue(content, key string) string { - if !strings.HasPrefix(content, "---\n") { - return "" - } - block, _, _ := strings.Cut(content[4:], "\n---") - for _, line := range strings.Split(block, "\n") { - if k, v, ok := strings.Cut(line, ":"); ok && strings.TrimSpace(k) == key { - return strings.Trim(strings.TrimSpace(v), `"'`) - } - } - return "" -} diff --git a/ingestion/internal/migrate/migrate_test.go b/ingestion/internal/migrate/migrate_test.go deleted file mode 100644 index 6dbc1d8..0000000 --- a/ingestion/internal/migrate/migrate_test.go +++ /dev/null @@ -1,243 +0,0 @@ -package migrate_test - -import ( - "context" - "os" - "path/filepath" - "strings" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/mathiasbq/hyperguild/ingestion/internal/migrate" - "github.com/mathiasbq/hyperguild/ingestion/internal/wing" -) - -// keywordClassifier picks a wing by a marker word in the text, and the scorer -// agrees by the same marker, so a page "belongs" to the wing whose marker it holds. -type keywordClassifier struct{} - -func (keywordClassifier) Classify(_ context.Context, text string, _ []string) (wing.Suggestion, error) { - for _, w := range []string{"alpha", "beta"} { - if strings.Contains(text, strings.ToUpper(w)) { - return wing.Suggestion{Wing: w, Confidence: 0.95, Reason: "marker"}, nil - } - } - return wing.Suggestion{}, nil -} - -type keywordScorer struct{} - -func (keywordScorer) Score(text string) map[string]float64 { - out := map[string]float64{} - for _, w := range []string{"alpha", "beta"} { - if strings.Contains(text, strings.ToUpper(w)) { - out[w] = 1 - } - } - return out -} - -func resolverFor(string) *wing.Resolver { - return &wing.Resolver{ - Known: []string{"alpha", "beta", "client"}, - Eligible: func(w string) bool { return w != "client" }, - Classifier: keywordClassifier{}, - Scorer: keywordScorer{}, - Threshold: 0.8, - } -} - -func page(t *testing.T, dir, rel, body string) { - t.Helper() - full := filepath.Join(dir, rel) - require.NoError(t, os.MkdirAll(filepath.Dir(full), 0o755)) - require.NoError(t, os.WriteFile(full, []byte(body), 0o644)) -} - -func src(title, marker string) string { - return "---\ntitle: " + title + "\ntype: video\n---\n\n## Summary\n\n" + marker + " content.\n" -} - -func entity(title string, sources ...string) string { - var b strings.Builder - b.WriteString("---\ntitle: " + title + "\ntype: tool\n---\n\n## Definition\n\nA thing.\n\n## Sources\n\n") - for _, s := range sources { - b.WriteString("- [[" + s + "|" + s + "]]\n") - } - return b.String() -} - -func byFrom(items []migrate.Item) map[string]migrate.Item { - m := map[string]migrate.Item{} - for _, it := range items { - m[it.From] = it - } - return m -} - -func TestPlan_AResolvedSourceGoesToItsWingsSourcesHall(t *testing.T) { - dir := t.TempDir() - page(t, dir, "wiki/sources/talk-a.md", src("Talk A", "ALPHA")) - - items, err := migrate.BuildPlan(context.Background(), dir, resolverFor) - require.NoError(t, err) - it := byFrom(items)["wiki/sources/talk-a.md"] - assert.Equal(t, migrate.Auto, it.Status) - assert.Equal(t, "wiki/alpha/sources/talk-a.md", it.To) -} - -func TestPlan_AutoTunnelLogsAreJunkNotSources(t *testing.T) { - dir := t.TempDir() - page(t, dir, "wiki/sources/tunnel-candidates-2026-06-14.md", src("t", "ALPHA")) - - items, err := migrate.BuildPlan(context.Background(), dir, resolverFor) - require.NoError(t, err) - it := byFrom(items)["wiki/sources/tunnel-candidates-2026-06-14.md"] - assert.Equal(t, migrate.Exception, it.Status, "never auto-moved, whatever the classifier says") - assert.Equal(t, "junk", it.Method) -} - -func TestPlan_AnEntityFollowsTheMajorityWingOfItsSources(t *testing.T) { - dir := t.TempDir() - page(t, dir, "wiki/sources/a1.md", src("A1", "ALPHA")) - page(t, dir, "wiki/sources/a2.md", src("A2", "ALPHA")) - page(t, dir, "wiki/sources/b1.md", src("B1", "BETA")) - page(t, dir, "wiki/entities/jev.md", entity("Jev", "a1", "a2", "b1")) - page(t, dir, "wiki/concepts/factory.md", "---\ntitle: Factory\n---\n\n## Definition\n\nx\n\n## Sources\n\n- [[a1|A1]]\n") - - items, err := migrate.BuildPlan(context.Background(), dir, resolverFor) - require.NoError(t, err) - m := byFrom(items) - assert.Equal(t, "wiki/alpha/facts/jev.md", m["wiki/entities/jev.md"].To) - assert.Equal(t, "entity", m["wiki/entities/jev.md"].Kind) - assert.Equal(t, "wiki/alpha/facts/factory.md", m["wiki/concepts/factory.md"].To) - assert.Equal(t, "concept", m["wiki/concepts/factory.md"].Kind) -} - -func TestPlan_ATieBetweenWingsIsAnException(t *testing.T) { - dir := t.TempDir() - page(t, dir, "wiki/sources/a1.md", src("A1", "ALPHA")) - page(t, dir, "wiki/sources/b1.md", src("B1", "BETA")) - page(t, dir, "wiki/entities/jev.md", entity("Jev", "a1", "b1")) - - items, err := migrate.BuildPlan(context.Background(), dir, resolverFor) - require.NoError(t, err) - it := byFrom(items)["wiki/entities/jev.md"] - assert.Equal(t, migrate.Exception, it.Status) - assert.Contains(t, it.Reason, "disagree") -} - -func TestPlan_AnEntityWhoseSourcesWereNotPlacedIsAnException(t *testing.T) { - dir := t.TempDir() - page(t, dir, "wiki/sources/mystery.md", src("Mystery", "nothing recognisable")) - page(t, dir, "wiki/entities/orphan.md", entity("Orphan", "mystery")) - page(t, dir, "wiki/entities/nosources.md", "---\ntitle: No Sources\n---\n\n## Definition\n\nx\n") - - items, err := migrate.BuildPlan(context.Background(), dir, resolverFor) - require.NoError(t, err) - m := byFrom(items) - assert.Equal(t, migrate.Exception, m["wiki/entities/orphan.md"].Status) - assert.Equal(t, migrate.Exception, m["wiki/entities/nosources.md"].Status) -} - -func TestPlan_ADestinationThatAlreadyExistsIsAnException(t *testing.T) { - dir := t.TempDir() - page(t, dir, "wiki/sources/talk-a.md", src("Talk A", "ALPHA")) - page(t, dir, "wiki/alpha/sources/talk-a.md", "someone else's page") - - items, err := migrate.BuildPlan(context.Background(), dir, resolverFor) - require.NoError(t, err) - it := byFrom(items)["wiki/sources/talk-a.md"] - assert.Equal(t, migrate.Exception, it.Status) - assert.Contains(t, it.Reason, "already exists", "the occupied destination is named as the reason, not a generic slug clash") -} - -func TestPlan_ASlugAlreadyUsedInAnotherWingIsAnException(t *testing.T) { - // Graph identity is the slug alone; two pages with one slug would merge. - dir := t.TempDir() - page(t, dir, "wiki/sources/talk-a.md", src("Talk A", "ALPHA")) - page(t, dir, "wiki/beta/facts/talk-a.md", "a beta page with the same slug") - - items, err := migrate.BuildPlan(context.Background(), dir, resolverFor) - require.NoError(t, err) - it := byFrom(items)["wiki/sources/talk-a.md"] - assert.Equal(t, migrate.Exception, it.Status) - assert.Contains(t, it.Reason, "slug") -} - -func TestApply_MovesOnlyAutoItemsAndStampsFrontmatterWithoutTouchingTheBody(t *testing.T) { - dir := t.TempDir() - body := src("Talk A", "ALPHA") + "\nSee [[jev|Jev]] and [[unknown-slug]].\n" - page(t, dir, "wiki/sources/talk-a.md", body) - page(t, dir, "wiki/sources/tunnel-candidates-2026-06-14.md", src("t", "ALPHA")) - page(t, dir, "wiki/entities/jev.md", entity("Jev", "talk-a")) - - items, err := migrate.BuildPlan(context.Background(), dir, resolverFor) - require.NoError(t, err) - n, err := migrate.Apply(dir, items) - require.NoError(t, err) - assert.Equal(t, 2, n) - - moved, err := os.ReadFile(filepath.Join(dir, "wiki/alpha/sources/talk-a.md")) - require.NoError(t, err) - assert.Contains(t, string(moved), "wing: alpha") - assert.Contains(t, string(moved), "hall: sources") - assert.Contains(t, string(moved), "See [[jev|Jev]] and [[unknown-slug]].", "links and body are byte-preserved") - - ent, err := os.ReadFile(filepath.Join(dir, "wiki/alpha/facts/jev.md")) - require.NoError(t, err) - assert.Contains(t, string(ent), "kind: entity") - - _, err = os.Stat(filepath.Join(dir, "wiki/sources/talk-a.md")) - assert.ErrorIs(t, err, os.ErrNotExist, "the original is removed after the copy is written") - _, err = os.Stat(filepath.Join(dir, "wiki/sources/tunnel-candidates-2026-06-14.md")) - assert.NoError(t, err, "an exception is left exactly where it is") -} - -func TestApply_NeverOverwritesAndKeepsTheSourceWhenTheDestinationAppearsLate(t *testing.T) { - dir := t.TempDir() - page(t, dir, "wiki/sources/talk-a.md", src("Talk A", "ALPHA")) - items, err := migrate.BuildPlan(context.Background(), dir, resolverFor) - require.NoError(t, err) - page(t, dir, "wiki/alpha/sources/talk-a.md", "appeared after the plan") - - n, err := migrate.Apply(dir, items) - require.Error(t, err) - assert.Zero(t, n) - got, _ := os.ReadFile(filepath.Join(dir, "wiki/alpha/sources/talk-a.md")) - assert.Equal(t, "appeared after the plan", string(got)) - _, statErr := os.Stat(filepath.Join(dir, "wiki/sources/talk-a.md")) - assert.NoError(t, statErr, "the source survives a failed write") -} - -func TestPlan_IsEmptyOnceEverythingIsMigrated(t *testing.T) { - dir := t.TempDir() - page(t, dir, "wiki/sources/talk-a.md", src("Talk A", "ALPHA")) - items, err := migrate.BuildPlan(context.Background(), dir, resolverFor) - require.NoError(t, err) - _, err = migrate.Apply(dir, items) - require.NoError(t, err) - - again, err := migrate.BuildPlan(context.Background(), dir, resolverFor) - require.NoError(t, err) - assert.Empty(t, again) -} - -// An exception that blocked on a collision still carries a destination; Apply -// must treat Status, not the presence of a path, as the decision. -func TestApply_LeavesACollisionExceptionAloneEvenThoughItHasADestination(t *testing.T) { - dir := t.TempDir() - page(t, dir, "wiki/sources/talk-a.md", src("Talk A", "ALPHA")) - page(t, dir, "wiki/alpha/sources/talk-a.md", "occupied") - items, err := migrate.BuildPlan(context.Background(), dir, resolverFor) - require.NoError(t, err) - require.Equal(t, migrate.Exception, byFrom(items)["wiki/sources/talk-a.md"].Status) - - n, err := migrate.Apply(dir, items) - require.NoError(t, err) - assert.Zero(t, n) - _, statErr := os.Stat(filepath.Join(dir, "wiki/sources/talk-a.md")) - assert.NoError(t, statErr) -} diff --git a/ingestion/internal/watcher/watcher_wing_test.go b/ingestion/internal/watcher/watcher_wing_test.go index 8a17f4c..3d6f717 100644 --- a/ingestion/internal/watcher/watcher_wing_test.go +++ b/ingestion/internal/watcher/watcher_wing_test.go @@ -3,9 +3,9 @@ package watcher import ( "context" "encoding/json" - "errors" "os" "path/filepath" + "regexp" "strings" "sync/atomic" "testing" @@ -17,37 +17,11 @@ import ( "github.com/mathiasbq/hyperguild/ingestion/internal/wing" ) -type stubClassifier struct { - s wing.Suggestion - err error - calls atomic.Int32 -} - -func (c *stubClassifier) Classify(context.Context, string, []string) (wing.Suggestion, error) { - c.calls.Add(1) - return c.s, c.err -} - -type stubScorer map[string]float64 - -func (s stubScorer) Score(string) map[string]float64 { return s } - -func wingResolver(c wing.Classifier, s wing.Scorer) *wing.Resolver { - return &wing.Resolver{ - Known: []string{"radar", "homelab", "client-x"}, - Eligible: func(w string) bool { return w != "client-x" }, - Classifier: c, - Scorer: s, - Threshold: 0.8, - } -} - -func wingTestCfg(t *testing.T, r wing.Placer, llmCalls *atomic.Int32) (Config, string) { +func wingTestCfg(t *testing.T, mk func(brainDir string) wing.Placer, llmCalls *atomic.Int32) (Config, string) { t.Helper() brainDir := setupBrainDir(t) - return Config{ + cfg := Config{ BrainDir: brainDir, - Resolver: r, Pipeline: pipeline.Config{ Schema: "# Schema", Complete: func(ctx context.Context, s, u string) (string, error) { @@ -55,7 +29,21 @@ func wingTestCfg(t *testing.T, r wing.Placer, llmCalls *atomic.Int32) (Config, s return successComplete(pipeline.RawPage{Title: "Talk", Type: "source", Subtype: "video", Content: "## Summary\n\nx\n"})(ctx, s, u) }, }, - }, brainDir + } + if mk != nil { + cfg.Resolver = mk(brainDir) + } + return cfg, brainDir +} + +// router is the production Router with a client-content marker. +func router(brainDir string) wing.Placer { + return wing.Router{BrainDir: brainDir, Default: "web-clip", ClientBlock: regexp.MustCompile(`(?i)acme-client`)} +} + +func makeWing(t *testing.T, brainDir, name string) { + t.Helper() + require.NoError(t, os.MkdirAll(filepath.Join(brainDir, "wiki", name, "facts"), 0o755)) } func put(t *testing.T, brainDir, rel, body string) { @@ -70,70 +58,91 @@ func has(brainDir, rel string) bool { return err == nil } +func mustJSONPages(pages []pipeline.RawPage) string { + b, err := json.Marshal(pages) + if err != nil { + panic(err) + } + return string(b) +} + +func parkedBody(t *testing.T, brainDir, name string) string { + t.Helper() + b, err := os.ReadFile(filepath.Join(brainDir, "raw", "needs-review", name)) + require.NoError(t, err) + return string(b) +} + +func linesStartingWith(s, prefix string) int { + n := 0 + for _, l := range strings.Split(s, "\n") { + if strings.HasPrefix(l, prefix) { + n++ + } + } + return n +} + func TestProcessDir_ASubfolderNamesTheWing(t *testing.T) { var llmCalls atomic.Int32 - clf := &stubClassifier{} - cfg, brainDir := wingTestCfg(t, wingResolver(clf, stubScorer{}), &llmCalls) + cfg, brainDir := wingTestCfg(t, router, &llmCalls) + makeWing(t, brainDir, "radar") put(t, brainDir, "radar/dark-factory.md", "A talk about factories.") - errs := processDir(context.Background(), cfg, "2026-10-20") + require.Empty(t, processDir(context.Background(), cfg, "2026-10-20")) - require.Empty(t, errs) assert.True(t, has(brainDir, "wiki/radar/sources/dark-factory.md"), "lands in the named wing") assert.False(t, has(brainDir, "raw/radar/dark-factory.md"), "inbox zero holds for subfolders too") - assert.Zero(t, clf.calls.Load(), "an explicit wing costs no classification") } -func TestProcessDir_ANoteTheResolverCannotPlaceIsParkedNotIngested(t *testing.T) { +func TestProcessDir_ADeliberateSubfolderIntoAnyExistingWingPlaces(t *testing.T) { var llmCalls atomic.Int32 - clf := &stubClassifier{s: wing.Suggestion{Wing: "radar", Confidence: 0.5, Reason: "maybe"}} - cfg, brainDir := wingTestCfg(t, wingResolver(clf, stubScorer{"radar": 3}), &llmCalls) - put(t, brainDir, "unsure.md", "Some note.") + cfg, brainDir := wingTestCfg(t, router, &llmCalls) + makeWing(t, brainDir, "rikatillsammans") + put(t, brainDir, "rikatillsammans/meeting.md", "Notes the owner put there on purpose.") - errs := processDir(context.Background(), cfg, "2026-10-20") + require.Empty(t, processDir(context.Background(), cfg, "2026-10-20")) + + assert.True(t, has(brainDir, "wiki/rikatillsammans/sources/meeting.md"), + "a folder is the owner's deliberate choice; extraction runs on the local model") +} + +func TestProcessDir_ANoteTheRouterCannotPlaceIsParkedNotIngested(t *testing.T) { + var llmCalls atomic.Int32 + cfg, brainDir := wingTestCfg(t, router, &llmCalls) + put(t, brainDir, "kickoff.md", "Some note about the ACME-CLIENT kickoff.") + + require.Empty(t, processDir(context.Background(), cfg, "2026-10-20")) - require.Empty(t, errs) assert.Zero(t, llmCalls.Load(), "no extraction runs for an unplaced note") - assert.False(t, has(brainDir, "raw/unsure.md"), "leaves the top of raw/") - parked, err := os.ReadFile(filepath.Join(brainDir, "raw", "needs-review", "unsure.md")) - require.NoError(t, err) - assert.Contains(t, string(parked), "proposed_wing: radar") - assert.Contains(t, string(parked), "below threshold") - assert.Contains(t, string(parked), "Some note.", "the note body is preserved") + assert.False(t, has(brainDir, "raw/kickoff.md"), "leaves the top of raw/") + parked := parkedBody(t, brainDir, "kickoff.md") + assert.Contains(t, parked, "proposal_reason:") + assert.Contains(t, parked, "client-content marker") + assert.Contains(t, parked, "ACME-CLIENT kickoff", "the note body is preserved") } -func TestProcessDir_ANoteInAClientWingFolderIsParked(t *testing.T) { +func TestProcessDir_ATypoedWingFolderIsParkedAndNeverCreated(t *testing.T) { var llmCalls atomic.Int32 - cfg, brainDir := wingTestCfg(t, wingResolver(&stubClassifier{}, stubScorer{}), &llmCalls) - put(t, brainDir, "client-x/secret.md", "Client material.") + cfg, brainDir := wingTestCfg(t, router, &llmCalls) + makeWing(t, brainDir, "homelab") + put(t, brainDir, "hoemlab/n.md", "Note.") require.Empty(t, processDir(context.Background(), cfg, "2026-10-20")) - assert.Zero(t, llmCalls.Load()) - assert.False(t, has(brainDir, "wiki/client-x"), "nothing is ever written into an ineligible wing") - assert.True(t, has(brainDir, "raw/needs-review/secret.md")) -} - -func TestProcessDir_ClassifierFailureParksTheNote(t *testing.T) { - var llmCalls atomic.Int32 - cfg, brainDir := wingTestCfg(t, wingResolver(&stubClassifier{err: errors.New("llm down")}, stubScorer{}), &llmCalls) - put(t, brainDir, "n.md", "Note.") - - require.Empty(t, processDir(context.Background(), cfg, "2026-10-20")) - - assert.Zero(t, llmCalls.Load()) + assert.False(t, has(brainDir, "wiki/hoemlab")) assert.True(t, has(brainDir, "raw/needs-review/n.md")) + assert.Zero(t, llmCalls.Load()) } func TestProcessDir_ParkedNotesAreNotReprocessed(t *testing.T) { var llmCalls atomic.Int32 - clf := &stubClassifier{s: wing.Suggestion{Wing: "radar", Confidence: 0.99}} - cfg, brainDir := wingTestCfg(t, wingResolver(clf, stubScorer{"radar": 3}), &llmCalls) + cfg, brainDir := wingTestCfg(t, router, &llmCalls) put(t, brainDir, "needs-review/waiting.md", "---\nproposed_wing: radar\n---\nA parked note.") require.Empty(t, processDir(context.Background(), cfg, "2026-10-20")) - assert.Zero(t, clf.calls.Load(), "needs-review/ is a human queue, like rejected/") + assert.Zero(t, llmCalls.Load(), "needs-review/ is a human queue, like rejected/") assert.True(t, has(brainDir, "raw/needs-review/waiting.md")) } @@ -147,141 +156,68 @@ func TestProcessDir_WithoutAResolverTheRetiredLayoutIsUnchanged(t *testing.T) { assert.True(t, has(brainDir, "wiki/sources/plain.md")) } -// The reason comes from an LLM and ends up in frontmatter: a newline in it must -// not be able to start a new key (PR #147: a title once forged `origin:`). -func TestProcessDir_AProposalReasonCannotForgeFrontmatterKeys(t *testing.T) { +// The reason comes from file and folder names and ends up in frontmatter: a +// newline in it must not be able to start a new key (PR #147: a title once +// forged `origin:`). +func TestProcessDir_AFolderNameCannotForgeFrontmatterKeys(t *testing.T) { var llmCalls atomic.Int32 - // A classifier error is the path by which model text reaches the reason. - clf := &stubClassifier{err: errors.New("bad reply\norigin: trusted\nwing: homelab")} - cfg, brainDir := wingTestCfg(t, wingResolver(clf, stubScorer{}), &llmCalls) - put(t, brainDir, "forge.md", "Note.") + cfg, brainDir := wingTestCfg(t, router, &llmCalls) + put(t, brainDir, "radar\nproposal_confidence: 0.99\nwing: homelab/forge.md", "Note.") require.Empty(t, processDir(context.Background(), cfg, "2026-10-20")) - parked, err := os.ReadFile(filepath.Join(brainDir, "raw", "needs-review", "forge.md")) + entries, err := os.ReadDir(filepath.Join(brainDir, "raw", "needs-review")) require.NoError(t, err) - assert.NotContains(t, string(parked), "\norigin: trusted", "no injected key at line start") - assert.NotContains(t, string(parked), "\nwing: homelab") + require.Len(t, entries, 1) + b := parkedBody(t, brainDir, entries[0].Name()) + assert.Equal(t, 1, linesStartingWith(b, "proposal_confidence:"), "exactly one confidence key at line start") + assert.Equal(t, 0, linesStartingWith(b, "wing:"), "no injected wing key") + assert.Equal(t, 0, linesStartingWith(b, "proposed_wing:"), "a non-plain wing name is never a proposal") } -// Syncthing can bring a removed note back (an offline peer, a restored backup). -// The source-hash drift check must keep that from costing a second extraction. -func TestProcessDir_ANoteThatReappearsUnchangedIsNotExtractedAgain(t *testing.T) { +func TestProcessDir_ANotesOwnProposalKeysDoNotOutrankTheRouters(t *testing.T) { var llmCalls atomic.Int32 - cfg, brainDir := wingTestCfg(t, wingResolver(&stubClassifier{}, stubScorer{}), &llmCalls) - - put(t, brainDir, "radar/talk.md", "The same talk, byte for byte.") - require.Empty(t, processDir(context.Background(), cfg, "2026-10-20")) - require.Equal(t, int32(1), llmCalls.Load(), "positive control: the first drop is extracted") - require.False(t, has(brainDir, "raw/radar/talk.md")) - - put(t, brainDir, "radar/talk.md", "The same talk, byte for byte.") // the peer sends it back - require.Empty(t, processDir(context.Background(), cfg, "2026-10-21")) - - assert.Equal(t, int32(1), llmCalls.Load(), "an unchanged source is not extracted twice") - assert.False(t, has(brainDir, "raw/radar/talk.md"), "and inbox zero still holds") -} - -func TestProcessDir_ANoteThatReappearsChangedIsExtractedAgain(t *testing.T) { - var llmCalls atomic.Int32 - cfg, brainDir := wingTestCfg(t, wingResolver(&stubClassifier{}, stubScorer{}), &llmCalls) - - put(t, brainDir, "radar/talk.md", "Version one.") - require.Empty(t, processDir(context.Background(), cfg, "2026-10-20")) - put(t, brainDir, "radar/talk.md", "Version two, edited.") - require.Empty(t, processDir(context.Background(), cfg, "2026-10-21")) - - assert.Equal(t, int32(2), llmCalls.Load(), "a real edit must still be picked up") -} - -// The parked copy is the only copy once the original is retired, so two notes -// that share a name must both survive. -func TestProcessDir_SameNamedNotesAreAllParkedNoneOverwritten(t *testing.T) { - var llmCalls atomic.Int32 - cfg, brainDir := wingTestCfg(t, wingResolver(&stubClassifier{}, stubScorer{}), &llmCalls) - for i, body := range []string{"ONE", "TWO", "THREE", "FOUR"} { - put(t, brainDir, filepath.Join("u"+string(rune('1'+i)), "x.md"), body) - } - // Unknown subfolders: each is an exception, all named x.md. - require.Empty(t, processDir(context.Background(), cfg, "2026-10-20")) - - files, err := filepath.Glob(filepath.Join(brainDir, "raw", "needs-review", "*.md")) - require.NoError(t, err) - var all string - for _, f := range files { - b, _ := os.ReadFile(f) - all += string(b) - } - for _, body := range []string{"ONE", "TWO", "THREE", "FOUR"} { - assert.Contains(t, all, body, "note %s must not have been overwritten", body) - } - assert.Len(t, files, 4) -} - -// proposed_wing comes from a model reply or a folder name. It must never be -// able to start another key, and the note's own proposal keys must not outrank -// the resolver's (frontmatter.get is first-wins). -func TestProcessDir_AHostileWingCannotForgeTheProposal(t *testing.T) { - var llmCalls atomic.Int32 - hostile := "radar\nproposal_confidence: 0.99\nproposal_reason: \"approved by operator\"" - clf := &stubClassifier{s: wing.Suggestion{Wing: hostile, Confidence: 1}} - cfg, brainDir := wingTestCfg(t, wingResolver(clf, stubScorer{"radar": 3}), &llmCalls) - put(t, brainDir, "n.md", "Note.") + cfg, brainDir := wingTestCfg(t, router, &llmCalls) + put(t, brainDir, "n.md", "---\nproposed_wing: homelab\nproposal_confidence: 1.00\nproposal_reason: \"self-approved\"\n---\nACME-CLIENT body.") require.Empty(t, processDir(context.Background(), cfg, "2026-10-20")) - b, err := os.ReadFile(filepath.Join(brainDir, "raw", "needs-review", "n.md")) - require.NoError(t, err) - assert.NotContains(t, string(b), "\nproposal_confidence: 0.99") - assert.NotContains(t, string(b), "approved by operator\"\n") - assert.Equal(t, 1, linesStartingWith(string(b), "proposal_confidence:"), "exactly one confidence key at line start") -} - -func TestProcessDir_ANotesOwnProposalKeysDoNotOutrankTheResolvers(t *testing.T) { - var llmCalls atomic.Int32 - cfg, brainDir := wingTestCfg(t, wingResolver(&stubClassifier{}, stubScorer{}), &llmCalls) - put(t, brainDir, "n.md", "---\nproposed_wing: homelab\nproposal_confidence: 1.00\nproposal_reason: \"self-approved\"\n---\nBody.") - - require.Empty(t, processDir(context.Background(), cfg, "2026-10-20")) - - b, _ := os.ReadFile(filepath.Join(brainDir, "raw", "needs-review", "n.md")) - assert.NotContains(t, string(b), "self-approved") - assert.Equal(t, 1, linesStartingWith(string(b), "proposal_reason:")) + b := parkedBody(t, brainDir, "n.md") + assert.NotContains(t, b, "self-approved") + assert.Equal(t, 1, linesStartingWith(b, "proposal_reason:")) + assert.Equal(t, 0, linesStartingWith(b, "proposed_wing:")) } func TestProcessDir_ACRLFNoteWithAWingKeyIsStillRead(t *testing.T) { var llmCalls atomic.Int32 - clf := &stubClassifier{} - cfg, brainDir := wingTestCfg(t, wingResolver(clf, stubScorer{}), &llmCalls) + cfg, brainDir := wingTestCfg(t, router, &llmCalls) + makeWing(t, brainDir, "radar") put(t, brainDir, "n.md", "---\r\nwing: radar\r\n---\r\nBody.\r\n") require.Empty(t, processDir(context.Background(), cfg, "2026-10-20")) assert.True(t, has(brainDir, "wiki/radar/sources/n.md"), "the declared wing is honoured") - assert.Zero(t, clf.calls.Load()) } -// linesStartingWith counts frontmatter-style keys: a key quoted inside another -// value is not at the start of a line and is not a key. -func linesStartingWith(s, prefix string) int { - n := 0 - for _, l := range strings.Split(s, "\n") { - if strings.HasPrefix(l, prefix) { - n++ - } - } - return n +// editingPlacer rewrites the note while the placement decision is being made, +// then parks it: the realistic window is a slow decision with Syncthing +// delivering a newer version meanwhile. +type editingPlacer struct{ edit func() } + +func (e editingPlacer) Resolve(context.Context, wing.Note) wing.Decision { + e.edit() + return wing.Decision{Source: wing.Exception, Reason: "test: parked after an edit"} } -// Resolving costs an LLM call. If Syncthing delivers a newer version of the note -// meanwhile, deleting the original loses that version: the parked copy is stale. -func TestProcessDir_ANoteEditedWhileTheResolverRunsIsNotDeleted(t *testing.T) { +func TestProcessDir_ANoteEditedWhileItIsBeingPlacedIsNotDeleted(t *testing.T) { var llmCalls atomic.Int32 var notePath string - clf := &editingClassifier{} - cfg, brainDir := wingTestCfg(t, wingResolver(clf, stubScorer{}), &llmCalls) + cfg, brainDir := wingTestCfg(t, func(string) wing.Placer { + return editingPlacer{edit: func() { + require.NoError(t, os.WriteFile(notePath, []byte("NEWER and longer version"), 0o644)) + }} + }, &llmCalls) notePath = filepath.Join(brainDir, "raw", "n.md") - clf.edit = func() { require.NoError(t, os.WriteFile(notePath, []byte("NEWER and longer version"), 0o644)) } put(t, brainDir, "n.md", "ORIGINAL") require.Empty(t, processDir(context.Background(), cfg, "2026-10-20")) @@ -291,21 +227,14 @@ func TestProcessDir_ANoteEditedWhileTheResolverRunsIsNotDeleted(t *testing.T) { assert.Equal(t, "NEWER and longer version", string(got)) } -type editingClassifier struct{ edit func() } - -func (c *editingClassifier) Classify(context.Context, string, []string) (wing.Suggestion, error) { - c.edit() - return wing.Suggestion{}, nil -} - // A planted symlink at raw/needs-review must not redirect a parked note out of // the brain, nor lose it from the review queue. func TestProcessDir_AParkedNoteIsNeverWrittenThroughASymlinkedNeedsReview(t *testing.T) { var llmCalls atomic.Int32 - cfg, brainDir := wingTestCfg(t, wingResolver(&stubClassifier{}, stubScorer{}), &llmCalls) + cfg, brainDir := wingTestCfg(t, router, &llmCalls) outside := t.TempDir() require.NoError(t, os.Symlink(outside, filepath.Join(brainDir, "raw", "needs-review"))) - put(t, brainDir, "n.md", "Note.") + put(t, brainDir, "n.md", "ACME-CLIENT note.") errs := processDir(context.Background(), cfg, "2026-10-20") @@ -331,10 +260,57 @@ func TestWithProposal_ANonFenceDashLineDoesNotCloseTheBlock(t *testing.T) { assert.NotContains(t, strings.Join(lines[:closing], "\n"), "foo: bar", "the unfenced note is body, not part of the block") } -func mustJSONPages(pages []pipeline.RawPage) string { - b, err := json.Marshal(pages) - if err != nil { - panic(err) +// The parked copy is the only copy once the original is retired, so two notes +// that share a name must both survive. +func TestProcessDir_SameNamedNotesAreAllParkedNoneOverwritten(t *testing.T) { + var llmCalls atomic.Int32 + cfg, brainDir := wingTestCfg(t, router, &llmCalls) + for i, body := range []string{"ONE", "TWO", "THREE", "FOUR"} { + put(t, brainDir, filepath.Join("typo"+string(rune('1'+i)), "x.md"), body) } - return string(b) + require.Empty(t, processDir(context.Background(), cfg, "2026-10-20")) + + files, err := filepath.Glob(filepath.Join(brainDir, "raw", "needs-review", "*.md")) + require.NoError(t, err) + var all string + for _, f := range files { + b, _ := os.ReadFile(f) + all += string(b) + } + for _, body := range []string{"ONE", "TWO", "THREE", "FOUR"} { + assert.Contains(t, all, body, "note %s must not have been overwritten", body) + } + assert.Len(t, files, 4) +} + +// Syncthing can bring a removed note back (an offline peer, a restored backup). +// The source-hash drift check must keep that from costing a second extraction. +func TestProcessDir_ANoteThatReappearsUnchangedIsNotExtractedAgain(t *testing.T) { + var llmCalls atomic.Int32 + cfg, brainDir := wingTestCfg(t, router, &llmCalls) + makeWing(t, brainDir, "radar") + + put(t, brainDir, "radar/talk.md", "The same talk, byte for byte.") + require.Empty(t, processDir(context.Background(), cfg, "2026-10-20")) + require.Equal(t, int32(1), llmCalls.Load(), "positive control: the first drop is extracted") + require.False(t, has(brainDir, "raw/radar/talk.md")) + + put(t, brainDir, "radar/talk.md", "The same talk, byte for byte.") // the peer sends it back + require.Empty(t, processDir(context.Background(), cfg, "2026-10-21")) + + assert.Equal(t, int32(1), llmCalls.Load(), "an unchanged source is not extracted twice") + assert.False(t, has(brainDir, "raw/radar/talk.md"), "and inbox zero still holds") +} + +func TestProcessDir_ANoteThatReappearsChangedIsExtractedAgain(t *testing.T) { + var llmCalls atomic.Int32 + cfg, brainDir := wingTestCfg(t, router, &llmCalls) + makeWing(t, brainDir, "radar") + + put(t, brainDir, "radar/talk.md", "Version one.") + require.Empty(t, processDir(context.Background(), cfg, "2026-10-20")) + put(t, brainDir, "radar/talk.md", "Version two, edited.") + require.Empty(t, processDir(context.Background(), cfg, "2026-10-21")) + + assert.Equal(t, int32(2), llmCalls.Load(), "a real edit must still be picked up") } diff --git a/ingestion/internal/wing/backtest.go b/ingestion/internal/wing/backtest.go deleted file mode 100644 index f334251..0000000 --- a/ingestion/internal/wing/backtest.go +++ /dev/null @@ -1,43 +0,0 @@ -package wing - -// Sample is one backtest observation: the human-decided wing of an existing -// page, and what the two independent opinions said about it with the wing -// label removed. -type Sample struct { - Truth string - Suggested string // classifier's wing, "" if it abstained or failed - Confidence float64 - ScorerTop string // scorer's single best wing, "" if no clear opinion -} - -// Result is the policy outcome at one threshold. -type Result struct { - N int - Auto int // placed without a human - Correct int // of Auto, matching Truth - Coverage float64 // Auto / N - Precision float64 // Correct / Auto; 0 when Auto == 0 -} - -// Evaluate applies the Resolver's classified-path rule (confident, and the -// scorer agrees) to recorded samples. Eligibility is not modelled: callers -// backtest only on wings they would allow. -func Evaluate(samples []Sample, threshold float64) Result { - r := Result{N: len(samples)} - for _, s := range samples { - if s.Suggested == "" || s.Confidence < threshold || s.ScorerTop != s.Suggested { - continue - } - r.Auto++ - if s.Suggested == s.Truth { - r.Correct++ - } - } - if r.N > 0 { - r.Coverage = float64(r.Auto) / float64(r.N) - } - if r.Auto > 0 { - r.Precision = float64(r.Correct) / float64(r.Auto) - } - return r -} diff --git a/ingestion/internal/wing/backtest_test.go b/ingestion/internal/wing/backtest_test.go deleted file mode 100644 index 88ebc90..0000000 --- a/ingestion/internal/wing/backtest_test.go +++ /dev/null @@ -1,38 +0,0 @@ -package wing_test - -import ( - "testing" - - "github.com/stretchr/testify/assert" - - "github.com/mathiasbq/hyperguild/ingestion/internal/wing" -) - -func TestEvaluate_PolicyAtAThreshold(t *testing.T) { - samples := []wing.Sample{ - {Truth: "a", Suggested: "a", Confidence: 0.9, ScorerTop: "a"}, // auto, correct - {Truth: "a", Suggested: "b", Confidence: 0.9, ScorerTop: "b"}, // auto, WRONG (both models agree on a wrong wing) - {Truth: "a", Suggested: "a", Confidence: 0.5, ScorerTop: "a"}, // below threshold -> exception - {Truth: "a", Suggested: "a", Confidence: 0.95, ScorerTop: "b"}, // scorer disagrees -> exception - {Truth: "a", Suggested: "", Confidence: 0.99, ScorerTop: "a"}, // abstain -> exception - } - r := wing.Evaluate(samples, 0.8) - assert.Equal(t, 5, r.N) - assert.Equal(t, 2, r.Auto) - assert.Equal(t, 1, r.Correct) - assert.InDelta(t, 0.4, r.Coverage, 1e-9) - assert.InDelta(t, 0.5, r.Precision, 1e-9) -} - -func TestEvaluate_NothingAutoHasZeroPrecisionNotNaN(t *testing.T) { - r := wing.Evaluate([]wing.Sample{{Truth: "a", Suggested: "a", Confidence: 0.1, ScorerTop: "a"}}, 0.8) - assert.Zero(t, r.Auto) - assert.Zero(t, r.Precision) - assert.Zero(t, r.Coverage) -} - -func TestEvaluate_EmptyInput(t *testing.T) { - r := wing.Evaluate(nil, 0.8) - assert.Zero(t, r.N) - assert.Zero(t, r.Coverage) -} diff --git a/ingestion/internal/wing/bm25scorer.go b/ingestion/internal/wing/bm25scorer.go deleted file mode 100644 index fedd93b..0000000 --- a/ingestion/internal/wing/bm25scorer.go +++ /dev/null @@ -1,85 +0,0 @@ -package wing - -import ( - "sort" - "strings" - "unicode" - - "github.com/mathiasbq/hyperguild/ingestion/internal/search" -) - -const ( - maxQueryTerms = 12 - minTermLen = 5 -) - -// BM25Scorer scores a note against each wing by running the brain's own BM25 -// search scoped to that wing. It is deliberately a different mechanism from -// the LLM, so agreement between the two is evidence and not an echo. -type BM25Scorer struct { - brainDir string - wings []string - exclude string -} - -// NewBM25Scorer scores against wings under brainDir. exclude is a brain- -// relative path ignored in results (the backtest sample itself); "" for none. -func NewBM25Scorer(brainDir string, wings []string, exclude string) *BM25Scorer { - return &BM25Scorer{brainDir: brainDir, wings: wings, exclude: exclude} -} - -// Score implements Scorer. A wing whose search fails scores 0 rather than -// failing the note: the Resolver treats "no clear opinion" as an Exception. -func (b *BM25Scorer) Score(text string) map[string]float64 { - query := strings.Join(topTerms(text), " ") - out := map[string]float64{} - if query == "" { - return out - } - for _, w := range b.wings { - hits, err := search.Query(b.brainDir, search.QueryOptions{Query: query, Wing: w, Limit: 2}) - if err != nil { - continue - } - for _, h := range hits { - if b.exclude != "" && h.Path == b.exclude { - continue - } - // Hits come best-first: a wing is scored by its single best - // page, so a big wing cannot win on volume alone. - out[w] = float64(h.Score) - break - } - } - return out -} - -// topTerms picks the most frequent longer words, ties broken alphabetically -// so the query is deterministic. -func topTerms(text string) []string { - if len(text) > maxNoteChars { - text = text[:maxNoteChars] - } - counts := map[string]int{} - for _, f := range strings.FieldsFunc(strings.ToLower(text), func(r rune) bool { - return !unicode.IsLetter(r) && !unicode.IsDigit(r) - }) { - if len([]rune(f)) >= minTermLen { - counts[f]++ - } - } - terms := make([]string, 0, len(counts)) - for t := range counts { - terms = append(terms, t) - } - sort.Slice(terms, func(i, j int) bool { - if counts[terms[i]] != counts[terms[j]] { - return counts[terms[i]] > counts[terms[j]] - } - return terms[i] < terms[j] - }) - if len(terms) > maxQueryTerms { - terms = terms[:maxQueryTerms] - } - return terms -} diff --git a/ingestion/internal/wing/bm25scorer_test.go b/ingestion/internal/wing/bm25scorer_test.go deleted file mode 100644 index b6c8901..0000000 --- a/ingestion/internal/wing/bm25scorer_test.go +++ /dev/null @@ -1,61 +0,0 @@ -package wing_test - -import ( - "os" - "path/filepath" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/mathiasbq/hyperguild/ingestion/internal/wing" -) - -func writePage(t *testing.T, dir, rel, body string) { - t.Helper() - full := filepath.Join(dir, rel) - require.NoError(t, os.MkdirAll(filepath.Dir(full), 0o755)) - require.NoError(t, os.WriteFile(full, []byte(body), 0o644)) -} - -func scorerBrain(t *testing.T) string { - dir := t.TempDir() - writePage(t, dir, "wiki/infra/facts/k3s.md", "---\ntitle: K3s cluster\n---\nkubernetes cluster nodes scheduling pods deployment\n") - writePage(t, dir, "wiki/finance/facts/invoice.md", "---\ntitle: Invoices\n---\ninvoice payment bookkeeping ledger reconciliation\n") - return dir -} - -func TestBM25Scorer_RanksTheWingWhosePagesMatch(t *testing.T) { - s := wing.NewBM25Scorer(scorerBrain(t), []string{"infra", "finance"}, "") - got := s.Score("notes about kubernetes cluster scheduling and pods") - assert.Greater(t, got["infra"], got["finance"]) - assert.Greater(t, got["infra"], 0.0) -} - -func TestBM25Scorer_ExcludesThePageBeingClassified(t *testing.T) { - // A page must not vouch for its own wing when it is the backtest sample. - s := wing.NewBM25Scorer(scorerBrain(t), []string{"infra", "finance"}, "wiki/infra/facts/k3s.md") - got := s.Score("kubernetes cluster nodes scheduling pods deployment") - assert.Zero(t, got["infra"], "the excluded page is the only infra match") -} - -func TestBM25Scorer_NoUsableTermsScoresNothing(t *testing.T) { - s := wing.NewBM25Scorer(scorerBrain(t), []string{"infra", "finance"}, "") - assert.Empty(t, s.Score("a an to of")) -} - -// A big wing must not win just by having more pages that brush the query: a -// wing is scored by its single best page, not by the sum of its top hits. -func TestBM25Scorer_ABigWingDoesNotWinOnVolume(t *testing.T) { - dir := t.TempDir() - writePage(t, dir, "wiki/small/facts/target.md", "---\ntitle: Target\n---\nkubernetes cluster scheduling pods deployment nodes\n") - for i := 0; i < 8; i++ { - writePage(t, dir, filepath.Join("wiki/big/facts", "p"+string(rune('a'+i))+".md"), - "---\ntitle: Filler\n---\nkubernetes cluster scheduling mentioned in passing among bookkeeping ledger entries\n") - } - s := wing.NewBM25Scorer(dir, []string{"small", "big"}, "") - got := s.Score("kubernetes cluster scheduling pods deployment nodes") - top, ok := wing.TopWing(got) - assert.True(t, ok) - assert.Equal(t, "small", top) -} diff --git a/ingestion/internal/wing/configured.go b/ingestion/internal/wing/configured.go deleted file mode 100644 index c27236c..0000000 --- a/ingestion/internal/wing/configured.go +++ /dev/null @@ -1,32 +0,0 @@ -package wing - -import ( - "fmt" - - "github.com/mathiasbq/hyperguild/ingestion/internal/classification" -) - -// NewConfiguredResolver builds the production Resolver for a brain: Known from -// the wiki, Eligible from classification.yaml (a wing is auto-assignable only -// when it classifies below Confidential, so an untagged wing never is), the -// LLM classifier over complete, and BM25 over the known wings as the scorer. -// -// Known is read once. A wing created later is unknown to this Resolver until -// the process restarts, which fails safe: notes aimed at it are parked. -func NewConfiguredResolver(brainDir string, complete CompleteFunc, threshold float64) (*Resolver, error) { - known, err := KnownWings(brainDir) - if err != nil { - return nil, err - } - cls, err := classification.Load(brainDir) - if err != nil { - return nil, fmt.Errorf("load classification: %w", err) - } - return &Resolver{ - Known: known, - Eligible: func(w string) bool { return cls.Wing(w) < classification.Confidential }, - Classifier: NewLLMClassifier(complete), - Scorer: NewBM25Scorer(brainDir, known, ""), - Threshold: threshold, - }, nil -} diff --git a/ingestion/internal/wing/configured_test.go b/ingestion/internal/wing/configured_test.go deleted file mode 100644 index 16173ef..0000000 --- a/ingestion/internal/wing/configured_test.go +++ /dev/null @@ -1,47 +0,0 @@ -package wing_test - -import ( - "context" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/mathiasbq/hyperguild/ingestion/internal/wing" -) - -func configuredBrain(t *testing.T, classification string) string { - dir := t.TempDir() - writePage(t, dir, "wiki/homelab/facts/a.md", "x") - writePage(t, dir, "wiki/radar/facts/b.md", "x") - writePage(t, dir, "wiki/client-x/facts/c.md", "x") - if classification != "" { - writePage(t, dir, "classification.yaml", classification) - } - return dir -} - -func TestNewConfiguredResolver_EligibilityComesFromClassification(t *testing.T) { - dir := configuredBrain(t, "wings:\n radar: internal\n") - r, err := wing.NewConfiguredResolver(dir, func(context.Context, string, string) (string, error) { return "", nil }, 0.9) - require.NoError(t, err) - - assert.ElementsMatch(t, []string{"client-x", "homelab", "radar"}, r.Known) - assert.True(t, r.Eligible("radar"), "tagged internal") - assert.True(t, r.Eligible("homelab"), "internal by built-in default") - assert.False(t, r.Eligible("client-x"), "an untagged wing fails safe to confidential") - assert.InDelta(t, 0.9, r.Threshold, 1e-9) -} - -func TestNewConfiguredResolver_AMalformedClassificationFileIsAnError(t *testing.T) { - dir := configuredBrain(t, "wings: [not, a, map") - _, err := wing.NewConfiguredResolver(dir, nil, 0.9) - assert.Error(t, err, "a classification source that cannot be trusted must not degrade to 'everything eligible'") -} - -func TestNewConfiguredResolver_AConfidentialTagIsNeverEligible(t *testing.T) { - dir := configuredBrain(t, "wings:\n radar: confidential\n") - r, err := wing.NewConfiguredResolver(dir, nil, 0.9) - require.NoError(t, err) - assert.False(t, r.Eligible("radar")) -} diff --git a/ingestion/internal/wing/helpers_test.go b/ingestion/internal/wing/helpers_test.go new file mode 100644 index 0000000..8ed6931 --- /dev/null +++ b/ingestion/internal/wing/helpers_test.go @@ -0,0 +1,16 @@ +package wing_test + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func writePage(t *testing.T, dir, rel, body string) { + t.Helper() + full := filepath.Join(dir, rel) + require.NoError(t, os.MkdirAll(filepath.Dir(full), 0o755)) + require.NoError(t, os.WriteFile(full, []byte(body), 0o644)) +} diff --git a/ingestion/internal/wing/llmclassifier.go b/ingestion/internal/wing/llmclassifier.go deleted file mode 100644 index 995d2a2..0000000 --- a/ingestion/internal/wing/llmclassifier.go +++ /dev/null @@ -1,99 +0,0 @@ -package wing - -import ( - "context" - "encoding/json" - "errors" - "fmt" - "strings" -) - -// CompleteFunc is a chat completion, matching llm.Client.Complete. It must be -// backed by a local model: note content is sent to it. -type CompleteFunc func(ctx context.Context, system, user string) (string, error) - -// maxNoteChars bounds what is sent for classification; the start of a note -// carries its subject, and a whole transcript is cost with no benefit. -const maxNoteChars = 6000 - -const classifySystem = `You file notes into a personal knowledge base. Pick the single wing the note belongs to, from the candidate list only. -Answer with ONLY a JSON object: {"wing": "", "confidence": <0..1>, "reason": ""}. -Use "none" when no candidate clearly fits. Do not invent wings.` - -// LLMClassifier asks a model to pick a wing. -type LLMClassifier struct{ complete CompleteFunc } - -// NewLLMClassifier wraps a chat completion as a Classifier. -func NewLLMClassifier(c CompleteFunc) *LLMClassifier { return &LLMClassifier{complete: c} } - -type classifyReply struct { - Wing string `json:"wing"` - Confidence float64 `json:"confidence"` - Reason string `json:"reason"` -} - -// Classify implements Classifier. Any reply it cannot parse is an error, which -// the Resolver turns into an Exception: an unreadable answer is not a guess. -func (l *LLMClassifier) Classify(ctx context.Context, text string, candidates []string) (Suggestion, error) { - if len(text) > maxNoteChars { - text = text[:maxNoteChars] - } - user := "Candidate wings: " + strings.Join(candidates, ", ") + "\n\nNote:\n" + text - out, err := l.complete(ctx, classifySystem, user) - if err != nil { - return Suggestion{}, fmt.Errorf("classify: %w", err) - } - raw, ok := firstJSONObject(out) - if !ok { - return Suggestion{}, errors.New("classify: reply contains no JSON object") - } - var r classifyReply - if err := json.Unmarshal([]byte(raw), &r); err != nil { - return Suggestion{}, fmt.Errorf("classify: parse reply: %w", err) - } - w := strings.TrimSpace(r.Wing) - if strings.EqualFold(w, "none") { - w = "" - } - return Suggestion{Wing: w, Confidence: clamp01(r.Confidence), Reason: r.Reason}, nil -} - -// firstJSONObject returns the first balanced {...} in s, ignoring braces -// inside JSON strings, so prose or code fences around the reply are tolerated. -func firstJSONObject(s string) (string, bool) { - start := strings.IndexByte(s, '{') - if start < 0 { - return "", false - } - depth, inStr, esc := 0, false, false - for i := start; i < len(s); i++ { - c := s[i] - switch { - case esc: - esc = false - case inStr && c == '\\': - esc = true - case c == '"': - inStr = !inStr - case inStr: - case c == '{': - depth++ - case c == '}': - depth-- - if depth == 0 { - return s[start : i+1], true - } - } - } - return "", false -} - -func clamp01(f float64) float64 { - switch { - case f < 0: - return 0 - case f > 1: - return 1 - } - return f -} diff --git a/ingestion/internal/wing/llmclassifier_test.go b/ingestion/internal/wing/llmclassifier_test.go deleted file mode 100644 index 4f4a73b..0000000 --- a/ingestion/internal/wing/llmclassifier_test.go +++ /dev/null @@ -1,67 +0,0 @@ -package wing_test - -import ( - "context" - "errors" - "strings" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/mathiasbq/hyperguild/ingestion/internal/wing" -) - -func completes(reply string, err error) wing.CompleteFunc { - return func(_ context.Context, _, _ string) (string, error) { return reply, err } -} - -func TestLLMClassifier_ParsesTheReply(t *testing.T) { - tests := []struct { - name string - reply string - want wing.Suggestion - wantErr bool - }{ - {"bare json", `{"wing":"tapir","confidence":0.9,"reason":"video summariser"}`, wing.Suggestion{Wing: "tapir", Confidence: 0.9, Reason: "video summariser"}, false}, - {"json inside prose and fences", "Sure!\n```json\n{\"wing\":\"homelab\",\"confidence\":0.7,\"reason\":\"k3s\"}\n```\nDone.", wing.Suggestion{Wing: "homelab", Confidence: 0.7, Reason: "k3s"}, false}, - {"none means abstain", `{"wing":"none","confidence":0.2,"reason":"generic"}`, wing.Suggestion{Wing: "", Confidence: 0.2, Reason: "generic"}, false}, - {"confidence above 1 is clamped", `{"wing":"tapir","confidence":7,"reason":"x"}`, wing.Suggestion{Wing: "tapir", Confidence: 1, Reason: "x"}, false}, - {"negative confidence is clamped", `{"wing":"tapir","confidence":-1,"reason":"x"}`, wing.Suggestion{Wing: "tapir", Confidence: 0, Reason: "x"}, false}, - {"empty reply (a reasoning model that ran out of tokens)", ``, wing.Suggestion{}, true}, - {"no json at all", `I think it is tapir`, wing.Suggestion{}, true}, - {"malformed json", `{"wing":"tapir","confidence":`, wing.Suggestion{}, true}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - c := wing.NewLLMClassifier(completes(tt.reply, nil)) - got, err := c.Classify(context.Background(), "note", []string{"tapir", "homelab"}) - if tt.wantErr { - require.Error(t, err) - return - } - require.NoError(t, err) - assert.Equal(t, tt.want, got) - }) - } -} - -func TestLLMClassifier_PropagatesTheModelError(t *testing.T) { - boom := errors.New("model down") - _, err := wing.NewLLMClassifier(completes("", boom)).Classify(context.Background(), "n", []string{"tapir"}) - assert.ErrorIs(t, err, boom) -} - -func TestLLMClassifier_PromptListsCandidatesAndBoundsTheNote(t *testing.T) { - var gotUser string - c := wing.NewLLMClassifier(func(_ context.Context, _, user string) (string, error) { - gotUser = user - return `{"wing":"tapir","confidence":0.9,"reason":"x"}`, nil - }) - long := strings.Repeat("word ", 5000) - _, err := c.Classify(context.Background(), long, []string{"tapir", "homelab"}) - require.NoError(t, err) - assert.Contains(t, gotUser, "tapir") - assert.Contains(t, gotUser, "homelab") - assert.Less(t, len(gotUser), 12000, "a whole transcript must not be sent as the classification input") -} diff --git a/ingestion/internal/wing/resolve.go b/ingestion/internal/wing/resolve.go index f109f07..d4823a3 100644 --- a/ingestion/internal/wing/resolve.go +++ b/ingestion/internal/wing/resolve.go @@ -1,17 +1,17 @@ // Package wing decides which brain wing an incoming raw note belongs to -// without the human typing it (hyperguild#205). +// without the human typing it (hyperguild#205, #208). // -// The contract is conservative: a note is placed automatically only when the -// decision is explicit, or when two independent opinions (an LLM and a -// deterministic scorer) agree with enough confidence on a wing that is both -// known and eligible. Everything else is an Exception for a human to approve. -// A wing is never created here, and a wing the caller does not mark eligible -// (client wings, anything not deliberately tagged) is never auto-assigned. +// An earlier design classified each note with a local model and cross-checked +// it with a text score. Measured against pages whose wing a human had chosen, +// the model was right 60% of the time with a saturated self-reported +// confidence, and the cross-check was right 11% of the time, so the gate it +// formed placed 2 of 55 pages (hyperguild#208). The Router that replaced it +// decides with rules only: a deliberate folder or frontmatter wing, else one +// default wing; nothing it cannot decide safely is guessed. package wing import ( "context" - "fmt" "strings" ) @@ -19,38 +19,18 @@ import ( type Source string const ( - Explicit Source = "explicit" // subfolder or frontmatter named the wing - Default Source = "default" // no signal; the configured default wing - Classified Source = "classified" // classifier + scorer agreed, confident - Exception Source = "exception" // needs a human + Explicit Source = "explicit" // subfolder or frontmatter named the wing + Default Source = "default" // no signal; the configured default wing + Exception Source = "exception" // needs a human ) -// Note is what the resolver sees of an incoming raw note. +// Note is what the Placer sees of an incoming raw note. type Note struct { Dir string // first subfolder under raw/, "" when dropped at the top FrontmatterWing string // `wing:` from the note's frontmatter, "" if absent Text string } -// Suggestion is a Classifier's answer. An empty Wing means "none fits". -type Suggestion struct { - Wing string - Confidence float64 // 0..1 - Reason string -} - -// Classifier proposes a wing from the candidates. Implementations must run on -// a local model; the resolver cannot enforce that. -type Classifier interface { - Classify(ctx context.Context, text string, candidates []string) (Suggestion, error) -} - -// Scorer is an independent, deterministic opinion (e.g. BM25 against each -// wing's index). Higher is better; the key is the wing name. -type Scorer interface { - Score(text string) map[string]float64 -} - // Decision is the outcome. For an Exception, Wing holds the proposal when // there was one (so a reviewer can approve it) and is empty otherwise. type Decision struct { @@ -60,114 +40,10 @@ type Decision struct { Reason string } -// Resolver holds the policy. Known is every existing wing; Eligible reports -// whether a known wing may be assigned automatically. -type Resolver struct { - Known []string - Eligible func(wing string) bool - Classifier Classifier - Scorer Scorer - Threshold float64 -} - -// Resolve never returns an error: anything it cannot decide safely is an -// Exception with the reason spelled out. -func (r Resolver) Resolve(ctx context.Context, n Note) Decision { - dir := norm(n.Dir) - fm := norm(n.FrontmatterWing) - switch { - case dir != "" && fm != "" && dir != fm: - return exception("", 0, fmt.Sprintf("subfolder %q and frontmatter wing %q disagree", dir, fm)) - case dir != "": - return r.explicit(dir, "subfolder") - case fm != "": - return r.explicit(fm, "frontmatter") - } - return r.classify(ctx, n) -} - -func (r Resolver) explicit(w, via string) Decision { - if why := r.refuse(w); why != "" { - return exception(w, 0, fmt.Sprintf("%s names %q: %s", via, w, why)) - } - return Decision{Wing: w, Source: Explicit, Confidence: 1, Reason: "named by " + via} -} - -func (r Resolver) classify(ctx context.Context, n Note) Decision { - if r.Classifier == nil { - return exception("", 0, "no classifier configured") - } - s, err := r.Classifier.Classify(ctx, n.Text, r.Known) - if err != nil { - return exception("", 0, fmt.Sprintf("classifier failed: %v", err)) - } - w := norm(s.Wing) - if w == "" { - return exception("", s.Confidence, "classifier found no fitting wing") - } - if why := r.refuse(w); why != "" { - return exception(w, s.Confidence, fmt.Sprintf("classifier proposed %q: %s", w, why)) - } - if s.Confidence < r.Threshold { - return exception(w, s.Confidence, fmt.Sprintf("confidence %.2f below threshold %.2f", s.Confidence, r.Threshold)) - } - top, ok := r.scorerTop(n.Text) - if !ok { - return exception(w, s.Confidence, "scorer has no clear opinion to cross-check against") - } - if top != w { - return exception(w, s.Confidence, fmt.Sprintf("classifier says %q, scorer says %q", w, top)) - } - return Decision{Wing: w, Source: Classified, Confidence: s.Confidence, - Reason: fmt.Sprintf("classifier and scorer agree (%.2f): %s", s.Confidence, s.Reason)} -} - -// refuse returns why a wing may not be auto-assigned, or "" if it may. -func (r Resolver) refuse(w string) string { - if !r.known(w) { - return "not an existing wing (new wings are never created automatically)" - } - if r.Eligible == nil || !r.Eligible(w) { - return "not eligible for automatic assignment" - } - return "" -} - -func (r Resolver) known(w string) bool { - for _, k := range r.Known { - if norm(k) == w { - return true - } - } - return false -} - -// scorerTop returns the single best-scoring wing; a missing scorer, no scores, -// or a tie at the top is "no clear opinion". -func (r Resolver) scorerTop(text string) (string, bool) { - if r.Scorer == nil { - return "", false - } - return TopWing(r.Scorer.Score(text)) -} - -// TopWing returns the unique highest-scoring wing in scores, or false when -// there is none or the top is tied. Shared by the Resolver and the backtest so -// both apply the same notion of "the scorer has an opinion". -func TopWing(scores map[string]float64) (string, bool) { - best, bestScore, tied := "", 0.0, false - for k, v := range scores { - switch { - case v > bestScore: - best, bestScore, tied = norm(k), v, false - case v == bestScore && v > 0: - tied = true - } - } - if best == "" || tied { - return "", false - } - return best, true +// Placer decides where a raw note goes. Router is the production +// implementation; the interface lets the watcher stay ignorant of how. +type Placer interface { + Resolve(ctx context.Context, n Note) Decision } func exception(w string, conf float64, reason string) Decision { diff --git a/ingestion/internal/wing/resolve_test.go b/ingestion/internal/wing/resolve_test.go deleted file mode 100644 index 7d78e03..0000000 --- a/ingestion/internal/wing/resolve_test.go +++ /dev/null @@ -1,112 +0,0 @@ -package wing_test - -import ( - "context" - "errors" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/mathiasbq/hyperguild/ingestion/internal/wing" -) - -type fakeClassifier struct { - s wing.Suggestion - err error - calls int -} - -func (f *fakeClassifier) Classify(_ context.Context, _ string, _ []string) (wing.Suggestion, error) { - f.calls++ - return f.s, f.err -} - -type fakeScorer map[string]float64 - -func (f fakeScorer) Score(string) map[string]float64 { return f } - -// eligibleExcept allows every name except the listed ones. Being permissive on -// names it has never heard of means only the "known wing" check can stop an -// invented wing, so a test of that check cannot pass by accident. -func eligibleExcept(names ...string) func(string) bool { - deny := map[string]bool{} - for _, n := range names { - deny[n] = true - } - return func(w string) bool { return !deny[w] } -} - -func newResolver(c wing.Classifier, s wing.Scorer) wing.Resolver { - return wing.Resolver{ - Known: []string{"homelab", "hyperguild", "tapir", "rikatillsammans"}, - Eligible: eligibleExcept("rikatillsammans"), // a client wing: known, never auto - Classifier: c, - Scorer: s, - Threshold: 0.8, - } -} - -func TestResolve_ExplicitSignal(t *testing.T) { - tests := []struct { - name string - note wing.Note - want wing.Source - wantWing string - }{ - {"subfolder of a known eligible wing", wing.Note{Dir: "homelab"}, wing.Explicit, "homelab"}, - {"frontmatter of a known eligible wing", wing.Note{FrontmatterWing: "tapir"}, wing.Explicit, "tapir"}, - {"subfolder and frontmatter agree", wing.Note{Dir: "tapir", FrontmatterWing: "tapir"}, wing.Explicit, "tapir"}, - {"subfolder and frontmatter disagree", wing.Note{Dir: "tapir", FrontmatterWing: "homelab"}, wing.Exception, ""}, - {"unknown wing is never created", wing.Note{Dir: "hoemlab"}, wing.Exception, "hoemlab"}, - {"known but not eligible (client) wing is never auto-assigned", wing.Note{Dir: "rikatillsammans"}, wing.Exception, "rikatillsammans"}, - {"subfolder is case-insensitive", wing.Note{Dir: "HomeLab"}, wing.Explicit, "homelab"}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - c := &fakeClassifier{} - d := newResolver(c, fakeScorer{}).Resolve(context.Background(), tt.note) - assert.Equal(t, tt.want, d.Source) - assert.Equal(t, tt.wantWing, d.Wing) - assert.Zero(t, c.calls, "an explicit signal must not spend an LLM call") - assert.NotEmpty(t, d.Reason) - }) - } -} - -func TestResolve_Classified(t *testing.T) { - tests := []struct { - name string - sugg wing.Suggestion - err error - scores fakeScorer - want wing.Source - wantWing string - }{ - {"confident and scorer agrees", wing.Suggestion{Wing: "tapir", Confidence: 0.9}, nil, fakeScorer{"tapir": 3, "homelab": 1}, wing.Classified, "tapir"}, - {"confidence exactly at threshold passes", wing.Suggestion{Wing: "tapir", Confidence: 0.8}, nil, fakeScorer{"tapir": 3}, wing.Classified, "tapir"}, - {"below threshold is an exception, proposal kept", wing.Suggestion{Wing: "tapir", Confidence: 0.79}, nil, fakeScorer{"tapir": 3}, wing.Exception, "tapir"}, - {"scorer disagrees", wing.Suggestion{Wing: "tapir", Confidence: 0.95}, nil, fakeScorer{"homelab": 5, "tapir": 1}, wing.Exception, "tapir"}, - {"scorer has no opinion also needs review", wing.Suggestion{Wing: "tapir", Confidence: 0.95}, nil, fakeScorer{}, wing.Exception, "tapir"}, - {"classifier proposes a client wing", wing.Suggestion{Wing: "rikatillsammans", Confidence: 0.99}, nil, fakeScorer{"rikatillsammans": 9}, wing.Exception, "rikatillsammans"}, - {"classifier invents a wing", wing.Suggestion{Wing: "brand-new-wing", Confidence: 0.99}, nil, fakeScorer{"brand-new-wing": 9}, wing.Exception, "brand-new-wing"}, - {"classifier abstains", wing.Suggestion{Wing: "", Confidence: 0}, nil, fakeScorer{"tapir": 3}, wing.Exception, ""}, - {"classifier error", wing.Suggestion{}, errors.New("llm down"), fakeScorer{"tapir": 3}, wing.Exception, ""}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - c := &fakeClassifier{s: tt.sugg, err: tt.err} - d := newResolver(c, tt.scores).Resolve(context.Background(), wing.Note{Text: "some note"}) - assert.Equal(t, tt.want, d.Source) - assert.Equal(t, tt.wantWing, d.Wing) - assert.NotEmpty(t, d.Reason, "every decision says why, so a spot-check can judge it") - assert.Equal(t, 1, c.calls) - }) - } -} - -func TestResolve_NilDependenciesFailClosed(t *testing.T) { - r := wing.Resolver{Known: []string{"tapir"}, Eligible: eligibleExcept(), Threshold: 0.8} - d := r.Resolve(context.Background(), wing.Note{Text: "x"}) - require.Equal(t, wing.Exception, d.Source, "no classifier configured must route to review, not panic or guess") -} diff --git a/ingestion/internal/wing/router.go b/ingestion/internal/wing/router.go index 1ea5001..e5167b5 100644 --- a/ingestion/internal/wing/router.go +++ b/ingestion/internal/wing/router.go @@ -8,12 +8,6 @@ import ( "strings" ) -// Placer decides where a raw note goes. Router is the production -// implementation; the interface lets the watcher stay ignorant of how. -type Placer interface { - Resolve(ctx context.Context, n Note) Decision -} - // Router places a note with rules only, no model (hyperguild#208): // // 1. A subfolder or frontmatter `wing:` is the owner's deliberate choice. It @@ -68,3 +62,23 @@ func (r Router) explicit(w, via string) Decision { } return Decision{Wing: w, Source: Explicit, Confidence: 1, Reason: "named by " + via} } + +// NewRouter builds the production Router. defaultWing must be a plain wing +// name; clientPattern, when set, is a case-insensitive regexp over note text +// whose match parks an unsignalled note instead of filing it. Both fail at +// startup rather than later: a client filter that silently does nothing is +// worse than none. +func NewRouter(brainDir, defaultWing, clientPattern string) (Router, error) { + if defaultWing == "" || !plainWingName(defaultWing) { + return Router{}, fmt.Errorf("default wing %q is not a plain wing name", defaultWing) + } + r := Router{BrainDir: brainDir, Default: defaultWing} + if clientPattern != "" { + re, err := regexp.Compile("(?i)" + clientPattern) + if err != nil { + return Router{}, fmt.Errorf("client-content pattern: %w", err) + } + r.ClientBlock = re + } + return r, nil +} diff --git a/ingestion/internal/wing/router_test.go b/ingestion/internal/wing/router_test.go index 6c2cd63..8499100 100644 --- a/ingestion/internal/wing/router_test.go +++ b/ingestion/internal/wing/router_test.go @@ -85,3 +85,30 @@ func TestRouter_ANameThatIsNotPlainIsRefusedAsSuch(t *testing.T) { }) } } + +func TestNewRouter(t *testing.T) { + dir := routerBrain(t) + t.Run("valid", func(t *testing.T) { + r, err := wing.NewRouter(dir, "web-clip", "acme-client|globex") + assert.NoError(t, err) + d := r.Resolve(context.Background(), wing.Note{Text: "kickoff with GLOBEX"}) + assert.Equal(t, wing.Exception, d.Source, "the marker is matched case-insensitively") + }) + t.Run("no marker pattern means no content check", func(t *testing.T) { + r, err := wing.NewRouter(dir, "web-clip", "") + assert.NoError(t, err) + assert.Equal(t, wing.Default, r.Resolve(context.Background(), wing.Note{Text: "anything"}).Source) + }) + t.Run("a default that is not a plain wing name is a startup error", func(t *testing.T) { + _, err := wing.NewRouter(dir, "Web Clip", "") + assert.Error(t, err) + }) + t.Run("an empty default is a startup error", func(t *testing.T) { + _, err := wing.NewRouter(dir, "", "") + assert.Error(t, err) + }) + t.Run("a pattern that does not compile is a startup error", func(t *testing.T) { + _, err := wing.NewRouter(dir, "web-clip", "([unclosed") + assert.Error(t, err, "a client filter that silently does nothing is worse than none") + }) +} -- 2.54.0 From 8e488b6e4ba29dcaa736d5391669e9a12c7e7395 Mon Sep 17 00:00:00 2001 From: Mathias Date: Tue, 20 Oct 2026 20:22:20 +0200 Subject: [PATCH 06/14] fix(pipeline): a slug used in two places is refused even when one is the wing's own (#208) FindSlug returned the first hit, so a wing's own page of a name masked another page of the same name (the retired dirs are searched before the wings, and wings alphabetically). FindSlugs returns every match and callers refuse any that is not their own. Found by the legacy-move planner's own test. Co-Authored-By: Claude Sonnet 5.5 --- ingestion/internal/pipeline/pipeline.go | 15 +++++-- .../internal/pipeline/wing_layout_test.go | 14 ++++++ ingestion/internal/wiki/findslug_test.go | 30 +++++++------ ingestion/internal/wiki/inventory.go | 45 ++++++++++--------- 4 files changed, 65 insertions(+), 39 deletions(-) diff --git a/ingestion/internal/pipeline/pipeline.go b/ingestion/internal/pipeline/pipeline.go index cd4ec03..eab1e3e 100644 --- a/ingestion/internal/pipeline/pipeline.go +++ b/ingestion/internal/pipeline/pipeline.go @@ -53,11 +53,18 @@ func Run(ctx context.Context, cfg Config, brainDir, content, source string, dryR var err error if cfg.Wing != "" { // A source slug is a page's graph identity: refuse to take over a slug - // that lives anywhere but this wing's own sources/. - if existing, found, ferr := wiki.FindSlug(brainDir, wiki.Slug(source)); ferr != nil { + // that lives anywhere but this wing's own sources/ — including when + // this wing's own page of that name also exists. + slug := wiki.Slug(source) + own := "wiki/" + cfg.Wing + "/sources/" + slug + ".md" + existing, ferr := wiki.FindSlugs(brainDir, slug) + if ferr != nil { return Result{}, fmt.Errorf("check source slug: %w", ferr) - } else if own := "wiki/" + cfg.Wing + "/sources/" + wiki.Slug(source) + ".md"; found && existing != own { - return Result{}, fmt.Errorf("source slug %q is already used by %s", wiki.Slug(source), existing) + } + for _, p := range existing { + if p != own { + return Result{}, fmt.Errorf("source slug %q is already used by %s", slug, p) + } } inventory, err = wiki.LoadWingInventory(brainDir, cfg.Wing) } else { diff --git a/ingestion/internal/pipeline/wing_layout_test.go b/ingestion/internal/pipeline/wing_layout_test.go index 5f5b240..d53b514 100644 --- a/ingestion/internal/pipeline/wing_layout_test.go +++ b/ingestion/internal/pipeline/wing_layout_test.go @@ -140,3 +140,17 @@ func listAll(t *testing.T, dir string) []string { _ = filepath.WalkDir(dir, func(p string, _ os.DirEntry, _ error) error { out = append(out, p); return nil }) return out } + +// The first match must not hide a second one: this wing's own page of the name +// exists AND a page of the same name lives elsewhere. +func TestRun_ASlugUsedInTwoPlacesIsRefusedEvenWhenTheOwnPageIsOneOfThem(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "wiki/web-clip/sources"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "wiki/web-clip/sources/talk.md"), []byte("---\ntitle: Talk\n---\nown"), 0o644)) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "wiki/entities"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "wiki/entities/talk.md"), []byte("---\ntitle: Talk\n---\nan entity"), 0o644)) + + _, err := Run(context.Background(), wingCfg("web-clip", sourceEntityConcept()...), dir, "body", "talk", false) + require.Error(t, err) + assert.Contains(t, err.Error(), "already used") +} diff --git a/ingestion/internal/wiki/findslug_test.go b/ingestion/internal/wiki/findslug_test.go index bb89795..92202a3 100644 --- a/ingestion/internal/wiki/findslug_test.go +++ b/ingestion/internal/wiki/findslug_test.go @@ -16,37 +16,41 @@ func put(t *testing.T, dir, rel string) { require.NoError(t, os.WriteFile(full, []byte("---\ntitle: x\n---\n"), 0o644)) } -func TestFindSlug(t *testing.T) { +func TestFindSlugs(t *testing.T) { dir := t.TempDir() put(t, dir, "wiki/sources/legacy-src.md") put(t, dir, "wiki/concepts/a-concept.md") put(t, dir, "wiki/entities/an-entity.md") put(t, dir, "wiki/homelab/failures/in-a-hall.md") put(t, dir, "wiki/web-clip/sources/a-clip.md") + // One slug in two places: the first hit must not hide the second. + put(t, dir, "wiki/sources/shared-slug.md") + put(t, dir, "wiki/entities/shared-slug.md") + put(t, dir, "wiki/aaa/facts/shared-slug.md") tests := []struct { slug string - want string + want []string }{ - {"legacy-src", "wiki/sources/legacy-src.md"}, - {"a-concept", "wiki/concepts/a-concept.md"}, - {"an-entity", "wiki/entities/an-entity.md"}, - {"in-a-hall", "wiki/homelab/failures/in-a-hall.md"}, - {"a-clip", "wiki/web-clip/sources/a-clip.md"}, - {"nothing-here", ""}, + {"legacy-src", []string{"wiki/sources/legacy-src.md"}}, + {"a-concept", []string{"wiki/concepts/a-concept.md"}}, + {"an-entity", []string{"wiki/entities/an-entity.md"}}, + {"in-a-hall", []string{"wiki/homelab/failures/in-a-hall.md"}}, + {"a-clip", []string{"wiki/web-clip/sources/a-clip.md"}}, + {"shared-slug", []string{"wiki/aaa/facts/shared-slug.md", "wiki/entities/shared-slug.md", "wiki/sources/shared-slug.md"}}, + {"nothing-here", nil}, } for _, tt := range tests { t.Run(tt.slug, func(t *testing.T) { - got, found, err := FindSlug(dir, tt.slug) + got, err := FindSlugs(dir, tt.slug) require.NoError(t, err) - assert.Equal(t, tt.want != "", found) assert.Equal(t, tt.want, got) }) } } -func TestFindSlug_NoWikiDirIsNotAFinding(t *testing.T) { - _, found, err := FindSlug(t.TempDir(), "x") +func TestFindSlugs_NoWikiDirFindsNothing(t *testing.T) { + got, err := FindSlugs(t.TempDir(), "x") require.NoError(t, err) - assert.False(t, found) + assert.Empty(t, got) } diff --git a/ingestion/internal/wiki/inventory.go b/ingestion/internal/wiki/inventory.go index b87f5f8..44f8be1 100644 --- a/ingestion/internal/wiki/inventory.go +++ b/ingestion/internal/wiki/inventory.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "io/fs" + "sort" "strings" "github.com/mathiasbq/hyperguild/ingestion/internal/brainfs" @@ -133,45 +134,46 @@ func LoadWingInventory(brainDir, wing string) (map[PageType][]Entry, error) { return result, nil } -// FindSlug returns the brain-relative path of an existing page whose basename is -// slug, searching the retired dirs (wiki/{sources,concepts,entities}/) and every -// wing hall (wiki///). The slug is a page's graph identity, so two -// pages must not share one. -func FindSlug(brainDir, slug string) (string, bool, error) { +// FindSlugs returns the brain-relative path of EVERY existing page whose +// basename is slug, searching the retired dirs (wiki/{sources,concepts, +// entities}/) and every wing hall (wiki///), sorted. The slug is a +// page's graph identity, so two pages must not share one; returning all of +// them (not the first) is what lets a caller tell "only my own page" from "my +// own page and somebody else's". +func FindSlugs(brainDir, slug string) ([]string, error) { bfs, err := brainfs.Open(brainDir) if err != nil { - return "", false, fmt.Errorf("open brain: %w", err) + return nil, fmt.Errorf("open brain: %w", err) } defer func() { _ = bfs.Close() }() file := slug + ".md" - has := func(dir string) (bool, error) { + var found []string + check := func(dir string) error { names, err := bfs.ReadDirRegular(dir) if errors.Is(err, fs.ErrNotExist) { - return false, nil + return nil } if err != nil { - return false, err + return err } for _, n := range names { if n == file { - return true, nil + found = append(found, dir+"/"+file) } } - return false, nil + return nil } for _, d := range []string{"wiki/sources", "wiki/concepts", "wiki/entities"} { - if ok, err := has(d); err != nil { - return "", false, err - } else if ok { - return d + "/" + file, true, nil + if err := check(d); err != nil { + return nil, err } } wings, err := bfs.ReadDirDirs("wiki") if errors.Is(err, fs.ErrNotExist) { - return "", false, nil + return found, nil } if err != nil { - return "", false, err + return nil, err } for _, w := range wings { if w == "sources" || w == "concepts" || w == "entities" { @@ -182,12 +184,11 @@ func FindSlug(brainDir, slug string) (string, bool, error) { continue } for _, h := range halls { - if ok, err := has("wiki/" + w + "/" + h); err != nil { - return "", false, err - } else if ok { - return "wiki/" + w + "/" + h + "/" + file, true, nil + if err := check("wiki/" + w + "/" + h); err != nil { + return nil, err } } } - return "", false, nil + sort.Strings(found) + return found, nil } -- 2.54.0 From ae02d4764c632bb01fd1692fcd36edb84cf1d4ec Mon Sep 17 00:00:00 2001 From: Mathias Date: Tue, 20 Oct 2026 20:27:28 +0200 Subject: [PATCH 07/14] fix(pipeline): same-named sources in two wings, entity/source slug clashes, model-chosen types (review of #211) - a source whose slug is taken by anything but this wing's own page gets a wing-qualified, stable slug (talk-homelab) instead of failing the ingest - an extracted entity/concept whose slug belongs to a different page is skipped with a warning, so one page can no longer lock a source out of re-ingest - in a wing only source/concept/entity are valid page types; the model (and so the clip) can no longer choose wiki// directories Mutation-checked; stable-slug choice pinned directly because title resolution masks it at the Run level. Co-Authored-By: Claude Sonnet 5.5 --- ingestion/internal/pipeline/build.go | 7 +- ingestion/internal/pipeline/pipeline.go | 95 ++++++++++++--- .../internal/pipeline/wing_layout_test.go | 113 +++++++++++++++--- 3 files changed, 183 insertions(+), 32 deletions(-) diff --git a/ingestion/internal/pipeline/build.go b/ingestion/internal/pipeline/build.go index 5479db5..d0f57b0 100644 --- a/ingestion/internal/pipeline/build.go +++ b/ingestion/internal/pipeline/build.go @@ -28,7 +28,7 @@ func BuildPagesIn(wing string, rawPages []RawPage, sourceSlug, date string) ([]w for _, rp := range rawPages { // The type becomes a directory under wiki/; "../raw" wrote outside it // (PR #147 review). Only a plain lower-case name is a page type. - if !validPageType.MatchString(rp.Type) { + if !validPageType.MatchString(rp.Type) || (wing != "" && !layoutType(rp.Type)) { warnings = append(warnings, fmt.Sprintf("skipped page with invalid type %q", rp.Type)) continue } @@ -42,6 +42,11 @@ func BuildPagesIn(wing string, rawPages []RawPage, sourceSlug, date string) ([]w return out, warnings } +// layoutType reports whether a wing-mode page type has a place in the layout. +// The type comes from the model, so from the clip being ingested: anything else +// would name a directory (wiki//) of the model's choosing. +func layoutType(t string) bool { return t == "source" || t == "concept" || t == "entity" } + var validPageType = regexp.MustCompile(`^[a-z][a-z0-9-]*$`) func computeSlug(rp RawPage, sourceSlug string) string { diff --git a/ingestion/internal/pipeline/pipeline.go b/ingestion/internal/pipeline/pipeline.go index eab1e3e..51d5dd3 100644 --- a/ingestion/internal/pipeline/pipeline.go +++ b/ingestion/internal/pipeline/pipeline.go @@ -10,6 +10,7 @@ import ( "github.com/mathiasbq/hyperguild/ingestion/internal/brain" "github.com/mathiasbq/hyperguild/ingestion/internal/brainfs" "io/fs" + "path/filepath" "strings" "time" @@ -52,20 +53,6 @@ func Run(ctx context.Context, cfg Config, brainDir, content, source string, dryR var inventory map[wiki.PageType][]wiki.Entry var err error if cfg.Wing != "" { - // A source slug is a page's graph identity: refuse to take over a slug - // that lives anywhere but this wing's own sources/ — including when - // this wing's own page of that name also exists. - slug := wiki.Slug(source) - own := "wiki/" + cfg.Wing + "/sources/" + slug + ".md" - existing, ferr := wiki.FindSlugs(brainDir, slug) - if ferr != nil { - return Result{}, fmt.Errorf("check source slug: %w", ferr) - } - for _, p := range existing { - if p != own { - return Result{}, fmt.Errorf("source slug %q is already used by %s", slug, p) - } - } inventory, err = wiki.LoadWingInventory(brainDir, cfg.Wing) } else { inventory, err = wiki.LoadInventory(brainDir) @@ -80,6 +67,12 @@ func Run(ctx context.Context, cfg Config, brainDir, content, source string, dryR } sourceSlug := wiki.Slug(source) + if cfg.Wing != "" { + var rerr error + if sourceSlug, rerr = resolveSourceSlug(brainDir, cfg.Wing, sourceSlug); rerr != nil { + return Result{}, rerr + } + } date := time.Now().UTC().Format("2006-01-02") // Drift check BEFORE any extraction: an unchanged source costs a full LLM @@ -151,6 +144,15 @@ func buildAndWrite(ctx context.Context, wing string, rawPages []RawPage, sourceS warnings = append(warnings, linkWarnings...) withRefs := injectSourceRefs(canonicalized, inventory, brainDir) merged := mergeAll(withRefs) + if wing != "" { + var clashWarnings []string + var cerr error + merged, clashWarnings, cerr = dropSlugClashes(brainDir, merged) + if cerr != nil { + return Result{}, cerr + } + warnings = append(warnings, clashWarnings...) + } var written []string for _, page := range merged { @@ -333,3 +335,68 @@ func loadSchema(brainDir string) string { } return strings.TrimSpace(string(b)) } + +// resolveSourceSlug returns the slug this wing's source page uses. A slug is a +// page's graph identity and must be unique brain-wide, but two wings dropping a +// file of the same name (notes.md, readme.md) is ordinary, so when the plain +// slug is taken by anything but this wing's own source page the wing name is +// appended (talk-homelab, then talk-homelab-2, ...). The choice is stable: a +// re-ingest finds the page it made, because that page is "its own". +func resolveSourceSlug(brainDir, wing, base string) (string, error) { + for n := 1; n <= 100; n++ { + cand := base + switch n { + case 1: + case 2: + cand = base + "-" + wing + default: + cand = fmt.Sprintf("%s-%s-%d", base, wing, n-1) + } + existing, err := wiki.FindSlugs(brainDir, cand) + if err != nil { + return "", fmt.Errorf("check source slug: %w", err) + } + own := "wiki/" + wing + "/sources/" + cand + ".md" + free := true + for _, p := range existing { + if p != own { + free = false + break + } + } + if free { + return cand, nil + } + } + return "", fmt.Errorf("no free source slug for %q in wing %q", base, wing) +} + +// dropSlugClashes removes, from the pages about to be written in a wing, any +// page whose slug already belongs to a different page elsewhere in the brain. +// The page's own path is not a clash (a shared entity merging into itself). +// A skipped page is a warning, not a failure: the ingest as a whole is fine, +// and one extracted page must not be able to lock a source out of re-ingest. +func dropSlugClashes(brainDir string, pages []wiki.Page) ([]wiki.Page, []string, error) { + var kept []wiki.Page + var warnings []string + for _, p := range pages { + slug := strings.TrimSuffix(filepath.Base(p.Path), ".md") + existing, err := wiki.FindSlugs(brainDir, slug) + if err != nil { + return nil, nil, fmt.Errorf("check slug %q: %w", slug, err) + } + clash := "" + for _, e := range existing { + if e != p.Path { + clash = e + break + } + } + if clash != "" { + warnings = append(warnings, fmt.Sprintf("skipped %s: slug %q is already used by %s", p.Path, slug, clash)) + continue + } + kept = append(kept, p) + } + return kept, warnings, nil +} diff --git a/ingestion/internal/pipeline/wing_layout_test.go b/ingestion/internal/pipeline/wing_layout_test.go index d53b514..5664e4c 100644 --- a/ingestion/internal/pipeline/wing_layout_test.go +++ b/ingestion/internal/pipeline/wing_layout_test.go @@ -4,6 +4,7 @@ import ( "context" "os" "path/filepath" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -92,27 +93,77 @@ func TestRun_TheSameEntityCitedFromTwoWingsIsOnePageWithBothSources(t *testing.T } // A source slug is the graph's identity too: a second wing must not take over -// the slug of a source that lives elsewhere. -func TestRun_ASourceSlugAlreadyUsedInAnotherWingIsRefused(t *testing.T) { +// the slug of a page that lives elsewhere. Two wings dropping a file of the same +// name (notes.md, readme.md) is ordinary, so the second source gets a distinct, +// stable slug instead of failing. +func TestRun_ASourceWhoseSlugIsTakenElsewhereGetsADistinctStableSlug(t *testing.T) { dir := t.TempDir() _, err := Run(context.Background(), wingCfg("web-clip", sourceEntityConcept()...), dir, "body", "talk", false) require.NoError(t, err) require.NoError(t, os.MkdirAll(filepath.Join(dir, "wiki/homelab/facts"), 0o755)) _, err = Run(context.Background(), wingCfg("homelab", sourceEntityConcept()...), dir, "different body", "talk", false) - require.Error(t, err) - assert.Contains(t, err.Error(), "already used") + require.NoError(t, err) + assert.True(t, exists(dir, "wiki/web-clip/sources/talk.md"), "the first wing's page is untouched") + assert.True(t, exists(dir, "wiki/homelab/sources/talk-homelab.md"), "the second gets a wing-qualified slug") assert.False(t, exists(dir, "wiki/homelab/sources/talk.md")) + + _, err = Run(context.Background(), wingCfg("homelab", sourceEntityConcept()...), dir, "different body, edited", "talk", false) + require.NoError(t, err) + files, _ := filepath.Glob(filepath.Join(dir, "wiki/homelab/sources/*.md")) + assert.Len(t, files, 1, "re-ingesting finds the same page, it does not make talk-homelab-2") } -func TestRun_ASourceSlugAlreadyInTheRetiredLayoutIsRefused(t *testing.T) { +func TestRun_ASourceWhoseSlugIsTakenByARetiredPageOrASharedPageGetsADistinctSlug(t *testing.T) { dir := t.TempDir() require.NoError(t, os.MkdirAll(filepath.Join(dir, "wiki/sources"), 0o755)) require.NoError(t, os.WriteFile(filepath.Join(dir, "wiki/sources/talk.md"), []byte("---\ntitle: Talk\n---\nold"), 0o644)) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "wiki/entities"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "wiki/entities/other.md"), []byte("---\ntitle: Other\n---\nx"), 0o644)) _, err := Run(context.Background(), wingCfg("web-clip", sourceEntityConcept()...), dir, "body", "talk", false) - require.Error(t, err) - assert.Contains(t, err.Error(), "already used") + require.NoError(t, err) + assert.True(t, exists(dir, "wiki/web-clip/sources/talk-web-clip.md")) + assert.Equal(t, "---\ntitle: Talk\n---\nold", read(t, dir, "wiki/sources/talk.md"), "the retired page is untouched") +} + +// The other direction: a model-extracted entity or concept whose slug is a +// source's slug in some wing would lock that source out of re-ingest. The page +// is skipped with a warning; the ingest itself succeeds. +func TestRun_AnEntityWhoseSlugIsASourceElsewhereIsSkippedNotWritten(t *testing.T) { + dir := t.TempDir() + _, err := Run(context.Background(), wingCfg("web-clip", []RawPage{ + {Title: "Jev", Type: "source", Subtype: "video", Content: "## Summary\n\nA clip titled Jev.\n"}, + }...), dir, "body", "jev", false) + require.NoError(t, err) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "wiki/homelab/facts"), 0o755)) + + res, err := Run(context.Background(), wingCfg("homelab", sourceEntityConcept()...), dir, "another body", "other-talk", false) + require.NoError(t, err) + assert.False(t, exists(dir, "wiki/entities/jev.md"), "the entity would take the source's slug") + assert.Contains(t, strings.Join(res.Warnings, "\n"), "jev") + + _, err = Run(context.Background(), wingCfg("web-clip", []RawPage{ + {Title: "Jev", Type: "source", Subtype: "video", Content: "## Summary\n\nEdited.\n"}, + }...), dir, "body, edited", "jev", false) + require.NoError(t, err, "the first wing can still re-ingest its own source") +} + +// The type comes from the model, so from the clip: in a wing it may only be one +// of the three the layout has a place for. +func TestRun_WithAWingAModelChosenTypeCannotWriteOutsideTheLayout(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "wiki/alpha/facts"), 0o755)) + pages := append(sourceEntityConcept(), + RawPage{Title: "Planted", Type: "sources", Content: "## x\n\nplanted\n"}, + RawPage{Title: "Loose", Type: "alpha", Content: "## x\n\nloose\n"}, + RawPage{Title: "Sneaky", Type: "homelab", Content: "## x\n\nsneaky\n"}) + res, err := Run(context.Background(), wingCfg("web-clip", pages...), dir, "body", "talk", false) + require.NoError(t, err) + assert.False(t, exists(dir, "wiki/sources/planted.md")) + assert.False(t, exists(dir, "wiki/alpha/loose.md")) + assert.False(t, exists(dir, "wiki/homelab")) + assert.Contains(t, strings.Join(res.Warnings, "\n"), "invalid type") } func TestRun_ReIngestingTheSameSourceIntoItsOwnWingIsNotACollision(t *testing.T) { @@ -141,16 +192,44 @@ func listAll(t *testing.T, dir string) []string { return out } -// The first match must not hide a second one: this wing's own page of the name -// exists AND a page of the same name lives elsewhere. -func TestRun_ASlugUsedInTwoPlacesIsRefusedEvenWhenTheOwnPageIsOneOfThem(t *testing.T) { +// Title resolution can mask a wrong slug choice at the Run level, so the +// stability of the choice is pinned directly: the page this wing made earlier +// under the qualified slug is "its own" and is found again. +func TestResolveSourceSlug(t *testing.T) { dir := t.TempDir() - require.NoError(t, os.MkdirAll(filepath.Join(dir, "wiki/web-clip/sources"), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(dir, "wiki/web-clip/sources/talk.md"), []byte("---\ntitle: Talk\n---\nown"), 0o644)) - require.NoError(t, os.MkdirAll(filepath.Join(dir, "wiki/entities"), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(dir, "wiki/entities/talk.md"), []byte("---\ntitle: Talk\n---\nan entity"), 0o644)) + for _, rel := range []string{ + "wiki/web-clip/sources/talk.md", + "wiki/homelab/sources/talk-homelab.md", + "wiki/entities/kube.md", + } { + require.NoError(t, os.MkdirAll(filepath.Dir(filepath.Join(dir, rel)), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, rel), []byte("---\ntitle: x\n---\n"), 0o644)) + } + tests := []struct { + name, wing, base, want string + }{ + {"free slug is used as is", "homelab", "fresh", "fresh"}, + {"own page under the plain slug", "web-clip", "talk", "talk"}, + {"taken elsewhere: wing-qualified", "tapir", "talk", "talk-tapir"}, + {"own earlier qualified page is found again", "homelab", "talk", "talk-homelab"}, + {"taken by a shared page", "tapir", "kube", "kube-tapir"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := resolveSourceSlug(dir, tt.wing, tt.base) + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} - _, err := Run(context.Background(), wingCfg("web-clip", sourceEntityConcept()...), dir, "body", "talk", false) - require.Error(t, err) - assert.Contains(t, err.Error(), "already used") +func TestResolveSourceSlug_AFurtherClashGetsANumber(t *testing.T) { + dir := t.TempDir() + for _, rel := range []string{"wiki/a/sources/talk.md", "wiki/entities/talk-b.md"} { + require.NoError(t, os.MkdirAll(filepath.Dir(filepath.Join(dir, rel)), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, rel), []byte("x"), 0o644)) + } + got, err := resolveSourceSlug(dir, "b", "talk") + require.NoError(t, err) + assert.Equal(t, "talk-b-2", got) } -- 2.54.0 From 6b6d43038337bdfc676b96199b636cac4ec63343 Mon Sep 17 00:00:00 2001 From: Mathias Date: Tue, 20 Oct 2026 20:29:09 +0200 Subject: [PATCH 08/14] fix(wing): the client filter survives cheap evasions; lookalike and reserved folder names are refused (review of #211) - client pattern is matched against the text and against an NFKC-folded, format-character-free, dash-normalised, whitespace-free form (non-breaking hyphen, zero-width characters, soft hyphen, line splits, fullwidth letters). x/text/unicode/norm was already in the module graph as an indirect dependency; this makes it direct, no new download. - folder/frontmatter wing names must be ASCII (strings.ToLower folds the Kelvin sign to 'k') and must not be sources/concepts/entities/wiki; such names are never proposed as a wing, and a reserved default is a startup error Mutation-checked. Co-Authored-By: Claude Sonnet 5.5 --- ingestion/go.mod | 2 +- ingestion/internal/wing/router.go | 63 +++++++++++++++++++++++--- ingestion/internal/wing/router_test.go | 62 ++++++++++++++++++++++++- 3 files changed, 118 insertions(+), 9 deletions(-) diff --git a/ingestion/go.mod b/ingestion/go.mod index ac4eea7..17b1996 100644 --- a/ingestion/go.mod +++ b/ingestion/go.mod @@ -69,6 +69,6 @@ require ( golang.org/x/crypto v0.51.0 // indirect golang.org/x/sync v0.21.0 // indirect golang.org/x/sys v0.45.0 - golang.org/x/text v0.39.0 // indirect + golang.org/x/text v0.39.0 gopkg.in/yaml.v3 v3.0.1 ) diff --git a/ingestion/internal/wing/router.go b/ingestion/internal/wing/router.go index e5167b5..8f628db 100644 --- a/ingestion/internal/wing/router.go +++ b/ingestion/internal/wing/router.go @@ -6,6 +6,9 @@ import ( "regexp" "slices" "strings" + "unicode" + + xnorm "golang.org/x/text/unicode/norm" ) // Router places a note with rules only, no model (hyperguild#208): @@ -28,6 +31,13 @@ type Router struct { // Resolve implements Placer. It never returns an error: anything it cannot // decide safely is an Exception with the reason spelled out. func (r Router) Resolve(_ context.Context, n Note) Decision { + for _, raw := range []string{n.Dir, n.FrontmatterWing} { + if !isASCII(raw) { + // strings.ToLower folds some non-ASCII runes to ASCII (the Kelvin + // sign becomes "k"), which would let a lookalike name a real wing. + return exception("", 0, fmt.Sprintf("wing name %q contains non-ASCII characters", strings.TrimSpace(raw))) + } + } dir, fm := norm(n.Dir), norm(n.FrontmatterWing) switch { case dir != "" && fm != "" && dir != fm: @@ -37,20 +47,20 @@ func (r Router) Resolve(_ context.Context, n Note) Decision { case fm != "": return r.explicit(fm, "frontmatter") } - if r.ClientBlock != nil && r.ClientBlock.MatchString(n.Text) { + if r.ClientBlock != nil && matchesClient(r.ClientBlock, n.Text) { return exception("", 0, "text matches the client-content marker; not defaulted into a shared wing") } - if r.Default == "" || !plainWingName(r.Default) { + if r.Default == "" || !usableWingName(r.Default) { return exception("", 0, "no valid default wing configured") } return Decision{Wing: r.Default, Source: Default, Confidence: 1, Reason: "no wing named; default wing"} } func (r Router) explicit(w, via string) Decision { - if !plainWingName(w) { - return exception("", 0, fmt.Sprintf("%s names %q, which is not a plain wing name", via, w)) + if !usableWingName(w) { + return exception("", 0, fmt.Sprintf("%s names %q, which is not a usable wing name", via, w)) } - if w == r.Default && plainWingName(w) { + if w == r.Default { return Decision{Wing: w, Source: Explicit, Confidence: 1, Reason: "named by " + via} } known, err := KnownWings(r.BrainDir) @@ -69,8 +79,8 @@ func (r Router) explicit(w, via string) Decision { // startup rather than later: a client filter that silently does nothing is // worse than none. func NewRouter(brainDir, defaultWing, clientPattern string) (Router, error) { - if defaultWing == "" || !plainWingName(defaultWing) { - return Router{}, fmt.Errorf("default wing %q is not a plain wing name", defaultWing) + if defaultWing == "" || !usableWingName(defaultWing) { + return Router{}, fmt.Errorf("default wing %q is not a usable wing name", defaultWing) } r := Router{BrainDir: brainDir, Default: defaultWing} if clientPattern != "" { @@ -82,3 +92,42 @@ func NewRouter(brainDir, defaultWing, clientPattern string) (Router, error) { } return r, nil } + +// reservedWingNames are directory names under wiki/ that are not wings: the +// retired layout and the root. A note must never be proposed into one. +var reservedWingNames = map[string]bool{"sources": true, "concepts": true, "entities": true, "wiki": true} + +// usableWingName: a plain name that is not reserved. +func usableWingName(w string) bool { return plainWingName(w) && !reservedWingNames[w] } + +func isASCII(s string) bool { + for _, r := range s { + if r > unicode.MaxASCII { + return false + } + } + return true +} + +// matchesClient applies the client-content pattern to the text and to two +// folded forms of it, so the cheap evasions do not defeat it: NFKC (fullwidth +// letters), format characters removed (zero-width joiners, soft hyphens), every +// dash made "-", and whitespace removed ("acme-\nclient", "acme - client"). It catches what it names and nothing else; it is a filter, +// not a classifier. +func matchesClient(re *regexp.Regexp, text string) bool { + folded := xnorm.NFKC.String(text) + var b strings.Builder + for _, r := range folded { + switch { + case unicode.Is(unicode.Cf, r): + continue + case unicode.Is(unicode.Pd, r) || r == '\u2212': + b.WriteRune('-') + default: + b.WriteRune(r) + } + } + clean := b.String() + squeezed := strings.Join(strings.Fields(clean), "") + return re.MatchString(text) || re.MatchString(squeezed) +} diff --git a/ingestion/internal/wing/router_test.go b/ingestion/internal/wing/router_test.go index 8499100..68c9ed5 100644 --- a/ingestion/internal/wing/router_test.go +++ b/ingestion/internal/wing/router_test.go @@ -81,7 +81,7 @@ func TestRouter_ANameThatIsNotPlainIsRefusedAsSuch(t *testing.T) { d := r.Resolve(context.Background(), wing.Note{Dir: bad, Text: "x"}) assert.Equal(t, wing.Exception, d.Source) assert.Empty(t, d.Wing, "a malformed name is never echoed as a proposal") - assert.Contains(t, d.Reason, "not a plain wing name") + assert.Contains(t, d.Reason, "not a usable wing name") }) } } @@ -112,3 +112,63 @@ func TestNewRouter(t *testing.T) { assert.Error(t, err, "a client filter that silently does nothing is worse than none") }) } + +// From the review of #211: the content filter is the only guard on the default +// lane, so it must survive the cheap evasions. +func TestRouter_ClientMarkerSurvivesCheapEvasions(t *testing.T) { + r := wing.Router{BrainDir: routerBrain(t), Default: "web-clip", ClientBlock: regexp.MustCompile(`(?i)acme-client`)} + tests := []struct{ name, text string }{ + {"non-breaking hyphen", "notes on acme‑client"}, + {"zero-width joiner inside", "notes on acme\u200d-client"}, + {"zero-width space inside", "notes on ac\u200bme-client"}, + {"split across lines", "notes on acme-\nclient"}, + {"spaced hyphen", "notes on acme - client"}, + {"fullwidth letters", "notes on ａｃｍｅ-client"}, + {"soft hyphen", "notes on acme\u00ad-client"}, + {"en dash", "notes on acme–client"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + d := r.Resolve(context.Background(), wing.Note{Text: tt.text}) + assert.Equal(t, wing.Exception, d.Source, tt.text) + }) + } + t.Run("an unrelated text still files", func(t *testing.T) { + d := r.Resolve(context.Background(), wing.Note{Text: "a talk about agents and a client-server model"}) + assert.Equal(t, wing.Default, d.Source) + }) +} + +func TestRouter_FolderNamesAreAsciiAndNotReserved(t *testing.T) { + dir := routerBrain(t) + writePage(t, dir, "wiki/kubernetes/facts/a.md", "x") + r := wing.Router{BrainDir: dir, Default: "web-clip"} + tests := []struct { + name string + dir string + }{ + {"Kelvin sign lowercases to an ASCII k", "Kubernetes"}, + {"cyrillic lookalike", "hоmelab"}, + {"retired layout name: sources", "sources"}, + {"retired layout name: concepts", "concepts"}, + {"retired layout name: entities", "entities"}, + {"wiki", "wiki"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + d := r.Resolve(context.Background(), wing.Note{Dir: tt.dir, Text: "x"}) + assert.Equal(t, wing.Exception, d.Source) + assert.Empty(t, d.Wing, "never proposed as a wing") + }) + } + t.Run("plain ASCII case folding still works", func(t *testing.T) { + d := r.Resolve(context.Background(), wing.Note{Dir: "KubernetEs", Text: "x"}) + assert.Equal(t, wing.Explicit, d.Source) + assert.Equal(t, "kubernetes", d.Wing) + }) +} + +func TestNewRouter_AReservedDefaultIsRefused(t *testing.T) { + _, err := wing.NewRouter(routerBrain(t), "sources", "") + assert.Error(t, err) +} -- 2.54.0 From 87c40fe36439e34b732598103f544c66e86b9406 Mon Sep 17 00:00:00 2001 From: Mathias Date: Tue, 20 Oct 2026 20:34:52 +0200 Subject: [PATCH 09/14] fix(api,watcher): name the queue, list parked non-markdown, reserved wings, top-level-only frontmatter key; document the inbox (review of #211) - brain_promote/brain_reject take an optional queue (inbox | needs-review); a name present in both is refused instead of silently acting on the top-level note; brain_pending labels every entry with its queue - parked pdf/txt files are listed (they were invisible); only markdown can be promoted, the others are moved or rejected - promote refuses sources/concepts/entities/wiki as a wing - frontmatterWing reads only a top-level key inside a properly fenced block - docs/raw-inbox.md: the contract, the three env vars, the known limits Each behaviour has a test that fails without it (mutation-checked). Co-Authored-By: Claude Sonnet 5.5 --- docs/raw-inbox.md | 49 +++++++ ingestion/internal/api/handler.go | 3 +- ingestion/internal/api/pending_review_test.go | 2 +- ingestion/internal/api/promote.go | 103 ++++++++++---- ingestion/internal/api/promote_queue_test.go | 126 ++++++++++++++++++ ingestion/internal/api/promote_review_test.go | 13 -- ingestion/internal/mcp/handlers.go | 8 +- ingestion/internal/mcp/handlers_test.go | 25 ++++ ingestion/internal/watcher/watcher.go | 13 +- .../internal/watcher/watcher_wing_test.go | 21 +++ 10 files changed, 316 insertions(+), 47 deletions(-) create mode 100644 docs/raw-inbox.md create mode 100644 ingestion/internal/api/promote_queue_test.go diff --git a/docs/raw-inbox.md b/docs/raw-inbox.md new file mode 100644 index 0000000..bb2e374 --- /dev/null +++ b/docs/raw-inbox.md @@ -0,0 +1,49 @@ +# The raw/ inbox + +`raw/` is an inbox: only unprocessed material sits there. Syncthing shares it +with the machines you drop files from. (hyperguild#205, #208, #209) + +## What happens to a file you drop + +| You drop | Result | +|---|---| +| `raw/clip.md` (no folder, no `wing:`) | extracted and filed as a source in `wiki/web-clip/sources/`; its concepts and entities update the shared `wiki/concepts/` and `wiki/entities/` pages; the original moves to `raw/processed//`, so `raw/` is empty again | +| `raw//clip.md` or `wing: ` in the frontmatter | the same, with the source in `wiki//sources/`. The wing must already exist; a typo is parked, never created. This is your deliberate choice and is not second-guessed | +| a note whose text matches the client-content pattern, with no folder | parked in `raw/needs-review/`, not extracted | +| a folder or `wing:` naming an unknown, reserved or non-ASCII wing, or two disagreeing signals | parked in `raw/needs-review/` | +| a note over the size cap, or one that cannot be archived | stays in `raw/`, reported once, not retried until it changes | + +Concepts and entities are shared across wings because their slug is the graph's +identity. A source whose slug is taken gets a wing-qualified one (`notes-homelab`). + +## Parked notes + +`brain_pending` lists them with `queue: needs-review`, a proposed wing and the +reason. To act on one: `brain_promote` / `brain_reject` (pass `queue` when the +same name also sits in the inbox), or move the file into `raw//` to have it +extracted. Parked pdf/txt files are listed but can only be moved or rejected. + +## Switching it on + +Routing is off by default (the retired layout is used). Set on the ingestion +deployment: + +| Variable | Meaning | +|---|---| +| `INGEST_WING_ROUTING=1` | enable the rules above | +| `INGEST_DEFAULT_WING` | wing for unsignalled notes (default `web-clip`; must be a plain name, not `sources`/`concepts`/`entities`/`wiki`) | +| `INGEST_CLIENT_BLOCK` | case-insensitive regexp over note text; a match parks an unsignalled note. Falls back to `CLAUDE_INGEST_CLIENT_BLOCK`. Matched after folding fullwidth letters, removing zero-width characters, normalising dashes and removing whitespace. It catches what it names and nothing else | + +A bad default or pattern stops the server at startup. A wing created while the +server runs is usable without a restart. + +## Known limits + +- A deliberate folder into a sensitive wing places the note there, and the shared + concept/entity pages then cite that source by title. Client material does not + belong in the brain at all; the pattern only guards the unsignalled lane. +- A merge into a shared page refreshes `last_updated` and `origin` to the latest + ingest; the first wing's attribution is not kept. +- A marker file (`.processed`) is only written when a processed original + cannot be removed. One at least as new as its note holds the note (and says + so once); an older or future-dated one is ignored and removed. diff --git a/ingestion/internal/api/handler.go b/ingestion/internal/api/handler.go index 313fba4..4daba38 100644 --- a/ingestion/internal/api/handler.go +++ b/ingestion/internal/api/handler.go @@ -617,6 +617,7 @@ type promoteRequest struct { Wing string `json:"wing"` Hall string `json:"hall"` Slug string `json:"slug,omitempty"` + Queue string `json:"queue,omitempty"` } // Promote handles POST /promote — move a raw/ note into the wiki. A bad @@ -630,7 +631,7 @@ func (h *Handler) Promote(w http.ResponseWriter, r *http.Request) { // Field by field: this door carries no principal, so the promoted note is // stamped origin: unknown and the body cannot set provenance (#145). rel, err := PromoteNote(h.brainDir, PromoteOptions{ - Filename: req.Filename, Wing: req.Wing, Hall: req.Hall, Slug: req.Slug, + Filename: req.Filename, Wing: req.Wing, Hall: req.Hall, Slug: req.Slug, Queue: req.Queue, }) if err != nil { writeError(w, http.StatusBadRequest, err.Error()) diff --git a/ingestion/internal/api/pending_review_test.go b/ingestion/internal/api/pending_review_test.go index 0283c24..86f0bfa 100644 --- a/ingestion/internal/api/pending_review_test.go +++ b/ingestion/internal/api/pending_review_test.go @@ -34,7 +34,7 @@ func TestListPending_ShowsParkedNotesWithTheirProposal(t *testing.T) { assert.NotContains(t, got[0].Excerpt, "proposed_wing", "excerpt is body, not frontmatter") assert.Equal(t, "fresh.md", got[1].Filename) - assert.Empty(t, got[1].Queue, "a top-level note has no queue label") + assert.Equal(t, QueueInbox, got[1].Queue) assert.Empty(t, got[1].ProposedWing) } diff --git a/ingestion/internal/api/promote.go b/ingestion/internal/api/promote.go index 6a1a966..a0a86cb 100644 --- a/ingestion/internal/api/promote.go +++ b/ingestion/internal/api/promote.go @@ -26,7 +26,7 @@ type PendingNote struct { // (hyperguild#205): it was not placed automatically, and the proposal says // what the resolver would have done and why it did not. Approve by moving // the note into raw// and the watcher ingests it there. - Queue string `json:"queue,omitempty"` + Queue string `json:"queue"` ProposedWing string `json:"proposed_wing,omitempty"` ProposalReason string `json:"proposal_reason,omitempty"` ProposalConfidence float64 `json:"proposal_confidence,omitempty"` @@ -49,7 +49,7 @@ func ListPending(brainDir string) ([]PendingNote, error) { } defer func() { _ = root.Close() }() - out, err := listNotes(root, "") + out, err := listNotes(root, QueueInbox) if err != nil { return nil, err } @@ -57,7 +57,7 @@ func ListPending(brainDir string) ([]PendingNote, error) { // or note there is refused exactly like at the top level (#153). if sub, serr := root.OpenRoot("needs-review"); serr == nil { defer func() { _ = sub.Close() }() - parked, perr := listNotes(sub, "needs-review") + parked, perr := listNotes(sub, QueueNeedsReview) if perr != nil { return nil, perr } @@ -76,9 +76,25 @@ func listNotes(root *os.Root, queue string) ([]PendingNote, error) { } out := make([]PendingNote, 0, len(entries)) for _, e := range entries { - if !e.Type().IsRegular() || !strings.HasSuffix(e.Name(), ".md") || strings.HasPrefix(e.Name(), "tunnel-candidates-") { + isMD := strings.HasSuffix(e.Name(), ".md") + // A parked pdf or txt is listed too (the watcher parks any supported + // file); at the top level only markdown ever was. + parkedOther := queue == QueueNeedsReview && (strings.HasSuffix(e.Name(), ".txt") || strings.HasSuffix(e.Name(), ".pdf")) + if !e.Type().IsRegular() || (!isMD && !parkedOther) || strings.HasPrefix(e.Name(), "tunnel-candidates-") { continue // directories, and symlinks whose target may be anywhere (#153) } + if !isMD { + info, ierr := root.Lstat(e.Name()) + if ierr != nil { + continue + } + out = append(out, PendingNote{ + Filename: e.Name(), CreatedAt: info.ModTime().UTC().Format(time.RFC3339), SizeBytes: info.Size(), + Excerpt: "(not markdown; the wing proposal is in log.md. Move the file into raw// to ingest it.)", + Queue: queue, + }) + continue + } raw, info, readErr := readRawNote(root, e.Name()) if readErr != nil { continue @@ -95,7 +111,7 @@ func listNotes(root *os.Root, queue string) ([]PendingNote, error) { Excerpt: excerpt(body, 200), Queue: queue, } - if queue != "" { + if queue == QueueNeedsReview { n.ProposedWing = fm.get("proposed_wing") n.ProposalReason = unquoteScalar(fm.get("proposal_reason")) n.ProposalConfidence, _ = strconv.ParseFloat(fm.get("proposal_confidence"), 64) @@ -111,6 +127,9 @@ type PromoteOptions struct { Wing string Hall string Slug string // optional; defaults to Filename minus date prefix + .md + // Queue names which listing the note is in: QueueInbox or QueueNeedsReview. + // Empty is accepted only when the name exists in exactly one of them. + Queue string // Origin/WrittenBy re-attribute the promoted note to the promoter: the raw // note's own frontmatter claims are not provenance (hyperguild#145). Origin string @@ -133,17 +152,26 @@ func PromoteNote(brainDir string, opts PromoteOptions) (string, error) { return "", fmt.Errorf("invalid filename %q", opts.Filename) } + if !strings.HasSuffix(base, ".md") { + return "", fmt.Errorf("only a markdown note can be promoted; move %q into raw// to ingest it", base) + } slug := opts.Slug if slug == "" { slug = datePrefix.ReplaceAllString(strings.TrimSuffix(base, ".md"), "") } + // The retired layout's directory names are not wings: a note promoted into + // "sources" would land in wiki/sources//, inside the retired dir. + switch brain.Sanitise(opts.Wing) { + case "sources", "concepts", "entities", "wiki": + return "", fmt.Errorf("%q is not a wing", opts.Wing) + } // NotePath validates hall + wing + slug; do this before reading anything. dest, err := brain.NotePath(brainDir, opts.Wing, opts.Hall, slug) if err != nil { return "", err } - raw, srcRel, err := readRawSource(brainDir, base) + raw, srcRel, err := readRawSource(brainDir, base, opts.Queue) if err != nil { return "", err } @@ -198,6 +226,7 @@ func PromoteNote(brainDir string, opts PromoteOptions) (string, error) { type RejectOptions struct { Filename string // basename in brain/raw/ Reason string // optional + Queue string // as PromoteOptions.Queue } // RejectNote records a human decision not to promote a brain/raw/ note, @@ -220,7 +249,7 @@ func RejectNote(brainDir string, opts RejectOptions) (string, error) { return "", fmt.Errorf("invalid filename %q", opts.Filename) } - raw, srcRel, err := readRawSource(brainDir, base) + raw, srcRel, err := readRawSource(brainDir, base, opts.Queue) if err != nil { return "", err } @@ -263,7 +292,10 @@ func excerpt(s string, n int) string { // readRawSource reads a pending note for promote/reject through the raw root, // refusing symlinks and a symlinked raw/ (hyperguild#153). -func readRawSource(brainDir, base string) ([]byte, string, error) { +func readRawSource(brainDir, base, queue string) ([]byte, string, error) { + if queue != "" && queue != QueueInbox && queue != QueueNeedsReview { + return nil, "", fmt.Errorf("unknown queue %q (want %q or %q)", queue, QueueInbox, QueueNeedsReview) + } root, _, err := OpenRawRoot(brainDir) if err != nil { if errors.Is(err, os.ErrNotExist) { @@ -272,34 +304,49 @@ func readRawSource(brainDir, base string) ([]byte, string, error) { return nil, "", fmt.Errorf("read source: %w", err) } defer func() { _ = root.Close() }() - raw, _, err := readRawNote(root, base) - if err == nil { - return raw, "raw/" + base, nil + + var topRaw, parkedRaw []byte + var topErr, parkedErr error + if queue != QueueNeedsReview { + topRaw, _, topErr = readRawNote(root, base) } - if !errors.Is(err, os.ErrNotExist) { - return nil, "", fmt.Errorf("read source: %w", err) - } - // brain_pending also lists notes parked in raw/needs-review/ under their - // bare filename (hyperguild#208); the top level wins on a name clash. The - // sub-root refuses a symlinked directory like OpenRawRoot does. - sub, serr := root.OpenRoot(parkedDir) - if serr != nil { - return nil, "", fmt.Errorf("pending note %q does not exist in raw/", base) - } - defer func() { _ = sub.Close() }() - raw, _, err = readRawNote(sub, base) - if err != nil { - if errors.Is(err, os.ErrNotExist) { - return nil, "", fmt.Errorf("pending note %q does not exist in raw/", base) + if queue != QueueInbox { + // The sub-root refuses a symlinked directory like OpenRawRoot does. + if sub, serr := root.OpenRoot(parkedDir); serr != nil { + parkedErr = serr + } else { + parkedRaw, _, parkedErr = readRawNote(sub, base) + _ = sub.Close() } - return nil, "", fmt.Errorf("read source: %w", err) } - return raw, "raw/" + parkedDir + "/" + base, nil + haveTop := queue != QueueNeedsReview && topErr == nil + haveParked := queue != QueueInbox && parkedErr == nil + switch { + case haveTop && haveParked: + return nil, "", fmt.Errorf("%q is in both raw/ and raw/%s/: name the queue (%q or %q)", base, parkedDir, QueueInbox, QueueNeedsReview) + case haveTop: + return topRaw, "raw/" + base, nil + case haveParked: + return parkedRaw, "raw/" + parkedDir + "/" + base, nil + } + for _, e := range []error{topErr, parkedErr} { + if e != nil && !errors.Is(e, os.ErrNotExist) { + return nil, "", fmt.Errorf("read source: %w", e) + } + } + return nil, "", fmt.Errorf("pending note %q does not exist in raw/", base) } // parkedDir is where the watcher parks notes it could not place. const parkedDir = "needs-review" +// The two review queues brain_pending lists. promote and reject take one of +// them (or neither, when the name is unambiguous). +const ( + QueueInbox = "inbox" // raw/ + QueueNeedsReview = "needs-review" // raw/needs-review/ +) + // unquoteScalar undoes the Go-style double quoting the watcher writes around a // free-text value (strconv.Quote keeps it on one line); a bare value is // returned as is. diff --git a/ingestion/internal/api/promote_queue_test.go b/ingestion/internal/api/promote_queue_test.go new file mode 100644 index 0000000..7e58e95 --- /dev/null +++ b/ingestion/internal/api/promote_queue_test.go @@ -0,0 +1,126 @@ +package api + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// brain_pending lists a top-level note and a parked note of the same name as two +// entries with the same filename. Acting on "a.md" must not silently pick one. +func TestPromoteNote_ANameInBothQueuesNeedsTheQueueToBeNamed(t *testing.T) { + dir := t.TempDir() + writeRaw(t, dir, "a.md", "---\ncreated_at: 2026-10-20T08:00:00Z\n---\nTop-level body.\n") + writeParked(t, dir, "a.md", parkedNote) + + _, err := PromoteNote(dir, PromoteOptions{Filename: "a.md", Wing: "homelab", Hall: "facts"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "both") + assert.Contains(t, err.Error(), "queue") + _, statErr := os.Stat(filepath.Join(dir, "wiki/homelab/facts/a.md")) + assert.ErrorIs(t, statErr, os.ErrNotExist, "nothing was promoted") + + rel, err := PromoteNote(dir, PromoteOptions{Filename: "a.md", Wing: "homelab", Hall: "facts", Queue: QueueNeedsReview}) + require.NoError(t, err) + got, _ := os.ReadFile(filepath.Join(dir, rel)) + assert.Contains(t, string(got), "Body of the parked note.") + _, statErr = os.Stat(filepath.Join(dir, "raw", "a.md")) + assert.NoError(t, statErr, "the top-level note of the same name is untouched") +} + +func TestPromoteNote_QueueInboxSelectsTheTopLevelNote(t *testing.T) { + dir := t.TempDir() + writeRaw(t, dir, "a.md", "---\ncreated_at: 2026-10-20T08:00:00Z\n---\nTop-level body.\n") + writeParked(t, dir, "a.md", parkedNote) + + rel, err := PromoteNote(dir, PromoteOptions{Filename: "a.md", Wing: "homelab", Hall: "facts", Queue: QueueInbox}) + require.NoError(t, err) + got, _ := os.ReadFile(filepath.Join(dir, rel)) + assert.Contains(t, string(got), "Top-level body.") + _, statErr := os.Stat(filepath.Join(dir, "raw", "needs-review", "a.md")) + assert.NoError(t, statErr, "the parked note is untouched") +} + +func TestPromoteNote_ANamedQueueThatDoesNotHoldTheNoteIsAnError(t *testing.T) { + dir := t.TempDir() + writeRaw(t, dir, "a.md", "top") + _, err := PromoteNote(dir, PromoteOptions{Filename: "a.md", Wing: "homelab", Hall: "facts", Queue: QueueNeedsReview}) + assert.Error(t, err) + _, err = PromoteNote(dir, PromoteOptions{Filename: "a.md", Wing: "homelab", Hall: "facts", Queue: "elsewhere"}) + assert.Error(t, err, "an unknown queue name is refused") +} + +func TestRejectNote_ANameInBothQueuesNeedsTheQueueToBeNamed(t *testing.T) { + dir := t.TempDir() + writeRaw(t, dir, "a.md", "top") + writeParked(t, dir, "a.md", parkedNote) + + _, err := RejectNote(dir, RejectOptions{Filename: "a.md"}) + require.Error(t, err) + _, err = RejectNote(dir, RejectOptions{Filename: "a.md", Queue: QueueNeedsReview}) + require.NoError(t, err) + _, statErr := os.Stat(filepath.Join(dir, "raw", "a.md")) + assert.NoError(t, statErr, "the top-level note is untouched") +} + +func TestPromoteNote_NeverIntoARetiredLayoutName(t *testing.T) { + for _, w := range []string{"sources", "concepts", "entities", "wiki"} { + t.Run(w, func(t *testing.T) { + dir := t.TempDir() + writeRaw(t, dir, "a.md", "top") + _, err := PromoteNote(dir, PromoteOptions{Filename: "a.md", Wing: w, Hall: "facts"}) + assert.Error(t, err) + _, statErr := os.Stat(filepath.Join(dir, "wiki", w)) + assert.ErrorIs(t, statErr, os.ErrNotExist) + }) + } +} + +func TestListPending_LabelsEveryEntryWithItsQueue(t *testing.T) { + dir := t.TempDir() + writeRaw(t, dir, "fresh.md", "---\ncreated_at: 2026-10-20T08:00:00Z\n---\nx\n") + writeParked(t, dir, "p.md", parkedNote) + got, err := ListPending(dir) + require.NoError(t, err) + require.Len(t, got, 2) + queues := map[string]string{} + for _, n := range got { + queues[n.Filename] = n.Queue + } + assert.Equal(t, QueueInbox, queues["fresh.md"]) + assert.Equal(t, QueueNeedsReview, queues["p.md"]) +} + +// Parked notes that are not markdown (a pdf, a txt) were invisible: the only +// trace was a log.md line. +func TestListPending_ShowsParkedNonMarkdownFiles(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "raw", "needs-review"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "raw", "needs-review", "paper.pdf"), []byte("%PDF-1.4 binary"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "raw", "needs-review", "plain.txt"), []byte("some text"), 0o644)) + + got, err := ListPending(dir) + require.NoError(t, err) + names := map[string]PendingNote{} + for _, n := range got { + names[n.Filename] = n + } + require.Contains(t, names, "paper.pdf") + require.Contains(t, names, "plain.txt") + assert.Equal(t, QueueNeedsReview, names["paper.pdf"].Queue) + assert.Contains(t, names["paper.pdf"].Excerpt, "proposal", "points the reader at where the proposal is") +} + +func TestPromoteNote_OnlyMarkdownCanBePromoted(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "raw", "needs-review"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "raw", "needs-review", "paper.pdf"), []byte("%PDF binary"), 0o644)) + _, err := PromoteNote(dir, PromoteOptions{Filename: "paper.pdf", Wing: "homelab", Hall: "facts"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "markdown") + _, statErr := os.Stat(filepath.Join(dir, "raw", "needs-review", "paper.pdf")) + assert.NoError(t, statErr, "refused, untouched") +} diff --git a/ingestion/internal/api/promote_review_test.go b/ingestion/internal/api/promote_review_test.go index 37e7df5..66490de 100644 --- a/ingestion/internal/api/promote_review_test.go +++ b/ingestion/internal/api/promote_review_test.go @@ -48,19 +48,6 @@ func TestRejectNote_ActsOnANoteParkedInNeedsReview(t *testing.T) { assert.Empty(t, pending, "a rejected note is not re-listed") } -func TestPromoteNote_ATopLevelNoteWinsOverAParkedOneOfTheSameName(t *testing.T) { - dir := t.TempDir() - writeRaw(t, dir, "same.md", "---\ncreated_at: 2026-10-20T08:00:00Z\n---\nTop-level body.\n") - writeParked(t, dir, "same.md", parkedNote) - - _, err := PromoteNote(dir, PromoteOptions{Filename: "same.md", Wing: "homelab", Hall: "facts"}) - require.NoError(t, err) - got, _ := os.ReadFile(filepath.Join(dir, "wiki/homelab/facts/same.md")) - assert.Contains(t, string(got), "Top-level body.") - _, statErr := os.Stat(filepath.Join(dir, "raw", "needs-review", "same.md")) - assert.NoError(t, statErr, "the parked note of the same name is left alone") -} - func TestPromoteNote_ASymlinkedNeedsReviewIsNotRead(t *testing.T) { dir := t.TempDir() outside := t.TempDir() diff --git a/ingestion/internal/mcp/handlers.go b/ingestion/internal/mcp/handlers.go index 62872e1..7556086 100644 --- a/ingestion/internal/mcp/handlers.go +++ b/ingestion/internal/mcp/handlers.go @@ -94,6 +94,7 @@ func (s *Server) tools() []map[string]any { "wing": str("target wing, e.g. jepa-fx"), "hall": enum("target hall", halls...), "slug": str("optional target slug; defaults to filename minus date prefix"), + "queue": enum("which brain_pending listing the note is in; required only when the name exists in both", "inbox", "needs-review"), }), }, { @@ -102,6 +103,7 @@ func (s *Server) tools() []map[string]any { "inputSchema": schema([]string{"filename"}, map[string]any{ "filename": str("basename in brain/raw/, e.g. 2026-04-23-fed-sentiment-scrape.md"), "reason": str("optional; why this isn't worth promoting"), + "queue": enum("which brain_pending listing the note is in; required only when the name exists in both", "inbox", "needs-review"), }), }, noteToolDescriptors()[0], @@ -383,6 +385,7 @@ type brainPromoteArgs struct { Wing string `json:"wing"` Hall string `json:"hall"` Slug string `json:"slug,omitempty"` + Queue string `json:"queue,omitempty"` } // brainPromote moves a raw/ note into the structured wiki (frontmatter @@ -395,7 +398,7 @@ func (s *Server) brainPromote(ctx context.Context, args json.RawMessage) (json.R } origin, writtenBy := provenanceFrom(ctx) relPath, err := api.PromoteNote(s.brainDir, api.PromoteOptions{ - Filename: a.Filename, Wing: a.Wing, Hall: a.Hall, Slug: a.Slug, + Filename: a.Filename, Wing: a.Wing, Hall: a.Hall, Slug: a.Slug, Queue: a.Queue, Origin: origin, WrittenBy: writtenBy, }) if err != nil { @@ -408,6 +411,7 @@ func (s *Server) brainPromote(ctx context.Context, args json.RawMessage) (json.R type brainRejectArgs struct { Filename string `json:"filename"` Reason string `json:"reason,omitempty"` + Queue string `json:"queue,omitempty"` } // brainReject records a decision not to promote a raw/ note (api.RejectNote: @@ -418,7 +422,7 @@ func (s *Server) brainReject(_ context.Context, args json.RawMessage) (json.RawM if err := json.Unmarshal(args, &a); err != nil { return nil, fmt.Errorf("parse args: %w", err) } - relPath, err := api.RejectNote(s.brainDir, api.RejectOptions{Filename: a.Filename, Reason: a.Reason}) + relPath, err := api.RejectNote(s.brainDir, api.RejectOptions{Filename: a.Filename, Reason: a.Reason, Queue: a.Queue}) if err != nil { return nil, err } diff --git a/ingestion/internal/mcp/handlers_test.go b/ingestion/internal/mcp/handlers_test.go index 84b5114..d03b9ac 100644 --- a/ingestion/internal/mcp/handlers_test.go +++ b/ingestion/internal/mcp/handlers_test.go @@ -431,3 +431,28 @@ func TestBrainRejectMissingSourceErrors(t *testing.T) { resp := toolCall(t, srv, "brain_reject", map[string]any{"filename": "ghost.md"}) require.NotNil(t, resp["error"]) } + +// brain_pending lists two entries for one name when a top-level note and a +// parked one share it; the tool must let the caller say which it means. +func TestBrainPromoteAndRejectTakeTheQueueOfAnAmbiguousName(t *testing.T) { + brainDir := t.TempDir() + raw := filepath.Join(brainDir, "raw") + require.NoError(t, os.MkdirAll(filepath.Join(raw, "needs-review"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(raw, "a.md"), []byte("---\ncreated_at: 2026-10-20T00:00:00Z\n---\nTOP\n"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(raw, "needs-review", "a.md"), []byte("---\nproposed_wing: homelab\n---\nPARKED\n"), 0o644)) + srv := mcp.NewServer(brainDir, nil, nil, nil) + + resp := toolCall(t, srv, "brain_promote", map[string]any{"filename": "a.md", "wing": "homelab", "hall": "facts"}) + require.NotNil(t, resp["error"], "an ambiguous name is refused, not guessed") + + resp = toolCall(t, srv, "brain_promote", map[string]any{"filename": "a.md", "wing": "homelab", "hall": "facts", "queue": "needs-review"}) + require.Nil(t, resp["error"], "got: %v", resp["error"]) + got, err := os.ReadFile(filepath.Join(brainDir, "wiki/homelab/facts/a.md")) + require.NoError(t, err) + assert.Contains(t, string(got), "PARKED") + + resp = toolCall(t, srv, "brain_reject", map[string]any{"filename": "a.md", "queue": "inbox", "reason": "no"}) + require.Nil(t, resp["error"], "got: %v", resp["error"]) + _, err = os.Stat(filepath.Join(brainDir, "raw/rejected/a.md")) + assert.NoError(t, err) +} diff --git a/ingestion/internal/watcher/watcher.go b/ingestion/internal/watcher/watcher.go index 9956315..94b941c 100644 --- a/ingestion/internal/watcher/watcher.go +++ b/ingestion/internal/watcher/watcher.go @@ -392,14 +392,23 @@ func topDir(rel string) string { return "" } -// frontmatterWing reads `wing:` from a note's leading frontmatter, "" if none. +// frontmatterWing reads a top-level `wing:` from a note's leading frontmatter, +// "" if there is none. The block counts only if it has a real closing fence (a +// line that is exactly "---"), and an indented `wing:` belongs to some other +// mapping, not to the note. func frontmatterWing(content string) string { content = strings.ReplaceAll(content, "\r\n", "\n") if !strings.HasPrefix(content, "---\n") { return "" } - block, _, _ := strings.Cut(content[4:], "\n---") + block, _, found := cutFence(content[4:]) + if !found { + return "" + } for _, line := range strings.Split(block, "\n") { + if line == "" || line[0] == ' ' || line[0] == '\t' { + continue + } if key, val, ok := strings.Cut(line, ":"); ok && strings.TrimSpace(key) == "wing" { return strings.Trim(strings.TrimSpace(val), `"'`) } diff --git a/ingestion/internal/watcher/watcher_wing_test.go b/ingestion/internal/watcher/watcher_wing_test.go index 3d6f717..ff7a68b 100644 --- a/ingestion/internal/watcher/watcher_wing_test.go +++ b/ingestion/internal/watcher/watcher_wing_test.go @@ -314,3 +314,24 @@ func TestProcessDir_ANoteThatReappearsChangedIsExtractedAgain(t *testing.T) { assert.Equal(t, int32(2), llmCalls.Load(), "a real edit must still be picked up") } + +func TestFrontmatterWing(t *testing.T) { + tests := []struct { + name, in, want string + }{ + {"plain key", "---\nwing: radar\n---\nbody", "radar"}, + {"quoted", "---\nwing: \"radar\"\n---\nbody", "radar"}, + {"CRLF", "---\r\nwing: radar\r\n---\r\nbody", "radar"}, + {"no frontmatter", "wing: radar\nbody", ""}, + {"an indented key belongs to another mapping", "---\nmeta:\n wing: radar\n---\nbody", ""}, + {"a key before a non-fence dash line is not frontmatter", "---\nwing: radar\n----\nbody", ""}, + {"no closing fence", "---\nwing: radar\nbody", ""}, + {"closing fence at end of input", "---\nwing: radar\n---", "radar"}, + {"another key first", "---\ntitle: x\nwing: radar\n---\nbody", "radar"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, frontmatterWing(tt.in)) + }) + } +} -- 2.54.0 From 74d6a08c7bb82b3ff55bea8830d40617b4d047cb Mon Sep 17 00:00:00 2001 From: Mathias Date: Tue, 20 Oct 2026 20:43:39 +0200 Subject: [PATCH 10/14] fix(pipeline): slug clashes inside one batch; the source page is never the one skipped (review of #211, pass 2) dropSlugClashes only compared against what was on disk, so a clip named like an entity the model extracted wrote both pages and neither could be updated again. In-batch winners: source over entity over concept. Against the brain, only entities/concepts are skipped; the source's slug was already chosen free or its own by resolveSourceSlug, so an existing bad pair no longer locks the source out. Mutation-checked. Co-Authored-By: Claude Sonnet 5.5 --- ingestion/internal/pipeline/pipeline.go | 48 +++++++++++++-- .../internal/pipeline/wing_layout_test.go | 59 +++++++++++++++++++ 2 files changed, 102 insertions(+), 5 deletions(-) diff --git a/ingestion/internal/pipeline/pipeline.go b/ingestion/internal/pipeline/pipeline.go index 51d5dd3..f3ed9c4 100644 --- a/ingestion/internal/pipeline/pipeline.go +++ b/ingestion/internal/pipeline/pipeline.go @@ -372,15 +372,53 @@ func resolveSourceSlug(brainDir, wing, base string) (string, error) { } // dropSlugClashes removes, from the pages about to be written in a wing, any -// page whose slug already belongs to a different page elsewhere in the brain. -// The page's own path is not a clash (a shared entity merging into itself). -// A skipped page is a warning, not a failure: the ingest as a whole is fine, -// and one extracted page must not be able to lock a source out of re-ingest. +// page whose slug belongs to a different page. A slug is the graph identity, so +// two pages must not share one; and a page that shares one cannot be updated +// again, because every later ingest sees the other as its clash. +// +// Two kinds of clash, both by slug, neither failing the ingest (one extracted +// page must not be able to lock a source out of re-ingest): +// - inside the batch (a clip named notes.md whose model output also has an +// entity "Notes"): the source wins over an entity, an entity over a concept; +// - against the brain: an entity or concept whose slug is another page's. The +// SOURCE page is exempt: it is the note's own output, and its slug was +// already chosen free (or its own) by resolveSourceSlug. +// +// A page's own path is never a clash (a shared entity merging into itself). func dropSlugClashes(brainDir string, pages []wiki.Page) ([]wiki.Page, []string, error) { + rank := func(p wiki.Page) int { + switch { + case isSourcePath(p.Path): + return 0 + case strings.HasPrefix(p.Path, "wiki/entities/"): + return 1 + case strings.HasPrefix(p.Path, "wiki/concepts/"): + return 2 + } + return 3 + } + slugOf := func(p wiki.Page) string { return strings.TrimSuffix(filepath.Base(p.Path), ".md") } + + winner := map[string]wiki.Page{} + for _, p := range pages { + w, seen := winner[slugOf(p)] + if !seen || (w.Path != p.Path && rank(p) < rank(w)) { + winner[slugOf(p)] = p + } + } + var kept []wiki.Page var warnings []string for _, p := range pages { - slug := strings.TrimSuffix(filepath.Base(p.Path), ".md") + slug := slugOf(p) + if w := winner[slug]; w.Path != p.Path { + warnings = append(warnings, fmt.Sprintf("skipped %s: slug %q is also used by %s in this batch", p.Path, slug, w.Path)) + continue + } + if rank(p) == 0 { + kept = append(kept, p) + continue + } existing, err := wiki.FindSlugs(brainDir, slug) if err != nil { return nil, nil, fmt.Errorf("check slug %q: %w", slug, err) diff --git a/ingestion/internal/pipeline/wing_layout_test.go b/ingestion/internal/pipeline/wing_layout_test.go index 5664e4c..a881690 100644 --- a/ingestion/internal/pipeline/wing_layout_test.go +++ b/ingestion/internal/pipeline/wing_layout_test.go @@ -233,3 +233,62 @@ func TestResolveSourceSlug_AFurtherClashGetsANumber(t *testing.T) { require.NoError(t, err) assert.Equal(t, "talk-b-2", got) } + +// From the second review of #211: slug clashes INSIDE one batch. dropSlugClashes +// only looked at what was already on disk, so a source and an entity extracted +// together with one slug were both written, and neither could ever be updated. +func TestRun_ASourceAndAnEntityWithOneSlugInTheSameBatchWriteOnlyTheSource(t *testing.T) { + dir := t.TempDir() + pages := []RawPage{ + {Title: "Notes", Type: "source", Subtype: "note", Content: "## Summary\n\nfirst\n"}, + {Title: "Notes", Type: "entity", Subtype: "tool", Content: "## Definition\n\nan entity named like its source\n"}, + } + res, err := Run(context.Background(), wingCfg("homelab", pages...), dir, "body one", "notes", false) + require.NoError(t, err) + assert.True(t, exists(dir, "wiki/homelab/sources/notes.md")) + assert.False(t, exists(dir, "wiki/entities/notes.md"), "two pages must not share a slug") + assert.Contains(t, strings.Join(res.Warnings, "\n"), "notes") + + pages[0].Content = "## Summary\n\nfirst\n\n## Key Claims\n\n- second\n" + _, err = Run(context.Background(), wingCfg("homelab", pages...), dir, "body two, changed", "notes", false) + require.NoError(t, err) + assert.Contains(t, read(t, dir, "wiki/homelab/sources/notes.md"), "second", "the source can still be updated") +} + +func TestRun_AConceptAndAnEntityWithOneSlugInTheSameBatchWriteOne(t *testing.T) { + dir := t.TempDir() + pages := []RawPage{ + {Title: "Talk", Type: "source", Subtype: "video", Content: "## Summary\n\nx\n"}, + {Title: "Foo", Type: "concept", Content: "## Definition\n\na concept\n"}, + {Title: "Foo", Type: "entity", Subtype: "tool", Content: "## Definition\n\nan entity\n"}, + } + res, err := Run(context.Background(), wingCfg("homelab", pages...), dir, "body", "talk", false) + require.NoError(t, err) + written := 0 + for _, rel := range []string{"wiki/concepts/foo.md", "wiki/entities/foo.md"} { + if exists(dir, rel) { + written++ + } + } + assert.Equal(t, 1, written, "one slug, one page") + assert.Contains(t, strings.Join(res.Warnings, "\n"), "foo") +} + +// A brain that already holds the bad pair (written before this fix) must not +// lock the source out: the source page is the note's own output and is never +// the one skipped. +func TestRun_AnExistingSourceEntityPairDoesNotLockTheSourceOut(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "wiki/homelab/sources"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "wiki/entities"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "wiki/homelab/sources/notes.md"), + []byte("---\ntitle: 'Notes'\ntype: 'note'\nwing: homelab\nhall: sources\naliases:\n - 'Notes'\n---\n\n## Summary\n\nold\n"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "wiki/entities/notes.md"), + []byte("---\ntitle: 'Notes'\naliases:\n - 'Notes'\n---\n\n## Definition\n\nstale entity\n"), 0o644)) + + pages := []RawPage{{Title: "Notes", Type: "source", Subtype: "note", Content: "## Summary\n\nnew text\n\n## Key Claims\n\n- fresh\n"}} + _, err := Run(context.Background(), wingCfg("homelab", pages...), dir, "changed body", "notes", false) + require.NoError(t, err) + assert.Contains(t, read(t, dir, "wiki/homelab/sources/notes.md"), "fresh", "the source took the update") + assert.Contains(t, read(t, dir, "wiki/entities/notes.md"), "stale entity", "the entity is untouched") +} -- 2.54.0 From 6f5fb481ad06becbda25e4db27eb0566025302ad Mon Sep 17 00:00:00 2001 From: Mathias Date: Tue, 20 Oct 2026 20:45:00 +0200 Subject: [PATCH 11/14] fix(wing): client filter handles multi-word/\s/\b patterns, homoglyphs, marks, fillers; checked in bounded chunks (review of #211, pass 2) Views of each chunk: as is; NFKD + marks/format/fillers dropped + Cyrillic/Greek lookalikes mapped + dashes normalised, with whitespace collapsed (acme corp, \s) or removed (acme-\nclient); and a soft hyphen read as a dash. Chunks of 1 MiB with a 4 KiB overlap bound the folding's allocation (it was ~14x the input: 919 MiB for a 64 MiB note). Documented limits: leetspeak, punctuation splits, encodings and homoglyphs outside a small table. Mutation-checked. Co-Authored-By: Claude Sonnet 5.5 --- ingestion/internal/wing/router.go | 111 ++++++++++++++++++++++--- ingestion/internal/wing/router_test.go | 64 +++++++++++++- 2 files changed, 161 insertions(+), 14 deletions(-) diff --git a/ingestion/internal/wing/router.go b/ingestion/internal/wing/router.go index 8f628db..8d284ef 100644 --- a/ingestion/internal/wing/router.go +++ b/ingestion/internal/wing/router.go @@ -26,6 +26,12 @@ type Router struct { BrainDir string Default string ClientBlock *regexp.Regexp // nil = no content check + + // FilterChunk is how many bytes of a note the content filter folds and + // matches at a time (0 = 1 MiB). Folding allocates several times its input, + // so a 64 MiB note is checked in pieces (with an overlap, so a marker on a + // boundary is still found) instead of all at once. + FilterChunk int } // Resolve implements Placer. It never returns an error: anything it cannot @@ -47,7 +53,7 @@ func (r Router) Resolve(_ context.Context, n Note) Decision { case fm != "": return r.explicit(fm, "frontmatter") } - if r.ClientBlock != nil && matchesClient(r.ClientBlock, n.Text) { + if r.ClientBlock != nil && matchesClient(r.ClientBlock, n.Text, r.FilterChunk) { return exception("", 0, "text matches the client-content marker; not defaulted into a shared wing") } if r.Default == "" || !usableWingName(r.Default) { @@ -109,25 +115,104 @@ func isASCII(s string) bool { return true } -// matchesClient applies the client-content pattern to the text and to two -// folded forms of it, so the cheap evasions do not defeat it: NFKC (fullwidth -// letters), format characters removed (zero-width joiners, soft hyphens), every -// dash made "-", and whitespace removed ("acme-\nclient", "acme - client"). It catches what it names and nothing else; it is a filter, -// not a classifier. -func matchesClient(re *regexp.Regexp, text string) bool { - folded := xnorm.NFKC.String(text) +const ( + defaultFilterChunk = 1 << 20 + filterOverlap = 4096 +) + +// matchesClient applies the client-content pattern to the text in bounded +// chunks, each overlapping the next, so memory stays proportional to the chunk +// and a marker on a chunk edge is still found. +func matchesClient(re *regexp.Regexp, text string, chunk int) bool { + if chunk <= 0 { + chunk = defaultFilterChunk + } + overlap := filterOverlap + if overlap >= chunk { + overlap = chunk / 2 + } + for start := 0; start < len(text); { + end := start + chunk + if end >= len(text) { + end = len(text) + } + if matchesFolded(re, text[start:end]) { + return true + } + if end == len(text) { + return false + } + // A cut inside a rune is harmless: the overlap carries every marker + // shorter than it whole into the next chunk. + start = end - overlap + } + return false +} + +// matchesFolded matches the pattern against the chunk as it is and against +// folded forms, so the cheap evasions do not defeat it. Folding: NFKD (fullwidth +// letters, accents become base letter + mark), then marks, format characters and +// invisible fillers dropped, Cyrillic/Greek lookalikes mapped to Latin, every +// dash made "-". Views: whitespace collapsed to one space (so "acme corp" and +// \s work), whitespace removed (so "acme-\nclient" works), and the same with a +// soft hyphen read as a dash. It catches what it names and nothing else: +// leetspeak, punctuation splits (a.c.m.e), encodings and the many homoglyphs +// not in the small table below are not caught. It is a filter, not a classifier. +func matchesFolded(re *regexp.Regexp, chunk string) bool { + if re.MatchString(chunk) { + return true + } + decomposed := xnorm.NFKD.String(chunk) + return re.MatchString(fold(decomposed, false, true)) || + re.MatchString(fold(decomposed, false, false)) || + re.MatchString(fold(decomposed, true, false)) +} + +// fold builds one view. softAsDash reads U+00AD as "-" instead of dropping it; +// keepSpace collapses whitespace to a single space instead of removing it. +func fold(s string, softAsDash, keepSpace bool) string { var b strings.Builder - for _, r := range folded { + b.Grow(len(s)) + space := false + for _, r := range s { switch { - case unicode.Is(unicode.Cf, r): + case r == '\u00ad' && softAsDash: + b.WriteRune('-') + space = false continue + case unicode.Is(unicode.Mn, r) || unicode.Is(unicode.Me, r) || unicode.Is(unicode.Mc, r) || + unicode.Is(unicode.Cf, r) || invisibleFiller[r]: + continue + case unicode.IsSpace(r) || unicode.Is(unicode.Zs, r): + if keepSpace && !space { + b.WriteByte(' ') + space = true + } + continue + } + space = false + switch { case unicode.Is(unicode.Pd, r) || r == '\u2212': b.WriteRune('-') default: + if l, ok := latinLookalike[r]; ok { + r = l + } b.WriteRune(r) } } - clean := b.String() - squeezed := strings.Join(strings.Fields(clean), "") - return re.MatchString(text) || re.MatchString(squeezed) + return b.String() +} + +// invisibleFiller are characters that render as nothing but are not format +// characters: Hangul fillers and the blank Braille pattern. +var invisibleFiller = map[rune]bool{0x115F: true, 0x1160: true, 0x3164: true, 0xFFA0: true, 0x2800: true} + +// latinLookalike maps the commonest Cyrillic and Greek letters that read as +// Latin ones. Deliberately small; see matchesFolded. +var latinLookalike = map[rune]rune{ + '\u0430': 'a', '\u0435': 'e', '\u043e': 'o', '\u0440': 'p', '\u0441': 'c', '\u0445': 'x', '\u0443': 'y', + '\u0456': 'i', '\u0458': 'j', '\u0455': 's', '\u04bb': 'h', '\u0501': 'd', '\u043a': 'k', '\u043d': 'h', '\u0442': 't', + '\u0410': 'A', '\u0412': 'B', '\u0415': 'E', '\u041a': 'K', '\u041c': 'M', '\u041d': 'H', '\u041e': 'O', '\u0420': 'P', '\u0421': 'C', '\u0422': 'T', '\u0425': 'X', + '\u03bf': 'o', '\u03bd': 'v', '\u03c1': 'p', '\u0391': 'A', '\u0392': 'B', '\u0395': 'E', '\u039f': 'O', '\u03a1': 'P', } diff --git a/ingestion/internal/wing/router_test.go b/ingestion/internal/wing/router_test.go index 68c9ed5..384192f 100644 --- a/ingestion/internal/wing/router_test.go +++ b/ingestion/internal/wing/router_test.go @@ -3,6 +3,7 @@ package wing_test import ( "context" "regexp" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -148,7 +149,7 @@ func TestRouter_FolderNamesAreAsciiAndNotReserved(t *testing.T) { dir string }{ {"Kelvin sign lowercases to an ASCII k", "Kubernetes"}, - {"cyrillic lookalike", "hоmelab"}, + {"cyrillic lookalike", "h\u043emelab"}, {"retired layout name: sources", "sources"}, {"retired layout name: concepts", "concepts"}, {"retired layout name: entities", "entities"}, @@ -172,3 +173,64 @@ func TestNewRouter_AReservedDefaultIsRefused(t *testing.T) { _, err := wing.NewRouter(routerBrain(t), "sources", "") assert.Error(t, err) } + +// From the second review of #211: the multi-word, \s and \b patterns an operator +// would actually write, and the next tier of evasions. +func TestRouter_ClientMarkerHandlesRealisticPatternsAndHarderEvasions(t *testing.T) { + tests := []struct { + name, pattern, text string + }{ + {"space in pattern vs NBSP", `acme corp`, "about acme\u00a0corp today"}, + {"space in pattern vs two spaces", `acme corp`, "about acme corp today"}, + {"space in pattern vs newline", `acme corp`, "about acme\ncorp today"}, + {"backslash-s pattern vs NBSP", `acme\s+corp`, "about acme\u00a0corp today"}, + {"word boundary vs a zero-width char inside", `\bacme\b`, "the ac\u200bme corp"}, + {"dash pattern vs a soft hyphen as the dash", `acme-client`, "acme\u00adclient"}, + {"cyrillic a", `acme-client`, "\u0430cme-client"}, + {"cyrillic e", `acme-client`, "acm\u0435-client"}, + {"cyrillic o and c", `coop`, "c\u043e\u043ep"}, + {"combining acute", `acme-client`, "a\u0301cme-client"}, + {"precomposed accent", `acme-client`, "\u00e1cme-client"}, + {"hangul filler", `acme-client`, "ac\u3164me-client"}, + {"halfwidth hangul filler", `acme-client`, "ac\uffa0me-client"}, + {"braille blank", `acme-client`, "ac\u2800me-client"}, + {"grapheme joiner", `acme-client`, "ac\u034fme-client"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r, err := wing.NewRouter(routerBrain(t), "web-clip", tt.pattern) + assert.NoError(t, err) + assert.Equal(t, wing.Exception, r.Resolve(context.Background(), wing.Note{Text: tt.text}).Source, "%q should be caught by %q", tt.text, tt.pattern) + }) + } + t.Run("ordinary text is not caught by a multi-word pattern", func(t *testing.T) { + r, _ := wing.NewRouter(routerBrain(t), "web-clip", `acme corp`) + assert.Equal(t, wing.Default, r.Resolve(context.Background(), wing.Note{Text: "acme is not corp, and corp is not acme"}).Source) + }) +} + +// The folding allocates several times the text, so a large note is checked in +// bounded chunks. A marker that straddles a chunk edge must still be found. +func TestRouter_ClientMarkerIsFoundAcrossAChunkBoundary(t *testing.T) { + r, err := wing.NewRouter(routerBrain(t), "web-clip", `acme-client`) + assert.NoError(t, err) + r.FilterChunk = 64 + for _, at := range []int{0, 30, 55, 58, 62, 64, 70, 120, 190} { + text := strings.Repeat("x", at) + "acme-client" + strings.Repeat("y", 200) + assert.Equal(t, wing.Exception, r.Resolve(context.Background(), wing.Note{Text: text}).Source, "marker at offset %d was missed", at) + } + clean := strings.Repeat("lorem ipsum ", 100) + assert.Equal(t, wing.Default, r.Resolve(context.Background(), wing.Note{Text: clean}).Source) +} + +func TestRouter_ClientMarkerChunkingDoesNotSplitAMultiByteRune(t *testing.T) { + r, err := wing.NewRouter(routerBrain(t), "web-clip", `acme-client`) + assert.NoError(t, err) + r.FilterChunk = 65 // odd, over two-byte runes: every boundary falls inside a rune unless it is moved + for pad := 0; pad < 8; pad++ { + text := strings.Repeat("\u00e9", 40+pad) + "acme-client" + strings.Repeat("\u00e9", 40) + assert.Equal(t, wing.Exception, r.Resolve(context.Background(), wing.Note{Text: text}).Source, "pad %d", pad) + } + clean := strings.Repeat("\u00e9", 300) + assert.Equal(t, wing.Default, r.Resolve(context.Background(), wing.Note{Text: clean}).Source, "no panic or false match on a text of 2-byte runes") +} -- 2.54.0 From 49286ba8b8d43f54c5a828db5aa80adb0c426456 Mon Sep 17 00:00:00 2001 From: Mathias Date: Tue, 20 Oct 2026 20:49:10 +0200 Subject: [PATCH 12/14] fix(api,watcher,server): reject moves non-markdown untouched; read caps; BOM; strict routing flag (review of #211, pass 2) - brain_reject on a parked pdf/txt moves the bytes; it no longer prepends YAML - raw/ notes are read through a cap (readCapped); an endless reader test fails instead of exhausting memory - a byte-order mark no longer hides a note's wing: key - INGEST_WING_ROUTING is parsed strictly; a typo stops the server rather than leaving routing silently off - tests for the reserved-wing check in every disguise (Sources, sources., ...) Mutation-checked. Co-Authored-By: Claude Sonnet 5.5 --- docs/raw-inbox.md | 4 +- ingestion/cmd/server/main.go | 22 ++++++- ingestion/cmd/server/main_test.go | 29 ++++++++ ingestion/internal/api/promote.go | 16 +++-- ingestion/internal/api/promote_queue_test.go | 66 +++++++++++++++++++ ingestion/internal/api/rawroot.go | 21 +++++- ingestion/internal/watcher/watcher.go | 1 + .../internal/watcher/watcher_wing_test.go | 4 ++ 8 files changed, 152 insertions(+), 11 deletions(-) diff --git a/docs/raw-inbox.md b/docs/raw-inbox.md index bb2e374..2814ff0 100644 --- a/docs/raw-inbox.md +++ b/docs/raw-inbox.md @@ -30,9 +30,9 @@ deployment: | Variable | Meaning | |---|---| -| `INGEST_WING_ROUTING=1` | enable the rules above | +| `INGEST_WING_ROUTING` | `1`/`true` enables the rules above; unset or `0`/`false` is off; any other value stops the server at startup | | `INGEST_DEFAULT_WING` | wing for unsignalled notes (default `web-clip`; must be a plain name, not `sources`/`concepts`/`entities`/`wiki`) | -| `INGEST_CLIENT_BLOCK` | case-insensitive regexp over note text; a match parks an unsignalled note. Falls back to `CLAUDE_INGEST_CLIENT_BLOCK`. Matched after folding fullwidth letters, removing zero-width characters, normalising dashes and removing whitespace. It catches what it names and nothing else | +| `INGEST_CLIENT_BLOCK` | case-insensitive regexp over note text; a match parks an unsignalled note. Falls back to `CLAUDE_INGEST_CLIENT_BLOCK`. Matched after folding fullwidth letters, removing zero-width characters, normalising dashes and removing whitespace. Patterns with spaces, `\\s` or `\\b` work (whitespace is collapsed in one view and removed in another). It catches what it names and nothing else: leetspeak, punctuation splits (`a.c.m.e`), encodings and homoglyphs outside a small Cyrillic/Greek table are not caught. Large notes are checked in 1 MiB chunks | A bad default or pattern stops the server at startup. A wing created while the server runs is usable without a restart. diff --git a/ingestion/cmd/server/main.go b/ingestion/cmd/server/main.go index 07d26e7..d33af07 100644 --- a/ingestion/cmd/server/main.go +++ b/ingestion/cmd/server/main.go @@ -150,6 +150,21 @@ func envInt(key string, fallback int) int { return fallback } +// envFlag reads a boolean switch. Unset is off; a value strconv.ParseBool does +// not understand is an error, so a typo ("ture") cannot silently leave a +// safety-relevant switch off. +func envFlag(key string) (bool, error) { + v := os.Getenv(key) + if v == "" { + return false, nil + } + b, err := strconv.ParseBool(v) + if err != nil { + return false, fmt.Errorf("%s=%q: want 1, 0, true or false", key, v) + } + return b, nil +} + // buildNotifier returns the ntfy notifier, or a nil interface when // BRAIN_NTFY_URL is unset. A nil *interface* (not a typed-nil) is required so // DegradingSink/Reconcile and the governance observer skip it cleanly. @@ -405,7 +420,12 @@ func main() { // the layout this flag exists to leave. The interface stays nil when off // (a nil *Router inside it would not be nil). var placer wing.Placer - if os.Getenv("INGEST_WING_ROUTING") == "1" { + routing, ferr := envFlag("INGEST_WING_ROUTING") + if ferr != nil { + logger.Error("INGEST_WING_ROUTING", "err", ferr) + os.Exit(1) + } + if routing { pattern := os.Getenv("INGEST_CLIENT_BLOCK") if pattern == "" { pattern = os.Getenv("CLAUDE_INGEST_CLIENT_BLOCK") diff --git a/ingestion/cmd/server/main_test.go b/ingestion/cmd/server/main_test.go index b52f0c7..24f7bb2 100644 --- a/ingestion/cmd/server/main_test.go +++ b/ingestion/cmd/server/main_test.go @@ -100,3 +100,32 @@ func TestClaudeSinkDoesNotWriteThroughASymlinkedArchiveDir(t *testing.T) { }) } } + +func TestEnvFlag(t *testing.T) { + tests := []struct { + name, set string + want bool + wantErr bool + }{ + {"unset is off", "", false, false}, + {"1", "1", true, false}, + {"true", "true", true, false}, + {"TRUE", "TRUE", true, false}, + {"0", "0", false, false}, + {"false", "false", false, false}, + {"a typo is an error, not silently off", "ture", false, true}, + {"yes is not a bool here", "yes", false, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("TEST_ENV_FLAG", tt.set) + got, err := envFlag("TEST_ENV_FLAG") + if tt.wantErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/ingestion/internal/api/promote.go b/ingestion/internal/api/promote.go index a0a86cb..f056398 100644 --- a/ingestion/internal/api/promote.go +++ b/ingestion/internal/api/promote.go @@ -261,15 +261,19 @@ func RejectNote(brainDir string, opts RejectOptions) (string, error) { return "", fmt.Errorf("a rejected note named %s already exists", filepath.ToSlash(rel)) } - fm, body := parseFrontmatter(string(raw)) - fm.set("rejected_at", time.Now().UTC().Format(time.RFC3339)) - if opts.Reason != "" { - fm.set("reject_reason", opts.Reason) - } + content := string(raw) + if strings.HasSuffix(base, ".md") { + fm, body := parseFrontmatter(string(raw)) + fm.set("rejected_at", time.Now().UTC().Format(time.RFC3339)) + if opts.Reason != "" { + fm.set("reject_reason", opts.Reason) + } + content = fm.render() + body + } // a pdf or txt is moved byte for byte: prepending frontmatter would alter it // Write-then-delete: the source survives any write failure. Both go // through the brain root (hyperguild#188). - if err := createBrainFile(brainDir, dest, []byte(fm.render()+body)); err != nil { + if err := createBrainFile(brainDir, dest, []byte(content)); err != nil { return "", fmt.Errorf("write rejected note: %w", err) } if err := removeBrainFile(brainDir, src); err != nil { diff --git a/ingestion/internal/api/promote_queue_test.go b/ingestion/internal/api/promote_queue_test.go index 7e58e95..32a1b74 100644 --- a/ingestion/internal/api/promote_queue_test.go +++ b/ingestion/internal/api/promote_queue_test.go @@ -1,8 +1,10 @@ package api import ( + "errors" "os" "path/filepath" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -124,3 +126,67 @@ func TestPromoteNote_OnlyMarkdownCanBePromoted(t *testing.T) { _, statErr := os.Stat(filepath.Join(dir, "raw", "needs-review", "paper.pdf")) assert.NoError(t, statErr, "refused, untouched") } + +// brain.Sanitise normalises case, trailing dots and separators; the reserved-name +// check must look at the normalised name or "Sources" and "sources." walk past it. +func TestPromoteNote_ReservedWingNamesAreRefusedInEveryDisguise(t *testing.T) { + for _, w := range []string{"Sources", "sources.", "SOURCES ", "Entities/", "concepts\\", " wiki "} { + t.Run(w, func(t *testing.T) { + dir := t.TempDir() + writeRaw(t, dir, "a.md", "top") + _, err := PromoteNote(dir, PromoteOptions{Filename: "a.md", Wing: w, Hall: "facts"}) + assert.Error(t, err) + _, statErr := os.Stat(filepath.Join(dir, "raw", "a.md")) + assert.NoError(t, statErr, "refused, untouched") + }) + } +} + +// Reject moves the bytes; it must not prepend frontmatter to a pdf or txt. +func TestRejectNote_AParkedNonMarkdownFileIsMovedUntouched(t *testing.T) { + dir := t.TempDir() + pdf := "%PDF-1.4\r\n\x00\xff binary \r\n" + require.NoError(t, os.MkdirAll(filepath.Join(dir, "raw", "needs-review"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "raw", "needs-review", "paper.pdf"), []byte(pdf), 0o644)) + + rel, err := RejectNote(dir, RejectOptions{Filename: "paper.pdf", Reason: "not relevant"}) + require.NoError(t, err) + got, err := os.ReadFile(filepath.Join(dir, rel)) + require.NoError(t, err) + assert.Equal(t, pdf, string(got), "byte for byte") + _, statErr := os.Stat(filepath.Join(dir, "raw", "needs-review", "paper.pdf")) + assert.ErrorIs(t, statErr, os.ErrNotExist) +} + +func TestReadRawNote_IsBoundedByTheCap(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "big.md"), []byte(strings.Repeat("x", 200)), 0o644)) + root, err := os.OpenRoot(dir) + require.NoError(t, err) + defer func() { _ = root.Close() }() + prev := maxRawNoteBytes + maxRawNoteBytes = 100 + t.Cleanup(func() { maxRawNoteBytes = prev }) + + _, _, err = readRawNote(root, "big.md") + require.Error(t, err) + assert.Contains(t, err.Error(), "exceeds") +} + +type endless struct{ n int64 } + +func (e *endless) Read(p []byte) (int, error) { + if e.n > 8<<20 { // a regression fails the test instead of exhausting memory + return 0, errors.New("endless reader drained") + } + e.n += int64(len(p)) + return len(p), nil +} + +func TestReadCapped_StopsAtTheCap(t *testing.T) { + e := &endless{} + _, err := readCapped(e, 1000) + require.Error(t, err) + assert.Contains(t, err.Error(), "exceeds") + assert.LessOrEqual(t, e.n, int64(1000+1+32*1024), "pulled far more than the cap: %d", e.n) +} diff --git a/ingestion/internal/api/rawroot.go b/ingestion/internal/api/rawroot.go index 203e9be..6beb5d0 100644 --- a/ingestion/internal/api/rawroot.go +++ b/ingestion/internal/api/rawroot.go @@ -38,6 +38,10 @@ func OpenRawRoot(brainDir string) (*os.Root, string, error) { return r, p, nil } +// maxRawNoteBytes bounds what promote, reject and the pending listing read from +// one raw/ entry. A variable so a test can lower it. +var maxRawNoteBytes int64 = 64 << 20 + // readRawNote reads one raw/ entry by basename through the root, refusing // anything that is not a regular file. // @@ -59,9 +63,22 @@ func readRawNote(r *os.Root, base string) ([]byte, os.FileInfo, error) { return nil, nil, err } defer func() { _ = f.Close() }() - b, err := io.ReadAll(f) + b, err := readCapped(f, maxRawNoteBytes) if err != nil { - return nil, nil, err + return nil, nil, fmt.Errorf("%s: %w", base, err) } return b, fi, nil } + +// readCapped reads r but never more than max+1 bytes, so an oversize or growing +// file costs at most that much memory. +func readCapped(r io.Reader, max int64) ([]byte, error) { + b, err := io.ReadAll(io.LimitReader(r, max+1)) + if err != nil { + return nil, err + } + if int64(len(b)) > max { + return nil, fmt.Errorf("note exceeds %d bytes", max) + } + return b, nil +} diff --git a/ingestion/internal/watcher/watcher.go b/ingestion/internal/watcher/watcher.go index 94b941c..8e7e3b2 100644 --- a/ingestion/internal/watcher/watcher.go +++ b/ingestion/internal/watcher/watcher.go @@ -397,6 +397,7 @@ func topDir(rel string) string { // line that is exactly "---"), and an indented `wing:` belongs to some other // mapping, not to the note. func frontmatterWing(content string) string { + content = strings.TrimPrefix(content, "\ufeff") // a byte-order mark must not hide the block content = strings.ReplaceAll(content, "\r\n", "\n") if !strings.HasPrefix(content, "---\n") { return "" diff --git a/ingestion/internal/watcher/watcher_wing_test.go b/ingestion/internal/watcher/watcher_wing_test.go index ff7a68b..16da56d 100644 --- a/ingestion/internal/watcher/watcher_wing_test.go +++ b/ingestion/internal/watcher/watcher_wing_test.go @@ -335,3 +335,7 @@ func TestFrontmatterWing(t *testing.T) { }) } } + +func TestFrontmatterWing_ABOMDoesNotHideTheKey(t *testing.T) { + assert.Equal(t, "homelab", frontmatterWing("\ufeff---\nwing: homelab\n---\nbody")) +} -- 2.54.0 From 27b1579b801b62069c1c545cf5beac3ce9976089 Mon Sep 17 00:00:00 2001 From: Mathias Date: Tue, 20 Oct 2026 20:56:03 +0200 Subject: [PATCH 13/14] fix(pipeline,api,wing): never archive a note as processed when nothing was ingested; titles do not merge sources; ties prefer the page on disk (review of #211, pass 3) - in a wing, an ingest that writes no source page is an error: the note goes to raw/failed with the warnings in log.md instead of leaving the inbox as 'processed' (model returned [], only invalid types, only clashing entities) - the wing's source path is deterministic, so Resolve no longer redirects a source onto another note's page because the model gave both the same title ('Home'); that overwrote the hash and the text - a batch tie between concept and entity prefers the page already on disk, so its update is not lost; the wing's own source page keeps its slug even when a foreign page shares it - brain_pending lists an unreadable (over-cap) note with the reason instead of dropping it; the client filter's 4 KiB match-span limit is documented Each has a test that fails without it (mutation-checked). Co-Authored-By: Claude Sonnet 5.5 --- docs/raw-inbox.md | 8 ++ ingestion/internal/api/promote.go | 10 +++ ingestion/internal/api/promote_queue_test.go | 18 +++++ ingestion/internal/pipeline/pipeline.go | 72 +++++++++++++++-- ingestion/internal/pipeline/resolve.go | 13 +++ .../internal/pipeline/wing_layout_test.go | 80 +++++++++++++++++++ .../internal/watcher/watcher_wing_test.go | 17 ++++ ingestion/internal/wing/router.go | 6 +- 8 files changed, 214 insertions(+), 10 deletions(-) diff --git a/docs/raw-inbox.md b/docs/raw-inbox.md index 2814ff0..c2c8adf 100644 --- a/docs/raw-inbox.md +++ b/docs/raw-inbox.md @@ -12,6 +12,7 @@ with the machines you drop files from. (hyperguild#205, #208, #209) | a note whose text matches the client-content pattern, with no folder | parked in `raw/needs-review/`, not extracted | | a folder or `wing:` naming an unknown, reserved or non-ASCII wing, or two disagreeing signals | parked in `raw/needs-review/` | | a note over the size cap, or one that cannot be archived | stays in `raw/`, reported once, not retried until it changes | +| a note the model returned nothing usable for (no source page could be written) | fails: the note is kept in `raw/failed/` and the reason is in `log.md`; it is never archived as processed | Concepts and entities are shared across wings because their slug is the graph's identity. A source whose slug is taken gets a wing-qualified one (`notes-homelab`). @@ -39,6 +40,13 @@ server runs is usable without a restart. ## Known limits +- The client filter works on 1 MiB chunks that overlap by 4 KiB. A match that can + span more than 4 KiB (crafted padding between two words of a multi-word + pattern, exactly at a chunk edge) is not guaranteed to be found; this affects + notes over 1 MiB only. On a 64 MiB note the check takes about ten seconds. +- A pattern that matches the empty string (`.*`, `x*`) parks every unsignalled + note. That fails closed and only the operator's own configuration can do it. + - A deliberate folder into a sensitive wing places the note there, and the shared concept/entity pages then cite that source by title. Client material does not belong in the brain at all; the pattern only guards the unsignalled lane. diff --git a/ingestion/internal/api/promote.go b/ingestion/internal/api/promote.go index f056398..fe9115e 100644 --- a/ingestion/internal/api/promote.go +++ b/ingestion/internal/api/promote.go @@ -97,6 +97,16 @@ func listNotes(root *os.Root, queue string) ([]PendingNote, error) { } raw, info, readErr := readRawNote(root, e.Name()) if readErr != nil { + // Not skipped: a note that cannot be read (over the cap, say) must + // not silently vanish from the queue it is waiting in. + fi, ierr := root.Lstat(e.Name()) + if ierr != nil { + continue + } + out = append(out, PendingNote{ + Filename: e.Name(), CreatedAt: fi.ModTime().UTC().Format(time.RFC3339), SizeBytes: fi.Size(), + Excerpt: "(cannot be read: " + readErr.Error() + ")", Queue: queue, + }) continue } fm, body := parseFrontmatter(string(raw)) diff --git a/ingestion/internal/api/promote_queue_test.go b/ingestion/internal/api/promote_queue_test.go index 32a1b74..e7c1e6e 100644 --- a/ingestion/internal/api/promote_queue_test.go +++ b/ingestion/internal/api/promote_queue_test.go @@ -190,3 +190,21 @@ func TestReadCapped_StopsAtTheCap(t *testing.T) { assert.Contains(t, err.Error(), "exceeds") assert.LessOrEqual(t, e.n, int64(1000+1+32*1024), "pulled far more than the cap: %d", e.n) } + +// An .md over the read cap used to vanish from the listing (readRawNote failed +// and the entry was skipped). It must stay visible, with the reason. +func TestListPending_AnUnreadableNoteIsListedWithTheReason(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "raw"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "raw", "huge.md"), []byte(strings.Repeat("x", 200)), 0o644)) + prev := maxRawNoteBytes + maxRawNoteBytes = 100 + t.Cleanup(func() { maxRawNoteBytes = prev }) + + got, err := ListPending(dir) + require.NoError(t, err) + require.Len(t, got, 1) + assert.Equal(t, "huge.md", got[0].Filename) + assert.Contains(t, got[0].Excerpt, "cannot be read") + assert.Contains(t, got[0].Excerpt, "exceeds") +} diff --git a/ingestion/internal/pipeline/pipeline.go b/ingestion/internal/pipeline/pipeline.go index f3ed9c4..f166a4a 100644 --- a/ingestion/internal/pipeline/pipeline.go +++ b/ingestion/internal/pipeline/pipeline.go @@ -113,7 +113,19 @@ func Run(ctx context.Context, cfg Config, brainDir, content, source string, dryR meta.BodyHash = bodyHash applySourceMeta(allRaw, meta) - return buildAndWrite(ctx, cfg.Wing, allRaw, sourceSlug, date, brainDir, source, inventory, allWarnings, dryRun) + res, err := buildAndWrite(ctx, cfg.Wing, allRaw, sourceSlug, date, brainDir, source, inventory, allWarnings, dryRun) + if err != nil || cfg.Wing == "" { + return res, err + } + // In a wing every ingest must leave its source page: it is what ties the + // note to the wiki (and carries the hash the drift check reads). Without one + // nothing of the note exists, and the caller would archive it as processed. + for _, p := range res.Pages { + if isSourcePath(p) { + return res, nil + } + } + return res, fmt.Errorf("no source page was written for %q; nothing of the note was ingested (%s)", source, strings.Join(res.Warnings, "; ")) } // RunRaw runs the pipeline on pre-parsed RawPages, skipping the LLM extraction @@ -357,13 +369,19 @@ func resolveSourceSlug(brainDir, wing, base string) (string, error) { return "", fmt.Errorf("check source slug: %w", err) } own := "wiki/" + wing + "/sources/" + cand + ".md" - free := true + free, hasOwn := true, false for _, p := range existing { - if p != own { + if p == own { + hasOwn = true + } else { free = false - break } } + // The wing's own page is its identity: keep using it even if some other + // page shares the slug (that other page is the one dropSlugClashes skips). + if hasOwn { + return cand, nil + } if free { return cand, nil } @@ -399,12 +417,42 @@ func dropSlugClashes(brainDir string, pages []wiki.Page) ([]wiki.Page, []string, } slugOf := func(p wiki.Page) string { return strings.TrimSuffix(filepath.Base(p.Path), ".md") } - winner := map[string]wiki.Page{} + // Group by slug; where several pages with different paths want one slug, + // the source wins, then a page that already exists on disk (so its update + // is not lost), then entity over concept. + bySlug := map[string][]wiki.Page{} for _, p := range pages { - w, seen := winner[slugOf(p)] - if !seen || (w.Path != p.Path && rank(p) < rank(w)) { - winner[slugOf(p)] = p + bySlug[slugOf(p)] = append(bySlug[slugOf(p)], p) + } + winner := map[string]wiki.Page{} + for slug, group := range bySlug { + best := group[0] + if distinctPaths(group) > 1 { + existing, err := wiki.FindSlugs(brainDir, slug) + if err != nil { + return nil, nil, fmt.Errorf("check slug %q: %w", slug, err) + } + onDisk := func(p wiki.Page) int { + for _, e := range existing { + if e == p.Path { + return 0 + } + } + return 1 + } + for _, p := range group[1:] { + if rank(p) != rank(best) && (rank(p) == 0 || rank(best) == 0) { + if rank(p) < rank(best) { + best = p + } + continue + } + if onDisk(p) < onDisk(best) || (onDisk(p) == onDisk(best) && rank(p) < rank(best)) { + best = p + } + } } + winner[slug] = best } var kept []wiki.Page @@ -438,3 +486,11 @@ func dropSlugClashes(brainDir string, pages []wiki.Page) ([]wiki.Page, []string, } return kept, warnings, nil } + +func distinctPaths(pages []wiki.Page) int { + seen := map[string]bool{} + for _, p := range pages { + seen[p.Path] = true + } + return len(seen) +} diff --git a/ingestion/internal/pipeline/resolve.go b/ingestion/internal/pipeline/resolve.go index 21e04e3..528ad28 100644 --- a/ingestion/internal/pipeline/resolve.go +++ b/ingestion/internal/pipeline/resolve.go @@ -32,6 +32,13 @@ func Resolve(proposed []wiki.Page, inventory map[wiki.PageType][]wiki.Entry) []w out := make([]wiki.Page, 0, len(proposed)) for _, page := range proposed { + if isWingSourcePath(page.Path) { + // In a wing the source's path is its filename-derived slug. Matching + // by TITLE would merge two different notes the model happened to + // title alike ("Home", "Untitled") into one page. + out = append(out, page) + continue + } pt := pageTypeOf(page) title := extractTitle(page.Content) k := key{pt: pt, normalized: normalizeTitle(title)} @@ -97,3 +104,9 @@ func extractTitle(content string) string { } return "" } + +// isWingSourcePath reports whether path is wiki//sources/.md. +func isWingSourcePath(path string) bool { + parts := strings.Split(filepath.ToSlash(path), "/") + return len(parts) == 4 && parts[0] == "wiki" && parts[2] == "sources" +} diff --git a/ingestion/internal/pipeline/wing_layout_test.go b/ingestion/internal/pipeline/wing_layout_test.go index a881690..fe9915d 100644 --- a/ingestion/internal/pipeline/wing_layout_test.go +++ b/ingestion/internal/pipeline/wing_layout_test.go @@ -292,3 +292,83 @@ func TestRun_AnExistingSourceEntityPairDoesNotLockTheSourceOut(t *testing.T) { assert.Contains(t, read(t, dir, "wiki/homelab/sources/notes.md"), "fresh", "the source took the update") assert.Contains(t, read(t, dir, "wiki/entities/notes.md"), "stale entity", "the entity is untouched") } + +// Pass 3 of #211: a note must never leave the inbox as "processed" when nothing +// of it was written. +func TestRun_WithAWingAnIngestThatWritesNoSourcePageIsAnError(t *testing.T) { + tests := []struct { + name string + pages []RawPage + prep func(t *testing.T, dir string) + }{ + {"the model returned nothing", nil, nil}, + {"only pages of a type the layout has no place for", []RawPage{{Title: "Claim", Type: "claim", Content: "## x\n\ny\n"}}, nil}, + {"only an entity, and its slug is taken by a source", []RawPage{{Title: "S1", Type: "entity", Subtype: "tool", Content: "## Definition\n\nx\n"}}, + func(t *testing.T, dir string) { + require.NoError(t, os.MkdirAll(filepath.Join(dir, "wiki/web-clip/sources"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "wiki/web-clip/sources/s1.md"), []byte("---\ntitle: 'S1'\n---\nx"), 0o644)) + }}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + if tt.prep != nil { + tt.prep(t, dir) + } + res, err := Run(context.Background(), wingCfg("web-clip", tt.pages...), dir, "a note body", "other-note", false) + require.Error(t, err, "nothing was ingested; the caller must not archive it as done") + assert.Empty(t, res.Pages) + assert.Contains(t, err.Error(), "no source page") + }) + } +} + +func TestRun_WithAWingAnUnchangedSourceIsStillASkipNotAnError(t *testing.T) { + dir := t.TempDir() + _, err := Run(context.Background(), wingCfg("web-clip", sourceEntityConcept()...), dir, "same body", "talk", false) + require.NoError(t, err) + res, err := Run(context.Background(), wingCfg("web-clip", sourceEntityConcept()...), dir, "same body", "talk", false) + require.NoError(t, err) + assert.True(t, res.Skipped) +} + +// Title resolution must not merge two different notes into one source page just +// because the model titled both "Home": in a wing the source path is already +// deterministic (the filename), so the title must not redirect it. +func TestRun_WithAWingTwoNotesTheModelTitlesAlikeKeepTheirOwnSourcePages(t *testing.T) { + dir := t.TempDir() + home := func(body string) []RawPage { + return []RawPage{{Title: "Home", Type: "source", Subtype: "article", Content: "## Summary\n\n" + body + "\n"}} + } + _, err := Run(context.Background(), wingCfg("web-clip", home("note A text")...), dir, "body A", "note-a", false) + require.NoError(t, err) + _, err = Run(context.Background(), wingCfg("web-clip", home("note B text")...), dir, "body B", "note-b", false) + require.NoError(t, err) + + assert.True(t, exists(dir, "wiki/web-clip/sources/note-a.md")) + assert.True(t, exists(dir, "wiki/web-clip/sources/note-b.md"), "note B has its own page") + assert.Contains(t, read(t, dir, "wiki/web-clip/sources/note-a.md"), "note A text") + assert.Contains(t, read(t, dir, "wiki/web-clip/sources/note-b.md"), "note B text") + + res, err := Run(context.Background(), wingCfg("web-clip", home("note A text")...), dir, "body A", "note-a", false) + require.NoError(t, err) + assert.True(t, res.Skipped, "note A's hash was not overwritten by note B") +} + +// Both a concept and an entity named "Foo" arrive and the CONCEPT already exists +// on disk: the page that exists wins the tie, so its source reference is added. +func TestRun_ABatchTieBetweenAConceptAndAnEntityPrefersThePageAlreadyOnDisk(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "wiki/concepts"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "wiki/concepts/foo.md"), + []byte("---\ntitle: 'Foo'\naliases:\n - 'Foo'\n---\n\n## Definition\n\nexisting concept\n\n## Sources\n"), 0o644)) + pages := []RawPage{ + {Title: "Talk", Type: "source", Subtype: "video", Content: "## Summary\n\nx\n\n## Concepts Introduced or Reinforced\n\n[[Foo]]\n"}, + {Title: "Foo", Type: "entity", Subtype: "tool", Content: "## Definition\n\nan entity\n"}, + {Title: "Foo", Type: "concept", Content: "## Definition\n\na concept\n\n## Sources\n"}, + } + _, err := Run(context.Background(), wingCfg("web-clip", pages...), dir, "body", "talk", false) + require.NoError(t, err) + assert.False(t, exists(dir, "wiki/entities/foo.md")) + assert.Contains(t, read(t, dir, "wiki/concepts/foo.md"), "talk", "the existing concept got the new source reference") +} diff --git a/ingestion/internal/watcher/watcher_wing_test.go b/ingestion/internal/watcher/watcher_wing_test.go index 16da56d..da865cd 100644 --- a/ingestion/internal/watcher/watcher_wing_test.go +++ b/ingestion/internal/watcher/watcher_wing_test.go @@ -339,3 +339,20 @@ func TestFrontmatterWing(t *testing.T) { func TestFrontmatterWing_ABOMDoesNotHideTheKey(t *testing.T) { assert.Equal(t, "homelab", frontmatterWing("\ufeff---\nwing: homelab\n---\nbody")) } + +// A note that produced nothing must not leave the inbox as "processed". +func TestProcessDir_AWingIngestThatWritesNothingFailsTheNoteVisibly(t *testing.T) { + var llmCalls atomic.Int32 + cfg, brainDir := wingTestCfg(t, router, &llmCalls) + cfg.Pipeline.Complete = func(context.Context, string, string) (string, error) { return "[]", nil } + put(t, brainDir, "empty-result.md", "A note the model returned nothing for.") + + require.Empty(t, processDir(context.Background(), cfg, "2026-10-20")) + + assert.True(t, has(brainDir, "raw/failed/empty-result.md"), "kept where a human looks") + processed, _ := filepath.Glob(filepath.Join(brainDir, "raw", "processed", "*", "*")) + assert.Empty(t, processed, "never archived as processed") + logb, err := os.ReadFile(filepath.Join(brainDir, "log.md")) + require.NoError(t, err) + assert.Contains(t, string(logb), "no source page") +} diff --git a/ingestion/internal/wing/router.go b/ingestion/internal/wing/router.go index 8d284ef..3df65ed 100644 --- a/ingestion/internal/wing/router.go +++ b/ingestion/internal/wing/router.go @@ -142,8 +142,10 @@ func matchesClient(re *regexp.Regexp, text string, chunk int) bool { if end == len(text) { return false } - // A cut inside a rune is harmless: the overlap carries every marker - // shorter than it whole into the next chunk. + // A cut inside a rune is harmless: the overlap carries every match that + // spans at most filterOverlap bytes whole into the next chunk. A match + // longer than that (a pattern like `acme.{5000}corp`, or padding + // between two words) can be missed on a chunk edge; documented limit. start = end - overlap } return false -- 2.54.0 From 90752f2ac983d18e59c72a545ff3c25d51d87be3 Mon Sep 17 00:00:00 2001 From: Mathias Date: Tue, 20 Oct 2026 21:00:18 +0200 Subject: [PATCH 14/14] fix(pipeline,api): write the source page last; say how to clear an unreadable note (review of #211, pass 4) The source page carries source_hash, which the drift check trusts. Written first, a batch that failed midway left a hash behind, so re-ingesting the same content was skipped and the rest of the batch was lost for good. In a wing it is now written last. An over-cap note in brain_pending says promote/reject cannot act on it and to delete or move it by hand. Mutation-checked. Co-Authored-By: Claude Sonnet 5.5 --- ingestion/internal/api/promote.go | 2 +- ingestion/internal/pipeline/pipeline.go | 7 ++++++ .../internal/pipeline/wing_layout_test.go | 22 +++++++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/ingestion/internal/api/promote.go b/ingestion/internal/api/promote.go index fe9115e..9e489db 100644 --- a/ingestion/internal/api/promote.go +++ b/ingestion/internal/api/promote.go @@ -105,7 +105,7 @@ func listNotes(root *os.Root, queue string) ([]PendingNote, error) { } out = append(out, PendingNote{ Filename: e.Name(), CreatedAt: fi.ModTime().UTC().Format(time.RFC3339), SizeBytes: fi.Size(), - Excerpt: "(cannot be read: " + readErr.Error() + ")", Queue: queue, + Excerpt: "(cannot be read: " + readErr.Error() + "; promote and reject cannot act on it: delete or move it by hand)", Queue: queue, }) continue } diff --git a/ingestion/internal/pipeline/pipeline.go b/ingestion/internal/pipeline/pipeline.go index f166a4a..654f71e 100644 --- a/ingestion/internal/pipeline/pipeline.go +++ b/ingestion/internal/pipeline/pipeline.go @@ -11,6 +11,7 @@ import ( "github.com/mathiasbq/hyperguild/ingestion/internal/brainfs" "io/fs" "path/filepath" + "sort" "strings" "time" @@ -166,6 +167,12 @@ func buildAndWrite(ctx context.Context, wing string, rawPages []RawPage, sourceS warnings = append(warnings, clashWarnings...) } + if wing != "" { + // The source page carries source_hash, which the drift check trusts: it + // goes last, so a batch that fails midway leaves nothing to hide behind. + sort.SliceStable(merged, func(i, j int) bool { return !isSourcePath(merged[i].Path) && isSourcePath(merged[j].Path) }) + } + var written []string for _, page := range merged { if !dryRun { diff --git a/ingestion/internal/pipeline/wing_layout_test.go b/ingestion/internal/pipeline/wing_layout_test.go index fe9915d..17958d0 100644 --- a/ingestion/internal/pipeline/wing_layout_test.go +++ b/ingestion/internal/pipeline/wing_layout_test.go @@ -372,3 +372,25 @@ func TestRun_ABatchTieBetweenAConceptAndAnEntityPrefersThePageAlreadyOnDisk(t *t assert.False(t, exists(dir, "wiki/entities/foo.md")) assert.Contains(t, read(t, dir, "wiki/concepts/foo.md"), "talk", "the existing concept got the new source reference") } + +// Pass 4 of #211: the source page carries the hash the drift check reads. If it +// is written FIRST and a later page fails to write, the next ingest of the same +// content is skipped and the rest of the batch is lost for good. Written LAST, +// a failed batch leaves no hash and is simply redone. +func TestRun_WithAWingAFailedBatchLeavesNoSourcePageToHideBehind(t *testing.T) { + dir := t.TempDir() + // An entity path that cannot be written: a directory where the page goes. + require.NoError(t, os.MkdirAll(filepath.Join(dir, "wiki/entities/jev.md"), 0o755)) + + _, err := Run(context.Background(), wingCfg("web-clip", sourceEntityConcept()...), dir, "same body", "talk", false) + require.Error(t, err, "the entity write fails") + assert.False(t, exists(dir, "wiki/web-clip/sources/talk.md"), + "the source page (and its hash) must not exist when the batch did not complete") + + require.NoError(t, os.RemoveAll(filepath.Join(dir, "wiki/entities/jev.md"))) + res, err := Run(context.Background(), wingCfg("web-clip", sourceEntityConcept()...), dir, "same body", "talk", false) + require.NoError(t, err) + assert.False(t, res.Skipped, "the retry is a real ingest, not a hash skip") + assert.True(t, exists(dir, "wiki/entities/jev.md")) + assert.True(t, exists(dir, "wiki/web-clip/sources/talk.md")) +} -- 2.54.0