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) }