fix(#5915 J1): P0 segment-awareness — stats/reindex-detect/tier-reload (prevents data-loss at flip)

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.<gen>/ 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.<gen>/ 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)

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.<gen>/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)
+	}
+}

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
 	}

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

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 <genDir>/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)
+		}
+	}
+}