From 6ca089286c14e247b70a50d83b3fa278099add79 Mon Sep 17 00:00:00 2001 From: altair823 Date: Sun, 3 May 2026 06:25:13 +0000 Subject: [PATCH] =?UTF-8?q?review(p9-fb-18):=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::build_retriever(mode) -> Result>` 추출. `ask` 와 `ask_with_session` 모두 사용. 35+ 줄 retriever stack 중복 제거 — 미래 retriever 변경이 한 곳만. - V005 migration `chat_sessions.sql` 의 `citations_json` doc 수정: `Vec` → `Vec` (실제 stored type 과 일치). AnswerCitation 가 marker + Citation 등 포함하므로 deserialize 시 type mismatch 회피. 15 app lib + 9 store chat_sessions + clippy 통과. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/kebab-app/src/app.rs | 65 +++++++++--------------------- migrations/V005__chat_sessions.sql | 7 ++-- 2 files changed, 22 insertions(+), 50 deletions(-) diff --git a/crates/kebab-app/src/app.rs b/crates/kebab-app/src/app.rs index e1cc135..e538efe 100644 --- a/crates/kebab-app/src/app.rs +++ b/crates/kebab-app/src/app.rs @@ -254,7 +254,20 @@ impl App { /// Run a RAG `ask` against the configured retriever + LLM. Reuses /// the memoized embedder / vector / LLM where applicable. pub fn ask(&self, query: &str, opts: AskOpts) -> Result { - let retriever: Arc = match opts.mode { + let retriever = self.build_retriever(opts.mode)?; + let llm = self.llm()?; + let pipeline = + RagPipeline::new(self.config.clone(), retriever, llm, self.sqlite.clone()); + pipeline.ask(query, opts) + } + + /// p9-fb-18: shared retriever-stack builder used by [`Self::ask`] + /// and [`Self::ask_with_session`]. Lexical mode uses the FTS5 + /// retriever directly; vector / hybrid require embeddings (and + /// surface the same "switch to --mode lexical" error from + /// [`Self::require_embeddings`] when disabled). + fn build_retriever(&self, mode: SearchMode) -> Result> { + Ok(match mode { SearchMode::Lexical => Arc::new(LexicalRetriever::with_settings( self.sqlite.clone(), lexical_index_version(&self.config), @@ -292,12 +305,7 @@ impl App { )) as Arc; Arc::new(HybridRetriever::new(&self.config, lex, vec_retr)) } - }; - - let llm = self.llm()?; - let pipeline = - RagPipeline::new(self.config.clone(), retriever, llm, self.sqlite.clone()); - pipeline.ask(query, opts) + }) } /// p9-fb-18: ask under a persistent chat session. Loads the @@ -353,46 +361,9 @@ impl App { }) .collect(); - // Build the retriever stack the same way `ask` does. - let retriever: Arc = match opts.mode { - SearchMode::Lexical => Arc::new(LexicalRetriever::with_settings( - self.sqlite.clone(), - lexical_index_version(&self.config), - self.config.search.snippet_chars, - )), - SearchMode::Vector => { - let (emb, vec_store) = self.require_embeddings()?; - let vec_iv = vector_index_version(emb.as_ref()); - let vec_dyn: Arc = vec_store; - let emb_dyn: Arc = emb; - Arc::new(VectorRetriever::with_settings( - vec_dyn, - emb_dyn, - self.sqlite.clone(), - vec_iv, - self.config.search.snippet_chars, - )) - } - SearchMode::Hybrid => { - let lex = Arc::new(LexicalRetriever::with_settings( - self.sqlite.clone(), - lexical_index_version(&self.config), - self.config.search.snippet_chars, - )) as Arc; - let (emb, vec_store) = self.require_embeddings()?; - let vec_iv = vector_index_version(emb.as_ref()); - let vec_dyn: Arc = vec_store; - let emb_dyn: Arc = emb; - let vec_retr = Arc::new(VectorRetriever::with_settings( - vec_dyn, - emb_dyn, - self.sqlite.clone(), - vec_iv, - self.config.search.snippet_chars, - )) as Arc; - Arc::new(HybridRetriever::new(&self.config, lex, vec_retr)) - } - }; + // p9-fb-18 R1: shared retriever builder removes the prior + // copy of `ask`'s 35-line stack — see [`Self::build_retriever`]. + let retriever = self.build_retriever(opts.mode)?; let llm = self.llm()?; let pipeline = RagPipeline::new(self.config.clone(), retriever, llm, self.sqlite.clone()); diff --git a/migrations/V005__chat_sessions.sql b/migrations/V005__chat_sessions.sql index 080d442..9fc9403 100644 --- a/migrations/V005__chat_sessions.sql +++ b/migrations/V005__chat_sessions.sql @@ -25,9 +25,10 @@ -- max_context_tokens that produced the session so a retroactive -- answer-quality regression can be re-traced. -- --- * `citations_json` carries `Vec` so the answer can be --- redisplayed with the same citation markers a future session --- sees on resume. +-- * `citations_json` carries `Vec` (per p9-fb-18) — +-- each AnswerCitation holds a `Citation` plus `marker`, so the +-- answer can be redisplayed with the same citation markers a +-- future session sees on resume. -- -- * `INTEGER` timestamps (unix epoch seconds) — same convention the -- rest of the schema uses (P1-7 baselines this).