From d36667589f39a02300b149f8f444dfc60ca20ec5 Mon Sep 17 00:00:00 2001
From: altair823 <dlsrks0734@gmail.com>
Date: Sun, 3 May 2026 05:07:48 +0000
Subject: [PATCH] =?UTF-8?q?review(p9-fb-19):=20=ED=9A=8C=EC=B0=A8=201=20ni?=
 =?UTF-8?q?t=20=EB=B0=98=EC=98=81?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- `App::search` 의 두 cache.lock() 호출이 mutex poison 시 silently
  bypass 하던 것을 `unwrap_or_else(|e| { warn!; e.into_inner() })`
  recovery 로 교체. cache 가 poison 됐어도 다음 호출은 정상이고
  한 번은 warn 로그가 남아 panic 흔적 추적 가능. lookup 후 lock
  drop → retriever 호출 → 재 lock 으로 lock granularity 도 짧게.
- `clear_search_cache` 도 같은 recovery 패턴.
- `SearchCacheKey` doc 에 spec 와 impl 의 naming 차이 (index_version
  vs corpus_revision) 명시 + HOTFIXES entry 추가. spec 의 index_
  version 명칭이 design §9 의 기존 `IndexVersion` newtype (embedding
  -index identity 라벨) 과 충돌해서 corpus_revision 으로 rename.

7 tests/search_lexical 통과. clippy clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 crates/kebab-app/src/app.rs | 54 +++++++++++++++++++++++++------------
 tasks/HOTFIXES.md           | 20 ++++++++++++++
 2 files changed, 57 insertions(+), 17 deletions(-)

diff --git a/crates/kebab-app/src/app.rs b/crates/kebab-app/src/app.rs
index ce6d4c3..22c4084 100644
--- a/crates/kebab-app/src/app.rs
+++ b/crates/kebab-app/src/app.rs
@@ -89,6 +89,14 @@ pub struct App {
 /// Lexical mode has no embedding identity → empty string in that
 /// slot, harmless because the rest of the key still distinguishes
 /// queries.
+///
+/// **Naming note**: spec p9-fb-19 calls the invalidation counter
+/// `index_version`, but the impl renames it to `corpus_revision` to
+/// avoid confusion with the pre-existing `IndexVersion` newtype
+/// (design §9 — embedding-index identity label, a completely
+/// different concept). The `corpus_revision` row in the §9
+/// versioning table documents the new dimension; HOTFIXES entry
+/// tracks the rename.
 #[derive(Clone, Debug, Eq, Hash, PartialEq)]
 pub(crate) struct SearchCacheKey {
     pub query_norm: String,
@@ -161,22 +169,35 @@ impl App {
         // *subsequent* identical queries.
         let key = self.build_cache_key(&query)?;
         // Lock the cache long enough to lookup; clone the hit out so
-        // we can drop the lock before returning.
-        if let Ok(mut guard) = cache.lock() {
-            if let Some(hits) = guard.get(&key) {
-                tracing::debug!(
-                    target: "kebab-app",
-                    cache = "hit",
-                    corpus_revision = key.corpus_revision,
-                    "search served from LRU cache"
-                );
-                return Ok(hits.clone());
-            }
+        // we can drop the lock before returning. Mutex poison
+        // recovery: `into_inner()` of a poison error returns the
+        // (still-valid) underlying guard so we can keep using the
+        // cache after a panic in another thread. Log once so the
+        // poison itself is visible — the cache is still functional
+        // but a panic in a previous search is worth knowing about.
+        let mut guard = cache.lock().unwrap_or_else(|e| {
+            tracing::warn!(
+                target: "kebab-app",
+                "search_cache mutex was poisoned; recovering and continuing — \
+                 a previous search-thread panic preceded this call"
+            );
+            e.into_inner()
+        });
+        if let Some(hits) = guard.get(&key) {
+            tracing::debug!(
+                target: "kebab-app",
+                cache = "hit",
+                corpus_revision = key.corpus_revision,
+                "search served from LRU cache"
+            );
+            return Ok(hits.clone());
         }
+        // Drop the lock before the (potentially slow) retriever call
+        // so other in-flight searches can use the cache concurrently.
+        drop(guard);
         let hits = self.search_uncached(query)?;
-        if let Ok(mut guard) = cache.lock() {
-            guard.put(key, hits.clone());
-        }
+        let mut guard = cache.lock().unwrap_or_else(|e| e.into_inner());
+        guard.put(key, hits.clone());
         Ok(hits)
     }
 
@@ -375,9 +396,8 @@ impl App {
     /// clear` admin command). No-op when the cache is disabled.
     pub fn clear_search_cache(&self) {
         if let Some(cache) = self.search_cache.as_ref() {
-            if let Ok(mut guard) = cache.lock() {
-                guard.clear();
-            }
+            let mut guard = cache.lock().unwrap_or_else(|e| e.into_inner());
+            guard.clear();
         }
     }
 
diff --git a/tasks/HOTFIXES.md b/tasks/HOTFIXES.md
index f50b3d1..d87debd 100644
--- a/tasks/HOTFIXES.md
+++ b/tasks/HOTFIXES.md
@@ -14,6 +14,26 @@ historical contract that was implemented; this file accumulates the
 deltas so phase 5+ readers can find the live behavior without diffing
 git history.
 
+## 2026-05-03 — p9-fb-19 spec `index_version` → impl `corpus_revision` rename
+
+**Spec amended**: `tasks/p9/p9-fb-19-search-cache.md` (frozen — original
+contract uses `index_version` for the monotonic counter that ingest
+bumps and `App::search` snapshots into its cache key).
+
+**Why renamed**: design §9 already has an `index_version` identifier
+(`IndexVersion` newtype, used in the §4.2 `index_id` recipe and on
+`SearchHit`) — a *string label* for embedding-index identity. Reusing
+the name for the monotonic u64 counter would collide silently on every
+grep / type-search.
+
+**Live name**: `corpus_revision` (added as a new row in design §9
+versioning table). `SqliteStore::corpus_revision()` /
+`bump_corpus_revision()` methods + `kv['corpus_revision']` row.
+`SearchCacheKey.corpus_revision` field on `App`.
+
+**Behavior unchanged**: every other detail (monotonic, ingest-commit
+bump, in-key snapshot, no-bump on no-op reingest) matches the spec.
+
 ## 2026-05-02 — Config defaults: LLM = gemma4:e4b + workspace.root tilde expansion
 
 **Discovered**: 사용자가 도그푸딩 환경에 `kebab init` 으로 생성된 `~/.config/kebab/config.toml` 검토하던 중.