From 652c082b8d5bf7c2f64d2159c2ddfc989903f3fb Mon Sep 17 00:00:00 2001 From: Jorge Cajas Date: Thu, 23 Jul 2026 00:45:44 +0800 Subject: [PATCH 1/4] fix(daemon): segment-set-aware dead-ref grace window (#5915 J2 slice-3 site 1, data loss) refGraphMtime/recentlyIndexed in deadref.go resolved freshness only via os.Stat(graph.CurrentGraphPath(refDir)), a flat-.fb-only path absent for a segment-set ref (graph./ dir + manifest.json). A freshly-indexed segment-set ref read as mtime-zero, fell outside the grace window, and the reaper could delete a valid cold ref. Add graph.CurrentGraphMtime (internal/graph/genpath.go): single-file resolves the .fb mtime (unchanged); segment-set resolves the gen dir's manifest.json mtime. internal/daemon cannot import groupalgo's existing unexported graphSourceMtime (cycle), so the helper lives in internal/graph; groupalgo.graphSourceMtime now delegates to it. Tests: TestCurrentGraphMtime_SegmentSet/SingleFileParity/Absent; TestRecentlyIndexed_SegmentSet, TestRefGraphMtime_SegmentSet, and the load-bearing TestDeadRef_segmentSetRefSurvivesGraceWindow (dead-in-git segment-set ref indexed within grace survives a full Sweep), plus TestRecentlyIndexed_SingleFileParity. Verified RED before the fix, GREEN after. --- internal/daemon/deadref.go | 62 +++++---- internal/daemon/deadref_segmentset_test.go | 140 +++++++++++++++++++++ internal/graph/genpath.go | 44 +++++++ internal/graph/genpath_mtime_test.go | 61 +++++++++ internal/graph/groupalgo/groupalgo.go | 31 ++--- 5 files changed, 288 insertions(+), 50 deletions(-) create mode 100644 internal/daemon/deadref_segmentset_test.go create mode 100644 internal/graph/genpath_mtime_test.go diff --git a/internal/daemon/deadref.go b/internal/daemon/deadref.go index f43ce9a69..9aa5fa881 100644 --- a/internal/daemon/deadref.go +++ b/internal/daemon/deadref.go @@ -361,40 +361,48 @@ func (s *DeadRefSweeper) reapRef(repo, ref, refDir string, res *DeadRefResult, c return true } -// refGraphMtime returns the newest graph.fb / graph.json mtime under refDir, or -// the zero time when neither exists. Used to order grace-protected refs for the -// retention cap (oldest evicted first). +// refGraphMtime returns the newest graph mtime under refDir — the active +// FlatBuffers graph (single-file or segment-set) or graph.json, whichever is +// newest — or the zero time when neither exists. Used to order +// grace-protected refs for the retention cap (oldest evicted first). +// +// #5915 J2 slice-3: os.Stat(graph.CurrentGraphPath(refDir)) — the pattern +// this replaced — only ever names a flat .fb path, which is ABSENT for a +// segment-set ref (graph./ dir + manifest.json, no flat .fb). That would +// silently report mtime-zero for a freshly-indexed segment-set ref. Routing +// through graph.CurrentGraphMtime (segment-set aware) fixes it while leaving +// the single-file/legacy resolution byte-identical. func refGraphMtime(refDir string) time.Time { var newest time.Time - // #5891: resolve the active generation (flat graph.fb fallback) so a - // gen-layout ref reports its real freshness, not mtime-zero. - for _, p := range []string{graph.CurrentGraphPath(refDir), filepath.Join(refDir, "graph.json")} { - fi, err := os.Stat(p) - if err != nil { - continue - } - if fi.ModTime().After(newest) { - newest = fi.ModTime() - } + if mt, ok := graph.CurrentGraphMtime(refDir); ok && mt.After(newest) { + newest = mt + } + if fi, err := os.Stat(filepath.Join(refDir, "graph.json")); err == nil && fi.ModTime().After(newest) { + newest = fi.ModTime() } return newest } -// recentlyIndexed reports whether the ref dir holds a graph.fb (or graph.json) -// whose mtime is at/after cutoff — i.e. it was indexed inside the grace window. -// A missing graph file is treated as NOT recent (eligible for reaping). +// recentlyIndexed reports whether the ref dir holds an active graph (the +// resolved FlatBuffers graph — single-file or segment-set — or graph.json) +// whose mtime is at/after cutoff — i.e. it was indexed inside the grace +// window. A missing graph file is treated as NOT recent (eligible for +// reaping). +// +// #5915 J2 slice-3 (DATA LOSS fix): this used to resolve ONLY +// graph.CurrentGraphPath(refDir), a flat-.fb-only path that is absent for a +// segment-set ref — so a freshly-indexed segment-set ref would read as NOT +// recently indexed and fall outside the grace window, making the dead-ref +// reaper prematurely delete a valid cold ref. graph.CurrentGraphMtime +// resolves the segment-set's manifest.json mtime instead, grace-protecting it +// exactly like a single-file ref; the single-file/legacy resolution is +// byte-identical to before. func recentlyIndexed(refDir string, cutoff time.Time) bool { - // #5891: resolve the active generation so a freshly-indexed gen-layout ref - // is correctly protected by the grace window (a fixed graph.fb no longer - // exists after migration). - for _, p := range []string{graph.CurrentGraphPath(refDir), filepath.Join(refDir, "graph.json")} { - fi, err := os.Stat(p) - if err != nil { - continue - } - if !fi.ModTime().Before(cutoff) { - return true - } + if mt, ok := graph.CurrentGraphMtime(refDir); ok && !mt.Before(cutoff) { + return true + } + if fi, err := os.Stat(filepath.Join(refDir, "graph.json")); err == nil && !fi.ModTime().Before(cutoff) { + return true } return false } diff --git a/internal/daemon/deadref_segmentset_test.go b/internal/daemon/deadref_segmentset_test.go new file mode 100644 index 000000000..c7cfa1a79 --- /dev/null +++ b/internal/daemon/deadref_segmentset_test.go @@ -0,0 +1,140 @@ +package daemon + +import ( + "os" + "path/filepath" + "testing" + "time" + + "github.com/cajasmota/grafel/internal/graph" + "github.com/cajasmota/grafel/internal/graph/fbwriter" +) + +// writeSegmentSetRefStore hand-builds a 1-segment gen dir (graph./ with +// seg-0000.fb + manifest.json) under //, points `current` +// at it, and stamps manifest.json (the segment-set's atomic commit point) +// with mtime. Mirrors writeRefStore (the flat-graph.fb fixture already used +// in this package) and the fixture builders at +// internal/daemon/state_path_graphfbexists_test.go / +// internal/graph/segmentset_test.go. +func writeSegmentSetRefStore(t *testing.T, refsRoot, refSafe string, gen uint64, mtime time.Time) string { + t.Helper() + refDir := filepath.Join(refsRoot, refSafe) + genDir := filepath.Join(refDir, graph.GenDirName(gen)) + doc := &graph.Document{Repo: "seg", Entities: []graph.Entity{ + {ID: "aa1", QualifiedName: "p.A", Kind: "function", Name: "A"}, + }} + name := graph.SegmentFileName(0) + if err := fbwriter.WriteAtomic(filepath.Join(genDir, name), doc); err != nil { + t.Fatalf("write %s: %v", name, err) + } + m := &graph.Manifest{FormatVersion: graph.ManifestFormatVersion, Segments: []graph.SegmentMeta{{ + File: name, Kind: graph.SegmentEntities, EntityCount: 1, + MinKey: "aa1", MaxKey: "aa1", + }}} + if err := graph.WriteManifest(genDir, m); err != nil { + t.Fatalf("write manifest: %v", err) + } + manifestPath := filepath.Join(genDir, graph.ManifestFileName) + if err := os.Chtimes(manifestPath, mtime, mtime); err != nil { + t.Fatal(err) + } + if err := graph.WriteCurrentPointerRaw(refDir, graph.GenDirName(gen)); err != nil { + t.Fatalf("write current: %v", err) + } + return refDir +} + +// TestRecentlyIndexed_SegmentSet is the RED unit test for #5915 J2 slice-3 +// (DATA LOSS fix): a segment-set ref (graph./ dir + manifest.json, no +// flat graph.fb) whose manifest.json mtime is inside the grace cutoff must be +// reported recentlyIndexed==true. Before the fix, recentlyIndexed resolved +// only graph.CurrentGraphPath(refDir) (always absent for a segment-set ref), +// so this read false and the ref fell outside grace protection. +func TestRecentlyIndexed_SegmentSet(t *testing.T) { + root := t.TempDir() + now := time.Unix(1_700_000_000, 0) + refDir := writeSegmentSetRefStore(t, root, "wip-seg", 3, now.Add(-1*time.Hour)) + + if !recentlyIndexed(refDir, now.Add(-24*time.Hour)) { + t.Fatal("recentlyIndexed(segment-set ref, within grace) = false, want true") + } + if recentlyIndexed(refDir, now.Add(1*time.Hour)) { + t.Fatal("recentlyIndexed(segment-set ref, cutoff in the future) = true, want false") + } +} + +// TestRefGraphMtime_SegmentSet guards refGraphMtime resolves the manifest.json +// mtime for a segment-set ref instead of reporting the zero time. +func TestRefGraphMtime_SegmentSet(t *testing.T) { + root := t.TempDir() + now := time.Unix(1_700_000_000, 0) + mtime := now.Add(-1 * time.Hour) + refDir := writeSegmentSetRefStore(t, root, "wip-seg", 3, mtime) + + got := refGraphMtime(refDir) + if !got.Equal(mtime) { + t.Fatalf("refGraphMtime(segment-set ref) = %v, want %v", got, mtime) + } +} + +// TestDeadRef_segmentSetRefSurvivesGraceWindow is the load-bearing end-to-end +// RED test for #5915 J2 slice-3's DATA LOSS fix: a segment-set ref that is +// dead-in-git but was indexed inside the grace window must survive a full GC +// pass (Sweep) — it must NOT be reaped. Before the fix, recentlyIndexed +// resolved mtime-zero for this ref (no flat graph.fb ever existed), so the +// grace guard never engaged and the ref was reaped despite being fresh. +func TestDeadRef_segmentSetRefSurvivesGraceWindow(t *testing.T) { + repo := mkLiveRepo(t) + refsRoot := filepath.Join(t.TempDir(), "refs") + now := time.Unix(1_700_000_000, 0) + + // Primary ref, single-file, indexed long ago — present so the fixture + // mirrors a real store; survives purely on the primary guard. + writeRefStore(t, refsRoot, "main", 1000, now.Add(-100*time.Hour)) + // A dead-in-git SEGMENT-SET ref, indexed within the grace window. + segDir := writeSegmentSetRefStore(t, refsRoot, "wip-seg", 3, now.Add(-1*time.Hour)) + + ff := &fakeRefForgetter{held: map[[2]string]bool{}} + var dropped [][2]string + // git lists NOTHING for either ref — only the grace window can protect + // wip-seg (main is separately protected by the primary guard). + live := map[string]struct{}{} + s := NewDeadRefSweeper(DeadRefConfig{ + TrackedRepos: func() []string { return []string{repo} }, + LiveRefs: func(string) (map[string]struct{}, bool) { return live, true }, + PrimaryRef: func(string) string { return "main" }, + RefsDirForRepo: func(string) string { return refsRoot }, + Tier: ff, + DropReader: func(rp, ref string) { dropped = append(dropped, [2]string{rp, ref}) }, + GraceWindow: 24 * time.Hour, + Now: func() time.Time { return now }, + }) + + res := s.Sweep() + + if res.RefsReaped != 0 { + t.Fatalf("RefsReaped=%d want 0 (segment-set ref must survive the grace window)", res.RefsReaped) + } + if _, err := os.Stat(segDir); err != nil { + t.Errorf("segment-set ref dir was reaped despite grace window: %v", err) + } + if len(dropped) != 0 { + t.Errorf("DropReader called unexpectedly: %v", dropped) + } +} + +// TestRecentlyIndexed_SingleFileParity guards that the single-gen-file (and +// legacy flat) resolution is byte-identical to the pre-fix behavior. +func TestRecentlyIndexed_SingleFileParity(t *testing.T) { + root := t.TempDir() + now := time.Unix(1_700_000_000, 0) + refDir := writeRefStore(t, root, "wip", 1000, now.Add(-1*time.Hour)) + + if !recentlyIndexed(refDir, now.Add(-24*time.Hour)) { + t.Fatal("recentlyIndexed(single-file ref, within grace) = false, want true (parity)") + } + if recentlyIndexed(refDir, now.Add(1*time.Hour)) { + t.Fatal("recentlyIndexed(single-file ref, cutoff in the future) = true, want false (parity)") + } +} diff --git a/internal/graph/genpath.go b/internal/graph/genpath.go index fb5fce53a..a3565f521 100644 --- a/internal/graph/genpath.go +++ b/internal/graph/genpath.go @@ -282,6 +282,50 @@ func parseSegPointer(raw string) (genDirName string, ok bool) { return name, true } +// CurrentGraphMtime resolves dir's active graph — segment-set aware — and +// returns its freshness mtime. #5915 J2 slice-3: every existence/freshness +// gate that used to os.Stat(CurrentGraphPath(dir)) directly only ever sees a +// flat .fb path, which is ABSENT for a segment-set repo (graph./ dir + +// manifest.json, no flat .fb) — that stat silently reports "never indexed" / +// mtime-zero for a repo that is in fact freshly indexed. This is the shared, +// exported resolution those call sites route through instead of duplicating +// the descriptor-branch inline: +// +// - GraphSingleFile (including the legacy flat fallback): the resolved .fb +// file's own mtime — byte-identical to the pre-fix os.Stat behavior. +// - GraphSegmentSet: the gen dir's manifest.json mtime, the atomic +// commit point of a segment-set rebuild (verified newer than every +// segment file it names) — mirrors internal/graph/groupalgo's +// graphSourceMtime and cmd/grafel/daemon_tier.go's tierReloadCallback, +// which use the same signal for cold-wake / overlay staleness. +// - GraphAbsent, or a resolved path whose stat fails: ok=false. +// +// Lives here (not in internal/graph/groupalgo, which already has an +// unexported graphSourceMtime) because groupalgo imports internal/daemon, +// so internal/daemon call sites (deadref.go, algo/cache.go) cannot import +// groupalgo without a cycle; internal/graph has no such constraint and is +// already imported by every one of these sites. +func CurrentGraphMtime(dir string) (mtime time.Time, ok bool) { + desc, err := CurrentGraphDescriptor(dir) + if err != nil { + return time.Time{}, false + } + var path string + switch desc.Kind { + case GraphSingleFile: + path = desc.Path + case GraphSegmentSet: + path = filepath.Join(desc.GenDir, ManifestFileName) + default: + return time.Time{}, false + } + fi, statErr := os.Stat(path) + if statErr != nil { + return time.Time{}, false + } + return fi.ModTime(), true +} + // NextGen returns the next generation integer to write in dir: one greater // than the maximum generation observed among the `current` pointer and every // graph..fb already present. Returns 1 for a dir that has never held a diff --git a/internal/graph/genpath_mtime_test.go b/internal/graph/genpath_mtime_test.go new file mode 100644 index 000000000..9cf791080 --- /dev/null +++ b/internal/graph/genpath_mtime_test.go @@ -0,0 +1,61 @@ +package graph_test + +import ( + "os" + "path/filepath" + "testing" + + "github.com/cajasmota/grafel/internal/graph" +) + +// TestCurrentGraphMtime_SegmentSet is the RED test for #5915 J2 slice-3: a +// segment-set repo (graph./ dir + manifest.json, no flat graph.fb) must +// report the manifest.json mtime — the pre-fix os.Stat(CurrentGraphPath(dir)) +// pattern this helper replaces would report ok=false (mtime absent) because +// CurrentGraphPath only ever names a flat .fb path. +func TestCurrentGraphMtime_SegmentSet(t *testing.T) { + dir := t.TempDir() + genDir := writeSegmentSet(t, dir, 7, threeSegDocs()) + + mt, ok := graph.CurrentGraphMtime(dir) + if !ok { + t.Fatal("CurrentGraphMtime(segment-set dir) ok = false, want true") + } + manifestInfo, err := os.Stat(filepath.Join(genDir, graph.ManifestFileName)) + if err != nil { + t.Fatal(err) + } + if !mt.Equal(manifestInfo.ModTime()) { + t.Fatalf("CurrentGraphMtime = %v, want manifest mtime %v", mt, manifestInfo.ModTime()) + } +} + +// TestCurrentGraphMtime_SingleFileParity guards that the single-gen-file (and +// legacy flat) case reports the resolved .fb file's own mtime — byte-identical +// to the pre-fix os.Stat(CurrentGraphPath(dir)) behavior. +func TestCurrentGraphMtime_SingleFileParity(t *testing.T) { + dir := t.TempDir() + flat := filepath.Join(dir, "graph.fb") + if err := os.WriteFile(flat, []byte("legacy"), 0o644); err != nil { + t.Fatal(err) + } + mt, ok := graph.CurrentGraphMtime(dir) + if !ok { + t.Fatal("CurrentGraphMtime(flat) ok = false, want true") + } + fi, err := os.Stat(flat) + if err != nil { + t.Fatal(err) + } + if !mt.Equal(fi.ModTime()) { + t.Fatalf("CurrentGraphMtime = %v, want %v", mt, fi.ModTime()) + } +} + +// TestCurrentGraphMtime_Absent guards a never-indexed repo reports ok=false. +func TestCurrentGraphMtime_Absent(t *testing.T) { + dir := t.TempDir() + if _, ok := graph.CurrentGraphMtime(dir); ok { + t.Fatal("CurrentGraphMtime(never-indexed dir) ok = true, want false") + } +} diff --git a/internal/graph/groupalgo/groupalgo.go b/internal/graph/groupalgo/groupalgo.go index 481203994..a29e55611 100644 --- a/internal/graph/groupalgo/groupalgo.go +++ b/internal/graph/groupalgo/groupalgo.go @@ -358,30 +358,15 @@ func RunGroupAlgorithmsIncremental(group string) (*GroupAlgoResult, error) { // no flat .fb), so that stat would report the repo as never-indexed and // silently drop it from the union / freeze its overlay-staleness signal. // -// Resolution: -// - GraphSingleFile (including the legacy flat fallback): the resolved -// .fb file's own mtime — byte-identical to the pre-fix behavior. -// - GraphSegmentSet: the manifest.json mtime, the atomic flip point for a -// segment-set rebuild (mirrors #5915 J1's cmd/grafel/daemon_tier.go -// tierReloadCallback, which uses the same signal for cold-wake staleness). -// - GraphAbsent, or a resolved path whose stat fails: ok=false. +// Delegates to graph.CurrentGraphMtime (#5915 J2 slice-3), the shared +// descriptor-mtime resolution promoted to internal/graph so every other +// flat-.fb-only mtime gate (internal/daemon/deadref.go, internal/daemon/algo, +// internal/cli/branches.go) can reuse it instead of duplicating this +// descriptor branch. func graphSourceMtime(stateDir string) (mtimeNanos int64, ok bool) { - desc, err := graph.CurrentGraphDescriptor(stateDir) - if err != nil { - return 0, false - } - var path string - switch desc.Kind { - case graph.GraphSingleFile: - path = desc.Path - case graph.GraphSegmentSet: - path = filepath.Join(desc.GenDir, graph.ManifestFileName) - default: - return 0, false - } - fi, statErr := os.Stat(path) - if statErr != nil { + mt, ok := graph.CurrentGraphMtime(stateDir) + if !ok { return 0, false } - return fi.ModTime().UnixNano(), true + return mt.UnixNano(), true } From 642d546ea814cd357a12348cd1554ad987481425 Mon Sep 17 00:00:00 2001 From: Jorge Cajas Date: Thu, 23 Jul 2026 00:47:16 +0800 Subject: [PATCH 2/4] fix(daemon/algo): segment-set-aware algo-results cache staleness gate (#5915 J2 slice-3 site 2) readFromDisk/writeToDisk resolved graph freshness via os.Stat(graph.CurrentGraphPath(stateDir)), a flat-.fb-only path absent for a segment-set state dir (graph./ dir + manifest.json, no flat .fb). That made the disk cache permanently unusable for a segment-set repo: readFromDisk always missed (forcing recompute on every Get) and writeToDisk always errored (the cache was never persisted). Switch both to graph.CurrentGraphMtime (added in the deadref.go fix, internal/graph/genpath.go): single-file resolution is byte-identical to before; segment-set resolves the gen dir's manifest.json mtime. Tests: TestCacheMissComputesAndPersists_SegmentSet, TestCacheHitServesFromDisk_SegmentSet, TestStaleCacheRecomputesWhenSegmentSetGenAdvances (a reindex that flips current to a new gen invalidates the cache). Verified RED before the fix (2 of 3 failed), GREEN after; existing single-file tests unaffected. --- internal/daemon/algo/cache.go | 36 +++-- internal/daemon/algo/cache_segmentset_test.go | 152 ++++++++++++++++++ 2 files changed, 173 insertions(+), 15 deletions(-) create mode 100644 internal/daemon/algo/cache_segmentset_test.go diff --git a/internal/daemon/algo/cache.go b/internal/daemon/algo/cache.go index 0118c5ead..f702b56cf 100644 --- a/internal/daemon/algo/cache.go +++ b/internal/daemon/algo/cache.go @@ -160,21 +160,27 @@ func cacheKey(stateDir string) string { return stateDir } // staleness. func readFromDisk(stateDir string) (*Results, bool) { cachePath := filepath.Join(stateDir, cacheFileName) - graphPath := graph.CurrentGraphPath(stateDir) // #5891: resolve active gen + // #5915 J2 slice-3: graph.CurrentGraphMtime is segment-set aware — the + // prior os.Stat(graph.CurrentGraphPath(stateDir)) only ever named a flat + // .fb path, which is ABSENT for a segment-set stateDir (graph./ dir + + // manifest.json, no flat .fb). That made the disk cache permanently + // unvalidatable for a segment-set repo: readFromDisk always missed + // (forcing recompute every call) and writeToDisk always errored (the + // cache was never persisted). + graphMtime, ok := graph.CurrentGraphMtime(stateDir) + if !ok { + return nil, false // no active graph found; nothing to validate against + } cacheInfo, err := os.Stat(cachePath) if err != nil { return nil, false // cache file does not exist } - graphInfo, err := os.Stat(graphPath) - if err != nil { - return nil, false // graph.fb not found; nothing to validate against - } - // Staleness check: graph.fb must not be newer than the cache. + // Staleness check: the active graph must not be newer than the cache. // Use a 1-second tolerance to absorb filesystem timestamp granularity. - if graphInfo.ModTime().After(cacheInfo.ModTime().Add(time.Second)) { - return nil, false // graph.fb was updated after we cached + if graphMtime.After(cacheInfo.ModTime().Add(time.Second)) { + return nil, false // graph was updated after we cached } data, err := os.ReadFile(cachePath) @@ -185,8 +191,8 @@ func readFromDisk(stateDir string) (*Results, bool) { if err := json.Unmarshal(data, &env); err != nil { return nil, false } - // Validate the embedded mtime matches the current graph.fb mtime. - if env.GraphMtime != graphInfo.ModTime().UnixNano() { + // Validate the embedded mtime matches the current graph's mtime. + if env.GraphMtime != graphMtime.UnixNano() { return nil, false } return &env.Results, true @@ -195,14 +201,14 @@ func readFromDisk(stateDir string) (*Results, bool) { // writeToDisk atomically writes the results to /algo_results.fb. // Uses a temp-file + rename for crash-safety. func writeToDisk(stateDir string, r *Results) error { - graphPath := graph.CurrentGraphPath(stateDir) // #5891: resolve active gen - graphInfo, err := os.Stat(graphPath) - if err != nil { - return fmt.Errorf("stat graph.fb: %w", err) + // #5915 J2 slice-3: segment-set aware (see readFromDisk). + graphMtime, ok := graph.CurrentGraphMtime(stateDir) + if !ok { + return fmt.Errorf("stat graph: no active graph found in %s", stateDir) } env := envelope{ - GraphMtime: graphInfo.ModTime().UnixNano(), + GraphMtime: graphMtime.UnixNano(), Results: *r, } data, err := json.Marshal(&env) diff --git a/internal/daemon/algo/cache_segmentset_test.go b/internal/daemon/algo/cache_segmentset_test.go new file mode 100644 index 000000000..576bd54ff --- /dev/null +++ b/internal/daemon/algo/cache_segmentset_test.go @@ -0,0 +1,152 @@ +package algo + +import ( + "context" + "os" + "path/filepath" + "sync/atomic" + "testing" + "time" + + "github.com/cajasmota/grafel/internal/graph" + "github.com/cajasmota/grafel/internal/graph/fbwriter" +) + +// makeSegmentSetStateDir hand-builds a 1-segment gen dir (graph./ with +// seg-0000.fb + manifest.json) under a fresh temp state dir and points +// `current` at it — mirroring the fixture builders at +// internal/daemon/state_path_graphfbexists_test.go / +// internal/graph/segmentset_test.go. No flat graph.fb is ever written. +func makeSegmentSetStateDir(t *testing.T, gen uint64) string { + t.Helper() + dir := t.TempDir() + doc := &graph.Document{Repo: "seg", Entities: []graph.Entity{ + {ID: "aa1", QualifiedName: "p.A", Kind: "function", Name: "A"}, + }} + genDir := filepath.Join(dir, graph.GenDirName(gen)) + name := graph.SegmentFileName(0) + if err := fbwriter.WriteAtomic(filepath.Join(genDir, name), doc); err != nil { + t.Fatalf("write %s: %v", name, err) + } + m := &graph.Manifest{FormatVersion: graph.ManifestFormatVersion, Segments: []graph.SegmentMeta{{ + File: name, Kind: graph.SegmentEntities, EntityCount: 1, + MinKey: "aa1", MaxKey: "aa1", + }}} + if err := graph.WriteManifest(genDir, m); err != nil { + t.Fatalf("write manifest: %v", err) + } + if err := graph.WriteCurrentPointerRaw(dir, graph.GenDirName(gen)); err != nil { + t.Fatalf("write current: %v", err) + } + return dir +} + +// TestCacheMissComputesAndPersists_SegmentSet is the RED test for #5915 J2 +// slice-3: a segment-set state dir (graph./ dir + manifest.json, no flat +// graph.fb) must still persist algo_results.fb after a compute. Before the +// fix, writeToDisk's os.Stat(graph.CurrentGraphPath(stateDir)) always failed +// for a segment-set dir (no flat .fb to stat), so the cache was silently +// never written to disk. +func TestCacheMissComputesAndPersists_SegmentSet(t *testing.T) { + dir := makeSegmentSetStateDir(t, 3) + + var calls atomic.Int32 + c := New(func(_ context.Context, _, _ string) (*Results, error) { + calls.Add(1) + return fakeResults(), nil + }) + + r, err := c.Get(context.Background(), dir, "/repo", "main") + if err != nil { + t.Fatalf("Get: %v", err) + } + if calls.Load() != 1 { + t.Fatalf("expected 1 compute call, got %d", calls.Load()) + } + if r.PageRank["a"] != 0.5 { + t.Errorf("unexpected PageRank: %v", r.PageRank) + } + if _, err := os.Stat(filepath.Join(dir, cacheFileName)); err != nil { + t.Errorf("algo_results.fb not written for segment-set state dir: %v", err) + } +} + +// TestCacheHitServesFromDisk_SegmentSet is the RED test guarding that a +// segment-set state dir's disk cache is actually served on a second Get +// (not recomputed every time). Before the fix, readFromDisk's +// os.Stat(graph.CurrentGraphPath(stateDir)) always failed for a segment-set +// dir, so every call recomputed. +func TestCacheHitServesFromDisk_SegmentSet(t *testing.T) { + dir := makeSegmentSetStateDir(t, 3) + + var calls atomic.Int32 + c := New(func(_ context.Context, _, _ string) (*Results, error) { + calls.Add(1) + return fakeResults(), nil + }) + + if _, err := c.Get(context.Background(), dir, "/repo", "main"); err != nil { + t.Fatalf("first Get: %v", err) + } + + // New Cache instance simulates a fresh in-memory state — must still hit + // the disk cache. + c2 := New(func(_ context.Context, _, _ string) (*Results, error) { + calls.Add(1) + return fakeResults(), nil + }) + if _, err := c2.Get(context.Background(), dir, "/repo", "main"); err != nil { + t.Fatalf("second Get: %v", err) + } + if calls.Load() != 1 { + t.Errorf("expected 1 total compute call (second should be disk hit) for segment-set state dir, got %d", calls.Load()) + } +} + +// TestStaleCacheRecomputesWhenSegmentSetGenAdvances guards that a fresh +// segment-set generation (a reindex that flips `current` to a new gen dir, +// advancing the manifest.json mtime) invalidates the disk cache, mirroring +// TestStaleCacheRecomputesWhenGraphFBUpdated for the single-file case. +func TestStaleCacheRecomputesWhenSegmentSetGenAdvances(t *testing.T) { + dir := makeSegmentSetStateDir(t, 3) + + var calls atomic.Int32 + c := New(func(_ context.Context, _, _ string) (*Results, error) { + calls.Add(1) + return fakeResults(), nil + }) + + if _, err := c.Get(context.Background(), dir, "/repo", "main"); err != nil { + t.Fatalf("first Get: %v", err) + } + + // Simulate a reindex: write a NEW generation and flip `current` to it. + // Sleep to ensure the manifest.json mtime advances beyond the staleness + // tolerance (1s). + time.Sleep(1100 * time.Millisecond) + doc := &graph.Document{Repo: "seg", Entities: []graph.Entity{ + {ID: "bb2", QualifiedName: "p.B", Kind: "function", Name: "B"}, + }} + newGenDir := filepath.Join(dir, graph.GenDirName(4)) + name := graph.SegmentFileName(0) + if err := fbwriter.WriteAtomic(filepath.Join(newGenDir, name), doc); err != nil { + t.Fatalf("write new gen %s: %v", name, err) + } + m := &graph.Manifest{FormatVersion: graph.ManifestFormatVersion, Segments: []graph.SegmentMeta{{ + File: name, Kind: graph.SegmentEntities, EntityCount: 1, + MinKey: "bb2", MaxKey: "bb2", + }}} + if err := graph.WriteManifest(newGenDir, m); err != nil { + t.Fatalf("write new manifest: %v", err) + } + if err := graph.WriteCurrentPointerRaw(dir, graph.GenDirName(4)); err != nil { + t.Fatalf("flip current: %v", err) + } + + if _, err := c.Get(context.Background(), dir, "/repo", "main"); err != nil { + t.Fatalf("second Get: %v", err) + } + if calls.Load() != 2 { + t.Errorf("expected recompute after segment-set gen advance, got %d compute calls", calls.Load()) + } +} From cbe20a33fc06fbdf4f818db00846814e713a7122 Mon Sep 17 00:00:00 2001 From: Jorge Cajas Date: Thu, 23 Jul 2026 00:49:10 +0800 Subject: [PATCH 3/4] fix(cli): segment-set-aware tier inference for branches (#5915 J2 slice-3 site 3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit inferTierFromDisk stat'd only [graph.CurrentGraphPath(stateDir), graph.json] to determine a ref's tier/lastSeen — a flat-.fb-only gate absent for a segment-set ref (graph./ dir + manifest.json, no flat .fb). A freshly-indexed segment-set ref therefore inferred tier cold with a zero lastSeen. Switch to graph.CurrentGraphMtime (internal/graph/genpath.go), preserving the existing max-with-graph.json comparison. Single-file resolution is byte-identical to before. Tests: TestInferTierFromDisk_SegmentSet (RED before, GREEN after), TestInferTierFromDisk_SingleFileParity, TestInferTierFromDisk_NeverIndexed. Verified RED before the fix (tier=cold, lastSeen=zero for a 1-minute-old segment-set ref), GREEN after. --- internal/cli/branches.go | 22 +++-- internal/cli/branches_segmentset_test.go | 102 +++++++++++++++++++++++ 2 files changed, 115 insertions(+), 9 deletions(-) create mode 100644 internal/cli/branches_segmentset_test.go diff --git a/internal/cli/branches.go b/internal/cli/branches.go index a62b50b2d..f0b3f7b31 100644 --- a/internal/cli/branches.go +++ b/internal/cli/branches.go @@ -233,19 +233,23 @@ func repoBaseForSlug(storeRoot string, repo registry.Repo) string { } // inferTierFromDisk makes a best-effort tier determination from file mtimes -// without requiring a live daemon. It reads the graph.fb mtime and maps it -// to HOT/WARM/COLD/EXPIRED using the default TTL windows. +// without requiring a live daemon. It reads the active graph's mtime and maps +// it to HOT/WARM/COLD/EXPIRED using the default TTL windows. +// +// #5915 J2 slice-3: graph.CurrentGraphMtime is segment-set aware — the prior +// os.Stat over [graph.CurrentGraphPath(stateDir), graph.json] only ever named +// a flat .fb path, which is ABSENT for a segment-set ref (graph./ dir + +// manifest.json, no flat .fb). That silently inferred mtime-zero (tier "cold" +// with a zero lastSeen) for a freshly-indexed segment-set ref. func inferTierFromDisk(stateDir string) (tierStr string, lastSeen time.Time) { - fbPath := graph.CurrentGraphPath(stateDir) // #5891: resolve active gen jsonPath := filepath.Join(stateDir, "graph.json") var mtime time.Time - for _, p := range []string{fbPath, jsonPath} { - if fi, err := os.Stat(p); err == nil { - if fi.ModTime().After(mtime) { - mtime = fi.ModTime() - } - } + if mt, ok := graph.CurrentGraphMtime(stateDir); ok { + mtime = mt + } + if fi, err := os.Stat(jsonPath); err == nil && fi.ModTime().After(mtime) { + mtime = fi.ModTime() } if mtime.IsZero() { diff --git a/internal/cli/branches_segmentset_test.go b/internal/cli/branches_segmentset_test.go new file mode 100644 index 000000000..b264d1bad --- /dev/null +++ b/internal/cli/branches_segmentset_test.go @@ -0,0 +1,102 @@ +package cli + +import ( + "os" + "path/filepath" + "testing" + "time" + + "github.com/cajasmota/grafel/internal/graph" + "github.com/cajasmota/grafel/internal/graph/fbwriter" +) + +// writeSegmentSetFixtureForTierTest hand-builds a 1-segment gen dir +// (graph./ with seg-0000.fb + manifest.json) under stateDir and points +// `current` at it, then stamps the manifest.json mtime — same fixture shape +// used elsewhere in this slice (internal/daemon/state_path_graphfbexists_test.go, +// internal/cli/status_stats_segmentset_test.go). +func writeSegmentSetFixtureForTierTest(t *testing.T, stateDir string, gen uint64, mtime time.Time) { + t.Helper() + doc := &graph.Document{Repo: "seg", Entities: []graph.Entity{ + {ID: "aa1", QualifiedName: "p.A", Kind: "function", Name: "A"}, + }} + genDir := filepath.Join(stateDir, graph.GenDirName(gen)) + name := graph.SegmentFileName(0) + if err := fbwriter.WriteAtomic(filepath.Join(genDir, name), doc); err != nil { + t.Fatalf("write %s: %v", name, err) + } + m := &graph.Manifest{FormatVersion: graph.ManifestFormatVersion, Segments: []graph.SegmentMeta{{ + File: name, Kind: graph.SegmentEntities, EntityCount: 1, + MinKey: "aa1", MaxKey: "aa1", + }}} + if err := graph.WriteManifest(genDir, m); err != nil { + t.Fatalf("write manifest: %v", err) + } + manifestPath := filepath.Join(genDir, graph.ManifestFileName) + if err := os.Chtimes(manifestPath, mtime, mtime); err != nil { + t.Fatal(err) + } + if err := graph.WriteCurrentPointerRaw(stateDir, graph.GenDirName(gen)); err != nil { + t.Fatalf("write current: %v", err) + } +} + +// TestInferTierFromDisk_SegmentSet is the RED test for #5915 J2 slice-3: a +// segment-set state dir (graph./ dir + manifest.json, no flat graph.fb) +// indexed a minute ago must infer tier "hot" with a non-zero lastSeen. Before +// the fix, inferTierFromDisk stat'd only [graph.CurrentGraphPath(stateDir), +// graph.json] — both absent for a segment-set dir — so it silently inferred +// tier "cold" with a zero lastSeen (mtime never found). +func TestInferTierFromDisk_SegmentSet(t *testing.T) { + stateDir := t.TempDir() + mtime := time.Now().Add(-1 * time.Minute) + writeSegmentSetFixtureForTierTest(t, stateDir, 5, mtime) + + tierStr, lastSeen := inferTierFromDisk(stateDir) + + if tierStr != "hot" { + t.Errorf("tierStr = %q, want %q", tierStr, "hot") + } + if lastSeen.IsZero() { + t.Fatal("lastSeen is zero, want non-zero (segment-set manifest.json mtime)") + } + if diff := lastSeen.Sub(mtime); diff < -time.Second || diff > time.Second { + t.Errorf("lastSeen = %v, want ~%v", lastSeen, mtime) + } +} + +// TestInferTierFromDisk_SingleFileParity guards the single-gen-file (and +// legacy flat) resolution is byte-identical to the pre-fix behavior. +func TestInferTierFromDisk_SingleFileParity(t *testing.T) { + stateDir := t.TempDir() + flat := filepath.Join(stateDir, "graph.fb") + mtime := time.Now().Add(-1 * time.Minute) + if err := os.WriteFile(flat, []byte("legacy"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Chtimes(flat, mtime, mtime); err != nil { + t.Fatal(err) + } + + tierStr, lastSeen := inferTierFromDisk(stateDir) + + if tierStr != "hot" { + t.Errorf("tierStr = %q, want %q", tierStr, "hot") + } + if diff := lastSeen.Sub(mtime); diff < -time.Second || diff > time.Second { + t.Errorf("lastSeen = %v, want ~%v", lastSeen, mtime) + } +} + +// TestInferTierFromDisk_NeverIndexed guards a never-indexed state dir infers +// tier "cold" with a zero lastSeen. +func TestInferTierFromDisk_NeverIndexed(t *testing.T) { + stateDir := t.TempDir() + tierStr, lastSeen := inferTierFromDisk(stateDir) + if tierStr != "cold" { + t.Errorf("tierStr = %q, want %q", tierStr, "cold") + } + if !lastSeen.IsZero() { + t.Errorf("lastSeen = %v, want zero", lastSeen) + } +} From 41346852c3ec1b6f91bb5efa33b83a6cd687cf28 Mon Sep 17 00:00:00 2001 From: Jorge Cajas Date: Thu, 23 Jul 2026 01:03:24 +0800 Subject: [PATCH 4/4] fix(cli): segment-set-aware embedding-GC walk in cleanup.go (#5915 J2 slice-3, site 4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit collectActiveEmbeddingHashes walked the daemon store looking for graph.IsGraphFileName matches (flat graph.fb or gen-file graph..fb), then guarded against double-processing a ref's state dir with 'path != graph.CurrentGraphPath(filepath.Dir(path))'. Neither recognizes a segment-set ref (graph./ dir of seg-NNNN.fb + manifest.json, no flat .fb): seg-NNNN.fb matches neither IsGraphFileName pattern, so the walk silently skipped a live segment-set ref's state dir entirely — its entities' embedding_ref values were never added to the 'active' set fed into the embedding-cache TTL sweep (grafel cleanup), so a still-referenced .vec cache entry for a segment-set repo could be reaped purely by age. Fix: recognize filepath.Base(path) == graph.ManifestFileName as an additional segment-set marker inside the walk, then validate via graph.CurrentGraphDescriptor(candidateStateDir) that the manifest is in fact the CURRENT generation's manifest (not a stale/previous gen left on disk) before treating candidateStateDir as a ref to load. Single-file / legacy flat behavior is unchanged byte-for-byte: the existing IsGraphFileName + CurrentGraphPath dedup branch is untouched. Note on scope, precisely: the walk callback never deletes graph files itself — collectActiveEmbeddingHashes only builds the 'active' hash set that a SEPARATE embed.Cache.Sweep(active, ttlDays) call consults to decide which .vec files to remove. So the observable effect of this bug was never 'a live segment-set's segments/manifest get deleted' (nothing in this walk deletes graph files); it was 'a live segment-set repo's embedding cache entries can be swept as stale despite being referenced,' which is what the new tests assert against. Also fixes a separate, pre-existing bug surfaced while building the segment-set test fixture: internal/graph/load.go's fbEntityToGraphEntity (the FB->Document conversion shared by both the single-file and segment-set loaders) never restored an entity's EmbeddingRef (PH8 #2100) from its FlatBuffers slot, even though fbwriter correctly persists it. Every FB-backed load — single-file or segment-set — silently returned EmbeddingRef == "" regardless of what was written, which is why even the SINGLE-FILE parity test for this walk initially failed RED alongside the segment-set test. Purely additive (EmbeddingRef was always the zero value before), covered by a new dedicated regression test in internal/graph/load_test.go (TestLoadGraphFromDir_EmbeddingRefRoundTrip). Tests: internal/cli/cleanup_segmentset_test.go adds TestCollectActiveEmbeddingHashes_SegmentSet (RED before the cleanup.go fix: active map came back empty for a live segment-set ref) and TestCollectActiveEmbeddingHashes_SingleFileParity (guards the existing flat/gen-file resolution is unchanged). Verified isolated RED for the walk-routing fix alone (load.go's EmbeddingRef fix kept, cleanup.go's fix reverted): segment-set test fails, single-file parity test passes — the two bugs are cleanly separated. gofmt -l clean; go vet and go test green across internal/cli, internal/graph, internal/daemon, internal/daemon/algo, internal/embed. Part of #5915 J2 slice-3 (site 4 of 4). --- internal/cli/cleanup.go | 55 ++++++++++++--- internal/cli/cleanup_segmentset_test.go | 94 +++++++++++++++++++++++++ internal/graph/load.go | 15 ++++ internal/graph/load_test.go | 47 +++++++++++++ 4 files changed, 202 insertions(+), 9 deletions(-) create mode 100644 internal/cli/cleanup_segmentset_test.go diff --git a/internal/cli/cleanup.go b/internal/cli/cleanup.go index 50cc8db38..d9a1ee005 100644 --- a/internal/cli/cleanup.go +++ b/internal/cli/cleanup.go @@ -120,6 +120,18 @@ func runCleanup(w io.Writer, dryRun bool, ttlDays int) error { // plus (legacy) /.grafel/graph.fb for repos indexed before PH1a. // We walk the daemon store directory; .grafel sidecar graphs are not // swept (they are repo-local, managed by the repo owner). +// +// #5915 J2 slice-3: a segment-set ref (graph./ dir + seg-NNNN.fb + +// manifest.json, no flat .fb) has NO file matching graph.IsGraphFileName — +// seg-NNNN.fb matches neither the legacy flat name nor the graph..fb +// pattern — so the walk skipped every segment-set ref's state dir entirely. +// Its entities' embedding refs were never added to `active`, so the sweep +// below (embed cache TTL GC) could reap embedding cache entries still +// referenced by a live segment-set graph. The walk now ALSO recognises +// manifest.json as a segment-set's canonical marker file and, when it is the +// active generation's manifest, loads the ref's STATE dir (the manifest's +// gen dir's parent) exactly once — mirroring the single-file branch's +// dedup-to-the-active-generation guard. func collectActiveEmbeddingHashes() (map[string]bool, error) { h, err := registry.HomeDir() if err != nil { @@ -136,17 +148,42 @@ func collectActiveEmbeddingHashes() (map[string]bool, error) { if d.IsDir() { return nil } - // #5891: recognise generation files (graph..fb), and process each - // state dir exactly once — only when this path is the ACTIVE generation - // resolved by the pointer (or the legacy flat graph.fb) — so a dir with - // current+previous gens isn't walked twice. - if !graph.IsGraphFileName(filepath.Base(path)) { - return nil - } - if path != graph.CurrentGraphPath(filepath.Dir(path)) { + + var stateDir string + switch { + case filepath.Base(path) == graph.ManifestFileName: + // Candidate segment-set marker: path = /graph./manifest.json. + genDir := filepath.Dir(path) + candidate := filepath.Dir(genDir) + desc, dErr := graph.CurrentGraphDescriptor(candidate) + if dErr != nil || desc.Kind != graph.GraphSegmentSet { + return nil + } + // Only the ACTIVE generation's manifest counts, so a stale + // previous-gen manifest (kept around by GCStaleGens' retention + // window) isn't double-processed. + if filepath.Join(desc.GenDir, graph.ManifestFileName) != path { + return nil + } + stateDir = candidate + + case graph.IsGraphFileName(filepath.Base(path)): + // #5891: recognise generation files (graph..fb), and process + // each state dir exactly once — only when this path is the ACTIVE + // generation resolved by the pointer (or the legacy flat + // graph.fb) — so a dir with current+previous gens isn't walked + // twice. + candidate := filepath.Dir(path) + if path != graph.CurrentGraphPath(candidate) { + return nil + } + stateDir = candidate + + default: return nil } - doc, loadErr := graph.LoadGraphFromDir(filepath.Dir(path)) + + doc, loadErr := graph.LoadGraphFromDir(stateDir) if loadErr != nil { return nil // corrupt/missing — skip } diff --git a/internal/cli/cleanup_segmentset_test.go b/internal/cli/cleanup_segmentset_test.go new file mode 100644 index 000000000..66b02a605 --- /dev/null +++ b/internal/cli/cleanup_segmentset_test.go @@ -0,0 +1,94 @@ +package cli + +import ( + "os" + "path/filepath" + "testing" + + "github.com/cajasmota/grafel/internal/graph" + "github.com/cajasmota/grafel/internal/graph/fbwriter" +) + +// writeSegmentSetRefForEmbeddingWalk hand-builds a 1-segment gen dir +// (graph./ with seg-0000.fb + manifest.json) under refDir, points +// `current` at it, and stamps the single entity with embeddingRef — mirroring +// the fixture builders used elsewhere in this slice +// (internal/daemon/state_path_graphfbexists_test.go). No flat graph.fb is +// ever written, matching a real segment-set ref. +func writeSegmentSetRefForEmbeddingWalk(t *testing.T, refDir string, gen uint64, embeddingRef string) { + t.Helper() + doc := &graph.Document{Repo: "seg", Entities: []graph.Entity{ + {ID: "aa1", QualifiedName: "p.A", Kind: "function", Name: "A", EmbeddingRef: embeddingRef}, + }} + genDir := filepath.Join(refDir, graph.GenDirName(gen)) + name := graph.SegmentFileName(0) + if err := fbwriter.WriteAtomic(filepath.Join(genDir, name), doc); err != nil { + t.Fatalf("write %s: %v", name, err) + } + m := &graph.Manifest{FormatVersion: graph.ManifestFormatVersion, Segments: []graph.SegmentMeta{{ + File: name, Kind: graph.SegmentEntities, EntityCount: 1, + MinKey: "aa1", MaxKey: "aa1", + }}} + if err := graph.WriteManifest(genDir, m); err != nil { + t.Fatalf("write manifest: %v", err) + } + if err := graph.WriteCurrentPointerRaw(refDir, graph.GenDirName(gen)); err != nil { + t.Fatalf("write current: %v", err) + } +} + +// TestCollectActiveEmbeddingHashes_SegmentSet is the RED test for #5915 J2 +// slice-3: a live segment-set ref's embedding_ref values must be reported +// active. Before the fix, seg-NNNN.fb matches neither graph.IsGraphFileName's +// legacy-flat nor graph..fb pattern, so the walk skipped a segment-set +// ref's state dir entirely — its embedding refs were never collected as +// active, so a subsequent TTL-based embedding-cache sweep could reap a +// .vec cache entry that a live segment-set graph still references. +func TestCollectActiveEmbeddingHashes_SegmentSet(t *testing.T) { + home := t.TempDir() + t.Setenv("GRAFEL_HOME", home) + + refDir := filepath.Join(home, "store", "myrepo-abc123", "refs", "main") + if err := os.MkdirAll(refDir, 0o755); err != nil { + t.Fatal(err) + } + writeSegmentSetRefForEmbeddingWalk(t, refDir, 4, "sha256:seg-live-hash") + + active, err := collectActiveEmbeddingHashes() + if err != nil { + t.Fatalf("collectActiveEmbeddingHashes: %v", err) + } + if !active["sha256:seg-live-hash"] { + t.Fatalf("active = %v, want it to include the segment-set ref's embedding hash", active) + } +} + +// TestCollectActiveEmbeddingHashes_SingleFileParity guards the single-gen-file +// (and legacy flat) resolution is unchanged: exactly one state dir's +// embedding refs are collected, not double-counted across generations. +func TestCollectActiveEmbeddingHashes_SingleFileParity(t *testing.T) { + home := t.TempDir() + t.Setenv("GRAFEL_HOME", home) + + refDir := filepath.Join(home, "store", "myrepo-abc123", "refs", "main") + if err := os.MkdirAll(refDir, 0o755); err != nil { + t.Fatal(err) + } + doc := &graph.Document{Repo: "single", Entities: []graph.Entity{ + {ID: "aaaa0001", QualifiedName: "p.A", Kind: "function", Name: "A", EmbeddingRef: "sha256:single-live-hash"}, + }} + if err := fbwriter.WriteAtomic(filepath.Join(refDir, graph.GenFileName(1)), doc); err != nil { + t.Fatal(err) + } + if err := graph.WriteCurrentPointer(refDir, graph.GenFileName(1)); err != nil { + t.Fatal(err) + } + + active, err := collectActiveEmbeddingHashes() + if err != nil { + t.Fatalf("collectActiveEmbeddingHashes: %v", err) + } + if !active["sha256:single-live-hash"] { + t.Fatalf("active = %v, want it to include the single-file ref's embedding hash (parity)", active) + } +} diff --git a/internal/graph/load.go b/internal/graph/load.go index 1ab6164d7..16f28a566 100644 --- a/internal/graph/load.go +++ b/internal/graph/load.go @@ -565,6 +565,21 @@ func fbEntityToGraphEntity(e *fb.Entity, si *stringInterner) Entity { StartLine: int(e.SourceLine()), } ent.properties = props + // #5915 J2 slice-3 (discovered while testing cleanup.go's embedding-GC + // walk): restore the PH8 (#2100) embedding_ref field from its dedicated + // FB slot. This was never restored here — every entity loaded via the + // FB path (loadFBDocument AND the segment-set loader, which shares this + // same conversion) came back with EmbeddingRef == "" regardless of what + // fbwriter actually wrote, silently defeating + // internal/cli/cleanup.go's collectActiveEmbeddingHashes for every + // FB-backed graph (single-file or segment-set) — the embedding TTL sweep + // could never see an entity's embedding as "active", so unreferenced-ness + // was determined by TTL age alone. Purely additive: EmbeddingRef was + // always the zero value before, so this can only ever populate a + // previously-empty field, never change an already-non-empty one. + if er := string(e.EmbeddingRef()); er != "" { + ent.EmbeddingRef = er + } // The Module field is stored as a top-level FB scalar by the writer // (see fbwriter.buildEntity). Restore it into Properties["module"] // so callers that read props["module"] continue to work. PropSet keeps diff --git a/internal/graph/load_test.go b/internal/graph/load_test.go index 8b2703a66..dc2799545 100644 --- a/internal/graph/load_test.go +++ b/internal/graph/load_test.go @@ -178,3 +178,50 @@ func TestLoadGraphFromDir_EntityProperties(t *testing.T) { handlerEnt.PropGet("framework"), "gin") } } + +// TestLoadGraphFromDir_EmbeddingRefRoundTrip verifies that an entity's +// EmbeddingRef (PH8 / #2100) is preserved through the FB round-trip. +// +// Regression test: fbEntityToGraphEntity (the shared FB->Document entity +// conversion used by both the single-file and segment-set load paths) never +// copied e.EmbeddingRef() into the resulting graph.Entity, even though +// fbwriter correctly persists it. Every FB-backed load (single-file or +// segment-set) silently came back with EmbeddingRef == "" regardless of what +// was written, which defeated internal/cli/cleanup.go's +// collectActiveEmbeddingHashes: an entity's embedding could never be +// reported "active", so the embedding-cache TTL sweep in `grafel cleanup` +// determined unreferenced-ness by age alone. Discovered while building the +// #5915 J2 slice-3 segment-set fixture for that walk. Purely additive fix: +// EmbeddingRef was always the zero value before, so this can only ever +// populate a previously-empty field. +func TestLoadGraphFromDir_EmbeddingRefRoundTrip(t *testing.T) { + t.Parallel() + dir := t.TempDir() + doc := makeTestDoc() + + doc.Entities[0].EmbeddingRef = "sha256:embedding-round-trip-hash" + + if err := fbwriter.WriteAtomic(filepath.Join(dir, "graph.fb"), doc); err != nil { + t.Fatalf("write graph.fb: %v", err) + } + + got, err := graph.LoadGraphFromDir(dir) + if err != nil { + t.Fatalf("LoadGraphFromDir: %v", err) + } + + var handlerEnt *graph.Entity + for i := range got.Entities { + if got.Entities[i].Name == "MyHandler" { + handlerEnt = &got.Entities[i] + break + } + } + if handlerEnt == nil { + t.Fatal("MyHandler entity not found after FB round-trip") + } + if handlerEnt.EmbeddingRef != "sha256:embedding-round-trip-hash" { + t.Errorf("EmbeddingRef: got %q want %q", + handlerEnt.EmbeddingRef, "sha256:embedding-round-trip-hash") + } +}