diff --git a/cmd/grafel/daemon_tier.go b/cmd/grafel/daemon_tier.go index 1e0a44f05..7c9466a1b 100644 --- a/cmd/grafel/daemon_tier.go +++ b/cmd/grafel/daemon_tier.go @@ -150,9 +150,15 @@ func registerKnownGroupsCold(logger *slog.Logger) { if ref == "_unknown" { continue // sentinel — skip per ErrUnknownRef semantics } - fbPath := graph.CurrentGraphPath(filepath.Join(refsDir, ref)) // #5891 - if _, statErr := os.Stat(fbPath); statErr != nil { - continue // no graph yet (gen or legacy flat) + // #5915 J1 FIX-3: segment-aware existence probe. os.Stat of the + // flat CurrentGraphPath is absent for a segment-set (the pointer + // names a graph./ dir), so the old check would skip — never + // cold-register, hence never warm — a segmented repo at boot. + // Route existence through the descriptor: GraphAbsent (or a corrupt + // manifest) → skip; single-file/legacy/segment-set → register. + refStateDir := filepath.Join(refsDir, ref) + if desc, dErr := graph.CurrentGraphDescriptor(refStateDir); dErr != nil || desc.Kind == graph.GraphAbsent { + continue // no graph yet (gen file, gen dir, or legacy flat) } isPinned := tier.IsDefaultBranch(repoPath, ref) kind := tier.SlotKindBranchFeature @@ -395,9 +401,14 @@ func tierEvictCallback(key tier.SlotKey) { // is safe even when the subscription is already active. func tierReloadCallback(key tier.SlotKey) error { stateDir := daemon.StateDirForRepoRef(key.RepoPath, key.Ref) - fbPath := graph.CurrentGraphPath(stateDir) // #5891: resolve active generation - // Prime the cache by opening and immediately releasing the reader. - _, release, err := daemonMCPCache.Get(fbPath) + // #5915 J1 FIX-3: prime the cache via the segment-aware ref entry point. + // The old code called Get(CurrentGraphPath(stateDir)); for a segment-set + // (graph./ dir + manifest) the flat .fb path is absent → Get's stat + // errors → the cold-wake reload fails → a cold-evicted segmented repo could + // never be re-warmed (monolith serving break). GetForRepoRef resolves the + // descriptor and keys a segment-set on its gen dir (opening a MultiReader), + // while single-file/legacy keys on the resolved .fb path exactly as before. + _, release, err := daemonMCPCache.GetForRepoRef(key.RepoPath, key.Ref) if err != nil { return err } @@ -408,8 +419,15 @@ func tierReloadCallback(key tier.SlotKey) error { daemonWatcherMgr.Resume(key.RepoPath, key.Ref) } - // Stale-detection: if the repo has file changes newer than graph.fb, - // enqueue a reactive reindex so the next query gets a fresh graph. + // Stale-detection freshness reference: the resolved gen file, or — for a + // segment-set — the manifest.json (a real file whose mtime advances on each + // rebuild). The flat CurrentGraphPath would be absent for a segment-set, so + // isRepoDirtyAfter would read "graph missing → never dirty" and silently skip + // the reactive reindex. + fbPath := graph.CurrentGraphPath(stateDir) + if desc, dErr := graph.CurrentGraphDescriptor(stateDir); dErr == nil && desc.Kind == graph.GraphSegmentSet { + fbPath = filepath.Join(desc.GenDir, graph.ManifestFileName) + } if isRepoDirtyAfter(key.RepoPath, fbPath) { if daemonSchedulerEnqueue != nil { daemonSchedulerEnqueue(key.RepoPath) diff --git a/cmd/grafel/daemon_tier_segmentset_test.go b/cmd/grafel/daemon_tier_segmentset_test.go new file mode 100644 index 000000000..0700effe1 --- /dev/null +++ b/cmd/grafel/daemon_tier_segmentset_test.go @@ -0,0 +1,115 @@ +package main + +import ( + "os" + "path/filepath" + "sort" + "testing" + + "github.com/cajasmota/grafel/internal/daemon" + "github.com/cajasmota/grafel/internal/daemon/tier" + "github.com/cajasmota/grafel/internal/graph" + "github.com/cajasmota/grafel/internal/graph/fbwriter" +) + +// writeSegmentSetAt hand-builds a multi-segment generation under stateDir: +// graph./seg-NNNN.fb + manifest.json + a `current` pointer naming the gen +// dir. Mirrors internal/graph's writeSegmentSet test helper (not importable +// across packages). Returns the gen dir. +func writeSegmentSetAt(t *testing.T, stateDir string, gen uint64, docs []*graph.Document) string { + t.Helper() + if err := os.MkdirAll(stateDir, 0o755); err != nil { + t.Fatal(err) + } + genDir := filepath.Join(stateDir, graph.GenDirName(gen)) + if err := os.MkdirAll(genDir, 0o755); err != nil { + t.Fatal(err) + } + m := &graph.Manifest{FormatVersion: graph.ManifestFormatVersion} + for i, doc := range docs { + name := graph.SegmentFileName(i) + if err := fbwriter.WriteAtomic(filepath.Join(genDir, name), doc); err != nil { + t.Fatalf("write %s: %v", name, err) + } + seg := graph.SegmentMeta{ + File: name, + Kind: graph.SegmentEntities, + EntityCount: len(doc.Entities), + RelCount: len(doc.Relationships), + } + if len(doc.Entities) > 0 { + ids := make([]string, 0, len(doc.Entities)) + for _, e := range doc.Entities { + ids = append(ids, e.ID) + } + sort.Strings(ids) + seg.MinKey, seg.MaxKey = ids[0], ids[len(ids)-1] + } else { + seg.Kind = graph.SegmentRelationships + } + m.Segments = append(m.Segments, seg) + } + if err := graph.WriteManifest(genDir, m); err != nil { + t.Fatalf("write manifest: %v", err) + } + if err := graph.WriteCurrentPointerRaw(stateDir, graph.GenDirName(gen)); err != nil { + t.Fatalf("write current pointer: %v", err) + } + return genDir +} + +func segTestDocs() []*graph.Document { + return []*graph.Document{ + {Repo: "seg", Entities: []graph.Entity{ + {ID: "a", QualifiedName: "p.A", Kind: "function", Name: "A"}, + {ID: "b", QualifiedName: "p.B", Kind: "struct", Name: "B"}, + }, Relationships: []graph.Relationship{{FromID: "a", ToID: "z", Kind: "CALLS"}}}, + {Repo: "seg", Entities: []graph.Entity{ + {ID: "y", QualifiedName: "p.Y", Kind: "function", Name: "Y"}, + {ID: "z", QualifiedName: "p.Z", Kind: "struct", Name: "Z"}, + }, Relationships: []graph.Relationship{{FromID: "y", ToID: "a", Kind: "REFERENCES"}}}, + } +} + +// TestTierReloadCallback_SegmentSet is the FIX-3 test: the cold-wake reload +// callback must warm a segment-set repo. Before #5915 J1 it did +// daemonMCPCache.Get(CurrentGraphPath(stateDir)); for a segment-set the flat +// path is absent → Get errors → a cold-evicted segmented repo could never be +// re-warmed (monolith serving break). The fix routes through the segment-aware +// GetForRepoRef, which resolves the descriptor and opens a MultiReader over the +// gen dir. +func TestTierReloadCallback_SegmentSet(t *testing.T) { + if daemonMCPCache == nil { + t.Skip("daemonMCPCache not initialised in this test binary") + } + // Isolate the store under a temp GRAFEL_DAEMON_ROOT so StateDirForRepoRef + // (used identically by tierReloadCallback and GetForRepoRef) resolves here. + t.Setenv(daemon.EnvRoot, t.TempDir()) + + repoPath := t.TempDir() // empty working tree — no files newer than the graph + const ref = "main" + stateDir := daemon.StateDirForRepoRef(repoPath, ref) + genDir := writeSegmentSetAt(t, stateDir, 3, segTestDocs()) + + // Evict any resident handle so this is a genuine cold wake. + daemonMCPCache.InvalidateDir(stateDir) + t.Cleanup(func() { daemonMCPCache.InvalidateDir(stateDir) }) // Windows-safe: drop mmap before TempDir cleanup + + if err := tierReloadCallback(tier.SlotKey{RepoPath: repoPath, Ref: ref}); err != nil { + t.Fatalf("tierReloadCallback(segment-set) errored (cold-wake warm failed): %v", err) + } + + // Prove the segment-aware path was taken: the cache now holds a handle keyed + // on the gen DIR (not the absent flat .fb), summing entities across segments. + v, release, err := daemonMCPCache.GetForRepoRef(repoPath, ref) + if err != nil { + t.Fatalf("post-reload GetForRepoRef: %v", err) + } + defer release() + if v.EntityCount() != 4 { + t.Errorf("warmed segment-set EntityCount = %d, want 4 (summed across segments)", v.EntityCount()) + } + if filepath.Base(genDir) != graph.GenDirName(3) { + t.Fatalf("unexpected gen dir %q", genDir) + } +} diff --git a/internal/graph/load.go b/internal/graph/load.go index 1f818773b..1ab6164d7 100644 --- a/internal/graph/load.go +++ b/internal/graph/load.go @@ -78,8 +78,18 @@ func (e *FormatVersionError) Error() string { // that could go stale or get clobbered by a later write from a different // writer. func ReindexRequiredReason(dir string) (required bool, reason string) { - fbPath := CurrentGraphPath(dir) - r, err := fbreader.Open(fbPath) + // #5915 J1 FIX-2: route through the segment-aware descriptor. The old code + // opened CurrentGraphPath(dir)+fbreader.Open, so a segment-set's absent flat + // .fb path returned (false,"") — segment-blind: after a future fbversion bump + // a below-min segment-set would NOT be detected stale, so it would NOT be + // auto-reindexed and would serve empty (defeating #5907 for exactly the large + // repos that segment). ReaderForDir opens fbreader.Open(desc.Path) for a + // single-file/legacy graph (byte-identical — same version read, absent → + // (false,"")) and a *MultiReader for a segment-set, whose Version() is segment + // 0's version. The same v < minSupportedFBFormatVersion comparison + + // FormatVersionReason then applies uniformly. (fbversion is still 4, so a + // current v4 segment-set correctly returns false — a no-op today.) + r, err := ReaderForDir(dir) if err != nil { return false, "" } @@ -242,8 +252,17 @@ type PersistedStats struct { // Returns ok=false when graph.fb is absent or cannot be opened (in which case // callers should treat the repo as genuinely never-indexed via graph.fb). func PersistedStatsFromDir(dir string) (PersistedStats, bool) { - fbPath := CurrentGraphPath(dir) - r, err := fbreader.Open(fbPath) + // #5915 J1 FIX-1: route through the segment-aware descriptor rather than the + // flat CurrentGraphPath. ReaderForDir opens fbreader.Open(desc.Path) for a + // single-file/legacy graph (byte-identical to the old path — desc.Path == + // CurrentGraphPath(dir) there, and an absent flat path returns the same open + // error → ok=false) and a *MultiReader over the gen dir for a segment-set, + // whose EntityCount/RelationshipCount already sum across every segment. The + // old code opened only the (absent) flat .fb for a segment-set → ok=false → + // the "0 entities / never indexed" cascade (statuswriter's status plane, the + // incremental.go:331 force-reindex loop, dashboard store, status_stats, + // daemon.go:747, index_commit). + r, err := ReaderForDir(dir) if err != nil { return PersistedStats{}, false } diff --git a/internal/graph/persistedstats_segmentset_test.go b/internal/graph/persistedstats_segmentset_test.go new file mode 100644 index 000000000..c2d32db38 --- /dev/null +++ b/internal/graph/persistedstats_segmentset_test.go @@ -0,0 +1,78 @@ +package graph_test + +import ( + "path/filepath" + "testing" + + "github.com/cajasmota/grafel/internal/graph" + "github.com/cajasmota/grafel/internal/graph/fbwriter" +) + +// TestPersistedStatsFromDir_SegmentSet is the FIX-1 RED test (#5915 J1): the +// cheap header-only stats probe must be segment-aware. Before this slice it +// opened CurrentGraphPath(dir)+fbreader.Open, and for a segment-set that flat +// path is absent → ok=false → the caller sees "0 entities / never indexed" and +// the incremental.go:331 gate force-reindex-loops the repo. It must instead +// resolve the descriptor, open a MultiReader over the gen dir, and report the +// summed counts with ok=true. +func TestPersistedStatsFromDir_SegmentSet(t *testing.T) { + dir := t.TempDir() + writeSegmentSet(t, dir, 3, threeSegDocs()) + + ps, ok := graph.PersistedStatsFromDir(dir) + if !ok { + t.Fatal("PersistedStatsFromDir(segment-set) ok=false; want ok=true (segment-aware). This is the 'never indexed' cascade at incremental.go:331.") + } + if ps.Entities != 6 { + t.Errorf("Entities = %d, want 6 (summed across segments, NOT 0)", ps.Entities) + } + if ps.Relationships != 3 { + t.Errorf("Relationships = %d, want 3 (summed across segments)", ps.Relationships) + } +} + +// TestPersistedStatsFromDir_IncrementalGateCleared is the explicit cascade +// guard: the incremental.go:331 "absent-graph-nonempty-tree → force full +// reindex" gate keys off exactly PersistedStatsFromDir(...) ok. A segment-set +// must read ok=true so a segmented repo is NOT treated as never-indexed and does +// NOT enter the forced-reindex loop. +func TestPersistedStatsFromDir_IncrementalGateCleared(t *testing.T) { + dir := t.TempDir() + writeSegmentSet(t, dir, 7, threeSegDocs()) + if _, ok := graph.PersistedStatsFromDir(dir); !ok { + t.Fatal("segment-set reads as !ok at the incremental.go:331 gate → forced-reindex loop") + } +} + +// TestPersistedStatsFromDir_SingleFileParity guards that the single-file gen +// path is byte-identical after the descriptor-routing change: correct counts, +// ok=true. +func TestPersistedStatsFromDir_SingleFileParity(t *testing.T) { + dir := t.TempDir() + doc := &graph.Document{Repo: "single", Entities: []graph.Entity{ + {ID: "aaaa0001", QualifiedName: "p.A", Kind: "function", Name: "A"}, + {ID: "aaaa0002", QualifiedName: "p.B", Kind: "struct", Name: "B"}, + }, Relationships: []graph.Relationship{{FromID: "aaaa0001", ToID: "aaaa0002", Kind: "CALLS"}}} + if err := fbwriter.WriteAtomic(filepath.Join(dir, graph.GenFileName(1)), doc); err != nil { + t.Fatal(err) + } + if err := graph.WriteCurrentPointer(dir, graph.GenFileName(1)); err != nil { + t.Fatal(err) + } + ps, ok := graph.PersistedStatsFromDir(dir) + if !ok { + t.Fatal("single-file parity: ok=false, want true") + } + if ps.Entities != 2 || ps.Relationships != 1 { + t.Errorf("single-file counts = %d/%d, want 2/1", ps.Entities, ps.Relationships) + } +} + +// TestPersistedStatsFromDir_Absent: a never-indexed dir still reports ok=false +// (parity with the pre-#5915 absent semantics). +func TestPersistedStatsFromDir_Absent(t *testing.T) { + dir := t.TempDir() + if _, ok := graph.PersistedStatsFromDir(dir); ok { + t.Error("absent dir: ok=true, want false") + } +} diff --git a/internal/graph/reindex_required_segmentset_test.go b/internal/graph/reindex_required_segmentset_test.go new file mode 100644 index 000000000..576255cd6 --- /dev/null +++ b/internal/graph/reindex_required_segmentset_test.go @@ -0,0 +1,76 @@ +package graph_test + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/cajasmota/grafel/internal/graph" + fb "github.com/cajasmota/grafel/internal/graph/fbgraph" + "github.com/cajasmota/grafel/internal/graph/fbversion" + "github.com/cajasmota/grafel/internal/graph/fbwriter" +) + +// patchSegmentVersion rewrites /seg-0000.fb with a copy of doc whose +// on-disk Graph.version scalar is mutated down to oldVersion — the segment-set +// counterpart to writeOldFormatGraphFB. MultiReader.Version() reports segment +// 0's version, so patching seg 0 is sufficient to simulate a below-min +// segment-set. doc MUST equal the doc originally written as seg 0 so the +// manifest's MinKey/MaxKey stay valid. +func patchSegmentVersion(t *testing.T, genDir string, doc *graph.Document, oldVersion int) { + t.Helper() + buf, err := fbwriter.Marshal(doc) + if err != nil { + t.Fatalf("marshal: %v", err) + } + root := fb.GetRootAsGraph(buf, 0) + if !root.MutateVersion(int32(oldVersion)) { + t.Fatalf("MutateVersion(%d) returned false — slot missing?", oldVersion) + } + if err := os.WriteFile(filepath.Join(genDir, graph.SegmentFileName(0)), buf, 0o644); err != nil { + t.Fatalf("overwrite seg 0: %v", err) + } +} + +// TestReindexRequiredReason_SegmentSet_CurrentVersion is the FIX-2 no-op guard: +// a current-version segment-set must NOT be flagged (required=false). Before +// #5915 J1 the header-only detector opened the absent flat .fb → returned +// (false,"") by accident; now it must return (false,"") by actually reading the +// MultiReader's version and finding it >= min. +func TestReindexRequiredReason_SegmentSet_CurrentVersion(t *testing.T) { + dir := t.TempDir() + writeSegmentSet(t, dir, 3, threeSegDocs()) + + if required, reason := graph.ReindexRequiredReason(dir); required { + t.Errorf("current-version segment-set flagged stale: reason=%q", reason) + } +} + +// TestReindexRequiredReason_SegmentSet_OldVersion is the FIX-2 detection test: +// a segment-set whose segment 0 is stamped below fbversion.Version must be +// detected as reindex-required (defeating the "v4 segment-set not auto-reindexed +// after an fbversion bump → serves empty" break for exactly the large repos that +// segment). +func TestReindexRequiredReason_SegmentSet_OldVersion(t *testing.T) { + dir := t.TempDir() + docs := threeSegDocs() + genDir := writeSegmentSet(t, dir, 4, docs) + patchSegmentVersion(t, genDir, docs[0], fbversion.Version-1) + + required, reason := graph.ReindexRequiredReason(dir) + if !required { + t.Fatal("old-version segment-set NOT detected as reindex-required") + } + wantSubstrs := []string{ + fmt.Sprintf("v%d", fbversion.Version-1), + fmt.Sprintf("v%d", fbversion.Version), + "reindex", + } + for _, want := range wantSubstrs { + if !strings.Contains(strings.ToLower(reason), strings.ToLower(want)) { + t.Errorf("reason %q missing expected substring %q", reason, want) + } + } +}