diff --git a/backend/agent_dispatch/_composed.py b/backend/agent_dispatch/_composed.py new file mode 100644 index 0000000..e87159a --- /dev/null +++ b/backend/agent_dispatch/_composed.py @@ -0,0 +1,66 @@ +"""What every backend/agent_dispatch mixin relies on the composed +AgentDispatcher to provide. + +The nine `*DispatchOps` classes are mixins, not standalone types: each is +composed exactly once, by +`class AgentDispatcher(DispatcherCoreOps, BuilderDispatchOps, ...)` in +backend/agents.py. They use `self._runs`, `self._settings_manager` and a few +of DispatcherCoreOps' own methods across the composition - correct at +runtime, invisible to a type checker reading one mixin on its own. + +Same shape, and same fix, as settings_store/_composed.py's +SettingsManagerParts and backend/domain/_composed.py's SceneDocumentParts. +See the first of those for the full reasoning. + +EVERYTHING HERE IS TYPE_CHECKING-ONLY: at runtime the class is empty, so +inheriting it adds no attributes, no methods and no `__init__`. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Any + +if TYPE_CHECKING: + from backend.events import SessionBus + from graphlink_settings_store import SettingsManager + + +class DispatcherParts: + """Type-only declaration of the composed AgentDispatcher's shared surface. + + DispatcherCoreOps inherits it too: it supplies most of what is declared + here and consumes the rest, the same way PersistenceOps does in + settings_store. + """ + + if TYPE_CHECKING: + # Established by DispatcherCoreOps.__init__. + _runs: Any # backend.run_lifecycle.RunRegistry + _settings_manager: SettingsManager + _provider_runtime: Any + _diagnostics: Any + + 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: ... + + # The run engine every start_* surface funnels through, and the + # plain-blocking-action skeleton the gitlink/code-review surfaces + # share. Both live on DispatcherCoreOps. + async def _dispatch(self, *args: Any, **kwargs: Any) -> Any: ... + + async def _run_node_blocking_action( + self, + *, + bus: SessionBus, + notifications_state: Any, + node: Any, + action: Any, + timeout: float, + timeout_message: str, + error_log_message: str, + error_notify_prefix: str, + default: Any = None, + ) -> Any: ... diff --git a/backend/agent_dispatch/builder.py b/backend/agent_dispatch/builder.py index 50824ff..2022372 100644 --- a/backend/agent_dispatch/builder.py +++ b/backend/agent_dispatch/builder.py @@ -24,8 +24,10 @@ import asyncio import threading +from backend.agent_dispatch._composed import DispatcherParts -class BuilderDispatchOps: + +class BuilderDispatchOps(DispatcherParts): """The Builder agent loop and its tool-registry assembly (mixin - see module docstring).""" def builder_tool_registry(self, document) -> "object": diff --git a/backend/agent_dispatch/chat.py b/backend/agent_dispatch/chat.py index 3a8a1d5..23309ad 100644 --- a/backend/agent_dispatch/chat.py +++ b/backend/agent_dispatch/chat.py @@ -30,8 +30,10 @@ if TYPE_CHECKING: from backend.events import SessionBus +from backend.agent_dispatch._composed import DispatcherParts -class ChatDispatchOps: + +class ChatDispatchOps(DispatcherParts): """Chat, conversation, and image reply surfaces (mixin - see module docstring).""" async def start_chat_reply( diff --git a/backend/agent_dispatch/code_review.py b/backend/agent_dispatch/code_review.py index 45ad1d6..77718f7 100644 --- a/backend/agent_dispatch/code_review.py +++ b/backend/agent_dispatch/code_review.py @@ -34,8 +34,10 @@ if TYPE_CHECKING: from backend.events import SessionBus +from backend.agent_dispatch._composed import DispatcherParts -class CodeReviewDispatchOps: + +class CodeReviewDispatchOps(DispatcherParts): """Review Lens dispatch: PR-diff fetch plus the Review and Ask surfaces (mixin - see module docstring).""" async def fetch_code_review_diff(self, *, bus: SessionBus, notifications_state, node, pr_url: str): diff --git a/backend/agent_dispatch/code_sandbox.py b/backend/agent_dispatch/code_sandbox.py index 87afc10..0b21098 100644 --- a/backend/agent_dispatch/code_sandbox.py +++ b/backend/agent_dispatch/code_sandbox.py @@ -29,8 +29,10 @@ if TYPE_CHECKING: from backend.events import SessionBus +from backend.agent_dispatch._composed import DispatcherParts -class CodeSandboxDispatchOps: + +class CodeSandboxDispatchOps(DispatcherParts): """The Execution Sandbox run surface and its plumbing (mixin - see module docstring).""" async def remove_code_sandbox_scratch_dir(self, sandbox_id: str) -> None: diff --git a/backend/agent_dispatch/content.py b/backend/agent_dispatch/content.py index 09a0f2e..6150922 100644 --- a/backend/agent_dispatch/content.py +++ b/backend/agent_dispatch/content.py @@ -26,8 +26,10 @@ if TYPE_CHECKING: from backend.events import SessionBus +from backend.agent_dispatch._composed import DispatcherParts -class ContentDispatchOps: + +class ContentDispatchOps(DispatcherParts): """Chart, note, branch-comparison, and branch-synthesis generation (mixin - see module docstring).""" async def start_chart_generation( diff --git a/backend/agent_dispatch/core.py b/backend/agent_dispatch/core.py index bb35d2f..d7f383c 100644 --- a/backend/agent_dispatch/core.py +++ b/backend/agent_dispatch/core.py @@ -34,8 +34,10 @@ from backend.events import SessionBus from graphlink_settings_store import SettingsManager +from backend.agent_dispatch._composed import DispatcherParts -class DispatcherCoreOps: + +class DispatcherCoreOps(DispatcherParts): """The shared dispatcher core: construction, model resolution, cancellation, approvals, and `_dispatch` (mixin - see module docstring).""" def __init__(self, settings_manager: SettingsManager, provider_runtime=None, diagnostics=None): diff --git a/backend/agent_dispatch/gitlink.py b/backend/agent_dispatch/gitlink.py index 907cb03..97fa5a8 100644 --- a/backend/agent_dispatch/gitlink.py +++ b/backend/agent_dispatch/gitlink.py @@ -34,8 +34,10 @@ if TYPE_CHECKING: from backend.events import SessionBus +from backend.agent_dispatch._composed import DispatcherParts -class GitlinkDispatchOps: + +class GitlinkDispatchOps(DispatcherParts): """Gitlink dispatch: repo plumbing plus the Run and Apply surfaces (mixin - see module docstring).""" async def fetch_gitlink_repositories(self, *, bus: SessionBus, notifications_state, node) -> list[str]: diff --git a/backend/agent_dispatch/harness.py b/backend/agent_dispatch/harness.py index 1b79e96..b00aa26 100644 --- a/backend/agent_dispatch/harness.py +++ b/backend/agent_dispatch/harness.py @@ -24,8 +24,10 @@ import asyncio import threading +from backend.agent_dispatch._composed import DispatcherParts -class HarnessDispatchOps: + +class HarnessDispatchOps(DispatcherParts): """The Harness agent: grants, process bookkeeping, and the Harness run (mixin - see module docstring).""" def answer_harness_question(self, request_id: str, answer) -> bool: diff --git a/backend/agent_dispatch/research.py b/backend/agent_dispatch/research.py index 48209f3..7a86242 100644 --- a/backend/agent_dispatch/research.py +++ b/backend/agent_dispatch/research.py @@ -29,8 +29,10 @@ if TYPE_CHECKING: from backend.events import SessionBus +from backend.agent_dispatch._composed import DispatcherParts -class ResearchDispatchOps: + +class ResearchDispatchOps(DispatcherParts): """Web Research and Artifact generation dispatch (mixin - see module docstring).""" async def start_web_research( diff --git a/backend/domain/_composed.py b/backend/domain/_composed.py new file mode 100644 index 0000000..345fd2f --- /dev/null +++ b/backend/domain/_composed.py @@ -0,0 +1,79 @@ +"""What every backend/domain mixin relies on SceneDocument to provide. + +BranchOps, GroupOps, LayoutOps and CommandOps are mixins, not standalone +types: each is composed exactly once, by +`class SceneDocument(BranchOps, GroupOps, LayoutOps, CommandOps)` in +backend/domain/graph.py. They freely use `self.nodes`, `self.edges`, +`self.command_log` and a handful of SceneDocument's own methods - correct at +runtime, and invisible to a type checker looking at one mixin in isolation. + +That invisibility is most of why `[tool.mypy].files` could not be widened to +backend/domain/. Nothing declared the contract between a mixin and the class +composing it, so the tree could not be checked at all - not because it was +badly typed, but because it was untypeable. + +Same shape, and same fix, as settings_store/_composed.py's +SettingsManagerParts. See that module's docstring for the full reasoning. + +EVERYTHING HERE IS TYPE_CHECKING-ONLY. At runtime this class is empty, so +inheriting it adds no attributes, no methods and no `__init__` - the real +implementations still come from SceneDocument's own body and from the +mixins themselves. It declares what the composed object has; it is never a +second source of it. +""" + +from __future__ import annotations + +import itertools +from collections import deque +from typing import TYPE_CHECKING, Any + +if TYPE_CHECKING: + from backend.domain.model import SceneEdge, SceneNode + + +class SceneDocumentParts: + """Type-only declaration of the composed SceneDocument's shared surface. + + Every mixin in this package inherits it. CommandOps supplies + command_log/redo_stack itself and consumes the rest, exactly as + PersistenceOps does in settings_store - a mixin both providing and + consuming is normal here. + """ + + if TYPE_CHECKING: + # Core graph state, held on SceneDocument itself. + nodes: dict[str, SceneNode] + edges: dict[str, SceneEdge] + image_assets: dict[str, Any] + measured_sizes: dict[str, Any] + pins: Any + _counter: itertools.count + + # The undo/redo command layer (CommandOps' own, consumed by siblings). + command_log: deque + redo_stack: list + _composite_depth: int + _composite_buffer: list + + # SceneDocument methods the mixins call across the composition. + def connect(self, source: str, target: str) -> SceneEdge: ... + + def remove_nodes(self, node_ids: list[str]) -> None: ... + + def add_chat_node( + self, x: float, y: float, content: str, is_user: bool, + parent_id: str | None = None, *args: Any, **kwargs: Any, + ) -> SceneNode: ... + + def place_root(self, kind: str) -> tuple[float, float]: ... + + def place_child( + self, parent_id: str | None, kind: str, *, prefer: str = "below", + ) -> tuple[float, float]: ... + + def _recompute_group_bounds(self, node_id: str) -> None: ... + + def _reaches(self, start: str, goal: str) -> bool: ... + + def _detach_node_from_membership(self, node_id: str) -> None: ... diff --git a/backend/domain/branches.py b/backend/domain/branches.py index 0c0bb5a..22313fa 100644 --- a/backend/domain/branches.py +++ b/backend/domain/branches.py @@ -31,8 +31,10 @@ if TYPE_CHECKING: from graphlink_model_catalog import ModelRef +from backend.domain._composed import SceneDocumentParts -class BranchOps: + +class BranchOps(SceneDocumentParts): def mark_branch_comparison_note(self, node_id: str, source_node_ids: list[str]) -> None: """ADR-002 Workstream 1 ("Compare Branches"): stamps an already-created diff --git a/backend/domain/commands.py b/backend/domain/commands.py index eff8628..671567c 100644 --- a/backend/domain/commands.py +++ b/backend/domain/commands.py @@ -85,6 +85,8 @@ from backend.domain.model import SceneEdge, SceneNode from graphlink_navigation_pins import NavigationPinRecord +from backend.domain._composed import SceneDocumentParts + T = TypeVar("T") # Asset bytes (image/chart PNGs) are the one thing in this module that IS @@ -444,7 +446,7 @@ def _merge_commands(command_type, provenance, commands, run_id=None): return merged -class CommandOps: +class CommandOps(SceneDocumentParts): def record_command( self, command_type: str, diff --git a/backend/domain/groups.py b/backend/domain/groups.py index 38b584f..97a057f 100644 --- a/backend/domain/groups.py +++ b/backend/domain/groups.py @@ -31,8 +31,10 @@ ) from backend.domain.node_states import ContainerState, FrameState +from backend.domain._composed import SceneDocumentParts -class GroupOps: + +class GroupOps(SceneDocumentParts): def _member_footprint(self, member: SceneNode) -> tuple[float, float]: """One member's (width, height) for bbox purposes, best source first: diff --git a/backend/domain/layout.py b/backend/domain/layout.py index 45a1c87..00821ac 100644 --- a/backend/domain/layout.py +++ b/backend/domain/layout.py @@ -34,6 +34,8 @@ SceneNode, ) +from backend.domain._composed import SceneDocumentParts + # Clearance between neighbouring nodes. Horizontal is a little wider than # vertical so sibling fans read as distinct columns; both are large enough # that edge routing has room to breathe between cards. @@ -91,7 +93,7 @@ def _rects_clear( ) -class LayoutOps: +class LayoutOps(SceneDocumentParts): """Placement/layout mixin for SceneDocument (same pattern as GroupOps). Reads self.nodes / self.edges / self.measured_sizes.""" diff --git a/tests/test_mixin_declaration_bases.py b/tests/test_mixin_declaration_bases.py new file mode 100644 index 0000000..d92d424 --- /dev/null +++ b/tests/test_mixin_declaration_bases.py @@ -0,0 +1,83 @@ +"""The type-only mixin declaration bases must stay type-only. + +Three of them now exist, one per mixin family: + + settings_store/_composed.py SettingsManagerParts + backend/domain/_composed.py SceneDocumentParts + backend/agent_dispatch/_composed.py DispatcherParts + +Each declares what a mixin's composing class provides, so a checker can read +one mixin in isolation. Together they took `mypy backend` from 1,059 errors +to 844 and eliminated the "*Ops has no attribute" class entirely. + +Each also sits LAST in its composed class's MRO, immediately before object, +which makes it the perfect place for an accident: anything defined outside +the `if TYPE_CHECKING:` block becomes a real attribute that shadows nothing +today but would be silently picked up the moment a sibling mixin stopped +defining its own. A fallback nobody asked for, in the one class written to +have no behaviour at all. + +tests/test_settings_mixin_contract.py covers SettingsManagerParts in more +depth (signature matching, name-by-name provisioning). This file holds the +properties that must be true of ALL of them, so a fourth base added later is +covered by construction rather than by remembering. +""" + +from __future__ import annotations + +import pytest + +from backend.agent_dispatch._composed import DispatcherParts +from backend.agents import AgentDispatcher +from backend.domain._composed import SceneDocumentParts +from backend.domain.graph import SceneDocument +from graphlink_settings_store import SettingsManager +from settings_store._composed import SettingsManagerParts + +# (declaration base, the class that composes it) +BASES = [ + pytest.param(SettingsManagerParts, SettingsManager, id="settings"), + pytest.param(SceneDocumentParts, SceneDocument, id="domain"), + pytest.param(DispatcherParts, AgentDispatcher, id="dispatch"), +] + + +@pytest.mark.parametrize("base, composed", BASES) +def test_the_declaration_base_has_no_runtime_body(base, composed): + """Nothing but dunders. A method or attribute here would be a real + implementation in a class whose entire purpose is to have none.""" + own = [name for name in vars(base) if not name.startswith("__")] + assert own == [], f"{base.__name__} gained a runtime member: {own}" + + +@pytest.mark.parametrize("base, composed", BASES) +def test_it_defines_no_initializer(base, composed): + """An __init__ here would land in the composed class's MRO after every + real mixin and quietly change construction.""" + assert "__init__" not in vars(base) + + +@pytest.mark.parametrize("base, composed", BASES) +def test_the_base_sits_last_in_the_mro(base, composed): + """Immediately before object: it must never take precedence over a real + mixin's implementation of the same name.""" + mro = composed.__mro__ + assert mro[-1] is object + assert mro[-2] is base, [c.__name__ for c in mro] + + +@pytest.mark.parametrize("base, composed", BASES) +def test_every_mixin_in_the_family_inherits_it(base, composed): + """A mixin that does not inherit the base is invisible to the checker + again - the exact hole these bases were added to close. Catches a new + sibling landing without one.""" + mixins = [ + cls for cls in composed.__mro__ + if cls not in (composed, base, object) and cls.__name__.endswith("Ops") + ] + assert mixins, f"no *Ops mixins found in {composed.__name__}'s MRO" + missing = [cls.__name__ for cls in mixins if not issubclass(cls, base)] + assert not missing, ( + f"these {composed.__name__} mixins do not inherit {base.__name__}, so nothing " + f"declares what they use from the composition: {missing}" + )