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 <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
// 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.
|
// page whose slug belongs to a different page. A slug is the graph identity, so
|
||||||
// The page's own path is not a clash (a shared entity merging into itself).
|
// two pages must not share one; and a page that shares one cannot be updated
|
||||||
// A skipped page is a warning, not a failure: the ingest as a whole is fine,
|
// again, because every later ingest sees the other as its clash.
|
||||||
// and one extracted page must not be able to lock a source out of re-ingest.
|
//
|
||||||
|
// 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) {
|
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 kept []wiki.Page
|
||||||
var warnings []string
|
var warnings []string
|
||||||
for _, p := range pages {
|
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)
|
existing, err := wiki.FindSlugs(brainDir, slug)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, nil, fmt.Errorf("check slug %q: %w", slug, err)
|
return nil, nil, fmt.Errorf("check slug %q: %w", slug, err)
|
||||||
|
|||||||
@@ -233,3 +233,62 @@ func TestResolveSourceSlug_AFurtherClashGetsANumber(t *testing.T) {
|
|||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
assert.Equal(t, "talk-b-2", got)
|
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")
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user