refactor: relocate CueCache tests, guard RetrievalCache::lookup, fix cache-layer doc drift (#605)

docs/architecture.html

@@ -172,7 +172,7 @@ <h2>The server layer — the registry and the file family</h2>
       <li>Boot registers every context <b>cold</b> (only pinned ones preload). The first access loads transparently; past the cache budget (<code>TAGURU_CACHE_BYTES</code>) least-recently-used contexts are evicted. Pinning is the floor of that policy; <code>TAGURU_CONTEXT_QUOTAS</code> (issue #136) adds the ceiling side — a context past its declared <code>cache_bytes</code> share is evicted <i>before</i> any compliant one under pressure (no reservation while there is slack), and its declared <code>storage_bytes</code> makes every growth entrance — graph batches, passage stores, <code>/import</code> batch by batch — refuse with 507 <code>storage_full</code> once the on-disk family reaches it, while retract/compact/delete stay open as the ways back under. The storage gate reads the same numbers the per-context gauges serve: the live WAL lanes plus the flush-refreshed snapshot, so enforcement and observability cannot disagree. Compaction's own rebuild can shrink the graph in memory even at the ceiling — reaching the ceiling is exactly when its DISK write is most likely to fail, so its response carries <code>image_persisted</code> to say whether the smaller image actually landed, distinct from the call succeeding at all.</li>
       <li>A write only marks its context dirty. Persistence happens on the periodic flusher (<code>TAGURU_FLUSH_SECS</code>), on eviction, and on shutdown.</li>
       <li id="revision">Every directory row carries <b>revision counters</b> <code>{graph, passages, config}</code> (issue #149) — applied graph writes, the passage log watermark, and config/embedding changes respectively — the "has anything changed since I last looked" token a retrieval cache keys on, with a group-level <code>fingerprint</code> hashing the member contexts' counters. The honest contract: within one process every read is live and strictly monotonic; across a clean shutdown the persisted values are exact; across a <i>crash</i> a cold context can serve a lagging value until its first load catches the graph counter up against the WAL replay (the same posture as the cold stats snapshot — and the search paths load before computing, so a cache fill never keys on the stale seed). Compare for equality only; a cache that outlives the process must treat a server restart or a delete-recreate as invalidation.</li>
-      <li>The <b>exact-match retrieval cache</b> (issue #150) is those counters' first in-process consumer: an identical recall/query/passage-search request (cross variants included, and MCP tool calls, which dispatch onto the same routes) against an unchanged corpus answers from the stored response bytes without re-running the search. Invalidation IS the key — each key carries, per resolved target, the pair of revision lanes that surface depends on (recall/query: graph+passages, for section enrichment; passage search: passages+config, for published vectors and the context floor) plus a per-incarnation identity nonce, all read <i>before</i> the search runs — so a bumped lane simply makes old entries unreachable, a delete-recreate or replica lineage switch changes the nonce, and there is no purge hook and no TTL anywhere. Scope is materialized into the resolved target list, so two credentials share an entry exactly when their grants resolve a request identically. Byte-budgeted tick-LRU (<code>TAGURU_RETRIEVAL_CACHE_BYTES</code>, default 32 MiB, <code>0</code> = off); hits replay the served-response metrics so <code>taguru_searches_total</code> and the lane-contribution counters read continuously, while the hit/miss split lives in <code>taguru_retrieval_cache_total</code>.</li>
+      <li>The <b>exact-match retrieval cache</b> (issue #150) is those counters' first in-process consumer: an identical recall/query/passage-search request (cross variants included, and MCP tool calls, which dispatch onto the same routes) against an unchanged corpus answers from the stored response bytes without re-running the search. Invalidation IS the key — each key carries, per resolved target, the pair of revision lanes that surface depends on (recall/query: graph+passages, for section enrichment; passage search: passages+config, for published vectors and the context floor) plus a per-incarnation identity nonce, all read <i>before</i> the search runs — so a bumped lane simply makes old entries unreachable, and a delete-recreate, a replica lineage switch, or a compaction (which drops what the revision counter alone can't express — retracted edges and orphaned aliases stop appearing in query results even though nothing was written) changes the nonce, and there is no purge hook and no TTL anywhere. Scope is materialized into the resolved target list, so two credentials share an entry exactly when their grants resolve a request identically. Byte-budgeted tick-LRU (<code>TAGURU_RETRIEVAL_CACHE_BYTES</code>, default 32 MiB, <code>0</code> = off); hits replay the served-response metrics so <code>taguru_searches_total</code> and the lane-contribution counters read continuously, while the hit/miss split lives in <code>taguru_retrieval_cache_total</code>.</li>
       <li>The <b>semantic cache tier</b> (issue #153, passage search only, off unless <code>TAGURU_SEMANTIC_CACHE_THRESHOLD</code> is set) stores no payloads and invalidates nothing: it holds only <i>equivalence claims</i> — "this query asks what that earlier query asked", proven by query-vs-query embedding cosine over the threshold AND a text guard finding no negation/number/entity mismatch (cosine alone routinely conflates a question with its negation). A claim that holds rewrites the request's exact-cache key to the canonical query's parameters under the request's own current fingerprints and serves those bytes — so freshness rides entirely on the exact tier's revision lanes and identity nonce (a write turns the claim's serve into a <code>stale</code> fall-through, and the fresh fill re-canonicalizes the cluster), and a semantic serve never contaminates the exact tier's "identical is literal" contract. The query embedding shares the search's own cue cache, so the fresh path still pays exactly one provider call. Outcomes land in <code>taguru_semantic_cache_total{outcome="hit"|"stale"|"guarded"|"miss"}</code> — <code>guarded</code> is the tuning signal — with a claim-count gauge beside it.</li>
       <li>Every search response carries its <b>execution plan</b> (issue #151): the contexts actually consulted in effective order (for the cross variants, the resolved target list — groups expanded, grants applied), and — for passage search, per context — whether each lane ran, the reason when one was skipped (embeddings off / nothing embedded yet / model changed / provider refused, the same prose the explain endpoint uses), the effective cosine floor when the vector lane swept, and — when the request carried a source filter (issues #167/#169) — a <code>filter: {eligible_sources, total_sources}</code> block naming how many of the context's sources were eligible before either lane ran. The plan lives <i>inside</i> the result, so the retrieval caches replay it byte-identically with the hits it accounts for — coherent by construction, because every event that could change a plan (a corpus write, a vector publish, a floor change, a different filter) also moves the cache key. The one transient state that recovers without a revision bump — a refused query embedding — is therefore never cached at all: the degraded BM25-only page is served but not filled, so a provider blip is not pinned until the next unrelated write.</li>
       <li>The truth of one context is its whole file family — back it up <b>as a set, always</b> (<a href="getting-started.html#ops">operational basics</a>).</li>

src/metrics/taxonomy.rs

@@ -92,11 +92,15 @@ impl SearchOp {
 
 /// The retrieval surfaces the exact-match cache fronts — the label
 /// vocabulary of `taguru_retrieval_cache_total`, and the cache key's
-/// op discriminant (each op reads a different pair of revision lanes,
-/// so the same request text under two ops must never collide). Cross
-/// variants fold into their base op: the resolved target list already
-/// distinguishes them in the key, and a per-variant label would split
-/// the hit-rate signal without adding meaning.
+/// op discriminant (`op_lanes` groups these into two revision-lane
+/// pairs, `Recall`/`Query` sharing one and `SearchPassages`/
+/// `SearchCommunities` the other — issue #605 corrected this from
+/// claiming all four read distinct pairs — but the discriminant itself
+/// still must separate them: the same request text under two ops must
+/// never collide). Cross variants fold into their base op: the
+/// resolved target list already distinguishes them in the key, and a
+/// per-variant label would split the hit-rate signal without adding
+/// meaning.
 #[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)]
 pub enum RetrievalCacheOp {
     Recall,

src/registry/engine.rs

@@ -4358,6 +4358,95 @@ mod tests {
         assert_eq!(cache.tick, 3, "even a miss advances the clock");
     }
 
+    // Moved from retrieval_cache.rs (issue #605): that file neither
+    // defines nor uses CueCache, `RetrievalCache`'s own separate LRU.
+
+    #[test]
+    fn cue_cache_get_promotes_recency_so_eviction_spares_the_touched_entry() {
+        let mut cache = CueCache::default();
+        for i in 0..CueCache::CAP {
+            cache.insert(format!("cue{i}"), Arc::new(vec![i as f32]));
+        }
+        // Touching cue0 makes it the most recently used entry, even
+        // though it was the first one inserted.
+        assert!(cache.get("cue0").is_some());
+        // The cache is at capacity, so this eviction must reach for the
+        // least recently used entry — cue1, never touched again after
+        // its insert — not the oldest insertion, which is cue0.
+        cache.insert("fresh-cue".to_string(), Arc::new(vec![-1.0]));
+        assert!(
+            cache.get("cue0").is_some(),
+            "a touched entry must survive the next eviction"
+        );
+        assert!(
+            cache.get("cue1").is_none(),
+            "the least recently used entry must be the one evicted"
+        );
+        assert!(cache.get("fresh-cue").is_some());
+    }
+
+    #[test]
+    fn cue_cache_insert_does_not_overwrite_an_existing_key() {
+        let mut cache = CueCache::default();
+        cache.insert("cue".to_string(), Arc::new(vec![1.0]));
+        cache.insert("cue".to_string(), Arc::new(vec![2.0]));
+        assert_eq!(*cache.get("cue").unwrap(), vec![1.0]);
+    }
+
+    /// Issue #563 item 3: re-inserting an already-resident key must
+    /// still count as a recency touch, or a cue that keeps getting
+    /// resolved (its `get` at the top of `cue_vector` misses because
+    /// it raced eviction, then `insert` re-adds it) reads as
+    /// never-touched to the LRU and can be evicted while genuinely hot.
+    #[test]
+    fn cue_cache_reinsert_of_an_existing_key_counts_as_a_recency_touch() {
+        let mut cache = CueCache::default();
+        for i in 0..CueCache::CAP {
+            cache.insert(format!("cue{i}"), Arc::new(vec![i as f32]));
+        }
+        // Re-inserting cue0 (its value already resident) must promote
+        // it exactly like a `get` would, with no read in between.
+        cache.insert("cue0".to_string(), Arc::new(vec![0.0]));
+        cache.insert("fresh-cue".to_string(), Arc::new(vec![-1.0]));
+        assert!(
+            cache.get("cue0").is_some(),
+            "a re-inserted entry must survive the next eviction"
+        );
+        assert!(
+            cache.get("cue1").is_none(),
+            "the least recently touched entry must be the one evicted"
+        );
+    }
+
+    /// Issue #563 item 1's other half: a resident cue's width can
+    /// drift out from under it if a backend swap changes the
+    /// embedding dimension behind an unchanged model name. Every
+    /// resident vector must agree on one width — a stale-width cue
+    /// left in place would score a silent 0.0 against every table
+    /// (`similarity`'s width-mismatch sentinel) forever, since nothing
+    /// else in the cue cache ever re-checks it.
+    #[test]
+    fn cue_cache_insert_at_a_new_width_clears_stale_width_entries() {
+        let mut cache = CueCache::default();
+        cache.insert("old".to_string(), Arc::new(vec![1.0, 0.0, 0.0]));
+        assert!(cache.get("old").is_some());
+
+        // A backend swap answers a different width under the same
+        // model name: the next insert must wipe the old-width entry
+        // rather than let it sit unreachable-but-present.
+        cache.insert("new".to_string(), Arc::new(vec![1.0, 0.0]));
+        assert!(
+            cache.get("old").is_none(),
+            "a width change must clear every entry at the stale width"
+        );
+        assert_eq!(*cache.get("new").unwrap(), vec![1.0, 0.0]);
+
+        // Further same-width inserts are unaffected.
+        cache.insert("newer".to_string(), Arc::new(vec![0.0, 1.0]));
+        assert!(cache.get("new").is_some());
+        assert!(cache.get("newer").is_some());
+    }
+
     /// Every operation stamps the LRU clock onto the entry — and the
     /// stamp is the clock's NEW value, so a fresh boot's first touch
     /// is 1, never the pre-increment 0 that reads as never-touched.

src/registry/retrieval_cache.rs

@@ -175,6 +175,15 @@ impl RetrievalCache {
     }
 
     fn lookup(&mut self, key: &RetrievalKey) -> Option<CachedRetrieval> {
+        // Structurally enforces the struct doc's "0 = disabled: lookup
+        // and insert both no-op" (issue #605) — unreachable today,
+        // since `AppState::retrieval_key` already refuses to mint a
+        // key while disabled, but that guard living in a different
+        // file is caller discipline, not something this method itself
+        // holds.
+        if !self.is_enabled() {
+            return None;
+        }
         self.tick += 1;
         let tick = self.tick;
         let slot = self.entries.get_mut(key)?;
@@ -334,7 +343,6 @@ impl AppState {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::registry::CueCache;
 
     fn key(params: &str) -> RetrievalKey {
         RetrievalKey {
@@ -471,90 +479,23 @@ mod tests {
         assert_eq!(cache.len(), 0);
     }
 
+    /// `lookup` holds its own `is_enabled` guard (issue #605) rather
+    /// than relying on `insert` alone leaving the map empty while
+    /// disabled: force the two out of sync (a real entry, `budget`
+    /// dropped to `0` after the fact — `AppState::retrieval_key`
+    /// prevents this in practice, but `lookup` must not depend on that)
+    /// to prove the guard, not empty-map luck, is why a disabled
+    /// lookup misses.
     #[test]
-    fn cue_cache_get_promotes_recency_so_eviction_spares_the_touched_entry() {
-        let mut cache = CueCache::default();
-        for i in 0..CueCache::CAP {
-            cache.insert(format!("cue{i}"), Arc::new(vec![i as f32]));
-        }
-        // Touching cue0 makes it the most recently used entry, even
-        // though it was the first one inserted.
-        assert!(cache.get("cue0").is_some());
-        // The cache is at capacity, so this eviction must reach for the
-        // least recently used entry — cue1, never touched again after
-        // its insert — not the oldest insertion, which is cue0.
-        cache.insert("fresh-cue".to_string(), Arc::new(vec![-1.0]));
-        assert!(
-            cache.get("cue0").is_some(),
-            "a touched entry must survive the next eviction"
-        );
-        assert!(
-            cache.get("cue1").is_none(),
-            "the least recently used entry must be the one evicted"
-        );
-        assert!(cache.get("fresh-cue").is_some());
-    }
-
-    #[test]
-    fn cue_cache_insert_does_not_overwrite_an_existing_key() {
-        let mut cache = CueCache::default();
-        cache.insert("cue".to_string(), Arc::new(vec![1.0]));
-        cache.insert("cue".to_string(), Arc::new(vec![2.0]));
-        assert_eq!(*cache.get("cue").unwrap(), vec![1.0]);
-    }
-
-    /// Issue #563 item 3: re-inserting an already-resident key must
-    /// still count as a recency touch, or a cue that keeps getting
-    /// resolved (its `get` at the top of `cue_vector` misses because
-    /// it raced eviction, then `insert` re-adds it) reads as
-    /// never-touched to the LRU and can be evicted while genuinely hot.
-    #[test]
-    fn cue_cache_reinsert_of_an_existing_key_counts_as_a_recency_touch() {
-        let mut cache = CueCache::default();
-        for i in 0..CueCache::CAP {
-            cache.insert(format!("cue{i}"), Arc::new(vec![i as f32]));
-        }
-        // Re-inserting cue0 (its value already resident) must promote
-        // it exactly like a `get` would, with no read in between.
-        cache.insert("cue0".to_string(), Arc::new(vec![0.0]));
-        cache.insert("fresh-cue".to_string(), Arc::new(vec![-1.0]));
-        assert!(
-            cache.get("cue0").is_some(),
-            "a re-inserted entry must survive the next eviction"
-        );
-        assert!(
-            cache.get("cue1").is_none(),
-            "the least recently touched entry must be the one evicted"
-        );
-    }
-
-    /// Issue #563 item 1's other half: a resident cue's width can
-    /// drift out from under it if a backend swap changes the
-    /// embedding dimension behind an unchanged model name. Every
-    /// resident vector must agree on one width — a stale-width cue
-    /// left in place would score a silent 0.0 against every table
-    /// (`similarity`'s width-mismatch sentinel) forever, since nothing
-    /// else in the cue cache ever re-checks it.
-    #[test]
-    fn cue_cache_insert_at_a_new_width_clears_stale_width_entries() {
-        let mut cache = CueCache::default();
-        cache.insert("old".to_string(), Arc::new(vec![1.0, 0.0, 0.0]));
-        assert!(cache.get("old").is_some());
-
-        // A backend swap answers a different width under the same
-        // model name: the next insert must wipe the old-width entry
-        // rather than let it sit unreachable-but-present.
-        cache.insert("new".to_string(), Arc::new(vec![1.0, 0.0]));
+    fn lookup_refuses_a_hit_the_instant_the_budget_drops_to_zero() {
+        let mut cache = RetrievalCache::new(DEFAULT_RETRIEVAL_CACHE_BYTES);
+        cache.insert(key("a"), value(10));
         assert!(
-            cache.get("old").is_none(),
-            "a width change must clear every entry at the stale width"
+            cache.lookup(&key("a")).is_some(),
+            "sanity: the entry is live"
         );
-        assert_eq!(*cache.get("new").unwrap(), vec![1.0, 0.0]);
-
-        // Further same-width inserts are unaffected.
-        cache.insert("newer".to_string(), Arc::new(vec![0.0, 1.0]));
-        assert!(cache.get("new").is_some());
-        assert!(cache.get("newer").is_some());
+        cache.budget = 0;
+        assert!(cache.lookup(&key("a")).is_none());
     }
 
     /// The documented default budget and the slot-cost formula, by

src/registry/semantic_cache.rs

@@ -149,7 +149,17 @@ impl SemanticCache {
 
     /// Every slot in the bucket at or above the threshold, similarity
     /// descending, recency-touched (a candidate consulted is a
-    /// candidate in use, whatever the guard later says).
+    /// candidate in use, whatever the guard later says). Unlike
+    /// [`super::retrieval_cache::RetrievalCache`], whose struct doc
+    /// promises tick uniqueness by construction (one tick per
+    /// operation), every slot ONE sweep touches here shares the same
+    /// `tick` — a known blind spot (issue #605, same class as
+    /// [`super::CueCache`]'s own documented one): since a sweep only
+    /// ever touches slots within THIS bucket, `evict_stalest`'s tie
+    /// among same-tick slots resolves by this bucket's `Vec` position
+    /// (CodeRabbit, PR #635), not a further recency distinction — and
+    /// `swap_remove` reshuffles later positions on every eviction, so
+    /// even that isn't a stable order to rely on.
     fn candidates(&mut self, bucket: &SemanticBucket, embedding: &[f32]) -> Vec<Candidate> {
         let Some(threshold) = self.threshold else {
             return Vec::new();