diff --git a/EvoScientist/cli/tui_interactive.py b/EvoScientist/cli/tui_interactive.py index 0025ad7..202257f 100644 --- a/EvoScientist/cli/tui_interactive.py +++ b/EvoScientist/cli/tui_interactive.py @@ -2002,13 +2002,32 @@ def run_textual_interactive( if isinstance(focused, MCPBrowserWidget): focused.action_cancel() return + # ModelPickerWidget: when in "Custom Ollama" input mode, its + # child Input widget owns focus, so ``focused`` isn't the + # picker itself. Walk the parent chain to find it, then let + # the widget's own action_cancel decide whether to close the + # picker (list mode) or just exit input mode. + picker: ModelPickerWidget | None = None if isinstance(focused, ModelPickerWidget): - focused.action_cancel() - if ( - self._model_picker_future - and not self._model_picker_future.done() - ): - self._model_picker_future.set_result(None) + picker = focused + else: + node = focused.parent + while node is not None and not isinstance(node, ModelPickerWidget): + node = node.parent + picker = node + if picker is not None: + prev_mode = getattr(picker, "_mode", "list") + picker.action_cancel() + # In list mode action_cancel posted Cancelled; resolve the + # future immediately to avoid a frame of lag. In input + # mode action_cancel flipped back to list — keep picker + # open, do NOT close the future. + if prev_mode == "list": + if ( + self._model_picker_future + and not self._model_picker_future.done() + ): + self._model_picker_future.set_result(None) return if self._queued_messages: self._queued_messages.pop() @@ -2048,8 +2067,19 @@ def run_textual_interactive( if isinstance(focused, MCPBrowserWidget): focused.action_move_up() return + # ModelPickerWidget: Up from the Custom Ollama Input child + # must reach the picker (to exit input mode). See the Esc + # handler above for the parent-walk rationale. + picker_up: ModelPickerWidget | None = None if isinstance(focused, ModelPickerWidget): - focused.action_move_up() + picker_up = focused + else: + node = focused.parent + while node is not None and not isinstance(node, ModelPickerWidget): + node = node.parent + picker_up = node + if picker_up is not None: + picker_up.action_move_up() return if self._queued_messages: last = self._queued_messages.pop() @@ -2105,8 +2135,17 @@ def run_textual_interactive( if isinstance(focused, MCPBrowserWidget): focused.action_move_down() return + # Same parent-walk rationale as action_edit_queued / cancel. + picker_down: ModelPickerWidget | None = None if isinstance(focused, ModelPickerWidget): - focused.action_move_down() + picker_down = focused + else: + node = focused.parent + while node is not None and not isinstance(node, ModelPickerWidget): + node = node.parent + picker_down = node + if picker_down is not None: + picker_down.action_move_down() return # History browsing (down key) diff --git a/EvoScientist/cli/widgets/model_picker.py b/EvoScientist/cli/widgets/model_picker.py index 590e6ad..b7a44ee 100644 --- a/EvoScientist/cli/widgets/model_picker.py +++ b/EvoScientist/cli/widgets/model_picker.py @@ -6,20 +6,26 @@ Models are grouped by provider with a search/filter input. from __future__ import annotations -from typing import TYPE_CHECKING, Any, ClassVar +from typing import TYPE_CHECKING, Any, ClassVar, Literal from rich.text import Text from textual.binding import Binding, BindingType from textual.containers import Container from textual.message import Message from textual.widget import Widget -from textual.widgets import Static +from textual.widgets import Input, Static if TYPE_CHECKING: from textual import events from textual.app import ComposeResult +# Sentinel ``model_id`` used for the "Custom Ollama model..." pseudo-row. +# Selecting this row switches the widget into free-text input mode instead +# of posting ``Picked`` — the user types a model name, Enter confirms. +_CUSTOM_OLLAMA_ID = "__custom_ollama__" + + def _build_items( entries: list[tuple[str, str, str]], current_model: str | None = None, @@ -33,16 +39,27 @@ def _build_items( {"type": "header", "label": str} {"type": "model", "name": str, "model_id": str, "provider": str, "current": bool} """ - # Apply filter + # Apply filter. The Custom Ollama sentinel is the user's escape hatch + # when no local models match; it must remain visible regardless of filter. if filter_text: ft = filter_text.lower() entries = [ - (n, mid, p) for n, mid, p in entries if ft in n.lower() or ft in p.lower() + (n, mid, p) + for n, mid, p in entries + if mid == _CUSTOM_OLLAMA_ID or ft in n.lower() or ft in p.lower() ] - # Group by provider preserving order + # Group by provider preserving order. Deduplicate the Custom Ollama + # sentinel defensively — if callers somehow pass two sentinel rows + # (state reuse, stale merges), collapse them into one to avoid + # rendering duplicate "Custom Ollama model..." rows in the picker. groups: dict[str, list[tuple[str, str, str]]] = {} + seen_sentinel = False for name, model_id, provider in entries: + if model_id == _CUSTOM_OLLAMA_ID: + if seen_sentinel: + continue + seen_sentinel = True if provider not in groups: groups[provider] = [] groups[provider].append((name, model_id, provider)) @@ -72,7 +89,9 @@ class ModelPickerWidget(Widget): """ can_focus = True - can_focus_children = False + # Required so the Custom Ollama ``Input`` child can hold focus when the + # user is typing a model name. + can_focus_children = True DEFAULT_CSS = """ ModelPickerWidget { @@ -83,6 +102,10 @@ class ModelPickerWidget(Widget): background: $surface; border: solid $primary; } + ModelPickerWidget .picker-custom-input { + height: 3; + margin: 1 0 0 0; + } ModelPickerWidget .picker-title { height: 1; text-style: bold; @@ -158,6 +181,12 @@ class ModelPickerWidget(Widget): self._selected = self._first_model_index() self._row_widgets: list[Static] = [] self._filter_widget: Static | None = None + # "list" = arrow-key selection over models; "input" = free-text entry + # for Custom Ollama model name. Transitions: selecting the sentinel + # row enters input mode; Esc or Up arrow inside input mode returns to + # list mode without closing the picker. + self._mode: Literal["list", "input"] = "list" + self._custom_input: Input | None = None def _first_model_index(self) -> int: for i, item in enumerate(self._items): @@ -209,6 +238,14 @@ class ModelPickerWidget(Widget): widget = Static("", classes=css) self._row_widgets.append(widget) yield widget + # Hidden until the user selects "Custom Ollama model..." \u2014 then shown + # and focused for free-text entry of an Ollama model name. + self._custom_input = Input( + placeholder="Type Ollama model name (e.g. llama3.3)...", + classes="picker-custom-input", + ) + self._custom_input.display = False + yield self._custom_input yield Static( "\u2191/\u2193 navigate \u00b7 Enter select \u00b7 Type to filter \u00b7 Esc cancel", classes="picker-help", @@ -257,6 +294,9 @@ class ModelPickerWidget(Widget): widget.scroll_visible() def on_key(self, event: events.Key) -> None: + # In input mode, the Input child owns printable keys + backspace. + if self._mode == "input": + return # Let bindings handle special keys if event.key in ("up", "down", "enter", "escape", "backspace"): return @@ -267,28 +307,84 @@ class ModelPickerWidget(Widget): event.prevent_default() def action_backspace(self) -> None: + if self._mode == "input": + # Input widget handles its own backspace. + return if self._filter_text: self._filter_text = self._filter_text[:-1] self._rebuild() def action_move_up(self) -> None: + if self._mode == "input": + # Up from the Input field escapes back to list selection. + self._exit_input_mode() + return self._move(-1) def action_move_down(self) -> None: + if self._mode == "input": + # Down in input mode is ambiguous; absorb rather than toggle. + return self._move(1) def action_select(self) -> None: + if self._mode == "input": + self._submit_custom_input() + return if not self._items or self._selected >= len(self._items): self.post_message(self.Cancelled()) return item = self._items[self._selected] - if item["type"] == "model": - self.post_message(self.Picked(item["name"], item["provider"])) - else: + if item["type"] != "model": self.post_message(self.Cancelled()) + return + if item["provider"] == "ollama" and item["model_id"] == _CUSTOM_OLLAMA_ID: + self._enter_input_mode() + return + self.post_message(self.Picked(item["name"], item["provider"])) def action_cancel(self) -> None: + if self._mode == "input": + # Esc returns to list selection; does NOT close the picker. + self._exit_input_mode() + return self.post_message(self.Cancelled()) def on_blur(self, event: events.Blur) -> None: + # When the Input child has focus we must NOT steal it back. + if self._mode == "input": + return self.call_after_refresh(self.focus) + + def on_input_submitted(self, event: Input.Submitted) -> None: + """Safety net: Enter fired inside the Input widget rather than + bubbling to ``action_select``. Route to the same submit path.""" + if event.input is self._custom_input: + event.stop() + self._submit_custom_input() + + def _enter_input_mode(self) -> None: + """Show the Custom Ollama Input and move focus into it.""" + self._mode = "input" + if self._custom_input is not None: + self._custom_input.display = True + # Carry any filter text over as a nice touch — user may have + # started typing a model name thinking it would filter. + self._custom_input.value = self._filter_text + self._custom_input.focus() + + def _exit_input_mode(self) -> None: + """Hide the Input and return focus to the list.""" + self._mode = "list" + if self._custom_input is not None: + self._custom_input.display = False + self._custom_input.value = "" + self.focus() + + def _submit_custom_input(self) -> None: + """Confirm the typed Ollama model name. Empty input is a no-op — + user can Esc out or keep typing.""" + typed = (self._custom_input.value if self._custom_input else "").strip() + if not typed: + return + self.post_message(self.Picked(typed, "ollama")) diff --git a/EvoScientist/commands/implementation/model.py b/EvoScientist/commands/implementation/model.py index dfb15eb..f8d65b7 100644 --- a/EvoScientist/commands/implementation/model.py +++ b/EvoScientist/commands/implementation/model.py @@ -16,13 +16,21 @@ def extract_model_and_provider(args: list[str]) -> tuple[str, str]: ``(model_name, provider)`` tuple. Raises: - ValueError: If the model is not in the registry. + ValueError: If the model is not in the registry. Skipped when + ``provider_override == "ollama"``, since Ollama models are + locally-installed and never appear in ``MODELS``. """ from ...llm.models import MODELS model_name = args[0] provider_override = args[1] if len(args) > 1 else None + # Ollama models are locally-installed — not in the registry. Pass the name + # through verbatim; get_chat_model's "Assume full model ID" fallback + # (models.py) accepts them. + if provider_override == "ollama": + return model_name, "ollama" + if model_name not in MODELS: raise ValueError(f"Unknown model '{model_name}'") @@ -90,6 +98,22 @@ class ModelCommand(Command): return entries = list_models_by_provider() + + # Ollama models are locally-installed — probe the daemon for the list + # the user has actually pulled. Gated on ollama_base_url being set + # (issue non-goal forbids implicit localhost detection). + ollama_base_url = getattr(cfg, "ollama_base_url", None) + if ollama_base_url: + from ...llm.ollama_discovery import discover_ollama_models + + detected = await discover_ollama_models(ollama_base_url, timeout=1.5) + for detected_name in detected: + entries.append((detected_name, detected_name, "ollama")) + # Always append the sentinel so users can type a name even when + # the daemon is down or no models have been pulled yet. The widget + # swaps the sentinel name for the typed value before posting Picked. + entries.append(("Custom Ollama model...", "__custom_ollama__", "ollama")) + result = await ctx.ui.wait_for_model_pick( entries, current_model=current_model, @@ -99,6 +123,14 @@ class ModelCommand(Command): return name, provider = result + # Defense-in-depth: the widget should have replaced the sentinel with + # the user-typed name. If it didn't, treat as cancel rather than try + # to switch to a literal "__custom_ollama__" model. + if provider == "ollama" and name in ( + "Custom Ollama model...", + "__custom_ollama__", + ): + return await self._apply_model(ctx, name, provider, save=save) async def _apply_model( @@ -139,6 +171,20 @@ class ModelCommand(Command): _mod._EvoScientist_agent, ) + def _restore_globals() -> None: + """Roll back the four module globals to their pre-call values. + + Keeps the two failure sites (``_load_agent`` and + ``set_chat_model``) in sync — adding a new snapshotted global + only requires updating ``snap`` and this helper. + """ + ( + _mod._config, + _mod._chat_model, + _mod._chat_model_key, + _mod._EvoScientist_agent, + ) = snap + try: new_agent = _load_agent( workspace_dir=ctx.workspace_dir, @@ -146,12 +192,7 @@ class ModelCommand(Command): config=temp_cfg, ) except Exception as e: - ( - _mod._config, - _mod._chat_model, - _mod._chat_model_key, - _mod._EvoScientist_agent, - ) = snap + _restore_globals() ctx.ui.append_system(f"Failed to switch model: {e}", style="red") return @@ -159,6 +200,9 @@ class ModelCommand(Command): try: set_chat_model(model_name, provider=provider) except Exception as e: + # _load_agent already mutated the four globals; restore them so a + # failure here doesn't leave the session half-switched. + _restore_globals() ctx.ui.append_system(f"Failed to switch model: {e}", style="red") return diff --git a/EvoScientist/config/onboard.py b/EvoScientist/config/onboard.py index fbab5b7..c29775f 100644 --- a/EvoScientist/config/onboard.py +++ b/EvoScientist/config/onboard.py @@ -22,6 +22,7 @@ from rich.panel import Panel from rich.text import Text from ..llm import get_models_for_provider +from ..llm.ollama_discovery import validate_ollama_connection from .settings import ( EvoScientistConfig, get_config_path, @@ -577,36 +578,6 @@ def validate_tavily_key(api_key: str) -> tuple[bool, str]: return False, f"Error: {e}" -def validate_ollama_connection(base_url: str) -> tuple[bool, str, list[str]]: - """Validate that Ollama is reachable at the given base URL. - - Args: - base_url: The Ollama server base URL. - - Returns: - Tuple of (is_valid, message, model_names). - model_names is a list of pulled model names (empty if unreachable). - """ - if not base_url: - return True, "Skipped (no URL provided)", [] - - try: - import httpx - - resp = httpx.get(f"{base_url.rstrip('/')}/api/tags", timeout=5) - if resp.status_code == 200: - data = resp.json() - models = data.get("models", []) - names = [m.get("name", "?") for m in models] - if names: - preview = ", ".join(names[:5]) - return True, f"Connected — {len(names)} model(s): {preview}", names - return True, "Connected (no models pulled yet)", [] - return False, f"HTTP {resp.status_code}", [] - except Exception as e: - return False, f"Cannot reach Ollama: {e}", [] - - # ============================================================================= # Display Helpers # ============================================================================= diff --git a/EvoScientist/llm/ollama_discovery.py b/EvoScientist/llm/ollama_discovery.py new file mode 100644 index 0000000..55dc18e --- /dev/null +++ b/EvoScientist/llm/ollama_discovery.py @@ -0,0 +1,72 @@ +"""Ollama server probing — shared by onboard wizard and /model picker. + +Ollama models are whatever the user has ``ollama pull``ed locally; they +cannot be enumerated in ``_MODEL_ENTRIES``. Both the setup wizard and the +interactive model picker need to hit ``GET {base_url}/api/tags`` to see +what is actually installed. + +This module keeps a single implementation of that probe. ``onboard.py`` +uses the sync variant (it drives a synchronous questionary flow); +``/model`` uses the async variant (command dispatch is already ``async`` +and benefits from ``httpx.AsyncClient``). +""" + +from __future__ import annotations + + +def validate_ollama_connection(base_url: str) -> tuple[bool, str, list[str]]: + """Sync probe. Returns ``(is_reachable, human_msg, model_names)``. + + Used by the onboarding wizard. Verbatim semantics from the original + implementation that lived in ``config/onboard.py`` — 5 s timeout, + swallows all exceptions into a ``(False, error_msg, [])`` tuple. + """ + if not base_url: + return True, "Skipped (no URL provided)", [] + + try: + import httpx + + resp = httpx.get(f"{base_url.rstrip('/')}/api/tags", timeout=5) + if resp.status_code == 200: + data = resp.json() + models = data.get("models", []) + names = [m.get("name", "?") for m in models] + if names: + preview = ", ".join(names[:5]) + return True, f"Connected — {len(names)} model(s): {preview}", names + return True, "Connected (no models pulled yet)", [] + return False, f"HTTP {resp.status_code}", [] + except Exception as e: + return False, f"Cannot reach Ollama: {e}", [] + + +async def discover_ollama_models( + base_url: str | None, *, timeout: float = 1.5 +) -> list[str]: + """Async probe for the ``/model`` picker. + + Returns the list of installed model names, or an empty list on any + failure (unreachable daemon, timeout, HTTP error, malformed JSON). + Never raises — the picker must always open, even when the daemon + is down. + + If ``base_url`` is falsy, returns ``[]`` immediately without making + an HTTP call. Implicit ``localhost:11434`` probing is deliberately + out of scope (see ``.issue_ollama_model_picker.md`` non-goals); + ``ollama_base_url`` must be explicitly configured to activate. + """ + if not base_url: + return [] + + try: + import httpx + + async with httpx.AsyncClient(timeout=timeout) as client: + resp = await client.get(f"{base_url.rstrip('/')}/api/tags") + if resp.status_code != 200: + return [] + data = resp.json() + return [m.get("name", "") for m in data.get("models", []) if m.get("name")] + except Exception: + return [] diff --git a/tests/test_model_command.py b/tests/test_model_command.py index 25f4f77..d41f313 100644 --- a/tests/test_model_command.py +++ b/tests/test_model_command.py @@ -56,6 +56,26 @@ class TestExtractModelAndProvider: assert name == "claude-sonnet-4-6" assert prov == "openrouter" + def test_ollama_provider_accepts_arbitrary_name(self): + """Ollama models are locally-installed — the registry doesn't know + them. The ``ollama`` provider must pass any name through verbatim.""" + from EvoScientist.commands.implementation.model import ( + extract_model_and_provider, + ) + + name, prov = extract_model_and_provider(["llama3.3:8b", "ollama"]) + assert name == "llama3.3:8b" + assert prov == "ollama" + + def test_ollama_provider_accepts_dotted_tag(self): + from EvoScientist.commands.implementation.model import ( + extract_model_and_provider, + ) + + name, prov = extract_model_and_provider(["qwen3-coder-next:latest", "ollama"]) + assert name == "qwen3-coder-next:latest" + assert prov == "ollama" + class TestModelCommandUnknownModel: """Verify error message for unknown models.""" @@ -633,3 +653,282 @@ class TestApplyModelLoadAgentFailureTransactional: # User sees an error message. msg = ctx.ui.append_system.call_args[0][0] assert "Failed to switch model" in msg + + +class TestApplyModelSetChatModelFailureTransactional: + """Regression (CodeRabbit review on PR #187): if ``set_chat_model`` + raises *after* ``_load_agent`` has already mutated module globals, + those globals must be restored. Without the rollback the session + ends up half-switched — new ``_config`` / ``_chat_model`` committed, + but no successful agent to back them. + + Complements :class:`TestApplyModelLoadAgentFailureTransactional` + which covers the earlier failure site. + """ + + def test_globals_restored_when_set_chat_model_raises(self, evo_module_state): + from EvoScientist.commands.implementation.model import ModelCommand + from EvoScientist.config.settings import EvoScientistConfig + + mod = evo_module_state + sentinels: dict[tuple[str, str | None], MagicMock] = {} + + def _fake_get_chat_model(model, provider=None): + key = (model, provider) + sentinels.setdefault(key, MagicMock(name=f"chat_model[{model}|{provider}]")) + return sentinels[key] + + def _fake_load_agent( + workspace_dir=None, + checkpointer=None, + config=None, + *, + on_mcp_progress=None, + ): + # Mimic the real ``create_cli_agent``: mutate globals via + # ``_ensure_config`` + ``_ensure_chat_model``, then succeed. + mod._ensure_config(config) + mod._ensure_chat_model() + return MagicMock(name="new-agent") + + cfg = EvoScientistConfig(model="claude-sonnet-4-6", provider="anthropic") + old_model = _fake_get_chat_model("claude-sonnet-4-6", "anthropic") + old_agent = MagicMock(name="old-default-agent") + + mod._config = cfg + mod._chat_model = old_model + mod._chat_model_key = ("claude-sonnet-4-6", "anthropic") + mod._EvoScientist_agent = old_agent + + ctx = MagicMock() + ctx.ui = MagicMock() + ctx.ui.supports_interactive = True + ctx.workspace_dir = "/tmp/test_rollback_set" + ctx.checkpointer = None + + with ( + patch( + "EvoScientist.llm.get_chat_model", + side_effect=_fake_get_chat_model, + ), + patch( + "EvoScientist.cli.agent._load_agent", + side_effect=_fake_load_agent, + ), + patch( + "EvoScientist.EvoScientist.set_chat_model", + side_effect=RuntimeError("API key missing at commit step"), + ), + ): + cmd = ModelCommand() + _run(cmd._apply_model(ctx, "minimax-m2.7", "openrouter")) + + # All four globals restored — the new agent was built, but the + # commit step (set_chat_model) failed, so the session must remain + # on the original model. + assert mod._config is cfg + assert mod._chat_model is old_model + assert mod._chat_model_key == ("claude-sonnet-4-6", "anthropic") + assert mod._EvoScientist_agent is old_agent + # cfg itself must not have been mutated (happens after the commit). + assert cfg.model == "claude-sonnet-4-6" + assert cfg.provider == "anthropic" + # User sees an error message. + msg = ctx.ui.append_system.call_args[0][0] + assert "Failed to switch model" in msg + + +class TestModelCommandOllamaPicker: + """Verify Ollama discovery augments the picker entries and the sentinel + is always present when Ollama is configured.""" + + def _make_ctx_and_cfg(self, *, ollama_base_url: str | None): + cfg = SimpleNamespace( + model="claude-sonnet-4-6", + provider="anthropic", + ollama_base_url=ollama_base_url, + ) + ui = MagicMock() + ui.supports_interactive = True + ui.wait_for_model_pick = AsyncMock(return_value=None) + ctx = MagicMock() + ctx.ui = ui + return ctx, cfg, ui + + def test_picker_entries_include_detected_ollama_models(self): + """When Ollama is reachable, detected models appear in entries with + provider='ollama' and the Custom sentinel is appended.""" + from EvoScientist.commands.implementation.model import ModelCommand + + ctx, cfg, ui = self._make_ctx_and_cfg(ollama_base_url="http://localhost:11434") + + async def fake_discover(base_url, *, timeout): + return ["llama3.3:latest", "qwen3:8b"] + + with ( + patch( + "EvoScientist.EvoScientist._ensure_config", + return_value=cfg, + ), + patch( + "EvoScientist.llm.ollama_discovery.discover_ollama_models", + side_effect=fake_discover, + ), + ): + _run(ModelCommand().execute(ctx, [])) + + entries = ui.wait_for_model_pick.call_args[0][0] + ollama_rows = [(n, mid, p) for (n, mid, p) in entries if p == "ollama"] + assert ("llama3.3:latest", "llama3.3:latest", "ollama") in ollama_rows + assert ("qwen3:8b", "qwen3:8b", "ollama") in ollama_rows + assert ( + "Custom Ollama model...", + "__custom_ollama__", + "ollama", + ) in ollama_rows + + def test_picker_entries_include_sentinel_when_discovery_empty(self): + """Daemon unreachable / no models pulled — sentinel is the user's + escape hatch and must always be present.""" + from EvoScientist.commands.implementation.model import ModelCommand + + ctx, cfg, ui = self._make_ctx_and_cfg(ollama_base_url="http://localhost:11434") + + async def fake_discover(base_url, *, timeout): + return [] + + with ( + patch( + "EvoScientist.EvoScientist._ensure_config", + return_value=cfg, + ), + patch( + "EvoScientist.llm.ollama_discovery.discover_ollama_models", + side_effect=fake_discover, + ), + ): + _run(ModelCommand().execute(ctx, [])) + + entries = ui.wait_for_model_pick.call_args[0][0] + ollama_rows = [(n, mid, p) for (n, mid, p) in entries if p == "ollama"] + assert ollama_rows == [ + ("Custom Ollama model...", "__custom_ollama__", "ollama") + ] + + def test_picker_skips_ollama_section_when_not_configured(self): + """ollama_base_url unset → no discovery call, no ollama entries, + no sentinel (issue non-goal: no implicit localhost detection).""" + from EvoScientist.commands.implementation.model import ModelCommand + + ctx, cfg, ui = self._make_ctx_and_cfg(ollama_base_url="") + + discovery = AsyncMock(return_value=["should-never-appear"]) + + with ( + patch( + "EvoScientist.EvoScientist._ensure_config", + return_value=cfg, + ), + patch( + "EvoScientist.llm.ollama_discovery.discover_ollama_models", + discovery, + ), + ): + _run(ModelCommand().execute(ctx, [])) + + discovery.assert_not_called() + entries = ui.wait_for_model_pick.call_args[0][0] + assert not any(p == "ollama" for (_, _, p) in entries) + + def test_picker_handles_cfg_without_ollama_base_url_attr(self): + """getattr(cfg, 'ollama_base_url', None) fallback: old configs + (or SimpleNamespace test fixtures) may not carry the attribute + at all. Must not raise AttributeError, must not probe.""" + from EvoScientist.commands.implementation.model import ModelCommand + + # Deliberately omit ollama_base_url from the namespace. + cfg = SimpleNamespace(model="claude-sonnet-4-6", provider="anthropic") + ui = MagicMock() + ui.supports_interactive = True + ui.wait_for_model_pick = AsyncMock(return_value=None) + ctx = MagicMock() + ctx.ui = ui + + discovery = AsyncMock(return_value=["should-never-appear"]) + + with ( + patch( + "EvoScientist.EvoScientist._ensure_config", + return_value=cfg, + ), + patch( + "EvoScientist.llm.ollama_discovery.discover_ollama_models", + discovery, + ), + ): + _run(ModelCommand().execute(ctx, [])) + + discovery.assert_not_called() + entries = ui.wait_for_model_pick.call_args[0][0] + assert not any(p == "ollama" for (_, _, p) in entries) + + def test_picker_sentinel_result_is_treated_as_cancel(self): + """Defense-in-depth: if the widget ever returns the sentinel name + itself (shouldn't happen — it should substitute the typed name), + dispatch treats it as a cancel and does NOT call _apply_model.""" + from EvoScientist.commands.implementation.model import ModelCommand + + ctx, cfg, ui = self._make_ctx_and_cfg(ollama_base_url="http://localhost:11434") + ui.wait_for_model_pick = AsyncMock(return_value=("__custom_ollama__", "ollama")) + + async def fake_discover(base_url, *, timeout): + return [] + + with ( + patch( + "EvoScientist.EvoScientist._ensure_config", + return_value=cfg, + ), + patch( + "EvoScientist.llm.ollama_discovery.discover_ollama_models", + side_effect=fake_discover, + ), + patch("EvoScientist.cli.agent._load_agent") as load_agent, + ): + _run(ModelCommand().execute(ctx, [])) + + load_agent.assert_not_called() + assert cfg.model == "claude-sonnet-4-6" # unchanged + + def test_picker_applies_detected_ollama_model(self): + """User picks a live-detected Ollama model → _apply_model is invoked + with (name, "ollama") and the agent is rebuilt.""" + from EvoScientist.commands.implementation.model import ModelCommand + + ctx, cfg, ui = self._make_ctx_and_cfg(ollama_base_url="http://localhost:11434") + ctx.workspace_dir = "/tmp/test" + ctx.checkpointer = MagicMock() + ui.wait_for_model_pick = AsyncMock(return_value=("llama3.3", "ollama")) + + async def fake_discover(base_url, *, timeout): + return ["llama3.3"] + + with ( + patch( + "EvoScientist.EvoScientist._ensure_config", + return_value=cfg, + ), + patch( + "EvoScientist.llm.ollama_discovery.discover_ollama_models", + side_effect=fake_discover, + ), + patch("EvoScientist.EvoScientist.set_chat_model"), + patch( + "EvoScientist.cli.agent._load_agent", + return_value=MagicMock(), + ), + ): + _run(ModelCommand().execute(ctx, [])) + + assert cfg.model == "llama3.3" + assert cfg.provider == "ollama" diff --git a/tests/test_ollama_discovery.py b/tests/test_ollama_discovery.py new file mode 100644 index 0000000..1fb785f --- /dev/null +++ b/tests/test_ollama_discovery.py @@ -0,0 +1,182 @@ +"""Tests for EvoScientist.llm.ollama_discovery. + +Covers both the sync ``validate_ollama_connection`` (used by the onboarding +wizard) and the async ``discover_ollama_models`` (used by the /model +picker). The async variant must never raise — the picker's UX depends on +a silent empty-list fallback when the daemon is down or misbehaving. +""" + +from __future__ import annotations + +from unittest.mock import MagicMock, patch + +import httpx +import pytest + +from EvoScientist.llm.ollama_discovery import ( + discover_ollama_models, + validate_ollama_connection, +) +from tests.conftest import run_async as _run + + +class TestValidateOllamaConnection: + """Sync probe — guards onboard's existing contract.""" + + def test_empty_base_url_returns_skipped(self): + ok, msg, names = validate_ollama_connection("") + assert ok is True + assert "Skipped" in msg + assert names == [] + + def test_200_with_models_returns_names(self): + resp = MagicMock() + resp.status_code = 200 + resp.json.return_value = { + "models": [{"name": "llama3.3:latest"}, {"name": "qwen3:8b"}] + } + with patch("httpx.get", return_value=resp): + ok, msg, names = validate_ollama_connection("http://localhost:11434") + assert ok is True + assert "Connected" in msg + assert names == ["llama3.3:latest", "qwen3:8b"] + + def test_200_with_no_models_still_ok(self): + resp = MagicMock() + resp.status_code = 200 + resp.json.return_value = {"models": []} + with patch("httpx.get", return_value=resp): + ok, msg, names = validate_ollama_connection("http://localhost:11434") + assert ok is True + assert "no models pulled" in msg + assert names == [] + + def test_non_200_returns_error(self): + resp = MagicMock() + resp.status_code = 500 + with patch("httpx.get", return_value=resp): + ok, msg, names = validate_ollama_connection("http://localhost:11434") + assert ok is False + assert "500" in msg + assert names == [] + + def test_connect_error_returns_error(self): + with patch("httpx.get", side_effect=httpx.ConnectError("refused")): + ok, msg, names = validate_ollama_connection("http://localhost:11434") + assert ok is False + assert "Cannot reach Ollama" in msg + assert names == [] + + +class TestDiscoverOllamaModels: + """Async probe — contract: never raise, return list[str].""" + + def test_empty_base_url_returns_empty_without_http(self): + # No HTTP call should be made for an empty base_url — verified by + # the fact that no mock is set up and the test completes. + names = _run(discover_ollama_models("")) + assert names == [] + + def test_none_base_url_returns_empty(self): + names = _run(discover_ollama_models(None)) + assert names == [] + + def test_200_returns_names(self): + async def fake_get(self, url): + resp = MagicMock() + resp.status_code = 200 + resp.json = MagicMock( + return_value={ + "models": [{"name": "llama3.3:latest"}, {"name": "qwen3:8b"}] + } + ) + return resp + + with patch.object(httpx.AsyncClient, "get", fake_get): + names = _run(discover_ollama_models("http://localhost:11434")) + assert names == ["llama3.3:latest", "qwen3:8b"] + + def test_strips_entries_without_name(self): + async def fake_get(self, url): + resp = MagicMock() + resp.status_code = 200 + resp.json = MagicMock( + return_value={ + "models": [ + {"name": "llama3.3"}, + {"name": ""}, # dropped + {}, # dropped + ] + } + ) + return resp + + with patch.object(httpx.AsyncClient, "get", fake_get): + names = _run(discover_ollama_models("http://localhost:11434")) + assert names == ["llama3.3"] + + def test_timeout_returns_empty(self): + async def fake_get(self, url): + raise httpx.TimeoutException("timed out") + + with patch.object(httpx.AsyncClient, "get", fake_get): + names = _run(discover_ollama_models("http://localhost:11434")) + assert names == [] + + def test_connect_error_returns_empty(self): + async def fake_get(self, url): + raise httpx.ConnectError("refused") + + with patch.object(httpx.AsyncClient, "get", fake_get): + names = _run(discover_ollama_models("http://localhost:11434")) + assert names == [] + + def test_non_200_returns_empty(self): + async def fake_get(self, url): + resp = MagicMock() + resp.status_code = 500 + return resp + + with patch.object(httpx.AsyncClient, "get", fake_get): + names = _run(discover_ollama_models("http://localhost:11434")) + assert names == [] + + def test_malformed_json_returns_empty(self): + async def fake_get(self, url): + resp = MagicMock() + resp.status_code = 200 + resp.json = MagicMock(side_effect=ValueError("bad json")) + return resp + + with patch.object(httpx.AsyncClient, "get", fake_get): + names = _run(discover_ollama_models("http://localhost:11434")) + assert names == [] + + def test_missing_models_key_returns_empty(self): + async def fake_get(self, url): + resp = MagicMock() + resp.status_code = 200 + resp.json = MagicMock(return_value={"unexpected": "shape"}) + return resp + + with patch.object(httpx.AsyncClient, "get", fake_get): + names = _run(discover_ollama_models("http://localhost:11434")) + assert names == [] + + def test_trailing_slash_stripped_from_url(self): + called = {} + + async def fake_get(self, url): + called["url"] = url + resp = MagicMock() + resp.status_code = 200 + resp.json = MagicMock(return_value={"models": []}) + return resp + + with patch.object(httpx.AsyncClient, "get", fake_get): + _run(discover_ollama_models("http://localhost:11434/")) + assert called["url"] == "http://localhost:11434/api/tags" + + +if __name__ == "__main__": + pytest.main([__file__, "-v"]) diff --git a/tests/test_tui_widgets.py b/tests/test_tui_widgets.py index 989fc7c..a478610 100644 --- a/tests/test_tui_widgets.py +++ b/tests/test_tui_widgets.py @@ -977,5 +977,275 @@ class TestCompletionLogic(unittest.TestCase): assert app._comp_index == 0 # unchanged +@unittest.skipUnless(_has_textual, "textual not installed") +class TestModelPickerWidgetOllama(unittest.TestCase): + """ModelPickerWidget Ollama fallback: sentinel row renders under the + ollama group, selecting it enters free-text input mode, Enter confirms + with ``Picked(typed, "ollama")``, Esc returns to list, and filtering + never hides the sentinel.""" + + def _make_widget(self, entries=None, *, current_model=None): + """Build a widget with a mix of providers + the sentinel row. + + When ``entries`` is ``None``, a default mix (anthropic + one ollama + model + sentinel) is used. When provided, it fully replaces the + default — callers that need extra rows should pass the complete list. + """ + from unittest.mock import MagicMock + + from EvoScientist.cli.widgets.model_picker import ( + _CUSTOM_OLLAMA_ID, + ModelPickerWidget, + ) + + if entries is None: + entries = [ + ("claude-sonnet-4-6", "claude-sonnet-4-6", "anthropic"), + ("llama3.3", "llama3.3", "ollama"), + ("Custom Ollama model...", _CUSTOM_OLLAMA_ID, "ollama"), + ] + w = ModelPickerWidget(entries, current_model=current_model) + # Stub out Textual-dependent side effects so we can drive actions + # directly (follows the module's "no pilot" test pattern). + w.post_message = MagicMock() + w.focus = MagicMock() + # Fake the Input child — real one requires a mounted app. + custom_input = MagicMock() + custom_input.value = "" + custom_input.display = False + w._custom_input = custom_input + return w + + def _sentinel_index(self, widget): + from EvoScientist.cli.widgets.model_picker import _CUSTOM_OLLAMA_ID + + for i, item in enumerate(widget._items): + if item["type"] == "model" and item.get("model_id") == _CUSTOM_OLLAMA_ID: + return i + raise AssertionError(f"sentinel not found in items: {widget._items}") + + def test_sentinel_rendered_under_ollama_group(self): + w = self._make_widget() + # The sentinel must be grouped under an "ollama" header. + headers = [i["label"] for i in w._items if i["type"] == "header"] + assert "ollama" in headers + + def test_selecting_regular_row_posts_picked(self): + """Baseline: non-sentinel selection still works.""" + from EvoScientist.cli.widgets.model_picker import ModelPickerWidget + + w = self._make_widget() + # Find the claude row + claude_idx = next( + i + for i, item in enumerate(w._items) + if item["type"] == "model" and item["name"] == "claude-sonnet-4-6" + ) + w._selected = claude_idx + w.action_select() + assert w._mode == "list" + msgs = [c.args[0] for c in w.post_message.call_args_list] + assert any( + isinstance(m, ModelPickerWidget.Picked) + and m.name == "claude-sonnet-4-6" + and m.provider == "anthropic" + for m in msgs + ) + + def test_selecting_sentinel_enters_input_mode(self): + w = self._make_widget() + w._selected = self._sentinel_index(w) + w.action_select() + + assert w._mode == "input" + assert w._custom_input.display is True + w._custom_input.focus.assert_called_once() + # Entering input mode must NOT post any message — the user hasn't + # submitted anything yet. + w.post_message.assert_not_called() + + def test_enter_with_typed_name_posts_picked(self): + from EvoScientist.cli.widgets.model_picker import ModelPickerWidget + + w = self._make_widget() + w._mode = "input" + w._custom_input.value = "qwen3-coder-next" + w._custom_input.display = True + + w.action_select() + + msgs = [c.args[0] for c in w.post_message.call_args_list] + picked = [m for m in msgs if isinstance(m, ModelPickerWidget.Picked)] + assert len(picked) == 1 + assert picked[0].name == "qwen3-coder-next" + assert picked[0].provider == "ollama" + + def test_enter_with_empty_input_is_noop(self): + w = self._make_widget() + w._mode = "input" + w._custom_input.value = "" + + w.action_select() + + w.post_message.assert_not_called() + # Still in input mode — user can keep typing or Esc out. + assert w._mode == "input" + + def test_enter_with_whitespace_only_input_is_noop(self): + w = self._make_widget() + w._mode = "input" + w._custom_input.value = " \t " + + w.action_select() + + w.post_message.assert_not_called() + assert w._mode == "input" + + def test_esc_in_input_mode_returns_to_list(self): + from EvoScientist.cli.widgets.model_picker import ModelPickerWidget + + w = self._make_widget() + w._mode = "input" + w._custom_input.value = "partial" + w._custom_input.display = True + + w.action_cancel() + + assert w._mode == "list" + assert w._custom_input.display is False + assert w._custom_input.value == "" + # No Cancelled message — Esc from input returns to list, not closes. + cancelled = [ + c.args[0] + for c in w.post_message.call_args_list + if isinstance(c.args[0], ModelPickerWidget.Cancelled) + ] + assert cancelled == [] + + def test_esc_in_list_mode_cancels(self): + from EvoScientist.cli.widgets.model_picker import ModelPickerWidget + + w = self._make_widget() + w._mode = "list" + w.action_cancel() + msgs = [c.args[0] for c in w.post_message.call_args_list] + assert any(isinstance(m, ModelPickerWidget.Cancelled) for m in msgs) + + def test_up_in_input_mode_exits_to_list(self): + w = self._make_widget() + w._mode = "input" + w._custom_input.display = True + + w.action_move_up() + + assert w._mode == "list" + assert w._custom_input.display is False + + def test_down_in_input_mode_absorbed(self): + w = self._make_widget() + w._mode = "input" + before_selected = w._selected + w._custom_input.display = True + + w.action_move_down() + + # State unchanged — key was absorbed. + assert w._mode == "input" + assert w._custom_input.display is True + assert w._selected == before_selected + + def test_backspace_in_input_mode_no_filter_change(self): + w = self._make_widget() + w._mode = "input" + w._filter_text = "foo" + + w.action_backspace() + + # Input widget handles its own backspace — filter unchanged. + assert w._filter_text == "foo" + + def test_printable_key_in_input_mode_does_not_filter(self): + from unittest.mock import MagicMock + + w = self._make_widget() + w._mode = "input" + w._filter_text = "" + + event = MagicMock() + event.key = "a" + event.character = "a" + + w.on_key(event) + + assert w._filter_text == "" + + def test_duplicate_sentinels_collapsed(self): + """Defense-in-depth: even if callers pass two sentinel rows (state + reuse, stale merges), only one "Custom Ollama model..." renders.""" + from EvoScientist.cli.widgets.model_picker import ( + _CUSTOM_OLLAMA_ID, + _build_items, + ) + + entries = [ + ("claude-sonnet-4-6", "claude-sonnet-4-6", "anthropic"), + ("Custom Ollama model...", _CUSTOM_OLLAMA_ID, "ollama"), + ("Custom Ollama model...", _CUSTOM_OLLAMA_ID, "ollama"), + ("Custom Ollama model...", _CUSTOM_OLLAMA_ID, "ollama"), + ] + items = _build_items(entries) + sentinel_rows = [ + i + for i in items + if i["type"] == "model" and i.get("model_id") == _CUSTOM_OLLAMA_ID + ] + assert len(sentinel_rows) == 1, f"duplicate sentinels rendered: {items}" + + def test_sentinel_survives_filter(self): + """The Custom Ollama row is the user's escape hatch — filtering + must never hide it.""" + from EvoScientist.cli.widgets.model_picker import ( + _CUSTOM_OLLAMA_ID, + _build_items, + ) + + entries = [ + ("claude-sonnet-4-6", "claude-sonnet-4-6", "anthropic"), + ("llama3.3", "llama3.3", "ollama"), + ("Custom Ollama model...", _CUSTOM_OLLAMA_ID, "ollama"), + ] + # A filter that matches NOTHING in the normal entries. + items = _build_items(entries, filter_text="zzzzzz") + sentinel_rows = [ + i + for i in items + if i["type"] == "model" and i.get("model_id") == _CUSTOM_OLLAMA_ID + ] + assert len(sentinel_rows) == 1, f"sentinel hidden by filter: {items}" + + def test_on_input_submitted_routes_to_submit(self): + """Belt-and-suspenders: Enter fired inside the Input widget should + be handled the same way as action_select in input mode.""" + from unittest.mock import MagicMock + + from EvoScientist.cli.widgets.model_picker import ModelPickerWidget + + w = self._make_widget() + w._mode = "input" + w._custom_input.value = "mymodel" + + event = MagicMock() + event.input = w._custom_input # the Input child we stubbed + + w.on_input_submitted(event) + + event.stop.assert_called_once() + msgs = [c.args[0] for c in w.post_message.call_args_list] + picked = [m for m in msgs if isinstance(m, ModelPickerWidget.Picked)] + assert len(picked) == 1 + assert picked[0].name == "mymodel" + assert picked[0].provider == "ollama" + + if __name__ == "__main__": unittest.main()