diff --git a/docs/testing.md b/docs/testing.md index f8a2091..fd9e467 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -22,17 +22,22 @@ │ │ ├── test_events.py # 17 tests │ │ ├── test_context_builder.py │ │ ├── test_compressor.py -│ │ ├── test_registry.py +│ │ ├── test_registry.py # registries, backend discovery, context provider registry │ │ ├── test_planning.py │ │ ├── test_agent_context_size.py │ │ └── test_agent_stream_guard.py +│ ├── llm/ +│ │ └── test_ollama.py # timeout/error classification + fallback timeout wiring │ ├── memory/ │ │ ├── test_store.py │ │ └── test_extractor.py │ ├── tools/ │ │ ├── test_filesystem.py │ │ ├── test_code_exec.py -│ │ └── test_terminal.py +│ │ ├── test_terminal.py +│ │ ├── test_share_file.py +│ │ └── test_content_publish.py +│ └── test_content_store.py │ ├── profiles/ │ │ └── test_base.py │ └── config/ @@ -90,7 +95,7 @@ |-------|--------|-------|--------| | 1 | `navi.core.events` | 17 | ✅ Done | | 1 | `navi.core.compressor` | 14 | ✅ Done | -| 1 | `navi.core.registry` | 10 | ✅ Done | +| 1 | `navi.core.registry` + `ContextProviderRegistry` | 13 | ✅ Done | | 1 | `navi.core.context_builder` | 9 | ✅ Done | | 1 | `navi.profiles.base` | 9 | ✅ Done | | 2 | `navi.memory.store` | 18 | ✅ Done | @@ -102,12 +107,82 @@ | 5 | `navi.tools.filesystem` | 13 | ✅ Done | | 5 | `navi.tools.code_exec` | 5 | ✅ Done | | 5 | `navi.tools.terminal` | 4 | ✅ Done | +| 5 | `navi.tools.share_file` | 4 | ✅ Basic | +| 5 | `navi.tools.content_publish` | 4 | ✅ Basic | +| 5 | `navi.content_store` | 3 | ✅ Basic | +| 5 | `navi.llm.ollama` + fallback timeout wiring | 3 | ✅ Basic | | 6 | `webclient/api` | 8 | ✅ Done | | 6 | `webclient/stores/chat` | 23 | ✅ Done | | 6 | `webclient/stores/sessions` | 6 | ✅ Done | | 6 | `webclient/stores/profiles` | 3 | ✅ Done | | 6 | `webclient/composables/useWebSocket` | 7 | ✅ Done | +## Coverage roadmap + +This is the living plan for what still needs tests. Keep it updated whenever a +bug is fixed, a new module is added, or a planned area becomes covered. + +Status meanings: +- ✅ Covered enough for current risk +- 🟨 Basic coverage exists, important edge cases remain +- ⬜ Not covered yet +- 🔴 Regression target from a real bug + +### Phase 7 — Recent Regression Coverage + +| Priority | Area | Target tests | Status | +|---|---|---|---| +| P0 | `navi.content_store.ensure_tables()` | creates `session_content`, creates `idx_session_content_file`, is idempotent when index already exists | 🟨 | +| P0 | `navi.content_store.publish()` | repeated publish of same `(session_id, filename)` updates one row instead of creating duplicates | 🟨 🔴 | +| P0 | `navi.main` startup | registries are initialized before `_check_embed()` so memory has an embedding backend | ⬜ 🔴 | +| P0 | `navi.core.registry._discover_backends()` | primary Ollama backend receives HTTP timeout >= `LLM_COMPLETE_TIMEOUT` and `LLM_STREAM_FIRST_CHUNK_TIMEOUT` | ✅ 🔴 | +| P0 | `navi.llm.fallback.FallbackOllamaBackend` | per-server `OllamaBackend` clients receive the same expanded timeout | ✅ 🔴 | +| P1 | `navi.tools.content_publish` | missing file, directory instead of file, successful publish metadata, filename path stripping | ✅ | +| P1 | `navi.tools.share_file` | duplicate filename collision produces numbered output without overwrite | ⬜ | +| P1 | `navi.api.routes.sessions` file endpoints | upload duplicate naming, forbidden extension, download path traversal, content disposition | 🟨 | + +### Phase 8 — Agent Loop Behavior + +| Priority | Area | Target tests | Status | +|---|---|---|---| +| P0 | `Agent.run_stream()` planning entry | first user message forces planning; later messages follow `planning_enabled` | ⬜ | +| P0 | plan → todo bridge | numbered plan steps auto-populate todo exactly once per plan | ⬜ | +| P0 | stop handling | stop during stream prefill yields `StreamStopped` and closes LLM generator | 🟨 | +| P1 | subagent forwarding | parent forwards subagent tool events and counts subagent tokens/tool calls | ⬜ | +| P1 | adaptive replan | newly failed todo step injects replan prompt on next iteration | ⬜ | +| P1 | anti-stall | repeated tool calls or no todo progress inject warning after threshold | ⬜ | +| P1 | workers | post-turn workers run after `StreamEnd`; worker failure is logged and non-fatal | ⬜ | + +### Phase 9 — Memory And Embeddings + +| Priority | Area | Target tests | Status | +|---|---|---|---| +| P0 | embedding backend wiring | `get_registries()` wires dedicated `EMBEDDING_OLLAMA_HOST` backend into `MemoryStore` | ⬜ | +| P0 | pgvector detection | `_has_pgvector()` true/false paths and caching behavior | ⬜ | +| P1 | embedding generation | invalid/empty/NaN vectors are skipped before PostgreSQL update | 🟨 | +| P1 | backfill | `backfill_embeddings()` batches rows and only updates rows with valid vectors | 🟨 | +| P1 | search | vector search falls back to ILIKE when vector search unavailable or empty | 🟨 | + +### Phase 10 — WebSocket And API Lifecycles + +| Priority | Area | Target tests | Status | +|---|---|---|---| +| P0 | active run guard | duplicate message while a run is active returns `run_already_active` | 🟨 | +| P0 | reconnect replay | reconnect receives missed events and session sync after finished run | 🟨 | +| P1 | stop endpoint | `POST /sessions/{id}/stop` sets stop event and is idempotent | 🟨 | +| P1 | malformed input | oversize images, invalid file refs, and non-string payloads are rejected or sanitized | ⬜ | +| P1 | startup cleanup | session file cleanup task is started once and deletes orphaned dirs | ⬜ | + +### Phase 11 — Frontend + +| Priority | Area | Target tests | Status | +|---|---|---|---| +| P0 | `ContentCard.vue` | renders download links and inline viewer links; handles encoded filenames | ⬜ | +| P0 | streaming chat | auto-scroll and streaming message updates stay reactive | 🟨 | +| P1 | session switching | concurrent session loads cannot overwrite active session with stale response | 🟨 | +| P1 | error surfaces | API/store failures show recoverable UI state, no unhandled rejection | 🟨 | +| P1 | file upload UI | upload success/failure, duplicate names, large-file errors | ⬜ | + ## Running tests ```bash diff --git a/navi/context_providers/_loader.py b/navi/context_providers/_loader.py index e5e67c1..03f53b3 100644 --- a/navi/context_providers/_loader.py +++ b/navi/context_providers/_loader.py @@ -45,6 +45,9 @@ def all_names(self) -> list[str]: return list(self._providers) + def all(self) -> list[Any]: + return list(self._providers.values()) + def get_globals(self) -> list[Any]: return [p for p in self._providers.values() if getattr(p, "global_provider", False)] diff --git a/navi/core/registry.py b/navi/core/registry.py index 4cb3940..99f6820 100644 --- a/navi/core/registry.py +++ b/navi/core/registry.py @@ -116,6 +116,11 @@ from navi.llm.ollama import OllamaBackend from navi.llm.fallback import FallbackOllamaBackend from navi.llm.openai_backend import OpenAIBackend + ollama_http_timeout = max( + settings.ollama_request_timeout, + settings.llm_complete_timeout, + settings.llm_stream_first_chunk_timeout, + ) # Ollama backend (primary) if settings.ollama_backends_file: @@ -126,6 +131,7 @@ model=settings.ollama_default_model, host=settings.ollama_host, api_key=settings.ollama_api_key, + timeout=ollama_http_timeout, ))) # OpenAI backend (if configured) diff --git a/navi/llm/fallback.py b/navi/llm/fallback.py index 163d87f..9bb8106 100644 --- a/navi/llm/fallback.py +++ b/navi/llm/fallback.py @@ -79,9 +79,14 @@ def _get_client(self, server: ServerEntry) -> OllamaBackend: from navi.config import settings if server.host not in self._clients: + ollama_http_timeout = max( + settings.ollama_request_timeout, + settings.llm_complete_timeout, + settings.llm_stream_first_chunk_timeout, + ) self._clients[server.host] = OllamaBackend( model="", host=server.host, api_key=server.api_key, - timeout=settings.ollama_request_timeout, + timeout=ollama_http_timeout, ) return self._clients[server.host] diff --git a/navi/llm/ollama.py b/navi/llm/ollama.py index aa5db4c..5e0146c 100644 --- a/navi/llm/ollama.py +++ b/navi/llm/ollama.py @@ -71,7 +71,10 @@ return LLMModelNotFoundError(str(e)) return LLMBackendError(str(e)) # Catch httpx / socket connection failures by message + err_type = type(e).__name__.lower() err_str = str(e).lower() + if "timeout" in err_type: + return LLMConnectionError(str(e) or type(e).__name__) if any(kw in err_str for kw in ("connect", "connection refused", "name or service not known", "network", "timeout", "unreachable", "nodename")): return LLMConnectionError(str(e)) diff --git a/tests/unit/core/test_registry.py b/tests/unit/core/test_registry.py index 9737488..bed9e98 100644 --- a/tests/unit/core/test_registry.py +++ b/tests/unit/core/test_registry.py @@ -3,6 +3,7 @@ import pytest from navi.core.registry import BackendRegistry, ProfileRegistry, ToolRegistry +from navi.context_providers import ContextProviderRegistry from navi.exceptions import ProfileNotFound, ToolNotFound from tests.conftest_factory import FakeLLMBackend, FakeTool, make_profile @@ -77,3 +78,45 @@ reg.register("a", FakeLLMBackend()) reg.register("b", FakeLLMBackend()) assert sorted(reg.all_keys()) == ["a", "b"] + + +class TestBackendDiscovery: + def test_primary_ollama_uses_expanded_timeout(self, monkeypatch): + import navi.core.registry as registry_mod + + captured = {} + + class FakeOllamaBackend: + def __init__(self, **kwargs): + captured.update(kwargs) + + monkeypatch.setattr(registry_mod.settings, "ollama_backends_file", "") + monkeypatch.setattr(registry_mod.settings, "openai_api_key", "") + monkeypatch.setattr(registry_mod.settings, "ollama_request_timeout", 30) + monkeypatch.setattr(registry_mod.settings, "llm_complete_timeout", 120) + monkeypatch.setattr(registry_mod.settings, "llm_stream_first_chunk_timeout", 180) + + import navi.llm.ollama as ollama_mod + + monkeypatch.setattr(ollama_mod, "OllamaBackend", FakeOllamaBackend) + + discovered = registry_mod._discover_backends() + + assert discovered[0][0] == "ollama" + assert captured["timeout"] == 180 + + +class TestContextProviderRegistry: + def test_all_returns_registered_providers(self): + reg = ContextProviderRegistry() + + class Provider: + name = "provider" + description = "Provider" + + async def get_context(self): + return None + + reg.register(Provider) + + assert reg.all() == [Provider] diff --git a/tests/unit/llm/test_ollama.py b/tests/unit/llm/test_ollama.py new file mode 100644 index 0000000..e3ed91b --- /dev/null +++ b/tests/unit/llm/test_ollama.py @@ -0,0 +1,43 @@ +"""Unit tests for Ollama backend helpers.""" + +import httpx + +from navi.exceptions import LLMConnectionError +from navi.llm.fallback import FallbackOllamaBackend, ServerEntry +from navi.llm.ollama import _classify_error + + +def test_classify_read_timeout_as_connection_error(): + err = _classify_error(httpx.ReadTimeout("timed out")) + + assert isinstance(err, LLMConnectionError) + + +def test_classify_empty_timeout_message_as_connection_error(): + err = _classify_error(httpx.ReadTimeout("")) + + assert isinstance(err, LLMConnectionError) + assert str(err) == "ReadTimeout" + + +def test_fallback_client_uses_expanded_timeout(monkeypatch): + import navi.llm.fallback as fallback_mod + import navi.config as config_mod + + captured = {} + + class FakeOllamaBackend: + def __init__(self, **kwargs): + captured.update(kwargs) + + monkeypatch.setattr(fallback_mod, "OllamaBackend", FakeOllamaBackend) + monkeypatch.setattr(config_mod.settings, "ollama_request_timeout", 30) + monkeypatch.setattr(config_mod.settings, "llm_complete_timeout", 120) + monkeypatch.setattr(config_mod.settings, "llm_stream_first_chunk_timeout", 180) + + backend = FallbackOllamaBackend([ServerEntry(host="http://ollama.test")]) + backend._get_client(ServerEntry(host="http://ollama.test", api_key="secret")) + + assert captured["host"] == "http://ollama.test" + assert captured["api_key"] == "secret" + assert captured["timeout"] == 180 diff --git a/tests/unit/test_content_store.py b/tests/unit/test_content_store.py new file mode 100644 index 0000000..121800a --- /dev/null +++ b/tests/unit/test_content_store.py @@ -0,0 +1,65 @@ +"""Unit tests for session content metadata store.""" + +import pytest + +import navi.content_store as content_store +from tests.conftest_factory import FakeConnection, FakePool + + +class TestEnsureTables: + async def test_creates_session_content_table_and_indexes(self, monkeypatch): + conn = FakeConnection() + + async def fake_pool(): + return FakePool(conn) + + monkeypatch.setattr(content_store, "_get_db_pool", fake_pool) + + await content_store.ensure_tables() + + executed = "\n".join(call[1] for call in conn.calls if call[0] == "execute") + assert "CREATE TABLE IF NOT EXISTS session_content" in executed + assert "idx_session_content_session" in executed + assert "CREATE UNIQUE INDEX IF NOT EXISTS idx_session_content_file" in executed + assert "ON session_content (session_id, filename)" in executed + + +class TestPublish: + @pytest.fixture(autouse=True) + def _session_dir(self, monkeypatch, tmp_path): + monkeypatch.setattr(content_store.settings, "session_files_dir", str(tmp_path / "sessions")) + monkeypatch.setattr(content_store.settings, "public_url", "http://localhost:8000") + + async def test_upserts_by_session_and_filename(self, monkeypatch, tmp_path): + conn = FakeConnection() + conn.enqueue("INSERT 0 1") + + async def fake_pool(): + return FakePool(conn) + + monkeypatch.setattr(content_store, "_get_db_pool", fake_pool) + sess_dir = tmp_path / "sessions" / "sess-1" + sess_dir.mkdir(parents=True) + (sess_dir / "logo.svg").write_text("") + + info = await content_store.publish( + session_id="sess-1", + filename="logo.svg", + title="Logo", + ) + + assert info["id"] == "sess-1_logo.svg" + assert info["url"] == "http://localhost:8000/sessions/sess-1/files/logo.svg" + call = conn.calls[0] + assert call[0] == "execute" + assert "ON CONFLICT (session_id, filename) DO UPDATE" in call[1] + assert call[2][1:5] == ("sess-1", "logo.svg", "svg", "Logo") + + async def test_missing_file_raises(self, monkeypatch, tmp_path): + async def fake_pool(): + return FakePool(FakeConnection()) + + monkeypatch.setattr(content_store, "_get_db_pool", fake_pool) + + with pytest.raises(FileNotFoundError): + await content_store.publish(session_id="sess-1", filename="missing.svg") diff --git a/tests/unit/tools/test_content_publish.py b/tests/unit/tools/test_content_publish.py new file mode 100644 index 0000000..6471647 --- /dev/null +++ b/tests/unit/tools/test_content_publish.py @@ -0,0 +1,78 @@ +"""Unit tests for content_publish tool.""" + +import pytest + +import navi.tools.content_publish as content_publish_mod +from navi.tools.base import current_session_id +from navi.tools.content_publish import ContentPublishTool + + +class TestContentPublishTool: + @pytest.fixture + def tool(self, monkeypatch, tmp_path): + monkeypatch.setattr(content_publish_mod.settings, "session_files_dir", str(tmp_path / "sessions")) + token = current_session_id.set("sess-1") + try: + yield ContentPublishTool() + finally: + current_session_id.reset(token) + + async def test_requires_active_session(self): + token = current_session_id.set(None) + try: + result = await ContentPublishTool().execute({"filename": "logo.svg"}) + finally: + current_session_id.reset(token) + + assert not result.success + assert result.error == "no_session" + + async def test_missing_file_reports_session_dir(self, tool, tmp_path): + result = await tool.execute({"filename": "missing.svg"}) + + assert not result.success + assert result.error == "not_found" + assert str(tmp_path / "sessions" / "sess-1") in result.output + assert "SESSION_FILES_DIR" in result.output + + async def test_rejects_directory(self, tool, tmp_path): + sess_dir = tmp_path / "sessions" / "sess-1" + (sess_dir / "folder").mkdir(parents=True) + + result = await tool.execute({"filename": "folder"}) + + assert not result.success + assert result.error == "not_a_file" + + async def test_strips_path_components_and_publishes(self, tool, monkeypatch, tmp_path): + sess_dir = tmp_path / "sessions" / "sess-1" + sess_dir.mkdir(parents=True) + (sess_dir / "logo.svg").write_text("") + calls = [] + + async def fake_publish(**kwargs): + calls.append(kwargs) + return { + "id": "content-id", + "url": "http://localhost:8000/sessions/sess-1/files/logo.svg", + "filename": kwargs["filename"], + "content_type": kwargs["content_type"] or "svg", + "title": kwargs["title"] or kwargs["filename"], + } + + monkeypatch.setattr(content_publish_mod, "publish", fake_publish) + + result = await tool.execute({ + "filename": "../logo.svg", + "title": "Logo", + "content_type": "svg", + }) + + assert result.success + assert calls == [{ + "session_id": "sess-1", + "filename": "logo.svg", + "title": "Logo", + "content_type": "svg", + }] + assert result.metadata["filename"] == "logo.svg"