Skip to content

fix(memory): silent graph index corruption — graphSlot not authoritative in ingestion and recall paths #497

Description

@novaspectrayan

Summary

Silent data corruption in graph recall — the global graph subsystems (Hebbian, Temporal Chain, Entity Directory, Hypergraph) are indexed using incompatible coordinate systems between the ingestion and recall paths. This causes wrong neighbors to be returned, IndexOutOfBoundsException on graph expansion, and forgotten memory slots to be silently reused by new ingestions.

Strategic rationale: This is a P0 correctness fix that affects every user of spector-memory today. The bug fires within a single partition (not just on rollover) because offsetToRecordIndex is tier-relative. Fixing this unblocks TKG promotion, semantic entity resolution, and context assembly features identified in the Zep Graphiti competitive analysis.

User Story

As a developer using Spector Memory, I want graph-assisted recall (Hebbian spreading activation, temporal chain traversal, entity linking) to return correct neighbors so that my application's memory quality does not silently degrade.

Root Cause — Four Distinct Bugs

# Bug Location Impact
1 Tier-relative index used to query global graph GraphExpansionStage:239,277 (offsetToRecordIndex) Wrong neighbors returned — episodic slot 5 and semantic slot 5 both map to graph index 5
2 Stride math on global neighbor indices GraphExpansionStage:242,332 (findMemoryByApproximateIndex) IndexOutOfBoundsException or corrupted memory lookups
3 size() shrinks on forget → slot reuse CognitiveIngestionTarget:457,682 (index.size() - 1) New memories inherit graph edges of forgotten memories
4 buildGraphSlotMappings ignores persisted graphSlot IndexRecordMemory:416-428 Rebuilt map diverges from persisted map after any forget

Acceptance Criteria

  • graphSlot (MIDX v6 [24:4]) is the sole authoritative graph node identifier everywhere — ingestion, recall, and rebuild paths
  • graphSlotHighWater (AtomicInteger) provides monotonic allocation; persisted in the index header (MIDX v7)
  • On load, high-water mark restored as max(persisted, maxSlotSeen + 1) for legacy store safety
  • remove(id) tombstones the bimap (slotToId[slot] = null, idToSlot.remove(id)) without shifting; existing orderedIds removal preserved
  • Bimap provides O(1) idAt(int slot) and slotOf(String id) lookups
  • CognitiveIngestionTarget uses loc.graphSlot() instead of index.size() - 1
  • GraphExpansionStage uses loc.graphSlot() instead of offsetToRecordIndex(loc)
  • GraphExpansionStage uses index.idAt(idx) instead of findMemoryByApproximateIndex(idx)
  • Dead methods offsetToRecordIndex and findMemoryByApproximateIndex deleted from GraphExpansionStage (bench harness copy left alone)
  • buildGraphSlotMappings reads loc.graphSlot() from locations, not ordinal position
  • MIDX v7 migration via kernel.codec CodecStep — clears and cold-starts all four graph structures with WARN log
  • GlobalGraphIndexTest passes: partition rollover + mixed tiers + forget-in-middle, in both legacy-file and bundle modes
  • Property test: idAt(slotOf(id)) == id for all live IDs before and after forget
  • Regression test: after forget(B) then ingest(D), slotOf(D) != oldSlot(B) and D has zero inherited edges
  • Invariant test: no graph node ID ever ≥ graphSlotHighWater
  • Migration fixture test: load pre-v7 .midx, assert clean upgrade + warning log

Scope

IN scope (this issue — Phase 1 only):

  • IndexRecordMemory.java — v7 bump, graphSlotHighWater, bimap, buildGraphSlotMappings fix
  • CognitiveIngestionTarget.java — replace index.size() - 1
  • GraphExpansionStage.java — replace offsetToRecordIndex/findMemoryByApproximateIndex, delete dead methods
  • kernel.codec migration step for v6 → v7
  • Comprehensive test suite for the above

OUT of scope (tracked separately):

  • TKG promotion (assertFact/retractFact/factsAbout API)
  • Semantic entity resolution (embedding + LLM adjudication)
  • UserContextAssembler / TemporalResolver / MmrReranker
  • temporal-facts.tfacts persistence bug (separate issue, blocks TKG phase)
  • temporal.chain header count=0 bug
  • coactivation.tracker empty bug
  • 53 orphaned semantic.mem slots bug
  • Hypergraph benchmark re-run (post-fix validation, separate task)

Getting Started

  1. Build: mvn clean test -pl memory/spector-memory
  2. Key files:
    • memory/spector-memory/src/main/java/com/spectrayan/spector/memory/index/IndexRecordMemory.java
    • memory/spector-memory/src/main/java/com/spectrayan/spector/memory/pipeline/CognitiveIngestionTarget.java
    • memory/spector-memory/src/main/java/com/spectrayan/spector/memory/pipeline/GraphExpansionStage.java
    • memory/spector-memory/src/main/java/com/spectrayan/spector/memory/kernel/layout/IndexEntryLayout.java
  3. Patterns: follow existing orderedIdsLock (ReentrantLock) locking; repo standards forbid synchronized

RICE-C Score

Factor Score Rationale
Reach 5/5 Every user of graph-assisted recall is affected
Impact 3x Silent data corruption → correct recall is a foundational fix
Confidence 95% Code-verified across 4 distinct bug sites
Effort M ~5 files modified, 1 migration step, comprehensive tests
Cognitive Fit 5/5 Core memory correctness is our #1 differentiator

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions