From 7505baee9c9aae834b69064ed4f218e4b171a88a Mon Sep 17 00:00:00 2001 From: Jeff Larson Date: Tue, 7 Jul 2026 22:52:31 -0700 Subject: [PATCH] fix(verdict-cache): bounded per-entry LRU so a return to a recently-judged state HITS (JEF-390) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The verdict cache was a single slot per entry, so a workload whose evidence oscillates between states A and B re-judged on every flip: judging B overwrote A's cached verdict, and flipping back to A missed. Widen the `cached` gate from one slot to a bounded per-entry LRU of (fingerprint -> decisive Verdict) — default 32 slots, env-configurable via PROTECTOR_VERDICT_CACHE_SLOTS (unset/unparseable/0 -> default; positive values floored at 2 so the cache can always retain one prior state). `cached_for` now hits on ANY fingerprint in the entry's LRU and promotes it to most-recently-used; `cache_decisive` inserts most-recently-used and evicts the least-recently-used past the cap. Dependency-light: an ordered Vec with move-to-front + truncate, no crate. Invariants preserved: only DECISIVE verdicts are ever inserted — an Uncertain is never cached and still arms the JEF-234 exponential backoff; an exact-fingerprint hit is byte-identical evidence (the fingerprint is the whole-prompt hash), so a served verdict is exactly as valid as when judged, just more retained — no new staleness. Journal-restore, display carry-forward, journaled dedup, recency, and the lock discipline are untouched. Tests (sibling verdict_store_tests.rs to stay under the 1,000-line cap): A->B->A serves A on the return (2 judgements, not 3); beyond the cap the LRU evicts the least-recently-used state and re-judges it (proving LRU, not FIFO); Uncertain is not cached and the entry backs off; the env knob defaults on invalid/zero and floors small values. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01VtjoJttCvBY4dzCoE4f9vP --- engine/src/engine/state/verdict_store.rs | 129 +++++++++++-- .../src/engine/state/verdict_store_tests.rs | 173 ++++++++++++++++++ 2 files changed, 287 insertions(+), 15 deletions(-) create mode 100644 engine/src/engine/state/verdict_store_tests.rs diff --git a/engine/src/engine/state/verdict_store.rs b/engine/src/engine/state/verdict_store.rs index a8585ff..c5dc9e7 100644 --- a/engine/src/engine/state/verdict_store.rs +++ b/engine/src/engine/state/verdict_store.rs @@ -18,6 +18,77 @@ use crate::engine::reason::backoff::{CircuitBreaker, EntryBackoff}; use super::recency::{Delta, RecencyInfo, StoredPosture}; +/// Default number of decisive-verdict slots retained per entry (JEF-390). Wide enough that a +/// workload whose evidence oscillates between a handful of recently-judged states serves every +/// return to a recent state from cache instead of re-judging the (slow, CPU-bound) model. +pub const DEFAULT_VERDICT_CACHE_SLOTS: usize = 32; + +/// The smallest per-entry cache the env may configure (JEF-390). A single slot is exactly the +/// pre-JEF-390 behaviour that thrashes on an A→B→A flip, so even a hostile/typo'd env can't +/// shrink the cache below the two slots it takes to retain the previous state across one flip. +const MIN_VERDICT_CACHE_SLOTS: usize = 2; + +/// The per-entry cache capacity from `PROTECTOR_VERDICT_CACHE_SLOTS` (default +/// [`DEFAULT_VERDICT_CACHE_SLOTS`]). Unset / unparseable / `0` → the default; a positive value +/// is honoured but floored at [`MIN_VERDICT_CACHE_SLOTS`] so the LRU can always retain at least +/// one prior state. +pub fn verdict_cache_slots() -> usize { + parse_verdict_cache_slots( + std::env::var("PROTECTOR_VERDICT_CACHE_SLOTS") + .ok() + .as_deref(), + ) +} + +/// Pure parse of the `PROTECTOR_VERDICT_CACHE_SLOTS` value, split out so it's testable without +/// process-global env: unset / unparseable / `0` → [`DEFAULT_VERDICT_CACHE_SLOTS`]; any positive +/// value is honoured, floored at [`MIN_VERDICT_CACHE_SLOTS`]. +fn parse_verdict_cache_slots(raw: Option<&str>) -> usize { + raw.and_then(|s| s.trim().parse::().ok()) + .filter(|&n| n > 0) + .map(|n| n.max(MIN_VERDICT_CACHE_SLOTS)) + .unwrap_or(DEFAULT_VERDICT_CACHE_SLOTS) +} + +/// A bounded per-entry LRU of `(evidence fingerprint → decisive Verdict)` — the JEF-390 widening +/// of the old single verdict slot. A workload whose evidence oscillates between recently-judged +/// states (A→B→A) returns to a state that is still cached and HITS, so it re-judges only on a +/// genuinely new fingerprint rather than on every flip. Ordered most-recently-used FIRST; at +/// N≈32 an ordered `Vec` with move-to-front + truncate is the whole data structure (no crate). +/// +/// Only DECISIVE verdicts are ever inserted (JEF-234): an `Uncertain` is a transient model +/// outage gated by [`EntryBackoff`], never pinned here. An exact-fingerprint hit is byte-identical +/// evidence (the fingerprint is the whole-prompt hash), so a served verdict is exactly as valid as +/// when it was judged — wider retention introduces no new staleness. +#[derive(Debug, Clone, Default)] +pub struct VerdictLru { + /// `(fingerprint, verdict)` pairs, most-recently-used first. Bounded by the store's cap on + /// every insert; a fingerprint is unique within the vec (a re-insert refreshes in place). + slots: Vec<(String, Verdict)>, +} + +impl VerdictLru { + /// Serve the verdict whose fingerprint matches ANY slot, moving that slot to most-recently-used + /// so a repeatedly-revisited state survives eviction. `None` (a miss) means re-judge. + fn get(&mut self, fingerprint: &str) -> Option { + let pos = self.slots.iter().position(|(fp, _)| fp == fingerprint)?; + // Move-to-front: the just-served state is now the most-recently-used. + let hit = self.slots.remove(pos); + let verdict = hit.1.clone(); + self.slots.insert(0, hit); + Some(verdict) + } + + /// Insert a fresh decisive verdict as most-recently-used, evicting the least-recently-used + /// slot(s) once the vec exceeds `cap`. A repeat fingerprint is de-duplicated (refreshed in + /// place) rather than stored twice. + fn insert(&mut self, fingerprint: String, verdict: Verdict, cap: usize) { + self.slots.retain(|(fp, _)| fp != &fingerprint); + self.slots.insert(0, (fingerprint, verdict)); + self.slots.truncate(cap.max(1)); + } +} + /// The behavioral-bake snapshot (JEF-48): what the behavioral port saw in the most /// recent pass. The same per-pass figures feed the OTLP counters (JEF-100) — this is the /// in-process mirror. Purely observational: it carries no per-pod payload, only counts and @@ -165,10 +236,12 @@ pub struct VerdictEntry { /// — held until a live verdict supersedes it (formerly `restored_verdicts`). Cleared /// once `display` lands a live verdict for the entry. pub restored: Option, - /// The cached DECISIVE verdict and the evidence fingerprint it was judged against — - /// the re-judge gate (formerly `verdict_cache`). Present only for a decisive verdict; - /// an unchanged fingerprint serves this without calling the (slow CPU) model again. - pub cached: Option<(String, Verdict)>, + /// The bounded per-entry LRU of DECISIVE verdicts keyed by evidence fingerprint — + /// the re-judge gate (formerly `verdict_cache`, a single slot). JEF-390 widened it from + /// one slot to N so a workload whose evidence oscillates between recently-judged states + /// (A→B→A) HITS on the return instead of re-judging every flip. Only decisive verdicts + /// are ever inserted; a matching fingerprint serves without calling the (slow CPU) model. + pub cached: VerdictLru, /// The last verdict summary journaled + notified for this entry — the dedup key /// (formerly `journaled_verdicts`), so a steady-state cluster writes/notifies once /// per change, not per pass. @@ -237,7 +310,6 @@ impl VerdictEntry { /// [`super::Findings::snapshot`]) and the per-pass display derive each finding's verdict by /// looking its entry up here at read time, so a verdict is visible the moment it is written — /// there is no end-of-pass re-publish lag. Keyed by the entry's node key. -#[derive(Default)] pub struct VerdictStore { entries: Mutex>, /// The GLOBAL inconclusive-adjudication circuit-breaker (JEF-234): when the model @@ -246,6 +318,16 @@ pub struct VerdictStore { /// total calls-per-window is bounded regardless of entry count. A decisive success /// closes it. Separate lock from `entries` — it is touched once per call, not per entry. breaker: Mutex, + /// The per-entry LRU capacity (JEF-390), resolved once at construction from + /// `PROTECTOR_VERDICT_CACHE_SLOTS` so every entry's cache is bounded consistently and the + /// process-global env is read exactly once, not per insert. + cache_slots: usize, +} + +impl Default for VerdictStore { + fn default() -> Self { + Self::new() + } } /// A stable per-entry seed for the backoff jitter (JEF-234), derived from the entry key @@ -261,7 +343,18 @@ fn jitter_seed(entry: &str) -> u64 { impl VerdictStore { pub fn new() -> Self { - Self::default() + Self::with_cache_slots(verdict_cache_slots()) + } + + /// Construct with an explicit per-entry LRU capacity (JEF-390), bypassing the + /// process-global env — so tests exercise eviction deterministically without racing on + /// `PROTECTOR_VERDICT_CACHE_SLOTS` under parallel `nextest`. + fn with_cache_slots(cache_slots: usize) -> Self { + Self { + entries: Mutex::default(), + breaker: Mutex::default(), + cache_slots: cache_slots.max(MIN_VERDICT_CACHE_SLOTS), + } } /// The display summary for an entry, if any — what a finding from that entry shows @@ -344,22 +437,24 @@ impl VerdictStore { .map(|e| e.recency_info(now)) } - /// The cached decisive verdict for an entry whose fingerprint matches — the re-judge - /// gate. `Some(verdict)` serves the cache (no model call); `None` means re-judge. + /// The cached decisive verdict for an entry whose fingerprint matches ANY slot in its + /// per-entry LRU — the re-judge gate. `Some(verdict)` serves the cache (no model call) and + /// promotes that state to most-recently-used, so a repeatedly-revisited state survives + /// eviction (JEF-390); `None` means re-judge. Takes `&mut` through the lock because a HIT + /// reorders the LRU. pub fn cached_for(&self, entry: &str, fingerprint: &str) -> Option { self.entries .lock() .expect("verdict store mutex poisoned") - .get(entry) - .and_then(|e| match &e.cached { - Some((fp, v)) if fp == fingerprint => Some(v.clone()), - _ => None, - }) + .get_mut(entry) + .and_then(|e| e.cached.get(fingerprint)) } - /// Cache a fresh DECISIVE verdict + its fingerprint for the re-judge gate. + /// Cache a fresh DECISIVE verdict + its fingerprint in the entry's per-entry LRU (JEF-390), + /// evicting the least-recently-used state once the cache exceeds its capacity. pub fn cache_decisive(&self, entry: &str, fingerprint: String, verdict: Verdict) { - self.update(entry, |e| e.cached = Some((fingerprint, verdict))); + let cap = self.cache_slots; + self.update(entry, |e| e.cached.insert(fingerprint, verdict, cap)); } /// JEF-234 — whether the judging loop should SKIP the model call for `entry` this pass @@ -454,3 +549,7 @@ impl VerdictStore { .retain(|entry, _| present.contains(entry)); } } + +#[cfg(test)] +#[path = "verdict_store_tests.rs"] +mod tests; diff --git a/engine/src/engine/state/verdict_store_tests.rs b/engine/src/engine/state/verdict_store_tests.rs new file mode 100644 index 0000000..bd63bdb --- /dev/null +++ b/engine/src/engine/state/verdict_store_tests.rs @@ -0,0 +1,173 @@ +//! Tests for the bounded per-entry LRU verdict cache (JEF-390) and its env knob. Extracted to a +//! sibling file to keep `verdict_store.rs` well under the 1,000-line cap (CLAUDE.md). +//! +//! The LRU widens the old single verdict slot so a workload whose evidence oscillates between +//! recently-judged states (A→B→A) HITS on the return instead of re-judging every flip, WITHOUT +//! regressing the JEF-234 invariant that only decisive verdicts are cached and an `Uncertain` +//! still arms exponential backoff. + +use std::time::Instant; + +use super::*; + +/// A DECISIVE verdict tagged so distinct states are distinguishable in assertions. +fn decisive(tag: &str) -> Verdict { + Verdict::Refuted(tag.to_string()) +} + +/// Model ONE judging pass for `entry` at fingerprint `fp`, mirroring the engine loop +/// (`engine/src/engine/mod.rs`): serve the cache on a hit; on a miss, honour JEF-234 backoff, +/// then "judge" — caching only a DECISIVE verdict and arming backoff on an `Uncertain`. Returns +/// `true` iff a fresh (would-be model) call actually happened, so tests can count re-judges. +fn judge_pass( + store: &VerdictStore, + entry: &str, + fp: &str, + would_return: &Verdict, + now: Instant, +) -> bool { + if store.cached_for(entry, fp).is_some() { + return false; // cache hit — no model call + } + if store.entry_backing_off(entry, now) { + return false; // JEF-234: in backoff after a recent inconclusive — skip the model + } + match would_return { + Verdict::Uncertain(_) => store.record_inconclusive(entry, now), + d => { + store.cache_decisive(entry, fp.to_string(), d.clone()); + store.record_decisive(entry); + } + } + true +} + +#[test] +fn a_b_a_returns_a_from_cache_with_two_judgements_not_three() { + let store = VerdictStore::with_cache_slots(32); + let now = Instant::now(); + let (va, vb) = (decisive("A"), decisive("B")); + + let mut judgements = 0; + if judge_pass(&store, "entry", "fpA", &va, now) { + judgements += 1; + } + if judge_pass(&store, "entry", "fpB", &vb, now) { + judgements += 1; + } + // The return to state A must be served from the LRU — under the old single slot, judging B + // had overwritten A and this would re-judge (a third call). + let re_judged = judge_pass(&store, "entry", "fpA", &va, now); + + assert!( + !re_judged, + "return to a recently-judged state A must HIT the cache" + ); + assert_eq!( + judgements, 2, + "only A and B were judged; the return to A is a hit" + ); + assert_eq!( + store.cached_for("entry", "fpA"), + Some(va), + "the served verdict is exactly A's decisive verdict" + ); +} + +#[test] +fn beyond_the_cap_lru_evicts_least_recently_used_and_re_judges_it() { + // Two slots: the smallest cache that still retains one prior state across a flip. + let store = VerdictStore::with_cache_slots(2); + let now = Instant::now(); + let (va, vb, vc) = (decisive("A"), decisive("B"), decisive("C")); + + assert!(judge_pass(&store, "entry", "fpA", &va, now)); // [A] + assert!(judge_pass(&store, "entry", "fpB", &vb, now)); // [B, A] + + // Revisit A: a HIT that promotes A to most-recently-used, so B is now the LRU victim. + assert!(!judge_pass(&store, "entry", "fpA", &va, now)); // [A, B] + + // A third state evicts the LRU (B), NOT the just-used A — proving it's an LRU, not a FIFO. + assert!(judge_pass(&store, "entry", "fpC", &vc, now)); // [C, A] + + assert!( + store.cached_for("entry", "fpA").is_some(), + "the recently-used A survives eviction" + ); + assert!( + store.cached_for("entry", "fpB").is_none(), + "the least-recently-used B was evicted past the cap" + ); + // Returning to the evicted state is a genuine miss → a fresh re-judge. + assert!( + judge_pass(&store, "entry", "fpB", &vb, now), + "the evicted state re-judges on its return" + ); +} + +#[test] +fn uncertain_is_never_cached_and_the_entry_backs_off() { + let store = VerdictStore::with_cache_slots(32); + let now = Instant::now(); + let unc = Verdict::Uncertain("model unavailable".to_string()); + + // A first pass makes a fresh call that comes back inconclusive. + assert!(judge_pass(&store, "entry", "fpU", &unc, now)); + + // JEF-234: the Uncertain is NOT added to the LRU... + assert!( + store.cached_for("entry", "fpU").is_none(), + "an Uncertain verdict is never cached" + ); + // ...and the entry is now in exponential backoff, so the next pass skips the (struggling) + // model rather than re-judging the same failing fingerprint immediately. + assert!( + store.entry_backing_off("entry", now), + "the inconclusive armed the backoff" + ); + assert!( + !judge_pass(&store, "entry", "fpU", &unc, now), + "the backoff gate skips the model on the very next pass" + ); + + // A later decisive verdict resets the backoff and DOES populate the LRU. + let later = now + std::time::Duration::from_secs(3600); + let v = decisive("clear"); + assert!(judge_pass(&store, "entry", "fpU", &v, later)); + assert_eq!(store.cached_for("entry", "fpU"), Some(v)); + assert!( + !store.entry_backing_off("entry", later), + "a decisive verdict clears the backoff" + ); +} + +#[test] +fn env_slots_parse_defaults_on_invalid_or_zero_and_floors_small_values() { + // Unset / unparseable / zero all fall back to the default. + assert_eq!(parse_verdict_cache_slots(None), DEFAULT_VERDICT_CACHE_SLOTS); + assert_eq!( + parse_verdict_cache_slots(Some("")), + DEFAULT_VERDICT_CACHE_SLOTS + ); + assert_eq!( + parse_verdict_cache_slots(Some("nope")), + DEFAULT_VERDICT_CACHE_SLOTS + ); + assert_eq!( + parse_verdict_cache_slots(Some("0")), + DEFAULT_VERDICT_CACHE_SLOTS + ); + assert_eq!( + parse_verdict_cache_slots(Some("-4")), + DEFAULT_VERDICT_CACHE_SLOTS + ); + + // A positive value is honoured (and trimmed)... + assert_eq!(parse_verdict_cache_slots(Some("8")), 8); + assert_eq!(parse_verdict_cache_slots(Some(" 16 ")), 16); + // ...but a too-small value is floored so the cache can always retain one prior state. + assert_eq!( + parse_verdict_cache_slots(Some("1")), + MIN_VERDICT_CACHE_SLOTS + ); +}