Narrow node state at the access point, and put mypy on backend/domain

backend/domain/node_access.py

@@ -0,0 +1,81 @@
+"""require_node - fetch a node, check its kind, and narrow its state type.
+
+Every per-kind SceneDocument method opens with the same five lines: look the
+node up, raise if it is missing, raise if it is the wrong kind, then read or
+write `node.state.<kind>_<field>`. Eighteen copies of that preamble existed
+in the two extracted per-kind mixins alone.
+
+The repetition is the smaller half. The larger half is that
+`SceneNode.state` is typed `NodeState | None` against a marker class with no
+fields, so EVERY one of those field accesses is unverifiable - 327 of the
+845 remaining `mypy backend` errors are exactly this, and it is why
+[tool.mypy].files could not widen to backend/domain/.
+
+WHY THIS WORKS, when a `state = node.state` alias does not:
+tests/test_node_state_migration.py requires every migrated field to be read
+as `<anything>.state.<field>` - see its own `_is_via_state`, which checks
+only that the attribute being accessed sits on something whose attribute is
+`state`. It constrains the SHAPE of the access, not how the node was
+obtained. So narrowing the NODE is allowed where aliasing the STATE is not,
+and `node.state.code_review_pr_url` satisfies the gate and the type checker
+at the same time.
+
+_NodeWith exists only for the checker: at runtime require_node returns the
+plain SceneNode it looked up, unchanged.
+"""
+
+from __future__ import annotations
+
+from typing import TYPE_CHECKING, Generic, TypeVar
+
+from backend.domain.model import SceneError, SceneNode
+from backend.domain.node_states import NodeState
+
+_S = TypeVar("_S", bound=NodeState)
+
+if TYPE_CHECKING:
+    class _NodeWith(SceneNode, Generic[_S]):
+        """A SceneNode whose `state` is known to be a particular kind's.
+
+        Type-only: never constructed, never imported at runtime.
+        """
+
+        state: _S  # type: ignore[assignment]
+
+
+def require_node(
+    nodes: dict[str, SceneNode], node_id: str, kind: str, state_cls: type[_S],
+) -> _NodeWith[_S]:
+    """The node with `node_id`, guaranteed to exist and to be `kind`.
+
+    Raises SceneError with the same two messages every hand-written copy of
+    this preamble used, so the wire-level error text callers already depend
+    on is unchanged.
+
+    `state_cls` is not checked at runtime - the kind string is the real
+    guarantee, and the per-kind state class is assigned by the one
+    `add_*_node` constructor for that kind. It is here to carry the type
+    through, and to make the call site say which state it expects.
+    """
+    node = nodes.get(node_id)
+    if node is None:
+        raise SceneError(f"unknown node: {node_id}")
+    if node.kind != kind:
+        raise SceneError(f"node is not a {kind} node: {node_id}")
+    return node  # type: ignore[return-value]
+
+
+def optional_node(
+    nodes: dict[str, SceneNode], node_id: str, kind: str, state_cls: type[_S],
+) -> "_NodeWith[_S] | None":
+    """require_node's silent sibling: None instead of a raise.
+
+    The `fail_*_run` methods deliberately do nothing when their node has
+    already been deleted - a run that fails after the user removed its node
+    is not an error worth surfacing. They still need the narrowed state type
+    for the fields they set on the way through.
+    """
+    node = nodes.get(node_id)
+    if node is None or node.kind != kind:
+        return None
+    return node  # type: ignore[return-value]

backend/domain/nodes_code_review.py

@@ -22,6 +22,7 @@
 
 from backend.domain._composed import SceneDocumentParts
 from backend.domain.model import SceneError, SceneNode
+from backend.domain.node_access import optional_node, require_node
 from backend.domain.node_states import CodeReviewState
 
 def _bundle_int(value: object) -> int:
@@ -33,7 +34,7 @@ def _bundle_int(value: object) -> int:
     defence, for a bundle that reached here by any other route (a test, a
     future caller, a hand-built payload)."""
     try:
-        return max(0, int(value))  # type: ignore[arg-type]
+        return max(0, int(value))  # type: ignore[call-overload]
     except (TypeError, ValueError):
         return 0
 
@@ -66,11 +67,7 @@ def set_code_review_pr_url(self, node_id: str, pr_url: str) -> SceneNode:
         or pastes the PR link BEFORE ever clicking Fetch, with no other
         action call site to piggyback on (the setGitlinkLocalRoot
         precedent exactly)."""
-        node = self.nodes.get(node_id)
-        if node is None:
-            raise SceneError(f"unknown node: {node_id}")
-        if node.kind != "code_review":
-            raise SceneError(f"node is not a code_review node: {node_id}")
+        node = require_node(self.nodes, node_id, "code_review", CodeReviewState)
         node.state.code_review_pr_url = str(pr_url)
         return node
 
