|
| 1 | +# Design & Implementation Plan: #1558 Offline CLI KG Cache Invalidation |
| 2 | + |
| 3 | +## 1. Summary of Target Behavior |
| 4 | + |
| 5 | +The offline `terraphim-agent` CLI will automatically detect when KG markdown files have changed and rebuild its cached thesaurus on first use per session. No manual `kg rebuild` command required — staleness is detected transparently via `source_hash` comparison. |
| 6 | + |
| 7 | +**User Flow (After Fix):** |
| 8 | +``` |
| 9 | +User writes new concept files to kg/ |
| 10 | +User runs terraphim-agent extract "new concept" |
| 11 | +→ CLI detects hash mismatch → rebuilds thesaurus → returns correct matches |
| 12 | +``` |
| 13 | + |
| 14 | +## 2. Key Invariants and Acceptance Criteria |
| 15 | + |
| 16 | +### Invariants |
| 17 | +- Thesauri are always rebuilt when source hash changes |
| 18 | +- No stale concepts appear in `extract`/`search`/`suggest` results |
| 19 | +- Rebuild is transparent to user (silent) |
| 20 | + |
| 21 | +### Acceptance Criteria |
| 22 | +| ID | Criterion | Test Type | |
| 23 | +|----|----------|-----------| |
| 24 | +| AC1 | New `.md` file added to KG path → `extract` finds it within same session | Integration | |
| 25 | +| AC2 | Modified `.md` file in KG path → `extract` reflects changes within same session | Integration | |
| 26 | +| AC3 | Deleted `.md` file from KG path → `extract` no longer matches it within same session | Integration | |
| 27 | +| AC4 | Hash check adds <50ms latency on first `extract` per session | Benchmark | |
| 28 | +| AC5 | Session restart forces fresh build (no persistent cache) | Unit | |
| 29 | +| AC6 | `cargo test -p terraphim_agent` passes | CI | |
| 30 | +| AC7 | `cargo test -p terraphim_sessions` passes (unchanged) | CI | |
| 31 | + |
| 32 | +## 3. High-Level Design and Boundaries |
| 33 | + |
| 34 | +### Problem Analysis |
| 35 | +The offline CLI uses `OnceLock` which initializes once and never updates. The server uses `RwLock<Roles>` with hash-checking logic (`terraphim_service/src/lib.rs:524-577`). |
| 36 | + |
| 37 | +### Solution: Replace `OnceLock` with Hash-Checked Lazy Init |
| 38 | + |
| 39 | +**Current (broken):** |
| 40 | +```rust |
| 41 | +static VALIDATION_KG_THESAURUS: OnceLock<Option<Thesaurus>> = OnceLock::new(); |
| 42 | +// get_or_init() → returns cached value forever |
| 43 | +``` |
| 44 | + |
| 45 | +**New (fixed):** |
| 46 | +```rust |
| 47 | +struct CachedThesaurus { |
| 48 | + thesaurus: Thesaurus, |
| 49 | + source_hash: String, |
| 50 | +} |
| 51 | + |
| 52 | +static KG_CACHE: OnceLock<CachedThesaurus> = OnceLock::new(); |
| 53 | + |
| 54 | +fn get_thesaurus() -> &'static Thesaurus { |
| 55 | + let cached = KG_CACHE.get_or_init(|| { |
| 56 | + let (thesaurus, hash) = build_and_hash_kg(); |
| 57 | + CachedThesaurus { thesaurus, source_hash: hash } |
| 58 | + }); |
| 59 | + |
| 60 | + // Check if stale on EVERY call (per-session freshness) |
| 61 | + if let Ok(Some(new_hash)) = compute_kg_source_hash(&kg_path) { |
| 62 | + if new_hash != cached.source_hash { |
| 63 | + let (thesaurus, hash) = build_and_hash_kg(); |
| 64 | + return CachedThesaurus { thesaurus, source_hash: hash }; |
| 65 | + } |
| 66 | + } |
| 67 | + &cached.thesaurus |
| 68 | +} |
| 69 | +``` |
| 70 | + |
| 71 | +**Better approach — per-session check, not per-call:** |
| 72 | +Cache stores `(thesaurus, source_hash, session_id)` where session_id tracks if we're in same session. On first access per session, check hash. This avoids per-call overhead. |
| 73 | + |
| 74 | +Actually, simplest approach: **compute hash on first access, store it with thesaurus in OnceLock, compare on subsequent accesses within same process**. |
| 75 | + |
| 76 | +### Components |
| 77 | +| Component | Responsibility | Change | |
| 78 | +|-----------|---------------|--------| |
| 79 | +| `kg_validation.rs` | Global thesaurus cache | Replace `OnceLock<Option<Thesaurus>>` with `OnceLock<CachedThesaurus>` + hash check | |
| 80 | +| `learnings/capture.rs` | Build thesaurus from KG dir | Add `build_kg_thesaurus_with_hash()` returning (Thesaurus, hash) | |
| 81 | +| `terraphim_automata::builder` | Hash computation | Already exists as `compute_kg_source_hash()` | |
| 82 | + |
| 83 | +### Boundaries |
| 84 | +- **Inside scope**: `terraphim_agent` crate only |
| 85 | +- **Outside scope**: Server (`terraphim_service`), persistence layer, CLI commands |
| 86 | + |
| 87 | +## 4. File/Module-Level Change Plan |
| 88 | + |
| 89 | +| File/Module | Action | Before | After | |
| 90 | +|-------------|--------|--------|-------| |
| 91 | +| `crates/terraphim_agent/src/kg_validation.rs` | Modify | `OnceLock<Option<Thesaurus>>` | `OnceLock<CachedThesaurus>` with hash check | |
| 92 | +| `crates/terraphim_agent/src/learnings/capture.rs` | Modify | `build_kg_thesaurus_from_dir()` | Add `build_kg_thesaurus_with_hash()` returning (Thesaurus, hash) | |
| 93 | + |
| 94 | +### Detail |
| 95 | + |
| 96 | +**`kg_validation.rs` changes:** |
| 97 | +1. Add `CachedThesaurus` struct: `{ thesaurus: Thesaurus, source_hash: String, kg_path: PathBuf }` |
| 98 | +2. Change `VALIDATION_KG_THESAURUS: OnceLock<Option<Thesaurus>>` → `OnceLock<CachedThesaurus>` |
| 99 | +3. Add `get_thesaurus_with_auto_rebuild()` function that: |
| 100 | + - Gets cached value or initializes |
| 101 | + - Computes current `compute_kg_source_hash()` |
| 102 | + - If mismatch → rebuilds and updates cache |
| 103 | + - Returns `&'static Thesaurus` |
| 104 | +4. Update `validate_command_against_kg()` to call `get_thesaurus_with_auto_rebuild()` |
| 105 | + |
| 106 | +**`learnings/capture.rs` changes:** |
| 107 | +1. Add `build_kg_thesaurus_with_hash(kg_dir: &Path) -> Option<(Thesaurus, String)>`: |
| 108 | + - Builds thesaurus |
| 109 | + - Computes hash via `compute_kg_source_hash()` |
| 110 | + - Returns both |
| 111 | + |
| 112 | +**`main.rs` changes (if needed):** |
| 113 | +- May need to expose `kg_path` via `find_kg_dir()` for use in `kg_validation.rs` |
| 114 | + |
| 115 | +## 5. Step-by-Step Implementation Sequence |
| 116 | + |
| 117 | +### Step 1: Add `build_kg_thesaurus_with_hash()` helper |
| 118 | +**Purpose**: Return both thesaurus and hash for caching |
| 119 | +**File**: `learnings/capture.rs` |
| 120 | +**Deployable**: Yes (additive, no behavior change yet) |
| 121 | + |
| 122 | +### Step 2: Define `CachedThesaurus` struct |
| 123 | +**Purpose**: Store thesaurus + hash + kg_path together |
| 124 | +**File**: `kg_validation.rs` |
| 125 | +**Deployable**: Yes (new struct, no behavior change) |
| 126 | + |
| 127 | +### Step 3: Implement `get_thesaurus_with_auto_rebuild()` |
| 128 | +**Purpose**: Core logic — check hash, rebuild if stale |
| 129 | +**File**: `kg_validation.rs` |
| 130 | +**Deployable**: Yes (replaces `OnceLock` initialization) |
| 131 | + |
| 132 | +### Step 4: Update `validate_command_against_kg()` to use new getter |
| 133 | +**Purpose**: Wire up the auto-rebuild logic |
| 134 | +**File**: `kg_validation.rs` |
| 135 | +**Deployable**: Yes (transparent behavior change) |
| 136 | + |
| 137 | +### Step 5: Write integration tests |
| 138 | +**Purpose**: Verify AC1-AC3 |
| 139 | +**File**: New test in `kg_validation.rs` or integration test suite |
| 140 | +**Deployable**: Yes (tests only) |
| 141 | + |
| 142 | +### Step 6: Run full test suite |
| 143 | +**Purpose**: Verify no regressions |
| 144 | +**Deployable**: Yes (CI gate) |
| 145 | + |
| 146 | +## 6. Testing & Verification Strategy |
| 147 | + |
| 148 | +| Acceptance Criteria | Test Type | Test Location | |
| 149 | +|---------------------|-----------|--------------| |
| 150 | +| AC1: New file detected | Integration | Write KG file → call `validate_command_against_kg()` → assert match | |
| 151 | +| AC2: Modified file detected | Integration | Modify KG file → rebuild → assert new content | |
| 152 | +| AC3: Deleted file detected | Integration | Delete KG file → rebuild → assert no match | |
| 153 | +| AC4: Latency < 50ms | Benchmark | Time `get_thesaurus_with_auto_rebuild()` on warm cache | |
| 154 | +| AC5: Fresh on restart | Unit | `OnceLock` naturally reinitializes on new process | |
| 155 | +| AC6: Tests pass | CI | `cargo test -p terraphim_agent` | |
| 156 | +| AC7: No regressions | CI | `cargo test -p terraphim_sessions` | |
| 157 | + |
| 158 | +### Integration Test Pattern |
| 159 | +```rust |
| 160 | +#[test] |
| 161 | +fn test_new_kg_file_detected() { |
| 162 | + let temp_dir = TempDir::new().unwrap(); |
| 163 | + let kg_dir = temp_dir.path(); |
| 164 | + |
| 165 | + // Build initial thesaurus |
| 166 | + let file1 = kg_dir.join("concept1.md"); |
| 167 | + fs::write(&file1, "synonyms:: test1\n").unwrap(); |
| 168 | + |
| 169 | + let (thesaurus, hash) = build_kg_thesaurus_with_hash(kg_dir).unwrap(); |
| 170 | + assert!(thesaurus.get(&"test1".into()).is_some()); |
| 171 | + |
| 172 | + // Add new file |
| 173 | + let file2 = kg_dir.join("concept2.md"); |
| 174 | + fs::write(&file2, "synonyms:: test2\n").unwrap(); |
| 175 | + |
| 176 | + // Rebuild with same path |
| 177 | + let new_thesaurus = rebuild_if_stale(kg_dir, &hash); |
| 178 | + assert!(new_thesaurus.get(&"test2".into()).is_some()); |
| 179 | +} |
| 180 | +``` |
| 181 | + |
| 182 | +## 7. Risk & Complexity Review |
| 183 | + |
| 184 | +| Risk | Mitigation | Residual Risk | |
| 185 | +|------|------------|---------------| |
| 186 | +| `OnceLock` race on concurrent access | `OnceLock` guarantees single init; hash check after init is read-mostly | Low | |
| 187 | +| Hash computation slow on large KG | `compute_kg_source_hash` is O(n) dir scan; acceptable for CLI | Low | |
| 188 | +| Breaking `kg_validation` API | `validate_command_against_kg()` signature unchanged | None | |
| 189 | +| `find_kg_dir()` returns different path than server | Both use same `docs/src/kg` convention | Low | |
| 190 | + |
| 191 | +### Complexity Assessment |
| 192 | +- **Lines changed**: ~100-150 |
| 193 | +- **New functions**: 2 (`build_kg_thesaurus_with_hash`, `get_thesaurus_with_auto_rebuild`) |
| 194 | +- **Test additions**: 3-5 integration tests |
| 195 | +- **Breaking changes**: None |
| 196 | + |
| 197 | +## 8. Open Questions / Decisions for Human Review |
| 198 | + |
| 199 | +All questions answered by reviewer: |
| 200 | +1. ✅ Auto-detection chosen (vs explicit rebuild) |
| 201 | +2. ✅ No persistence — compute fresh each session |
| 202 | +3. ✅ Per-session check (first access per process) |
| 203 | +4. ✅ No backward compatibility needed |
| 204 | +5. ✅ Integration tests required |
| 205 | + |
| 206 | +**Remaining decision**: Should `kg_validation.rs` also update the `terraphim_sessions` cache if it exists? (No — separate concern, can be future work) |
0 commit comments