diff --git a/crates/kb-app/src/lib.rs b/crates/kb-app/src/lib.rs index dbad263..29ceef1 100644 --- a/crates/kb-app/src/lib.rs +++ b/crates/kb-app/src/lib.rs @@ -20,14 +20,18 @@ //! facade function in a later phase, remember: keep the return type pure, //! and add a matching `wire_*` helper in `kb-cli/src/wire.rs`. //! -//! ## Test seam +//! ## Config seam (`*_with_config`) //! -//! Each public free function has a `pub(crate) fn *_with_config` -//! companion that takes a fully-resolved `Config` directly. Public -//! callers go through the top-level functions which call -//! `load_config()`; integration tests call the `_with_config` variant -//! and pass a mutated `Config` pointing at a `TempDir` to avoid -//! polluting the user's real `data_dir` / `model_dir`. +//! Each public free function has a `#[doc(hidden)] pub fn *_with_config` +//! companion that takes a fully-resolved [`kb_config::Config`] directly. +//! Three callers go through it: (1) the top-level free functions +//! themselves, after `load_config()`; (2) `kb-cli` when the user passes +//! `--config ` (CLI builds the Config via +//! `Config::load(cli.config.as_deref())` and threads it in directly so +//! the flag is honored); (3) integration tests, which mutate a Config +//! to point at a `TempDir` to avoid polluting the user's real +//! `data_dir` / `model_dir`. `#[doc(hidden)]` keeps rustdoc clean while +//! still allowing the cross-crate calls. use std::path::PathBuf; use std::sync::Arc; @@ -136,6 +140,10 @@ fn expand_tilde(s: &str) -> PathBuf { /// what `kb-cli` does at the top of every subcommand path; we re-do /// the load inside each facade entry so callers don't have to thread /// a Config through. +/// +/// Callers that already have a Config in hand (CLI honoring `--config`, +/// integration tests, TUI session) should bypass this and call the +/// matching `*_with_config` helper directly. fn load_config() -> anyhow::Result { kb_config::Config::load(None) } @@ -147,8 +155,10 @@ pub fn ingest(scope: SourceScope, summary_only: bool) -> anyhow::Result anyhow::Result { anyhow::bail!("not yet wired (P4-3)") } -/// Run the doctor checks. P0 emits `config_loaded` + `data_dir_writable` -/// (downstream checks land in later phases). -pub fn doctor() -> anyhow::Result { +/// Run the doctor checks against the explicit config path the user +/// requested via `--config` (or the XDG default if `None`). The +/// `config_loaded` check reports the actual path probed and the +/// `data_dir_writable` check probes the resolved `storage.data_dir` +/// from that config (so `--config` users see their custom paths +/// reflected in the report rather than the XDG defaults). +pub fn doctor_with_config_path(config_path: Option<&std::path::Path>) -> anyhow::Result { tracing::debug!("doctor() invoked"); let mut checks = Vec::new(); - // config_loaded — defaults always load; from-file is best-effort. - let cfg_path = kb_config::Config::xdg_config_path(); - let (config_ok, config_detail) = if cfg_path.exists() { + // Resolve the config path the same way `Config::load` does: explicit + // override first, else XDG default. Report whichever was probed. + let cfg_path: PathBuf = match config_path { + Some(p) => p.to_path_buf(), + None => kb_config::Config::xdg_config_path(), + }; + let (config_ok, config_detail, loaded_cfg) = if cfg_path.exists() { match kb_config::Config::from_file(&cfg_path) { - Ok(_) => (true, cfg_path.display().to_string()), - Err(e) => (false, format!("{} ({e})", cfg_path.display())), + Ok(c) => (true, cfg_path.display().to_string(), Some(c)), + Err(e) => (false, format!("{} ({e})", cfg_path.display()), None), } + } else if config_path.is_some() { + // Explicit `--config ` that doesn't exist is a hard error + // — defaults would silently mask the user's intent. + ( + false, + format!("{} (not found)", cfg_path.display()), + None, + ) } else { - // Defaults are always loadable; report the path that would be read. - (true, format!("{} (defaults)", cfg_path.display())) + // No `--config` and no XDG file: defaults are always loadable. + (true, format!("{} (defaults)", cfg_path.display()), None) }; checks.push(DoctorCheck { name: "config_loaded".to_string(), @@ -830,13 +856,26 @@ pub fn doctor() -> anyhow::Result { detail: config_detail, hint: if config_ok { None + } else if config_path.is_some() { + Some("--config path does not exist or is malformed".to_string()) } else { Some("run `kb init` to seed config".to_string()) }, }); - // data_dir_writable — try to create the dir and write a probe file. - let data_dir = kb_config::Config::xdg_data_dir(); + // data_dir_writable — probe the resolved storage.data_dir from the + // loaded config when present, else the XDG default. Apply env + // overrides so KB_STORAGE_DATA_DIR is respected too. + let data_dir = match loaded_cfg.as_ref() { + Some(c) => { + // Re-apply env overrides on top so the same precedence as + // Config::load is preserved here. + let env: std::collections::HashMap = std::env::vars().collect(); + let merged = c.clone().apply_env(&env); + expand_tilde(&merged.storage.data_dir) + } + None => kb_config::Config::xdg_data_dir(), + }; let writable = (|| -> anyhow::Result<()> { std::fs::create_dir_all(&data_dir)?; let probe = data_dir.join(".kb-doctor-probe"); @@ -849,7 +888,7 @@ pub fn doctor() -> anyhow::Result { Err(e) => ( false, format!("{} ({e})", data_dir.display()), - Some("ensure XDG_DATA_HOME is writable".to_string()), + Some("ensure the configured data_dir is writable".to_string()), ), }; checks.push(DoctorCheck { @@ -866,3 +905,11 @@ pub fn doctor() -> anyhow::Result { checks, }) } + +/// Run the doctor checks against the XDG-default config. Convenience +/// wrapper that mirrors the historical `kb-app::doctor()` signature +/// for callers that don't honor `--config` (e.g., legacy code paths +/// or smoke harnesses). +pub fn doctor() -> anyhow::Result { + doctor_with_config_path(None) +} diff --git a/crates/kb-cli/src/main.rs b/crates/kb-cli/src/main.rs index b31bae5..c562586 100644 --- a/crates/kb-cli/src/main.rs +++ b/crates/kb-cli/src/main.rs @@ -219,7 +219,7 @@ fn run(cli: &Cli) -> anyhow::Result<()> { include: cfg.workspace.include.clone(), exclude: cfg.workspace.exclude.clone(), }; - let report = kb_app::ingest(scope, *summary_only)?; + let report = kb_app::ingest_with_config(cfg, scope, *summary_only)?; if cli.json { println!("{}", serde_json::to_string(&wire::wire_ingest(&report))?); } else { @@ -238,7 +238,8 @@ fn run(cli: &Cli) -> anyhow::Result<()> { Cmd::List { what } => match what { ListWhat::Docs => { - let docs = kb_app::list_docs(kb_core::DocFilter::default())?; + let cfg = kb_config::Config::load(cli.config.as_deref())?; + let docs = kb_app::list_docs_with_config(cfg, kb_core::DocFilter::default())?; if cli.json { println!("{}", serde_json::to_string(&wire::wire_doc_summaries(&docs))?); } else { @@ -252,8 +253,9 @@ fn run(cli: &Cli) -> anyhow::Result<()> { Cmd::Inspect { what } => match what { InspectWhat::Doc { id } => { + let cfg = kb_config::Config::load(cli.config.as_deref())?; let doc_id: kb_core::DocumentId = id.parse()?; - let doc = kb_app::inspect_doc(&doc_id)?; + let doc = kb_app::inspect_doc_with_config(cfg, &doc_id)?; // Inspect doc emits a `CanonicalDocument` — there's no §2 // wire schema for it (P1-5 will decide whether this also // becomes a tagged wrapper or stays as the raw domain @@ -263,8 +265,9 @@ fn run(cli: &Cli) -> anyhow::Result<()> { Ok(()) } InspectWhat::Chunk { id } => { + let cfg = kb_config::Config::load(cli.config.as_deref())?; let chunk_id: kb_core::ChunkId = id.parse()?; - let chunk = kb_app::inspect_chunk(&chunk_id)?; + let chunk = kb_app::inspect_chunk_with_config(cfg, &chunk_id)?; println!("{}", serde_json::to_string(&wire::wire_chunk_inspection(&chunk))?); Ok(()) } @@ -276,18 +279,34 @@ fn run(cli: &Cli) -> anyhow::Result<()> { mode, explain: _, } => { + let cfg = kb_config::Config::load(cli.config.as_deref())?; let q = kb_core::SearchQuery { text: query.clone(), mode: (*mode).into(), k: *k, filters: kb_core::SearchFilters::default(), }; - let hits = kb_app::search(q)?; + let hits = kb_app::search_with_config(cfg, q)?; if cli.json { println!("{}", serde_json::to_string(&wire::wire_search_hits(&hits))?); } else { for h in &hits { - println!("{:>2}. {:.2} {}", h.rank, h.retrieval.fusion_score, h.doc_path.0); + // Show 4-digit score so RRF fused scores (bounded + // ~0–0.033 for k_rrf=60) don't all collapse to "0.02". + // Append heading_path so multiple chunks from the same + // document are distinguishable on a single line. + let heading = if h.heading_path.is_empty() { + String::new() + } else { + format!(" > {}", h.heading_path.join(" / ")) + }; + println!( + "{:>2}. {:.4} {}{}", + h.rank, + h.retrieval.fusion_score, + h.doc_path.0, + heading, + ); } } Ok(()) @@ -322,7 +341,7 @@ fn run(cli: &Cli) -> anyhow::Result<()> { } Cmd::Doctor => { - let report = kb_app::doctor()?; + let report = kb_app::doctor_with_config_path(cli.config.as_deref())?; if cli.json { println!("{}", serde_json::to_string(&wire::wire_doctor(&report))?); } else {