From 9aabb01e2a67d2a121360c91d7c4a59b5efe9968 Mon Sep 17 00:00:00 2001 From: claude Date: Tue, 4 Aug 2026 03:18:42 +0400 Subject: [PATCH] recalleval: a filler id a case reuses is refused at load (V-386) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every case is scored over its own notes plus the whole filler set, and the two stores disagree about a repeated id: sqlite 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 — the one thing this harness exists to measure. It was dodged by hand during #373 by renaming two ids. The check sits in Load rather than in TestLoadFixture, so it covers every caller of the fixture and not only the one that remembers to look. --- internal/memory/recalleval/recalleval.go | 35 +++++++++++++++++++ internal/memory/recalleval/recalleval_test.go | 25 +++++++++++++ 2 files changed, 60 insertions(+) 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) + } +}