From 581ea13163fc1126caada3362ae78113e08dfd34 Mon Sep 17 00:00:00 2001 From: Logan Nguyen Date: Sun, 26 Apr 2026 13:42:47 -0500 Subject: [PATCH 1/4] refactor(inference): make Ollama the single source of truth for active model Eliminates the DEFAULT_MODEL_NAME fallback that silently downgraded existing users to gemma4:e2b on first launch of the model picker. Active model is now Option: None when no model is installed and no choice is persisted, otherwise the user's persisted slug or the first installed model. Renames [model] config section to [inference] (only field is ollama_url, so the section now describes the inference daemon, not the model). Frontend: capability mismatch strip extended for the no-model state, picker chip stays visible as the recovery affordance, send is gated through the existing shake animation, save short-circuits when active model is null. No backward compatibility for the section rename: pre-#103 was a placeholder default and PR #103 has not shipped to users yet. Signed-off-by: Logan Nguyen --- CHANGELOG.md | 7 + docs/configurations.md | 6 +- src-tauri/src/commands.rs | 54 +++++- src-tauri/src/config/defaults.rs | 4 - src-tauri/src/config/loader.rs | 6 +- src-tauri/src/config/mod.rs | 2 +- src-tauri/src/config/schema.rs | 8 +- src-tauri/src/config/tests.rs | 46 +++-- src-tauri/src/history.rs | 13 +- src-tauri/src/lib.rs | 16 +- src-tauri/src/models.rs | 173 +++++++++--------- src-tauri/src/search/mod.rs | 12 +- src/App.tsx | 14 +- src/__tests__/App.test.tsx | 36 ++++ src/components/ErrorCard.tsx | 9 +- src/components/WindowControls.tsx | 23 ++- .../__tests__/WindowControls.test.tsx | 35 +++- src/contexts/ConfigContext.tsx | 23 +-- src/contexts/__tests__/ConfigContext.test.tsx | 58 ++---- .../__tests__/useConversationHistory.test.tsx | 15 ++ .../__tests__/useModelSelection.test.tsx | 32 +++- src/hooks/useConversationHistory.ts | 9 +- src/hooks/useModelSelection.ts | 27 ++- src/hooks/useOllama.ts | 14 +- src/testUtils/mocks/tauri.ts | 39 +++- src/types/model.ts | 9 +- .../__tests__/capabilityConflicts.test.ts | 36 +++- src/utils/capabilityConflicts.ts | 19 +- src/view/AskBarView.tsx | 4 +- src/view/ConversationView.tsx | 5 +- src/view/onboarding/ModelCheckStep.tsx | 2 +- .../__tests__/ModelCheckStep.test.tsx | 7 +- 32 files changed, 501 insertions(+), 262 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fdf791d5..e69765a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased + +### Changed + +- **BREAKING**: Renamed the `[model]` section in `config.toml` to `[inference]`. The section still contains a single field, `ollama_url`, but the name now reflects what it actually configures (the inference daemon endpoint, not a model). There is no backward-compatibility shim: if you had a custom `[model]` section, rename it to `[inference]` after upgrading. +- Active model selection is now strictly Option-typed end to end. Ollama's `/api/tags` is the single source of truth: when nothing is installed and nothing is persisted, Thuki refuses to dispatch requests and surfaces a "Pick a model" prompt instead of falling back to a hardcoded slug. The previous `DEFAULT_MODEL_NAME` constant has been removed. + ## [0.6.1](https://github.com/quiet-node/thuki/compare/v0.6.0...v0.6.1) (2026-04-14) diff --git a/docs/configurations.md b/docs/configurations.md index 21f177a2..4d5993ff 100644 --- a/docs/configurations.md +++ b/docs/configurations.md @@ -26,7 +26,7 @@ open ~/Library/Application\ Support/com.quietnode.thuki/config.toml ### Example ```toml -[model] +[inference] # Where Thuki finds your local Ollama server. The active model itself is # selected from the in-app picker (which lists whatever is installed in # Ollama via /api/tags) and is stored in Thuki's local database, not here. @@ -80,10 +80,12 @@ Every domain below is shown as a single table that lists **all** constants Thuki ## Reference -### `[model]` +### `[inference]` Where to find your local Ollama server. The active model itself is **not** a TOML setting: Thuki discovers installed models live from Ollama's `/api/tags` endpoint, lets you pick one from the in-app model picker, and stores that selection in its local SQLite database (`app_config` table). Storing the active slug in TOML would duplicate ground truth from Ollama and break the moment you remove a model with `ollama rm`, so it lives next to the conversation history instead. +When no model is installed and no choice has been persisted, Thuki refuses to dispatch a chat request and surfaces a "Pick a model" prompt in the input area. Pull a model with `ollama pull ` and select it from the picker chip in the top-right of the overlay. + | Constant | Default | Tunable? | Why not tunable | Bounds | Description | | :----------- | :------------------------- | :------- | :-------------- | :------------ | :------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `ollama_url` | `"http://127.0.0.1:11434"` | Yes | — | non-empty URL | The web address where Thuki finds your local Ollama server. The default works if you run Ollama on this machine with its standard port. Change this only if you moved Ollama to a different port or another machine. | diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index 17702ab6..0e421110 100644 --- a/src-tauri/src/commands.rs +++ b/src-tauri/src/commands.rs @@ -17,10 +17,26 @@ pub enum OllamaErrorKind { NotRunning, /// The requested model has not been pulled yet (HTTP 404). ModelNotFound, + /// No active model has been selected. The user must pick a model from + /// the in-app picker before any chat request can be issued. Distinct + /// from `ModelNotFound`, which fires when the daemon answered 404 for + /// a slug we did try to use. + NoModelSelected, /// Any other unexpected error. Other, } +/// Builds the structured error returned when `ActiveModelState` holds `None` +/// at the time `ask_ollama` is invoked. Pulled out as a free function so the +/// exact title + body wording lives in one place and the branch is testable +/// without a full Tauri runtime. +pub fn no_model_selected_error() -> OllamaError { + OllamaError { + kind: OllamaErrorKind::NoModelSelected, + message: "No model selected\nPick a model in the picker.".to_string(), + } +} + /// Structured error emitted over the streaming channel. /// Rust owns all user-facing copy; the frontend only uses `kind` for styling. #[derive(Clone, Serialize, Debug)] @@ -351,12 +367,24 @@ pub async fn ask_ollama( config: State<'_, AppConfig>, active_model: State<'_, crate::models::ActiveModelState>, ) -> Result<(), String> { - let endpoint = format!("{}/api/chat", config.model.ollama_url.trim_end_matches('/')); + let endpoint = format!( + "{}/api/chat", + config.inference.ollama_url.trim_end_matches('/') + ); // Snapshot the active model slug; drop the guard before any `.await`. let model_name = { let guard = active_model.0.lock().map_err(|e| e.to_string())?; guard.clone() }; + let Some(model_name) = model_name else { + // Defense in depth: the onboarding gate already refuses to open the + // overlay without a selected model, so this branch only fires if the + // user removed their last installed model with `ollama rm` between + // launches and the picker hasn't been opened yet. Surface a typed + // error so the frontend can route the user to the picker. + let _ = on_event.send(StreamChunk::Error(no_model_selected_error())); + return Ok(()); + }; let cancel_token = CancellationToken::new(); generation.set_token(cancel_token.clone()); @@ -1266,6 +1294,30 @@ mod tests { assert!(err.message.contains("401")); } + #[test] + fn no_model_selected_error_uses_typed_kind_and_actionable_message() { + // The frontend keys off `kind` to route to the picker; the message + // is rendered verbatim. Both are part of the IPC contract: lock + // them down so accidental wording drift does not silently break + // the recovery path. + let err = no_model_selected_error(); + assert_eq!(err.kind, OllamaErrorKind::NoModelSelected); + assert!( + err.message.contains("Pick a model"), + "message should steer the user to the picker, got: {}", + err.message, + ); + } + + #[test] + fn ollama_error_kind_no_model_selected_serializes_as_pascal_case() { + // Wire format check: NoModelSelected must serialize verbatim in + // PascalCase so the React side can match on a stable string in the + // OllamaError discriminator. + let v = serde_json::to_value(OllamaErrorKind::NoModelSelected).unwrap(); + assert_eq!(v, serde_json::Value::String("NoModelSelected".to_string())); + } + #[tokio::test] async fn connection_refused_emits_not_running_error() { let client = reqwest::Client::new(); diff --git a/src-tauri/src/config/defaults.rs b/src-tauri/src/config/defaults.rs index be3af79e..feeb0b61 100644 --- a/src-tauri/src/config/defaults.rs +++ b/src-tauri/src/config/defaults.rs @@ -5,10 +5,6 @@ //! Changing a default here propagates to a fresh first-run config file and to //! any field a user has left unset or left empty in their existing file. -/// Default active model name, used when no config file exists yet and when a -/// user's `[model] available` list is empty after whitespace resolution. -pub const DEFAULT_MODEL_NAME: &str = "gemma4:e2b"; - /// Default Ollama HTTP endpoint (loopback, standard port). pub const DEFAULT_OLLAMA_URL: &str = "http://127.0.0.1:11434"; diff --git a/src-tauri/src/config/loader.rs b/src-tauri/src/config/loader.rs index 8149ead2..70eae85c 100644 --- a/src-tauri/src/config/loader.rs +++ b/src-tauri/src/config/loader.rs @@ -109,11 +109,11 @@ fn rename_corrupt(path: &Path) { /// and composes the system prompt appendix into `prompt.resolved_system`. /// After this runs, every `AppConfig` field holds a usable value. pub(crate) fn resolve(config: &mut AppConfig) { - // Model section: only the Ollama endpoint is configurable here. The + // Inference section: only the Ollama endpoint is configurable here. The // active model is runtime UI state owned by SQLite app_config, see // crate::models::ActiveModelState. - if config.model.ollama_url.trim().is_empty() { - config.model.ollama_url = DEFAULT_OLLAMA_URL.to_string(); + if config.inference.ollama_url.trim().is_empty() { + config.inference.ollama_url = DEFAULT_OLLAMA_URL.to_string(); } // Prompt section: empty base -> built-in. Compose resolved_system. diff --git a/src-tauri/src/config/mod.rs b/src-tauri/src/config/mod.rs index 89dc1e83..256d7918 100644 --- a/src-tauri/src/config/mod.rs +++ b/src-tauri/src/config/mod.rs @@ -25,7 +25,7 @@ pub mod writer; pub use error::ConfigError; pub use loader::load_from_path; -pub use schema::{AppConfig, ModelSection, PromptSection, QuoteSection, WindowSection}; +pub use schema::{AppConfig, InferenceSection, PromptSection, QuoteSection, WindowSection}; pub use writer::atomic_write; /// File name of the user config file inside the OS config dir. diff --git a/src-tauri/src/config/schema.rs b/src-tauri/src/config/schema.rs index 2b8e2735..4ed3c09b 100644 --- a/src-tauri/src/config/schema.rs +++ b/src-tauri/src/config/schema.rs @@ -22,7 +22,7 @@ use super::defaults::{ DEFAULT_SEARCH_TIMEOUT_S, DEFAULT_SEARXNG_MAX_RESULTS, DEFAULT_SEARXNG_URL, DEFAULT_TOP_K_URLS, }; -/// Static, user-tunable model configuration. +/// Static, user-tunable inference daemon configuration. /// /// The active model selection is NOT stored here. Active-model state is /// runtime UI state owned by [`crate::models::ActiveModelState`] and @@ -34,12 +34,12 @@ use super::defaults::{ /// endpoint URL. #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] #[serde(default)] -pub struct ModelSection { +pub struct InferenceSection { /// HTTP base URL of the local Ollama instance. pub ollama_url: String, } -impl Default for ModelSection { +impl Default for InferenceSection { fn default() -> Self { Self { ollama_url: DEFAULT_OLLAMA_URL.to_string(), @@ -176,7 +176,7 @@ impl Default for SearchSection { #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Default)] #[serde(default)] pub struct AppConfig { - pub model: ModelSection, + pub inference: InferenceSection, pub prompt: PromptSection, pub window: WindowSection, pub quote: QuoteSection, diff --git a/src-tauri/src/config/tests.rs b/src-tauri/src/config/tests.rs index 2dd67a9b..eec9da28 100644 --- a/src-tauri/src/config/tests.rs +++ b/src-tauri/src/config/tests.rs @@ -24,7 +24,7 @@ use super::defaults::{ use super::error::ConfigError; use super::loader::{compose_system_prompt, load_from_path}; use super::schema::{ - AppConfig, ModelSection, PromptSection, QuoteSection, SearchSection, WindowSection, + AppConfig, InferenceSection, PromptSection, QuoteSection, SearchSection, WindowSection, }; use super::writer::atomic_write; @@ -47,7 +47,7 @@ fn defaults_const_values_match_schema_defaults() { // Guard rail: a change to a default in defaults.rs must flow through to // AppConfig::default(). If this test fails, someone changed one but not both. let c = AppConfig::default(); - assert_eq!(c.model.ollama_url, DEFAULT_OLLAMA_URL); + assert_eq!(c.inference.ollama_url, DEFAULT_OLLAMA_URL); assert_eq!(c.prompt.system, ""); assert_eq!(c.prompt.resolved_system, ""); assert_eq!(c.window.overlay_width, DEFAULT_OVERLAY_WIDTH); @@ -85,7 +85,7 @@ fn defaults_prompt_base_is_nonempty() { #[test] fn section_defaults_are_sensible() { - let m = ModelSection::default(); + let m = InferenceSection::default(); assert_eq!(m.ollama_url, DEFAULT_OLLAMA_URL); let p = PromptSection::default(); @@ -105,7 +105,7 @@ fn app_config_serde_round_trip_matches_defaults() { let parsed: AppConfig = toml::from_str(&toml_str).expect("deserialize"); // prompt.resolved_system is marked #[serde(skip)] so it does not round-trip // through the file. Compare everything else. - assert_eq!(parsed.model, original.model); + assert_eq!(parsed.inference, original.inference); assert_eq!(parsed.prompt.system, original.prompt.system); assert_eq!(parsed.window, original.window); assert_eq!(parsed.quote, original.quote); @@ -115,11 +115,11 @@ fn app_config_serde_round_trip_matches_defaults() { fn app_config_partial_file_fills_missing_fields_with_defaults() { // Only declare one field; serde(default) fills the rest. let partial = r#" - [model] + [inference] ollama_url = "http://localhost:9999" "#; let parsed: AppConfig = toml::from_str(partial).expect("partial file parses"); - assert_eq!(parsed.model.ollama_url, "http://localhost:9999"); + assert_eq!(parsed.inference.ollama_url, "http://localhost:9999"); assert_eq!(parsed.window.overlay_width, DEFAULT_OVERLAY_WIDTH); assert_eq!( parsed.quote.max_display_lines, @@ -164,7 +164,7 @@ fn load_missing_file_seeds_defaults_and_returns_them() { let config = load_from_path(&path).expect("seed on first run"); assert!(path.exists(), "file should be seeded"); - assert_eq!(config.model.ollama_url, DEFAULT_OLLAMA_URL); + assert_eq!(config.inference.ollama_url, DEFAULT_OLLAMA_URL); // Resolved system prompt composed from default base plus appendix. assert!(config .prompt @@ -183,7 +183,7 @@ fn load_missing_file_in_missing_parent_dir_creates_dir() { let path = config_path_in(&nested); let config = load_from_path(&path).expect("creates parent dir and seeds"); assert!(path.exists()); - assert_eq!(config.model.ollama_url, DEFAULT_OLLAMA_URL); + assert_eq!(config.inference.ollama_url, DEFAULT_OLLAMA_URL); } #[test] @@ -209,14 +209,14 @@ fn load_existing_valid_file_returns_resolved_config() { std::fs::write( &path, r#" - [model] + [inference] ollama_url = "http://localhost:99999" "#, ) .unwrap(); let config = load_from_path(&path).unwrap(); - assert_eq!(config.model.ollama_url, "http://localhost:99999"); + assert_eq!(config.inference.ollama_url, "http://localhost:99999"); } #[test] @@ -248,7 +248,7 @@ fn load_corrupt_file_is_renamed_and_reseeded() { std::fs::write(&path, "this is = definitely not [ valid toml").unwrap(); let config = load_from_path(&path).expect("recover from corrupt file"); - assert_eq!(config.model.ollama_url, DEFAULT_OLLAMA_URL); + assert_eq!(config.inference.ollama_url, DEFAULT_OLLAMA_URL); // Original file renamed with .corrupt- prefix. let renamed_exists = std::fs::read_dir(&dir) @@ -274,7 +274,11 @@ fn load_unreadable_file_returns_in_memory_defaults() { let dir = fresh_temp_dir(); let path = config_path_in(&dir); - std::fs::write(&path, "[model]\nollama_url = \"http://127.0.0.1:11434\"\n").unwrap(); + std::fs::write( + &path, + "[inference]\nollama_url = \"http://127.0.0.1:11434\"\n", + ) + .unwrap(); std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o000)).unwrap(); // If the current user is root, the permission bits are ignored and this @@ -286,7 +290,7 @@ fn load_unreadable_file_returns_in_memory_defaults() { } let config = load_from_path(&path).expect("fallback to in-memory defaults"); - assert_eq!(config.model.ollama_url, DEFAULT_OLLAMA_URL); + assert_eq!(config.inference.ollama_url, DEFAULT_OLLAMA_URL); // Restore so cleanup works. let _ = std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o644)); } @@ -295,7 +299,7 @@ fn load_unreadable_file_returns_in_memory_defaults() { #[test] fn resolve_unknown_model_field_is_ignored() { - // Older config files seeded a `[model] available = [...]` list. After + // Older config files seeded a `[inference] available = [...]` list. After // removing that field from the schema, serde must silently drop it // rather than refusing to parse the file. let dir = fresh_temp_dir(); @@ -303,14 +307,14 @@ fn resolve_unknown_model_field_is_ignored() { std::fs::write( &path, r#" - [model] + [inference] available = ["legacy:model", "another:model"] ollama_url = "http://localhost:11434" "#, ) .unwrap(); let config = load_from_path(&path).unwrap(); - assert_eq!(config.model.ollama_url, "http://localhost:11434"); + assert_eq!(config.inference.ollama_url, "http://localhost:11434"); } #[test] @@ -320,13 +324,13 @@ fn resolve_empty_ollama_url_falls_back() { std::fs::write( &path, r#" - [model] + [inference] ollama_url = " " "#, ) .unwrap(); let config = load_from_path(&path).unwrap(); - assert_eq!(config.model.ollama_url, DEFAULT_OLLAMA_URL); + assert_eq!(config.inference.ollama_url, DEFAULT_OLLAMA_URL); } #[test] @@ -807,7 +811,11 @@ fn search_batch_timeout_invariant_corrected() { fn toml_without_search_section_deserializes_to_defaults() { let dir = fresh_temp_dir(); let path = config_path_in(&dir); - std::fs::write(&path, "[model]\nollama_url = \"http://127.0.0.1:11434\"\n").unwrap(); + std::fs::write( + &path, + "[inference]\nollama_url = \"http://127.0.0.1:11434\"\n", + ) + .unwrap(); let loaded = load_from_path(&path).unwrap(); assert_eq!( loaded.search.searxng_url, DEFAULT_SEARXNG_URL, diff --git a/src-tauri/src/history.rs b/src-tauri/src/history.rs index 864a109a..daa6f72b 100644 --- a/src-tauri/src/history.rs +++ b/src-tauri/src/history.rs @@ -75,6 +75,8 @@ pub fn save_conversation( let guard = active_model.0.lock().map_err(|e| e.to_string())?; guard.clone() }; + let model_slug = + model_slug.ok_or_else(|| "No model selected; cannot save conversation.".to_string())?; // Use the first user message (truncated) as the initial title placeholder. let placeholder_title = messages.iter().find(|m| m.role == "user").map(|m| { @@ -293,7 +295,7 @@ pub async fn generate_title( let endpoint = format!( "{}/api/chat", - app_config.model.ollama_url.trim_end_matches('/') + app_config.inference.ollama_url.trim_end_matches('/') ); let cancel_token = tokio_util::sync::CancellationToken::new(); @@ -384,12 +386,9 @@ mod tests { .find(|m| m.role == "user") .map(|m| m.content.trim().to_string()); - let conversation_id = database::create_conversation( - &conn, - placeholder_title.as_deref(), - crate::config::defaults::DEFAULT_MODEL_NAME, - ) - .unwrap(); + let conversation_id = + database::create_conversation(&conn, placeholder_title.as_deref(), "gemma4:e2b") + .unwrap(); let batch: Vec = messages .into_iter() diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index ba55310a..b3c33c73 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -781,18 +781,14 @@ pub fn run() { // The installed list isn't queried here (no async runtime yet). // get_model_picker_state reconciles against the live /api/tags // inventory on first picker open and may replace this seed. - // The placeholder DEFAULT_MODEL_NAME bootstrap is a transient - // value used only until that first reconciliation, and is the - // last-resort fallback when both the persisted slug and the - // live installed list are absent. Phase 3 will gate the - // overlay on a real installed model so that placeholder is - // never streamed to Ollama. + // When nothing is persisted the seed is `None`: there is no + // compiled fallback. The Phase 3 onboarding gate refuses to + // open the overlay until a real installed model is selected, + // so an unset slug never reaches `ask_ollama`. let persisted_active = database::get_config(&db_conn, models::ACTIVE_MODEL_KEY) .expect("failed to read active_model from app_config"); - let initial_active_model = models::resolve_seed_active_model( - persisted_active.as_deref(), - crate::config::defaults::DEFAULT_MODEL_NAME, - ); + let initial_active_model = + models::resolve_seed_active_model(persisted_active.as_deref()); app.manage(models::ActiveModelState(std::sync::Mutex::new( initial_active_model, ))); diff --git a/src-tauri/src/models.rs b/src-tauri/src/models.rs index e02c62be..dda97315 100644 --- a/src-tauri/src/models.rs +++ b/src-tauri/src/models.rs @@ -8,8 +8,9 @@ * * The backend treats Ollama's `/api/tags` response as authoritative: a * persisted model is only honored if it still appears in the live installed - * list. If not, we fall back to the first installed model, then to the - * bootstrap default from `THUKI_SUPPORTED_AI_MODELS`. + * list. If not, we fall back to the first installed model. There is no + * compiled fallback: when nothing is installed and nothing is persisted, + * the active model is `None` and the user is prompted to pick one. */ use std::collections::HashMap; @@ -42,8 +43,14 @@ pub const MODEL_NOT_INSTALLED_ERR_PREFIX: &str = "Model is not installed in Olla /// In-memory cache of the currently active model slug. Written once at /// startup (after `resolve_seed_active_model`) and updated every time the /// user picks a new model via `set_active_model`. +/// +/// `None` means no model has been chosen yet: either the user has never +/// picked one and Ollama has nothing installed, or the user removed every +/// model with `ollama rm` between launches. Consumers must treat `None` as +/// "refuse the request and steer the user to the picker", never as a +/// trigger to invent a default. #[derive(Default)] -pub struct ActiveModelState(pub Mutex); +pub struct ActiveModelState(pub Mutex>); /// Top-level shape of the Ollama `/api/tags` response. Only the `models` /// array is consumed; all other fields are ignored. @@ -60,46 +67,38 @@ struct TagsModel { name: String, } -/// Chooses which model slug should be active given a persisted preference, -/// the live installed list from Ollama, and an env-derived bootstrap value. +/// Chooses which model slug should be active given a persisted preference +/// and the live installed list from Ollama. /// /// Resolution rules, in order: /// 1. If `persisted` is `Some` and still appears in `installed`, use it. /// 2. Otherwise use the first entry in `installed`. -/// 3. Otherwise fall back to `bootstrap` (the compiled-in / env default). +/// 3. Otherwise return `None`: nothing is installed and nothing is persisted, +/// so there is no honest answer. /// /// This helper assumes `installed` reflects real Ollama ground truth. At /// startup when no ground truth is available, use /// [`resolve_seed_active_model`] instead so a valid persisted choice is -/// never overridden by the bootstrap default just because Ollama has not -/// been queried yet. -pub fn resolve_active_model( - persisted: Option<&str>, - installed: &[String], - bootstrap: &str, -) -> String { +/// never lost just because Ollama has not been queried yet. +pub fn resolve_active_model(persisted: Option<&str>, installed: &[String]) -> Option { if let Some(p) = persisted { if installed.iter().any(|m| m == p) { - return p.to_string(); + return Some(p.to_string()); } } - if let Some(first) = installed.first() { - return first.clone(); - } - bootstrap.to_string() + installed.first().cloned() } /// Startup-time resolver that never cross-checks against an installed list. /// /// At process start we cannot call Ollama (no async runtime yet), so the -/// safe behavior is to trust the persisted value when present and only fall -/// back to the bootstrap default when nothing was ever persisted. The first -/// `get_model_picker_state` call from the frontend reconciles against the -/// real installed list and may replace this seed. -pub fn resolve_seed_active_model(persisted: Option<&str>, bootstrap: &str) -> String { +/// safe behavior is to trust the persisted value when present and otherwise +/// return `None`. The first `get_model_picker_state` call from the frontend +/// reconciles against the real installed list and may replace this seed. +pub fn resolve_seed_active_model(persisted: Option<&str>) -> Option { match persisted { - Some(slug) if !slug.is_empty() => slug.to_string(), - _ => bootstrap.to_string(), + Some(slug) if !slug.is_empty() => Some(slug.to_string()), + _ => None, } } @@ -249,18 +248,16 @@ pub async fn get_model_picker_state( active_model: tauri::State<'_, ActiveModelState>, config: tauri::State<'_, AppConfig>, ) -> Result { - let installed = fetch_installed_model_names(&client, &config.model.ollama_url).await?; + let installed = fetch_installed_model_names(&client, &config.inference.ollama_url).await?; let resolved = { let conn = db.0.lock().map_err(|e| e.to_string())?; let persisted = get_config(&conn, ACTIVE_MODEL_KEY).map_err(|e| e.to_string())?; - let resolved = resolve_active_model( - persisted.as_deref(), - &installed, - crate::config::defaults::DEFAULT_MODEL_NAME, - ); - if should_persist_resolved(&installed, persisted.as_deref(), &resolved) { - set_config(&conn, ACTIVE_MODEL_KEY, &resolved).map_err(|e| e.to_string())?; + let resolved = resolve_active_model(persisted.as_deref(), &installed); + if let Some(slug) = resolved.as_deref() { + if should_persist_resolved(&installed, persisted.as_deref(), slug) { + set_config(&conn, ACTIVE_MODEL_KEY, slug).map_err(|e| e.to_string())?; + } } resolved }; @@ -270,7 +267,11 @@ pub async fn get_model_picker_state( *guard = resolved.clone(); } - Ok(serde_json::json!({ "active": resolved, "all": installed })) + let active_value = match resolved { + Some(slug) => serde_json::Value::String(slug), + None => serde_json::Value::Null, + }; + Ok(serde_json::json!({ "active": active_value, "all": installed })) } /// Persists `model` as the active model after validating its shape and @@ -287,7 +288,7 @@ pub async fn set_active_model( ) -> Result<(), String> { validate_model_slug(&model)?; - let installed = fetch_installed_model_names(&client, &config.model.ollama_url).await?; + let installed = fetch_installed_model_names(&client, &config.inference.ollama_url).await?; validate_model_installed(&model, &installed)?; { @@ -297,7 +298,7 @@ pub async fn set_active_model( { let mut guard = active_model.0.lock().map_err(|e| e.to_string())?; - *guard = model; + *guard = Some(model); } Ok(()) @@ -340,22 +341,28 @@ pub enum ModelSetupState { /// here. The Tauri command is a thin wrapper that calls the fetcher, /// reads the persisted slug from SQLite, then delegates here. /// -/// Resolution rules for the Ready arm match -/// [`resolve_active_model`]: prefer the persisted slug when it is still -/// installed; otherwise fall back to the first installed slug. The -/// `bootstrap` argument is the compile-time fallback used only when -/// both inputs are absent, which by definition cannot happen on the -/// Ready arm (it would have routed to NoModelsInstalled). +/// Resolution rules for the Ready arm match [`resolve_active_model`]: +/// prefer the persisted slug when it is still installed; otherwise fall +/// back to the first installed slug. Ready is gated on `!installed.is_empty()` +/// so `installed.first()` is always `Some`; the unwrap is therefore total. pub fn derive_model_setup_state( installed_result: Result, String>, persisted: Option<&str>, - bootstrap: &str, ) -> ModelSetupState { match installed_result { Err(_) => ModelSetupState::OllamaUnreachable, Ok(installed) if installed.is_empty() => ModelSetupState::NoModelsInstalled, Ok(installed) => { - let active_slug = resolve_active_model(persisted, &installed, bootstrap); + // The empty-list arm above guarantees `installed` has at least + // one entry. Mirror `resolve_active_model`'s logic inline so + // every branch is statically reachable from tests: when the + // persisted slug is still installed we keep it, otherwise we + // fall through to the first installed slug. This avoids a + // dead `unwrap_or_else` arm that coverage cannot exercise. + let active_slug = match persisted { + Some(p) if installed.iter().any(|m| m == p) => p.to_string(), + _ => installed[0].clone(), + }; ModelSetupState::Ready { active_slug, installed, @@ -390,18 +397,14 @@ pub async fn check_model_setup( active_model: tauri::State<'_, ActiveModelState>, config: tauri::State<'_, AppConfig>, ) -> Result { - let installed_result = fetch_installed_model_names(&client, &config.model.ollama_url).await; + let installed_result = fetch_installed_model_names(&client, &config.inference.ollama_url).await; let persisted = { let conn = db.0.lock().map_err(|e| e.to_string())?; get_config(&conn, ACTIVE_MODEL_KEY).map_err(|e| e.to_string())? }; - let state = derive_model_setup_state( - installed_result, - persisted.as_deref(), - crate::config::defaults::DEFAULT_MODEL_NAME, - ); + let state = derive_model_setup_state(installed_result, persisted.as_deref()); if let ModelSetupState::Ready { ref active_slug, @@ -413,7 +416,7 @@ pub async fn check_model_setup( set_config(&conn, ACTIVE_MODEL_KEY, active_slug).map_err(|e| e.to_string())?; } let mut guard = active_model.0.lock().map_err(|e| e.to_string())?; - *guard = active_slug.clone(); + *guard = Some(active_slug.clone()); } Ok(state) @@ -657,7 +660,7 @@ pub async fn get_model_capabilities( cache: tauri::State<'_, ModelCapabilitiesCache>, config: tauri::State<'_, AppConfig>, ) -> Result, String> { - let base_url = &config.model.ollama_url; + let base_url = &config.inference.ollama_url; let installed = fetch_installed_model_names(&client, base_url).await?; Ok(reconcile_capabilities(&client, &cache, base_url, &installed).await) } @@ -730,56 +733,59 @@ mod tests { #[test] fn resolve_prefers_persisted_when_still_installed() { let installed = vec!["gemma4:e2b".to_string(), "gemma4:e4b".to_string()]; - let result = resolve_active_model(Some("gemma4:e4b"), &installed, "gemma4:e2b"); - assert_eq!(result, "gemma4:e4b"); + let result = resolve_active_model(Some("gemma4:e4b"), &installed); + assert_eq!(result, Some("gemma4:e4b".to_string())); } #[test] fn resolve_falls_back_to_first_installed_when_persisted_missing() { let installed = vec!["gemma4:e2b".to_string(), "gemma4:e4b".to_string()]; - let result = resolve_active_model(Some("llama3:8b"), &installed, "bootstrap-model"); - assert_eq!(result, "gemma4:e2b"); + let result = resolve_active_model(Some("llama3:8b"), &installed); + assert_eq!(result, Some("gemma4:e2b".to_string())); } #[test] - fn resolve_falls_back_to_bootstrap_when_nothing_installed() { + fn resolve_returns_none_when_nothing_installed_and_nothing_persisted() { let installed: Vec = vec![]; - let result = resolve_active_model(None, &installed, "bootstrap-model"); - assert_eq!(result, "bootstrap-model"); + let result = resolve_active_model(None, &installed); + assert_eq!(result, None); } #[test] fn resolve_with_no_persisted_uses_first_installed() { let installed = vec!["gemma4:e2b".to_string()]; - let result = resolve_active_model(None, &installed, "bootstrap-model"); - assert_eq!(result, "gemma4:e2b"); + let result = resolve_active_model(None, &installed); + assert_eq!(result, Some("gemma4:e2b".to_string())); } #[test] - fn resolve_with_empty_persisted_bootstrap_used_when_installed_empty() { + fn resolve_returns_none_when_persisted_present_but_installed_empty() { + // The persisted slug names a model the user removed with `ollama rm` + // and Ollama now reports an empty inventory. There is no honest + // answer here; refuse to invent one. let installed: Vec = vec![]; - let result = resolve_active_model(Some("gemma4:e2b"), &installed, "fallback"); - assert_eq!(result, "fallback"); + let result = resolve_active_model(Some("gemma4:e2b"), &installed); + assert_eq!(result, None); } // ── resolve_seed_active_model ──────────────────────────────────────────── #[test] fn seed_resolve_prefers_persisted() { - let result = resolve_seed_active_model(Some("llama3:8b"), "bootstrap-model"); - assert_eq!(result, "llama3:8b"); + let result = resolve_seed_active_model(Some("llama3:8b")); + assert_eq!(result, Some("llama3:8b".to_string())); } #[test] - fn seed_resolve_falls_back_to_bootstrap_when_none() { - let result = resolve_seed_active_model(None, "bootstrap-model"); - assert_eq!(result, "bootstrap-model"); + fn seed_resolve_returns_none_when_nothing_persisted() { + let result = resolve_seed_active_model(None); + assert_eq!(result, None); } #[test] - fn seed_resolve_falls_back_to_bootstrap_when_empty_persisted() { - let result = resolve_seed_active_model(Some(""), "bootstrap-model"); - assert_eq!(result, "bootstrap-model"); + fn seed_resolve_returns_none_when_empty_persisted() { + let result = resolve_seed_active_model(Some("")); + assert_eq!(result, None); } // ── should_persist_resolved ───────────────────────────────────────────── @@ -1173,9 +1179,9 @@ mod tests { // ── ActiveModelState ───────────────────────────────────────────────────── #[test] - fn active_model_state_defaults_to_empty_string() { + fn active_model_state_defaults_to_none() { let state = ActiveModelState::default(); - assert_eq!(*state.0.lock().unwrap(), ""); + assert_eq!(*state.0.lock().unwrap(), None); } #[test] @@ -1183,9 +1189,9 @@ mod tests { let state = ActiveModelState::default(); { let mut guard = state.0.lock().unwrap(); - *guard = "gemma4:e2b".to_string(); + *guard = Some("gemma4:e2b".to_string()); } - assert_eq!(*state.0.lock().unwrap(), "gemma4:e2b"); + assert_eq!(*state.0.lock().unwrap(), Some("gemma4:e2b".to_string())); } // ── Persistence round-trip through app_config ─────────────────────────── @@ -1215,8 +1221,7 @@ mod tests { #[test] fn derive_setup_state_returns_unreachable_on_fetch_error() { - let state = - derive_model_setup_state(Err("connection refused".to_string()), None, "gemma4:e2b"); + let state = derive_model_setup_state(Err("connection refused".to_string()), None); assert_eq!(state, ModelSetupState::OllamaUnreachable); } @@ -1224,14 +1229,13 @@ mod tests { fn derive_setup_state_returns_unreachable_even_when_persisted_choice_exists() { // Past selection must NOT mask a current outage. The user needs to // see the "Ollama not detected" screen even if SQLite remembers a slug. - let state = - derive_model_setup_state(Err("timeout".to_string()), Some("gemma4:e4b"), "gemma4:e2b"); + let state = derive_model_setup_state(Err("timeout".to_string()), Some("gemma4:e4b")); assert_eq!(state, ModelSetupState::OllamaUnreachable); } #[test] fn derive_setup_state_returns_no_models_when_inventory_empty() { - let state = derive_model_setup_state(Ok(vec![]), None, "gemma4:e2b"); + let state = derive_model_setup_state(Ok(vec![]), None); assert_eq!(state, ModelSetupState::NoModelsInstalled); } @@ -1239,7 +1243,7 @@ mod tests { fn derive_setup_state_returns_no_models_even_with_stale_persisted_slug() { // Daemon up but the user removed every model with `ollama rm`. The // persisted slug is no longer valid; the gate must re-engage. - let state = derive_model_setup_state(Ok(vec![]), Some("removed-model:7b"), "gemma4:e2b"); + let state = derive_model_setup_state(Ok(vec![]), Some("removed-model:7b")); assert_eq!(state, ModelSetupState::NoModelsInstalled); } @@ -1248,7 +1252,6 @@ mod tests { let state = derive_model_setup_state( Ok(vec!["gemma4:e2b".to_string(), "llama3:8b".to_string()]), Some("llama3:8b"), - "gemma4:e2b", ); assert_eq!( state, @@ -1264,7 +1267,6 @@ mod tests { let state = derive_model_setup_state( Ok(vec!["gemma4:e4b".to_string(), "llama3:8b".to_string()]), Some("removed-model:7b"), - "gemma4:e2b", ); assert_eq!( state, @@ -1279,8 +1281,7 @@ mod tests { fn derive_setup_state_ready_uses_first_when_no_persisted_choice() { // First-time user who somehow has models installed already (rare: // they used Ollama for something else first). Pick the first. - let state = - derive_model_setup_state(Ok(vec!["qwen2.5:7b".to_string()]), None, "gemma4:e2b"); + let state = derive_model_setup_state(Ok(vec!["qwen2.5:7b".to_string()]), None); assert_eq!( state, ModelSetupState::Ready { diff --git a/src-tauri/src/search/mod.rs b/src-tauri/src/search/mod.rs index 05c34e90..68936e45 100644 --- a/src-tauri/src/search/mod.rs +++ b/src-tauri/src/search/mod.rs @@ -75,6 +75,16 @@ pub async fn search_pipeline( let guard = active_model_state.0.lock().map_err(|e| e.to_string())?; guard.clone() }; + let Some(model_name) = model_name else { + // Mirrors the chat-path gate: refuse to dispatch with no active + // model. The frontend strip already steers the user to the picker + // before this point, so this branch is defense-in-depth for the + // race where the user's last installed model was removed mid-run. + let _ = on_event.send(SearchEvent::Error { + message: "No model selected. Pick a model in the picker.".to_string(), + }); + return Ok(()); + }; // Pre-flight: verify both sandbox services are reachable before touching // the LLM or SearXNG. A 2-second probe prevents a long wait when the @@ -92,7 +102,7 @@ pub async fn search_pipeline( let ollama_endpoint = format!( "{}/api/chat", - app_config.model.ollama_url.trim_end_matches('/') + app_config.inference.ollama_url.trim_end_matches('/') ); let cancel_token = CancellationToken::new(); generation.set_token(cancel_token.clone()); diff --git a/src/App.tsx b/src/App.tsx index 12b412e8..00a5397c 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -36,9 +36,6 @@ import { } from './config/commands'; import './App.css'; -/** Fallback model name used before get_model_picker_state resolves at startup. */ -const DEFAULT_MODEL_FALLBACK = 'gemma4:e2b'; - const OVERLAY_VISIBILITY_EVENT = 'thuki://visibility'; const ONBOARDING_EVENT = 'thuki://onboarding'; @@ -667,9 +664,10 @@ function App() { if (isSaved) { await unsave(); } else { - // activeModel is empty string until the model picker hook resolves on first - // load; fall back to the bootstrap default during that brief window. - await save(messages, activeModel || DEFAULT_MODEL_FALLBACK); + // `save` accepts `string | null` and short-circuits internally when + // there is no active model, so the no-model guard lives in one + // place rather than duplicated at every call site. + await save(messages, activeModel); } } catch { // State stays unchanged on failure; feedback is implicit in the icon. @@ -710,7 +708,7 @@ function App() { const handleSaveAndLoad = useCallback( async (id: string) => { try { - await save(messages, activeModel || DEFAULT_MODEL_FALLBACK); + await save(messages, activeModel); } catch { // Save failed - abort to avoid leaving the current session unprotected. return; @@ -784,7 +782,7 @@ function App() { /** Saves the current conversation then starts a fresh one. */ const handleSaveAndNew = useCallback(async () => { try { - await save(messages, activeModel || DEFAULT_MODEL_FALLBACK); + await save(messages, activeModel); } catch { return; } diff --git a/src/__tests__/App.test.tsx b/src/__tests__/App.test.tsx index a0c816df..9373e330 100644 --- a/src/__tests__/App.test.tsx +++ b/src/__tests__/App.test.tsx @@ -1521,6 +1521,8 @@ describe('App', () => { it('handleSaveAndNew aborts reset when save fails', async () => { invoke.mockImplementation(async (cmd: string) => { + if (cmd === 'get_model_picker_state') + return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; if (cmd === 'list_conversations') return []; if (cmd === 'save_conversation') throw new Error('disk full'); }); @@ -1637,6 +1639,8 @@ describe('App', () => { // Bug: without the early return on save failure, the load would still run // and could overwrite the current session with an unrelated conversation. invoke.mockImplementation(async (cmd: string) => { + if (cmd === 'get_model_picker_state') + return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; if (cmd === 'list_conversations') return [ { @@ -1991,6 +1995,8 @@ describe('App', () => { // Bug: without try/catch, setIsHistoryOpen(false) is never reached when // loadConversation() throws, leaving the panel open on failure. invoke.mockImplementation(async (cmd: string) => { + if (cmd === 'get_model_picker_state') + return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; if (cmd === 'list_conversations') return [ { @@ -2263,6 +2269,8 @@ describe('App', () => { it('handleImagesAttached removes image when backend fails', async () => { invoke.mockImplementation(async (cmd: string) => { + if (cmd === 'get_model_picker_state') + return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; if (cmd === 'save_image_command') throw new Error('disk full'); }); @@ -2311,6 +2319,8 @@ describe('App', () => { let saveCallCount = 0; invoke.mockImplementation( async (cmd: string, args?: Record) => { + if (cmd === 'get_model_picker_state') + return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; if (args && 'onEvent' in args) { // channel capture - no-op for this test } @@ -2681,6 +2691,8 @@ describe('App', () => { // Make save_image_command hang so the image stays as a blob URL invoke.mockImplementation( async (cmd: string, args?: Record) => { + if (cmd === 'get_model_picker_state') + return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; if (args && 'onEvent' in args) { // channel capture } @@ -2785,6 +2797,8 @@ describe('App', () => { // Make save_image_command hang forever (never resolve) invoke.mockImplementation( async (cmd: string, args?: Record) => { + if (cmd === 'get_model_picker_state') + return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; if (args && 'onEvent' in args) { // channel capture - no-op } @@ -2843,6 +2857,8 @@ describe('App', () => { const savePromises: Promise[] = []; invoke.mockImplementation( async (cmd: string, args?: Record) => { + if (cmd === 'get_model_picker_state') + return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; if (args && 'onEvent' in args) { // Accept channel for ask_ollama } @@ -2938,6 +2954,8 @@ describe('App', () => { let resolveCancel!: () => void; invoke.mockImplementation(async (cmd: string) => { + if (cmd === 'get_model_picker_state') + return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; if (cmd === 'search_pipeline') { return new Promise((res) => { resolveSearch = res; @@ -3005,6 +3023,8 @@ describe('App', () => { invoke.mockImplementation( async (cmd: string, args?: Record) => { + if (cmd === 'get_model_picker_state') + return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; if (args && 'onEvent' in args) { // Accept channel } @@ -3079,6 +3099,8 @@ describe('App', () => { const resolvers: ((path: string) => void)[] = []; invoke.mockImplementation( async (cmd: string, args?: Record) => { + if (cmd === 'get_model_picker_state') + return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; if (args && 'onEvent' in args) { // Accept channel } @@ -3147,6 +3169,8 @@ describe('App', () => { let rejectSave: ((err: Error) => void) | null = null; invoke.mockImplementation( async (cmd: string, args?: Record) => { + if (cmd === 'get_model_picker_state') + return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; if (args && 'onEvent' in args) { // channel capture } @@ -3734,6 +3758,8 @@ describe('App', () => { it('does not call ask when capture_full_screen_command throws', async () => { invoke.mockImplementation(async (cmd: string) => { + if (cmd === 'get_model_picker_state') + return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; if (cmd === 'capture_full_screen_command') { throw new Error('Permission denied'); } @@ -3764,6 +3790,8 @@ describe('App', () => { it('surfaces string errors from Tauri invoke directly', async () => { invoke.mockImplementation(async (cmd: string) => { + if (cmd === 'get_model_picker_state') + return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; if (cmd === 'capture_full_screen_command') { // Tauri v2 rejects with the Err(String) value as a plain string. return Promise.reject( @@ -3796,6 +3824,8 @@ describe('App', () => { it('handles non-Error non-string rejection values', async () => { invoke.mockImplementation(async (cmd: string) => { + if (cmd === 'get_model_picker_state') + return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; if (cmd === 'capture_full_screen_command') { return Promise.reject(42); } @@ -3820,6 +3850,8 @@ describe('App', () => { it('clears capture error when a new submit is attempted', async () => { enableChannelCapture(); invoke.mockImplementation(async (cmd: string) => { + if (cmd === 'get_model_picker_state') + return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; if (cmd === 'capture_full_screen_command') { throw new Error('capture failed'); } @@ -3986,6 +4018,8 @@ describe('App', () => { it('restores query with cleanQuery text when capture fails mid-message', async () => { invoke.mockImplementation(async (cmd: string) => { + if (cmd === 'get_model_picker_state') + return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; if (cmd === 'capture_full_screen_command') { throw new Error('Screen capture timed out'); } @@ -4666,6 +4700,8 @@ describe('App', () => { const savePromises: Promise[] = []; invoke.mockImplementation( async (cmd: string, args?: Record) => { + if (cmd === 'get_model_picker_state') + return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; if (args && 'onEvent' in args) { // Accept channel for ask_ollama } diff --git a/src/components/ErrorCard.tsx b/src/components/ErrorCard.tsx index 4507a718..a09d456c 100644 --- a/src/components/ErrorCard.tsx +++ b/src/components/ErrorCard.tsx @@ -1,4 +1,8 @@ -export type OllamaErrorKind = 'NotRunning' | 'ModelNotFound' | 'Other'; +export type OllamaErrorKind = + | 'NotRunning' + | 'ModelNotFound' + | 'NoModelSelected' + | 'Other'; interface ErrorCardProps { kind: OllamaErrorKind; @@ -8,6 +12,9 @@ interface ErrorCardProps { const barColors: Record = { NotRunning: '#ef4444', ModelNotFound: '#f59e0b', + // Same accent as ModelNotFound: this is a configuration/setup nudge, + // not a daemon failure, so the warning hue (amber) is the right read. + NoModelSelected: '#f59e0b', Other: 'rgba(255,255,255,0.2)', }; diff --git a/src/components/WindowControls.tsx b/src/components/WindowControls.tsx index db706819..ae5e2910 100644 --- a/src/components/WindowControls.tsx +++ b/src/components/WindowControls.tsx @@ -131,13 +131,16 @@ interface WindowControlsProps { */ onNewConversation?: () => void; /** - * Currently active model slug displayed in the pill trigger. - * Requires `onModelPickerToggle` to be present; omit either to hide the pill. + * Currently active model slug displayed in the pill trigger. When `null` + * or `undefined` the pill renders a "Pick a model" placeholder so the + * affordance stays visible: the picker is the recovery path when no + * model is selected, so it must be reachable even with a null active. */ - activeModel?: string; + activeModel?: string | null; /** * Called when the user clicks the active-model pill to open/close the picker. - * Requires `activeModel` to be present; omit either to hide the pill. + * Omit to hide the pill entirely. When provided the pill always renders, + * regardless of `activeModel`, so users can recover from a no-model state. */ onModelPickerToggle?: () => void; /** Drives `aria-expanded` on the pill button. */ @@ -208,8 +211,12 @@ export const WindowControls = memo(function WindowControls({ {/* Right-side header controls */}
- {/* Active model pill trigger: leftmost, before save */} - {activeModel !== undefined && onModelPickerToggle !== undefined && ( + {/* Active model pill trigger: leftmost, before save. The pill + renders whenever the picker callback is wired up, regardless of + whether a model is currently selected. When no model is active + the chip surfaces the "Pick a model" affordance so the user + has a one-click recovery path out of the no-model state. */} + {onModelPickerToggle !== undefined && ( diff --git a/src/components/__tests__/WindowControls.test.tsx b/src/components/__tests__/WindowControls.test.tsx index 51e1a29d..f407195d 100644 --- a/src/components/__tests__/WindowControls.test.tsx +++ b/src/components/__tests__/WindowControls.test.tsx @@ -88,9 +88,40 @@ describe('WindowControls', () => { expect(screen.getByText('gemma4:e2b')).toBeInTheDocument(); }); - it('hides model pill when activeModel is not provided', () => { + it('renders the picker chip with a "Pick a model" placeholder when activeModel is null', () => { + // The chip is the recovery affordance for the no-model state, so it + // must stay visible (and clickable) even when activeModel is null. + // Without this, the user has no path back to the picker. + render( + , + ); + expect( + screen.getByRole('button', { name: 'Choose model' }), + ).toBeInTheDocument(); + expect(screen.getByText('Pick a model')).toBeInTheDocument(); + }); + + it('renders the picker chip with a "Pick a model" placeholder when activeModel is omitted', () => { render(); - expect(screen.queryByRole('button', { name: 'Choose model' })).toBeNull(); + expect( + screen.getByRole('button', { name: 'Choose model' }), + ).toBeInTheDocument(); + expect(screen.getByText('Pick a model')).toBeInTheDocument(); + }); + + it('renders the picker chip with a "Pick a model" placeholder when activeModel is empty string', () => { + render( + , + ); + expect(screen.getByText('Pick a model')).toBeInTheDocument(); }); it('hides model pill when onModelPickerToggle is not provided', () => { diff --git a/src/contexts/ConfigContext.tsx b/src/contexts/ConfigContext.tsx index c11231a9..99929347 100644 --- a/src/contexts/ConfigContext.tsx +++ b/src/contexts/ConfigContext.tsx @@ -9,7 +9,9 @@ * * The Rust `AppConfig` serializes with snake_case field names (matching the * on-disk TOML schema). We translate to camelCase here so React components - * keep their idiomatic JS names. + * keep their idiomatic JS names. The active model is NOT in this config: + * Ollama's `/api/tags` is the source of truth and the active slug lives in + * the Tauri-side `ActiveModelState`, surfaced through `useModelSelection`. */ import { createContext, use, useEffect, useState, type ReactNode } from 'react'; @@ -17,8 +19,7 @@ import { invoke } from '@tauri-apps/api/core'; /** Shape returned by the Rust `get_config` command (snake_case). */ interface RawAppConfig { - model: { - available: string[]; + inference: { ollama_url: string; }; prompt: { @@ -39,11 +40,7 @@ interface RawAppConfig { /** Camel-cased, frontend-friendly view of the configuration. */ export interface AppConfig { - model: { - /** First entry of `available` (the list-order invariant). */ - active: string; - /** Full list, in order. */ - available: string[]; + inference: { ollamaUrl: string; }; prompt: { @@ -65,10 +62,8 @@ export interface AppConfig { function transform(raw: RawAppConfig): AppConfig { return { - model: { - active: raw.model.available[0] ?? '', - available: raw.model.available, - ollamaUrl: raw.model.ollama_url, + inference: { + ollamaUrl: raw.inference.ollama_url, }, prompt: { system: raw.prompt.system, @@ -165,9 +160,7 @@ export function ConfigProviderForTest({ * `src-tauri/src/config/defaults.rs`). */ export const DEFAULT_CONFIG: AppConfig = { - model: { - active: 'gemma4:e2b', - available: ['gemma4:e2b'], + inference: { ollamaUrl: 'http://127.0.0.1:11434', }, prompt: { system: '' }, diff --git a/src/contexts/__tests__/ConfigContext.test.tsx b/src/contexts/__tests__/ConfigContext.test.tsx index 41a5277f..80828057 100644 --- a/src/contexts/__tests__/ConfigContext.test.tsx +++ b/src/contexts/__tests__/ConfigContext.test.tsx @@ -13,7 +13,7 @@ function Probe() { const config = useConfig(); return ( <> -
{config.model.active}
+
{config.inference.ollamaUrl}
{config.window.overlayWidth}
{config.quote.maxDisplayLines}
{config.prompt.system}
@@ -30,8 +30,8 @@ describe('ConfigContext', () => { describe('useConfig fallback', () => { it('returns DEFAULT_CONFIG when no provider is in the tree', () => { render(); - expect(screen.getByTestId('active-model').textContent).toBe( - DEFAULT_CONFIG.model.active, + expect(screen.getByTestId('ollama-url').textContent).toBe( + DEFAULT_CONFIG.inference.ollamaUrl, ); expect(screen.getByTestId('overlay-width').textContent).toBe( String(DEFAULT_CONFIG.window.overlayWidth), @@ -46,9 +46,7 @@ describe('ConfigContext', () => { it('provides the supplied value to descendants', () => { const custom: AppConfig = { ...DEFAULT_CONFIG, - model: { - active: 'custom:model', - available: ['custom:model', 'other:model'], + inference: { ollamaUrl: 'http://example.test:11434', }, }; @@ -57,8 +55,8 @@ describe('ConfigContext', () => { , ); - expect(screen.getByTestId('active-model').textContent).toBe( - 'custom:model', + expect(screen.getByTestId('ollama-url').textContent).toBe( + 'http://example.test:11434', ); }); }); @@ -66,8 +64,7 @@ describe('ConfigContext', () => { describe('ConfigProvider', () => { it('hydrates from the backend and transforms snake_case to camelCase', async () => { invoke.mockResolvedValueOnce({ - model: { - available: ['gemma4:e4b', 'gemma4:e2b'], + inference: { ollama_url: 'http://127.0.0.1:11434', }, prompt: { system: 'custom base prompt' }, @@ -92,7 +89,9 @@ describe('ConfigContext', () => { // Let the useEffect + promise resolution flush. await act(async () => {}); - expect(screen.getByTestId('active-model').textContent).toBe('gemma4:e4b'); + expect(screen.getByTestId('ollama-url').textContent).toBe( + 'http://127.0.0.1:11434', + ); expect(screen.getByTestId('overlay-width').textContent).toBe('800'); expect(screen.getByTestId('max-display-lines').textContent).toBe('6'); expect(screen.getByTestId('system-prompt').textContent).toBe( @@ -111,43 +110,14 @@ describe('ConfigContext', () => { ); await act(async () => {}); - expect(screen.getByTestId('active-model').textContent).toBe( - DEFAULT_CONFIG.model.active, + expect(screen.getByTestId('ollama-url').textContent).toBe( + DEFAULT_CONFIG.inference.ollamaUrl, ); expect(screen.getByTestId('overlay-width').textContent).toBe( String(DEFAULT_CONFIG.window.overlayWidth), ); }); - it('falls back to DEFAULT_CONFIG when the available list is empty', async () => { - // Edge case: Rust loader always prevents this, but the frontend transform - // should still produce a usable `active` (empty string) from an empty list. - invoke.mockResolvedValueOnce({ - model: { available: [], ollama_url: 'http://127.0.0.1:11434' }, - prompt: { system: '' }, - window: { - overlay_width: 600, - collapsed_height: 80, - max_chat_height: 648, - hide_commit_delay_ms: 350, - }, - quote: { - max_display_lines: 4, - max_display_chars: 300, - max_context_length: 4096, - }, - }); - - render( - - - , - ); - await act(async () => {}); - - expect(screen.getByTestId('active-model').textContent).toBe(''); - }); - it('falls back to DEFAULT_CONFIG when invoke rejects', async () => { invoke.mockRejectedValueOnce(new Error('IPC bridge unavailable')); @@ -158,8 +128,8 @@ describe('ConfigContext', () => { ); await act(async () => {}); - expect(screen.getByTestId('active-model').textContent).toBe( - DEFAULT_CONFIG.model.active, + expect(screen.getByTestId('ollama-url').textContent).toBe( + DEFAULT_CONFIG.inference.ollamaUrl, ); expect(screen.getByTestId('overlay-width').textContent).toBe( String(DEFAULT_CONFIG.window.overlayWidth), diff --git a/src/hooks/__tests__/useConversationHistory.test.tsx b/src/hooks/__tests__/useConversationHistory.test.tsx index c841f973..df688016 100644 --- a/src/hooks/__tests__/useConversationHistory.test.tsx +++ b/src/hooks/__tests__/useConversationHistory.test.tsx @@ -138,6 +138,21 @@ describe('useConversationHistory', () => { expect(invoke).not.toHaveBeenCalled(); }); + it('save() is a no-op when active model is null', async () => { + // Ollama's /api/tags is the single source of truth: when no model is + // selected we cannot honestly attribute a conversation, so save() must + // short-circuit before any IPC call. Mirrors the backend save_conversation + // contract which also rejects on a null active model. + invoke.mockResolvedValue(undefined); + const { result } = renderHook(() => useConversationHistory()); + + await act(async () => { + await result.current.save(MESSAGES, null); + }); + + expect(invoke).not.toHaveBeenCalled(); + }); + it('persistTurn() is a no-op when not saved', async () => { const { result } = renderHook(() => useConversationHistory()); diff --git a/src/hooks/__tests__/useModelSelection.test.tsx b/src/hooks/__tests__/useModelSelection.test.tsx index aa83a65a..bb0b7d41 100644 --- a/src/hooks/__tests__/useModelSelection.test.tsx +++ b/src/hooks/__tests__/useModelSelection.test.tsx @@ -24,6 +24,26 @@ describe('useModelSelection', () => { ]); }); + it('starts with a null active model before the first refresh resolves', () => { + invoke.mockImplementationOnce(() => new Promise(() => {})); + const { result } = renderHook(() => useModelSelection()); + expect(result.current.activeModel).toBeNull(); + expect(result.current.availableModels).toEqual([]); + }); + + it('accepts a null active payload from the backend (no model selected)', async () => { + // Ollama is the single source of truth: when nothing is installed and + // nothing is persisted, the backend returns active: null. The hook + // must surface that as null so consumers can route the user to the picker. + invoke.mockResolvedValueOnce({ active: null, all: [] }); + + const { result } = renderHook(() => useModelSelection()); + await act(async () => {}); + + expect(result.current.activeModel).toBeNull(); + expect(result.current.availableModels).toEqual([]); + }); + it('persists a new active model and updates local state', async () => { invoke .mockResolvedValueOnce({ @@ -52,7 +72,7 @@ describe('useModelSelection', () => { await act(async () => {}); expect(result.current.availableModels).toEqual([]); - expect(result.current.activeModel).toBe(''); + expect(result.current.activeModel).toBeNull(); }); it('falls back to empty state when payload shape is invalid', async () => { @@ -62,7 +82,7 @@ describe('useModelSelection', () => { await act(async () => {}); expect(result.current.availableModels).toEqual([]); - expect(result.current.activeModel).toBe(''); + expect(result.current.activeModel).toBeNull(); }); it('re-fetches models when refreshModels is called', async () => { @@ -94,7 +114,7 @@ describe('useModelSelection', () => { await act(async () => {}); expect(result.current.availableModels).toEqual([]); - expect(result.current.activeModel).toBe(''); + expect(result.current.activeModel).toBeNull(); }); it('rejects non-object payloads from the backend', async () => { @@ -104,7 +124,7 @@ describe('useModelSelection', () => { await act(async () => {}); expect(result.current.availableModels).toEqual([]); - expect(result.current.activeModel).toBe(''); + expect(result.current.activeModel).toBeNull(); }); it('rejects payloads whose `all` array contains non-string entries', async () => { @@ -114,7 +134,7 @@ describe('useModelSelection', () => { await act(async () => {}); expect(result.current.availableModels).toEqual([]); - expect(result.current.activeModel).toBe(''); + expect(result.current.activeModel).toBeNull(); }); it('surfaces backend errors and leaves active model unchanged on rejection', async () => { @@ -155,7 +175,7 @@ describe('useModelSelection', () => { await result.current.refreshModels(); }); - expect(result.current.activeModel).toBe(''); + expect(result.current.activeModel).toBeNull(); expect(result.current.availableModels).toEqual([]); }); diff --git a/src/hooks/useConversationHistory.ts b/src/hooks/useConversationHistory.ts index d04a294b..fdd0b7fa 100644 --- a/src/hooks/useConversationHistory.ts +++ b/src/hooks/useConversationHistory.ts @@ -275,11 +275,16 @@ export function useConversationHistory() { * arrives (~2-5 seconds). * * @param messages The complete message history to persist. - * @param model The active Ollama model slug used for title generation. + * @param model The active Ollama model slug used for title generation, + * or `null` when no model is selected. A null model short-circuits the + * save (no conversation can be attributed to a missing model). The + * backend `save_conversation` command also enforces this contract; + * gating here keeps the IPC surface clean. */ const save = useCallback( - async (messages: Message[], model: string): Promise => { + async (messages: Message[], model: string | null): Promise => { if (isSaved) return; + if (model == null) return; const payloads = messages.map(toPayload); diff --git a/src/hooks/useModelSelection.ts b/src/hooks/useModelSelection.ts index 97df0bcb..890a96e4 100644 --- a/src/hooks/useModelSelection.ts +++ b/src/hooks/useModelSelection.ts @@ -5,13 +5,17 @@ import type { ModelPickerState } from '../types/model'; /** * Runtime guard for the IPC boundary. The Rust backend is trusted, but this * keeps the hook robust against shape drift (schema changes, legacy builds, - * mocks) without pulling in a schema library. + * mocks) without pulling in a schema library. Accepts `null` for `active` + * because Ollama's `/api/tags` is the single source of truth: the backend + * returns null when nothing is installed and nothing is persisted. */ function isModelPickerState(value: unknown): value is ModelPickerState { if (typeof value !== 'object' || value === null) return false; const candidate = value as { active?: unknown; all?: unknown }; + const activeOk = + candidate.active === null || typeof candidate.active === 'string'; return ( - typeof candidate.active === 'string' && + activeOk && Array.isArray(candidate.all) && candidate.all.every((entry) => typeof entry === 'string') ); @@ -21,13 +25,18 @@ function isModelPickerState(value: unknown): value is ModelPickerState { * Shape returned by {@link useModelSelection}. */ export interface UseModelSelectionResult { - /** The currently active Ollama model name. Empty string until loaded. */ - activeModel: string; + /** + * The currently active Ollama model name, or `null` when none is selected + * (either nothing is installed or the picker has not resolved yet). + * Consumers must treat `null` as "block the action and surface the picker", + * never as a trigger to invent a default. + */ + activeModel: string | null; /** All locally installed Ollama model names available for selection. */ availableModels: string[]; /** - * Re-fetch the model picker state from the backend. Clears both - * `activeModel` and `availableModels` when the backend returns a malformed + * Re-fetch the model picker state from the backend. Sets `activeModel` to + * `null` and clears `availableModels` when the backend returns a malformed * payload or the call rejects. Callers are the single trigger: this hook * does not auto-retry. */ @@ -55,7 +64,7 @@ export function useModelSelection(): UseModelSelectionResult { // The state setter is intentionally renamed because `setActiveModel` is the // public async callback returned by this hook. // eslint-disable-next-line @eslint-react/use-state - const [activeModel, setActiveModelState] = useState(''); + const [activeModel, setActiveModelState] = useState(null); const [availableModels, setAvailableModels] = useState([]); const mountedRef = useRef(true); @@ -79,7 +88,7 @@ export function useModelSelection(): UseModelSelectionResult { const state = await invoke('get_model_picker_state'); if (!isLatest(token)) return; if (!isModelPickerState(state)) { - setActiveModelState(''); + setActiveModelState(null); setAvailableModels([]); return; } @@ -87,7 +96,7 @@ export function useModelSelection(): UseModelSelectionResult { setAvailableModels(state.all); } catch { if (!isLatest(token)) return; - setActiveModelState(''); + setActiveModelState(null); setAvailableModels([]); } }, [isLatest]); diff --git a/src/hooks/useOllama.ts b/src/hooks/useOllama.ts index f292864e..4b5d042d 100644 --- a/src/hooks/useOllama.ts +++ b/src/hooks/useOllama.ts @@ -10,7 +10,11 @@ import type { } from '../types/search'; /** Mirrors the Rust OllamaErrorKind enum sent over IPC. */ -export type OllamaErrorKind = 'NotRunning' | 'ModelNotFound' | 'Other'; +export type OllamaErrorKind = + | 'NotRunning' + | 'ModelNotFound' + | 'NoModelSelected' + | 'Other'; /** Represents a single message in the chat thread. */ export interface Message { @@ -128,13 +132,13 @@ function finalizeSearchTraceSteps( * @param activeModel Ollama model slug that should be attributed to each * assistant message produced by this hook. Passed as a hook parameter (not * a per-call argument) so the latest App-level selection is captured via - * closure on every render. An empty string (briefly possible on startup, - * before the model list resolves) is coerced to `undefined` on the emitted - * `Message`, so no attribution chip is rendered rather than a blank one. + * closure on every render. `null` (no model selected) and an empty string + * are both coerced to `undefined` on the emitted `Message`, so no + * attribution chip is rendered rather than a blank one. * @param onTurnComplete Optional callback invoked after each completed turn. */ export function useOllama( - activeModel: string, + activeModel: string | null, onTurnComplete?: (userMsg: Message, assistantMsg: Message) => void, ) { const [messages, setMessages] = useState([]); diff --git a/src/testUtils/mocks/tauri.ts b/src/testUtils/mocks/tauri.ts index 753345f4..4638738b 100644 --- a/src/testUtils/mocks/tauri.ts +++ b/src/testUtils/mocks/tauri.ts @@ -20,6 +20,16 @@ export const invoke = vi.fn< (cmd: string, args?: Record) => Promise >(async () => {}); +/** + * Default model-picker state used by tests that do not opt into a specific + * inventory. Tests that need the no-model state should mock + * `get_model_picker_state` to `{ active: null, all: [] }` explicitly. + */ +export const TEST_DEFAULT_MODEL_PICKER_STATE = { + active: 'gemma4:e2b', + all: ['gemma4:e2b'], +} as const; + /** * Channel capture state (per test). * @@ -37,18 +47,39 @@ export function getLastChannel(): Channel | null { return lastChannel; } +/** + * Default seed for `get_model_picker_state`. Tests that do not opt into a + * specific model inventory still need a non-null active model so the + * capability-mismatch strip does not block submits with the "no model + * selected" copy. Real Ollama responses look like this on a fresh install + * with one model pulled. + */ +const DEFAULT_MODEL_PICKER_STATE = { + active: 'gemma4:e2b', + all: ['gemma4:e2b'], +} as const; + /** * Enable channel capture: when invoke() is called with an onEvent argument, * that Channel will be stored in lastChannel for test use. * + * Tests that do not specify their own `get_model_picker_state` response get a + * default single-model inventory so the no-model gate does not block their + * submits. Tests that need to assert no-model behaviour should use + * `enableChannelCaptureWithResponses({ get_model_picker_state: { active: null, + * all: [] } })` instead. + * * IMPORTANT: Call resetChannelCapture() in afterEach to avoid state leaking between tests. */ export function enableChannelCapture() { invoke.mockImplementation( - async (_cmd: string, args?: Record) => { + async (cmd: string, args?: Record) => { if (args && 'onEvent' in args) { lastChannel = args.onEvent as Channel; } + if (cmd === 'get_model_picker_state') { + return DEFAULT_MODEL_PICKER_STATE; + } }, ); } @@ -80,6 +111,12 @@ export function enableChannelCaptureWithResponses( if (Object.prototype.hasOwnProperty.call(responses, cmd)) { return responses[cmd]; } + // Same default-seeding rationale as `enableChannelCapture`: tests + // that do not explicitly mock `get_model_picker_state` still need a + // non-null active model so the capability strip does not block submits. + if (cmd === 'get_model_picker_state') { + return DEFAULT_MODEL_PICKER_STATE; + } }, ); } diff --git a/src/types/model.ts b/src/types/model.ts index 4c449e3d..440f12c6 100644 --- a/src/types/model.ts +++ b/src/types/model.ts @@ -4,14 +4,15 @@ * Snapshot of model picker state returned by the Rust * `get_model_picker_state` Tauri command. * - * - `active` is the currently selected Ollama model name. Never empty once - * the backend has completed startup seeding. + * - `active` is the currently selected Ollama model name, or `null` when + * nothing is installed and nothing is persisted. The user must pick a + * model from the in-app picker before any chat request can be issued. * - `all` is the full list of locally installed Ollama model names, in the * order the backend chose to surface them (typically matches `ollama list`). */ export interface ModelPickerState { - /** The currently active Ollama model name. */ - active: string; + /** The currently active Ollama model name, or null when none is selected. */ + active: string | null; /** All locally installed Ollama model names available for selection. */ all: string[]; } diff --git a/src/utils/__tests__/capabilityConflicts.test.ts b/src/utils/__tests__/capabilityConflicts.test.ts index 45e0eaef..624a89c7 100644 --- a/src/utils/__tests__/capabilityConflicts.test.ts +++ b/src/utils/__tests__/capabilityConflicts.test.ts @@ -88,30 +88,47 @@ describe('getCapabilityConflict', () => { expect(result).toContain('reads text only'); }); - it('falls back to a generic name when model name is empty', () => { + it('returns the no-model message when modelName is empty', () => { + // Ollama's /api/tags is the single source of truth for the active + // model. An empty name short-circuits to the picker prompt regardless + // of compose state: no model means no submit can succeed. const result = getCapabilityConflict('', TEXT_ONLY, { ...EMPTY, imageCount: 1, }); expect(result).toBe( - 'this model reads text only. Try a vision model for images.', + 'Thuki needs a model to think with. Pull one in Ollama and tap the picker chip above to wire it up.', ); }); - it('falls back to a generic name when model name is null', () => { + it('returns the no-model message when modelName is null', () => { const result = getCapabilityConflict(null, TEXT_ONLY, { ...EMPTY, imageCount: 1, }); - expect(result?.startsWith('this model')).toBe(true); + expect(result).toBe( + 'Thuki needs a model to think with. Pull one in Ollama and tap the picker chip above to wire it up.', + ); }); - it('falls back to a generic name when model name is undefined', () => { + it('returns the no-model message when modelName is undefined', () => { const result = getCapabilityConflict(undefined, TEXT_ONLY, { ...EMPTY, imageCount: 1, }); - expect(result?.startsWith('this model')).toBe(true); + expect(result).toBe( + 'Thuki needs a model to think with. Pull one in Ollama and tap the picker chip above to wire it up.', + ); + }); + + it('returns the no-model message even when compose state is empty', () => { + // The strip needs to fire as soon as the user opens the overlay with + // nothing installed, before they type anything. Empty compose is the + // default state and must surface the "pick a model" copy. + const result = getCapabilityConflict(null, undefined, EMPTY); + expect(result).toBe( + 'Thuki needs a model to think with. Pull one in Ollama and tap the picker chip above to wire it up.', + ); }); // ── max-images gate ─────────────────────────────────────────────────────── @@ -209,13 +226,16 @@ describe('getCapabilityConflict', () => { expect(result).toBeNull(); }); - it('falls back to a generic name when /think mismatches and name is empty', () => { + it('surfaces the no-model message when name is empty even with /think queued', () => { + // The no-model gate runs before any capability check, so an empty name + // short-circuits to the picker prompt regardless of which command is + // queued. The /think mismatch copy never reaches the user. const result = getCapabilityConflict('', TEXT_ONLY, { ...EMPTY, hasThinkCommand: true, }); expect(result).toBe( - "this model doesn't show reasoning. Try a thinking model for /think.", + 'Thuki needs a model to think with. Pull one in Ollama and tap the picker chip above to wire it up.', ); }); diff --git a/src/utils/capabilityConflicts.ts b/src/utils/capabilityConflicts.ts index 2d03ba74..553b122c 100644 --- a/src/utils/capabilityConflicts.ts +++ b/src/utils/capabilityConflicts.ts @@ -31,21 +31,30 @@ export interface ComposeCapabilityState { * The strip and the submit-time toast both render the returned string * verbatim so the wording lives in exactly one place. * - * Defaults to permissive: an unknown active model (capabilities not yet - * fetched, or fetch failed) returns `null` so the user is never blocked - * by missing metadata. The backend is the final authority and will - * surface a real error if the model truly cannot accept the payload. + * Empty / null / undefined `modelName` short-circuits to a "pick a model" + * message regardless of compose state: Ollama's `/api/tags` is the single + * source of truth and the user must select a model before any submit can + * succeed. Capabilities-aware checks below only run once a model is + * actually selected. + * + * For a selected model with unknown capabilities (not yet fetched, or + * fetch failed) the gate is permissive and returns `null` so the user is + * never blocked by missing metadata. The backend surfaces a real error + * if the model truly cannot accept the payload. */ export function getCapabilityConflict( modelName: string | undefined | null, capabilities: ModelCapabilities | undefined | null, state: ComposeCapabilityState, ): string | null { + if (!modelName) { + return 'Thuki needs a model to think with. Pull one in Ollama and tap the picker chip above to wire it up.'; + } const needsVision = state.imageCount > 0 || state.hasScreenCommand; const needsThinking = state.hasThinkCommand; if (!needsVision && !needsThinking) return null; if (!capabilities) return null; - const name = modelName && modelName.length > 0 ? modelName : 'this model'; + const name = modelName; // Vision is checked first when both apply because it is the more // fundamental constraint: a text-only model cannot consume the image diff --git a/src/view/AskBarView.tsx b/src/view/AskBarView.tsx index d7a8bab3..a4d74650 100644 --- a/src/view/AskBarView.tsx +++ b/src/view/AskBarView.tsx @@ -189,8 +189,8 @@ interface AskBarViewProps { * "normal" = violet ring; "max" = red ring + label; undefined = no ring. */ isDragOver?: 'normal' | 'max'; - /** Currently active Ollama model slug. Enables the model picker when set. */ - activeModel?: string; + /** Currently active Ollama model slug, or `null` when none is selected. */ + activeModel?: string | null; /** Full list of model slugs available for selection in the picker. */ availableModels?: string[]; /** diff --git a/src/view/ConversationView.tsx b/src/view/ConversationView.tsx index f3c5badc..8f2714a5 100644 --- a/src/view/ConversationView.tsx +++ b/src/view/ConversationView.tsx @@ -74,8 +74,9 @@ interface ConversationViewProps { * of the typing indicator. */ searchStage?: SearchStage; - /** Currently active model slug forwarded to the WindowControls pill trigger. */ - activeModel?: string; + /** Currently active model slug forwarded to the WindowControls pill trigger. + * `null` keeps the chip visible with a "Pick a model" placeholder. */ + activeModel?: string | null; /** Toggles the model picker panel; forwarded to WindowControls. */ onModelPickerToggle?: () => void; /** Whether the model picker panel is open; drives aria-expanded on the pill. */ diff --git a/src/view/onboarding/ModelCheckStep.tsx b/src/view/onboarding/ModelCheckStep.tsx index 510dca3e..08905672 100644 --- a/src/view/onboarding/ModelCheckStep.tsx +++ b/src/view/onboarding/ModelCheckStep.tsx @@ -455,7 +455,7 @@ function RowOne({ active, done }: RowOneProps) { letterSpacing: '-0.1px', }} > - Listening on {formatListenAddr(config.model.ollamaUrl)} + Listening on {formatListenAddr(config.inference.ollamaUrl)}

) : null}
diff --git a/src/view/onboarding/__tests__/ModelCheckStep.test.tsx b/src/view/onboarding/__tests__/ModelCheckStep.test.tsx index a8dffc42..c18f4242 100644 --- a/src/view/onboarding/__tests__/ModelCheckStep.test.tsx +++ b/src/view/onboarding/__tests__/ModelCheckStep.test.tsx @@ -106,7 +106,10 @@ describe('ModelCheckStep', () => { @@ -126,7 +129,7 @@ describe('ModelCheckStep', () => { From b81ec18a0f74eee7b069c5e1e91dcae21515deb2 Mon Sep 17 00:00:00 2001 From: Logan Nguyen Date: Sun, 26 Apr 2026 14:52:24 -0500 Subject: [PATCH 2/4] fix(inference): distinguish Ollama-unreachable from no-models, restore picker chip Phase A surfaced two defects in manual smoke test: 1. Picker chip was missing in chat mode when activeModel was null. The chip is the recovery affordance, so it must stay visible whenever the user is in chat mode regardless of model state. 2. The no-model warning strip rendered the same copy whether Ollama was unreachable or simply had zero models installed. The two states need different recovery actions: start Ollama vs pull a model. get_model_picker_state now returns a typed ollamaReachable flag so the frontend can pick the right strip copy. The chip is rendered unconditionally in chat mode and hides only when Ollama is unreachable (nothing to pick from). Signed-off-by: Logan Nguyen --- src-tauri/src/models.rs | 96 +++++- src/App.tsx | 60 ++-- src/__tests__/App.test.tsx | 289 ++++++++++++++++-- src/components/ModelPickerPanel.tsx | 17 +- .../__tests__/ModelPickerPanel.test.tsx | 21 +- .../__tests__/useModelSelection.test.tsx | 82 ++++- src/hooks/useModelSelection.ts | 36 ++- src/testUtils/mocks/tauri.ts | 5 +- src/types/model.ts | 8 + .../__tests__/capabilityConflicts.test.ts | 98 ++++-- src/utils/capabilityConflicts.ts | 63 +++- 11 files changed, 668 insertions(+), 107 deletions(-) diff --git a/src-tauri/src/models.rs b/src-tauri/src/models.rs index dda97315..62111a7c 100644 --- a/src-tauri/src/models.rs +++ b/src-tauri/src/models.rs @@ -231,15 +231,20 @@ async fn fetch_installed_model_names_inner( Ok(body.models.into_iter().map(|m| m.name).collect()) } -/// Returns the currently active model and the full list of installed models, -/// persisting the resolved active model so future launches see it. +/// Returns the currently active model, the full list of installed models, and +/// a flag telling the frontend whether Ollama itself is reachable. /// -/// Shape: `{ "active": "", "all": ["", ...] }`. +/// Shape: `{ "active": "" | null, "all": ["", ...], "ollamaReachable": bool }`. /// -/// Coalesces the read + conditional write into a single database critical -/// section to avoid a TOCTOU window where a concurrent `set_active_model` -/// could be clobbered, and refuses to persist when Ollama reports an empty -/// inventory so a partially-up daemon cannot corrupt the persisted choice. +/// The command intentionally never propagates a transport / fetch error to +/// the frontend. Instead, an unreachable Ollama collapses into a structured +/// `{ active: null, all: [], ollamaReachable: false }` payload so the UI can +/// distinguish "Ollama is down" from "Ollama is up but has no models" without +/// parsing error strings. The Ok branch coalesces the read + conditional +/// write into a single database critical section to avoid a TOCTOU window +/// where a concurrent `set_active_model` could be clobbered, and refuses to +/// persist when Ollama reports an empty inventory so a partially-up daemon +/// cannot corrupt the persisted choice. #[cfg_attr(coverage_nightly, coverage(off))] #[cfg_attr(not(coverage), tauri::command)] pub async fn get_model_picker_state( @@ -248,7 +253,19 @@ pub async fn get_model_picker_state( active_model: tauri::State<'_, ActiveModelState>, config: tauri::State<'_, AppConfig>, ) -> Result { - let installed = fetch_installed_model_names(&client, &config.inference.ollama_url).await?; + let fetch_result = fetch_installed_model_names(&client, &config.inference.ollama_url).await; + + let installed = match fetch_result { + Ok(installed) => installed, + Err(_) => { + // Mirror the `None` active into the in-memory state so downstream + // callers (ask_ollama, search_pipeline) see the same truth as the + // frontend: with Ollama unreachable, no model is active. + let mut guard = active_model.0.lock().map_err(|e| e.to_string())?; + *guard = None; + return Ok(build_picker_state_payload(None, &[], false)); + } + }; let resolved = { let conn = db.0.lock().map_err(|e| e.to_string())?; @@ -267,11 +284,30 @@ pub async fn get_model_picker_state( *guard = resolved.clone(); } - let active_value = match resolved { - Some(slug) => serde_json::Value::String(slug), + Ok(build_picker_state_payload( + resolved.as_deref(), + &installed, + true, + )) +} + +/// Pure helper that shapes the `get_model_picker_state` payload. Extracted so +/// the three states (unreachable, reachable + empty, reachable + populated) +/// can be unit-tested without spinning up a Tauri runtime or an HTTP server. +pub fn build_picker_state_payload( + active: Option<&str>, + installed: &[String], + ollama_reachable: bool, +) -> serde_json::Value { + let active_value = match active { + Some(slug) => serde_json::Value::String(slug.to_string()), None => serde_json::Value::Null, }; - Ok(serde_json::json!({ "active": active_value, "all": installed })) + serde_json::json!({ + "active": active_value, + "all": installed, + "ollamaReachable": ollama_reachable, + }) } /// Persists `model` as the active model after validating its shape and @@ -728,6 +764,44 @@ async fn reconcile_capabilities( mod tests { use super::*; + // ── build_picker_state_payload ─────────────────────────────────────────── + + #[test] + fn picker_payload_unreachable_emits_null_active_empty_list_and_flag() { + // S1 mirrors the unreachable case: no model can be resolved, the + // installed list is empty by definition, and the flag is false so + // the frontend can pick the right strip copy. + let payload = build_picker_state_payload(None, &[], false); + assert_eq!(payload["active"], serde_json::Value::Null); + assert_eq!(payload["all"], serde_json::json!([])); + assert_eq!(payload["ollamaReachable"], serde_json::Value::Bool(false)); + } + + #[test] + fn picker_payload_reachable_but_empty_keeps_flag_true_and_null_active() { + // S2: Ollama responded but installed list is empty. Active is null + // (nothing to resolve to) yet ollamaReachable is true so the strip + // can tell the user to pull a model rather than start the daemon. + let payload = build_picker_state_payload(None, &[], true); + assert_eq!(payload["active"], serde_json::Value::Null); + assert_eq!(payload["all"], serde_json::json!([])); + assert_eq!(payload["ollamaReachable"], serde_json::Value::Bool(true)); + } + + #[test] + fn picker_payload_reachable_with_models_carries_active_slug() { + // S4 (normal): active slug is present and ollamaReachable is true. + // The frontend renders the chip with the slug and skips the strip. + let installed = vec!["gemma4:e2b".to_string(), "gemma4:e4b".to_string()]; + let payload = build_picker_state_payload(Some("gemma4:e4b"), &installed, true); + assert_eq!(payload["active"], serde_json::json!("gemma4:e4b")); + assert_eq!( + payload["all"], + serde_json::json!(["gemma4:e2b", "gemma4:e4b"]) + ); + assert_eq!(payload["ollamaReachable"], serde_json::Value::Bool(true)); + } + // ── resolve_active_model ───────────────────────────────────────────────── #[test] diff --git a/src/App.tsx b/src/App.tsx index 00a5397c..39228526 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -17,7 +17,10 @@ import type { Message } from './hooks/useOllama'; import { useConversationHistory } from './hooks/useConversationHistory'; import { useModelSelection } from './hooks/useModelSelection'; import { useModelCapabilities } from './hooks/useModelCapabilities'; -import { getCapabilityConflict } from './utils/capabilityConflicts'; +import { + getCapabilityConflict, + getEnvironmentMessage, +} from './utils/capabilityConflicts'; import { Toast } from './components/Toast'; import { ConversationView } from './view/ConversationView'; import { AskBarView, MAX_IMAGES } from './view/AskBarView'; @@ -132,8 +135,13 @@ function App() { */ const morphingContainerNodeRef = useRef(null); - const { activeModel, availableModels, refreshModels, setActiveModel } = - useModelSelection(); + const { + activeModel, + availableModels, + ollamaReachable, + refreshModels, + setActiveModel, + } = useModelSelection(); const { capabilities: modelCapabilities, refresh: refreshModelCapabilities } = useModelCapabilities(); @@ -1077,13 +1085,25 @@ function App() { ); /** - * Live capability conflict for the current compose state. Drives the - * inline `CapabilityMismatchStrip` so the user sees the mismatch as - * soon as incompatible content lands in compose, not only at submit - * time. The strip is purely informational: recovery happens through - * the model picker chip. + * Live strip message for the current environment + compose state. Drives + * the inline `CapabilityMismatchStrip` so the user sees the right cue as + * soon as content lands in compose, not only at submit time. The strip + * is purely informational: recovery happens through the model picker + * chip (or starting Ollama, when that is the actual problem). + * + * Resolution order matters: environment-state messaging wins over + * capability conflicts because telling the user to "switch models" + * makes no sense when Ollama is down or has no models installed. Once + * an active model exists and Ollama is reachable, fall through to the + * per-message capability check. */ const liveCapabilityConflictMessage = useMemo(() => { + const envMessage = getEnvironmentMessage( + ollamaReachable, + availableModels.length, + activeModel, + ); + if (envMessage !== null) return envMessage; const trimmed = query.trim(); const { found } = parseCommands(trimmed); return getCapabilityConflict(activeModel, activeModelCapabilities, { @@ -1091,7 +1111,14 @@ function App() { hasThinkCommand: found.has('/think'), imageCount: attachedImages.length, }); - }, [query, attachedImages, activeModel, activeModelCapabilities]); + }, [ + query, + attachedImages, + activeModel, + activeModelCapabilities, + ollamaReachable, + availableModels.length, + ]); const handleSubmit = useCallback(() => { if ( @@ -1652,7 +1679,9 @@ function App() { onImagePreview={handleChatImagePreview} searchStage={searchStage} activeModel={activeModel} - onModelPickerToggle={handleModelPickerToggle} + onModelPickerToggle={ + ollamaReachable ? handleModelPickerToggle : undefined + } isModelPickerOpen={isModelPickerOpen} /> ) : null} @@ -1662,10 +1691,7 @@ function App() { In chat mode the trigger and drawer move to the header area above. */} {!isChatMode && ( - {isModelPickerOpen && - activeModel && - availableModels && - availableModels.length > 0 ? ( + {isModelPickerOpen && ollamaReachable ? ( - {isChatMode && - isModelPickerOpen && - activeModel && - availableModels && - availableModels.length > 0 ? ( + {isChatMode && isModelPickerOpen && ollamaReachable ? ( { get_model_picker_state: { active: 'gemma4:e2b', all: ['gemma4:e2b', 'qwen2.5:7b'], + ollamaReachable: true, }, }); @@ -54,6 +55,7 @@ describe('App', () => { get_model_picker_state: { active: 'gemma4:e2b', all: ['gemma4:e2b', 'qwen2.5:7b'], + ollamaReachable: true, }, }); @@ -66,11 +68,157 @@ describe('App', () => { ).toBeInTheDocument(); }); + it('keeps the chat-mode picker chip visible with "Pick a model" when active model disappears (S2)', async () => { + // S2: Ollama is reachable but no models are installed. The chip must + // stay in WindowControls in chat mode so the user has a one-click + // recovery path, and its label falls back to the picker prompt + // instead of showing a stale or empty slug. + // + // Simulating S2 from a cold start would block the submit at the + // env-state gate, so we cannot enter chat mode that way. Instead we + // start with an active model, complete a turn (which puts the user + // in chat mode), then arrange the next `get_model_picker_state` + // refresh to return the S2 payload. + enableChannelCaptureWithResponses({ + get_model_picker_state: { + active: 'gemma4:e2b', + all: ['gemma4:e2b'], + ollamaReachable: true, + }, + }); + + render(); + await act(async () => {}); + await showOverlay(); + + // Send one message + simulate Done so messages.length > 0 → chat mode. + const textarea = screen.getByPlaceholderText('Ask Thuki anything...'); + act(() => { + fireEvent.change(textarea, { target: { value: 'hello' } }); + }); + act(() => { + fireEvent.keyDown(textarea, { key: 'Enter', shiftKey: false }); + }); + await act(async () => {}); + act(() => { + getLastChannel()?.simulateMessage({ type: 'Token', data: 'hi' }); + getLastChannel()?.simulateMessage({ type: 'Done' }); + }); + await act(async () => {}); + + // Switch the picker mock to return the S2 payload, then trigger the + // chip click which calls `refreshModels` under the hood. + enableChannelCaptureWithResponses({ + get_model_picker_state: { + active: null, + all: [], + ollamaReachable: true, + }, + }); + + fireEvent.click(screen.getByRole('button', { name: 'Choose model' })); + await act(async () => {}); + + const chip = screen.getByRole('button', { name: 'Choose model' }); + expect(chip).toBeInTheDocument(); + expect(chip.textContent).toContain('Pick a model'); + }); + + it('hides the chat-mode picker chip when Ollama becomes unreachable (S1)', async () => { + // S1: nothing to pick from. The chip is hidden in chat mode so the + // user is not pointed at a dead-end action; the strip handles the + // "start Ollama" cue separately. We mirror the S2 test setup but + // swap the second picker fetch to the unreachable payload. + // + // Triggering the refresh through the chip click rather than the + // overlay show event matters: the show handler also resets messages + // (so isChatMode flips back to false and the chip unmounts for an + // unrelated reason). The chip click drives the refresh in place. + enableChannelCaptureWithResponses({ + get_model_picker_state: { + active: 'gemma4:e2b', + all: ['gemma4:e2b'], + ollamaReachable: true, + }, + }); + + render(); + await act(async () => {}); + await showOverlay(); + + const textarea = screen.getByPlaceholderText('Ask Thuki anything...'); + act(() => { + fireEvent.change(textarea, { target: { value: 'hello' } }); + }); + act(() => { + fireEvent.keyDown(textarea, { key: 'Enter', shiftKey: false }); + }); + await act(async () => {}); + act(() => { + getLastChannel()?.simulateMessage({ type: 'Token', data: 'hi' }); + getLastChannel()?.simulateMessage({ type: 'Done' }); + }); + await act(async () => {}); + + // Confirm we are in chat mode with the chip rendered before flipping + // the picker state to the unreachable variant. + expect( + screen.getByRole('button', { name: 'Choose model' }), + ).toBeInTheDocument(); + + enableChannelCaptureWithResponses({ + get_model_picker_state: { + active: null, + all: [], + ollamaReachable: false, + }, + }); + + fireEvent.click(screen.getByRole('button', { name: 'Choose model' })); + await act(async () => {}); + + expect(screen.queryByRole('button', { name: 'Choose model' })).toBeNull(); + }); + + it('renders the unreachable strip copy in compose mode when Ollama is down (S1)', async () => { + enableChannelCaptureWithResponses({ + get_model_picker_state: { + active: null, + all: [], + ollamaReachable: false, + }, + }); + render(); + await act(async () => {}); + await showOverlay(); + + const strip = screen.getByTestId('capability-mismatch-strip'); + expect(strip.textContent).toContain("Ollama isn't running"); + }); + + it('renders the no-models strip copy when Ollama is reachable but empty (S2)', async () => { + enableChannelCaptureWithResponses({ + get_model_picker_state: { + active: null, + all: [], + ollamaReachable: true, + }, + }); + render(); + await act(async () => {}); + await showOverlay(); + + const strip = screen.getByTestId('capability-mismatch-strip'); + expect(strip.textContent).toContain('Pull one in Ollama'); + expect(strip.textContent).toContain('ollama pull '); + }); + it('saves the conversation with the currently selected model', async () => { enableChannelCaptureWithResponses({ get_model_picker_state: { active: 'gemma4:e2b', all: ['gemma4:e2b', 'qwen2.5:7b'], + ollamaReachable: true, }, save_conversation: { conversation_id: 'conv-1' }, generate_title: undefined, @@ -116,6 +264,7 @@ describe('App', () => { get_model_picker_state: { active: 'gemma4:e2b', all: ['gemma4:e2b', 'qwen2.5:7b'], + ollamaReachable: true, }, }); render(); @@ -138,6 +287,7 @@ describe('App', () => { get_model_picker_state: { active: 'gemma4:e2b', all: ['gemma4:e2b', 'qwen2.5:7b'], + ollamaReachable: true, }, list_conversations: [], }); @@ -164,6 +314,7 @@ describe('App', () => { get_model_picker_state: { active: 'gemma4:e2b', all: ['gemma4:e2b', 'qwen2.5:7b'], + ollamaReachable: true, }, list_conversations: [], }); @@ -190,6 +341,7 @@ describe('App', () => { get_model_picker_state: { active: 'gemma4:e2b', all: ['gemma4:e2b', 'qwen2.5:7b'], + ollamaReachable: true, }, set_active_model: undefined, }); @@ -213,6 +365,7 @@ describe('App', () => { get_model_picker_state: { active: 'gemma4:e2b', all: ['gemma4:e2b', 'qwen2.5:7b'], + ollamaReachable: true, }, }); render(); @@ -238,6 +391,7 @@ describe('App', () => { get_model_picker_state: { active: 'gemma4:e2b', all: ['gemma4:e2b', 'qwen2.5:7b'], + ollamaReachable: true, }, }); render(); @@ -263,6 +417,7 @@ describe('App', () => { get_model_picker_state: { active: 'gemma4:e2b', all: ['gemma4:e2b', 'qwen2.5:7b'], + ollamaReachable: true, }, }); render(); @@ -302,6 +457,7 @@ describe('App', () => { get_model_picker_state: { active: 'gemma4:e2b', all: ['gemma4:e2b', 'qwen2.5:7b'], + ollamaReachable: true, }, }); render(); @@ -334,6 +490,7 @@ describe('App', () => { get_model_picker_state: { active: 'gemma4:e2b', all: ['gemma4:e2b', 'qwen2.5:7b'], + ollamaReachable: true, }, }); render(); @@ -376,6 +533,7 @@ describe('App', () => { get_model_picker_state: { active: 'gemma4:e2b', all: ['gemma4:e2b', 'qwen2.5:7b'], + ollamaReachable: true, }, }); render(); @@ -408,9 +566,17 @@ describe('App', () => { if (cmd === 'get_model_picker_state') { if (rejectionSeen) { refreshesAfterRejection += 1; - return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; + return { + active: 'gemma4:e2b', + all: ['gemma4:e2b'], + ollamaReachable: true, + }; } - return { active: 'gemma4:e2b', all: ['gemma4:e2b', 'qwen2.5:7b'] }; + return { + active: 'gemma4:e2b', + all: ['gemma4:e2b', 'qwen2.5:7b'], + ollamaReachable: true, + }; } if (cmd === 'set_active_model') { rejectionSeen = true; @@ -444,6 +610,7 @@ describe('App', () => { get_model_picker_state: { active: 'gemma4:e2b', all: ['gemma4:e2b', 'qwen2.5:7b'], + ollamaReachable: true, }, }); render(); @@ -1096,6 +1263,7 @@ describe('App', () => { get_model_picker_state: { active: 'gemma4:e2b', all: ['gemma4:e2b'], + ollamaReachable: true, }, list_conversations: [], }); @@ -1522,7 +1690,11 @@ describe('App', () => { it('handleSaveAndNew aborts reset when save fails', async () => { invoke.mockImplementation(async (cmd: string) => { if (cmd === 'get_model_picker_state') - return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; + return { + active: 'gemma4:e2b', + all: ['gemma4:e2b'], + ollamaReachable: true, + }; if (cmd === 'list_conversations') return []; if (cmd === 'save_conversation') throw new Error('disk full'); }); @@ -1640,7 +1812,11 @@ describe('App', () => { // and could overwrite the current session with an unrelated conversation. invoke.mockImplementation(async (cmd: string) => { if (cmd === 'get_model_picker_state') - return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; + return { + active: 'gemma4:e2b', + all: ['gemma4:e2b'], + ollamaReachable: true, + }; if (cmd === 'list_conversations') return [ { @@ -1996,7 +2172,11 @@ describe('App', () => { // loadConversation() throws, leaving the panel open on failure. invoke.mockImplementation(async (cmd: string) => { if (cmd === 'get_model_picker_state') - return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; + return { + active: 'gemma4:e2b', + all: ['gemma4:e2b'], + ollamaReachable: true, + }; if (cmd === 'list_conversations') return [ { @@ -2270,7 +2450,11 @@ describe('App', () => { it('handleImagesAttached removes image when backend fails', async () => { invoke.mockImplementation(async (cmd: string) => { if (cmd === 'get_model_picker_state') - return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; + return { + active: 'gemma4:e2b', + all: ['gemma4:e2b'], + ollamaReachable: true, + }; if (cmd === 'save_image_command') throw new Error('disk full'); }); @@ -2320,7 +2504,11 @@ describe('App', () => { invoke.mockImplementation( async (cmd: string, args?: Record) => { if (cmd === 'get_model_picker_state') - return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; + return { + active: 'gemma4:e2b', + all: ['gemma4:e2b'], + ollamaReachable: true, + }; if (args && 'onEvent' in args) { // channel capture - no-op for this test } @@ -2692,7 +2880,11 @@ describe('App', () => { invoke.mockImplementation( async (cmd: string, args?: Record) => { if (cmd === 'get_model_picker_state') - return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; + return { + active: 'gemma4:e2b', + all: ['gemma4:e2b'], + ollamaReachable: true, + }; if (args && 'onEvent' in args) { // channel capture } @@ -2798,7 +2990,11 @@ describe('App', () => { invoke.mockImplementation( async (cmd: string, args?: Record) => { if (cmd === 'get_model_picker_state') - return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; + return { + active: 'gemma4:e2b', + all: ['gemma4:e2b'], + ollamaReachable: true, + }; if (args && 'onEvent' in args) { // channel capture - no-op } @@ -2858,7 +3054,11 @@ describe('App', () => { invoke.mockImplementation( async (cmd: string, args?: Record) => { if (cmd === 'get_model_picker_state') - return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; + return { + active: 'gemma4:e2b', + all: ['gemma4:e2b'], + ollamaReachable: true, + }; if (args && 'onEvent' in args) { // Accept channel for ask_ollama } @@ -2955,7 +3155,11 @@ describe('App', () => { invoke.mockImplementation(async (cmd: string) => { if (cmd === 'get_model_picker_state') - return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; + return { + active: 'gemma4:e2b', + all: ['gemma4:e2b'], + ollamaReachable: true, + }; if (cmd === 'search_pipeline') { return new Promise((res) => { resolveSearch = res; @@ -3024,7 +3228,11 @@ describe('App', () => { invoke.mockImplementation( async (cmd: string, args?: Record) => { if (cmd === 'get_model_picker_state') - return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; + return { + active: 'gemma4:e2b', + all: ['gemma4:e2b'], + ollamaReachable: true, + }; if (args && 'onEvent' in args) { // Accept channel } @@ -3100,7 +3308,11 @@ describe('App', () => { invoke.mockImplementation( async (cmd: string, args?: Record) => { if (cmd === 'get_model_picker_state') - return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; + return { + active: 'gemma4:e2b', + all: ['gemma4:e2b'], + ollamaReachable: true, + }; if (args && 'onEvent' in args) { // Accept channel } @@ -3170,7 +3382,11 @@ describe('App', () => { invoke.mockImplementation( async (cmd: string, args?: Record) => { if (cmd === 'get_model_picker_state') - return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; + return { + active: 'gemma4:e2b', + all: ['gemma4:e2b'], + ollamaReachable: true, + }; if (args && 'onEvent' in args) { // channel capture } @@ -3280,6 +3496,7 @@ describe('App', () => { get_model_picker_state: { active: 'llama3', all: ['llama3', 'llama3.2-vision'], + ollamaReachable: true, }, get_model_capabilities: { llama3: { @@ -3312,6 +3529,7 @@ describe('App', () => { get_model_picker_state: { active: 'llama3', all: ['llama3'], + ollamaReachable: true, }, get_model_capabilities: { llama3: { @@ -3365,7 +3583,11 @@ describe('App', () => { vi.useFakeTimers(); try { enableChannelCaptureWithResponses({ - get_model_picker_state: { active: 'llama3', all: ['llama3'] }, + get_model_picker_state: { + active: 'llama3', + all: ['llama3'], + ollamaReachable: true, + }, get_model_capabilities: { llama3: { vision: false, @@ -3416,6 +3638,7 @@ describe('App', () => { get_model_picker_state: { active: 'llama3.2-vision', all: ['llama3.2-vision'], + ollamaReachable: true, }, get_model_capabilities: { 'llama3.2-vision': { @@ -3759,7 +3982,11 @@ describe('App', () => { it('does not call ask when capture_full_screen_command throws', async () => { invoke.mockImplementation(async (cmd: string) => { if (cmd === 'get_model_picker_state') - return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; + return { + active: 'gemma4:e2b', + all: ['gemma4:e2b'], + ollamaReachable: true, + }; if (cmd === 'capture_full_screen_command') { throw new Error('Permission denied'); } @@ -3791,7 +4018,11 @@ describe('App', () => { it('surfaces string errors from Tauri invoke directly', async () => { invoke.mockImplementation(async (cmd: string) => { if (cmd === 'get_model_picker_state') - return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; + return { + active: 'gemma4:e2b', + all: ['gemma4:e2b'], + ollamaReachable: true, + }; if (cmd === 'capture_full_screen_command') { // Tauri v2 rejects with the Err(String) value as a plain string. return Promise.reject( @@ -3825,7 +4056,11 @@ describe('App', () => { it('handles non-Error non-string rejection values', async () => { invoke.mockImplementation(async (cmd: string) => { if (cmd === 'get_model_picker_state') - return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; + return { + active: 'gemma4:e2b', + all: ['gemma4:e2b'], + ollamaReachable: true, + }; if (cmd === 'capture_full_screen_command') { return Promise.reject(42); } @@ -3851,7 +4086,11 @@ describe('App', () => { enableChannelCapture(); invoke.mockImplementation(async (cmd: string) => { if (cmd === 'get_model_picker_state') - return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; + return { + active: 'gemma4:e2b', + all: ['gemma4:e2b'], + ollamaReachable: true, + }; if (cmd === 'capture_full_screen_command') { throw new Error('capture failed'); } @@ -4019,7 +4258,11 @@ describe('App', () => { it('restores query with cleanQuery text when capture fails mid-message', async () => { invoke.mockImplementation(async (cmd: string) => { if (cmd === 'get_model_picker_state') - return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; + return { + active: 'gemma4:e2b', + all: ['gemma4:e2b'], + ollamaReachable: true, + }; if (cmd === 'capture_full_screen_command') { throw new Error('Screen capture timed out'); } @@ -4701,7 +4944,11 @@ describe('App', () => { invoke.mockImplementation( async (cmd: string, args?: Record) => { if (cmd === 'get_model_picker_state') - return { active: 'gemma4:e2b', all: ['gemma4:e2b'] }; + return { + active: 'gemma4:e2b', + all: ['gemma4:e2b'], + ollamaReachable: true, + }; if (args && 'onEvent' in args) { // Accept channel for ask_ollama } diff --git a/src/components/ModelPickerPanel.tsx b/src/components/ModelPickerPanel.tsx index a0ba50f2..ad223c4e 100644 --- a/src/components/ModelPickerPanel.tsx +++ b/src/components/ModelPickerPanel.tsx @@ -56,8 +56,12 @@ export function formatCapabilityLabel( export interface ModelPickerPanelProps { /** Full list of available model slugs. */ models: string[]; - /** Currently active model slug. */ - activeModel: string; + /** + * Currently active model slug, or `null` when nothing is selected. The + * panel handles `null` by simply rendering no row as active; it never + * tries to invent a default from an empty `models` list. + */ + activeModel: string | null; /** Called with the chosen slug when the user clicks or keyboard-selects a row. */ onSelect: (model: string) => void; /** @@ -221,8 +225,13 @@ export function ModelPickerPanel({ className="overflow-y-auto py-1 max-h-[280px]" > {models.length === 0 ? ( -

- No models available. +

+ No models installed. Run{' '} + ollama pull <model>{' '} + in your terminal, then come back.

) : filtered.length === 0 ? (

diff --git a/src/components/__tests__/ModelPickerPanel.test.tsx b/src/components/__tests__/ModelPickerPanel.test.tsx index 7eb3a394..9be7d796 100644 --- a/src/components/__tests__/ModelPickerPanel.test.tsx +++ b/src/components/__tests__/ModelPickerPanel.test.tsx @@ -105,12 +105,29 @@ describe('ModelPickerPanel', () => { } }); - it('shows no-models-available message when models list is empty', () => { + it('shows the ollama-pull empty state when models list is empty', () => { + // Empty state copy mirrors S2 in `getEnvironmentMessage`: route the + // user to `ollama pull ` rather than implying anything is + // wrong with the picker itself. renderPanel({ models: [] }); - expect(screen.getByText(/no models available/i)).toBeInTheDocument(); + const empty = screen.getByTestId('model-picker-empty'); + expect(empty).toBeInTheDocument(); + expect(empty.textContent).toContain('No models installed'); + expect(empty.textContent).toContain('ollama pull '); expect(screen.queryByRole('option')).toBeNull(); }); + it('renders no row as active when activeModel is null', () => { + // S2/S3: the chip stays clickable with a null active model. The panel + // must accept null without inventing a default and simply mark no row + // as aria-selected. + renderPanel({ activeModel: null as unknown as string }); + for (const model of MODELS) { + const option = screen.getByRole('option', { name: model }); + expect(option).toHaveAttribute('aria-selected', 'false'); + } + }); + it('marks the filter input as an aria-activedescendant combobox', () => { renderPanel(); const input = screen.getByPlaceholderText(/filter models/i); diff --git a/src/hooks/__tests__/useModelSelection.test.tsx b/src/hooks/__tests__/useModelSelection.test.tsx index bb0b7d41..9e88b7ed 100644 --- a/src/hooks/__tests__/useModelSelection.test.tsx +++ b/src/hooks/__tests__/useModelSelection.test.tsx @@ -12,6 +12,7 @@ describe('useModelSelection', () => { invoke.mockResolvedValueOnce({ active: 'gemma4:e2b', all: ['gemma4:e2b', 'qwen2.5:7b'], + ollamaReachable: true, }); const { result } = renderHook(() => useModelSelection()); @@ -22,6 +23,7 @@ describe('useModelSelection', () => { 'gemma4:e2b', 'qwen2.5:7b', ]); + expect(result.current.ollamaReachable).toBe(true); }); it('starts with a null active model before the first refresh resolves', () => { @@ -29,19 +31,46 @@ describe('useModelSelection', () => { const { result } = renderHook(() => useModelSelection()); expect(result.current.activeModel).toBeNull(); expect(result.current.availableModels).toEqual([]); + // Optimistic default avoids a cold-start flash of the unreachable strip + // while the first picker fetch is in flight. + expect(result.current.ollamaReachable).toBe(true); }); it('accepts a null active payload from the backend (no model selected)', async () => { // Ollama is the single source of truth: when nothing is installed and - // nothing is persisted, the backend returns active: null. The hook - // must surface that as null so consumers can route the user to the picker. - invoke.mockResolvedValueOnce({ active: null, all: [] }); + // nothing is persisted, the backend returns active: null with the + // reachable flag still true so the strip points at "pull a model" + // instead of "start Ollama". + invoke.mockResolvedValueOnce({ + active: null, + all: [], + ollamaReachable: true, + }); const { result } = renderHook(() => useModelSelection()); await act(async () => {}); expect(result.current.activeModel).toBeNull(); expect(result.current.availableModels).toEqual([]); + expect(result.current.ollamaReachable).toBe(true); + }); + + it('marks Ollama unreachable when the backend reports it cannot connect', async () => { + // S1: backend collapses a transport failure into a structured payload + // so the hook can surface ollamaReachable=false without parsing error + // strings. + invoke.mockResolvedValueOnce({ + active: null, + all: [], + ollamaReachable: false, + }); + + const { result } = renderHook(() => useModelSelection()); + await act(async () => {}); + + expect(result.current.activeModel).toBeNull(); + expect(result.current.availableModels).toEqual([]); + expect(result.current.ollamaReachable).toBe(false); }); it('persists a new active model and updates local state', async () => { @@ -49,6 +78,7 @@ describe('useModelSelection', () => { .mockResolvedValueOnce({ active: 'gemma4:e2b', all: ['gemma4:e2b', 'qwen2.5:7b'], + ollamaReachable: true, }) .mockResolvedValueOnce(undefined); @@ -65,7 +95,7 @@ describe('useModelSelection', () => { expect(result.current.activeModel).toBe('qwen2.5:7b'); }); - it('clears available models when backend fetch fails', async () => { + it('clears available models and marks unreachable when backend fetch rejects', async () => { invoke.mockRejectedValueOnce(new Error('backend offline')); const { result } = renderHook(() => useModelSelection()); @@ -73,6 +103,9 @@ describe('useModelSelection', () => { expect(result.current.availableModels).toEqual([]); expect(result.current.activeModel).toBeNull(); + // A rejected IPC call is treated as unreachable: we cannot trust any + // field, so route the user toward starting Ollama rather than pulling. + expect(result.current.ollamaReachable).toBe(false); }); it('falls back to empty state when payload shape is invalid', async () => { @@ -83,14 +116,23 @@ describe('useModelSelection', () => { expect(result.current.availableModels).toEqual([]); expect(result.current.activeModel).toBeNull(); + // A malformed payload is also treated as unreachable: the hook cannot + // tell whether the daemon is healthy, so the safe default mirrors the + // rejection branch. + expect(result.current.ollamaReachable).toBe(false); }); it('re-fetches models when refreshModels is called', async () => { invoke - .mockResolvedValueOnce({ active: 'gemma4:e2b', all: ['gemma4:e2b'] }) + .mockResolvedValueOnce({ + active: 'gemma4:e2b', + all: ['gemma4:e2b'], + ollamaReachable: true, + }) .mockResolvedValueOnce({ active: 'qwen2.5:7b', all: ['gemma4:e2b', 'qwen2.5:7b'], + ollamaReachable: true, }); const { result } = renderHook(() => useModelSelection()); @@ -128,13 +170,35 @@ describe('useModelSelection', () => { }); it('rejects payloads whose `all` array contains non-string entries', async () => { - invoke.mockResolvedValueOnce({ active: 'gemma4:e2b', all: ['ok', 7] }); + invoke.mockResolvedValueOnce({ + active: 'gemma4:e2b', + all: ['ok', 7], + ollamaReachable: true, + }); + + const { result } = renderHook(() => useModelSelection()); + await act(async () => {}); + + expect(result.current.availableModels).toEqual([]); + expect(result.current.activeModel).toBeNull(); + }); + + it('rejects payloads with non-boolean ollamaReachable', async () => { + // Defense-in-depth: the backend always emits a boolean, but the guard + // keeps the hook robust against shape drift in legacy builds or + // mocks. + invoke.mockResolvedValueOnce({ + active: 'gemma4:e2b', + all: ['gemma4:e2b'], + ollamaReachable: 'yes', + }); const { result } = renderHook(() => useModelSelection()); await act(async () => {}); expect(result.current.availableModels).toEqual([]); expect(result.current.activeModel).toBeNull(); + expect(result.current.ollamaReachable).toBe(false); }); it('surfaces backend errors and leaves active model unchanged on rejection', async () => { @@ -142,6 +206,7 @@ describe('useModelSelection', () => { .mockResolvedValueOnce({ active: 'gemma4:e2b', all: ['gemma4:e2b', 'qwen2.5:7b'], + ollamaReachable: true, }) .mockRejectedValueOnce( new Error('Model is not installed in Ollama: mystery'), @@ -164,6 +229,7 @@ describe('useModelSelection', () => { .mockResolvedValueOnce({ active: 'gemma4:e2b', all: ['gemma4:e2b', 'qwen2.5:7b'], + ollamaReachable: true, }) .mockResolvedValueOnce({ active: 42, all: 'not-an-array' }); @@ -183,6 +249,7 @@ describe('useModelSelection', () => { invoke.mockResolvedValueOnce({ active: 'A', all: ['A', 'B', 'C'], + ollamaReachable: true, }); const { result } = renderHook(() => useModelSelection()); @@ -220,6 +287,7 @@ describe('useModelSelection', () => { invoke.mockResolvedValueOnce({ active: 'A', all: ['A', 'B', 'C'], + ollamaReachable: true, }); const { result } = renderHook(() => useModelSelection()); @@ -267,7 +335,7 @@ describe('useModelSelection', () => { // Resolving after unmount would setState on an unmounted component without // the mounted guard, producing a React warning / test failure. await act(async () => { - resolveLate({ active: 'A', all: ['A'] }); + resolveLate({ active: 'A', all: ['A'], ollamaReachable: true }); }); }); diff --git a/src/hooks/useModelSelection.ts b/src/hooks/useModelSelection.ts index 890a96e4..610db442 100644 --- a/src/hooks/useModelSelection.ts +++ b/src/hooks/useModelSelection.ts @@ -11,13 +11,18 @@ import type { ModelPickerState } from '../types/model'; */ function isModelPickerState(value: unknown): value is ModelPickerState { if (typeof value !== 'object' || value === null) return false; - const candidate = value as { active?: unknown; all?: unknown }; + const candidate = value as { + active?: unknown; + all?: unknown; + ollamaReachable?: unknown; + }; const activeOk = candidate.active === null || typeof candidate.active === 'string'; return ( activeOk && Array.isArray(candidate.all) && - candidate.all.every((entry) => typeof entry === 'string') + candidate.all.every((entry) => typeof entry === 'string') && + typeof candidate.ollamaReachable === 'boolean' ); } @@ -34,6 +39,15 @@ export interface UseModelSelectionResult { activeModel: string | null; /** All locally installed Ollama model names available for selection. */ availableModels: string[]; + /** + * Whether the most recent backend call reached the local Ollama daemon. + * `true` is the optimistic default before the first fetch resolves so the + * UI does not flash an "Ollama is down" strip during cold start. Set to + * `false` only when the backend explicitly reports unreachability or the + * IPC call itself rejects, so the strip can route the user to "start + * Ollama" instead of "pull a model". + */ + ollamaReachable: boolean; /** * Re-fetch the model picker state from the backend. Sets `activeModel` to * `null` and clears `availableModels` when the backend returns a malformed @@ -66,6 +80,10 @@ export function useModelSelection(): UseModelSelectionResult { // eslint-disable-next-line @eslint-react/use-state const [activeModel, setActiveModelState] = useState(null); const [availableModels, setAvailableModels] = useState([]); + // Optimistic default: assume reachable until the first fetch tells us + // otherwise. This prevents a cold-start flash of the "Ollama is down" + // strip while the IPC call is in flight. + const [ollamaReachable, setOllamaReachable] = useState(true); const mountedRef = useRef(true); const latestTokenRef = useRef(0); @@ -88,16 +106,22 @@ export function useModelSelection(): UseModelSelectionResult { const state = await invoke('get_model_picker_state'); if (!isLatest(token)) return; if (!isModelPickerState(state)) { + // Treat malformed payloads as a transport failure: we cannot trust + // any field, so fall back to the no-model state and assume Ollama + // is unreachable so the strip nudges the user toward starting it. setActiveModelState(null); setAvailableModels([]); + setOllamaReachable(false); return; } setActiveModelState(state.active); setAvailableModels(state.all); + setOllamaReachable(state.ollamaReachable); } catch { if (!isLatest(token)) return; setActiveModelState(null); setAvailableModels([]); + setOllamaReachable(false); } }, [isLatest]); @@ -124,5 +148,11 @@ export function useModelSelection(): UseModelSelectionResult { [isLatest], ); - return { activeModel, availableModels, refreshModels, setActiveModel }; + return { + activeModel, + availableModels, + ollamaReachable, + refreshModels, + setActiveModel, + }; } diff --git a/src/testUtils/mocks/tauri.ts b/src/testUtils/mocks/tauri.ts index 4638738b..725f3e36 100644 --- a/src/testUtils/mocks/tauri.ts +++ b/src/testUtils/mocks/tauri.ts @@ -23,11 +23,13 @@ export const invoke = vi.fn< /** * Default model-picker state used by tests that do not opt into a specific * inventory. Tests that need the no-model state should mock - * `get_model_picker_state` to `{ active: null, all: [] }` explicitly. + * `get_model_picker_state` to `{ active: null, all: [], ollamaReachable: true }` + * (S2) or `{ active: null, all: [], ollamaReachable: false }` (S1) explicitly. */ export const TEST_DEFAULT_MODEL_PICKER_STATE = { active: 'gemma4:e2b', all: ['gemma4:e2b'], + ollamaReachable: true, } as const; /** @@ -57,6 +59,7 @@ export function getLastChannel(): Channel | null { const DEFAULT_MODEL_PICKER_STATE = { active: 'gemma4:e2b', all: ['gemma4:e2b'], + ollamaReachable: true, } as const; /** diff --git a/src/types/model.ts b/src/types/model.ts index 440f12c6..cdd0c8fa 100644 --- a/src/types/model.ts +++ b/src/types/model.ts @@ -15,6 +15,14 @@ export interface ModelPickerState { active: string | null; /** All locally installed Ollama model names available for selection. */ all: string[]; + /** + * Whether the Rust backend successfully reached the local Ollama daemon + * during the last picker fetch. False when `/api/tags` errored (connection + * refused, timeout, DNS failure, port closed). The frontend uses this to + * distinguish "Ollama is down" from "Ollama is up but has no models" and + * to pick the correct recovery copy in `CapabilityMismatchStrip`. + */ + ollamaReachable: boolean; } /** diff --git a/src/utils/__tests__/capabilityConflicts.test.ts b/src/utils/__tests__/capabilityConflicts.test.ts index 624a89c7..d9d0c7eb 100644 --- a/src/utils/__tests__/capabilityConflicts.test.ts +++ b/src/utils/__tests__/capabilityConflicts.test.ts @@ -1,5 +1,10 @@ import { describe, it, expect } from 'vitest'; -import { getCapabilityConflict } from '../capabilityConflicts'; +import { + getCapabilityConflict, + getEnvironmentMessage, + NO_MODELS_INSTALLED_MESSAGE, + OLLAMA_UNREACHABLE_MESSAGE, +} from '../capabilityConflicts'; import type { ModelCapabilities } from '../../types/model'; import type { ComposeCapabilityState } from '../capabilityConflicts'; @@ -88,47 +93,31 @@ describe('getCapabilityConflict', () => { expect(result).toContain('reads text only'); }); - it('returns the no-model message when modelName is empty', () => { - // Ollama's /api/tags is the single source of truth for the active - // model. An empty name short-circuits to the picker prompt regardless - // of compose state: no model means no submit can succeed. + it('returns null when modelName is empty so the env-state helper can take over', () => { + // Environment-state messaging now lives in `getEnvironmentMessage`. + // The capability helper defers rather than emit a stale "pick a model" + // copy that would not know whether Ollama is reachable. const result = getCapabilityConflict('', TEXT_ONLY, { ...EMPTY, imageCount: 1, }); - expect(result).toBe( - 'Thuki needs a model to think with. Pull one in Ollama and tap the picker chip above to wire it up.', - ); + expect(result).toBeNull(); }); - it('returns the no-model message when modelName is null', () => { + it('returns null when modelName is null', () => { const result = getCapabilityConflict(null, TEXT_ONLY, { ...EMPTY, imageCount: 1, }); - expect(result).toBe( - 'Thuki needs a model to think with. Pull one in Ollama and tap the picker chip above to wire it up.', - ); + expect(result).toBeNull(); }); - it('returns the no-model message when modelName is undefined', () => { + it('returns null when modelName is undefined', () => { const result = getCapabilityConflict(undefined, TEXT_ONLY, { ...EMPTY, imageCount: 1, }); - expect(result).toBe( - 'Thuki needs a model to think with. Pull one in Ollama and tap the picker chip above to wire it up.', - ); - }); - - it('returns the no-model message even when compose state is empty', () => { - // The strip needs to fire as soon as the user opens the overlay with - // nothing installed, before they type anything. Empty compose is the - // default state and must surface the "pick a model" copy. - const result = getCapabilityConflict(null, undefined, EMPTY); - expect(result).toBe( - 'Thuki needs a model to think with. Pull one in Ollama and tap the picker chip above to wire it up.', - ); + expect(result).toBeNull(); }); // ── max-images gate ─────────────────────────────────────────────────────── @@ -226,17 +215,15 @@ describe('getCapabilityConflict', () => { expect(result).toBeNull(); }); - it('surfaces the no-model message when name is empty even with /think queued', () => { - // The no-model gate runs before any capability check, so an empty name - // short-circuits to the picker prompt regardless of which command is - // queued. The /think mismatch copy never reaches the user. + it('returns null when name is empty even with /think queued', () => { + // Empty name still short-circuits to null so the env-state helper + // owns the messaging. The /think mismatch copy is meaningless without + // a real model anyway. const result = getCapabilityConflict('', TEXT_ONLY, { ...EMPTY, hasThinkCommand: true, }); - expect(result).toBe( - 'Thuki needs a model to think with. Pull one in Ollama and tap the picker chip above to wire it up.', - ); + expect(result).toBeNull(); }); it('prefers the vision message when /think and images both mismatch', () => { @@ -273,3 +260,48 @@ describe('getCapabilityConflict', () => { expect(result).toBeNull(); }); }); + +describe('getEnvironmentMessage', () => { + it('returns the unreachable copy when Ollama cannot be reached (S1)', () => { + // S1: connection refused / timeout / DNS failure. Even if the + // installedCount and activeModel happen to be non-empty (stale state + // from a prior fetch), reachability is the dominant constraint. + expect(getEnvironmentMessage(false, 0, null)).toBe( + OLLAMA_UNREACHABLE_MESSAGE, + ); + }); + + it('returns the unreachable copy even with stale active/installed values', () => { + expect(getEnvironmentMessage(false, 3, 'gemma4:e4b')).toBe( + OLLAMA_UNREACHABLE_MESSAGE, + ); + }); + + it('returns the no-models copy when reachable but installed list is empty (S2)', () => { + expect(getEnvironmentMessage(true, 0, null)).toBe( + NO_MODELS_INSTALLED_MESSAGE, + ); + }); + + it('returns the pick-a-model copy when reachable, models present, none active (S3)', () => { + // S3 is the rare post-Phase-A defensive state. Backend auto-picks the + // first installed model on launch, but if a payload drift ever lands + // here we still surface a clear recovery cue instead of falling + // through to the capability helper with a null model. + const result = getEnvironmentMessage(true, 2, null); + expect(result).toBe('Pick a model from the chip above to start chatting.'); + }); + + it('returns null when an active model is set so per-message gates can run (S4)', () => { + expect(getEnvironmentMessage(true, 2, 'gemma4:e4b')).toBeNull(); + }); + + it('returns the pick-a-model copy when activeModel is the empty string', () => { + // Empty string is treated as "no active model" so the strip surfaces + // the recovery cue rather than letting the capability helper pretend + // the empty slug is a real selection. + expect(getEnvironmentMessage(true, 1, '')).toBe( + 'Pick a model from the chip above to start chatting.', + ); + }); +}); diff --git a/src/utils/capabilityConflicts.ts b/src/utils/capabilityConflicts.ts index 553b122c..ba2474a9 100644 --- a/src/utils/capabilityConflicts.ts +++ b/src/utils/capabilityConflicts.ts @@ -24,6 +24,54 @@ export interface ComposeCapabilityState { imageCount: number; } +/** + * Copy used when Ollama is reachable but the user has no models installed. + * Exported so tests can match it without duplicating the prose, and so + * App.tsx can route through one symbol per state. + */ +export const NO_MODELS_INSTALLED_MESSAGE = + 'Thuki needs a model to think with. Pull one in Ollama with `ollama pull `, then come back.'; + +/** + * Copy used when the local Ollama daemon cannot be reached (connection + * refused, timeout, port closed). The recovery action is "start Ollama", + * not "pull a model": telling the user to pull when the daemon is down + * sends them down the wrong rabbit hole. + */ +export const OLLAMA_UNREACHABLE_MESSAGE = + "Ollama isn't running. Start Ollama and try again."; + +/** + * Picks the right environment-state message to render in + * `CapabilityMismatchStrip`, or returns `null` when the environment is + * healthy enough that a per-message capability gate should run instead. + * + * Three states are distinguished so the strip never tells the user to + * "pull a model" when the actual problem is that Ollama is down: + * + * - S1: Ollama unreachable. Returns the unreachable copy regardless of + * `installedCount` or `activeModel` because we cannot trust either. + * - S2: Ollama reachable, zero models installed. Returns the no-models copy. + * - S3: Ollama reachable, models installed, none active. Returns the + * pick-a-model copy. This state is rare post-Phase-A because the backend + * auto-picks on first launch, but the strip handles it defensively. + * + * Returns `null` once a model is actually active so callers fall through + * to the per-message capability check. + */ +export function getEnvironmentMessage( + ollamaReachable: boolean, + installedCount: number, + activeModel: string | null | undefined, +): string | null { + if (!ollamaReachable) return OLLAMA_UNREACHABLE_MESSAGE; + if (installedCount === 0) return NO_MODELS_INSTALLED_MESSAGE; + if (!activeModel) { + return 'Pick a model from the chip above to start chatting.'; + } + return null; +} + /** * Returns a single human-readable reason why the active model cannot * send the current compose state, or `null` if the message is sendable. @@ -31,11 +79,11 @@ export interface ComposeCapabilityState { * The strip and the submit-time toast both render the returned string * verbatim so the wording lives in exactly one place. * - * Empty / null / undefined `modelName` short-circuits to a "pick a model" - * message regardless of compose state: Ollama's `/api/tags` is the single - * source of truth and the user must select a model before any submit can - * succeed. Capabilities-aware checks below only run once a model is - * actually selected. + * This helper is only meaningful once a model is actually active. + * Empty / null / undefined `modelName` short-circuits to `null` so the + * caller can fall back to {@link getEnvironmentMessage} for the right + * "Ollama is down / pull a model / pick a model" copy. Capabilities-aware + * checks below only run once a model is actually selected. * * For a selected model with unknown capabilities (not yet fetched, or * fetch failed) the gate is permissive and returns `null` so the user is @@ -48,7 +96,10 @@ export function getCapabilityConflict( state: ComposeCapabilityState, ): string | null { if (!modelName) { - return 'Thuki needs a model to think with. Pull one in Ollama and tap the picker chip above to wire it up.'; + // Environment-state messaging lives in `getEnvironmentMessage`. This + // helper has no insight into Ollama reachability or installed count, + // so the safe behavior is to defer rather than emit a stale copy. + return null; } const needsVision = state.imageCount > 0 || state.hasScreenCommand; const needsThinking = state.hasThinkCommand; From 93f889050745c714207e4bebeda03d4b3f5e4f6f Mon Sep 17 00:00:00 2001 From: Logan Nguyen Date: Sun, 26 Apr 2026 15:58:02 -0500 Subject: [PATCH 3/4] fix(inference): refine no-model UX from smoke-test feedback - Relax compose-mode chip gate so the picker chip stays visible whenever Ollama is reachable, even with zero models or no active selection. Match chat-mode by routing onModelPickerToggle through ollamaReachable. - Drop the duplicate capability-conflict toast on submit-gate. The persistent strip already surfaces the message; the transient toast added visual clutter and overflowed the window in compose mode. Removed the now-unused Toast component and its tests. - Update no-models strip copy: "Thuki couldn't find any local LLM models. Pull one from Ollama with ollama pull , then come back." Signed-off-by: Logan Nguyen --- src/App.tsx | 29 ++------- src/__tests__/App.test.tsx | 77 +++++------------------ src/components/Toast.tsx | 61 ------------------- src/components/__tests__/Toast.test.tsx | 81 ------------------------- src/utils/capabilityConflicts.ts | 2 +- src/view/AskBarView.tsx | 25 ++++---- src/view/__tests__/AskBarView.test.tsx | 36 +++++++---- 7 files changed, 56 insertions(+), 255 deletions(-) delete mode 100644 src/components/Toast.tsx delete mode 100644 src/components/__tests__/Toast.test.tsx diff --git a/src/App.tsx b/src/App.tsx index 39228526..03859a83 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -21,7 +21,6 @@ import { getCapabilityConflict, getEnvironmentMessage, } from './utils/capabilityConflicts'; -import { Toast } from './components/Toast'; import { ConversationView } from './view/ConversationView'; import { AskBarView, MAX_IMAGES } from './view/AskBarView'; import { OnboardingView } from './view/onboarding/index'; @@ -151,14 +150,6 @@ function App() { ? modelCapabilities[activeModel] : undefined; - /** - * Toast text shown by the submit-time capability gate. Set to a non-null - * string when the user attempts to send a message whose attached content - * the active model cannot handle (e.g. images on a text-only model). - * Cleared by the toast's auto-dismiss or on next submit attempt. - */ - const [capabilityToast, setCapabilityToast] = useState(null); - /** * Pulses true to trigger the ask-bar shake animation when the * submit-time gate refuses a message, then resets so the next blocked @@ -174,10 +165,6 @@ function App() { return () => clearTimeout(timer); }, [shakeAskBar]); - const dismissCapabilityToast = useCallback(() => { - setCapabilityToast(null); - }, []); - const { conversationId, isSaved, @@ -1141,11 +1128,11 @@ function App() { // the active model cannot handle (images on a text-only model). The // gate is the only gate: input affordances stay live so the user can // compose freely and recover via the model picker chip. When refused - // the ask bar shakes and a toast surfaces the reason. Compose state is - // preserved so the user does not lose their typing. + // the ask bar shakes; the persistent capability strip already surfaces + // the reason so we do not duplicate it in a transient toast. Compose + // state is preserved so the user does not lose their typing. if (liveCapabilityConflictMessage !== null) { setShakeAskBar(true); - setCapabilityToast(liveCapabilityConflictMessage); return; } @@ -1789,17 +1776,13 @@ function App() { onImagePreview={handleAskBarImagePreview} onScreenshot={handleScreenshot} isDragOver={isDragOver ?? undefined} - activeModel={activeModel} - availableModels={availableModels} - onModelPickerToggle={handleModelPickerToggle} + onModelPickerToggle={ + ollamaReachable ? handleModelPickerToggle : undefined + } isModelPickerOpen={isModelPickerOpen} capabilityConflictMessage={liveCapabilityConflictMessage} shake={shakeAskBar} /> - {/* Chat-mode model picker dropdown - floating card identical in style diff --git a/src/__tests__/App.test.tsx b/src/__tests__/App.test.tsx index 92d4d427..4ccc3715 100644 --- a/src/__tests__/App.test.tsx +++ b/src/__tests__/App.test.tsx @@ -209,7 +209,9 @@ describe('App', () => { await showOverlay(); const strip = screen.getByTestId('capability-mismatch-strip'); - expect(strip.textContent).toContain('Pull one in Ollama'); + expect(strip.textContent).toContain( + "Thuki couldn't find any local LLM models", + ); expect(strip.textContent).toContain('ollama pull '); }); @@ -3524,7 +3526,7 @@ describe('App', () => { ); }); - it('refuses submit and surfaces a toast when a text-only model has an image attached', async () => { + it('refuses submit and shakes the ask bar when a text-only model has an image attached', async () => { enableChannelCaptureWithResponses({ get_model_picker_state: { active: 'llama3', @@ -3562,12 +3564,11 @@ describe('App', () => { fireEvent.click(screen.getByRole('button', { name: /send message/i })); }); - // Toast surfaces the reason. - await vi.waitFor(() => { - expect(screen.getByTestId('toast')).toHaveTextContent( - 'llama3 reads text only', - ); - }); + // Capability strip remains the single surface for the conflict + // message; the duplicate transient toast was removed. + expect(screen.getByTestId('capability-mismatch-strip')).toHaveTextContent( + 'llama3 reads text only', + ); // ask_ollama is NOT invoked. const askInvocations = invoke.mock.calls.filter( (call) => call[0] === 'ask_ollama', @@ -3577,60 +3578,12 @@ describe('App', () => { expect(screen.getByPlaceholderText('Ask Thuki anything...')).toHaveValue( 'summarise these', ); - }); - - it('toast auto-dismisses after the default duration', async () => { - vi.useFakeTimers(); - try { - enableChannelCaptureWithResponses({ - get_model_picker_state: { - active: 'llama3', - all: ['llama3'], - ollamaReachable: true, - }, - get_model_capabilities: { - llama3: { - vision: false, - thinking: false, - }, - }, - save_image_command: '/tmp/staged/img1.jpg', - }); - render(); - await act(async () => {}); - await showOverlay(); - // Paste (real timers were running; pasteImage uses waitFor which - // works under fake timers if we advance them). - const textarea = screen.getByPlaceholderText('Ask Thuki anything...'); - const file = new File(['x'], 'p.png', { type: 'image/png' }); - await act(async () => { - fireEvent.paste(textarea, { - clipboardData: { - items: [{ type: 'image/png', getAsFile: () => file }], - }, - }); - }); - await act(async () => { - await vi.advanceTimersByTimeAsync(50); - }); - // Submit with no text but an image (canSubmit honors images alone). - await act(async () => { - fireEvent.click( - screen.getByRole('button', { name: /send message/i }), - ); - }); - await act(async () => { - await vi.advanceTimersByTimeAsync(10); - }); - expect(screen.queryByTestId('toast')).not.toBeNull(); - // Advance past the 3000ms default duration. - await act(async () => { - await vi.advanceTimersByTimeAsync(3100); - }); - expect(screen.queryByTestId('toast')).toBeNull(); - } finally { - vi.useRealTimers(); - } + // Wait past the 600 ms shake reset so the cleanup runs and the + // shake state pulses back to false. This exercises the effect's + // setTimeout/clearTimeout path that the gate relies on. + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 650)); + }); }); it('does not gate submit when the active model has vision', async () => { diff --git a/src/components/Toast.tsx b/src/components/Toast.tsx deleted file mode 100644 index d2132125..00000000 --- a/src/components/Toast.tsx +++ /dev/null @@ -1,61 +0,0 @@ -import { useEffect } from 'react'; - -/** Props for the {@link Toast} component. */ -export interface ToastProps { - /** Body text shown in the toast. The toast renders only when truthy. */ - message: string | null; - /** Called when the auto-dismiss timer fires or the user closes the toast. */ - onDismiss: () => void; - /** Auto-dismiss delay in ms. Defaults to 3000. */ - durationMs?: number; -} - -const DEFAULT_DURATION_MS = 3000; - -/** - * Bottom-anchored transient toast used by the submit-time capability - * gate. Renders nothing when `message` is null. Schedules a single - * auto-dismiss timer per non-null `message` and clears it on unmount or - * before the next message replaces it. - * - * Positioning is `absolute` against the nearest positioned ancestor; the - * caller wraps it in a `relative` container to anchor. - */ -export function Toast({ - message, - onDismiss, - durationMs = DEFAULT_DURATION_MS, -}: ToastProps) { - useEffect(() => { - if (!message) return; - const timer = setTimeout(onDismiss, durationMs); - return () => clearTimeout(timer); - }, [message, onDismiss, durationMs]); - - if (!message) return null; - - return ( -

-
- ); -} diff --git a/src/components/__tests__/Toast.test.tsx b/src/components/__tests__/Toast.test.tsx deleted file mode 100644 index 1821d575..00000000 --- a/src/components/__tests__/Toast.test.tsx +++ /dev/null @@ -1,81 +0,0 @@ -import { render, screen, act } from '@testing-library/react'; -import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import { Toast } from '../Toast'; - -describe('Toast', () => { - beforeEach(() => { - vi.useFakeTimers(); - }); - afterEach(() => { - vi.useRealTimers(); - }); - - it('renders nothing when message is null', () => { - render( {}} />); - expect(screen.queryByTestId('toast')).toBeNull(); - }); - - it('renders the message when provided', () => { - render( {}} />); - expect(screen.getByTestId('toast')).toHaveTextContent('hello'); - }); - - it('auto-dismisses after the default 3000ms', () => { - const onDismiss = vi.fn(); - render(); - expect(onDismiss).not.toHaveBeenCalled(); - act(() => { - vi.advanceTimersByTime(3000); - }); - expect(onDismiss).toHaveBeenCalledTimes(1); - }); - - it('honors a custom durationMs', () => { - const onDismiss = vi.fn(); - render(); - act(() => { - vi.advanceTimersByTime(499); - }); - expect(onDismiss).not.toHaveBeenCalled(); - act(() => { - vi.advanceTimersByTime(1); - }); - expect(onDismiss).toHaveBeenCalledTimes(1); - }); - - it('clears the timer when message changes to null before timeout', () => { - const onDismiss = vi.fn(); - const { rerender } = render( - , - ); - act(() => { - vi.advanceTimersByTime(500); - }); - rerender(); - act(() => { - vi.advanceTimersByTime(2000); - }); - expect(onDismiss).not.toHaveBeenCalled(); - }); - - it('resets the timer when message changes to a new value', () => { - const onDismiss = vi.fn(); - const { rerender } = render( - , - ); - act(() => { - vi.advanceTimersByTime(900); - }); - rerender( - , - ); - act(() => { - vi.advanceTimersByTime(900); - }); - expect(onDismiss).not.toHaveBeenCalled(); - act(() => { - vi.advanceTimersByTime(200); - }); - expect(onDismiss).toHaveBeenCalledTimes(1); - }); -}); diff --git a/src/utils/capabilityConflicts.ts b/src/utils/capabilityConflicts.ts index ba2474a9..1a697321 100644 --- a/src/utils/capabilityConflicts.ts +++ b/src/utils/capabilityConflicts.ts @@ -30,7 +30,7 @@ export interface ComposeCapabilityState { * App.tsx can route through one symbol per state. */ export const NO_MODELS_INSTALLED_MESSAGE = - 'Thuki needs a model to think with. Pull one in Ollama with `ollama pull `, then come back.'; + "Thuki couldn't find any local LLM models. Pull one from Ollama with `ollama pull `, then come back."; /** * Copy used when the local Ollama daemon cannot be reached (connection diff --git a/src/view/AskBarView.tsx b/src/view/AskBarView.tsx index a4d74650..f1a705c2 100644 --- a/src/view/AskBarView.tsx +++ b/src/view/AskBarView.tsx @@ -189,13 +189,13 @@ interface AskBarViewProps { * "normal" = violet ring; "max" = red ring + label; undefined = no ring. */ isDragOver?: 'normal' | 'max'; - /** Currently active Ollama model slug, or `null` when none is selected. */ - activeModel?: string | null; - /** Full list of model slugs available for selection in the picker. */ - availableModels?: string[]; /** * Called when the user clicks the model picker trigger. App.tsx owns the * open/close state and renders the ModelPickerPanel as an inline drawer. + * In compose mode App.tsx gates this on `ollamaReachable`, so its presence + * doubles as the signal that Ollama is reachable: the chip stays visible + * even when there is no active model or zero installed models so the user + * can recover by opening the picker. */ onModelPickerToggle?: () => void; /** Whether the model picker panel is currently open (drives aria-expanded). */ @@ -288,8 +288,6 @@ export function AskBarView({ onImagePreview, onScreenshot, isDragOver, - activeModel, - availableModels, onModelPickerToggle, isModelPickerOpen, capabilityConflictMessage, @@ -330,15 +328,14 @@ export function AskBarView({ /** * Prerequisites for rendering the chip trigger in the input bar. - * Hidden in chat mode — the pill trigger moves to the WindowControls header. + * Hidden in chat mode (the pill trigger moves to the WindowControls + * header). The chip renders whenever the picker callback is wired up + * regardless of model state: with no active model it surfaces the + * "Pick a model" recovery affordance, and the caller is expected to + * omit `onModelPickerToggle` (Ollama unreachable) when the chip should + * stay hidden. */ - const modelPickerAvailable = Boolean( - !isChatMode && - activeModel && - availableModels && - availableModels.length > 0 && - onModelPickerToggle, - ); + const modelPickerAvailable = Boolean(!isChatMode && onModelPickerToggle); // ─── Command suggestion state ───────────────────────────────────────────── diff --git a/src/view/__tests__/AskBarView.test.tsx b/src/view/__tests__/AskBarView.test.tsx index c08d29b9..464a5db9 100644 --- a/src/view/__tests__/AskBarView.test.tsx +++ b/src/view/__tests__/AskBarView.test.tsx @@ -223,8 +223,6 @@ describe('AskBarView', () => { onSubmit={vi.fn()} onCancel={vi.fn()} inputRef={makeRef()} - activeModel="gemma4:e2b" - availableModels={['gemma4:e2b', 'qwen2.5:7b']} onModelPickerToggle={vi.fn()} />, ); @@ -244,8 +242,6 @@ describe('AskBarView', () => { onSubmit={vi.fn()} onCancel={vi.fn()} inputRef={makeRef()} - activeModel="gemma4:e2b" - availableModels={['gemma4:e2b', 'qwen2.5:7b']} onModelPickerToggle={vi.fn()} />, ); @@ -264,8 +260,6 @@ describe('AskBarView', () => { onSubmit={vi.fn()} onCancel={vi.fn()} inputRef={makeRef()} - activeModel="gemma4:e2b" - availableModels={['gemma4:e2b', 'qwen2.5:7b']} onModelPickerToggle={onModelPickerToggle} />, ); @@ -284,8 +278,6 @@ describe('AskBarView', () => { onSubmit={vi.fn()} onCancel={vi.fn()} inputRef={makeRef()} - activeModel="gemma4:e2b" - availableModels={['gemma4:e2b', 'qwen2.5:7b']} onModelPickerToggle={vi.fn()} isModelPickerOpen={true} />, @@ -306,8 +298,6 @@ describe('AskBarView', () => { onSubmit={vi.fn()} onCancel={vi.fn()} inputRef={makeRef()} - activeModel="gemma4:e2b" - availableModels={['gemma4:e2b', 'qwen2.5:7b']} onModelPickerToggle={vi.fn()} />, ); @@ -318,7 +308,11 @@ describe('AskBarView', () => { ); }); - it('hides the model picker trigger in ask-bar mode when no models are available', () => { + it('still shows the model picker trigger in ask-bar mode with no models so users can recover via the picker', () => { + // The compose-mode chip stays visible whenever the picker callback is + // wired up (Ollama reachable). With zero models or no active selection + // the user must still be able to open the picker to install or pick a + // model; hiding the chip would strand them. render( { onSubmit={vi.fn()} onCancel={vi.fn()} inputRef={makeRef()} - activeModel="" - availableModels={[]} onModelPickerToggle={vi.fn()} />, ); + expect( + screen.getByRole('button', { name: 'Choose model' }), + ).toBeInTheDocument(); + }); + + it('hides the model picker trigger in ask-bar mode when onModelPickerToggle is not provided (Ollama unreachable)', () => { + render( + , + ); expect(screen.queryByRole('button', { name: 'Choose model' })).toBeNull(); }); From 377cbdbea826b3ae9466bd5587aed79b32f51953 Mon Sep 17 00:00:00 2001 From: Logan Nguyen Date: Sun, 26 Apr 2026 16:45:32 -0500 Subject: [PATCH 4/4] refactor: address PR #106 review feedback - Move MAX_MODEL_SLUG_LEN from models.rs to config/defaults.rs so all baked-in pipeline constants live in the single source of truth declared by CLAUDE.md. - Document MAX_MODEL_SLUG_LEN in docs/configurations.md alongside the other Ollama API safety limits. - Switch useOllama.ts modelName coercion from `||` to `??` so an empty slug (malformed state) is no longer silently dropped; only a true null absence becomes undefined. Updated the matching test contract and added an askSearch null-active-model branch test to keep coverage at 100%. Signed-off-by: Logan Nguyen --- docs/configurations.md | 1 + src-tauri/src/config/defaults.rs | 5 +++++ src-tauri/src/models.rs | 7 +------ src/hooks/__tests__/useOllama.test.tsx | 27 ++++++++++++++++++++++++-- src/hooks/useOllama.ts | 4 ++-- 5 files changed, 34 insertions(+), 10 deletions(-) diff --git a/docs/configurations.md b/docs/configurations.md index 4d5993ff..f90dae1c 100644 --- a/docs/configurations.md +++ b/docs/configurations.md @@ -100,6 +100,7 @@ The table below also lists the baked-in safety limits that govern Thuki's commun | `DEFAULT_OLLAMA_SHOW_REQUEST_TIMEOUT_SECS` | `5 s` | No | Protocol cap on a hung daemon to keep the UI responsive. Same rationale as the tags timeout above. | — | How long Thuki waits for Ollama's `/api/show` endpoint to respond before giving up. Used when fetching capability flags (vision, thinking) for each installed model. | | `MAX_OLLAMA_TAGS_BODY_BYTES` | `4 MiB` | No | Defense-in-depth bound on attacker-controlled response body. A misbehaving or compromised Ollama could otherwise stream an unbounded payload and exhaust memory. | — | The largest `/api/tags` response body Thuki will accept. 4 MiB fits thousands of model entries; anything larger is rejected immediately and the request returns an error. | | `MAX_OLLAMA_SHOW_BODY_BYTES` | `4 MiB` | No | Defense-in-depth bound on attacker-controlled response body. Same rationale as `MAX_OLLAMA_TAGS_BODY_BYTES`. | — | The largest `/api/show` response body Thuki will accept. Full Modelfiles and parameters can be sizable, but 4 MiB is well above any real model; larger responses are rejected. | +| `MAX_MODEL_SLUG_LEN` | `256 B` | No | Defense-in-depth bound on adversarial input. Real Ollama slugs are a handful of characters; capping the length stops malformed values long before any network or DB work. | — | The longest model slug Thuki will accept from `set_active_model`. Anything longer is rejected immediately by `validate_model_slug`. | ### `[prompt]` diff --git a/src-tauri/src/config/defaults.rs b/src-tauri/src/config/defaults.rs index feeb0b61..8ff9c441 100644 --- a/src-tauri/src/config/defaults.rs +++ b/src-tauri/src/config/defaults.rs @@ -128,3 +128,8 @@ pub const MAX_OLLAMA_TAGS_BODY_BYTES: usize = 4 * 1024 * 1024; /// Modelfile and parameters can be sizable, but 4 MiB is comfortably above /// any real model and bounds attacker-controlled inputs. pub const MAX_OLLAMA_SHOW_BODY_BYTES: usize = 4 * 1024 * 1024; + +/// Maximum accepted byte length for a model slug passed to `set_active_model`. +/// Real Ollama slugs are a handful of characters; 256 is generous while still +/// capping adversarial inputs long before any network or database work. +pub const MAX_MODEL_SLUG_LEN: usize = 256; diff --git a/src-tauri/src/models.rs b/src-tauri/src/models.rs index 62111a7c..1a5219e6 100644 --- a/src-tauri/src/models.rs +++ b/src-tauri/src/models.rs @@ -21,7 +21,7 @@ use serde::{Deserialize, Serialize}; use crate::config::defaults::{ DEFAULT_OLLAMA_SHOW_REQUEST_TIMEOUT_SECS, DEFAULT_OLLAMA_TAGS_REQUEST_TIMEOUT_SECS, - MAX_OLLAMA_SHOW_BODY_BYTES, MAX_OLLAMA_TAGS_BODY_BYTES, + MAX_MODEL_SLUG_LEN, MAX_OLLAMA_SHOW_BODY_BYTES, MAX_OLLAMA_TAGS_BODY_BYTES, }; use crate::config::AppConfig; use crate::database::{get_config, set_config}; @@ -30,11 +30,6 @@ use crate::history::Database; /// `app_config` key used to persist the user's selected model slug. pub const ACTIVE_MODEL_KEY: &str = "active_model"; -/// Maximum accepted byte length for a model slug passed to `set_active_model`. -/// Real Ollama slugs are a handful of characters; 256 is generous while still -/// capping adversarial inputs long before any network or database work. -pub const MAX_MODEL_SLUG_LEN: usize = 256; - /// Shared error-message prefix used when a requested slug is not present in /// the live Ollama inventory. Exported so the frontend and tests can match /// against a stable constant instead of a prose string. diff --git a/src/hooks/__tests__/useOllama.test.tsx b/src/hooks/__tests__/useOllama.test.tsx index 5dcb930a..1ceddaa6 100644 --- a/src/hooks/__tests__/useOllama.test.tsx +++ b/src/hooks/__tests__/useOllama.test.tsx @@ -932,9 +932,9 @@ describe('useOllama', () => { }); }); - it('leaves modelName undefined when activeModel is an empty string', async () => { + it('leaves modelName undefined when activeModel is null', async () => { const onTurnComplete = vi.fn(); - const { result } = renderHook(() => useOllama('', onTurnComplete)); + const { result } = renderHook(() => useOllama(null, onTurnComplete)); await act(async () => { await result.current.ask('hi'); @@ -974,6 +974,29 @@ describe('useOllama', () => { const [, assistantMsg] = onTurnComplete.mock.calls[0]; expect(assistantMsg.modelName).toBe('qwen2.5:7b'); }); + + it('leaves modelName undefined when activeModel is null on askSearch()', async () => { + const onTurnComplete = vi.fn(); + const { result } = renderHook(() => useOllama(null, onTurnComplete)); + + let pending: Promise | undefined; + await act(async () => { + pending = result.current.askSearch('rust async'); + }); + + const channel = getChannel(); + act(() => { + channel!.simulateMessage({ type: 'Token', content: 'answer' }); + channel!.simulateMessage({ type: 'Done' }); + }); + + await act(async () => { + await pending; + }); + + const [, assistantMsg] = onTurnComplete.mock.calls[0]; + expect(assistantMsg.modelName).toBeUndefined(); + }); }); // ─── loadMessages() ────────────────────────────────────────────────────────── diff --git a/src/hooks/useOllama.ts b/src/hooks/useOllama.ts index 4b5d042d..1b8629b0 100644 --- a/src/hooks/useOllama.ts +++ b/src/hooks/useOllama.ts @@ -237,7 +237,7 @@ export function useOllama( role: 'assistant', content: '', fromThink: think ? true : undefined, - modelName: activeModel || undefined, + modelName: activeModel ?? undefined, }; setMessages((prev) => [...prev, userMsg, assistantMsg]); @@ -392,7 +392,7 @@ export function useOllama( role: 'assistant', content: '', fromSearch: true, - modelName: activeModel || undefined, + modelName: activeModel ?? undefined, }; setMessages((prev) => [...prev, userMsg, assistantMsg]);