diff --git a/internal/memory/recalleval/recalleval.go b/internal/memory/recalleval/recalleval.go index 1691b49..e1be017 100644 --- a/internal/memory/recalleval/recalleval.go +++ b/internal/memory/recalleval/recalleval.go @@ -90,9 +90,44 @@ func Load() (Fixture, error) { if len(f.Cases) == 0 { return Fixture{}, fmt.Errorf("fixture has no cases") } + if err := checkIDs(f); err != nil { + return Fixture{}, err + } return f, nil } +// checkIDs refuses a fixture where a case note and a filler note share an id. +// +// Every case is scored over its own notes plus the whole filler set, and the +// two stores disagree about what a repeated id means: the sqlite store upserts +// on it, the in-memory store appends. So one collision makes a case score +// differently on the two backends, and it reads as an embedder or gate +// difference, which is the one thing this harness exists to measure (Vikunja +// #386). It was dodged once by hand during #373 by renaming two ids. +// +// Checked in Load rather than in the test, so every caller of the fixture is +// covered and not only the one that remembers to look. +func checkIDs(f Fixture) error { + filler := make(map[string]bool, len(f.Filler)) + for _, n := range f.Filler { + if n.ID == "" { + return fmt.Errorf("filler note with an empty id") + } + if filler[n.ID] { + return fmt.Errorf("duplicate filler note id %q", n.ID) + } + filler[n.ID] = true + } + for _, c := range f.Cases { + for _, n := range c.Notes { + if filler[n.ID] { + return fmt.Errorf("case %s: note id %q collides with a filler note", c.ID, n.ID) + } + } + } + return nil +} + // NewStore builds an empty store for one case, plus a function to release it. // A factory rather than a store because every case needs a clean index — notes // from case A must not be visible to case B's query. diff --git a/internal/memory/recalleval/recalleval_test.go b/internal/memory/recalleval/recalleval_test.go index e9cea1c..f8b324e 100644 --- a/internal/memory/recalleval/recalleval_test.go +++ b/internal/memory/recalleval/recalleval_test.go @@ -337,3 +337,28 @@ func marginSweep(t *testing.T, emb router.Embedder, f Fixture) string { } return b.String() } + +// TestFillerIDCollisionIsRefused — the guard that keeps a fixture edit from +// looking like a backend difference (Vikunja #386). +func TestFillerIDCollisionIsRefused(t *testing.T) { + f := Fixture{ + SchemaVersion: SchemaVersion, + Cases: []Case{{ID: "ru-001", Notes: []StoredNote{{ID: "f1", Text: "..."}}}}, + Filler: []StoredNote{{ID: "f1", Text: "..."}}, + } + if err := checkIDs(f); err == nil { + t.Fatal("a case note reusing a filler id must be refused") + } + f.Filler = append(f.Filler, StoredNote{ID: "f1", Text: "..."}) + if err := checkIDs(Fixture{SchemaVersion: SchemaVersion, Filler: f.Filler}); err == nil { + t.Fatal("a duplicate filler id must be refused") + } + ok := Fixture{ + SchemaVersion: SchemaVersion, + Cases: []Case{{ID: "ru-001", Notes: []StoredNote{{ID: "n1", Text: "..."}}}}, + Filler: []StoredNote{{ID: "f1", Text: "..."}}, + } + if err := checkIDs(ok); err != nil { + t.Fatalf("a clean fixture must pass: %v", err) + } +}