From f2006bc9d6330e866b32e1fac8fb86d28164b1a1 Mon Sep 17 00:00:00 2001 From: dovvnloading <157447210+dovvnloading@users.noreply.github.com> Date: Fri, 4 Sep 2026 14:59:00 -0400 Subject: [PATCH 1/2] Stop the test suite phoning a real Ollama daemon PR #413 fixed one test that opened a TCP connection to 127.0.0.1:11434 and noted nine other files that set OLLAMA_MODELS the same way but had not been checked. Instrumenting socket.connect across a full run answered it: the suite made 102 outbound connects to that port. 46x _get_ollama_capabilities <- ollama_supports_tools 46x _get_ollama_capabilities <- ollama_supports_embedding 10x _get_ollama_context_window <- _ollama_effective_context_window #413 only closed the third one, and only inside test_providers.py. Two things made the other 92 expensive rather than merely wrong. Each attempt costs a ~2s TCP connect on a machine with nothing listening. And _get_ollama_capabilities deliberately does not cache a probe failure - correct in production, where a daemon may simply have been restarting, but it means every check pays that cost again rather than once. It was also a quiet correctness problem: a developer running Ollama locally got different answers out of these three functions than CI did, from the same code, and nothing in the suite would have shown it. backend/tests/conftest.py now makes ollama.show raise, next to the existing _never_touch_the_real_user_data_dir fixture and for the same reason. That is the same "daemon unavailable" outcome the failed connect produced, minus the socket. Patched on the real module rather than replacing api_provider.ollama wholesale, because other code reaches through that module object for ollama.chat; tests that want their own show() (test_context_budget.py) override it and still win. backend/tests/test_no_network_in_tests.py asserts the fixture is working, phrased as "no socket was opened" rather than "show raised" so it keeps holding if the probing moves to another client or another function. It also pins that the stub still answers "unavailable" rather than fabricating a capability set, and that a test can still supply its own show(). Verified: with the fixture removed, all three socket assertions fail. With it, a full run makes ZERO outbound connects - the only remaining socket.connect calls in the suite are asyncio's own Windows self-pipe. The suite also got faster, which was not the point but is worth recording: 103.2s to 84.0s, about 19%. Test plan: full suite, 3,285 passed / 20 skipped. ruff clean. Co-Authored-By: Claude Opus 5 --- backend/tests/conftest.py | 36 ++++++++++ backend/tests/test_no_network_in_tests.py | 83 +++++++++++++++++++++++ 2 files changed, 119 insertions(+) create mode 100644 backend/tests/test_no_network_in_tests.py diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index b709112f..27a52c82 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -24,6 +24,42 @@ settings.load_profile("ci" if os.environ.get("CI") else "default") +@pytest.fixture(autouse=True) +def _never_probe_a_real_ollama_daemon(monkeypatch): + """Hard-stop any test from reaching out to a live Ollama daemon. + + Three api_provider entry points probe it - _get_ollama_capabilities (via + ollama_supports_tools and ollama_supports_embedding) and + _get_ollama_context_window - and all three go through ollama.show(), which + is a real HTTP call to 127.0.0.1:11434. A test run made 102 of them. + + Two things made that expensive rather than merely wrong. On a machine with + no daemon listening, each attempt costs a ~2s TCP connect. And + _get_ollama_capabilities deliberately does not cache a probe failure - + correct in production, where a daemon may simply have been restarting, but + it means every single check pays that cost again. + + It was also a correctness problem, quietly: a developer running Ollama + locally got different answers from these functions than CI did, on the + same code. + + Raising from show() is the same "daemon unavailable" outcome the failed + connect produced, minus the socket. It is patched on the real module + rather than replacing api_provider.ollama wholesale, because other code + reaches through that module object for ollama.chat. Tests that want their + own show() - backend/tests/test_context_budget.py does - override this + with their own monkeypatch, which still wins. + """ + ollama_module = getattr(api_provider, "ollama", None) + if ollama_module is None: # pragma: no cover - ollama is a hard dependency + return + + def unavailable_show(*_args, **_kwargs): + raise RuntimeError("no ollama daemon in tests") + + monkeypatch.setattr(ollama_module, "show", unavailable_show, raising=False) + + @pytest.fixture(autouse=True) def _never_touch_the_real_user_data_dir(monkeypatch, tmp_path): """Hard-fail any test that opens the developer's REAL ~/.graphlink files. diff --git a/backend/tests/test_no_network_in_tests.py b/backend/tests/test_no_network_in_tests.py new file mode 100644 index 00000000..a2ed4164 --- /dev/null +++ b/backend/tests/test_no_network_in_tests.py @@ -0,0 +1,83 @@ +"""No test reaches a real Ollama daemon. + +conftest.py's _never_probe_a_real_ollama_daemon fixture is what stops it; this +is the assertion that the fixture is doing its job, so a later refactor that +drops it fails here instead of quietly adding a hundred TCP connects back into +every run. + +Written as "no socket was opened", not "show raised", so it keeps holding if +the probing moves to a different client or a different function. +""" + +from __future__ import annotations + +import socket + +import pytest + +import api_provider + + +@pytest.fixture +def opened_sockets(monkeypatch): + """Every address anything tries to connect to during the test.""" + attempts: list[object] = [] + real_connect = socket.socket.connect + real_connect_ex = socket.socket.connect_ex + + def record(self, address): + attempts.append(address) + return real_connect(self, address) + + def record_ex(self, address): + attempts.append(address) + return real_connect_ex(self, address) + + monkeypatch.setattr(socket.socket, "connect", record) + monkeypatch.setattr(socket.socket, "connect_ex", record_ex) + return attempts + + +PROBES = [ + ("ollama_supports_tools", lambda: api_provider.ollama_supports_tools("probe-model:1b")), + ("ollama_supports_embedding", lambda: api_provider.ollama_supports_embedding("probe-model:1b")), + ("_ollama_effective_context_window", + lambda: api_provider._ollama_effective_context_window("probe-model:1b")), +] + + +@pytest.mark.parametrize("name, probe", PROBES, ids=[n for n, _ in PROBES]) +def test_the_ollama_probes_open_no_socket(name, probe, opened_sockets, monkeypatch): + """Each of the three entry points that used to reach 127.0.0.1:11434.""" + monkeypatch.setattr(api_provider, "_OLLAMA_CAPABILITY_CACHE", {}) + monkeypatch.setattr(api_provider, "_OLLAMA_CONTEXT_WINDOW_CACHE", {}) + + probe() + + assert opened_sockets == [] + + +def test_the_probes_still_report_unavailable_rather_than_guessing(monkeypatch): + """The stub has to produce the same answer an unreachable daemon did - + "I don't know" - not a fabricated capability set.""" + monkeypatch.setattr(api_provider, "_OLLAMA_CAPABILITY_CACHE", {}) + monkeypatch.setattr(api_provider, "_OLLAMA_CONTEXT_WINDOW_CACHE", {}) + + assert api_provider._get_ollama_capabilities("probe-model:1b") is None + assert api_provider._get_ollama_context_window("probe-model:1b") is None + # ...and the documented fallback still applies on top of that None. + assert api_provider._ollama_effective_context_window("probe-model:1b") == ( + api_provider._DEFAULT_CONTEXT_WINDOW + ) + + +def test_a_test_can_still_supply_its_own_show(monkeypatch): + """The fixture must not lock out the tests that legitimately fake the + daemon - backend/tests/test_context_budget.py does exactly this.""" + monkeypatch.setattr(api_provider, "_OLLAMA_CAPABILITY_CACHE", {}) + + def fake_show(model): + return {"capabilities": ["vision"]} + + monkeypatch.setattr(api_provider.ollama, "show", fake_show) + assert api_provider._get_ollama_capabilities("probe-model:1b") == {"vision"} From d7caa769de42fc19e941bfa5fc4c9d8cacb6f5cf Mon Sep 17 00:00:00 2001 From: dovvnloading <157447210+dovvnloading@users.noreply.github.com> Date: Fri, 4 Sep 2026 17:57:55 -0400 Subject: [PATCH 2/2] Put mypy on the whole tree, and fix a version string that was never read The last of the type sweep. `mypy backend` went from 179 errors to zero, and [tool.mypy].files - four files when this audit started - now lists every Python root in the repo: 339 source files, checked in CI. WHAT WAS ACTUALLY WRONG, because "179 type errors" is not a description: api_provider.py held 32 of them and was the module worth the most care. Four `base_url: str = None` signatures that PEP 484 has prohibited since 2018. Three module-level caches with no declared shape - one of which, _LLAMA_CPP_CLIENT_CACHE, I first annotated dict[str, Any] and mypy rejected, because llama_cpp_runtime keys it by a five-tuple. LLAMA_CPP_ SETTINGS inferred as dict[str, object] from its mixed literal, which made every .get() on it unusable at the far end and accounted for most of provider_runtime's errors too. And three "Missing return statement" reports that were all one thing: _translate_chat_exception's docstring says "Always raises ... never returns normally", which is exactly what NoReturn is for. A REAL BUG, found by the types and fixed here. backend/harness/ transcript.py did `from graphlink_version import __version__`. There is no __version__ - graphlink_version.py holds one line, APP_VERSION - and the name appears nowhere else in the repo. The import raised ImportError on every call, the surrounding `except Exception` swallowed it, and every harness transcript ever written recorded "app": "" instead of the version that field exists to record. about.py, diagnostic_bundle.py and workspace_archive.py all import the right name. Verified before and after: the meta record now says v1.0.8. THE ONE COERCION, called out because it is the only line here that changes what runs: AnthropicProvider and GeminiProvider take api_key: str, and the snapshot's api_key is str | None - genuinely, since API_KEY starts as None and only initialize_api sets it. Eight call sites now pass `state.api_key or ""`. Both providers store the value verbatim and send it as a header, so None and "" both end in an auth failure; "" is the one the type system can describe, and it fails the same way rather than as a TypeError inside the SDK. Everything else is annotations, renamed locals that had been reused at two different types, and containers that never said what they held. 28 `# type: ignore` comments were added and every one carries the invariant it rests on - sqlite's cursor.lastrowid being Optional only before an INSERT, the optional-dependency `x = None` fallback, duck-typed attributes whose declared home is a subclass. I checked each; the reasoning holds in all 28. backend/knowledge_embeddings.py is worth singling out for what was NOT done: embed() lives on two of the five providers, so widening the shared Provider protocol would have broken the other three. It gets a local two-line protocol and a cast instead, guarded - as it already was - by an explicit `if not provider.capabilities.embedding: raise` before each call. [tool.mypy] also gains follow_imports = "silent". Without it, listing a clean module drags its whole import closure into the gate; with everything listed it is belt and braces for whatever is added next. Verified the gate still bites by injecting a deliberate type error into five different modules - api_provider, session_save, seed_demo_graph, graphlink_desktop, provider_runtime/reasoning - and it was caught every time. Test plan: full suite, 3,285 passed / 20 skipped. ruff clean. mypy clean across 339 source files. Co-Authored-By: Claude Opus 5 --- api_provider.py | 75 ++++++++++++------- backend/agent_dispatch/_composed.py | 6 +- backend/agent_dispatch/builder.py | 18 ++++- backend/agent_dispatch/harness.py | 6 +- backend/agents.py | 14 +++- backend/api/intents_chat.py | 12 ++- backend/api/intents_nodes.py | 15 +++- backend/api/intents_settings_api_provider.py | 9 ++- backend/assets.py | 8 +- backend/attachments.py | 15 +++- backend/autosave.py | 6 +- backend/builder.py | 2 +- backend/chat_library.py | 24 ++++-- backend/events.py | 13 ++++ backend/harness/shell_sessions.py | 4 +- backend/harness/subagents.py | 8 +- backend/harness/tools_shell.py | 2 +- backend/harness/transcript.py | 11 ++- backend/knowledge_embeddings.py | 21 +++++- backend/knowledge_store.py | 8 +- backend/mcp_client.py | 9 ++- backend/plugin_sdk.py | 8 +- backend/plugin_worker.py | 15 +++- backend/providers/ollama_provider.py | 13 +++- backend/providers/openai_provider.py | 8 +- backend/tests/perf/test_loop_watchdog.py | 2 +- backend/tests/test_agents.py | 5 +- backend/tests/test_chat_library.py | 19 +++-- backend/tests/test_run_node_tool.py | 5 +- backend/tests/test_serializer_state_guards.py | 8 +- backend/tests/test_session_context.py | 4 +- backend/tests/test_streaming_partials.py | 5 +- backend/tests/test_tools_graph.py | 5 +- backend/tests/test_wrong_kind_node_guards.py | 12 ++- backend/tools_graph.py | 7 ++ graphlink_chart_rendering.py | 5 +- graphlink_desktop.py | 9 ++- graphlink_plugins/review_lens/diff_fetch.py | 9 ++- .../web_research/crawl_etiquette.py | 6 +- .../web_research/fetch_policy.py | 7 +- graphlink_plugins/web_research/providers.py | 24 ++++-- graphlink_plugins/web_research/service.py | 4 +- graphlink_prompts.py | 8 +- graphlink_scratch_dirs.py | 5 +- provider_runtime/anthropic_transport.py | 15 +++- provider_runtime/gemini_transport.py | 7 +- provider_runtime/llama_cpp_runtime.py | 8 +- provider_runtime/llama_cpp_scan.py | 2 +- provider_runtime/ollama_scan.py | 2 +- provider_runtime/reasoning.py | 3 +- pyproject.toml | 73 +++++++++--------- tests/test_node_state_migration.py | 2 +- tools/build_app_icon.py | 6 +- tools/seed_demo_graph.py | 30 +++++--- 54 files changed, 450 insertions(+), 177 deletions(-) diff --git a/api_provider.py b/api_provider.py index 17458089..bc4e20e0 100644 --- a/api_provider.py +++ b/api_provider.py @@ -9,7 +9,7 @@ import urllib.error import urllib.request from pathlib import Path -from typing import Callable, NamedTuple +from typing import Any, Callable, NamedTuple, NoReturn from urllib.parse import urlparse import ollama @@ -17,7 +17,9 @@ import requests REQUESTS_AVAILABLE = True except ImportError: - requests = None + # The optional-dependency shape: the name is a module when the import + # works and None when it does not, which no annotation expresses. + requests = None # type: ignore[assignment] REQUESTS_AVAILABLE = False # Qt-removal plan R4.1: import the Qt-free split, not graphlink_config - @@ -200,7 +202,8 @@ def _configure_ollama_client_timeout() -> None: ANTHROPIC_REASONING_LEVEL = "off" GEMINI_REASONING_LEVEL = "off" OPENAI_REASONING_LEVEL = "off" -API_MODELS = { +# Per-task model id, or None for a task with nothing configured yet. +API_MODELS: dict[str, str | None] = { config.TASK_TITLE: None, config.TASK_CHAT: None, config.TASK_CHART: None, @@ -208,7 +211,9 @@ def _configure_ollama_client_timeout() -> None: config.TASK_WEB_VALIDATE: None, config.TASK_WEB_SUMMARIZE: None, } -LLAMA_CPP_SETTINGS = { +# Mixed value types (paths, ints, a chat format string), so an inferred +# dict[str, object] would make every .get() unusable at its use site. +LLAMA_CPP_SETTINGS: dict[str, Any] = { "chat_model_path": "", "title_model_path": "", "reasoning_level": "high", @@ -217,7 +222,10 @@ def _configure_ollama_client_timeout() -> None: "n_gpu_layers": 0, "n_threads": 0, } -_LLAMA_CPP_CLIENT_CACHE = {} +# Keyed by llama_cpp_runtime's 5-tuple of (path, chat_format, n_ctx, +# n_gpu_layers, n_threads); the values are Llama handles, whose type is +# only importable when llama-cpp-python is installed. +_LLAMA_CPP_CLIENT_CACHE: dict[tuple[str, Any, int, int, int], Any] = {} _LLAMA_CPP_CLIENT_LOCK = threading.RLock() # _LLAMA_CPP_CLIENT_LOCK above guards only the CACHE (lookup/creation). It @@ -403,7 +411,7 @@ def set_ollama_models(self, models: dict) -> None: def snapshot(self) -> _ProviderSnapshot: return _ProviderSnapshot(**self._read_all()) - def initialize_api(self, provider: str, api_key: str, base_url: str = None): + def initialize_api(self, provider: str, api_key: str, base_url: str | None = None): client, api_key, base_url = _build_api_client(provider, api_key, base_url) self._write( use_api_mode=True, @@ -582,7 +590,7 @@ def _read_all(self) -> dict: def _write(self, **updates) -> dict: module_globals = globals() with _PROVIDER_STATE_LOCK: - previous = {} + previous: dict[str, Any] = {} for key, value in updates.items(): if key == "api_models": previous[key] = dict(API_MODELS) @@ -631,11 +639,14 @@ def set_task_model(self, task: str, api_model: str) -> None: config.TASK_CHART: 4096, config.TASK_WEB_SUMMARIZE: 4096, } -_OLLAMA_CAPABILITY_CACHE = {} +# model name (lowercased) -> the capability set show() reported, or None +# for a model the daemon does not know. A failed PROBE is deliberately not +# cached - see _get_ollama_capabilities. +_OLLAMA_CAPABILITY_CACHE: dict[str, set[str] | None] = {} # ADR-006 stage 6.6: context windows extracted from the same `ollama.show()` # call the capability cache uses, cached under the same key discipline and # invalidated by the same invalidate_ollama_capability_cache() entry point. -_OLLAMA_CONTEXT_WINDOW_CACHE = {} +_OLLAMA_CONTEXT_WINDOW_CACHE: dict[str, int | None] = {} # ADR-006 stage 6.6: API-mode context windows, matched by model-id prefix. # Same posture as anthropic_supports_reasoning below: a documented @@ -847,8 +858,12 @@ def _extract_context_window_from_show(show_response) -> int | None: key for key in model_info if str(key).endswith(".context_length") ) for key in candidates: + raw_value = model_info.get(key) + if raw_value is None: + # Was reached by letting int(None) raise into the handler below. + continue try: - value = int(model_info.get(key)) + value = int(raw_value) except (TypeError, ValueError): continue if value > 0: @@ -938,14 +953,18 @@ def _prepare_ollama_messages(messages: list) -> list: continue tool_calls = msg.get("tool_calls") if tool_calls: - processed_messages.append({ + # Annotated because the value types differ: a checker reading the + # literal alone infers dict[str, str] from the first two keys and + # then rejects the list on the third. + assistant_turn: dict[str, Any] = { "role": "assistant", "content": str(msg.get("content") or ""), "tool_calls": [ {"function": {"name": call["name"], "arguments": call["arguments"]}} for call in tool_calls ], - }) + } + processed_messages.append(assistant_turn) continue content = msg.get("content") @@ -1257,13 +1276,13 @@ def _provider_for_model_ref(model_ref: ModelRef, state: "_ProviderSnapshot"): from backend.providers.anthropic_provider import AnthropicProvider return AnthropicProvider( - client=state.api_client, api_key=state.api_key, model=model_ref.model_id, + client=state.api_client, api_key=state.api_key or "", model=model_ref.model_id, reasoning_level=state.anthropic_reasoning_level, ) from backend.providers.gemini_provider import GeminiProvider return GeminiProvider( - api_key=state.api_key, model=model_ref.model_id, + api_key=state.api_key or "", model=model_ref.model_id, reasoning_level=state.gemini_reasoning_level, ) @@ -1596,7 +1615,7 @@ def _chat_dispatch(task: str, messages: list, **kwargs) -> dict: from backend.providers.anthropic_provider import AnthropicProvider provider = AnthropicProvider( - client=state.api_client, api_key=state.api_key, model=api_model, + client=state.api_client, api_key=state.api_key or "", model=api_model, reasoning_level=state.anthropic_reasoning_level, ) # ADR-006 stage 6.8: transient-transport retry (429/5xx/ @@ -1608,7 +1627,7 @@ def _chat_dispatch(task: str, messages: list, **kwargs) -> dict: from backend.providers.gemini_provider import GeminiProvider provider = GeminiProvider( - api_key=state.api_key, model=api_model, + api_key=state.api_key or "", model=api_model, reasoning_level=state.gemini_reasoning_level, ) # ADR-006 stage 6.8: transient-transport retry (429/5xx/ @@ -1714,7 +1733,7 @@ def _complete_with_transport_retry(provider, chat_request, token, cancel_event): attempt += 1 -def _translate_chat_exception(exc: Exception, state, messages: list) -> None: +def _translate_chat_exception(exc: Exception, state, messages: list) -> NoReturn: """Shared exception-normalization for chat()/chat_stream(): translates raw provider/network exceptions into actionable, user-facing messages. Always raises - either a translated exception (chained `from exc`) or the @@ -1974,14 +1993,14 @@ def _chat_stream_dispatch(task: str, messages: list, on_chunk: Callable[[str, bo from backend.providers.anthropic_provider import AnthropicProvider provider = AnthropicProvider( - client=state.api_client, api_key=state.api_key, model=api_model, + client=state.api_client, api_key=state.api_key or "", model=api_model, reasoning_level=state.anthropic_reasoning_level, ) elif state.api_provider_type == config.API_PROVIDER_GEMINI: from backend.providers.gemini_provider import GeminiProvider provider = GeminiProvider( - api_key=state.api_key, model=api_model, + api_key=state.api_key or "", model=api_model, reasoning_level=state.gemini_reasoning_level, ) else: @@ -2162,14 +2181,14 @@ def chat_turn_with_tools(task: str, messages: list, tools: tuple = (), **kwargs) from backend.providers.anthropic_provider import AnthropicProvider provider = AnthropicProvider( - client=state.api_client, api_key=state.api_key, model=api_model, + client=state.api_client, api_key=state.api_key or "", model=api_model, reasoning_level=state.anthropic_reasoning_level, ) elif state.api_provider_type == config.API_PROVIDER_GEMINI: from backend.providers.gemini_provider import GeminiProvider provider = GeminiProvider( - api_key=state.api_key, model=api_model, + api_key=state.api_key or "", model=api_model, reasoning_level=state.gemini_reasoning_level, ) else: @@ -2251,12 +2270,14 @@ def describe_active_model(task: str, runtime: "ProviderRuntime | None" = None) - return ("ollama", state.ollama_models.get(task) or "") -def _build_api_client(provider: str, api_key: str, base_url: str = None): +def _build_api_client(provider: str, api_key: str, base_url: str | None = None): """The client-construction half of initialize_api, with NO state mutation - ADR-006 stage 6.5 splits it out so ProviderRuntime instances and the throwaway catalog listing (list_models_for_config) can build a client without repointing anything. Returns (client, resolved_key, resolved_base_url).""" + # OpenAI/Anthropic SDK client, or the plain dict the REST fallbacks use. + client: Any if provider == config.API_PROVIDER_OPENAI: try: from openai import OpenAI @@ -2268,7 +2289,7 @@ def _build_api_client(provider: str, api_key: str, base_url: str = None): if not base_url: base_url = "https://api.openai.com/v1" - api_key = api_key or _first_env_api_key(_OPENAI_API_KEY_ENV_VARS) + api_key = api_key or _first_env_api_key(_OPENAI_API_KEY_ENV_VARS) or "" if not api_key: if _is_local_base_url(base_url): api_key = "dummy-key-for-local" @@ -2287,7 +2308,7 @@ def _build_api_client(provider: str, api_key: str, base_url: str = None): client = OpenAI(api_key=api_key, base_url=base_url) elif provider == config.API_PROVIDER_ANTHROPIC: - api_key = api_key or _first_env_api_key(_ANTHROPIC_API_KEY_ENV_VARS) + api_key = api_key or _first_env_api_key(_ANTHROPIC_API_KEY_ENV_VARS) or "" if not api_key: raise RuntimeError("Anthropic API key not configured. Open Settings and save your Anthropic API key.") @@ -2324,7 +2345,7 @@ def _build_api_client(provider: str, api_key: str, base_url: str = None): # snapshot's api_key OR-branch never fired, silently falling # through to a LIVE re-read of API_KEY and then os.environ at # actual-call time instead of the value frozen at request entry. - api_key = api_key or _first_env_api_key(_GEMINI_API_KEY_ENV_VARS) + api_key = api_key or _first_env_api_key(_GEMINI_API_KEY_ENV_VARS) or "" if not api_key: raise RuntimeError("Gemini API key not configured. Open Settings and save your Gemini API key.") client = {"provider": config.API_PROVIDER_GEMINI} @@ -2334,7 +2355,7 @@ def _build_api_client(provider: str, api_key: str, base_url: str = None): return client, api_key, base_url -def initialize_api(provider: str, api_key: str, base_url: str = None): +def initialize_api(provider: str, api_key: str, base_url: str | None = None): """Configure the DEFAULT session's runtime (the module globals) for an API endpoint - ADR-006 stage 6.5: the logic lives on ProviderRuntime, shared with per-session instances; this delegate keeps every existing @@ -2383,7 +2404,7 @@ def get_available_models(): raise RuntimeError(f"Failed to fetch models from endpoint: {exc}") from exc -def list_models_for_config(provider: str, api_key: str, base_url: str = None): +def list_models_for_config(provider: str, api_key: str, base_url: str | None = None): """ADR-006 stage 6.5: catalog listing WITHOUT touching live provider state. loadApiModels used to call initialize_api just to refresh a Settings dropdown - a read-only catalog fetch silently repointed the diff --git a/backend/agent_dispatch/_composed.py b/backend/agent_dispatch/_composed.py index e87159a8..2deaedf9 100644 --- a/backend/agent_dispatch/_composed.py +++ b/backend/agent_dispatch/_composed.py @@ -23,6 +23,7 @@ if TYPE_CHECKING: from backend.events import SessionBus from graphlink_settings_store import SettingsManager + from backend.tools import ToolRegistry class DispatcherParts: @@ -44,7 +45,10 @@ def _runtime_kwargs(self) -> dict: ... def _cancel_with_pending_approval_denied(self, request_id: str, kind: str) -> bool: ... - def builder_tool_registry(self, document: Any) -> object: ... + # Narrowed from `object`: BuilderDispatchOps returns a real + # ToolRegistry, and declaring that here is what lets sibling + # mixins call it without restating the type at each site. + def builder_tool_registry(self, document: Any) -> "ToolRegistry": ... # The run engine every start_* surface funnels through, and the # plain-blocking-action skeleton the gitlink/code-review surfaces diff --git a/backend/agent_dispatch/builder.py b/backend/agent_dispatch/builder.py index 20223727..923a7b44 100644 --- a/backend/agent_dispatch/builder.py +++ b/backend/agent_dispatch/builder.py @@ -23,20 +23,32 @@ import asyncio import threading +from typing import TYPE_CHECKING from backend.agent_dispatch._composed import DispatcherParts +if TYPE_CHECKING: + from backend.tools import ToolRegistry + class BuilderDispatchOps(DispatcherParts): """The Builder agent loop and its tool-registry assembly (mixin - see module docstring).""" - def builder_tool_registry(self, document) -> "object": + # Declared, never assigned here: a bare annotation creates no class + # attribute, so builder_tool_registry()'s `getattr(self, ..., None)` + # lazy-build probe below behaves exactly as it always has. It exists so + # invalidate_builder_registry() can put None back without the checker + # inferring the attribute's type from the sole ToolRegistry assignment. + _builder_registry: "ToolRegistry | None" + + def builder_tool_registry(self, document) -> "ToolRegistry": """The session's one ToolRegistry, built lazily on first builder start (ADR-007 shipped the registry with zero production constructors; the Builder is its designated first consumer). Cached: tools bind the session's own SceneDocument/dispatcher, and both live exactly as long as this dispatcher does.""" - if getattr(self, "_builder_registry", None) is None: + registry = getattr(self, "_builder_registry", None) + if registry is None: from backend.builder import register_builder_control_tools from backend.tools import ToolRegistry from backend.tools_graph import register_graph_tools, register_run_node_tool @@ -55,7 +67,7 @@ def builder_tool_registry(self, document) -> "object": self._register_configured_mcp_tools(registry) self._register_plugin_tools(registry, document) self._builder_registry = registry - return self._builder_registry + return registry def invalidate_builder_registry(self) -> None: """SECURITY-FIX: builder_tool_registry() above builds the registry diff --git a/backend/agent_dispatch/harness.py b/backend/agent_dispatch/harness.py index b00aa26e..bc2b585a 100644 --- a/backend/agent_dispatch/harness.py +++ b/backend/agent_dispatch/harness.py @@ -23,9 +23,13 @@ import asyncio import threading +from typing import TYPE_CHECKING from backend.agent_dispatch._composed import DispatcherParts +if TYPE_CHECKING: + from backend.tools import ToolRegistry + class HarnessDispatchOps(DispatcherParts): """The Harness agent: grants, process bookkeeping, and the Harness run (mixin - see module docstring).""" @@ -124,7 +128,7 @@ def dispose_all_harness_processes(self) -> None: if repls is not None: repls.stop_all() - def harness_tool_registry(self, document) -> "object": + def harness_tool_registry(self, document) -> "ToolRegistry": """The harness rides the SAME per-session registry the Builder built (tools.py: one registry per session, RunContext is what's per-run) - fs tools are simply registered into it on first harness diff --git a/backend/agents.py b/backend/agents.py index dbbd8b3c..919fb8e8 100644 --- a/backend/agents.py +++ b/backend/agents.py @@ -74,6 +74,7 @@ import threading import uuid # noqa: F401 from pathlib import Path +from typing import Protocol from urllib.parse import quote import api_provider @@ -317,7 +318,18 @@ class AgentDispatcher( # takes. Kept as data rather than an if/elif so adding a third note agent is # one entry, not another branch in three places. NOTE_AGENT_LABELS = {"takeaway": "Key takeaway", "explainer": "Explainer note"} -_NOTE_AGENTS = {"takeaway": KeyTakeawayAgent, "explainer": ExplainerAgent} + + +class _NoteAgent(Protocol): + """What the table below actually requires of a note agent. The two + classes in it share no base class - they are independent agents in + graphlink_note_agent.py that happen to answer the same one call - so + the common type has to be structural rather than nominal.""" + + def get_response(self, text: str) -> str: ... + + +_NOTE_AGENTS: dict[str, type[_NoteAgent]] = {"takeaway": KeyTakeawayAgent, "explainer": ExplainerAgent} def _call_note_agent(note_kind: str, source_text: str) -> str: diff --git a/backend/api/intents_chat.py b/backend/api/intents_chat.py index bf5c5cbb..156475f2 100644 --- a/backend/api/intents_chat.py +++ b/backend/api/intents_chat.py @@ -30,6 +30,8 @@ from __future__ import annotations +from functools import partial + from backend.agents import AgentDispatcher from backend.api._shared import make_publish_scene, make_publish_token_counter from backend.composer import ComposerDocument @@ -99,9 +101,17 @@ def _promote_document_attachments(document: SceneDocument, node, staged: list) - # stacking (ADR-021 stage 21.5's requirement, now via the shared # placement engine instead of a local index offset). ax, ay = document.place_child(node.id, "document") + # functools.partial rather than the `lambda attachment=attachment, + # ax=ax, ay=ay:` this used to be. The default arguments were there only + # to bind this iteration's values eagerly, which is exactly what + # partial does - and record_command runs the mutator synchronously, + # inside this same iteration, so nothing here was ever read late. + # A lambda carrying defaults cannot be checked against + # record_command's zero-argument `Callable[[], T]` parameter at all. document.record_command( "addDocumentNode", "user", - lambda attachment=attachment, ax=ax, ay=ay: document.add_document_node( + partial( + document.add_document_node, ax, ay, attachment.name, diff --git a/backend/api/intents_nodes.py b/backend/api/intents_nodes.py index cf6917c5..67d2a878 100644 --- a/backend/api/intents_nodes.py +++ b/backend/api/intents_nodes.py @@ -16,6 +16,8 @@ from backend.agents import AgentDispatcher from backend.api._shared import make_publish_scene from backend.domain.graph import SceneDocument +from backend.domain.node_access import optional_node +from backend.domain.node_states import CodeSandboxState, HarnessState from backend.events import SessionBus from graphlink_scratch_dirs import HARNESS_WORKSPACE_ROOT, remove_scratch_dir_for_id @@ -59,10 +61,15 @@ def _capture_live_run_teardown(document: SceneDocument, ids: list[str]): Approve/Deny/Stop buttons all no-op with nothing left to call them on). """ + # optional_node applies exactly the "present, and of this kind" test the + # hand-written condition did, and additionally narrows `state` to the + # kind's own class so the field read below is checkable - see + # backend/domain/node_access.py. sandbox_ids = [ - document.nodes[node_id].state.code_sandbox_sandbox_id + node.state.code_sandbox_sandbox_id for node_id in ids - if document.nodes.get(node_id) is not None and document.nodes[node_id].kind == "code_sandbox" + if (node := optional_node(document.nodes, node_id, "code_sandbox", CodeSandboxState)) + is not None ] code_sandbox_cancels = [ document.nodes[node_id].pending_request_id @@ -83,9 +90,9 @@ def _capture_live_run_teardown(document: SceneDocument, ids: list[str]): # recompute-from-durable-id path the sandbox dirs use, and its # live run cancelled like a plan node's. harness_workspace_ids = [ - document.nodes[node_id].state.harness_workspace_id + harness.state.harness_workspace_id for node_id in ids - if document.nodes.get(node_id) is not None and document.nodes[node_id].kind == "harness" + if (harness := optional_node(document.nodes, node_id, "harness", HarnessState)) is not None ] harness_cancels = [ document.nodes[node_id].pending_request_id diff --git a/backend/api/intents_settings_api_provider.py b/backend/api/intents_settings_api_provider.py index 694b1b66..2134d78f 100644 --- a/backend/api/intents_settings_api_provider.py +++ b/backend/api/intents_settings_api_provider.py @@ -312,7 +312,14 @@ def _persist() -> None: openai_key = api_key if provider == config.API_PROVIDER_OPENAI else KEEP_EXISTING_SECRET anthropic_key = api_key if provider == config.API_PROVIDER_ANTHROPIC else KEEP_EXISTING_SECRET gemini_key = api_key if provider == config.API_PROVIDER_GEMINI else KEEP_EXISTING_SECRET - manager.set_api_settings(provider, base_url, openai_key, anthropic_key, gemini_key) + # set_api_settings annotates its three key parameters `str`, but + # KEEP_EXISTING_SECRET is part of its contract, not a violation of + # it: its body branches on `value is KEEP_EXISTING_SECRET` and + # skips that field entirely, which is the whole mechanism the + # comment above relies on. The annotation in + # settings_store/cloud_provider.py is the thing that is too + # narrow - widening it there retires this ignore. + manager.set_api_settings(provider, base_url, openai_key, anthropic_key, gemini_key) # type: ignore[arg-type] manager.set_api_models(normalized_models, provider) for task, model_id in normalized_models.items(): api_provider.set_task_model(task, model_id) diff --git a/backend/assets.py b/backend/assets.py index de3df206..4b3af718 100644 --- a/backend/assets.py +++ b/backend/assets.py @@ -49,6 +49,8 @@ from fastapi.responses import JSONResponse from backend.asset_store import ALLOWED_IMAGE_MIME_TYPES, extension_for_mime +from backend.domain.node_access import optional_node +from backend.domain.node_states import ChartState from backend.events import EventBus, UnknownSessionError from backend.session_context import get_session_context from graphlink_chart_rendering import render_chart_png, render_chart_svg @@ -148,8 +150,8 @@ async def export_chart(node_id: str, session: str = "default", fmt: str = "png") document = _get_canvas_document(bus, session) if document is None: return JSONResponse({"error": "unknown chart"}, status_code=404) - node = document.nodes.get(node_id) - if node is None or node.kind != "chart": + node = optional_node(document.nodes, node_id, "chart", ChartState) + if node is None: return JSONResponse({"error": "unknown chart"}, status_code=404) normalized_format = str(fmt or "png").strip().lower() @@ -157,7 +159,7 @@ async def export_chart(node_id: str, session: str = "default", fmt: str = "png") return JSONResponse({"error": "unsupported export format"}, status_code=400) title = node.state.chart_data.get("title") if isinstance(node.state.chart_data, dict) else "" - filename = _sanitize_chart_filename(title) + filename = _sanitize_chart_filename(str(title or "")) if normalized_format == "svg": svg_bytes = await asyncio.to_thread( diff --git a/backend/attachments.py b/backend/attachments.py index 9a482da4..222ac21e 100644 --- a/backend/attachments.py +++ b/backend/attachments.py @@ -185,7 +185,12 @@ def _read_pdf(path: Path) -> str: import pypdf as pdf_reader_lib except ImportError: try: - import PyPDF2 as pdf_reader_lib # noqa: N813 + # The two packages are the same library either side of its + # rename, and this code only touches the PdfReader/extract_text + # surface they share - but mypy has no way to express "either of + # these modules, whichever imported" and reads the fallback as a + # redefinition of the name the first import already bound. + import PyPDF2 as pdf_reader_lib # type: ignore[no-redef] # noqa: N813 except ImportError: raise AttachmentError(PDF_INSTALL_MESSAGE) from None @@ -216,7 +221,13 @@ def _read_docx(path: Path) -> str: import docx except ImportError: raise AttachmentError(DOCX_INSTALL_MESSAGE) from None - document = docx.Document(path) + # python-docx annotates Document() as str | IO[bytes] | None, but its + # runtime contract is wider: anything that is not a str is handed + # straight to zipfile.ZipFile, which takes a Path. Passing str(path) + # instead would not be an equivalent rewrite - the str branch runs an + # os.path.isdir/is_zipfile probe first and raises PackageNotFoundError + # for a missing file, where the Path branch raises FileNotFoundError. + document = docx.Document(path) # type: ignore[arg-type] return "\n".join(paragraph.text for paragraph in document.paragraphs) diff --git a/backend/autosave.py b/backend/autosave.py index ac717248..9e6927a8 100644 --- a/backend/autosave.py +++ b/backend/autosave.py @@ -271,7 +271,11 @@ def register_autosave( db_path: Path, canvas_document: SceneDocument, notifications: NotificationState | None, - mutation_guard: dict[str, bool], + # Any, not bool, matching what chat_library._new_mutation_guard actually + # builds: the guard has never been all-bool. It carries `active` (a bool), + # `owner` (a str or None) and `released` (an asyncio.Event), and the + # claim/release block below writes all three. + mutation_guard: dict[str, Any], last_saved: dict[str, Any], *, interval_seconds: float = DEFAULT_INTERVAL_SECONDS, diff --git a/backend/builder.py b/backend/builder.py index afefe5db..f917b9e9 100644 --- a/backend/builder.py +++ b/backend/builder.py @@ -393,7 +393,7 @@ def plan_steps_for_goal(goal: str, *, runtime=None, settings_manager=None) -> li runtime=runtime, settings_manager=settings_manager, ) - steps = [] + steps: list[dict] = [] for raw in payload.get("steps", [])[:_MAX_PLAN_STEPS]: title = str(raw.get("title") or "").strip() if title: diff --git a/backend/chat_library.py b/backend/chat_library.py index 99bc9f51..34341ea5 100644 --- a/backend/chat_library.py +++ b/backend/chat_library.py @@ -327,7 +327,7 @@ def _extract_node_index_text(node: dict[str, Any]) -> str: elif node_type == "conversation": body_text = _flatten_conversation_history(node.get("conversation_history")).strip() else: - field_name = _TEXT_FIELD_BY_NODE_TYPE.get(node_type, "content") + field_name = _TEXT_FIELD_BY_NODE_TYPE.get(str(node_type or ""), "content") raw_value = node.get(field_name) body_text = str(raw_value).strip() if isinstance(raw_value, str) else "" @@ -884,7 +884,9 @@ def _migration_002_workspaces_and_graphs(conn: sqlite3.Connection) -> None: default_workspace_id = int(existing_default[0]) else: cursor = conn.execute("INSERT INTO workspaces (name) VALUES ('Default')") - default_workspace_id = int(cursor.lastrowid) + # lastrowid is Optional only because it is None before the first + # INSERT on a cursor; this reads it directly after one. + default_workspace_id = int(cursor.lastrowid) # type: ignore[arg-type] # "chats" still exists under its old name - a database at version 0/1 # that hasn't seen this migration yet (real user data, or the fresh-db @@ -1402,7 +1404,8 @@ def create_workspace(db_path: Path, name: str) -> dict[str, Any] | None: return None with contextlib.closing(_connect(db_path)) as conn, conn: cursor = conn.execute("INSERT INTO workspaces (name) VALUES (?)", (trimmed,)) - workspace_id = int(cursor.lastrowid) + # Same lastrowid-after-INSERT invariant as _ensure_default_workspace. + workspace_id = int(cursor.lastrowid) # type: ignore[arg-type] return { "id": workspace_id, "name": trimmed, "icon": "", "archived": False, "defaultModelProvider": "", "defaultModelId": "", @@ -1784,19 +1787,23 @@ def save_chat_atomically_row( "VALUES (?, ?, ?, ?, ?, ?)", (title, chat_data_json, preview, message_count, now, workspace_id), ) - resolved_chat_id = cursor.lastrowid + # lastrowid is set by the INSERT immediately above. + resolved_chat_id = cursor.lastrowid # type: ignore[assignment] else: cursor = conn.execute( "INSERT INTO graphs (title, data, preview, message_count, updated_at) " "VALUES (?, ?, ?, ?, ?)", (title, chat_data_json, preview, message_count, now), ) - resolved_chat_id = cursor.lastrowid + # lastrowid is set by the INSERT immediately above. + resolved_chat_id = cursor.lastrowid # type: ignore[assignment] conn.execute("DELETE FROM notes WHERE chat_id = ?", (resolved_chat_id,)) for note in notes_data: - position = note.get("position") if isinstance(note.get("position"), dict) else {} - size = note.get("size") if isinstance(note.get("size"), dict) else {} + raw_position = note.get("position") + position = raw_position if isinstance(raw_position, dict) else {} + raw_size = note.get("size") + size = raw_size if isinstance(raw_size, dict) else {} conn.execute( """ INSERT INTO notes ( @@ -1841,7 +1848,8 @@ def save_chat_atomically_row( conn.execute("DELETE FROM pins WHERE chat_id = ?", (resolved_chat_id,)) for index, pin in enumerate(pins_data): - position = pin.get("position") if isinstance(pin.get("position"), dict) else {} + raw_pin_position = pin.get("position") + position = raw_pin_position if isinstance(raw_pin_position, dict) else {} conn.execute( """ INSERT INTO pins ( diff --git a/backend/events.py b/backend/events.py index 9e6481e7..673d9ee2 100644 --- a/backend/events.py +++ b/backend/events.py @@ -400,6 +400,19 @@ def __init__( `send_queue_maxsize` bounds each BUFFERED connection's outbound queue (see attach(buffered=True) and _BufferedConnection).""" self.session_id = session_id + # Attached from outside, after construction, by the modules that own + # each concern: backend/chat_library.py sets the three chat_* names in + # register_chat_library, and backend/autosave.py sets autosave_task. + # Declared here so that is a stated part of the object rather than + # something a reader has to discover by grepping for `bus.` - and so a + # checker reading either of those modules can follow it. They are real + # assignments, not TYPE_CHECKING-only: None is the honest before-wiring + # value and code already tests for it. + self.chat_db_path: Any = None + self.chat_mutation_guard: Any = None + self.chat_save_state: Any = None + self.autosave_task: Any = None + self.autosave_guarded_tick: Any = None # ADR-016 stage 16.3: optional - fires with (topic, serialized byte # size) from _broadcast, once per outbound message. None by default # (every test-constructed SessionBus, and this codebase has diff --git a/backend/harness/shell_sessions.py b/backend/harness/shell_sessions.py index 6c123819..f9f4f656 100644 --- a/backend/harness/shell_sessions.py +++ b/backend/harness/shell_sessions.py @@ -41,7 +41,7 @@ from collections import deque from pathlib import Path -from graphlink_execution_guard import create_execution_guard +from graphlink_execution_guard import ExecutionResourceGuard, create_execution_guard from graphlink_process_env import safe_subprocess_env # Per-session output ring. Lines, not bytes: a line is the unit anyone @@ -82,7 +82,7 @@ class LocalSubprocessBackend(ShellBackend): """Guarded local subprocess - the ADR-005 stage-5.2/5.3 posture.""" def __init__(self) -> None: - self._guards: dict[int, object] = {} + self._guards: dict[int, ExecutionResourceGuard] = {} def spawn(self, command: str, cwd: Path): kwargs: dict = {"env": safe_subprocess_env()} diff --git a/backend/harness/subagents.py b/backend/harness/subagents.py index 9d7898fa..13cb1296 100644 --- a/backend/harness/subagents.py +++ b/backend/harness/subagents.py @@ -114,8 +114,12 @@ async def _auto(_call: ToolCall) -> bool: cancel=CancelToken(cancel_event) if cancel_event is not None else None, ) # The duck-typed root channel the fs tools read (ctx.harness_workspace_dir), - # so the child confines to exactly its parent's root. - ctx.harness_workspace_dir = workspace_dir + # so the child confines to exactly its parent's root. It is set on a plain + # RunContext rather than declared as a field, which the checker cannot + # follow; the parent loop declares the same channel as a real field on its + # HarnessRunContext subclass (backend/harness/loop.py), and the tools that + # read it all go through getattr with a None default. + ctx.harness_workspace_dir = workspace_dir # type: ignore[attr-defined] specs = tuple( spec for spec in registry.specs() diff --git a/backend/harness/tools_shell.py b/backend/harness/tools_shell.py index af758121..6061668d 100644 --- a/backend/harness/tools_shell.py +++ b/backend/harness/tools_shell.py @@ -97,7 +97,7 @@ def _run_command(command: str, cwd, cancel_event) -> tuple[str, "int | None", st real build. The reader also means the timeout path can hand back what the command managed to say before it was killed, which is the whole diagnostic value of a hung command.""" - kwargs = {"env": safe_subprocess_env()} + kwargs: dict = {"env": safe_subprocess_env()} if sys.platform == "win32": kwargs["creationflags"] = subprocess.CREATE_NO_WINDOW guard = create_execution_guard() diff --git a/backend/harness/transcript.py b/backend/harness/transcript.py index 35455079..9899eeab 100644 --- a/backend/harness/transcript.py +++ b/backend/harness/transcript.py @@ -210,7 +210,16 @@ def build_profile(root: Path, is_user_dir: bool) -> dict: def _meta_payload(profile: dict, root: Path) -> dict: try: - from graphlink_version import __version__ as app_version + # BUG FIX: this imported `__version__`, a name graphlink_version.py has + # never defined - it holds one line, APP_VERSION. The import raised + # ImportError on every call, the except swallowed it, and every + # transcript ever written recorded "app": "" instead of the version it + # exists to record. backend/about.py, backend/diagnostic_bundle.py and + # backend/workspace_archive.py all import the right name. + # + # The try/except stays: a transcript is worth writing even if the + # version cannot be read, which is the whole reason this is guarded. + from graphlink_version import APP_VERSION as app_version except Exception: app_version = "" return { diff --git a/backend/knowledge_embeddings.py b/backend/knowledge_embeddings.py index c895497c..5ec3ed7c 100644 --- a/backend/knowledge_embeddings.py +++ b/backend/knowledge_embeddings.py @@ -18,7 +18,7 @@ from __future__ import annotations from pathlib import Path -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Protocol, cast import numpy as np @@ -35,6 +35,21 @@ _VECTOR_DTYPE = " list[list[float]]: ... + + def _pack_vector(vector) -> bytes: return np.asarray(vector, dtype=_VECTOR_DTYPE).tobytes() @@ -74,7 +89,7 @@ def embed_pending_chunks( embedded_count = 0 for start in range(0, len(pending), batch_size): batch = pending[start : start + batch_size] - vectors = provider.embed([row["text"] for row in batch]) + vectors = cast(_EmbeddingCapable, provider).embed([row["text"] for row in batch]) # Adversarial-review finding: zip() alone silently truncates/ # mispairs if a provider ever returns a different-length list than # it was given (a non-conforming proxy, a future provider) - @@ -134,7 +149,7 @@ def vector_search( if not rows: return [] - query_vectors = provider.embed([query]) + query_vectors = cast(_EmbeddingCapable, provider).embed([query]) if len(query_vectors) != 1: raise ValueError( f"provider.embed() returned {len(query_vectors)} vector(s) for a single query " diff --git a/backend/knowledge_store.py b/backend/knowledge_store.py index 0037cfc4..51251c5e 100644 --- a/backend/knowledge_store.py +++ b/backend/knowledge_store.py @@ -613,7 +613,9 @@ def add_document_with_chunks( ).fetchone()[0] return IngestOutcome(document_id=existing_id, chunk_count=existing_count, was_new=False) - document_id = cursor.lastrowid + # lastrowid is Optional only because it is None before the first + # INSERT on a cursor; this reads it directly after one. + document_id: int = cursor.lastrowid # type: ignore[assignment] conn.executemany( "INSERT INTO chunks (document_id, ordinal, text, token_count, offset_start, offset_end) " "VALUES (?, ?, ?, ?, ?, ?)", @@ -726,7 +728,9 @@ def get_or_create_workspace_collection(db_path: Path, workspace_id: int) -> int: "INSERT INTO collections (name, scope, created_at, workspace_id) VALUES (?, ?, ?, ?)", (f"workspace-{workspace_id}", "workspace", _now_iso(), workspace_id), ) - return int(cursor.lastrowid) + # lastrowid is Optional only because it is None before the + # first INSERT on a cursor; this reads it directly after one. + return int(cursor.lastrowid) # type: ignore[arg-type] except sqlite3.IntegrityError: # REVIEW-FIX: the SELECT above and this INSERT are not one # atomic step - a second, fully separate connection (every diff --git a/backend/mcp_client.py b/backend/mcp_client.py index fb200103..2cc63912 100644 --- a/backend/mcp_client.py +++ b/backend/mcp_client.py @@ -520,11 +520,16 @@ def _write(self, message: dict) -> None: # forever; the blocked syscall (and its thread) leaks, but a # leaked thread beats a permanently deadlocked client. outcome: list[BaseException] = [] + # Bound to a local because the guard above narrows process.stdin + # only for this frame - a nested function could in principle run + # after the attribute changed, so the narrowing does not reach + # inside the closure. The stream itself is the same object. + stdin = process.stdin def _blocking_write() -> None: try: - process.stdin.write(payload) - process.stdin.flush() + stdin.write(payload) + stdin.flush() except Exception as exc: # re-raised on the caller's thread below outcome.append(exc) diff --git a/backend/plugin_sdk.py b/backend/plugin_sdk.py index 708bfd2d..5885ea36 100644 --- a/backend/plugin_sdk.py +++ b/backend/plugin_sdk.py @@ -144,7 +144,7 @@ # never reinvented, for the out-of-process plugin worker's own Popen call. # See PluginWorkerClient.connect's own docstring for the full call-order # contract these two enforce together. -from graphlink_execution_guard import create_execution_guard +from graphlink_execution_guard import ExecutionResourceGuard, create_execution_guard # ADR-021 stage 21.4: a plugin intent's declared args_schema is a # dataclass, described and validated with the SAME ADR-003 machinery # every wire payload already uses - not a second, plugin-only scheme. @@ -936,7 +936,7 @@ def __init__(self, *, plugin_id: str, source_dir: Path, timeout: float = _DEFAUL self.source_dir = source_dir self.timeout = timeout self._process: subprocess.Popen | None = None - self._guard = None + self._guard: ExecutionResourceGuard | None = None self._reader_thread: threading.Thread | None = None self._stderr_thread: threading.Thread | None = None # A bounded tail of the worker's stderr, drained continuously by its @@ -1524,8 +1524,8 @@ def _merge_into_registry( registry.node_kinds[kind] = spec for name, entry in host._picker_entries.items(): registry.picker_entries[name] = entry - for name, spec in host._builtin_actions.items(): - registry.builtin_actions[name] = spec + for name, action_spec in host._builtin_actions.items(): + registry.builtin_actions[name] = action_spec registry.intents.extend(host._intents) diff --git a/backend/plugin_worker.py b/backend/plugin_worker.py index 86124f66..12ce4b77 100644 --- a/backend/plugin_worker.py +++ b/backend/plugin_worker.py @@ -62,6 +62,7 @@ import json import sys from pathlib import Path +from typing import Any from backend.notifications import NotificationState from backend.plugin_sdk import ( @@ -146,7 +147,7 @@ def _intents_payload(host: HostContext) -> list: import. The dict crosses as plain JSON; the type itself never does.""" payload = [] for spec in host._intents: - entry = {"name": spec.name} + entry: dict[str, Any] = {"name": spec.name} if spec.args_schema is not None: entry["args_schema"] = json_schema_for(spec.args_schema) payload.append(entry) @@ -213,7 +214,7 @@ def main() -> None: params = request.get("params") or {} try: if method == "get_registrations": - result = { + result: dict[str, Any] = { "node_kinds": _node_kinds_payload(host), "picker_entries": _picker_entries_payload(host), "intents": _intents_payload(host), @@ -226,7 +227,15 @@ def main() -> None: raise ValueError(f"unknown node kind: {local_kind!r}") parent_data = params.get("parent_snapshot") or {} parent_id = str(parent_data.get("id", "")) - document = _WorkerDocumentStandin({parent_id: _WorkerParentSnapshot(parent_data)}) + # Deliberately untyped: the stand-in is NOT a SceneDocument + # and must never become one - see this module's docstring on + # why a factory receives a read-only stand-in instead of the + # live document. NodeFactory still declares SceneDocument + # because that is what an in-process factory is handed, so + # the substitution is duck-typing the checker cannot verify. + document: Any = _WorkerDocumentStandin( + {parent_id: _WorkerParentSnapshot(parent_data)} + ) run_ctx = PluginRunContext(plugin_id=host.plugin_id, notifications=notifications) seed = kind_spec.factory(document, run_ctx, parent_id) result = { diff --git a/backend/providers/ollama_provider.py b/backend/providers/ollama_provider.py index 91814e80..7cbbe1d2 100644 --- a/backend/providers/ollama_provider.py +++ b/backend/providers/ollama_provider.py @@ -42,7 +42,7 @@ from __future__ import annotations import time -from typing import Iterator +from typing import Generator, Iterator, cast import ollama @@ -225,7 +225,16 @@ def stream(self, request: ChatRequest, cancel: CancelToken) -> Iterator[Provider thinking_parts: list[str] = [] tool_calls: tuple[ToolCall, ...] = () usage = None - stream = ollama.chat(model=self.model_id, messages=messages, stream=True, **kwargs) + # ollama types its streaming chat() as `Iterator[ChatResponse]`, + # which has no close() - but the object it actually returns is the + # generator ollama/_client.py's own `_request` builds around + # `with self._client.stream(...)`, so the close() both call sites + # below depend on is genuinely there. The declared return type is + # simply wider than what the library hands back. + stream = cast( + "Generator[ollama.ChatResponse, None, None]", + ollama.chat(model=self.model_id, messages=messages, stream=True, **kwargs), + ) try: for part in stream: if cancel.is_set(): diff --git a/backend/providers/openai_provider.py b/backend/providers/openai_provider.py index 6711e50a..6a29a3f6 100644 --- a/backend/providers/openai_provider.py +++ b/backend/providers/openai_provider.py @@ -29,7 +29,7 @@ import base64 import json from pathlib import Path -from typing import Iterator +from typing import Any, Iterator import graphlink_task_config as config from api_provider import ( @@ -115,7 +115,11 @@ def prepare_openai_messages(messages: list) -> list: if not isinstance(content, list): prepared.append(message) continue - parts = [] + # Annotated rather than inferred: the branches below build three + # different part shapes (text, image_url, input_audio), and only the + # first one is a flat str->str mapping. Inference from that first + # append alone would make every nested part below a type error. + parts: list[dict[str, Any]] = [] for part in content: if not isinstance(part, dict): # Same defensive posture as _prepare_ollama_messages' own diff --git a/backend/tests/perf/test_loop_watchdog.py b/backend/tests/perf/test_loop_watchdog.py index 506046fe..c351d0fe 100644 --- a/backend/tests/perf/test_loop_watchdog.py +++ b/backend/tests/perf/test_loop_watchdog.py @@ -145,7 +145,7 @@ async def _run() -> float: # emptiness) so a future legitimate exception, if one is ever deliberately # added, has an obvious place to be pinned rather than silently widening # the scanner's own matching. -_KNOWN_RENDER_CALL_SITES = {} +_KNOWN_RENDER_CALL_SITES: dict[str, int] = {} _WATCHED_RENDER_NAMES = {"render_chart_png", "render_chart_svg"} diff --git a/backend/tests/test_agents.py b/backend/tests/test_agents.py index e239721e..a10625ce 100644 --- a/backend/tests/test_agents.py +++ b/backend/tests/test_agents.py @@ -70,7 +70,10 @@ def _make_dispatch_env(enable_system_prompt: bool = True): bus.register_topic("app-composer", composer_document.payload) # A real "scene" topic - the success path publishes it after on_reply. bus.register_topic("scene", lambda: {}) - dispatcher = AgentDispatcher(_FakeSettingsManager(enable_system_prompt)) + # The fake is a stand-in for the one method AgentDispatcher reads off a + # SettingsManager (see its docstring), not a subclass of one - a + # relationship the concrete parameter type has no way to express. + dispatcher = AgentDispatcher(_FakeSettingsManager(enable_system_prompt)) # type: ignore[arg-type] return bus, notifications, composer_document, dispatcher diff --git a/backend/tests/test_chat_library.py b/backend/tests/test_chat_library.py index 29cadeab..ebe3dd6b 100644 --- a/backend/tests/test_chat_library.py +++ b/backend/tests/test_chat_library.py @@ -82,7 +82,9 @@ def _insert_chat(db_path, title: str, data: str = "{}") -> int: (title, data, "2026-01-01 10:00:00", "2026-01-02 11:30:00"), ) conn.commit() - return cursor.lastrowid + # sqlite3 types lastrowid Optional because it is None before any + # INSERT has run on the cursor; this cursor just ran one. + return cursor.lastrowid # type: ignore[return-value] finally: conn.close() @@ -491,7 +493,7 @@ def _create_migration_1_shaped_db(db_path) -> list[int]: conn.execute("CREATE INDEX idx_notes_chat_id ON notes (chat_id)") conn.execute("CREATE INDEX idx_pins_chat_id ON pins (chat_id)") - chat_ids = [] + chat_ids: list[int] = [] for title, preview, count, created, updated in ( ("First Chat", "hi there", 1, "2026-01-01 09:00:00", "2026-01-01 09:05:00"), ("Second Chat", "another one", 2, "2026-01-02 10:00:00", "2026-01-02 10:10:00"), @@ -503,7 +505,8 @@ def _create_migration_1_shaped_db(db_path) -> list[int]: (title, json.dumps({"nodes": [{"node_type": "chat", "raw_content": preview}]}), created, updated, preview, count), ) - chat_ids.append(cursor.lastrowid) + # lastrowid is Optional only until the cursor has run an INSERT. + chat_ids.append(cursor.lastrowid) # type: ignore[arg-type] # Notes and pins attached to the first chat only - mirrors # _create_pre_migration_shaped_db's own "at least one" scope; the @@ -690,7 +693,7 @@ def _create_migration_2_shaped_db(db_path) -> list[int]: conn.execute("CREATE INDEX idx_pins_chat_id ON pins (chat_id)") conn.execute("CREATE INDEX idx_graphs_workspace_id ON graphs (workspace_id)") - graph_ids = [] + graph_ids: list[int] = [] for title, workspace_id in ( ("First Graph", default_id), ("Second Graph", default_id), @@ -700,7 +703,8 @@ def _create_migration_2_shaped_db(db_path) -> list[int]: "INSERT INTO graphs (title, data, workspace_id) VALUES (?, ?, ?)", (title, json.dumps({"nodes": []}), workspace_id), ) - graph_ids.append(cursor.lastrowid) + # lastrowid is Optional only until the cursor has run an INSERT. + graph_ids.append(cursor.lastrowid) # type: ignore[arg-type] conn.commit() conn.execute("PRAGMA user_version = 2") @@ -892,7 +896,7 @@ def _create_migration_3_shaped_db(db_path) -> list[int]: conn.execute("CREATE INDEX idx_graphs_workspace_id ON graphs (workspace_id)") conn.execute("CREATE INDEX idx_graph_tags_tag_id ON graph_tags (tag_id)") - graph_ids = [] + graph_ids: list[int] = [] for title, workspace_id in ( ("First Graph", default_id), ("Second Graph", default_id), @@ -902,7 +906,8 @@ def _create_migration_3_shaped_db(db_path) -> list[int]: "INSERT INTO graphs (title, data, workspace_id) VALUES (?, ?, ?)", (title, json.dumps({"nodes": []}), workspace_id), ) - graph_ids.append(cursor.lastrowid) + # lastrowid is Optional only until the cursor has run an INSERT. + graph_ids.append(cursor.lastrowid) # type: ignore[arg-type] conn.commit() conn.execute("PRAGMA user_version = 3") diff --git a/backend/tests/test_run_node_tool.py b/backend/tests/test_run_node_tool.py index 5ccdf387..07d7d570 100644 --- a/backend/tests/test_run_node_tool.py +++ b/backend/tests/test_run_node_tool.py @@ -53,7 +53,10 @@ async def approve(call: ToolCall) -> bool: ctx = RunContext(granted_scopes=frozenset(scopes), request_approval=approve) if run_id is not None: - ctx.run_id = run_id + # run_id lives on the builder's own RunContext subclass, and + # _run_id_of() reads it duck-typed; the base class it is attached to + # here has no such field to declare. Same seam as test_tools_graph.py. + ctx.run_id = run_id # type: ignore[attr-defined] return ctx diff --git a/backend/tests/test_serializer_state_guards.py b/backend/tests/test_serializer_state_guards.py index fdcfbece..42ca5b3a 100644 --- a/backend/tests/test_serializer_state_guards.py +++ b/backend/tests/test_serializer_state_guards.py @@ -20,6 +20,8 @@ from __future__ import annotations +from typing import Any + import pytest from backend.canvas import SceneNode @@ -28,8 +30,10 @@ from backend import session_save -# (serializer, kind it is registered for, extra positional args after the node) -SERIALIZERS = [ +# (serializer, kind it is registered for, extra positional args after the node). +# The extras are heterogeneous and mostly empty, so the element type has to be +# spelled out rather than inferred from the rows. +SERIALIZERS: list[tuple[Any, str, tuple[Any, ...]]] = [ (session_save._serialize_chat_node, "chat", ()), (session_save._serialize_code_node, "code", ()), (session_save._serialize_document_node, "document", ()), diff --git a/backend/tests/test_session_context.py b/backend/tests/test_session_context.py index deb9b159..92720843 100644 --- a/backend/tests/test_session_context.py +++ b/backend/tests/test_session_context.py @@ -28,7 +28,9 @@ def get(self, *_a, **_k): def _make_context() -> SessionContext: return SessionContext( - agent_dispatcher=AgentDispatcher(_FakeSettingsManager()), + # Nothing in these tests reaches settings at all, so the fake stands + # in for a SettingsManager it cannot be declared a subclass of. + agent_dispatcher=AgentDispatcher(_FakeSettingsManager()), # type: ignore[arg-type] canvas_document=SceneDocument(), ) diff --git a/backend/tests/test_streaming_partials.py b/backend/tests/test_streaming_partials.py index 188f5551..d4b33969 100644 --- a/backend/tests/test_streaming_partials.py +++ b/backend/tests/test_streaming_partials.py @@ -239,7 +239,10 @@ def _make_canvas_env(session_name: str): bus.register_topic("notification", notifications.payload) composer_document = ComposerDocument() bus.register_topic("app-composer", composer_document.payload) - dispatcher = AgentDispatcher(_FakeSettingsManager()) + # test_agents.py's fake, borrowed wholesale: it implements the one method + # AgentDispatcher reads, which a concrete SettingsManager parameter type + # cannot say is enough. + dispatcher = AgentDispatcher(_FakeSettingsManager()) # type: ignore[arg-type] document = register_canvas(bus, notifications, dispatcher, composer_document) return bus, notifications, composer_document, dispatcher, document diff --git a/backend/tests/test_tools_graph.py b/backend/tests/test_tools_graph.py index 2fcd7e28..53f8ecf2 100644 --- a/backend/tests/test_tools_graph.py +++ b/backend/tests/test_tools_graph.py @@ -61,8 +61,9 @@ async def request_approval(call: ToolCall) -> bool: if run_id is not None: # The builder's own context subclass carries run_id (backend/builder.py, # stage 8.3); until it exists, tests attach the attribute the same way - # _run_id_of() reads it - duck-typed, deliberately. - ctx.run_id = run_id + # _run_id_of() reads it - duck-typed, deliberately, which is exactly + # what the base RunContext cannot declare. + ctx.run_id = run_id # type: ignore[attr-defined] return ctx, prompts diff --git a/backend/tests/test_wrong_kind_node_guards.py b/backend/tests/test_wrong_kind_node_guards.py index 728deeff..07718f60 100644 --- a/backend/tests/test_wrong_kind_node_guards.py +++ b/backend/tests/test_wrong_kind_node_guards.py @@ -28,6 +28,8 @@ from __future__ import annotations +from typing import Any + import pytest from backend.domain.model import SceneError @@ -39,8 +41,12 @@ def _chat_id(doc: SceneDocument) -> str: return doc.add_chat_node(0.0, 0.0, "hello", True).id -# (method name, positional args after node_id, keyword args) -RAISES = [ +# (method name, positional args after node_id, keyword args). Spelled out +# rather than inferred because the two tables are concatenated below, and +# per-table inference gives them element types too narrow to add together. +_Call = tuple[str, tuple[Any, ...], dict[str, Any]] + +RAISES: list[_Call] = [ ("complete_web_research_run", ({"summary": "s"},), {}), ("fail_web_research_run", (), {"cancelled": False, "message": "boom"}), ("append_artifact_user_message", ("write it again",), {}), @@ -49,7 +55,7 @@ def _chat_id(doc: SceneDocument) -> str: ("complete_gitlink_apply", (2,), {}), ] -RETURNS_NONE = [ +RETURNS_NONE: list[_Call] = [ ("apply_web_research_progress", (object(),), {}), ("fail_artifact_generation", ("boom",), {}), ("complete_code_sandbox_run", ("code", "out", "analysis"), {}), diff --git a/backend/tools_graph.py b/backend/tools_graph.py index b5c89ea0..4e88b11d 100644 --- a/backend/tools_graph.py +++ b/backend/tools_graph.py @@ -38,6 +38,7 @@ import asyncio import json +from collections.abc import Callable from typing import Any from backend.domain.graph import SceneDocument, SceneError @@ -392,6 +393,12 @@ async def handler(call: ToolCall, ctx: RunContext) -> ToolResult: return _error(f"Unknown node: {node_id!r}.") run_id = _run_id_of(ctx) + # record_command is generic in whatever its mutator returns, and this + # call site discards that value, so the branches below are free to + # differ: some wrap a domain method that hands the node back, others + # one that returns None. Declared here so the first branch's return + # type is not silently imposed on all the rest. + mutator: Callable[[], object] if node.kind == "chat" and isinstance(node.state, ChatState): mutator = lambda: document.update_chat_node_content(node_id, content) elif node.kind == "note" and isinstance(node.state, NoteState): diff --git a/graphlink_chart_rendering.py b/graphlink_chart_rendering.py index e9186c43..a03e109e 100644 --- a/graphlink_chart_rendering.py +++ b/graphlink_chart_rendering.py @@ -422,7 +422,10 @@ def _render_sankey_chart(figure, ax, chart_data: dict[str, Any], theme: dict[str flows = chart_data["flows"] incoming = defaultdict(list) outgoing = defaultdict(list) - indegree = defaultdict(int) + # The `incoming`/`outgoing` defaultdicts above take their element type + # from the .append() calls below; this one is only ever read through + # augmented assignment, which gives mypy nothing to infer from. + indegree: defaultdict[str, int] = defaultdict(int) nodes = set() for flow in flows: diff --git a/graphlink_desktop.py b/graphlink_desktop.py index 612f5e2e..ef0536bd 100644 --- a/graphlink_desktop.py +++ b/graphlink_desktop.py @@ -35,7 +35,7 @@ import time import urllib.request from pathlib import Path -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any if TYPE_CHECKING: # ADR-015 stage 15.3: _start_backend/_shutdown_backend's own annotations @@ -289,7 +289,12 @@ def main() -> int: # # Rebuild assets/graphlink.ico with: python tools/build_app_icon.py icon_path = REPO_ROOT / "assets" / "graphlink.ico" - start_kwargs = {"debug": bool(os.environ.get("GRAPHLINK_DEBUG_WEBVIEW"))} + # Mixed value types (a bool, then a str path below), so an inferred + # dict[str, bool] would reject the icon - and, being **-splatted into + # webview.start(), every one of its keyword parameters too. + start_kwargs: dict[str, Any] = { + "debug": bool(os.environ.get("GRAPHLINK_DEBUG_WEBVIEW")), + } if icon_path.is_file(): start_kwargs["icon"] = str(icon_path) else: diff --git a/graphlink_plugins/review_lens/diff_fetch.py b/graphlink_plugins/review_lens/diff_fetch.py index 9632c3ea..ed1b7458 100644 --- a/graphlink_plugins/review_lens/diff_fetch.py +++ b/graphlink_plugins/review_lens/diff_fetch.py @@ -163,8 +163,13 @@ def _int(value: Any) -> int: diff_text, diff_truncated = _fetch_unified_diff(client, metadata_url) - base = metadata.get("base") if isinstance(metadata.get("base"), dict) else {} - head = metadata.get("head") if isinstance(metadata.get("head"), dict) else {} + # Bound to locals before the isinstance check rather than looked up twice + # inline: a check on one call cannot tell a type checker anything about a + # second, so the narrowing has to happen on a single value. + raw_base = metadata.get("base") + raw_head = metadata.get("head") + base = raw_base if isinstance(raw_base, dict) else {} + head = raw_head if isinstance(raw_head, dict) else {} return { "repo": slug, "pr_number": number, diff --git a/graphlink_plugins/web_research/crawl_etiquette.py b/graphlink_plugins/web_research/crawl_etiquette.py index 48a8aa56..b143e82f 100644 --- a/graphlink_plugins/web_research/crawl_etiquette.py +++ b/graphlink_plugins/web_research/crawl_etiquette.py @@ -109,7 +109,11 @@ def _load_robots(self, url: str, fetch: RobotsFetcher) -> "urllib.robotparser.Ro # Mirrors RobotFileParser.read()'s own convention: a robots.txt # we're not authorized to see is treated as "no bots wanted here # at all", not as "no rules exist". - parser.disallow_all = True + # RobotFileParser.__init__ really does set self.disallow_all, + # and its own read() assigns True to it on exactly this 401/403 + # branch - the attribute is just missing from typeshed's stub, so + # the ignore covers a gap there rather than anything wrong here. + parser.disallow_all = True # type: ignore[attr-defined] self._robots[origin] = parser return parser # Any other non-200 (404 included) - no robots.txt in effect, diff --git a/graphlink_plugins/web_research/fetch_policy.py b/graphlink_plugins/web_research/fetch_policy.py index 7fcc784b..7ce8841e 100644 --- a/graphlink_plugins/web_research/fetch_policy.py +++ b/graphlink_plugins/web_research/fetch_policy.py @@ -5,6 +5,7 @@ import ipaddress import socket from dataclasses import dataclass +from typing import Any, Callable, Sequence from urllib.parse import SplitResult, urlsplit, urlunsplit @@ -67,7 +68,11 @@ class FetchPolicy: read_timeout_seconds: float = 15.0 total_timeout_seconds: float = 30.0 max_bytes: int = 2 * 1024 * 1024 - resolver: object = socket.getaddrinfo + # Injected so tests (and any future caller with its own resolution + # policy) can substitute a resolver; the shape that matters is + # socket.getaddrinfo's - called with a host and a port, returning + # records whose fifth element is the sockaddr. + resolver: Callable[..., Sequence[Any]] = socket.getaddrinfo def _resolve_addresses(self, parsed: SplitResult) -> list[str]: if not parsed.hostname: diff --git a/graphlink_plugins/web_research/providers.py b/graphlink_plugins/web_research/providers.py index 7fc76441..c255223f 100644 --- a/graphlink_plugins/web_research/providers.py +++ b/graphlink_plugins/web_research/providers.py @@ -28,21 +28,31 @@ from ddgs import DDGS DUCKDUCKGO_SEARCH_AVAILABLE = True except ImportError: # pragma: no cover - exercised through diagnostics - DDGS = None + # This module's contract for every optional dependency is that the + # imported name is either the real thing or None, with the *_AVAILABLE + # flag beside it saying which - callers check the flag before touching + # the name (see dependency_status() and the guards in the providers + # below). mypy has no way to express a name whose type depends on what + # happens to be installed, so it reads the fallback as clobbering the + # imported class. That is what the ignore here and on the requests and + # bs4 fallbacks below covers, and nothing more. + DDGS = None # type: ignore[assignment, misc] DUCKDUCKGO_SEARCH_AVAILABLE = False try: import requests REQUESTS_AVAILABLE = True except ImportError: # pragma: no cover - exercised through diagnostics - requests = None + # See the ddgs fallback above for why this assignment needs an ignore. + requests = None # type: ignore[assignment] REQUESTS_AVAILABLE = False try: from bs4 import BeautifulSoup BEAUTIFULSOUP_AVAILABLE = True except ImportError: # pragma: no cover - exercised through diagnostics - BeautifulSoup = None + # See the ddgs fallback above for why this assignment needs an ignore. + BeautifulSoup = None # type: ignore[assignment, misc] BEAUTIFULSOUP_AVAILABLE = False @@ -165,7 +175,11 @@ def send(self, request, **kwargs): ) return super().send(request, **kwargs) else: - _PinnedHTTPAdapter = None + # The same optional-dependency shape as the imports at the top of this + # module: with no requests there is no HTTPAdapter to subclass, so the + # name is None and every path that constructs the adapter (all of which + # need a live requests Session to mount it on) is unreachable. + _PinnedHTTPAdapter = None # type: ignore[assignment, misc] class RequestsDocumentFetcher: @@ -455,7 +469,7 @@ def extract(self, payload: FetchedPayload, *, limits: ResearchLimits, token: Can except json.JSONDecodeError: text = decoded title = urlsplit(payload.final_url).hostname or "JSON source" - sections = (text,) + sections: tuple[str, ...] = (text,) elif payload.content_type == "text/plain": text = decoded title = urlsplit(payload.final_url).hostname or "Text source" diff --git a/graphlink_plugins/web_research/service.py b/graphlink_plugins/web_research/service.py index f96018b5..3debacd2 100644 --- a/graphlink_plugins/web_research/service.py +++ b/graphlink_plugins/web_research/service.py @@ -222,7 +222,7 @@ def run(self, request: WebResearchRequest, *, token: CancellationToken | None = from .domain import ResearchCitation - result = ResearchResult( + research_result = ResearchResult( request_id=request.request_id, original_query=query, effective_query=effective_query, @@ -233,4 +233,4 @@ def run(self, request: WebResearchRequest, *, token: CancellationToken | None = provider_snapshot=dict(request.provider_snapshot), ) self._emit(request, progress, ResearchStage.COMPLETED, "Research completed.", len(candidates), len(candidates)) - return result + return research_result diff --git a/graphlink_prompts.py b/graphlink_prompts.py index 2a24e404..f85bbaea 100644 --- a/graphlink_prompts.py +++ b/graphlink_prompts.py @@ -180,14 +180,18 @@ def _resolve_harness_core() -> str: return loop.HARNESS_SYSTEM_PROMPT +# reasoning_budget_hint returns `str | None` because "medium" deliberately +# has no hint - it is the model's own default. "low" and "high" are the two +# rungs that do carry text, which is why they are the two that are registered +# here at all, so neither resolver can actually see the None. def _resolve_reasoning_hint_low() -> str: import api_provider - return api_provider.reasoning_budget_hint("low") + return api_provider.reasoning_budget_hint("low") # type: ignore[return-value] def _resolve_reasoning_hint_high() -> str: import api_provider - return api_provider.reasoning_budget_hint("high") + return api_provider.reasoning_budget_hint("high") # type: ignore[return-value] _PROMPT_RESOLVERS = { diff --git a/graphlink_scratch_dirs.py b/graphlink_scratch_dirs.py index 67ced897..629d3617 100644 --- a/graphlink_scratch_dirs.py +++ b/graphlink_scratch_dirs.py @@ -96,7 +96,10 @@ def _ensure_private_scratch_root(root: Path) -> None: crashing outright - real POSIX systems always have it.""" root.mkdir(parents=True, exist_ok=True) try: - this_uid = os.getuid() + # mypy resolves `os` against the win32 stubs when it runs on Windows, + # where getuid genuinely does not exist - which is the case the + # except below is written for, not an error to fix. + this_uid = os.getuid() # type: ignore[attr-defined] except AttributeError: this_uid = None if this_uid is not None: diff --git a/provider_runtime/anthropic_transport.py b/provider_runtime/anthropic_transport.py index 1047d327..1bffd286 100644 --- a/provider_runtime/anthropic_transport.py +++ b/provider_runtime/anthropic_transport.py @@ -20,6 +20,7 @@ import base64 import json +from typing import Any def _anthropic_headers(api_key: str, extra_headers: dict | None = None) -> dict: @@ -61,7 +62,13 @@ def _attach_http_error_metadata(error: Exception, exc) -> Exception: predicate needs both. Attaches `status_code` (int) and `retry_after` (float seconds parsed from the Retry-After header, or None) onto the error about to be raised.""" - error.status_code = getattr(exc, "code", None) + # setattr, not plain attribute assignment: status_code and retry_after + # are metadata bolted onto an arbitrary Exception instance, so they are + # declared on no class. Every reader already goes through + # getattr(exc, ..., None) - see _is_transport_retryable and + # _retry_after_from_exception in api_provider - and setattr is simply + # the symmetric write side of that. + setattr(error, "status_code", getattr(exc, "code", None)) retry_after = None try: headers = getattr(exc, "headers", None) @@ -70,7 +77,7 @@ def _attach_http_error_metadata(error: Exception, exc) -> Exception: retry_after = float(str(header_value).strip()) except (TypeError, ValueError): retry_after = None - error.retry_after = retry_after + setattr(error, "retry_after", retry_after) return error @@ -206,7 +213,7 @@ def _anthropic_content_block_from_part(part: dict) -> dict | None: def _prepare_anthropic_messages(messages: list, cancel_event=None) -> tuple[str | None, list]: import api_provider as _mod # deferred: patch-seam safety (see module docstring) system_parts = [] - anthropic_messages = [] + anthropic_messages: list[dict[str, Any]] = [] for msg in messages: _mod._raise_if_cancelled(cancel_event) @@ -315,7 +322,7 @@ def _prepare_anthropic_kwargs(task: str, kwargs: dict, model_id: str = "", reaso def _extract_anthropic_text(response) -> str: import api_provider as _mod # deferred: patch-seam safety (see module docstring) answer_parts = [] - reasoning_parts = [] + reasoning_parts: list[str] = [] reasoning_seen: set[str] = set() for block in _mod._extract_response_field(response, "content", []) or []: diff --git a/provider_runtime/gemini_transport.py b/provider_runtime/gemini_transport.py index 223c6def..4df9f79d 100644 --- a/provider_runtime/gemini_transport.py +++ b/provider_runtime/gemini_transport.py @@ -21,6 +21,7 @@ import base64 import json import os +from typing import Any def _gemini_headers(api_key: str, extra_headers: dict | None = None) -> dict: @@ -259,8 +260,8 @@ def _gemini_part_from_content(part: dict, uploaded_files: list, cancel_event=Non def _prepare_gemini_contents(messages: list, cancel_event=None, api_key: str | None = None) -> tuple[str | None, list, list]: import api_provider as _mod # deferred: patch-seam safety (see module docstring) system_prompt = None - contents = [] - uploaded_files = [] + contents: list[dict[str, Any]] = [] + uploaded_files: list[str] = [] for msg in messages: _mod._raise_if_cancelled(cancel_event) @@ -273,7 +274,7 @@ def _prepare_gemini_contents(messages: list, cancel_event=None, api_key: str | N # tool_result block) and, like Ollama, provides no native call id - # GeminiProvider.stream() synthesizes one the same way Ollama's does. if role_name == "tool": - parts = [{ + parts: list[dict[str, Any]] = [{ "functionResponse": { "name": msg.get("name", ""), "response": {"result": str(msg.get("content") or "")}, diff --git a/provider_runtime/llama_cpp_runtime.py b/provider_runtime/llama_cpp_runtime.py index 278ac33b..a40a016d 100644 --- a/provider_runtime/llama_cpp_runtime.py +++ b/provider_runtime/llama_cpp_runtime.py @@ -91,7 +91,7 @@ def _get_llama_cpp_model_path(task: str, settings: dict | None = None) -> str: return active_settings.get("chat_model_path", "") -def _validate_llama_cpp_model_path(model_path: str, task: str): +def _validate_llama_cpp_model_path(model_path: str | None, task: str): import api_provider as _mod # deferred: patch-seam safety (see module docstring) raw_model_path = str(model_path or "").strip() if not raw_model_path: @@ -284,7 +284,11 @@ def graphlink_chat_handler(**call_kwargs): call_kwargs["enable_thinking"] = getattr(client, "_graphlink_enable_thinking", False) return base_handler(**call_kwargs) - graphlink_chat_handler._graphlink_wrapped_handler = True + # setattr, not plain attribute assignment: the marker is an ad-hoc + # attribute on a function object, declared on no type. Both readers + # above already use getattr(..., False), so setattr is the symmetric + # write side of the same convention. + setattr(graphlink_chat_handler, "_graphlink_wrapped_handler", True) client._graphlink_base_chat_handler = base_handler client._graphlink_enable_thinking = enable_thinking client.chat_handler = graphlink_chat_handler diff --git a/provider_runtime/llama_cpp_scan.py b/provider_runtime/llama_cpp_scan.py index 3a19323b..229c59f5 100644 --- a/provider_runtime/llama_cpp_scan.py +++ b/provider_runtime/llama_cpp_scan.py @@ -21,7 +21,7 @@ import os -def _normalize_llama_cpp_scan_root(path_value: str | None) -> Path | None: +def _normalize_llama_cpp_scan_root(path_value: str | Path | None) -> Path | None: normalized = str(path_value or "").strip() if not normalized: return None diff --git a/provider_runtime/ollama_scan.py b/provider_runtime/ollama_scan.py index e0a42a9c..a273ca87 100644 --- a/provider_runtime/ollama_scan.py +++ b/provider_runtime/ollama_scan.py @@ -26,7 +26,7 @@ from graphlink_model_catalog import ModelDescriptor -def _normalize_ollama_models_root(path_value: str | None) -> Path | None: +def _normalize_ollama_models_root(path_value: str | Path | None) -> Path | None: normalized = str(path_value or "").strip() if not normalized: return None diff --git a/provider_runtime/reasoning.py b/provider_runtime/reasoning.py index 8739b6ee..378a90c6 100644 --- a/provider_runtime/reasoning.py +++ b/provider_runtime/reasoning.py @@ -19,6 +19,7 @@ from __future__ import annotations import re +from typing import Any def normalize_reasoning_level(value: str | None) -> str: @@ -138,7 +139,7 @@ def anthropic_reasoning_kwargs(model_id: str, level: str, max_tokens: int) -> di # request that has reasoning enabled. return {"output_config": {"effort": level}} budget = _mod._ANTHROPIC_BUDGET_TOKENS[level] - result = {"thinking": {"type": "enabled", "budget_tokens": budget}} + result: dict[str, Any] = {"thinking": {"type": "enabled", "budget_tokens": budget}} if max_tokens <= budget: result["max_tokens"] = budget + _mod._ANTHROPIC_THINKING_HEADROOM_TOKENS return result diff --git a/pyproject.toml b/pyproject.toml index d5e951fe..ae1e480c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -352,46 +352,43 @@ select = ["E9", "F", "T201"] # [tool.ruff]'s comment above) rejects. Ratchet this list wider as more # modules earn real annotations - never narrower. files = [ - "contracts", - "backend/events.py", - "backend/token_counter.py", - "graphlink_process_env.py", - # 2026-09-04: backend/domain/, the whole package. - # - # The blocker was never annotation effort - it was that SceneNode.state is - # typed `NodeState | None` against a field-less marker, so every per-kind - # field access was unverifiable. backend/domain/node_access.py's - # require_node/optional_node/is_node_of narrow the NODE, which - # tests/test_node_state_migration.py permits (it constrains the ACCESS - # SHAPE, `.state.`, not how the node was obtained) where - # aliasing the state does not. + # 2026-09-04: everything. This list began as four files and was widened a + # module at a time as each one earned it; the last sweep took the tree from + # 179 errors to zero, so the ratchet's end state is simply "all of it". # - # Widened in three steps as the per-kind method groups came out of - # SceneDocument: three modules, then thirteen, then all eighteen once the - # four cross-cutting mixins were narrowed too. That last group could not - # use require_node directly - they read per-kind state off nodes they hold - # rather than look up by id, which is what is_node_of is for. - "backend/domain", - # 2026-09-04: the first widening since this list was written, and the - # ratchet moving the direction its own comment asks for. - # - # settings_store/ used to report 115 errors, every one attr-defined, and - # 101 of those were three names: state, _save_state, _protect_and_track. - # The package was not badly typed, it was untypeable - a mixin cannot be - # checked in isolation when nothing declares what the class composing it - # provides. settings_store/_composed.py declares exactly that, in - # TYPE_CHECKING only, and the package now checks clean with no - # behavioural change. See that module's own docstring. + # Kept as an explicit list of roots rather than ".", so that a new + # top-level directory has to be added here deliberately rather than + # arriving unchecked - and so this stays the record of what is covered. + "backend", + "contracts", + "graphlink_plugins", + "provider_runtime", "settings_store", - # 2026-09-04: the persistence layer, which was 60% of everything left. - # - # session_save.py alone held 246 of the 444 remaining `mypy backend` - # errors, every one the same NodeState | None union-attr. Its per-kind - # serializers are reached through a kind-keyed dispatch table, so each one - # already knows what it is looking at - it just had no way to say so. - # node_access.with_state is that way; the bodies are untouched. - "backend/session_save.py", - "backend/session_load.py", + "tests", + "tools", + "api_provider.py", + "graphlink_artifact_agent.py", + "graphlink_audio.py", + "graphlink_chart_data.py", + "graphlink_chart_rendering.py", + "graphlink_chat_agent.py", + "graphlink_desktop.py", + "graphlink_execution_guard.py", + "graphlink_grid_view_settings.py", + "graphlink_memory.py", + "graphlink_migrations.py", + "graphlink_model_catalog.py", + "graphlink_navigation_pins.py", + "graphlink_note_agent.py", + "graphlink_process_env.py", + "graphlink_prompts.py", + "graphlink_scratch_dirs.py", + "graphlink_secrets.py", + "graphlink_settings_store.py", + "graphlink_task_config.py", + "graphlink_token_estimator.py", + "graphlink_version.py", + "graphlink_wire_schema.py", ] ignore_missing_imports = true # Check the files in `files` fully; use everything they import for type diff --git a/tests/test_node_state_migration.py b/tests/test_node_state_migration.py index 3e61fbba..950b021f 100644 --- a/tests/test_node_state_migration.py +++ b/tests/test_node_state_migration.py @@ -440,7 +440,7 @@ def test_scene_payload_key_set_is_unchanged_by_the_migration(): # key-set-only test above cannot see, since the key was always present, # only its fallback VALUE for a non-owning node's row was wrong. Grows one # entry per migrated field, alongside MIGRATED_KIND_FIELDS above. -_EXPECTED_NON_OWNING_KIND_WIRE_DEFAULTS = { +_EXPECTED_NON_OWNING_KIND_WIRE_DEFAULTS: dict[str, object] = { "imageAssetId": "", "htmlSplitterState": None, "artifactContent": "", diff --git a/tools/build_app_icon.py b/tools/build_app_icon.py index 062a3032..a831ef3c 100644 --- a/tools/build_app_icon.py +++ b/tools/build_app_icon.py @@ -138,7 +138,11 @@ def tx(p): for pts, colour in STROKES: _draw_polyline(draw, [tx(p) for p in pts], stroke * scale, colour) - return img.resize((size, size), Image.LANCZOS) + # Pillow 9.1 moved the resampling filters onto Image.Resampling and + # left Image.LANCZOS as a deprecated alias the stubs no longer + # declare. The alias still resolves at runtime on the pinned + # version; naming its real home beats ignoring the report. + return img.resize((size, size), Image.Resampling.LANCZOS) def build_svg() -> str: diff --git a/tools/seed_demo_graph.py b/tools/seed_demo_graph.py index 582326cc..e498f0c0 100644 --- a/tools/seed_demo_graph.py +++ b/tools/seed_demo_graph.py @@ -36,6 +36,12 @@ from backend.chat_library import save_chat_atomically_row # noqa: E402 from backend.domain.graph import SceneDocument # noqa: E402 +from backend.domain.node_access import with_state # noqa: E402 +from backend.domain.node_states import ( # noqa: E402 + ChatState, + CodeSandboxState, + PlanState, +) from backend.session_save import build_chat_data # noqa: E402 QUESTION = ( @@ -216,8 +222,8 @@ def build_document() -> SceneDocument: hyp_db = doc.add_chat_node(-740, -330, HYPOTHESIS_DB, False, question.id) hyp_ser = doc.add_chat_node(-220, -330, HYPOTHESIS_SERIALIZATION, False, question.id) hyp_rb = doc.add_chat_node(300, -330, HYPOTHESIS_ROLLBACK, False, question.id) - hyp_ser.state.branch_status = "rejected" - hyp_rb.state.branch_status = "superseded" + with_state(hyp_ser, ChatState).state.branch_status = "rejected" + with_state(hyp_rb, ChatState).state.branch_status = "superseded" # Placed for the picture beside the branch it narrates; nothing else # references it. @@ -230,9 +236,10 @@ def build_document() -> SceneDocument: follow_up = doc.add_chat_node(-740, -60, FOLLOW_UP, True, hyp_db.id) sandbox = doc.add_code_sandbox_node(-1290, -520, follow_up.id) doc.set_code_sandbox_requirements(sandbox.id, "psycopg[binary]==3.2.1") - sandbox.state.code_sandbox_prompt = "Top offenders from pg_stat_statements, with plans" - sandbox.state.code_sandbox_code = SANDBOX_CODE - sandbox.state.code_sandbox_output = SANDBOX_OUTPUT + sandbox_node = with_state(sandbox, CodeSandboxState) + sandbox_node.state.code_sandbox_prompt = "Top offenders from pg_stat_statements, with plans" + sandbox_node.state.code_sandbox_code = SANDBOX_CODE + sandbox_node.state.code_sandbox_output = SANDBOX_OUTPUT research = doc.add_web_research_node(-1290, 190, follow_up.id) doc.start_web_research_run(research.id, RESEARCH_QUERY) @@ -266,12 +273,13 @@ def build_document() -> SceneDocument: plan = doc.add_plan_node(1400, -520, PLAN_GOAL, mode="copilot", max_steps=8, max_tokens=80_000, max_wall_seconds=1_800) doc.set_plan_steps(plan.id, PLAN_STEPS) - plan.state.builder_status = "done" - plan.state.builder_run_id = "run-demo-1" - plan.state.builder_spent_steps = 5 - plan.state.builder_spent_tokens = 42_710 - plan.state.builder_spent_wall_seconds = 763 - plan.state.builder_activity = ACTIVITY + plan_node = with_state(plan, PlanState) + plan_node.state.builder_status = "done" + plan_node.state.builder_run_id = "run-demo-1" + plan_node.state.builder_spent_steps = 5 + plan_node.state.builder_spent_tokens = 42_710 + plan_node.state.builder_spent_wall_seconds = 763 + plan_node.state.builder_activity = ACTIVITY note = doc.add_note(1400, -30) doc.set_note_content(note.id, NOTE_TEXT)