@@ -97,11 +94,7 @@ def store_code_review_diff(
         UNCONDITIONALLY (the R5.3 post-review FIX 6 precedent) so the
         frontend's lazy-diff guard can never serve a previous fetch's
         text for this one."""
-        node = self.nodes.get(node_id)
-        if node is None:
-            raise SceneError(f"unknown node: {node_id}")
-        if node.kind != "code_review":
-            raise SceneError(f"node is not a code_review node: {node_id}")
+        node = require_node(self.nodes, node_id, "code_review", CodeReviewState)
         pr_number_value = _bundle_int(bundle.get("pr_number"))
         node.state.code_review_pr_url = str(pr_url)
         node.state.code_review_repo = str(bundle.get("repo", ""))
@@ -144,23 +137,15 @@ def fetch_code_review_diff_text(self, node_id: str) -> str:
         EXCLUDED from scene_payload() (see CodeReviewState's own comment) -
         this is the only way the frontend ever gets the full text, via the
         read-only fetchCodeReviewDiffText intent."""
-        node = self.nodes.get(node_id)
-        if node is None:
-            raise SceneError(f"unknown node: {node_id}")
-        if node.kind != "code_review":
-            raise SceneError(f"node is not a code_review node: {node_id}")
+        node = require_node(self.nodes, node_id, "code_review", CodeReviewState)
         return node.state.code_review_diff_text
 
     def start_code_review_run(self, node_id: str) -> SceneNode:
         """Mark a review run started: clears the error banner but keeps any
         prior review visible until the new one lands (stale-while-
         revalidate, the start_gitlink_run precedent - a failed re-run must
         never blank a previously good review)."""
-        node = self.nodes.get(node_id)
-        if node is None:
-            raise SceneError(f"unknown node: {node_id}")
-        if node.kind != "code_review":
-            raise SceneError(f"node is not a code_review node: {node_id}")
+        node = require_node(self.nodes, node_id, "code_review", CodeReviewState)
         node.state.code_review_error = ""
         return node
 
@@ -187,11 +172,7 @@ def complete_code_review_run(
         review resets dismissals: finding ids are re-minted per review,
         so a dismissal of the old review's f3 must never hide the new
         review's f3."""
-        node = self.nodes.get(node_id)
-        if node is None:
-            raise SceneError(f"unknown node: {node_id}")
-        if node.kind != "code_review":
-            raise SceneError(f"node is not a code_review node: {node_id}")
+        node = require_node(self.nodes, node_id, "code_review", CodeReviewState)
         node.state.code_review_title = str(title)
         node.state.code_review_overview = str(overview)
         node.state.code_review_confidence = str(confidence)
@@ -222,7 +203,7 @@ def fail_code_review_run(self, node_id: str, message: str) -> SceneNode | None:
         (the fail_gitlink_run precedent). Deliberately does NOT clear any
         prior review - a failed re-run must never wipe out a previously
         good one; only the error banner reflects the new failure."""
-        node = self.nodes.get(node_id)
+        node = optional_node(self.nodes, node_id, "code_review", CodeReviewState)
         if node is None:
             return None
         node.state.code_review_error = str(message)
@@ -233,11 +214,7 @@ def dismiss_code_review_finding(self, node_id: str, finding_id: str) -> SceneNod
         affordance). Idempotent: unknown ids and repeats are quiet no-ops,
         never errors - dismissal is UI state, and a double-click must not
         be able to fail a run."""
-        node = self.nodes.get(node_id)
-        if node is None:
-            raise SceneError(f"unknown node: {node_id}")
-        if node.kind != "code_review":
-            raise SceneError(f"node is not a code_review node: {node_id}")
+        node = require_node(self.nodes, node_id, "code_review", CodeReviewState)
         dismissed = str(finding_id)
         known_ids = {
             str(item.get("id")) for item in (
@@ -252,11 +229,7 @@ def append_code_review_qa(self, node_id: str, question: str, answer: str) -> Sce
         """Land one answered follow-up. Capped at the 20 most recent
         entries - the Q&A list is on the wire (unlike the diff text), so
         unbounded growth here would be unbounded wire growth."""
-        node = self.nodes.get(node_id)
-        if node is None:
-            raise SceneError(f"unknown node: {node_id}")
-        if node.kind != "code_review":
-            raise SceneError(f"node is not a code_review node: {node_id}")
+        node = require_node(self.nodes, node_id, "code_review", CodeReviewState)
         node.state.code_review_qa.append({
             "question": str(question),
             "answer": str(answer),

backend/domain/nodes_gitlink.py

@@ -13,6 +13,7 @@
 
 from backend.domain._composed import SceneDocumentParts
 from backend.domain.model import SceneError, SceneNode
+from backend.domain.node_access import optional_node, require_node
 from backend.domain.node_states import GitlinkState
 
 class GitlinkOps(SceneDocumentParts):
@@ -46,23 +47,15 @@ def set_gitlink_local_root(self, node_id: str, local_root: str) -> SceneNode:
         action parameter instead): the user may type/paste a local checkout
         path BEFORE ever clicking Import/Build Context, with no other action
         call site to piggyback on."""
-        node = self.nodes.get(node_id)
-        if node is None:
-            raise SceneError(f"unknown node: {node_id}")
-        if node.kind != "gitlink":
-            raise SceneError(f"node is not a gitlink node: {node_id}")
+        node = require_node(self.nodes, node_id, "gitlink", GitlinkState)
         node.state.gitlink_local_root = str(local_root)
         return node
 
     def store_gitlink_repo_tree(self, node_id: str, repo: str, branch: str, file_paths: list[str]) -> SceneNode:
         """Lands a successful loadGitlinkRepoTree result: repo, branch
         (resolved server-side, including any default-branch lookup), and the
         scanned text-file path list."""
-        node = self.nodes.get(node_id)
-        if node is None:
-            raise SceneError(f"unknown node: {node_id}")
-        if node.kind != "gitlink":
-            raise SceneError(f"node is not a gitlink node: {node_id}")
+        node = require_node(self.nodes, node_id, "gitlink", GitlinkState)
         node.state.gitlink_repo = str(repo)
         node.state.gitlink_branch = str(branch)
         node.state.gitlink_repo_file_paths = list(file_paths)
@@ -73,11 +66,7 @@ def store_gitlink_snapshot_root(self, node_id: str, repo: str, branch: str, loca
         repo/branch/local_root AND gitlink_imported_root (so a later run
         knows this path came from an import, matching legacy repo_state's
         imported_root concept)."""
-        node = self.nodes.get(node_id)
-        if node is None:
-            raise SceneError(f"unknown node: {node_id}")
-        if node.kind != "gitlink":
-            raise SceneError(f"node is not a gitlink node: {node_id}")
+        node = require_node(self.nodes, node_id, "gitlink", GitlinkState)
         node.state.gitlink_repo = str(repo)
         node.state.gitlink_branch = str(branch)
         node.state.gitlink_local_root = str(local_root)
@@ -110,11 +99,7 @@ def store_gitlink_context(
         produce an IDENTICAL summary string (see that field's own comment on
         SceneNode), which was tricking the frontend's lazy-fetch-once guard
         into skipping a real refetch and showing stale XML."""
-        node = self.nodes.get(node_id)
-        if node is None:
-            raise SceneError(f"unknown node: {node_id}")
-        if node.kind != "gitlink":
-            raise SceneError(f"node is not a gitlink node: {node_id}")
+        node = require_node(self.nodes, node_id, "gitlink", GitlinkState)
         node.state.gitlink_scope_mode = str(scope_mode)
         node.state.gitlink_selected_paths = list(selected_paths)
         node.state.gitlink_context_xml = str(context_xml)
@@ -128,11 +113,7 @@ def fetch_gitlink_context_xml(self, node_id: str) -> str:
         from scene_payload() (see the field's own comment on SceneNode) - this
         is the only way the frontend ever gets the full text, via the
         read-only fetchGitlinkContext intent."""
-        node = self.nodes.get(node_id)
-        if node is None:
-            raise SceneError(f"unknown node: {node_id}")
-        if node.kind != "gitlink":
-            raise SceneError(f"node is not a gitlink node: {node_id}")
+        node = require_node(self.nodes, node_id, "gitlink", GitlinkState)
         return node.state.gitlink_context_xml
 
     def start_gitlink_run(self, node_id: str, task_prompt: str) -> SceneNode:
@@ -143,11 +124,7 @@ def start_gitlink_run(self, node_id: str, task_prompt: str) -> SceneNode:
         complete_gitlink_run lands a real result, same stale-while-revalidate
         posture web research's own start_web_research_run documents for
         research_result."""
-        node = self.nodes.get(node_id)
-        if node is None:
-            raise SceneError(f"unknown node: {node_id}")
-        if node.kind != "gitlink":
-            raise SceneError(f"node is not a gitlink node: {node_id}")
+        node = require_node(self.nodes, node_id, "gitlink", GitlinkState)
         node.state.gitlink_task_prompt = str(task_prompt)
         node.state.gitlink_error = ""
         return node
@@ -179,9 +156,7 @@ def complete_gitlink_run(
         `local_root` is compared as raw trimmed text against
         start_gitlink_apply's own local_root_text - stored stripped here so
         that comparison lines up exactly."""
-        node = self.nodes.get(node_id)
-        if node is None:
-            raise SceneError(f"unknown node: {node_id}")
+        node = require_node(self.nodes, node_id, "gitlink", GitlinkState)
         node.state.gitlink_proposal_markdown = str(proposal_markdown)
         node.state.gitlink_pending_changes = list(pending_changes or [])
         node.state.gitlink_preview_text = str(preview_text)
@@ -204,7 +179,7 @@ def fail_gitlink_run(self, node_id: str, message: str) -> SceneNode | None:
         change_state - a failed re-run must never wipe out a previously
         staged, still-valid proposal; only the error banner reflects the
         new failure."""
-        node = self.nodes.get(node_id)
+        node = optional_node(self.nodes, node_id, "gitlink", GitlinkState)
         if node is None:
             return None
         node.state.gitlink_error = str(message)
@@ -225,9 +200,7 @@ def complete_gitlink_apply(self, node_id: str, written_files: int) -> SceneNode:
         gitlink_proposal_markdown/gitlink_preview_text are DELIBERATELY left
         untouched - they remain visible as a historical record of what was
         applied."""
-        node = self.nodes.get(node_id)
-        if node is None:
-            raise SceneError(f"unknown node: {node_id}")
+        node = require_node(self.nodes, node_id, "gitlink", GitlinkState)
         node.state.gitlink_change_state = "applied"
         node.state.gitlink_error = ""
         node.state.gitlink_pending_changes = []
@@ -244,7 +217,7 @@ def fail_gitlink_apply(self, node_id: str, message: str) -> SceneNode | None:
         fingerprint-mismatch refusal path, the local_root-mismatch refusal
         path, and the write-failure path identically - all three are "the
         apply did not happen, here is why"."""
-        node = self.nodes.get(node_id)
+        node = optional_node(self.nodes, node_id, "gitlink", GitlinkState)
         if node is None:
             return None
         node.state.gitlink_change_state = "previewed"

backend/tests/test_review_lens_backend.py

@@ -475,3 +475,33 @@ async def run():
         assert node_id not in document.nodes
 
     asyncio.run(run())
+
+
+# -- wrong-kind guards, now uniform ------------------------------------------
+# require_node/optional_node (backend/domain/node_access.py) gave three gitlink
+# methods and two fail_*_run methods a kind check they did not have. Before,
+# passing a wrong-kind node id reached `node.state.<kind>_<field>` on a state
+# class without that field and raised AttributeError - which the WS layer does
+# not translate, unlike SceneError. These pin the new, uniform behaviour.
+
+
+def test_a_wrong_kind_node_raises_scene_error_not_attribute_error():
+    doc, _ = _doc_with_review()
+    chat = doc.add_chat_node(0, 0, "c", is_user=True)
+    for call in (
+        lambda: doc.store_code_review_diff(chat.id, pr_url="u", bundle=_bundle()),
+        lambda: doc.fetch_code_review_diff_text(chat.id),
+        lambda: doc.append_code_review_qa(chat.id, "q", "a"),
+    ):
+        with pytest.raises(SceneError):
+            call()
+
+
+def test_fail_run_is_a_quiet_no_op_for_a_wrong_kind_node():
+    """fail_*_run is documented as silent when its node has gone. A node that
+    is present but of another kind is the same situation from the run's point
+    of view, and used to raise AttributeError instead."""
+    doc, _ = _doc_with_review()
+    chat = doc.add_chat_node(0, 0, "c", is_user=True)
+    assert doc.fail_code_review_run(chat.id, "boom") is None
+    assert doc.fail_code_review_run("missing", "boom") is None

pyproject.toml

@@ -356,6 +356,20 @@ files = [
     "backend/events.py",
     "backend/token_counter.py",
     "graphlink_process_env.py",
+    # 2026-09-04, second widening: the first three backend/domain/ entries.
+    #
+    # 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 narrows the NODE, which
+    # tests/test_node_state_migration.py permits (it constrains the ACCESS
+    # SHAPE, `<anything>.state.<field>`, not how the node was obtained) where
+    # aliasing the state does not. These three modules check clean because of
+    # it; the rest of backend/domain/ follows as its per-kind groups are
+    # extracted the same way.
+    "backend/domain/node_access.py",
+    "backend/domain/nodes_code_review.py",
+    "backend/domain/nodes_gitlink.py",
     # 2026-09-04: the first widening since this list was written, and the
     # ratchet moving the direction its own comment asks for.
     #