diff --git a/src/ai/insight_chat.rs b/src/ai/insight_chat.rs index 1907728..b3126f4 100644 --- a/src/ai/insight_chat.rs +++ b/src/ai/insight_chat.rs @@ -915,18 +915,7 @@ impl InsightChatService { normalized: String, tx: tokio::sync::mpsc::Sender, ) -> Result<()> { - let effective_backend = req - .backend - .as_deref() - .map(|s| s.trim().to_lowercase()) - .filter(|s| !s.is_empty()) - .unwrap_or_else(|| "local".to_string()); - if !matches!(effective_backend.as_str(), "local" | "hybrid") { - bail!( - "unknown backend '{}'; expected 'local' or 'hybrid'", - effective_backend - ); - } + let effective_backend = resolve_bootstrap_backend(req.backend.as_deref())?; let is_hybrid = effective_backend == "hybrid"; let max_iterations = req @@ -976,13 +965,7 @@ impl InsightChatService { let tools = InsightGenerator::build_tool_definitions(gate_opts); // Build initial messages: persona system + user-turn-with-image. - let system_content = req - .system_prompt - .as_deref() - .map(str::trim) - .filter(|s| !s.is_empty()) - .unwrap_or(BOOTSTRAP_DEFAULT_SYSTEM_PROMPT) - .to_string(); + let system_content = resolve_bootstrap_system_prompt(req.system_prompt.as_deref()); let system_msg = ChatMessage::system(system_content); let mut user_msg = ChatMessage::user(user_content); if !is_hybrid && let Some(ref img) = image_base64 { @@ -1297,6 +1280,34 @@ const BOOTSTRAP_DEFAULT_SYSTEM_PROMPT: &str = "You are a helpful AI assistant an Use the available tools to gather context and answer their questions \ in a conversational tone."; +/// Pick the system prompt for bootstrap. Trimmed-non-empty supplied wins; +/// otherwise fall back to [`BOOTSTRAP_DEFAULT_SYSTEM_PROMPT`]. Returns an +/// owned `String` because the bootstrap caller persists it on the new +/// insight row. +fn resolve_bootstrap_system_prompt(supplied: Option<&str>) -> String { + supplied + .map(str::trim) + .filter(|s| !s.is_empty()) + .map(str::to_string) + .unwrap_or_else(|| BOOTSTRAP_DEFAULT_SYSTEM_PROMPT.to_string()) +} + +/// Pick the backend label for bootstrap. Bootstrap has no stored insight +/// to defer to (that's continuation's behaviour), so the default is +/// `"local"`. Returns an error if the supplied label is non-empty but +/// not one of the recognised values — same surface as continuation's +/// validation. +fn resolve_bootstrap_backend(supplied: Option<&str>) -> Result { + let lower = supplied + .map(|s| s.trim().to_lowercase()) + .filter(|s| !s.is_empty()) + .unwrap_or_else(|| "local".to_string()); + if !matches!(lower.as_str(), "local" | "hybrid") { + bail!("unknown backend '{}'; expected 'local' or 'hybrid'", lower); + } + Ok(lower) +} + /// Outcome of one streaming agentic loop pass. Shared between bootstrap /// and continuation. struct AgenticLoopOutcome { @@ -1840,4 +1851,73 @@ mod tests { assert_eq!(msgs.len(), 1); assert_eq!(msgs[0].role, "user"); } + + // ── Bootstrap prompt / backend resolution ───────────────────────── + // Resolution helpers run on every bootstrap turn — they pick the + // persisted system prompt and the chosen backend label. Bugs here + // would silently swap the persona or miscategorise a backend. + + #[test] + fn bootstrap_system_prompt_falls_back_to_default_for_none() { + let out = resolve_bootstrap_system_prompt(None); + assert_eq!(out, BOOTSTRAP_DEFAULT_SYSTEM_PROMPT); + } + + #[test] + fn bootstrap_system_prompt_falls_back_to_default_for_empty_string() { + // Apollo currently sends `''` when no persona is selected. + let out = resolve_bootstrap_system_prompt(Some("")); + assert_eq!(out, BOOTSTRAP_DEFAULT_SYSTEM_PROMPT); + } + + #[test] + fn bootstrap_system_prompt_falls_back_to_default_for_whitespace() { + let out = resolve_bootstrap_system_prompt(Some(" \n\t ")); + assert_eq!(out, BOOTSTRAP_DEFAULT_SYSTEM_PROMPT); + } + + #[test] + fn bootstrap_system_prompt_uses_supplied_when_non_empty() { + let out = resolve_bootstrap_system_prompt(Some("you are a journal")); + assert_eq!(out, "you are a journal"); + } + + #[test] + fn bootstrap_system_prompt_does_not_strip_inner_whitespace() { + // Trim only happens at the edges — interior newlines and spacing + // (which Apollo's persona uses for tool listings) must survive. + let prompt = "line one\nline two\n bullet"; + let out = resolve_bootstrap_system_prompt(Some(prompt)); + assert_eq!(out, prompt); + } + + #[test] + fn bootstrap_backend_defaults_to_local_when_none() { + let out = resolve_bootstrap_backend(None).unwrap(); + assert_eq!(out, "local"); + } + + #[test] + fn bootstrap_backend_defaults_to_local_when_empty() { + let out = resolve_bootstrap_backend(Some("")).unwrap(); + assert_eq!(out, "local"); + } + + #[test] + fn bootstrap_backend_accepts_local_and_hybrid_case_insensitively() { + assert_eq!(resolve_bootstrap_backend(Some("LOCAL")).unwrap(), "local"); + assert_eq!(resolve_bootstrap_backend(Some("Hybrid")).unwrap(), "hybrid"); + assert_eq!( + resolve_bootstrap_backend(Some(" local ")).unwrap(), + "local" + ); + } + + #[test] + fn bootstrap_backend_rejects_unknown_label() { + let err = resolve_bootstrap_backend(Some("openrouter")).unwrap_err(); + let msg = format!("{}", err); + assert!(msg.contains("unknown backend")); + assert!(msg.contains("openrouter")); + } }