From 904133cb25b710b9217474a6b2ff9b0dd86816e9 Mon Sep 17 00:00:00 2001 From: Elliot Slusky <44592435+ElliotSlusky@users.noreply.github.com> Date: Tue, 30 Jun 2026 13:46:18 -0700 Subject: [PATCH] fix(research): respect configured engine for Deep Research (#616) Fixes #575. Web Deep Research was hardcoded to OllamaEngine + DEFAULT_PLANNER_MODEL, ignoring the user's configured/active engine and model. Resolve the planner from [deep_research] override -> live app chat engine + selected model -> config defaults -> legacy Ollama, pass the chat picker's model from the frontend into /api/research, record the actual planner engine in telemetry, and refuse to silently fall back to a different engine (raise an actionable error instead). Adds config support and focused tests for resolution and the route. Related: #576 (duplicate). Co-Authored-By: Claude Opus 4.8 (1M context) --- frontend/src/components/Chat/InputArea.tsx | 6 +- frontend/src/lib/sse.ts | 4 +- src/openjarvis/agents/research_loop.py | 3 +- src/openjarvis/core/config.py | 15 ++ src/openjarvis/server/research_router.py | 147 +++++++++-- tests/core/test_deep_research_config.py | 58 +++++ tests/server/test_research_planner.py | 285 +++++++++++++++++++++ 7 files changed, 494 insertions(+), 24 deletions(-) create mode 100644 tests/core/test_deep_research_config.py create mode 100644 tests/server/test_research_planner.py diff --git a/frontend/src/components/Chat/InputArea.tsx b/frontend/src/components/Chat/InputArea.tsx index fecaacb0..a3ed970b 100644 --- a/frontend/src/components/Chat/InputArea.tsx +++ b/frontend/src/components/Chat/InputArea.tsx @@ -243,7 +243,11 @@ export function InputArea() { try { if (deepResearch) { - for await (const ev of streamResearch(content, controller.signal)) { + for await (const ev of streamResearch( + content, + selectedModel, + controller.signal, + )) { if (ev.type === 'search_call') { const trace: ResearchSearchTrace = { id: generateId(), diff --git a/frontend/src/lib/sse.ts b/frontend/src/lib/sse.ts index 7d5933ea..f4368d5e 100644 --- a/frontend/src/lib/sse.ts +++ b/frontend/src/lib/sse.ts @@ -60,6 +60,7 @@ export async function* streamChat( export async function* streamResearch( query: string, + model?: string, signal?: AbortSignal, ): AsyncGenerator { // /api/research is mounted at the server root — strip any trailing /v1 @@ -68,7 +69,7 @@ export async function* streamResearch( const response = await fetch(`${base}/api/research`, { method: 'POST', headers: authHeaders({ 'Content-Type': 'application/json' }), - body: JSON.stringify({ query }), + body: JSON.stringify({ query, ...(model ? { model } : {}) }), signal, }); @@ -106,4 +107,3 @@ export async function* streamResearch( reader.releaseLock(); } } - diff --git a/src/openjarvis/agents/research_loop.py b/src/openjarvis/agents/research_loop.py index 505544c2..97814ef3 100644 --- a/src/openjarvis/agents/research_loop.py +++ b/src/openjarvis/agents/research_loop.py @@ -2,7 +2,8 @@ A small, self-contained planner-executor loop: -* the planner is a local Ollama chat model (default ``gemma4:31b``), +* the planner is supplied by the caller (the web endpoint resolves it from + config, falling back to ``gemma4:31b`` on Ollama for legacy installs), * the only tool it can call is :meth:`HybridSearch.search`, * it gets up to ``max_iterations`` tool calls, * tool results are trimmed before re-entering the context window, and diff --git a/src/openjarvis/core/config.py b/src/openjarvis/core/config.py index bafd4788..593bfa1d 100644 --- a/src/openjarvis/core/config.py +++ b/src/openjarvis/core/config.py @@ -593,6 +593,14 @@ class IntelligenceConfig: stop_sequences: str = "" # Comma-separated stop strings +@dataclass(slots=True) +class DeepResearchConfig: + """Planner settings for the web Deep Research endpoint.""" + + engine: str = "" # Empty means use the active chat engine. + model: str = "" # Empty means use the active chat model. + + @dataclass(slots=True) class RoutingLearningConfig: """Routing sub-policy config within Learning.""" @@ -1578,6 +1586,7 @@ class JarvisConfig: hardware: HardwareInfo = field(default_factory=HardwareInfo) engine: EngineConfig = field(default_factory=EngineConfig) intelligence: IntelligenceConfig = field(default_factory=IntelligenceConfig) + deep_research: DeepResearchConfig = field(default_factory=DeepResearchConfig) learning: LearningConfig = field(default_factory=LearningConfig) tools: ToolsConfig = field(default_factory=ToolsConfig) agent: AgentConfig = field(default_factory=AgentConfig) @@ -1839,6 +1848,7 @@ def load_config(path: Optional[Path] = None) -> JarvisConfig: top_sections = ( "engine", "intelligence", + "deep_research", "learning", "agent", "server", @@ -2007,6 +2017,10 @@ max_tokens = 1024 # repetition_penalty = 1.0 # stop_sequences = "" +# [deep_research] +# engine = "" # empty = use [engine].default +# model = "" # empty = use [intelligence].default_model + [agent] default_agent = "simple" max_turns = 10 @@ -2177,6 +2191,7 @@ __all__ = [ "DEFAULT_CONFIG_DIR", "DEFAULT_CONFIG_PATH", "DiscordChannelConfig", + "DeepResearchConfig", "get_cache_dir", "get_config_dir", "get_config_path", diff --git a/src/openjarvis/server/research_router.py b/src/openjarvis/server/research_router.py index c462cadf..500c4fcf 100644 --- a/src/openjarvis/server/research_router.py +++ b/src/openjarvis/server/research_router.py @@ -27,7 +27,7 @@ import threading import time from typing import Any, AsyncGenerator, Callable, Dict, List, Optional -from fastapi import APIRouter +from fastapi import APIRouter, Request from fastapi.responses import StreamingResponse from pydantic import BaseModel, Field @@ -38,9 +38,10 @@ from openjarvis.agents.research_loop import ( from openjarvis.connectors.embeddings import OllamaEmbedder from openjarvis.connectors.hybrid_search import HybridSearch from openjarvis.connectors.store import KnowledgeStore -from openjarvis.core.config import DEFAULT_CONFIG_DIR +from openjarvis.core.config import DEFAULT_CONFIG_DIR, JarvisConfig, load_config from openjarvis.core.types import TelemetryRecord -from openjarvis.engine.ollama import OllamaEngine +from openjarvis.engine._base import InferenceEngine +from openjarvis.engine._discovery import get_engine from openjarvis.telemetry.store import TelemetryStore logger = logging.getLogger(__name__) @@ -48,13 +49,99 @@ logger = logging.getLogger(__name__) router = APIRouter(prefix="/api", tags=["research"]) _WEB_CLARIFY_RESPONSE = "no clarification available in web session" +_LEGACY_PLANNER_ENGINE = "ollama" # Sentinel placed on the queue when the agent thread terminates. _DONE = object() +def _first_nonempty(*values: str) -> str: + for value in values: + stripped = value.strip() + if stripped: + return stripped + return "" + + +def _resolve_planner_config( + config: JarvisConfig, + *, + active_engine_key: str = "", + active_model: str = "", + request_model: str = "", +) -> tuple[str, str]: + """Resolve the planner engine/model for web Deep Research. + + Resolution order: + + 1. explicit ``[deep_research]`` overrides, + 2. the active chat engine/request model, + 3. server/config defaults, + 4. legacy Ollama/gemma4 fallback for unconfigured installs. + """ + engine_key = _first_nonempty( + config.deep_research.engine, + active_engine_key, + config.engine.default, + _LEGACY_PLANNER_ENGINE, + ) + model = _first_nonempty( + config.deep_research.model, + request_model, + active_model, + config.server.model, + config.intelligence.default_model, + DEFAULT_PLANNER_MODEL, + ) + return engine_key, model + + +def _build_planner_engine( + config: JarvisConfig, + *, + active_engine: InferenceEngine | None = None, + active_engine_key: str = "", + active_model: str = "", + request_model: str = "", +) -> tuple[str, InferenceEngine, str]: + """Instantiate the exact configured planner engine. + + ``get_engine`` intentionally falls back to any healthy engine for general + chat routing. Deep Research must not do that here: if the configured chat + engine is LM Studio but unavailable, silently falling back to Ollama would + recreate the issue this endpoint is fixing. + """ + engine_key, model = _resolve_planner_config( + config, + active_engine_key=active_engine_key, + active_model=active_model, + request_model=request_model, + ) + if active_engine is not None and not config.deep_research.engine.strip(): + if model and not active_engine.can_serve(model): + raise RuntimeError( + "Deep Research planner engine " + f"{engine_key!r} cannot serve model {model!r}. " + "Choose a compatible model or set [deep_research] engine/model " + "in config.toml." + ) + return engine_key, active_engine, model + + resolved = get_engine(config, engine_key=engine_key, model=model) + if resolved is None or resolved[0] != engine_key: + raise RuntimeError( + "Deep Research planner engine " + f"{engine_key!r} is unavailable or cannot serve model {model!r}. " + "Start the configured engine, load the configured model, or set " + "[deep_research] engine/model in config.toml." + ) + resolved_key, engine = resolved + return resolved_key, engine, model + + def _record_research_telemetry( *, + engine_key: str, model: str, usage: Dict[str, int], latency_seconds: float, @@ -86,7 +173,7 @@ def _record_research_telemetry( rec = TelemetryRecord( timestamp=time.time(), model_id=model, - engine="ollama", + engine=engine_key, agent="research", prompt_tokens=int(usage.get("prompt_tokens", 0)), prompt_tokens_evaluated=int(usage.get("prompt_tokens", 0)), @@ -244,12 +331,11 @@ class _LiveGPUSampler: class ResearchRequest(BaseModel): query: str = Field(..., description="Natural-language question to research.") - # Deep Research has its own model requirements (function-calling support, - # sufficient reasoning capability) that the chat-model selector should not - # override. We accept the field for forward-compat with older clients but - # ignore it — the planner always runs on DEFAULT_PLANNER_MODEL. + # Preferred planner model from the active chat selector. Server-side + # [deep_research] config can still override it when a dedicated planner is + # desired. model: Optional[str] = Field( - default=None, description="Ignored; retained for client compatibility." + default=None, description="Preferred planner model for this request." ) @@ -290,7 +376,14 @@ def _chunk_synthesis(text: str, window_chars: int = 40) -> list[str]: # --------------------------------------------------------------------------- -async def _stream_research(query: str, model: str) -> AsyncGenerator[str, None]: +async def _stream_research( + query: str, + *, + active_engine: InferenceEngine | None = None, + active_engine_key: str = "", + active_model: str = "", + request_model: str = "", +) -> AsyncGenerator[str, None]: """Drive ResearchAgent on a worker thread; yield SSE frames as they land. Three error envelopes — setup, worker, consumer — all funnel into the @@ -298,7 +391,7 @@ async def _stream_research(query: str, model: str) -> AsyncGenerator[str, None]: ``{"type": "done", "usage": {...}}``. The client can rely on always seeing a ``done`` frame, even when the agent never started. """ - # Phase 1: setup. Failures here (Ollama daemon down, DB locked, etc.) + # Phase 1: setup. Failures here (planner engine down, DB locked, etc.) # yield error + done and return — nothing has been emitted yet so the # client gets a clean two-frame stream instead of a dangling connection. try: @@ -309,6 +402,15 @@ async def _stream_research(query: str, model: str) -> AsyncGenerator[str, None]: # Called from the agent's worker thread; bounce onto the event loop. loop.call_soon_threadsafe(queue.put_nowait, event) + config = load_config() + engine_key, engine, model = _build_planner_engine( + config, + active_engine=active_engine, + active_engine_key=active_engine_key, + active_model=active_model, + request_model=request_model, + ) + # Each request gets its own thin set of connectors. Constructing them # is cheap (SQLite open + HTTP keepalive) and avoids state leaks # between concurrent requests. @@ -320,7 +422,6 @@ async def _stream_research(query: str, model: str) -> AsyncGenerator[str, None]: ) embedder = None - engine = OllamaEngine() agent = ResearchAgent( engine=engine, search=HybridSearch(store, embedder), @@ -367,6 +468,7 @@ async def _stream_research(query: str, model: str) -> AsyncGenerator[str, None]: # rolls research into the same Power/Energy numbers as chat — # this is what the launch-video System panel reads. _record_research_telemetry( + engine_key=engine_key, model=model, usage=usage_dict, latency_seconds=time.time() - t0, @@ -472,7 +574,7 @@ async def _stream_research(query: str, model: str) -> AsyncGenerator[str, None]: @router.post("/research") -async def research(req: ResearchRequest) -> StreamingResponse: +async def research(req: ResearchRequest, request: Request) -> StreamingResponse: """Run a research query and stream the agent's trace + synthesis via SSE. Response is ``text/event-stream`` with one JSON event per frame. See the @@ -480,14 +582,19 @@ async def research(req: ResearchRequest) -> StreamingResponse: terminates the stream so clients can detect end-of-response without parsing the underlying ``[DONE]`` sentinel used by OpenAI-style routes. """ - if req.model and req.model != DEFAULT_PLANNER_MODEL: - logger.info( - "research: ignoring client model=%r; using DEFAULT_PLANNER_MODEL=%r", - req.model, - DEFAULT_PLANNER_MODEL, - ) + active_engine = getattr(request.app.state, "engine", None) + active_model = str(getattr(request.app.state, "model", "") or "") + active_engine_key = str(getattr(request.app.state, "engine_name", "") or "") + if active_engine is not None and not active_engine_key: + active_engine_key = str(getattr(active_engine, "engine_id", "") or "") return StreamingResponse( - _stream_research(req.query, DEFAULT_PLANNER_MODEL), + _stream_research( + req.query, + active_engine=active_engine, + active_engine_key=active_engine_key, + active_model=active_model, + request_model=req.model or "", + ), media_type="text/event-stream", headers={ "Cache-Control": "no-cache", diff --git a/tests/core/test_deep_research_config.py b/tests/core/test_deep_research_config.py new file mode 100644 index 00000000..76a58dd3 --- /dev/null +++ b/tests/core/test_deep_research_config.py @@ -0,0 +1,58 @@ +"""Tests for Deep Research planner configuration.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from openjarvis.core.config import ( + DeepResearchConfig, + HardwareInfo, + JarvisConfig, + generate_default_toml, + load_config, + validate_config_key, +) + + +def test_deep_research_config_defaults_to_chat_selection() -> None: + cfg = JarvisConfig() + + assert isinstance(cfg.deep_research, DeepResearchConfig) + assert cfg.deep_research.engine == "" + assert cfg.deep_research.model == "" + + +def test_loads_deep_research_overrides( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setenv("OPENJARVIS_HOME", str(tmp_path / "home")) + config_file = tmp_path / "config.toml" + config_file.write_text( + "\n".join( + [ + "[deep_research]", + 'engine = "lmstudio"', + 'model = "qwen/qwen3-14b"', + ] + ) + ) + + cfg = load_config(config_file) + + assert cfg.deep_research.engine == "lmstudio" + assert cfg.deep_research.model == "qwen/qwen3-14b" + + +def test_deep_research_keys_are_settable() -> None: + assert validate_config_key("deep_research.engine") is str + assert validate_config_key("deep_research.model") is str + + +def test_default_toml_documents_deep_research_override() -> None: + toml = generate_default_toml(HardwareInfo()) + + assert "# [deep_research]" in toml + assert '# engine = ""' in toml + assert '# model = ""' in toml diff --git a/tests/server/test_research_planner.py b/tests/server/test_research_planner.py new file mode 100644 index 00000000..11a515b1 --- /dev/null +++ b/tests/server/test_research_planner.py @@ -0,0 +1,285 @@ +"""Tests for web Deep Research planner engine selection.""" + +from __future__ import annotations + +import asyncio +from types import SimpleNamespace + +import pytest + +from openjarvis.agents.research_loop import DEFAULT_PLANNER_MODEL +from openjarvis.core.config import JarvisConfig +from openjarvis.server import research_router + + +class _DummyEngine: + def __init__(self, servable: bool = True) -> None: + self.servable = servable + + def can_serve(self, model: str) -> bool: + return self.servable + + +def test_resolve_planner_config_uses_chat_defaults() -> None: + cfg = JarvisConfig() + cfg.engine.default = "lmstudio" + cfg.intelligence.default_model = "local-model" + + assert research_router._resolve_planner_config(cfg) == ( + "lmstudio", + "local-model", + ) + + +def test_resolve_planner_config_prefers_active_chat_runtime() -> None: + cfg = JarvisConfig() + cfg.engine.default = "ollama" + cfg.intelligence.default_model = "" + + assert research_router._resolve_planner_config( + cfg, + active_engine_key="lmstudio", + active_model="server-model", + request_model="selected-model", + ) == ( + "lmstudio", + "selected-model", + ) + + +def test_resolve_planner_config_uses_server_model_before_legacy_default() -> None: + cfg = JarvisConfig() + cfg.engine.default = "ollama" + cfg.intelligence.default_model = "" + cfg.server.model = "serve-model" + + assert research_router._resolve_planner_config(cfg) == ( + "ollama", + "serve-model", + ) + + +def test_resolve_planner_config_allows_deep_research_override() -> None: + cfg = JarvisConfig() + cfg.engine.default = "lmstudio" + cfg.intelligence.default_model = "chat-model" + cfg.deep_research.engine = "vllm" + cfg.deep_research.model = "planner-model" + + assert research_router._resolve_planner_config(cfg) == ( + "vllm", + "planner-model", + ) + + +def test_resolve_planner_config_allows_partial_model_override() -> None: + cfg = JarvisConfig() + cfg.engine.default = "lmstudio" + cfg.intelligence.default_model = "chat-model" + cfg.deep_research.model = "planner-model" + + assert research_router._resolve_planner_config(cfg) == ( + "lmstudio", + "planner-model", + ) + + +def test_resolve_planner_config_allows_partial_engine_override() -> None: + cfg = JarvisConfig() + cfg.engine.default = "lmstudio" + cfg.intelligence.default_model = "chat-model" + cfg.deep_research.engine = "vllm" + + assert research_router._resolve_planner_config(cfg) == ( + "vllm", + "chat-model", + ) + + +def test_resolve_planner_config_keeps_legacy_fallback_when_unconfigured() -> None: + cfg = JarvisConfig() + cfg.engine.default = "" + cfg.intelligence.default_model = "" + + assert research_router._resolve_planner_config(cfg) == ( + "ollama", + DEFAULT_PLANNER_MODEL, + ) + + +def test_build_planner_engine_uses_configured_engine( + monkeypatch: pytest.MonkeyPatch, +) -> None: + cfg = JarvisConfig() + cfg.engine.default = "lmstudio" + cfg.intelligence.default_model = "local-model" + engine = _DummyEngine() + calls: list[tuple[str | None, str | None]] = [] + + def fake_get_engine( + config: JarvisConfig, + engine_key: str | None = None, + model: str | None = None, + ) -> tuple[str, _DummyEngine]: + calls.append((engine_key, model)) + return "lmstudio", engine + + monkeypatch.setattr(research_router, "get_engine", fake_get_engine) + + engine_key, resolved_engine, model = research_router._build_planner_engine(cfg) + + assert calls == [("lmstudio", "local-model")] + assert engine_key == "lmstudio" + assert resolved_engine is engine + assert model == "local-model" + + +def test_build_planner_engine_uses_active_engine_without_config_fallback( + monkeypatch: pytest.MonkeyPatch, +) -> None: + cfg = JarvisConfig() + cfg.engine.default = "ollama" + cfg.intelligence.default_model = "" + active_engine = _DummyEngine() + + def fail_get_engine(*args: object, **kwargs: object) -> None: + raise AssertionError("should use the live app engine") + + monkeypatch.setattr(research_router, "get_engine", fail_get_engine) + + engine_key, resolved_engine, model = research_router._build_planner_engine( + cfg, + active_engine=active_engine, + active_engine_key="lmstudio", + active_model="server-model", + request_model="selected-model", + ) + + assert engine_key == "lmstudio" + assert resolved_engine is active_engine + assert model == "selected-model" + + +def test_build_planner_engine_rejects_active_engine_that_cannot_serve_model() -> None: + cfg = JarvisConfig() + + with pytest.raises(RuntimeError, match="selected-model"): + research_router._build_planner_engine( + cfg, + active_engine=_DummyEngine(servable=False), + active_engine_key="cloud", + request_model="selected-model", + ) + + +def test_build_planner_engine_honors_explicit_deep_research_engine( + monkeypatch: pytest.MonkeyPatch, +) -> None: + cfg = JarvisConfig() + cfg.deep_research.engine = "vllm" + cfg.deep_research.model = "planner-model" + active_engine = _DummyEngine() + planner_engine = _DummyEngine() + + def fake_get_engine( + config: JarvisConfig, + engine_key: str | None = None, + model: str | None = None, + ) -> tuple[str, _DummyEngine]: + assert engine_key == "vllm" + assert model == "planner-model" + return "vllm", planner_engine + + monkeypatch.setattr(research_router, "get_engine", fake_get_engine) + + engine_key, resolved_engine, model = research_router._build_planner_engine( + cfg, + active_engine=active_engine, + active_engine_key="lmstudio", + active_model="chat-model", + request_model="selected-model", + ) + + assert engine_key == "vllm" + assert resolved_engine is planner_engine + assert model == "planner-model" + + +def test_research_route_passes_live_engine_and_selected_model( + monkeypatch: pytest.MonkeyPatch, +) -> None: + captured: dict[str, object] = {} + active_engine = _DummyEngine() + + def fake_stream(query: str, **kwargs: object): + captured["query"] = query + captured.update(kwargs) + + async def gen(): + yield "data: {\"type\":\"done\",\"usage\":{}}\n\n" + + return gen() + + request = SimpleNamespace( + app=SimpleNamespace( + state=SimpleNamespace( + engine=active_engine, + engine_name="lmstudio", + model="server-model", + ) + ) + ) + + monkeypatch.setattr(research_router, "_stream_research", fake_stream) + + response = asyncio.run( + research_router.research( + research_router.ResearchRequest( + query="find notes", + model="selected-model", + ), + request, # type: ignore[arg-type] + ) + ) + + assert response.media_type == "text/event-stream" + assert captured == { + "query": "find notes", + "active_engine": active_engine, + "active_engine_key": "lmstudio", + "active_model": "server-model", + "request_model": "selected-model", + } + + +def test_build_planner_engine_rejects_fallback_engine( + monkeypatch: pytest.MonkeyPatch, +) -> None: + cfg = JarvisConfig() + cfg.engine.default = "lmstudio" + cfg.intelligence.default_model = "local-model" + + def fake_get_engine( + config: JarvisConfig, + engine_key: str | None = None, + model: str | None = None, + ) -> tuple[str, _DummyEngine]: + return "ollama", _DummyEngine() + + monkeypatch.setattr(research_router, "get_engine", fake_get_engine) + + with pytest.raises(RuntimeError, match="lmstudio"): + research_router._build_planner_engine(cfg) + + +def test_build_planner_engine_rejects_unavailable_engine( + monkeypatch: pytest.MonkeyPatch, +) -> None: + cfg = JarvisConfig() + cfg.engine.default = "lmstudio" + cfg.intelligence.default_model = "local-model" + + monkeypatch.setattr(research_router, "get_engine", lambda *args, **kwargs: None) + + with pytest.raises(RuntimeError, match="local-model"): + research_router._build_planner_engine(cfg)