feat(#5904): description side-table — enrichment writeback stops rewriting graph.fb (#5915 P1)

internal/dashboard/graphstate.go

@@ -25,6 +25,7 @@ import (
 
 	"github.com/cajasmota/grafel/internal/daemon"
 	"github.com/cajasmota/grafel/internal/graph"
+	"github.com/cajasmota/grafel/internal/graph/descriptions"
 	"github.com/cajasmota/grafel/internal/graph/fbreader"
 	"github.com/cajasmota/grafel/internal/registry"
 	"github.com/cajasmota/grafel/internal/types"
@@ -637,6 +638,19 @@ func (c *GraphCache) loadGroupForRef(groupName, ref string) (*DashGroup, error)
 				})
 			}
 			dr.Doc = doc
+			// Description side-table (#5904 PR-a): ADDITIVELY stamp any
+			// <stateDir>/descriptions.json onto this repo's entities so the
+			// PropGet("description") readers (openapi/DAG export) surface agent
+			// write-back descriptions again now that write-back no longer rewrites
+			// the graph. Absence/corrupt/stale-tolerant: a miss leaves the entity's
+			// own (extractor-native) description intact.
+			if sc, ok := descriptions.Read(stateDir); ok {
+				for i := range doc.Entities {
+					if d, has := sc.Results[doc.Entities[i].ID]; has {
+						doc.Entities[i].PropSet("description", d)
+					}
+				}
+			}
 			// S8 (#2159): open a zero-copy mmap reader alongside the Document
 			// so handlers that only need to iterate entities/relationships can
 			// avoid materialising the full heap slice. Best-effort: failures

internal/dashboard/handlers_enrichment_writeback.go

@@ -41,7 +41,7 @@ import (
 
 	"github.com/cajasmota/grafel/internal/daemon"
 	"github.com/cajasmota/grafel/internal/graph"
-	"github.com/cajasmota/grafel/internal/graph/fbwriter"
+	"github.com/cajasmota/grafel/internal/graph/descriptions"
 )
 
 // ─────────────────────────────────────────────────────────────────────────────
@@ -193,49 +193,43 @@ func (s *Server) handleEnrichmentWriteback(w http.ResponseWriter, r *http.Reques
 	now := time.Now().UTC()
 	enrichedAt := now.Format(time.RFC3339)
 
-	// ─── Target 1: update in-memory graph property ────────────────────────
+	// ─── Target 1: description SIDE-TABLE upsert (no graph rewrite) ────────
+	// #5904 PR-a: the description is written to the per-repo
+	// <stateDir>/descriptions.json side-table and merged back onto entities at
+	// READ time (an additive PropSet of "description" — see descriptions.Read /
+	// applyDescriptionOverlay). The graph file / generation is NEVER touched.
+	//
+	// The previous implementation rewrote the WHOLE graph (fbwriter.WriteGraphGen
+	// + graph.WriteAtomic on graph.json). For a #5901 segment-set that resident
+	// Document is a COLLAPSED union of every segment, so re-serialising it as one
+	// flat graph.<gen>.fb re-materialised the entire graph in memory (the #5915 P1
+	// OOM) and discarded the segmented layout. The side-table removes that hazard:
+	// a segment-set stays a segment-set.
+	//
+	// The in-memory PropSet below keeps the CURRENTLY-resident group visibly
+	// enriched (so a read racing the Invalidate still sees it); durability comes
+	// from the sidecar, which the next load / serving-path re-check merges back.
 	if found.entity.PropLen() == 0 {
 		found.entity.PropsReplace(make(map[string]string))
 	}
 	found.entity.PropSet("description", req.Description)
 
-	// Persist the updated graph to disk — both graph.fb (canonical binary,
-	// preferred by LoadGraphFromDir / MCP) and graph.json (backward-compatible
-	// JSON, read by debug tools and external integrations). Writing only one
-	// format leaves the other stale, causing ghost entity IDs for direct
-	// graph.json readers (fixes #1702).
 	stateDir := daemon.StateDirForRepo(found.repoPath)
+	// graphPath is informational only (echoed in the response); it is NOT written.
 	graphPath := daemon.GraphPathForRepo(found.repoPath)
 
-	// #5891: write a NEW graph.<gen>.fb + flip the `current` pointer rather
-	// than overwriting graph.fb — this writeback path is executed by the serve
-	// process while the same graph.fb may be mmap'd, exactly the Windows
-	// ERROR_USER_MAPPED_FILE hazard the gen layout removes. fbPath is the gen
-	// file written (used for the identical-mtime stamp below).
-	fbPath, ferr := fbwriter.WriteGraphGen(stateDir, found.doc)
-	if ferr != nil {
+	if err := descriptions.Upsert(stateDir, subjectID, req.Description); err != nil {
 		s.auditor.Err("enrichment_writeback", group, map[string]any{
 			"subject_id": subjectID,
 			"kind":       req.Kind,
-		}, "write graph.fb: "+ferr.Error())
-		writeErr(w, http.StatusInternalServerError, "persist graph.fb: "+ferr.Error())
+		}, "write descriptions side-table: "+err.Error())
+		writeErr(w, http.StatusInternalServerError, "persist description side-table: "+err.Error())
 		return
 	}
