From db920b69a0ed000cc9bed565d3613778761c3b37 Mon Sep 17 00:00:00 2001 From: dovvnloading <157447210+dovvnloading@users.noreply.github.com> Date: Fri, 4 Sep 2026 12:45:50 -0400 Subject: [PATCH 1/2] Finish lifting the per-kind node methods out of SceneDocument PR #409 moved the two largest per-kind method groups (code_review, gitlink) into their own mixins and left the other thirteen kinds behind. This moves those thirteen, which is the rest of that job: 42 members and 983 lines out of graph.py, taking it from 2,591 lines to 1,502. Five new mixins, grouped by what the kinds actually have in common rather than one module per kind: nodes_agent_runs.py AgentRunOps web_research, artifact, code_sandbox - the three kinds with a start/progress/complete/ fail run lifecycle nodes_planning.py PlanningOps plan, harness nodes_conversational.py ConversationalOps chat, conversation nodes_content.py ContentOps document, thinking, html, note nodes_visual.py VisualOps chart, image Every one of the 42 moved members was verified AST-identical to its pre-move version (ast.unparse round-trip against `git show HEAD:backend/domain/graph.py`), verified absent from SceneDocument's own body afterwards, verified reachable on SceneDocument, and verified to be defined exactly once across the whole MRO. Methods are regrouped by kind inside each new module instead of keeping the order that successive increments happened to append them in; nothing else changed. Two supporting edits: SceneDocumentParts gains `adopt_pending_system_prompt`, which add_chat_node calls and which stays on SceneDocument, and its `add_chat_node` declaration is tightened from a `*args: Any, **kwargs: Any` hedge to the real signature. The hedge was fine while the body lived in SceneDocument itself; now that ConversationalOps defines it, an inexact declaration in a base class is an incompatible-override error rather than a harmless fiction. #409 also left two section-header comments in graph.py describing methods it had just moved away, plus 21 stray blank lines where they used to sit. The headers carried real information about those kinds' import posture, so that text moves into the nodes_gitlink.py and nodes_code_review.py module docstrings rather than being dropped. Test plan: full suite, 3,226 passed / 19 skipped. ruff clean. Co-Authored-By: Claude Opus 5 --- backend/domain/_composed.py | 11 +- backend/domain/graph.py | 1113 +----------------------- backend/domain/nodes_agent_runs.py | 338 +++++++ backend/domain/nodes_code_review.py | 6 + backend/domain/nodes_content.py | 238 +++++ backend/domain/nodes_conversational.py | 218 +++++ backend/domain/nodes_gitlink.py | 8 + backend/domain/nodes_planning.py | 149 ++++ backend/domain/nodes_visual.py | 278 ++++++ 9 files changed, 1257 insertions(+), 1102 deletions(-) create mode 100644 backend/domain/nodes_agent_runs.py create mode 100644 backend/domain/nodes_content.py create mode 100644 backend/domain/nodes_conversational.py create mode 100644 backend/domain/nodes_planning.py create mode 100644 backend/domain/nodes_visual.py diff --git a/backend/domain/_composed.py b/backend/domain/_composed.py index 345fd2f..442a0f8 100644 --- a/backend/domain/_composed.py +++ b/backend/domain/_composed.py @@ -61,11 +61,20 @@ def connect(self, source: str, target: str) -> SceneEdge: ... def remove_nodes(self, node_ids: list[str]) -> None: ... + # ConversationalOps' own, consumed by VisualOps' generated-image + # reply. Declared with the real signature, not a *args hedge: the + # implementation is now a sibling mixin the checker can read, so an + # inexact declaration here would be an incompatible-override error + # rather than the useful fiction it was while the body lived in + # SceneDocument itself. def add_chat_node( self, x: float, y: float, content: str, is_user: bool, - parent_id: str | None = None, *args: Any, **kwargs: Any, + parent_id: str | None = None, + content_parts: list[dict[str, Any]] | None = None, ) -> SceneNode: ... + def adopt_pending_system_prompt(self, root_id: str) -> SceneEdge | None: ... + def place_root(self, kind: str) -> tuple[float, float]: ... def place_child( diff --git a/backend/domain/graph.py b/backend/domain/graph.py index f13ccef..3019da2 100644 --- a/backend/domain/graph.py +++ b/backend/domain/graph.py @@ -36,11 +36,9 @@ import itertools import json import logging -import uuid from dataclasses import dataclass, field from typing import Any, Callable -from graphlink_chart_data import SUPPORTED_CHART_TYPES from graphlink_grid_view_settings import ( GRID_SIZE_PRESETS, GRID_STYLE_PRESETS, @@ -51,17 +49,10 @@ from backend.domain.branches import BranchOps from backend.domain.commands import CommandOps -from backend.domain.nodes_code_review import CodeReviewOps -from backend.domain.nodes_gitlink import GitlinkOps from backend.domain.content_codec import _content_codec from backend.domain.groups import GroupOps from backend.domain.layout import LayoutOps from backend.domain.model import ( - CHART_MAX_HEIGHT, - CHART_MAX_WIDTH, - CHART_MIN_HEIGHT, - CHART_MIN_WIDTH, - CHAT_TITLE_PREVIEW_LENGTH, CODE_TITLE_PREVIEW_LENGTH, DRAG_FACTOR_MAX, DRAG_FACTOR_MIN, @@ -69,12 +60,9 @@ FONT_SIZE_MAX, FONT_SIZE_MIN, GRID_COLOR_PRESETS, - HTML_TITLE_PREVIEW_LENGTH, - IMAGE_TITLE_PREVIEW_LENGTH, SceneEdge, SceneError, SceneNode, - THINKING_TITLE_PREVIEW_LENGTH, ) from backend.domain.node_states import ( ArtifactState, @@ -95,6 +83,13 @@ PlanState, WebResearchState, ) +from backend.domain.nodes_agent_runs import AgentRunOps +from backend.domain.nodes_code_review import CodeReviewOps +from backend.domain.nodes_content import ContentOps +from backend.domain.nodes_conversational import ConversationalOps +from backend.domain.nodes_gitlink import GitlinkOps +from backend.domain.nodes_planning import PlanningOps +from backend.domain.nodes_visual import VisualOps def _estimate_tokens(text: str) -> int: @@ -107,7 +102,11 @@ def _estimate_tokens(text: str) -> int: @dataclass -class SceneDocument(BranchOps, GroupOps, LayoutOps, CommandOps, CodeReviewOps, GitlinkOps): +class SceneDocument( + BranchOps, GroupOps, LayoutOps, CommandOps, + AgentRunOps, CodeReviewOps, ContentOps, ConversationalOps, GitlinkOps, + PlanningOps, VisualOps, +): """The canvas document for one session. Plain data + invariants; the R6 serializer will read/write exactly this shape.""" @@ -514,48 +513,6 @@ def clear_for_load(self) -> None: self._published_view = None self._published_meta = None - def add_chat_node( - self, - x: float, - y: float, - content: str, - is_user: bool, - parent_id: str | None = None, - content_parts: list[dict[str, Any]] | None = None, - ) -> SceneNode: - """The Qt-free ChatScene.add_chat_node equivalent: a real message- - bubble node, optionally connected to a parent (the branch it - continues). Mirrors add_node's id/dict bookkeeping; the only new - behavior is the parent-edge, ported from the legacy scene's own - ConnectionItem creation. - - R8a: content_parts is the real multimodal attachment payload - (image_bytes/audio_file parts) - the data-model capability - ChatState.content_parts (backend/domain/node_states.py) has carried - since R6.3, finally populated by a real caller. Optional and - additive: every existing caller keeps - passing only (x, y, content, is_user, parent_id) and gets exactly - the plain-text node it always did.""" - if parent_id is not None and parent_id not in self.nodes: - raise SceneError(f"unknown parent node: {parent_id}") - node_id = f"n{next(self._counter)}" - title = content[:CHAT_TITLE_PREVIEW_LENGTH] or ("You" if is_user else "Assistant") - node = SceneNode( - id=node_id, - x=float(x), - y=float(y), - title=title, - kind="chat", - content=str(content), - state=ChatState(is_user=bool(is_user), content_parts=content_parts), - ) - self.nodes[node_id] = node - if parent_id is not None: - self.connect(parent_id, node_id) - else: - self.adopt_pending_system_prompt(node_id) - return node - def adopt_pending_system_prompt(self, root_id: str) -> SceneEdge | None: """Connects an unattached system-prompt note to a new branch root. @@ -631,1040 +588,6 @@ def add_code_node( self.connect(parent_id, node_id) return node - def add_document_node( - self, - x: float, - y: float, - title: str, - content: str, - attachment_kind: str, - parent_id: str, - *, - file_path: str = "", - mime_type: str = "", - duration_seconds: float | None = None, - byte_size: int | None = None, - preview_label: str = "", - ) -> SceneNode: - """R3.9's document-node equivalent of add_chat_node/add_code_node: a - real file-attachment node (a document or an audio file), for the - legacy DocumentNode / ChatScene.add_document_node pair. UNLIKE - chat/code, parent_id is REQUIRED here, not optional: read fresh from - graphlink_scene.py, add_document_node(title, content, - parent_user_node, ...) takes parent_user_node as a plain required - positional with no default, and unconditionally constructs a - DocumentConnectionItem(parent_user_node, node) - there is no `if - parent_id` guard around that connection the way chat/code have - around theirs - so a DocumentNode can never exist unparented. - Document nodes are also NOT branch points (same as code): there is - no delete_document_node; deletion goes entirely through the - existing generic remove_nodes. - - The six attachment fields are stored verbatim - no title-preview - truncation (DocumentNode.title in the legacy app is just whatever - descriptive title/filename was passed in, confirmed by reading - DocumentNode.__init__: `self.title = title`, no slicing), and none - of the legacy view-layer formatting (byte-size/duration strings, - preview_label auto-fill, audio-preview suppression) happens here - - see DocumentState's own docstring (backend/domain/node_states.py) - for those exact rules. - """ - if parent_id is not None and parent_id not in self.nodes: - raise SceneError(f"unknown parent node: {parent_id}") - node_id = f"n{next(self._counter)}" - # Mirrors DocumentNode.__init__'s `(attachment_kind or - # "document").lower()` normalization - the attachment_kind param has - # no default in this signature (per spec), but an empty/None value - # still needs to fall back to "document" and casing still needs to - # normalize, since "audio" vs "Audio" is a real behavioral branch - # (metadata "Type" row, preview label, badge text all key off it). - normalized_kind = str(attachment_kind or "document").lower() - node = SceneNode( - id=node_id, - x=float(x), - y=float(y), - title=str(title), - kind="document", - content=str(content), - state=DocumentState( - attachment_kind=normalized_kind, - file_path=str(file_path), - mime_type=str(mime_type), - duration_seconds=duration_seconds, - byte_size=byte_size, - preview_label=str(preview_label), - ), - ) - self.nodes[node_id] = node - if parent_id is not None: - self.connect(parent_id, node_id) - return node - - def add_thinking_node( - self, - x: float, - y: float, - thinking_text: str, - parent_id: str, - ) -> SceneNode: - """R3.13's ThinkingNode equivalent of add_chat_node/add_code_node/ - add_document_node: a real reasoning-panel node. Same as - add_document_node (and unlike chat/code), parent_id is REQUIRED, not - optional - a ThinkingNode never exists unparented - so this - unconditionally connects to its parent, no `if parent_id` guard. - - Thinking text reuses the existing `content` field rather than a new - one - there is no separate thinking-text field. `is_docked` defaults - to False: a freshly-created thinking node is never pre-docked: dock() - is only ever invoked by explicit user action or on session-load - restore, never at construction time. - - Thinking nodes are also NOT branch points (same as code/document): - there is no delete_thinking_node; deletion goes entirely through the - existing generic remove_nodes. - """ - if parent_id is not None and parent_id not in self.nodes: - raise SceneError(f"unknown parent node: {parent_id}") - node_id = f"n{next(self._counter)}" - title = str(thinking_text)[:THINKING_TITLE_PREVIEW_LENGTH] or "Thinking" - node = SceneNode( - id=node_id, - x=float(x), - y=float(y), - title=title, - kind="thinking", - content=str(thinking_text), - ) - self.nodes[node_id] = node - if parent_id is not None: - self.connect(parent_id, node_id) - return node - - def add_html_node( - self, - x: float, - y: float, - html_content: str, - parent_id: str, - ) -> SceneNode: - """R3.17's HtmlViewNode equivalent of add_document_node/ - add_thinking_node: a real raw-HTML-source node. Same as - add_document_node/add_thinking_node (and unlike chat/code), parent_id - is REQUIRED, not optional - an HtmlViewNode never exists unparented - - so this unconditionally connects to its parent, no `if parent_id` - guard. - - The raw HTML source reuses the existing `content` field rather than a - new one - there is no separate html-content field, same reuse pattern - as R3.5's code text and R3.13's thinking text. The backend stores it - VERBATIM as an opaque string: it never parses, sanitizes, validates, - or otherwise interprets the HTML - that is the frontend's job (the - preview render is a 100% client-side action that never round-trips - here). - - Html nodes are also NOT branch points (same as code/document/ - thinking): there is no delete_html_node; deletion goes entirely - through the existing generic remove_nodes. - """ - if parent_id is not None and parent_id not in self.nodes: - raise SceneError(f"unknown parent node: {parent_id}") - node_id = f"n{next(self._counter)}" - title = str(html_content)[:HTML_TITLE_PREVIEW_LENGTH] or "HTML" - node = SceneNode( - id=node_id, - x=float(x), - y=float(y), - title=title, - kind="html", - content=str(html_content), - state=HtmlState(), - ) - self.nodes[node_id] = node - if parent_id is not None: - self.connect(parent_id, node_id) - return node - - def set_html_splitter_state(self, node_id: str, value: float) -> None: - """R6.3: persists an HtmlViewNode's draggable code/preview splitter - position. html kind only (SceneError otherwise), matching every - other kind-specific setter's guard pattern in this file (e.g. - resize_chart/toggle_frame_lock).""" - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") - if node.kind != "html": - raise SceneError(f"node is not an html node: {node_id}") - node.state.html_splitter_state = float(value) - - def add_image_node( - self, - x: float, - y: float, - image_bytes: bytes, - prompt: str, - parent_id: str, - *, - mime_type: str = "image/png", - ) -> SceneNode: - """R3.21's image-node equivalent of add_document_node/ - add_thinking_node/add_html_node: a real generated-image node. Same as - document/thinking/html (and unlike chat/code), parent_id is - REQUIRED, not optional - an image node never exists unparented - so - this unconditionally connects to its parent, no `if parent_id` guard. - - Image bytes do NOT live on SceneNode (see the transport-decision - comment on SceneDocument.image_assets) - they go into that - session-scoped store, keyed by a SEPARATE id. Unlike node/edge ids - (which only need to be unique within their own SceneDocument, since - nothing ever looks a node up across sessions), asset ids are read - back through GET /api/assets/{id}, a route that takes a bare id plus - an independent session query param - so a per-document counter here - would let two sessions mint the identical "imgN" id for unrelated - images (guaranteed, not just probabilistic, for sessions that create - nodes in the same order), and a caller that omits/mis-supplies the - session param would silently be served someone else's image instead - of a 404. A uuid4 hex keeps the id globally unique so cross-session - collision is not possible regardless of session query correctness. - image_asset_id on the node is just the opaque reference key into - that store. - - There is no natural title-preview text for an image the way there is - for text-based kinds, so the title is the prompt (truncated, same - 60-char convention as chat/thinking/html) when non-empty, else a - literal "Image". - - Image nodes are also NOT branch points (same as code/document/ - thinking/html): there is no delete_image_node; deletion goes - entirely through the existing generic remove_nodes, which - additionally evicts this node's image_assets entry so bytes never - outlive the node (see remove_nodes). - """ - if parent_id is not None and parent_id not in self.nodes: - raise SceneError(f"unknown parent node: {parent_id}") - node_id = f"n{next(self._counter)}" - asset_id = f"img{uuid.uuid4().hex}" - self.image_assets[asset_id] = (image_bytes, mime_type) - title = str(prompt)[:IMAGE_TITLE_PREVIEW_LENGTH] or "Image" - node = SceneNode( - id=node_id, - x=float(x), - y=float(y), - title=title, - kind="image", - content=str(prompt), - state=ImageState(image_asset_id=asset_id), - ) - self.nodes[node_id] = node - if parent_id is not None: - self.connect(parent_id, node_id) - return node - - def get_image_asset(self, asset_id: str) -> tuple[bytes, str] | None: - """The read-side of image_assets - the same lookup backend/assets.py's - GET /api/assets/{id} route calls to serve the raw bytes + mime type.""" - return self.image_assets.get(asset_id) - - def add_conversation_node(self, x: float, y: float, parent_id: str | None) -> SceneNode: - """R3.25's ConversationNode equivalent of add_document_node/ - add_thinking_node/add_html_node/add_image_node: a real multi-message - conversation node. Same as document/thinking/html/image (and unlike - chat/code), parent_id is REQUIRED, not optional - a ConversationNode - never exists unparented - so this unconditionally connects to its - parent, no `if parent_id` guard. - - Title is always the fixed literal "Conversation" - never derived or - truncated from any content, unlike every scalar-content kind before - it (chat/thinking/html/image all preview their own text). There is - no natural single preview string for a node whose content is a - growing LIST of messages, so the title never changes as messages are - appended (see append_conversation_user_message/ - append_conversation_assistant_message below - neither touches - title). Mirrors graphlink_conversation_node.py's `title_label = - QLabel("Conversation")`, a hardcoded literal, not derived state. - - `history` starts empty - a freshly-created conversation node has no - messages yet, same posture as `is_docked` defaulting False on a - freshly-created thinking node. - - Conversation nodes are also NOT branch points (same as code/document/ - thinking/html/image): there is no delete_conversation_node; deletion - goes entirely through the existing generic remove_nodes. - """ - if parent_id is not None and parent_id not in self.nodes: - raise SceneError(f"unknown parent node: {parent_id}") - node_id = f"n{next(self._counter)}" - node = SceneNode( - id=node_id, - x=float(x), - y=float(y), - title="Conversation", - kind="conversation", - ) - self.nodes[node_id] = node - if parent_id is not None: - self.connect(parent_id, node_id) - return node - - def append_conversation_user_message(self, node_id: str, text: str) -> SceneNode: - """Append a real user message to a conversation node's history - - mirrors graphlink_conversation_node.py's add_user_message, minus the - view-layer bubble creation (the frontend's job).""" - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") - node.history.append({"role": "user", "content": str(text)}) - return node - - def append_conversation_assistant_message( - self, node_id: str, text: str, incomplete: bool = False - ) -> SceneNode: - """Append a real assistant message to a conversation node's history - - mirrors graphlink_conversation_node.py's add_ai_message, minus the - view-layer bubble creation. - - ADR-006 stage 6.4: `incomplete=True` marks a PARTIAL reply whose - stream died mid-generation (H5 - the accumulated text is preserved - instead of lost). The key is only written when set - completed - messages keep their exact two-key {role, content} shape, so every - existing history consumer (session round-trip, agent context - assembly) sees byte-identical data for the normal path.""" - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") - message: dict[str, Any] = {"role": "assistant", "content": str(text)} - if incomplete: - message["incomplete"] = True - node.history.append(message) - return node - - def delete_conversation_message(self, node_id: str, message_index: int) -> None: - """Prune one message out of a conversation node's history by index - - mirrors graphlink_conversation_node.py's _remove_message's index- - synced pop, minus the view-layer bubble removal/re-layout (the - frontend's job).""" - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") - if message_index < 0 or message_index >= len(node.history): - raise SceneError(f"message index out of range: {message_index}") - node.history.pop(message_index) - - def send_conversation_message(self, node_id: str, text: str) -> SceneNode: - """The Conversation node's own Send action (R3.25): a thin wrapper - over append_conversation_user_message, kept as a separate method - (rather than only calling append_conversation_user_message directly - from the WS wrapper) so the WS intent name lines up 1:1 with the - domain method, the same way sendMessage/send_message already do for - ChatNode.""" - return self.append_conversation_user_message(node_id, text) - - # -- R5.1: web research node --------------------------------------------- - - def add_web_research_node(self, x: float, y: float, parent_id: str | None) -> SceneNode: - """The Web Research node's creation primitive - same required-parent - posture as document/thinking/html/image/conversation nodes (never - exists unparented). Title is always the fixed literal "Web Research" - (mirrors conversation node's own fixed "Conversation" title - there - is no meaningful single preview string before a query has ever been - run). Content starts empty; the query text only lands once - start_web_research_run is called.""" - if parent_id is not None and parent_id not in self.nodes: - raise SceneError(f"unknown parent node: {parent_id}") - node_id = f"n{next(self._counter)}" - node = SceneNode( - id=node_id, - x=float(x), - y=float(y), - title="Web Research", - kind="web_research", - state=WebResearchState(), - ) - self.nodes[node_id] = node - if parent_id is not None: - self.connect(parent_id, node_id) - return node - - def start_web_research_run(self, node_id: str, query: str) -> SceneNode: - """Begin one research run: stores the query text and resets this - run's progress fields. Deliberately does NOT clear research_result - - stale-while-revalidate: the previous run's answer stays visible until - this run replaces it on success, or fails/cancels (leaving the stale - result annotated by the new research_error).""" - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") - if node.kind != "web_research": - raise SceneError(f"node is not a web_research node: {node_id}") - node.content = str(query) - node.state.research_stage = "" - node.state.research_completed = 0 - node.state.research_total = 0 - node.state.research_active_source_id = None - node.state.research_error = "" - return node - - def apply_web_research_progress(self, node_id: str, event) -> SceneNode | None: - """Apply one duck-typed ProgressEvent-shaped update (.stage/.completed/ - .total/.source_id) - canvas.py deliberately does NOT import anything - from graphlink_plugins.web_research (mirrors how - start_conversation_reply's node param is duck-typed without - agents.py importing backend.canvas.SceneNode). Silent no-op (returns - None, never raises) if node_id is no longer in self.nodes - the node - may have been deleted while a background run was still in flight.""" - node = self.nodes.get(node_id) - if node is None: - return None - node.state.research_stage = event.stage.value - node.state.research_completed = event.completed - node.state.research_total = event.total - node.state.research_active_source_id = event.source_id - return node - - def complete_web_research_run(self, node_id: str, result_wire: dict) -> SceneNode: - """Land a successful run's result. Raises SceneError if the node is - gone - the WS wrapper's own liveness check (in register_canvas) - guards the actual mid-flight-delete race; this stays a hard - precondition here, same posture as update_chat_node_content.""" - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") - node.state.research_stage = "completed" - node.state.research_error = "" - node.state.research_active_source_id = None - node.state.research_result = result_wire - return node - - def fail_web_research_run(self, node_id: str, *, cancelled: bool, message: str) -> SceneNode: - """Land a failed or cancelled run. research_result is deliberately - left untouched (stale-while-revalidate - see start_web_research_run's - own docstring).""" - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") - node.state.research_stage = "cancelled" if cancelled else "failed" - node.state.research_error = message - node.state.research_active_source_id = None - return node - - # -- R5.2: artifact/drafter node ----------------------------------------- - - def add_artifact_node(self, x: float, y: float, parent_id: str | None) -> SceneNode: - """The Artifact/Drafter node's creation primitive - same required- - parent posture as document/thinking/html/image/conversation/ - web_research nodes (never exists unparented). Title is always the - fixed literal "Artifact" (mirrors conversation/web_research's own - fixed titles - there is no meaningful single preview string before a - document has ever been drafted). artifact_content starts empty; the - document text only lands once complete_artifact_generation is - called.""" - if parent_id is not None and parent_id not in self.nodes: - raise SceneError(f"unknown parent node: {parent_id}") - node_id = f"n{next(self._counter)}" - node = SceneNode( - id=node_id, - x=float(x), - y=float(y), - title="Artifact", - kind="artifact", - state=ArtifactState(), - ) - self.nodes[node_id] = node - if parent_id is not None: - self.connect(parent_id, node_id) - return node - - def append_artifact_user_message(self, node_id: str, text: str) -> SceneNode: - """Append a real user instruction to an artifact node's history - - mirrors append_conversation_user_message exactly (same shape, same - error-handling style).""" - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") - node.history.append({"role": "user", "content": str(text)}) - # A new instruction supersedes the previous failure - leaving the - # banner up beside an in-flight run would report a stale outcome. - node.state.artifact_error = "" - return node - - def fail_artifact_generation(self, node_id: str, message: str) -> SceneNode | None: - """Record a generation failure ON the node, so the card can say what - went wrong instead of leaving a session-wide toast to be matched to - one of several artifact nodes by guesswork. - - Returns None for a node that no longer exists rather than raising: - this is called from the dispatch task's own except/timeout paths, - where the node may well have been deleted mid-flight, and a failure - report is not worth turning into a second failure.""" - node = self.nodes.get(node_id) - if node is None: - return None - node.state.artifact_error = str(message) - return node - - def send_artifact_message(self, node_id: str, text: str) -> SceneNode: - """The Artifact node's own Send action: a thin wrapper over - append_artifact_user_message, kept as a separate method (rather than - only calling append_artifact_user_message directly from the WS - wrapper) so the WS intent name lines up 1:1 with the domain method, - the same way send_conversation_message/append_conversation_user_message - already do for ConversationNode.""" - return self.append_artifact_user_message(node_id, text) - - def complete_artifact_generation(self, node_id: str, new_content, ai_message: str) -> SceneNode: - """Land a successful generation turn: WHOLE-DOCUMENT REPLACE (never an - append/merge - the model returns the entire document every turn, see - ArtifactState's own comment, backend/domain/node_states.py), plus - append a real assistant turn to history. Raises SceneError if the node is - gone - this WS wrapper does NOT pre-check liveness before calling - this, same posture as send_conversation_message's own _on_reply, not - web_research's more defensive pre-check pattern (there is no - stage-stepper/persisted-error field here for a mid-flight delete to - race against).""" - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") - node.state.artifact_content = str(new_content) - node.state.artifact_error = "" - node.history.append({"role": "assistant", "content": str(ai_message)}) - return node - - # -- R5.3: gitlink node -------------------------------------------------- - # - # canvas.py imports NOTHING from graphlink_plugins.gitlink - every method - # below is pure state mutation on plain fields, matching how - # apply_web_research_progress already does duck-typed mutation without - # importing the domain package. The fingerprint mechanism itself - # (_fingerprint_changes) lives in backend/agents.py, which DOES import - # from graphlink_plugins.gitlink - same precedent as ArtifactAgent/ - # web_research.domain already being imported there, not here. - - - - - - - - - - - - - # -- Review Lens node ------------------------------------------------------ - # - # Same import posture as every other plugin-backed kind's domain methods: - # canvas.py imports NOTHING from graphlink_plugins.review_lens - every - # method below takes plain values (already fetched/normalized by the - # dispatch layer) and only stores them. - - - - - - - - - - - # -- R5.4: Execution Sandbox node ------------------------------------------ - # - # Same import posture as every other plugin-backed kind's domain methods: - # canvas.py imports NOTHING from graphlink_plugins.code_sandbox. - - def add_code_sandbox_node(self, x: float, y: float, parent_id: str | None) -> SceneNode: - """The Virtual Environment Runner node's creation primitive - same - required-parent posture as every R5 sibling. Title is always the - fixed literal "Virtual Environment Runner" (matches - backend/plugins.py's own plugin display name - renamed under - ADR-002 P0 from "Execution Sandbox", which oversold what is - actually a plain OS subprocess running inside a venv, not an - OS-level sandbox; the internal kind="code_sandbox" identifier is - UNCHANGED, since it's persisted wire/save-format state, not a - display string). code_sandbox_sandbox_id is minted here, ONCE, at - creation time - a short uuid4 hex used purely as this node's - sandbox directory name (VirtualEnvSandbox re-sanitizes it again on - its own side, but a short, already-safe id keeps the on-disk path - short and human-scannable).""" - if parent_id is not None and parent_id not in self.nodes: - raise SceneError(f"unknown parent node: {parent_id}") - node_id = f"n{next(self._counter)}" - node = SceneNode( - id=node_id, - x=float(x), - y=float(y), - title="Virtual Environment Runner", - kind="code_sandbox", - state=CodeSandboxState(code_sandbox_sandbox_id=uuid.uuid4().hex[:12]), - ) - self.nodes[node_id] = node - if parent_id is not None: - self.connect(parent_id, node_id) - return node - - def set_web_research_retain_to_knowledge(self, node_id: str, retain: bool) -> SceneNode: - """ADR-021 stage 21.5: the per-node "keep these sources" preference. - Same shape as set_code_sandbox_requirements below - validate the - node exists and is the right kind, then write one state field.""" - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") - if node.kind != "web_research": - raise SceneError(f"node is not a web_research node: {node_id}") - node.state.research_retain_to_knowledge = bool(retain) - return node - - def set_code_sandbox_requirements(self, node_id: str, requirements_text: str) -> SceneNode: - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") - if node.kind != "code_sandbox": - raise SceneError(f"node is not a code_sandbox node: {node_id}") - node.state.code_sandbox_requirements = str(requirements_text) - return node - - def set_code_sandbox_allow_source_builds(self, node_id: str, allow: bool) -> SceneNode: - """ADR-005 stage 5.5: the approval panel's own source-build opt-in - checkbox, fired on every toggle (same "ungated, fires immediately" - posture as set_code_sandbox_requirements above). Setting this outside - an open approval gate is harmless, not just permitted - agents.py - resets the field to False at the top of every gate-open, so a value - set here while no gate is open never reaches an actual run; the - approval panel is the only surface that ever renders this control, - and it only renders while awaiting_approval is true.""" - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") - if node.kind != "code_sandbox": - raise SceneError(f"node is not a code_sandbox node: {node_id}") - node.state.code_sandbox_approval_allow_source_builds = bool(allow) - return node - - def start_code_sandbox_run(self, node_id: str, input_text: str) -> SceneNode: - """Begin one Run: stores input_text into code_sandbox_prompt and - clears any previous error. Deliberately does NOT touch - code_sandbox_code here - the dispatch - method decides generate-vs-reuse by reading the EXISTING - code_sandbox_code value at call time, so this must not overwrite it - before that decision is made.""" - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") - if node.kind != "code_sandbox": - raise SceneError(f"node is not a code_sandbox node: {node_id}") - node.state.code_sandbox_prompt = str(input_text) - node.state.code_sandbox_error = "" - return node - - def complete_code_sandbox_run(self, node_id: str, code: str, output: str, analysis: str) -> SceneNode | None: - """Land a successful run. Execution Sandbox has no last_run_failed - flag: an unrecovered failure after exhausting its own repair - attempts surfaces as a failed run (see - AgentDispatcher.start_code_sandbox_run), never as a "succeeded but - flagged" result. Not kind-guarded: only ever reached via run_code_ - sandbox's own on_success closure (backend/api/ - intents_code_sandbox.py), whose node_id was already validated by - start_code_sandbox_run's own guard earlier in the same request - - same posture as complete_gitlink_run.""" - node = self.nodes.get(node_id) - if node is None: - return None - node.state.code_sandbox_code = str(code) - node.state.code_sandbox_output = str(output) - node.state.code_sandbox_analysis = str(analysis) - node.state.code_sandbox_awaiting_approval = False - node.state.code_sandbox_approval_requirements = "" - node.state.code_sandbox_approved_fingerprint = None - node.state.code_sandbox_approval_allow_source_builds = False - node.state.code_sandbox_approval_is_repair = False - node.state.code_sandbox_error = "" - return node - - def fail_code_sandbox_run(self, node_id: str, message: str) -> SceneNode | None: - """Land a failed (or denied-approval, or cancelled) run - the - awaiting_approval flag is ALWAYS cleared here too, unconditionally, - so a denied/cancelled approval never leaves the node stuck showing - the approval prompt forever (stale-while-revalidate: existing - code/output/analysis survive untouched). Not kind-guarded - see - complete_code_sandbox_run's own comment.""" - node = self.nodes.get(node_id) - if node is None: - return None - node.state.code_sandbox_awaiting_approval = False - node.state.code_sandbox_approval_requirements = "" - node.state.code_sandbox_approved_fingerprint = None - node.state.code_sandbox_approval_allow_source_builds = False - node.state.code_sandbox_approval_is_repair = False - node.state.code_sandbox_error = str(message) - return node - - # -- R6.1: Notes/Frames/Containers ---------------------------------------- - # - # Legacy canvas decorations, ported here for the first time (never - # covered by any prior increment). Notes are free-floating markdown - # sticky-notes with no parent-required posture (unlike almost every R3+ - # content kind). Frames/containers are "group" nodes: they never contain - # their members via any React Flow parent/extent mechanism - membership - # is plain data (item_ids) and enclosure is plain server-side math - # (_recompute_group_bounds below), matching the legacy behavior of - # always auto-growing to enclose members, never clipping them. - - def add_note( - self, - x: float, - y: float, - *, - is_system_prompt: bool = False, - is_summary_note: bool = False, - ) -> SceneNode: - """A note's creation primitive. UNLIKE every R3+ content kind, no - parent is required or accepted - notes are free-floating, never - branch-point children (mirrors the legacy note widget, which the - canvas places directly, not via a ChatNode-anchored connection).""" - node_id = f"n{next(self._counter)}" - node = SceneNode( - id=node_id, - x=float(x), - y=float(y), - title="Note", - kind="note", - content="Add note...", - state=NoteState( - is_system_prompt=bool(is_system_prompt), - is_summary_note=bool(is_summary_note), - ), - ) - self.nodes[node_id] = node - return node - - def set_note_content(self, node_id: str, content: str) -> None: - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") - node.content = str(content) - - # -- ADR-008 stage 8.3: plan node (the Builder's checklist) -------------- - - def add_plan_node( - self, - x: float, - y: float, - goal: str, - *, - mode: str = "copilot", - max_steps: int = 12, - max_tokens: int = 150_000, - max_wall_seconds: int = 900, - ) -> SceneNode: - """The Builder plan node's creation primitive. Free-floating like a - note (a build STARTS from a goal, it does not continue an existing - branch - the nodes the build creates are the ones that connect); - `content` reuses the goal text the same way web_research reuses - content for its query. Everything else lives on PlanState - see its - own docstring for the state machine and the plan-node-as-resume- - point contract.""" - if mode not in ("copilot", "autopilot"): - raise SceneError(f"unknown builder mode: {mode}") - node_id = f"n{next(self._counter)}" - node = SceneNode( - id=node_id, - x=float(x), - y=float(y), - title=f"Build: {str(goal)[:CHAT_TITLE_PREVIEW_LENGTH]}" if goal else "Build", - kind="plan", - content=str(goal), - state=PlanState( - plan_goal=str(goal), - builder_mode=mode, - builder_max_steps=int(max_steps), - builder_max_tokens=int(max_tokens), - builder_max_wall_seconds=int(max_wall_seconds), - ), - ) - self.nodes[node_id] = node - return node - - # -- PLAN-2026-08-24 H1: harness node (the workspace agent) -------------- - - def add_harness_node(self, x: float, y: float, goal: str, *, max_turns: int = 16) -> SceneNode: - """The harness node's creation primitive. Free-floating like a plan - node (a task starts from a prompt, it does not continue an existing - branch); harness_workspace_id is minted here, ONCE - the same - code_sandbox_sandbox_id precedent, see HarnessState's own docstring - for why node.id is not durable enough to name the on-disk - workspace.""" - node_id = f"n{next(self._counter)}" - node = SceneNode( - id=node_id, - x=float(x), - y=float(y), - title=f"Agent: {str(goal)[:CHAT_TITLE_PREVIEW_LENGTH]}" if goal else "Agent", - kind="harness", - content=str(goal), - state=HarnessState( - harness_goal=str(goal), - harness_workspace_id=uuid.uuid4().hex[:12], - harness_max_turns=int(max_turns), - ), - ) - self.nodes[node_id] = node - return node - - _PLAN_STEP_STATUSES = ("pending", "running", "done", "failed", "skipped") - - def set_plan_steps(self, node_id: str, steps: list) -> SceneNode: - """Replaces the plan's step list - the one plan mutator that goes - through record_command (a user editing the checklist, or the - model's replan tool): step CONTENT is document state a Ctrl+Z must - reach, unlike the run-lifecycle fields (builder_status/spent_*/ - awaiting_*) which the loop writes directly, exactly as Execution - Sandbox's own run pipeline writes its awaiting/progress fields. - - Steps whose status is not "pending" are immutable history - a - replacement must carry every non-pending step through unchanged - (same id, title, status), enforced here so neither a user edit nor - a model replan can rewrite what already happened.""" - node = self.nodes.get(node_id) - if node is None or not isinstance(node.state, PlanState): - raise SceneError(f"not a plan node: {node_id}") - normalized: list[dict[str, Any]] = [] - seen_ids: set[str] = set() - for raw in steps: - if not isinstance(raw, dict): - raise SceneError("each step must be an object") - step_id = str(raw.get("id") or f"s{len(normalized) + 1}") - if step_id in seen_ids: - raise SceneError(f"duplicate step id: {step_id}") - seen_ids.add(step_id) - status = str(raw.get("status") or "pending") - if status not in self._PLAN_STEP_STATUSES: - raise SceneError(f"unknown step status: {status}") - title = str(raw.get("title") or "").strip() - if not title: - raise SceneError("each step needs a title") - normalized.append({ - "id": step_id, "title": title, "status": status, - "detail": str(raw.get("detail") or ""), - }) - frozen = {s["id"]: s for s in node.state.plan_steps if s.get("status") != "pending"} - for step_id, original in frozen.items(): - replacement = next((s for s in normalized if s["id"] == step_id), None) - if replacement is None: - raise SceneError( - f"step {step_id!r} has already run ({original['status']}) and cannot be removed" - ) - if replacement["title"] != original["title"] or replacement["status"] != original["status"]: - raise SceneError( - f"step {step_id!r} has already run ({original['status']}) and cannot be rewritten" - ) - node.state.plan_steps = normalized - return node - - # -- R6.2: chart node ---------------------------------------------------- - - def add_chart_node( - self, - x: float, - y: float, - parent_id: str | None, - chart_type: str, - chart_data: dict[str, Any], - *, - chart_error: str = "", - ) -> SceneNode: - """The Chart node's creation primitive - same required-parent - posture as every other branch-point-child kind (web_research/ - artifact/gitlink/code_sandbox above) for every NEW chart: - the UI-driven generateChart intent always passes a real parent_id, - since a chart is always generated FROM some other node's content in - that flow. chart_type MUST be one of SUPPORTED_CHART_TYPES - (SceneError otherwise, same "validate up front, never construct a - half-invalid node" posture create_frame/create_container use for - their own item_ids checks). - - R6.4: parent_id is None-able for the session LOADER only - legacy - genuinely allows a chart with no parent at all (both - parent_node_index/parent_node_id absent in the persisted payload is - a real, valid legacy state, confirmed by recon), which the original - required-parent signature could not represent. When parent_id is - None, no parent-existence check runs and no edge is created - - chart_source_node_id stays "" rather than getting a real node id. - - chart_data is assumed ALREADY canonicalized by the CALLER - this - method deliberately does NOT call canonicalize_chart_data itself - (see chart_data's own field comment on SceneNode for the full - reasoning: the WS-intent wrapper needs to be able to catch - ChartDataError itself and still create a placeholder chart with - chart_error set, rather than have creation abort entirely). - - Title mirrors legacy ChartItem's own `self.title = str(self.data. - get("title") or "Chart")` - the chart's own title field if present, - else the literal "Chart" (not a chart-type-specific default; that is - genuinely what legacy does). - - ADR-013 stage 13.4: no longer renders a PNG here - the client-side - interactive renderer (stage 13.2) draws straight from chart_data, - and nothing has consumed the backend-rendered display asset since. - A chart's ONLY remaining matplotlib render is the export/copy - endpoint (backend/assets.py), a fresh re-render on every request.""" - if parent_id is not None and parent_id not in self.nodes: - raise SceneError(f"unknown parent node: {parent_id}") - normalized_type = str(chart_type or "").strip().lower() - if normalized_type not in SUPPORTED_CHART_TYPES: - raise SceneError(f"unsupported chart type: {chart_type}") - - node_id = f"n{next(self._counter)}" - safe_chart_data = dict(chart_data) if isinstance(chart_data, dict) else {} - title = str(safe_chart_data.get("title") or "Chart") - node = SceneNode( - id=node_id, - x=float(x), - y=float(y), - title=title, - kind="chart", - state=ChartState( - chart_type=normalized_type, - chart_data=safe_chart_data, - chart_error=str(chart_error), - chart_source_node_id=parent_id or "", - ), - ) - - self.nodes[node_id] = node - if parent_id is not None: - self.connect(parent_id, node_id) - return node - - def resize_chart(self, node_id: str, width: float, height: float) -> None: - """Chart kind only (SceneError otherwise). Clamps (width, height) - into [CHART_MIN_WIDTH, CHART_MAX_WIDTH] / [CHART_MIN_HEIGHT, - CHART_MAX_HEIGHT]. If chart_aspect_locked, preserves the aspect - ratio of the REQUESTED (width, height) pair AS SENT - the frontend/ - NodeResizer is responsible for computing a ratio-correct pair before - ever calling this; UNLIKE legacy ChartItem._clamp_size (which - consults self.resize_start_aspect_ratio, a value frozen at drag - START), this method has no concept of an in-progress gesture, so it - only ever has the two numbers it was given to work from. After the - plain min/max clamp, if aspect-locked, re-derives whichever - dimension keeps the REQUESTED ratio relative to the (already- - clamped) other dimension - same "pick whichever correction moves the - clamped pair least" algorithm legacy's own _clamp_size uses - then - re-clamps once more, so the final stored size never violates either - the lock or the min/max bounds even after that re-derivation. - - ADR-013 stage 13.4: no longer re-renders a PNG here - see - add_chart_node's own docstring for why.""" - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") - if node.kind != "chart": - raise SceneError(f"node is not a chart node: {node_id}") - - requested_width = float(width) - requested_height = float(height) - clamped_width = min(CHART_MAX_WIDTH, max(CHART_MIN_WIDTH, requested_width)) - clamped_height = min(CHART_MAX_HEIGHT, max(CHART_MIN_HEIGHT, requested_height)) - - if node.state.chart_aspect_locked and requested_width > 0 and requested_height > 0: - aspect_ratio = requested_width / requested_height - width_from_height = clamped_height * aspect_ratio - height_from_width = clamped_width / aspect_ratio - if abs(width_from_height - clamped_width) < abs(height_from_width - clamped_height): - clamped_width = width_from_height - clamped_height = clamped_width / aspect_ratio - else: - clamped_height = height_from_width - clamped_width = clamped_height * aspect_ratio - # Re-deriving one dimension from the other can overshoot the - # opposite bound for an extreme aspect ratio - one more clamp - # keeps the final pair inside both bounds unconditionally. - clamped_width = min(CHART_MAX_WIDTH, max(CHART_MIN_WIDTH, clamped_width)) - clamped_height = min(CHART_MAX_HEIGHT, max(CHART_MIN_HEIGHT, clamped_height)) - - node.state.chart_width = clamped_width - node.state.chart_height = clamped_height - - def toggle_chart_aspect_lock(self, node_id: str) -> None: - """Chart kind only (SceneError otherwise). Flips chart_aspect_locked.""" - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") - if node.kind != "chart": - raise SceneError(f"node is not a chart node: {node_id}") - node.state.chart_aspect_locked = not node.state.chart_aspect_locked - - def update_chat_node_content( - self, node_id: str, content: str, incomplete: bool = False - ) -> SceneNode: - """The regenerate primitive: mutate an EXISTING chat node's content in - place - the first in-place mutation of a content-bearing field in this - file (move_node/set_chat_collapsed/set_node_docked all mutate a - position/flag, never displayed text). Scope confirmed against legacy's - ChatNode.update_content (graphlink_nodes/graphlink_node_chat.py:677-686): - sets content ONLY. Does not touch title (legacy's update_content never - recomputes any title-like state either, and every other in-place mutator - here already leaves title untouched post-creation - consistent, not a - new carve-out). Does not touch is_user/is_collapsed/kind. - - ADR-006 stage 6.4: `incomplete` marks a PARTIAL reply committed after - its stream died (see ChatState.response_incomplete). A normal full - regenerate passes the default False, which doubles as the CLEAR for a - previously interrupted node - retry succeeds, banner goes away.""" - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") - node.content = str(content) - if isinstance(node.state, ChatState): - node.state.response_incomplete = bool(incomplete) - return node - - def add_generated_image_reply( - self, - parent_chat_node_id: str, - prompt: str, - image_bytes: bytes, - mime_type: str = "image/png", - ) -> tuple[SceneNode, SceneNode]: - """The Generate/Regenerate Image success primitive (R4.4a) - mirrors - legacy's handle_image_response exactly: unconditionally creates a NEW - assistant ChatNode (content=f'Generated image for prompt: "{prompt}"', - is_user=False, parent_id=parent_chat_node_id) then a NEW ImageNode - (content=prompt, parent_id=) - built entirely - from the existing add_chat_node/add_image_node primitives, zero new - mutation-in-place logic, matching this feature's create-new-nodes - scope decision. Positions via place_child (backend/domain/layout.py), - the same collision-resolved placement send_message/regenerate_ - response's own new-child placement uses. last_chat_node_id is DELIBERATELY untouched - - mirrors legacy: handle_image_response never assigns - self.current_node either, since image generation is side content, - not a branch-continuation point (same posture as - regenerate_response's own documented "last_chat_node_id: - DELIBERATELY untouched"). Raises SceneError if parent_chat_node_id is - unknown - defensive: a delete could race the in-flight generation - request (see the mid-flight-delete handling in the WS wrapper in - register_canvas).""" - parent = self.nodes.get(parent_chat_node_id) - if parent is None: - raise SceneError(f"unknown parent node: {parent_chat_node_id}") - ax, ay = self.place_child(parent_chat_node_id, "chat") - chat_node = self.add_chat_node( - ax, ay, f'Generated image for prompt: "{prompt}"', False, parent_id=parent_chat_node_id, - ) - ix, iy = self.place_child(chat_node.id, "image") - image_node = self.add_image_node(ix, iy, image_bytes, prompt, chat_node.id, mime_type=mime_type) - return chat_node, image_node - - def set_chat_collapsed(self, node_id: str, collapsed: bool) -> None: - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") - node.is_collapsed = bool(collapsed) - # R6.1: unlike every other kind this generic setter already served, - # a frame/container's is_collapsed also drives derived geometry - # (the collapsed pill size vs the auto-fit/manual bbox) - recompute - # here too so this stays correct no matter which entry point sets - # it, rather than only being safe via toggle_group_collapsed. - if node.kind in ("frame", "container"): - self._recompute_group_bounds(node_id) - def set_all_conversational_collapsed(self, collapsed: bool) -> None: """R7.5e: Collapse All / Expand All - the bulk counterpart to set_chat_collapsed above. Mirrors legacy's @@ -1681,18 +604,6 @@ def set_all_conversational_collapsed(self, collapsed: bool) -> None: if node.kind in ("chat", "conversation", "html"): node.is_collapsed = collapsed - def set_chat_scroll_value(self, node_id: str, value: float) -> None: - """R6.3: persists a chat node's own scroll position within its - content area. chat kind only (SceneError otherwise), matching every - other kind-specific setter's guard pattern in this file (e.g. - resize_chart/toggle_frame_lock).""" - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") - if node.kind != "chat": - raise SceneError(f"node is not a chat node: {node_id}") - node.state.chat_scroll_value = float(value) - def set_node_docked(self, node_id: str, docked: bool) -> None: """R3.13: a single generic setter handling both dock (docked=True) and undock (docked=False) - mirrors set_chat_collapsed's generic- diff --git a/backend/domain/nodes_agent_runs.py b/backend/domain/nodes_agent_runs.py new file mode 100644 index 0000000..33055a0 --- /dev/null +++ b/backend/domain/nodes_agent_runs.py @@ -0,0 +1,338 @@ +"""AgentRunOps - the SceneDocument methods for the three node kinds that +run an agent in the background (web research, artifact, code sandbox). + +A MIXIN, composed exactly once, by backend/domain/graph.py's +SceneDocument. Method bodies are relocated VERBATIM from graph.py; +only the class wrapper, its docstring and the imports are new, and the +methods are regrouped by kind rather than left in the order successive +increments happened to append them in. + +See backend/domain/nodes_code_review.py's docstring for why the +per-kind method groups are being lifted out of SceneDocument at all. +""" + +from __future__ import annotations + +import uuid + +from backend.domain._composed import SceneDocumentParts +from backend.domain.model import SceneError, SceneNode +from backend.domain.node_states import ( + ArtifactState, + CodeSandboxState, + WebResearchState, +) + + +class AgentRunOps(SceneDocumentParts): + """Every SceneDocument method belonging to a node kind that runs an agent + in the background: web research, artifact/drafter, and the Execution + Sandbox. + + They are one group because they are one shape. Each has a creation + primitive and then the same four-beat lifecycle - start the run, take + progress, complete it, fail it - and each fails silently when its node + is already gone, because a run that outlives the node the user deleted + is not an error worth surfacing. + """ + + # -- R5.1: web research node --------------------------------------------- + + def add_web_research_node(self, x: float, y: float, parent_id: str | None) -> SceneNode: + """The Web Research node's creation primitive - same required-parent + posture as document/thinking/html/image/conversation nodes (never + exists unparented). Title is always the fixed literal "Web Research" + (mirrors conversation node's own fixed "Conversation" title - there + is no meaningful single preview string before a query has ever been + run). Content starts empty; the query text only lands once + start_web_research_run is called.""" + if parent_id is not None and parent_id not in self.nodes: + raise SceneError(f"unknown parent node: {parent_id}") + node_id = f"n{next(self._counter)}" + node = SceneNode( + id=node_id, + x=float(x), + y=float(y), + title="Web Research", + kind="web_research", + state=WebResearchState(), + ) + self.nodes[node_id] = node + if parent_id is not None: + self.connect(parent_id, node_id) + return node + + def start_web_research_run(self, node_id: str, query: str) -> SceneNode: + """Begin one research run: stores the query text and resets this + run's progress fields. Deliberately does NOT clear research_result - + stale-while-revalidate: the previous run's answer stays visible until + this run replaces it on success, or fails/cancels (leaving the stale + result annotated by the new research_error).""" + node = self.nodes.get(node_id) + if node is None: + raise SceneError(f"unknown node: {node_id}") + if node.kind != "web_research": + raise SceneError(f"node is not a web_research node: {node_id}") + node.content = str(query) + node.state.research_stage = "" + node.state.research_completed = 0 + node.state.research_total = 0 + node.state.research_active_source_id = None + node.state.research_error = "" + return node + + def apply_web_research_progress(self, node_id: str, event) -> SceneNode | None: + """Apply one duck-typed ProgressEvent-shaped update (.stage/.completed/ + .total/.source_id) - canvas.py deliberately does NOT import anything + from graphlink_plugins.web_research (mirrors how + start_conversation_reply's node param is duck-typed without + agents.py importing backend.canvas.SceneNode). Silent no-op (returns + None, never raises) if node_id is no longer in self.nodes - the node + may have been deleted while a background run was still in flight.""" + node = self.nodes.get(node_id) + if node is None: + return None + node.state.research_stage = event.stage.value + node.state.research_completed = event.completed + node.state.research_total = event.total + node.state.research_active_source_id = event.source_id + return node + + def complete_web_research_run(self, node_id: str, result_wire: dict) -> SceneNode: + """Land a successful run's result. Raises SceneError if the node is + gone - the WS wrapper's own liveness check (in register_canvas) + guards the actual mid-flight-delete race; this stays a hard + precondition here, same posture as update_chat_node_content.""" + node = self.nodes.get(node_id) + if node is None: + raise SceneError(f"unknown node: {node_id}") + node.state.research_stage = "completed" + node.state.research_error = "" + node.state.research_active_source_id = None + node.state.research_result = result_wire + return node + + def fail_web_research_run(self, node_id: str, *, cancelled: bool, message: str) -> SceneNode: + """Land a failed or cancelled run. research_result is deliberately + left untouched (stale-while-revalidate - see start_web_research_run's + own docstring).""" + node = self.nodes.get(node_id) + if node is None: + raise SceneError(f"unknown node: {node_id}") + node.state.research_stage = "cancelled" if cancelled else "failed" + node.state.research_error = message + node.state.research_active_source_id = None + return node + + def set_web_research_retain_to_knowledge(self, node_id: str, retain: bool) -> SceneNode: + """ADR-021 stage 21.5: the per-node "keep these sources" preference. + Same shape as set_code_sandbox_requirements below - validate the + node exists and is the right kind, then write one state field.""" + node = self.nodes.get(node_id) + if node is None: + raise SceneError(f"unknown node: {node_id}") + if node.kind != "web_research": + raise SceneError(f"node is not a web_research node: {node_id}") + node.state.research_retain_to_knowledge = bool(retain) + return node + + # -- R5.2: artifact/drafter node ----------------------------------------- + + def add_artifact_node(self, x: float, y: float, parent_id: str | None) -> SceneNode: + """The Artifact/Drafter node's creation primitive - same required- + parent posture as document/thinking/html/image/conversation/ + web_research nodes (never exists unparented). Title is always the + fixed literal "Artifact" (mirrors conversation/web_research's own + fixed titles - there is no meaningful single preview string before a + document has ever been drafted). artifact_content starts empty; the + document text only lands once complete_artifact_generation is + called.""" + if parent_id is not None and parent_id not in self.nodes: + raise SceneError(f"unknown parent node: {parent_id}") + node_id = f"n{next(self._counter)}" + node = SceneNode( + id=node_id, + x=float(x), + y=float(y), + title="Artifact", + kind="artifact", + state=ArtifactState(), + ) + self.nodes[node_id] = node + if parent_id is not None: + self.connect(parent_id, node_id) + return node + + def append_artifact_user_message(self, node_id: str, text: str) -> SceneNode: + """Append a real user instruction to an artifact node's history - + mirrors append_conversation_user_message exactly (same shape, same + error-handling style).""" + node = self.nodes.get(node_id) + if node is None: + raise SceneError(f"unknown node: {node_id}") + node.history.append({"role": "user", "content": str(text)}) + # A new instruction supersedes the previous failure - leaving the + # banner up beside an in-flight run would report a stale outcome. + node.state.artifact_error = "" + return node + + def fail_artifact_generation(self, node_id: str, message: str) -> SceneNode | None: + """Record a generation failure ON the node, so the card can say what + went wrong instead of leaving a session-wide toast to be matched to + one of several artifact nodes by guesswork. + + Returns None for a node that no longer exists rather than raising: + this is called from the dispatch task's own except/timeout paths, + where the node may well have been deleted mid-flight, and a failure + report is not worth turning into a second failure.""" + node = self.nodes.get(node_id) + if node is None: + return None + node.state.artifact_error = str(message) + return node + + def send_artifact_message(self, node_id: str, text: str) -> SceneNode: + """The Artifact node's own Send action: a thin wrapper over + append_artifact_user_message, kept as a separate method (rather than + only calling append_artifact_user_message directly from the WS + wrapper) so the WS intent name lines up 1:1 with the domain method, + the same way send_conversation_message/append_conversation_user_message + already do for ConversationNode.""" + return self.append_artifact_user_message(node_id, text) + + def complete_artifact_generation(self, node_id: str, new_content, ai_message: str) -> SceneNode: + """Land a successful generation turn: WHOLE-DOCUMENT REPLACE (never an + append/merge - the model returns the entire document every turn, see + ArtifactState's own comment, backend/domain/node_states.py), plus + append a real assistant turn to history. Raises SceneError if the node is + gone - this WS wrapper does NOT pre-check liveness before calling + this, same posture as send_conversation_message's own _on_reply, not + web_research's more defensive pre-check pattern (there is no + stage-stepper/persisted-error field here for a mid-flight delete to + race against).""" + node = self.nodes.get(node_id) + if node is None: + raise SceneError(f"unknown node: {node_id}") + node.state.artifact_content = str(new_content) + node.state.artifact_error = "" + node.history.append({"role": "assistant", "content": str(ai_message)}) + return node + + # -- R5.4: Execution Sandbox node ---------------------------------------- + # + # Same import posture as every other plugin-backed kind's domain methods: + # nothing here imports from graphlink_plugins.code_sandbox. + + def add_code_sandbox_node(self, x: float, y: float, parent_id: str | None) -> SceneNode: + """The Virtual Environment Runner node's creation primitive - same + required-parent posture as every R5 sibling. Title is always the + fixed literal "Virtual Environment Runner" (matches + backend/plugins.py's own plugin display name - renamed under + ADR-002 P0 from "Execution Sandbox", which oversold what is + actually a plain OS subprocess running inside a venv, not an + OS-level sandbox; the internal kind="code_sandbox" identifier is + UNCHANGED, since it's persisted wire/save-format state, not a + display string). code_sandbox_sandbox_id is minted here, ONCE, at + creation time - a short uuid4 hex used purely as this node's + sandbox directory name (VirtualEnvSandbox re-sanitizes it again on + its own side, but a short, already-safe id keeps the on-disk path + short and human-scannable).""" + if parent_id is not None and parent_id not in self.nodes: + raise SceneError(f"unknown parent node: {parent_id}") + node_id = f"n{next(self._counter)}" + node = SceneNode( + id=node_id, + x=float(x), + y=float(y), + title="Virtual Environment Runner", + kind="code_sandbox", + state=CodeSandboxState(code_sandbox_sandbox_id=uuid.uuid4().hex[:12]), + ) + self.nodes[node_id] = node + if parent_id is not None: + self.connect(parent_id, node_id) + return node + + def set_code_sandbox_requirements(self, node_id: str, requirements_text: str) -> SceneNode: + node = self.nodes.get(node_id) + if node is None: + raise SceneError(f"unknown node: {node_id}") + if node.kind != "code_sandbox": + raise SceneError(f"node is not a code_sandbox node: {node_id}") + node.state.code_sandbox_requirements = str(requirements_text) + return node + + def set_code_sandbox_allow_source_builds(self, node_id: str, allow: bool) -> SceneNode: + """ADR-005 stage 5.5: the approval panel's own source-build opt-in + checkbox, fired on every toggle (same "ungated, fires immediately" + posture as set_code_sandbox_requirements above). Setting this outside + an open approval gate is harmless, not just permitted - agents.py + resets the field to False at the top of every gate-open, so a value + set here while no gate is open never reaches an actual run; the + approval panel is the only surface that ever renders this control, + and it only renders while awaiting_approval is true.""" + node = self.nodes.get(node_id) + if node is None: + raise SceneError(f"unknown node: {node_id}") + if node.kind != "code_sandbox": + raise SceneError(f"node is not a code_sandbox node: {node_id}") + node.state.code_sandbox_approval_allow_source_builds = bool(allow) + return node + + def start_code_sandbox_run(self, node_id: str, input_text: str) -> SceneNode: + """Begin one Run: stores input_text into code_sandbox_prompt and + clears any previous error. Deliberately does NOT touch + code_sandbox_code here - the dispatch + method decides generate-vs-reuse by reading the EXISTING + code_sandbox_code value at call time, so this must not overwrite it + before that decision is made.""" + node = self.nodes.get(node_id) + if node is None: + raise SceneError(f"unknown node: {node_id}") + if node.kind != "code_sandbox": + raise SceneError(f"node is not a code_sandbox node: {node_id}") + node.state.code_sandbox_prompt = str(input_text) + node.state.code_sandbox_error = "" + return node + + def complete_code_sandbox_run(self, node_id: str, code: str, output: str, analysis: str) -> SceneNode | None: + """Land a successful run. Execution Sandbox has no last_run_failed + flag: an unrecovered failure after exhausting its own repair + attempts surfaces as a failed run (see + AgentDispatcher.start_code_sandbox_run), never as a "succeeded but + flagged" result. Not kind-guarded: only ever reached via run_code_ + sandbox's own on_success closure (backend/api/ + intents_code_sandbox.py), whose node_id was already validated by + start_code_sandbox_run's own guard earlier in the same request - + same posture as complete_gitlink_run.""" + node = self.nodes.get(node_id) + if node is None: + return None + node.state.code_sandbox_code = str(code) + node.state.code_sandbox_output = str(output) + node.state.code_sandbox_analysis = str(analysis) + node.state.code_sandbox_awaiting_approval = False + node.state.code_sandbox_approval_requirements = "" + node.state.code_sandbox_approved_fingerprint = None + node.state.code_sandbox_approval_allow_source_builds = False + node.state.code_sandbox_approval_is_repair = False + node.state.code_sandbox_error = "" + return node + + def fail_code_sandbox_run(self, node_id: str, message: str) -> SceneNode | None: + """Land a failed (or denied-approval, or cancelled) run - the + awaiting_approval flag is ALWAYS cleared here too, unconditionally, + so a denied/cancelled approval never leaves the node stuck showing + the approval prompt forever (stale-while-revalidate: existing + code/output/analysis survive untouched). Not kind-guarded - see + complete_code_sandbox_run's own comment.""" + node = self.nodes.get(node_id) + if node is None: + return None + node.state.code_sandbox_awaiting_approval = False + node.state.code_sandbox_approval_requirements = "" + node.state.code_sandbox_approved_fingerprint = None + node.state.code_sandbox_approval_allow_source_builds = False + node.state.code_sandbox_approval_is_repair = False + node.state.code_sandbox_error = str(message) + return node diff --git a/backend/domain/nodes_code_review.py b/backend/domain/nodes_code_review.py index 1c5869d..579df83 100644 --- a/backend/domain/nodes_code_review.py +++ b/backend/domain/nodes_code_review.py @@ -16,6 +16,11 @@ The mixin needs only what SceneDocumentParts already declares - `nodes`, `connect`, `_counter` - so it inherits that and stays type-checkable in isolation, exactly like its cross-cutting siblings. + +Import posture, carried over with the code: nothing here imports from +graphlink_plugins.review_lens - every method below takes plain values, +already fetched and normalized by the dispatch layer, and only stores +them. """ from __future__ import annotations @@ -25,6 +30,7 @@ from backend.domain.node_access import optional_node, require_node from backend.domain.node_states import CodeReviewState + def _bundle_int(value: object) -> int: """Non-negative int from a fetch bundle, defaulting to 0. diff --git a/backend/domain/nodes_content.py b/backend/domain/nodes_content.py new file mode 100644 index 0000000..db4ee48 --- /dev/null +++ b/backend/domain/nodes_content.py @@ -0,0 +1,238 @@ +"""ContentOps - the SceneDocument methods for the document, thinking, html +and note node kinds. + +A MIXIN, composed exactly once, by backend/domain/graph.py's +SceneDocument. Method bodies are relocated VERBATIM from graph.py; +only the class wrapper, its docstring and the imports are new, and the +methods are regrouped by kind rather than left in the order successive +increments happened to append them in. + +See backend/domain/nodes_code_review.py's docstring for why the +per-kind method groups are being lifted out of SceneDocument at all. +""" + +from __future__ import annotations + +from backend.domain._composed import SceneDocumentParts +from backend.domain.model import ( + HTML_TITLE_PREVIEW_LENGTH, + THINKING_TITLE_PREVIEW_LENGTH, + SceneError, + SceneNode, +) +from backend.domain.node_states import DocumentState, HtmlState, NoteState + + +class ContentOps(SceneDocumentParts): + """The plain content kinds: document, thinking, html and note. + + Each stores text the user or an agent wrote and does nothing else with + it - no run, no thread, no render. Note is the one that may float free; + the other three require a parent, like every R3+ content kind. + """ + + def add_document_node( + self, + x: float, + y: float, + title: str, + content: str, + attachment_kind: str, + parent_id: str, + *, + file_path: str = "", + mime_type: str = "", + duration_seconds: float | None = None, + byte_size: int | None = None, + preview_label: str = "", + ) -> SceneNode: + """R3.9's document-node equivalent of add_chat_node/add_code_node: a + real file-attachment node (a document or an audio file), for the + legacy DocumentNode / ChatScene.add_document_node pair. UNLIKE + chat/code, parent_id is REQUIRED here, not optional: read fresh from + graphlink_scene.py, add_document_node(title, content, + parent_user_node, ...) takes parent_user_node as a plain required + positional with no default, and unconditionally constructs a + DocumentConnectionItem(parent_user_node, node) - there is no `if + parent_id` guard around that connection the way chat/code have + around theirs - so a DocumentNode can never exist unparented. + Document nodes are also NOT branch points (same as code): there is + no delete_document_node; deletion goes entirely through the + existing generic remove_nodes. + + The six attachment fields are stored verbatim - no title-preview + truncation (DocumentNode.title in the legacy app is just whatever + descriptive title/filename was passed in, confirmed by reading + DocumentNode.__init__: `self.title = title`, no slicing), and none + of the legacy view-layer formatting (byte-size/duration strings, + preview_label auto-fill, audio-preview suppression) happens here - + see DocumentState's own docstring (backend/domain/node_states.py) + for those exact rules. + """ + if parent_id is not None and parent_id not in self.nodes: + raise SceneError(f"unknown parent node: {parent_id}") + node_id = f"n{next(self._counter)}" + # Mirrors DocumentNode.__init__'s `(attachment_kind or + # "document").lower()` normalization - the attachment_kind param has + # no default in this signature (per spec), but an empty/None value + # still needs to fall back to "document" and casing still needs to + # normalize, since "audio" vs "Audio" is a real behavioral branch + # (metadata "Type" row, preview label, badge text all key off it). + normalized_kind = str(attachment_kind or "document").lower() + node = SceneNode( + id=node_id, + x=float(x), + y=float(y), + title=str(title), + kind="document", + content=str(content), + state=DocumentState( + attachment_kind=normalized_kind, + file_path=str(file_path), + mime_type=str(mime_type), + duration_seconds=duration_seconds, + byte_size=byte_size, + preview_label=str(preview_label), + ), + ) + self.nodes[node_id] = node + if parent_id is not None: + self.connect(parent_id, node_id) + return node + + def add_thinking_node( + self, + x: float, + y: float, + thinking_text: str, + parent_id: str, + ) -> SceneNode: + """R3.13's ThinkingNode equivalent of add_chat_node/add_code_node/ + add_document_node: a real reasoning-panel node. Same as + add_document_node (and unlike chat/code), parent_id is REQUIRED, not + optional - a ThinkingNode never exists unparented - so this + unconditionally connects to its parent, no `if parent_id` guard. + + Thinking text reuses the existing `content` field rather than a new + one - there is no separate thinking-text field. `is_docked` defaults + to False: a freshly-created thinking node is never pre-docked: dock() + is only ever invoked by explicit user action or on session-load + restore, never at construction time. + + Thinking nodes are also NOT branch points (same as code/document): + there is no delete_thinking_node; deletion goes entirely through the + existing generic remove_nodes. + """ + if parent_id is not None and parent_id not in self.nodes: + raise SceneError(f"unknown parent node: {parent_id}") + node_id = f"n{next(self._counter)}" + title = str(thinking_text)[:THINKING_TITLE_PREVIEW_LENGTH] or "Thinking" + node = SceneNode( + id=node_id, + x=float(x), + y=float(y), + title=title, + kind="thinking", + content=str(thinking_text), + ) + self.nodes[node_id] = node + if parent_id is not None: + self.connect(parent_id, node_id) + return node + + def add_html_node( + self, + x: float, + y: float, + html_content: str, + parent_id: str, + ) -> SceneNode: + """R3.17's HtmlViewNode equivalent of add_document_node/ + add_thinking_node: a real raw-HTML-source node. Same as + add_document_node/add_thinking_node (and unlike chat/code), parent_id + is REQUIRED, not optional - an HtmlViewNode never exists unparented - + so this unconditionally connects to its parent, no `if parent_id` + guard. + + The raw HTML source reuses the existing `content` field rather than a + new one - there is no separate html-content field, same reuse pattern + as R3.5's code text and R3.13's thinking text. The backend stores it + VERBATIM as an opaque string: it never parses, sanitizes, validates, + or otherwise interprets the HTML - that is the frontend's job (the + preview render is a 100% client-side action that never round-trips + here). + + Html nodes are also NOT branch points (same as code/document/ + thinking): there is no delete_html_node; deletion goes entirely + through the existing generic remove_nodes. + """ + if parent_id is not None and parent_id not in self.nodes: + raise SceneError(f"unknown parent node: {parent_id}") + node_id = f"n{next(self._counter)}" + title = str(html_content)[:HTML_TITLE_PREVIEW_LENGTH] or "HTML" + node = SceneNode( + id=node_id, + x=float(x), + y=float(y), + title=title, + kind="html", + content=str(html_content), + state=HtmlState(), + ) + self.nodes[node_id] = node + if parent_id is not None: + self.connect(parent_id, node_id) + return node + + def set_html_splitter_state(self, node_id: str, value: float) -> None: + """R6.3: persists an HtmlViewNode's draggable code/preview splitter + position. html kind only (SceneError otherwise), matching every + other kind-specific setter's guard pattern in this file (e.g. + resize_chart/toggle_frame_lock).""" + node = self.nodes.get(node_id) + if node is None: + raise SceneError(f"unknown node: {node_id}") + if node.kind != "html": + raise SceneError(f"node is not an html node: {node_id}") + node.state.html_splitter_state = float(value) + + # -- R6.1: notes ----------------------------------------------------------- + # + # Legacy canvas decorations. Notes are free-floating markdown sticky- + # notes with no parent-required posture, unlike almost every R3+ content + # kind. (Their neighbours in that increment, frames and containers, are + # group nodes and live in backend/domain/groups.py.) + + def add_note( + self, + x: float, + y: float, + *, + is_system_prompt: bool = False, + is_summary_note: bool = False, + ) -> SceneNode: + """A note's creation primitive. UNLIKE every R3+ content kind, no + parent is required or accepted - notes are free-floating, never + branch-point children (mirrors the legacy note widget, which the + canvas places directly, not via a ChatNode-anchored connection).""" + node_id = f"n{next(self._counter)}" + node = SceneNode( + id=node_id, + x=float(x), + y=float(y), + title="Note", + kind="note", + content="Add note...", + state=NoteState( + is_system_prompt=bool(is_system_prompt), + is_summary_note=bool(is_summary_note), + ), + ) + self.nodes[node_id] = node + return node + + def set_note_content(self, node_id: str, content: str) -> None: + node = self.nodes.get(node_id) + if node is None: + raise SceneError(f"unknown node: {node_id}") + node.content = str(content) diff --git a/backend/domain/nodes_conversational.py b/backend/domain/nodes_conversational.py new file mode 100644 index 0000000..ea4e4cd --- /dev/null +++ b/backend/domain/nodes_conversational.py @@ -0,0 +1,218 @@ +"""ConversationalOps - the SceneDocument methods for the chat and +conversation node kinds. + +A MIXIN, composed exactly once, by backend/domain/graph.py's +SceneDocument. Method bodies are relocated VERBATIM from graph.py; +only the class wrapper, its docstring and the imports are new, and the +methods are regrouped by kind rather than left in the order successive +increments happened to append them in. + +See backend/domain/nodes_code_review.py's docstring for why the +per-kind method groups are being lifted out of SceneDocument at all. +""" + +from __future__ import annotations + +from typing import Any + +from backend.domain._composed import SceneDocumentParts +from backend.domain.model import CHAT_TITLE_PREVIEW_LENGTH, SceneError, SceneNode +from backend.domain.node_states import ChatState + + +class ConversationalOps(SceneDocumentParts): + """The two message-threaded kinds: chat nodes (one bubble each) and + conversation nodes (a whole thread in one node). + + They are the same family - `set_all_conversational_collapsed` on + SceneDocument names it - and they share the history/content_parts + bookkeeping that no other kind has. + """ + + def add_chat_node( + self, + x: float, + y: float, + content: str, + is_user: bool, + parent_id: str | None = None, + content_parts: list[dict[str, Any]] | None = None, + ) -> SceneNode: + """The Qt-free ChatScene.add_chat_node equivalent: a real message- + bubble node, optionally connected to a parent (the branch it + continues). Mirrors add_node's id/dict bookkeeping; the only new + behavior is the parent-edge, ported from the legacy scene's own + ConnectionItem creation. + + R8a: content_parts is the real multimodal attachment payload + (image_bytes/audio_file parts) - the data-model capability + ChatState.content_parts (backend/domain/node_states.py) has carried + since R6.3, finally populated by a real caller. Optional and + additive: every existing caller keeps + passing only (x, y, content, is_user, parent_id) and gets exactly + the plain-text node it always did.""" + if parent_id is not None and parent_id not in self.nodes: + raise SceneError(f"unknown parent node: {parent_id}") + node_id = f"n{next(self._counter)}" + title = content[:CHAT_TITLE_PREVIEW_LENGTH] or ("You" if is_user else "Assistant") + node = SceneNode( + id=node_id, + x=float(x), + y=float(y), + title=title, + kind="chat", + content=str(content), + state=ChatState(is_user=bool(is_user), content_parts=content_parts), + ) + self.nodes[node_id] = node + if parent_id is not None: + self.connect(parent_id, node_id) + else: + self.adopt_pending_system_prompt(node_id) + return node + + def update_chat_node_content( + self, node_id: str, content: str, incomplete: bool = False + ) -> SceneNode: + """The regenerate primitive: mutate an EXISTING chat node's content in + place - the first in-place mutation of a content-bearing field in this + file (move_node/set_chat_collapsed/set_node_docked all mutate a + position/flag, never displayed text). Scope confirmed against legacy's + ChatNode.update_content (graphlink_nodes/graphlink_node_chat.py:677-686): + sets content ONLY. Does not touch title (legacy's update_content never + recomputes any title-like state either, and every other in-place mutator + here already leaves title untouched post-creation - consistent, not a + new carve-out). Does not touch is_user/is_collapsed/kind. + + ADR-006 stage 6.4: `incomplete` marks a PARTIAL reply committed after + its stream died (see ChatState.response_incomplete). A normal full + regenerate passes the default False, which doubles as the CLEAR for a + previously interrupted node - retry succeeds, banner goes away.""" + node = self.nodes.get(node_id) + if node is None: + raise SceneError(f"unknown node: {node_id}") + node.content = str(content) + if isinstance(node.state, ChatState): + node.state.response_incomplete = bool(incomplete) + return node + + def set_chat_collapsed(self, node_id: str, collapsed: bool) -> None: + node = self.nodes.get(node_id) + if node is None: + raise SceneError(f"unknown node: {node_id}") + node.is_collapsed = bool(collapsed) + # R6.1: unlike every other kind this generic setter already served, + # a frame/container's is_collapsed also drives derived geometry + # (the collapsed pill size vs the auto-fit/manual bbox) - recompute + # here too so this stays correct no matter which entry point sets + # it, rather than only being safe via toggle_group_collapsed. + if node.kind in ("frame", "container"): + self._recompute_group_bounds(node_id) + + def set_chat_scroll_value(self, node_id: str, value: float) -> None: + """R6.3: persists a chat node's own scroll position within its + content area. chat kind only (SceneError otherwise), matching every + other kind-specific setter's guard pattern in this file (e.g. + resize_chart/toggle_frame_lock).""" + node = self.nodes.get(node_id) + if node is None: + raise SceneError(f"unknown node: {node_id}") + if node.kind != "chat": + raise SceneError(f"node is not a chat node: {node_id}") + node.state.chat_scroll_value = float(value) + + # -- conversation node (a full thread in a single node) ------------------ + + def add_conversation_node(self, x: float, y: float, parent_id: str | None) -> SceneNode: + """R3.25's ConversationNode equivalent of add_document_node/ + add_thinking_node/add_html_node/add_image_node: a real multi-message + conversation node. Same as document/thinking/html/image (and unlike + chat/code), parent_id is REQUIRED, not optional - a ConversationNode + never exists unparented - so this unconditionally connects to its + parent, no `if parent_id` guard. + + Title is always the fixed literal "Conversation" - never derived or + truncated from any content, unlike every scalar-content kind before + it (chat/thinking/html/image all preview their own text). There is + no natural single preview string for a node whose content is a + growing LIST of messages, so the title never changes as messages are + appended (see append_conversation_user_message/ + append_conversation_assistant_message below - neither touches + title). Mirrors graphlink_conversation_node.py's `title_label = + QLabel("Conversation")`, a hardcoded literal, not derived state. + + `history` starts empty - a freshly-created conversation node has no + messages yet, same posture as `is_docked` defaulting False on a + freshly-created thinking node. + + Conversation nodes are also NOT branch points (same as code/document/ + thinking/html/image): there is no delete_conversation_node; deletion + goes entirely through the existing generic remove_nodes. + """ + if parent_id is not None and parent_id not in self.nodes: + raise SceneError(f"unknown parent node: {parent_id}") + node_id = f"n{next(self._counter)}" + node = SceneNode( + id=node_id, + x=float(x), + y=float(y), + title="Conversation", + kind="conversation", + ) + self.nodes[node_id] = node + if parent_id is not None: + self.connect(parent_id, node_id) + return node + + def append_conversation_user_message(self, node_id: str, text: str) -> SceneNode: + """Append a real user message to a conversation node's history - + mirrors graphlink_conversation_node.py's add_user_message, minus the + view-layer bubble creation (the frontend's job).""" + node = self.nodes.get(node_id) + if node is None: + raise SceneError(f"unknown node: {node_id}") + node.history.append({"role": "user", "content": str(text)}) + return node + + def append_conversation_assistant_message( + self, node_id: str, text: str, incomplete: bool = False + ) -> SceneNode: + """Append a real assistant message to a conversation node's history - + mirrors graphlink_conversation_node.py's add_ai_message, minus the + view-layer bubble creation. + + ADR-006 stage 6.4: `incomplete=True` marks a PARTIAL reply whose + stream died mid-generation (H5 - the accumulated text is preserved + instead of lost). The key is only written when set - completed + messages keep their exact two-key {role, content} shape, so every + existing history consumer (session round-trip, agent context + assembly) sees byte-identical data for the normal path.""" + node = self.nodes.get(node_id) + if node is None: + raise SceneError(f"unknown node: {node_id}") + message: dict[str, Any] = {"role": "assistant", "content": str(text)} + if incomplete: + message["incomplete"] = True + node.history.append(message) + return node + + def delete_conversation_message(self, node_id: str, message_index: int) -> None: + """Prune one message out of a conversation node's history by index - + mirrors graphlink_conversation_node.py's _remove_message's index- + synced pop, minus the view-layer bubble removal/re-layout (the + frontend's job).""" + node = self.nodes.get(node_id) + if node is None: + raise SceneError(f"unknown node: {node_id}") + if message_index < 0 or message_index >= len(node.history): + raise SceneError(f"message index out of range: {message_index}") + node.history.pop(message_index) + + def send_conversation_message(self, node_id: str, text: str) -> SceneNode: + """The Conversation node's own Send action (R3.25): a thin wrapper + over append_conversation_user_message, kept as a separate method + (rather than only calling append_conversation_user_message directly + from the WS wrapper) so the WS intent name lines up 1:1 with the + domain method, the same way sendMessage/send_message already do for + ChatNode.""" + return self.append_conversation_user_message(node_id, text) diff --git a/backend/domain/nodes_gitlink.py b/backend/domain/nodes_gitlink.py index ba9cade..5afa478 100644 --- a/backend/domain/nodes_gitlink.py +++ b/backend/domain/nodes_gitlink.py @@ -5,6 +5,13 @@ Method bodies are relocated VERBATIM from graph.py; only the class wrapper and the imports are new. See backend/domain/nodes_code_review.py's own docstring for why these per-kind groups are being lifted out. + +Import posture, carried over with the code: nothing here imports from +graphlink_plugins.gitlink - every method below is pure state mutation on +plain fields. The fingerprint mechanism itself (_fingerprint_changes) +lives in backend/agents.py, which DOES import from +graphlink_plugins.gitlink; same precedent as ArtifactAgent and +web_research.domain already being imported there, not here. """ from __future__ import annotations @@ -16,6 +23,7 @@ from backend.domain.node_access import optional_node, require_node from backend.domain.node_states import GitlinkState + class GitlinkOps(SceneDocumentParts): def add_gitlink_node(self, x: float, y: float, parent_id: str | None) -> SceneNode: """The Gitlink node's creation primitive - same required-parent diff --git a/backend/domain/nodes_planning.py b/backend/domain/nodes_planning.py new file mode 100644 index 0000000..fed0739 --- /dev/null +++ b/backend/domain/nodes_planning.py @@ -0,0 +1,149 @@ +"""PlanningOps - the SceneDocument methods for the plan and harness node +kinds. + +A MIXIN, composed exactly once, by backend/domain/graph.py's +SceneDocument. Method bodies are relocated VERBATIM from graph.py; +only the class wrapper, its docstring and the imports are new, and the +methods are regrouped by kind rather than left in the order successive +increments happened to append them in. + +See backend/domain/nodes_code_review.py's docstring for why the +per-kind method groups are being lifted out of SceneDocument at all. +""" + +from __future__ import annotations + +import uuid +from typing import Any + +from backend.domain._composed import SceneDocumentParts +from backend.domain.model import CHAT_TITLE_PREVIEW_LENGTH, SceneError, SceneNode +from backend.domain.node_states import HarnessState, PlanState + + +class PlanningOps(SceneDocumentParts): + """The two nodes that hold an agent's plan of work: the Builder's plan + checklist and the workspace harness. + + Both are orchestration scaffolding rather than content - a plan node + carries steps with a status apiece, a harness node carries the caps a + run must stay inside - so neither has the parent-required posture the + content kinds share. + """ + + # -- ADR-008 stage 8.3: plan node (the Builder's checklist) -------------- + + _PLAN_STEP_STATUSES = ("pending", "running", "done", "failed", "skipped") + + def add_plan_node( + self, + x: float, + y: float, + goal: str, + *, + mode: str = "copilot", + max_steps: int = 12, + max_tokens: int = 150_000, + max_wall_seconds: int = 900, + ) -> SceneNode: + """The Builder plan node's creation primitive. Free-floating like a + note (a build STARTS from a goal, it does not continue an existing + branch - the nodes the build creates are the ones that connect); + `content` reuses the goal text the same way web_research reuses + content for its query. Everything else lives on PlanState - see its + own docstring for the state machine and the plan-node-as-resume- + point contract.""" + if mode not in ("copilot", "autopilot"): + raise SceneError(f"unknown builder mode: {mode}") + node_id = f"n{next(self._counter)}" + node = SceneNode( + id=node_id, + x=float(x), + y=float(y), + title=f"Build: {str(goal)[:CHAT_TITLE_PREVIEW_LENGTH]}" if goal else "Build", + kind="plan", + content=str(goal), + state=PlanState( + plan_goal=str(goal), + builder_mode=mode, + builder_max_steps=int(max_steps), + builder_max_tokens=int(max_tokens), + builder_max_wall_seconds=int(max_wall_seconds), + ), + ) + self.nodes[node_id] = node + return node + + def set_plan_steps(self, node_id: str, steps: list) -> SceneNode: + """Replaces the plan's step list - the one plan mutator that goes + through record_command (a user editing the checklist, or the + model's replan tool): step CONTENT is document state a Ctrl+Z must + reach, unlike the run-lifecycle fields (builder_status/spent_*/ + awaiting_*) which the loop writes directly, exactly as Execution + Sandbox's own run pipeline writes its awaiting/progress fields. + + Steps whose status is not "pending" are immutable history - a + replacement must carry every non-pending step through unchanged + (same id, title, status), enforced here so neither a user edit nor + a model replan can rewrite what already happened.""" + node = self.nodes.get(node_id) + if node is None or not isinstance(node.state, PlanState): + raise SceneError(f"not a plan node: {node_id}") + normalized: list[dict[str, Any]] = [] + seen_ids: set[str] = set() + for raw in steps: + if not isinstance(raw, dict): + raise SceneError("each step must be an object") + step_id = str(raw.get("id") or f"s{len(normalized) + 1}") + if step_id in seen_ids: + raise SceneError(f"duplicate step id: {step_id}") + seen_ids.add(step_id) + status = str(raw.get("status") or "pending") + if status not in self._PLAN_STEP_STATUSES: + raise SceneError(f"unknown step status: {status}") + title = str(raw.get("title") or "").strip() + if not title: + raise SceneError("each step needs a title") + normalized.append({ + "id": step_id, "title": title, "status": status, + "detail": str(raw.get("detail") or ""), + }) + frozen = {s["id"]: s for s in node.state.plan_steps if s.get("status") != "pending"} + for step_id, original in frozen.items(): + replacement = next((s for s in normalized if s["id"] == step_id), None) + if replacement is None: + raise SceneError( + f"step {step_id!r} has already run ({original['status']}) and cannot be removed" + ) + if replacement["title"] != original["title"] or replacement["status"] != original["status"]: + raise SceneError( + f"step {step_id!r} has already run ({original['status']}) and cannot be rewritten" + ) + node.state.plan_steps = normalized + return node + + # -- PLAN-2026-08-24 H1: harness node (the workspace agent) -------------- + + def add_harness_node(self, x: float, y: float, goal: str, *, max_turns: int = 16) -> SceneNode: + """The harness node's creation primitive. Free-floating like a plan + node (a task starts from a prompt, it does not continue an existing + branch); harness_workspace_id is minted here, ONCE - the same + code_sandbox_sandbox_id precedent, see HarnessState's own docstring + for why node.id is not durable enough to name the on-disk + workspace.""" + node_id = f"n{next(self._counter)}" + node = SceneNode( + id=node_id, + x=float(x), + y=float(y), + title=f"Agent: {str(goal)[:CHAT_TITLE_PREVIEW_LENGTH]}" if goal else "Agent", + kind="harness", + content=str(goal), + state=HarnessState( + harness_goal=str(goal), + harness_workspace_id=uuid.uuid4().hex[:12], + harness_max_turns=int(max_turns), + ), + ) + self.nodes[node_id] = node + return node diff --git a/backend/domain/nodes_visual.py b/backend/domain/nodes_visual.py new file mode 100644 index 0000000..db40611 --- /dev/null +++ b/backend/domain/nodes_visual.py @@ -0,0 +1,278 @@ +"""VisualOps - the SceneDocument methods for the chart and image node kinds. + +A MIXIN, composed exactly once, by backend/domain/graph.py's +SceneDocument. Method bodies are relocated VERBATIM from graph.py; +only the class wrapper, its docstring and the imports are new, and the +methods are regrouped by kind rather than left in the order successive +increments happened to append them in. + +See backend/domain/nodes_code_review.py's docstring for why the +per-kind method groups are being lifted out of SceneDocument at all. +""" + +from __future__ import annotations + +import uuid +from typing import Any + +from graphlink_chart_data import SUPPORTED_CHART_TYPES + +from backend.domain._composed import SceneDocumentParts +from backend.domain.model import ( + CHART_MAX_HEIGHT, + CHART_MAX_WIDTH, + CHART_MIN_HEIGHT, + CHART_MIN_WIDTH, + IMAGE_TITLE_PREVIEW_LENGTH, + SceneError, + SceneNode, +) +from backend.domain.node_states import ChartState, ImageState + + +class VisualOps(SceneDocumentParts): + """The two kinds whose payload is a picture: charts and images. + + A chart node holds a spec the client renders interactively and a size + the user can drag; an image node holds a generated asset plus the reply + bubble that delivered it. Both therefore carry geometry that the generic + layout code must not touch, which is why their resize/aspect handling + lives with them rather than in LayoutOps. + """ + + # -- R6.2: chart node ---------------------------------------------------- + + def add_chart_node( + self, + x: float, + y: float, + parent_id: str | None, + chart_type: str, + chart_data: dict[str, Any], + *, + chart_error: str = "", + ) -> SceneNode: + """The Chart node's creation primitive - same required-parent + posture as every other branch-point-child kind (web_research/ + artifact/gitlink/code_sandbox above) for every NEW chart: + the UI-driven generateChart intent always passes a real parent_id, + since a chart is always generated FROM some other node's content in + that flow. chart_type MUST be one of SUPPORTED_CHART_TYPES + (SceneError otherwise, same "validate up front, never construct a + half-invalid node" posture create_frame/create_container use for + their own item_ids checks). + + R6.4: parent_id is None-able for the session LOADER only - legacy + genuinely allows a chart with no parent at all (both + parent_node_index/parent_node_id absent in the persisted payload is + a real, valid legacy state, confirmed by recon), which the original + required-parent signature could not represent. When parent_id is + None, no parent-existence check runs and no edge is created - + chart_source_node_id stays "" rather than getting a real node id. + + chart_data is assumed ALREADY canonicalized by the CALLER - this + method deliberately does NOT call canonicalize_chart_data itself + (see chart_data's own field comment on SceneNode for the full + reasoning: the WS-intent wrapper needs to be able to catch + ChartDataError itself and still create a placeholder chart with + chart_error set, rather than have creation abort entirely). + + Title mirrors legacy ChartItem's own `self.title = str(self.data. + get("title") or "Chart")` - the chart's own title field if present, + else the literal "Chart" (not a chart-type-specific default; that is + genuinely what legacy does). + + ADR-013 stage 13.4: no longer renders a PNG here - the client-side + interactive renderer (stage 13.2) draws straight from chart_data, + and nothing has consumed the backend-rendered display asset since. + A chart's ONLY remaining matplotlib render is the export/copy + endpoint (backend/assets.py), a fresh re-render on every request.""" + if parent_id is not None and parent_id not in self.nodes: + raise SceneError(f"unknown parent node: {parent_id}") + normalized_type = str(chart_type or "").strip().lower() + if normalized_type not in SUPPORTED_CHART_TYPES: + raise SceneError(f"unsupported chart type: {chart_type}") + + node_id = f"n{next(self._counter)}" + safe_chart_data = dict(chart_data) if isinstance(chart_data, dict) else {} + title = str(safe_chart_data.get("title") or "Chart") + node = SceneNode( + id=node_id, + x=float(x), + y=float(y), + title=title, + kind="chart", + state=ChartState( + chart_type=normalized_type, + chart_data=safe_chart_data, + chart_error=str(chart_error), + chart_source_node_id=parent_id or "", + ), + ) + + self.nodes[node_id] = node + if parent_id is not None: + self.connect(parent_id, node_id) + return node + + def resize_chart(self, node_id: str, width: float, height: float) -> None: + """Chart kind only (SceneError otherwise). Clamps (width, height) + into [CHART_MIN_WIDTH, CHART_MAX_WIDTH] / [CHART_MIN_HEIGHT, + CHART_MAX_HEIGHT]. If chart_aspect_locked, preserves the aspect + ratio of the REQUESTED (width, height) pair AS SENT - the frontend/ + NodeResizer is responsible for computing a ratio-correct pair before + ever calling this; UNLIKE legacy ChartItem._clamp_size (which + consults self.resize_start_aspect_ratio, a value frozen at drag + START), this method has no concept of an in-progress gesture, so it + only ever has the two numbers it was given to work from. After the + plain min/max clamp, if aspect-locked, re-derives whichever + dimension keeps the REQUESTED ratio relative to the (already- + clamped) other dimension - same "pick whichever correction moves the + clamped pair least" algorithm legacy's own _clamp_size uses - then + re-clamps once more, so the final stored size never violates either + the lock or the min/max bounds even after that re-derivation. + + ADR-013 stage 13.4: no longer re-renders a PNG here - see + add_chart_node's own docstring for why.""" + node = self.nodes.get(node_id) + if node is None: + raise SceneError(f"unknown node: {node_id}") + if node.kind != "chart": + raise SceneError(f"node is not a chart node: {node_id}") + + requested_width = float(width) + requested_height = float(height) + clamped_width = min(CHART_MAX_WIDTH, max(CHART_MIN_WIDTH, requested_width)) + clamped_height = min(CHART_MAX_HEIGHT, max(CHART_MIN_HEIGHT, requested_height)) + + if node.state.chart_aspect_locked and requested_width > 0 and requested_height > 0: + aspect_ratio = requested_width / requested_height + width_from_height = clamped_height * aspect_ratio + height_from_width = clamped_width / aspect_ratio + if abs(width_from_height - clamped_width) < abs(height_from_width - clamped_height): + clamped_width = width_from_height + clamped_height = clamped_width / aspect_ratio + else: + clamped_height = height_from_width + clamped_width = clamped_height * aspect_ratio + # Re-deriving one dimension from the other can overshoot the + # opposite bound for an extreme aspect ratio - one more clamp + # keeps the final pair inside both bounds unconditionally. + clamped_width = min(CHART_MAX_WIDTH, max(CHART_MIN_WIDTH, clamped_width)) + clamped_height = min(CHART_MAX_HEIGHT, max(CHART_MIN_HEIGHT, clamped_height)) + + node.state.chart_width = clamped_width + node.state.chart_height = clamped_height + + def toggle_chart_aspect_lock(self, node_id: str) -> None: + """Chart kind only (SceneError otherwise). Flips chart_aspect_locked.""" + node = self.nodes.get(node_id) + if node is None: + raise SceneError(f"unknown node: {node_id}") + if node.kind != "chart": + raise SceneError(f"node is not a chart node: {node_id}") + node.state.chart_aspect_locked = not node.state.chart_aspect_locked + + def add_image_node( + self, + x: float, + y: float, + image_bytes: bytes, + prompt: str, + parent_id: str, + *, + mime_type: str = "image/png", + ) -> SceneNode: + """R3.21's image-node equivalent of add_document_node/ + add_thinking_node/add_html_node: a real generated-image node. Same as + document/thinking/html (and unlike chat/code), parent_id is + REQUIRED, not optional - an image node never exists unparented - so + this unconditionally connects to its parent, no `if parent_id` guard. + + Image bytes do NOT live on SceneNode (see the transport-decision + comment on SceneDocument.image_assets) - they go into that + session-scoped store, keyed by a SEPARATE id. Unlike node/edge ids + (which only need to be unique within their own SceneDocument, since + nothing ever looks a node up across sessions), asset ids are read + back through GET /api/assets/{id}, a route that takes a bare id plus + an independent session query param - so a per-document counter here + would let two sessions mint the identical "imgN" id for unrelated + images (guaranteed, not just probabilistic, for sessions that create + nodes in the same order), and a caller that omits/mis-supplies the + session param would silently be served someone else's image instead + of a 404. A uuid4 hex keeps the id globally unique so cross-session + collision is not possible regardless of session query correctness. + image_asset_id on the node is just the opaque reference key into + that store. + + There is no natural title-preview text for an image the way there is + for text-based kinds, so the title is the prompt (truncated, same + 60-char convention as chat/thinking/html) when non-empty, else a + literal "Image". + + Image nodes are also NOT branch points (same as code/document/ + thinking/html): there is no delete_image_node; deletion goes + entirely through the existing generic remove_nodes, which + additionally evicts this node's image_assets entry so bytes never + outlive the node (see remove_nodes). + """ + if parent_id is not None and parent_id not in self.nodes: + raise SceneError(f"unknown parent node: {parent_id}") + node_id = f"n{next(self._counter)}" + asset_id = f"img{uuid.uuid4().hex}" + self.image_assets[asset_id] = (image_bytes, mime_type) + title = str(prompt)[:IMAGE_TITLE_PREVIEW_LENGTH] or "Image" + node = SceneNode( + id=node_id, + x=float(x), + y=float(y), + title=title, + kind="image", + content=str(prompt), + state=ImageState(image_asset_id=asset_id), + ) + self.nodes[node_id] = node + if parent_id is not None: + self.connect(parent_id, node_id) + return node + + def get_image_asset(self, asset_id: str) -> tuple[bytes, str] | None: + """The read-side of image_assets - the same lookup backend/assets.py's + GET /api/assets/{id} route calls to serve the raw bytes + mime type.""" + return self.image_assets.get(asset_id) + + def add_generated_image_reply( + self, + parent_chat_node_id: str, + prompt: str, + image_bytes: bytes, + mime_type: str = "image/png", + ) -> tuple[SceneNode, SceneNode]: + """The Generate/Regenerate Image success primitive (R4.4a) - mirrors + legacy's handle_image_response exactly: unconditionally creates a NEW + assistant ChatNode (content=f'Generated image for prompt: "{prompt}"', + is_user=False, parent_id=parent_chat_node_id) then a NEW ImageNode + (content=prompt, parent_id=) - built entirely + from the existing add_chat_node/add_image_node primitives, zero new + mutation-in-place logic, matching this feature's create-new-nodes + scope decision. Positions via place_child (backend/domain/layout.py), + the same collision-resolved placement send_message/regenerate_ + response's own new-child placement uses. last_chat_node_id is DELIBERATELY untouched + - mirrors legacy: handle_image_response never assigns + self.current_node either, since image generation is side content, + not a branch-continuation point (same posture as + regenerate_response's own documented "last_chat_node_id: + DELIBERATELY untouched"). Raises SceneError if parent_chat_node_id is + unknown - defensive: a delete could race the in-flight generation + request (see the mid-flight-delete handling in the WS wrapper in + register_canvas).""" + parent = self.nodes.get(parent_chat_node_id) + if parent is None: + raise SceneError(f"unknown parent node: {parent_chat_node_id}") + ax, ay = self.place_child(parent_chat_node_id, "chat") + chat_node = self.add_chat_node( + ax, ay, f'Generated image for prompt: "{prompt}"', False, parent_id=parent_chat_node_id, + ) + ix, iy = self.place_child(chat_node.id, "image") + image_node = self.add_image_node(ix, iy, image_bytes, prompt, chat_node.id, mime_type=mime_type) + return chat_node, image_node From 3b746c080c4dd28796e2d47e9a794e50456ec635 Mon Sep 17 00:00:00 2001 From: dovvnloading <157447210+dovvnloading@users.noreply.github.com> Date: Fri, 4 Sep 2026 12:57:05 -0400 Subject: [PATCH 2/2] Narrow the five new mixins, and widen mypy onto most of backend/domain The five mixins the previous commit created reported 92 mypy errors, every one the same union-attr shape: SceneNode.state is typed `NodeState | None` against a field-less marker, so `node.state.research_stage` cannot be verified. All 92 sat in exactly 17 methods, and all 17 obtain their node with the same hand-written preamble require_node/optional_node (backend/domain/node_access.py, PR #410) already replaces. Converted; the five modules now check clean. Nine of the seventeen were a straight swap - they already checked the kind and raised the same two messages require_node raises. The other eight are a behaviour change and should be read as one. The completion and failure paths for web_research, artifact and code_sandbox looked their node up WITHOUT checking its kind, on the reasoning - written into two of the docstrings - that the id had already been validated earlier in the same request by the matching start_* call. That is true, and it is still true: the check is redundant on every live path. It is also exactly why the gap was invisible. Node states are plain, non-slotted dataclasses, so `node.state.research_stage = "completed"` against a chat node does not fail; it grafts a phantom attribute onto ChatState and returns happily. backend/tests/test_wrong_kind_node_guards.py pins the new contract for all eight, plus complete_gitlink_run and complete_gitlink_apply, which gained the same check in #409 with nothing asserting it. Each case is checked three ways: the wrong-kind call raises (or, for the fail_* methods, returns None, matching their documented silence when the node has gone), and the wrong-kind node's state is left byte-identical. Verified to fail when a guard is removed. One error message changed: set_html_splitter_state said "node is not an html node" and now says "a html node", the wording every other kind produces. Nothing in the repo or the client reads it. [tool.mypy].files widens from 3 backend/domain/ modules to 13 of 18. The five left out are graph.py and the four cross-cutting mixins (groups/branches/commands/layout), which hold 124 errors of the same shape - they read per-kind state off nodes they look up generically, so require_node has no kind to narrow to. Recorded as the next piece of work rather than waved through. Test plan: full suite, 3,244 passed / 19 skipped. Two unrelated subprocess-timing tests (test_mcp_client stdin-drain, test_execution_guard grandchild-kill) failed under load in that run and pass on their own; neither imports backend.domain. ruff clean; mypy clean across 42 files. Co-Authored-By: Claude Opus 5 --- backend/domain/nodes_agent_runs.py | 72 +++++---------- backend/domain/nodes_content.py | 7 +- backend/domain/nodes_conversational.py | 7 +- backend/domain/nodes_visual.py | 13 +-- backend/tests/test_wrong_kind_node_guards.py | 94 ++++++++++++++++++++ pyproject.toml | 20 +++++ 6 files changed, 145 insertions(+), 68 deletions(-) create mode 100644 backend/tests/test_wrong_kind_node_guards.py diff --git a/backend/domain/nodes_agent_runs.py b/backend/domain/nodes_agent_runs.py index 33055a0..a846eef 100644 --- a/backend/domain/nodes_agent_runs.py +++ b/backend/domain/nodes_agent_runs.py @@ -17,6 +17,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 ( ArtifactState, CodeSandboxState, @@ -68,11 +69,7 @@ def start_web_research_run(self, node_id: str, query: str) -> SceneNode: stale-while-revalidate: the previous run's answer stays visible until this run replaces it on success, or fails/cancels (leaving the stale result annotated by the new research_error).""" - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") - if node.kind != "web_research": - raise SceneError(f"node is not a web_research node: {node_id}") + node = require_node(self.nodes, node_id, "web_research", WebResearchState) node.content = str(query) node.state.research_stage = "" node.state.research_completed = 0 @@ -89,7 +86,7 @@ def apply_web_research_progress(self, node_id: str, event) -> SceneNode | None: agents.py importing backend.canvas.SceneNode). Silent no-op (returns None, never raises) if node_id is no longer in self.nodes - the node may have been deleted while a background run was still in flight.""" - node = self.nodes.get(node_id) + node = optional_node(self.nodes, node_id, "web_research", WebResearchState) if node is None: return None node.state.research_stage = event.stage.value @@ -103,9 +100,7 @@ def complete_web_research_run(self, node_id: str, result_wire: dict) -> SceneNod gone - the WS wrapper's own liveness check (in register_canvas) guards the actual mid-flight-delete race; this stays a hard precondition here, same posture as update_chat_node_content.""" - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") + node = require_node(self.nodes, node_id, "web_research", WebResearchState) node.state.research_stage = "completed" node.state.research_error = "" node.state.research_active_source_id = None @@ -116,9 +111,7 @@ def fail_web_research_run(self, node_id: str, *, cancelled: bool, message: str) """Land a failed or cancelled run. research_result is deliberately left untouched (stale-while-revalidate - see start_web_research_run's own docstring).""" - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") + node = require_node(self.nodes, node_id, "web_research", WebResearchState) node.state.research_stage = "cancelled" if cancelled else "failed" node.state.research_error = message node.state.research_active_source_id = None @@ -128,11 +121,7 @@ def set_web_research_retain_to_knowledge(self, node_id: str, retain: bool) -> Sc """ADR-021 stage 21.5: the per-node "keep these sources" preference. Same shape as set_code_sandbox_requirements below - validate the node exists and is the right kind, then write one state field.""" - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") - if node.kind != "web_research": - raise SceneError(f"node is not a web_research node: {node_id}") + node = require_node(self.nodes, node_id, "web_research", WebResearchState) node.state.research_retain_to_knowledge = bool(retain) return node @@ -167,9 +156,7 @@ def append_artifact_user_message(self, node_id: str, text: str) -> SceneNode: """Append a real user instruction to an artifact node's history - mirrors append_conversation_user_message exactly (same shape, same error-handling style).""" - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") + node = require_node(self.nodes, node_id, "artifact", ArtifactState) node.history.append({"role": "user", "content": str(text)}) # A new instruction supersedes the previous failure - leaving the # banner up beside an in-flight run would report a stale outcome. @@ -185,7 +172,7 @@ def fail_artifact_generation(self, node_id: str, message: str) -> SceneNode | No this is called from the dispatch task's own except/timeout paths, where the node may well have been deleted mid-flight, and a failure report is not worth turning into a second failure.""" - node = self.nodes.get(node_id) + node = optional_node(self.nodes, node_id, "artifact", ArtifactState) if node is None: return None node.state.artifact_error = str(message) @@ -210,9 +197,7 @@ def complete_artifact_generation(self, node_id: str, new_content, ai_message: st web_research's more defensive pre-check pattern (there is no stage-stepper/persisted-error field here for a mid-flight delete to race against).""" - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") + node = require_node(self.nodes, node_id, "artifact", ArtifactState) node.state.artifact_content = str(new_content) node.state.artifact_error = "" node.history.append({"role": "assistant", "content": str(ai_message)}) @@ -254,11 +239,7 @@ def add_code_sandbox_node(self, x: float, y: float, parent_id: str | None) -> Sc return node def set_code_sandbox_requirements(self, node_id: str, requirements_text: str) -> SceneNode: - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") - if node.kind != "code_sandbox": - raise SceneError(f"node is not a code_sandbox node: {node_id}") + node = require_node(self.nodes, node_id, "code_sandbox", CodeSandboxState) node.state.code_sandbox_requirements = str(requirements_text) return node @@ -271,11 +252,7 @@ def set_code_sandbox_allow_source_builds(self, node_id: str, allow: bool) -> Sce set here while no gate is open never reaches an actual run; the approval panel is the only surface that ever renders this control, and it only renders while awaiting_approval is true.""" - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") - if node.kind != "code_sandbox": - raise SceneError(f"node is not a code_sandbox node: {node_id}") + node = require_node(self.nodes, node_id, "code_sandbox", CodeSandboxState) node.state.code_sandbox_approval_allow_source_builds = bool(allow) return node @@ -286,11 +263,7 @@ def start_code_sandbox_run(self, node_id: str, input_text: str) -> SceneNode: method decides generate-vs-reuse by reading the EXISTING code_sandbox_code value at call time, so this must not overwrite it before that decision is made.""" - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") - if node.kind != "code_sandbox": - raise SceneError(f"node is not a code_sandbox node: {node_id}") + node = require_node(self.nodes, node_id, "code_sandbox", CodeSandboxState) node.state.code_sandbox_prompt = str(input_text) node.state.code_sandbox_error = "" return node @@ -300,12 +273,15 @@ def complete_code_sandbox_run(self, node_id: str, code: str, output: str, analys flag: an unrecovered failure after exhausting its own repair attempts surfaces as a failed run (see AgentDispatcher.start_code_sandbox_run), never as a "succeeded but - flagged" result. Not kind-guarded: only ever reached via run_code_ - sandbox's own on_success closure (backend/api/ - intents_code_sandbox.py), whose node_id was already validated by - start_code_sandbox_run's own guard earlier in the same request - - same posture as complete_gitlink_run.""" - node = self.nodes.get(node_id) + flagged" result. Only ever reached via run_code_sandbox's own + on_success closure (backend/api/intents_code_sandbox.py), whose + node_id was already validated by start_code_sandbox_run's own guard + earlier in the same request, so the kind check here is redundant on + every live path - it is here so that a caller who gets it wrong gets + None rather than a phantom code_sandbox_* attribute grafted onto + some other kind's state (these are plain non-slotted dataclasses; + the bad write would otherwise succeed silently).""" + node = optional_node(self.nodes, node_id, "code_sandbox", CodeSandboxState) if node is None: return None node.state.code_sandbox_code = str(code) @@ -324,9 +300,9 @@ def fail_code_sandbox_run(self, node_id: str, message: str) -> SceneNode | None: awaiting_approval flag is ALWAYS cleared here too, unconditionally, so a denied/cancelled approval never leaves the node stuck showing the approval prompt forever (stale-while-revalidate: existing - code/output/analysis survive untouched). Not kind-guarded - see - complete_code_sandbox_run's own comment.""" - node = self.nodes.get(node_id) + code/output/analysis survive untouched). Same kind-check posture as + complete_code_sandbox_run - see its docstring.""" + node = optional_node(self.nodes, node_id, "code_sandbox", CodeSandboxState) if node is None: return None node.state.code_sandbox_awaiting_approval = False diff --git a/backend/domain/nodes_content.py b/backend/domain/nodes_content.py index db4ee48..e7fde51 100644 --- a/backend/domain/nodes_content.py +++ b/backend/domain/nodes_content.py @@ -20,6 +20,7 @@ SceneError, SceneNode, ) +from backend.domain.node_access import require_node from backend.domain.node_states import DocumentState, HtmlState, NoteState @@ -189,11 +190,7 @@ def set_html_splitter_state(self, node_id: str, value: float) -> None: position. html kind only (SceneError otherwise), matching every other kind-specific setter's guard pattern in this file (e.g. resize_chart/toggle_frame_lock).""" - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") - if node.kind != "html": - raise SceneError(f"node is not an html node: {node_id}") + node = require_node(self.nodes, node_id, "html", HtmlState) node.state.html_splitter_state = float(value) # -- R6.1: notes ----------------------------------------------------------- diff --git a/backend/domain/nodes_conversational.py b/backend/domain/nodes_conversational.py index ea4e4cd..c2ba406 100644 --- a/backend/domain/nodes_conversational.py +++ b/backend/domain/nodes_conversational.py @@ -17,6 +17,7 @@ from backend.domain._composed import SceneDocumentParts from backend.domain.model import CHAT_TITLE_PREVIEW_LENGTH, SceneError, SceneNode +from backend.domain.node_access import require_node from backend.domain.node_states import ChatState @@ -114,11 +115,7 @@ def set_chat_scroll_value(self, node_id: str, value: float) -> None: content area. chat kind only (SceneError otherwise), matching every other kind-specific setter's guard pattern in this file (e.g. resize_chart/toggle_frame_lock).""" - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") - if node.kind != "chat": - raise SceneError(f"node is not a chat node: {node_id}") + node = require_node(self.nodes, node_id, "chat", ChatState) node.state.chat_scroll_value = float(value) # -- conversation node (a full thread in a single node) ------------------ diff --git a/backend/domain/nodes_visual.py b/backend/domain/nodes_visual.py index db40611..91ec8bf 100644 --- a/backend/domain/nodes_visual.py +++ b/backend/domain/nodes_visual.py @@ -27,6 +27,7 @@ SceneError, SceneNode, ) +from backend.domain.node_access import require_node from backend.domain.node_states import ChartState, ImageState @@ -134,11 +135,7 @@ def resize_chart(self, node_id: str, width: float, height: float) -> None: ADR-013 stage 13.4: no longer re-renders a PNG here - see add_chart_node's own docstring for why.""" - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") - if node.kind != "chart": - raise SceneError(f"node is not a chart node: {node_id}") + node = require_node(self.nodes, node_id, "chart", ChartState) requested_width = float(width) requested_height = float(height) @@ -166,11 +163,7 @@ def resize_chart(self, node_id: str, width: float, height: float) -> None: def toggle_chart_aspect_lock(self, node_id: str) -> None: """Chart kind only (SceneError otherwise). Flips chart_aspect_locked.""" - node = self.nodes.get(node_id) - if node is None: - raise SceneError(f"unknown node: {node_id}") - if node.kind != "chart": - raise SceneError(f"node is not a chart node: {node_id}") + node = require_node(self.nodes, node_id, "chart", ChartState) node.state.chart_aspect_locked = not node.state.chart_aspect_locked def add_image_node( diff --git a/backend/tests/test_wrong_kind_node_guards.py b/backend/tests/test_wrong_kind_node_guards.py new file mode 100644 index 0000000..728deef --- /dev/null +++ b/backend/tests/test_wrong_kind_node_guards.py @@ -0,0 +1,94 @@ +"""Every per-kind run method rejects a node of the wrong kind. + +Node states are plain, non-slotted dataclasses. Writing +`node.state.research_stage = "completed"` onto a chat node therefore does +not fail - it grafts a phantom attribute onto ChatState and returns +happily, and the caller has no way to tell. That is the failure mode these +guards exist to stop, and it is invisible unless something asserts on it, +because no live call site passes the wrong kind today. + +Two groups are covered here: + + * the eight methods that gained a kind check when backend/domain/'s + per-kind mixins were extracted and narrowed. Each had one already for + its `add_*`/`start_*`/`set_*` siblings; the completion and failure + paths did not, on the reasoning (written into two of the docstrings) + that the id had been validated earlier in the same request. True, and + still true - the guard is redundant on every live path. It is here for + the path nobody has written yet. + + * complete_gitlink_run and complete_gitlink_apply, which gained the same + check in PR #409 without anything pinning it. + +`fail_*` methods return None for a wrong-kind node rather than raising: +they are documented as silent when their node has gone, and a node of +another kind is the same situation from a finished run's point of view. +Everything else raises SceneError. +""" + +from __future__ import annotations + +import pytest + +from backend.domain.model import SceneError +from backend.canvas import SceneDocument + + +def _chat_id(doc: SceneDocument) -> str: + """A node id that is definitely not any of the kinds under test.""" + return doc.add_chat_node(0.0, 0.0, "hello", True).id + + +# (method name, positional args after node_id, keyword args) +RAISES = [ + ("complete_web_research_run", ({"summary": "s"},), {}), + ("fail_web_research_run", (), {"cancelled": False, "message": "boom"}), + ("append_artifact_user_message", ("write it again",), {}), + ("complete_artifact_generation", ("# doc", "done"), {}), + ("complete_gitlink_run", ("## proposal", [], "", None, ""), {}), + ("complete_gitlink_apply", (2,), {}), +] + +RETURNS_NONE = [ + ("apply_web_research_progress", (object(),), {}), + ("fail_artifact_generation", ("boom",), {}), + ("complete_code_sandbox_run", ("code", "out", "analysis"), {}), + ("fail_code_sandbox_run", ("boom",), {}), +] + + +@pytest.mark.parametrize( + "method, args, kwargs", RAISES, ids=[m for m, _, _ in RAISES], +) +def test_a_wrong_kind_node_is_rejected(method, args, kwargs): + doc = SceneDocument() + node_id = _chat_id(doc) + with pytest.raises(SceneError): + getattr(doc, method)(node_id, *args, **kwargs) + + +@pytest.mark.parametrize( + "method, args, kwargs", RETURNS_NONE, ids=[m for m, _, _ in RETURNS_NONE], +) +def test_a_wrong_kind_node_is_a_quiet_no_op(method, args, kwargs): + doc = SceneDocument() + node_id = _chat_id(doc) + assert getattr(doc, method)(node_id, *args, **kwargs) is None + + +@pytest.mark.parametrize( + "method, args, kwargs", + RAISES + RETURNS_NONE, + ids=[m for m, _, _ in RAISES + RETURNS_NONE], +) +def test_the_wrong_kind_node_is_left_untouched(method, args, kwargs): + """The point of the guard: no phantom per-kind field is grafted onto a + state class that never declared one.""" + doc = SceneDocument() + node_id = _chat_id(doc) + before = dict(vars(doc.nodes[node_id].state)) + try: + getattr(doc, method)(node_id, *args, **kwargs) + except SceneError: + pass + assert vars(doc.nodes[node_id].state) == before diff --git a/pyproject.toml b/pyproject.toml index ed1da66..7488174 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -370,6 +370,26 @@ files = [ "backend/domain/node_access.py", "backend/domain/nodes_code_review.py", "backend/domain/nodes_gitlink.py", + # 2026-09-04, third widening: the rest of backend/domain/'s per-kind + # groups, once the last thirteen kinds were extracted out of + # SceneDocument the same way, plus the four leaf modules they sit on. + # + # 13 of the 18 modules in backend/domain/ are now checked. The five that + # are not are graph.py itself and the four cross-cutting mixins + # (groups/branches/commands/layout), which hold 124 errors between them + # of the same union-attr shape - they read per-kind state off nodes they + # look up generically, so require_node cannot narrow them without a kind + # to narrow to. That is the next piece of work, not a gap being waved + # through. + "backend/domain/_composed.py", + "backend/domain/content_codec.py", + "backend/domain/model.py", + "backend/domain/node_states.py", + "backend/domain/nodes_agent_runs.py", + "backend/domain/nodes_content.py", + "backend/domain/nodes_conversational.py", + "backend/domain/nodes_planning.py", + "backend/domain/nodes_visual.py", # 2026-09-04: the first widening since this list was written, and the # ratchet moving the direction its own comment asks for. #