-	if err := graph.WriteAtomic(graphPath, found.doc, false); err != nil {
-		s.auditor.Err("enrichment_writeback", group, map[string]any{
-			"subject_id": subjectID,
-			"kind":       req.Kind,
-		}, "write graph.json: "+err.Error())
-		writeErr(w, http.StatusInternalServerError, "persist graph.json: "+err.Error())
-		return
-	}
-	// Stamp identical mtime on both files so the on-disk pair is never
-	// mistaken for a partial write (#1626 pattern).
-	writeNow := time.Now()
-	_ = os.Chtimes(fbPath, writeNow, writeNow)
-	_ = os.Chtimes(graphPath, writeNow, writeNow)
 
-	// Invalidate the cache so the next GET re-reads the updated graph.
+	// Invalidate the cache so the next GET re-loads the group and re-merges the
+	// side-table (applyDescriptionOverlay / the loadGroup merge) onto fresh
+	// entities — the durable read path for the write we just made.
 	s.graphs.Invalidate(group)
 
 	// ─── Target 2: write YAML-frontmatter Markdown doc ────────────────────

internal/dashboard/handlers_enrichment_writeback_test.go

@@ -18,6 +18,7 @@ import (
 
 	"github.com/cajasmota/grafel/internal/daemon"
 	"github.com/cajasmota/grafel/internal/graph"
+	"github.com/cajasmota/grafel/internal/graph/descriptions"
 )
 
 // ─────────────────────────────────────────────────────────────────────────────
@@ -198,7 +199,20 @@ func TestWriteback_success(t *testing.T) {
 		t.Errorf("entity.Properties[description]: want %q, got %q", desc, entity.PropGet("description"))
 	}
 
-	// ── 3. graph.json updated on disk ─────────────────────────────────────
+	// ── 3. description lands in the side-table, graph NOT rewritten ────────
+	// #5904 PR-a: the description is persisted to <stateDir>/descriptions.json,
+	// and the graph file is NEVER rewritten (no collapse / OOM for a segment-set).
+	stateDir := daemon.StateDirForRepo(repoDir)
+	sc, ok := descriptions.Read(stateDir)
+	if !ok {
+		t.Fatalf("descriptions side-table not found / not fresh at %s", descriptions.Path(stateDir))
+	}
+	if got := sc.Results["aabbccddeeff0011"]; got != desc {
+		t.Errorf("side-table description: want %q, got %q", desc, got)
+	}
+
+	// The on-disk graph.json must be UNCHANGED — the write-back must not bake the
+	// description into the graph (that is the whole-graph rewrite this PR removes).
 	graphPath := daemon.GraphPathForRepo(repoDir)
 	raw, err := os.ReadFile(graphPath)
 	if err != nil {
@@ -208,18 +222,11 @@ func TestWriteback_success(t *testing.T) {
 	if err := json.Unmarshal(raw, &savedDoc); err != nil {
 		t.Fatalf("unmarshal graph.json: %v", err)
 	}
-	found := false
 	for _, e := range savedDoc.Entities {
-		if e.ID == "aabbccddeeff0011" {
-			found = true
-			if e.PropGet("description") != desc {
-				t.Errorf("persisted description: want %q, got %q", desc, e.PropGet("description"))
-			}
+		if e.ID == "aabbccddeeff0011" && e.PropGet("description") != "" {
+			t.Errorf("graph.json must NOT be rewritten with the description; got %q", e.PropGet("description"))
 		}
 	}
-	if !found {
-		t.Error("entity not found in persisted graph.json")
-	}
 
 	// ── 4. Markdown doc file created ─────────────────────────────────────
 	docPath := filepath.Join(repoDir, "docs", "enrichments", "http_endpoint", "aabbccddeeff0011.md")

internal/graph/descriptions/atomicrename_other.go

@@ -0,0 +1,16 @@
+//go:build !windows
+
+package descriptions
+
+import "os"
+
+// atomicRename performs the temp→dest swap that finalizes a sidecar write.
+//
+// On Unix, os.Rename over an existing destination is a single atomic syscall
+// even while concurrent readers hold the destination open (open files are
+// referenced by inode, not by directory entry), so one attempt is correct. The
+// retrying variant lives in atomicrename_windows.go, where renaming over a file
+// a reader still has open can transiently fail with a sharing violation.
+func atomicRename(tmp, dst string) error {
+	return os.Rename(tmp, dst)
+}

internal/graph/descriptions/atomicrename_windows.go

@@ -0,0 +1,62 @@
+//go:build windows
+
+package descriptions
+
+import (
+	"errors"
+	"io/fs"
+	"os"
+	"syscall"
+	"time"
+
+	"golang.org/x/sys/windows"
+)
+
+// atomicRenameRetries / atomicRenameRetryDelay bound the Windows rename-over
+// retry loop. On Windows, rename-over-existing fails with
+// ERROR_SHARING_VIOLATION / ERROR_ACCESS_DENIED while ANY process still holds
+// the destination open — including a concurrent reader mid os.ReadFile. The
+// sidecar's real reader opens, reads, and closes the file in a single brief
+// operation, so a short bounded backoff rides out that window. Variables (not
+// consts) so a future test can shrink the delay.
+var (
+	atomicRenameRetries    = 40
+	atomicRenameRetryDelay = 5 * time.Millisecond
+)
+
+// atomicRename performs the temp→dest swap that finalizes a sidecar write,
+// retrying on the transient Windows sharing/access violation raised when a
+// concurrent reader still has the destination open. A persistent lock still
+// surfaces the genuine error after the bounded loop exhausts.
+func atomicRename(tmp, dst string) error {
+	var err error
+	for i := 0; i < atomicRenameRetries; i++ {
+		err = os.Rename(tmp, dst)
+		if err == nil {
+			return nil
+		}
+		if !isSharingOrAccessError(err) {
+			return err
+		}
+		time.Sleep(atomicRenameRetryDelay)
+	}
+	return err
+}
+
+// isSharingOrAccessError reports whether err is the transient Windows lock
+// raised when renaming over a destination another handle still has open.
+func isSharingOrAccessError(err error) bool {
+	if errors.Is(err, fs.ErrPermission) {
+		return true
+	}
+	var errno syscall.Errno
+	if errors.As(err, &errno) {
+		switch errno {
+		case windows.ERROR_ACCESS_DENIED,
+			windows.ERROR_SHARING_VIOLATION,
+			windows.ERROR_LOCK_VIOLATION:
+			return true
+		}
+	}
+	return false
+}