Skip to content

Narrow the four cross-cutting domain mixins, and put mypy on the package #412

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dovvnloading merged 1 commit into from
Sep 4, 2026
Merged

Narrow the four cross-cutting domain mixins, and put mypy on the package #412

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions backend/domain/_composed.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,15 @@ class SceneDocumentParts:
pins: Any
_counter: itertools.count

# Scalars held on SceneDocument that the mixins both read and
# write. Without these BranchOps infers `str` from its own first
# assignment and then rejects the `= None` two lines later, which
# is how a field declared `str | None` on SceneDocument itself
# produced an error on SceneDocument's own declaration.
last_chat_node_id: str | None
final_deliverable_node_id: str | None
current_chat_id: int | None

# The undo/redo command layer (CommandOps' own, consumed by siblings).
command_log: deque
redo_stack: list
Expand Down
67 changes: 19 additions & 48 deletions backend/domain/branches.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@
if TYPE_CHECKING:
from graphlink_model_catalog import ModelRef

from backend.domain.node_access import require_node
from backend.domain.node_states import ChatState, NoteState

from backend.domain._composed import SceneDocumentParts


Expand All @@ -43,11 +46,7 @@ def mark_branch_comparison_note(self, node_id: str, source_node_ids: list[str])
set_note_content, mirroring set_group_color's own "extra setter call
right after creation" shape (see the WS intent wrapper in
register_canvas)."""
node = self.nodes.get(node_id)
if node is None:
raise SceneError(f"unknown node: {node_id}")
if node.kind != "note":
raise SceneError(f"node is not a note node: {node_id}")
node = require_node(self.nodes, node_id, "note", NoteState)
node.state.is_branch_comparison = True
node.item_ids = list(source_node_ids)

Expand All @@ -66,11 +65,7 @@ def mark_branch_synthesis(
of a note-kind one (see ChatState's own comment, backend/domain/
node_states.py, for why this is a distinct method/flag rather than
reusing Compare Branches')."""
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.is_branch_synthesis = True
node.item_ids = list(source_node_ids)
node.state.synthesis_instructions = str(instructions)
Expand All @@ -96,11 +91,7 @@ def set_branch_status(self, node_id: str, status: str) -> None:
already established that 2+ branches can be simultaneously
legitimate (its own item_ids records multiple sources at once) -
forcing exclusivity here would fight that existing workflow."""
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)
status = str(status)
if status not in self.BRANCH_STATUS_VALUES:
raise SceneError(f"invalid branch status: {status}")
Expand All @@ -115,11 +106,9 @@ def set_final_deliverable(self, node_id: str, is_final: bool) -> None:
branch_status on purpose - no validation ties them together (a
"rejected" node CAN technically be marked Final Deliverable; this
is not blocked, though not a realistic path either)."""
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}")
# Validation only - the node itself is not needed, just the two
# SceneErrors this raises for a missing or non-chat id.
require_node(self.nodes, node_id, "chat", ChatState)
if is_final:
self.final_deliverable_node_id = node_id
elif self.final_deliverable_node_id == node_id:
Expand All @@ -131,11 +120,7 @@ def set_model_override(self, node_id: str, provider: str, model_id: str) -> None
resolve_model_for_node's own docstring) resolves to. Both fields
write together, mirroring set_group_color's own "no partial value"
posture - a pin is a real (provider, model_id) pair or nothing."""
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)
provider = str(provider or "").strip()
model_id = str(model_id or "").strip()
if not provider or not model_id:
Expand All @@ -144,11 +129,7 @@ def set_model_override(self, node_id: str, provider: str, model_id: str) -> None
node.state.override_model_id = model_id

def clear_model_override(self, node_id: str) -> None:
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.override_provider = ""
node.state.override_model_id = ""

Expand All @@ -168,11 +149,7 @@ def set_chat_index_into_knowledge(self, node_id: str, enabled: bool) -> None:
intents own the side effects" separation (chat_library persistence
is owned by intents_chat_library.py, never by graph.py/branches.py
directly)."""
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.index_into_knowledge = bool(enabled)

def resolve_model_for_node(
Expand Down Expand Up @@ -349,9 +326,11 @@ def delete_chat_node(self, node_id: str) -> None:
for target in reconnect_targets:
self.connect(parent_id, target)

for edge in [parent_edge, *child_edges, *note_edges]:
if edge is not None:
self.edges.pop(edge.id, None)
# A distinct name from the `edge` above: this list also holds
# parent_edge, which may be None.
for stale_edge in [parent_edge, *child_edges, *note_edges]:
if stale_edge is not None:
self.edges.pop(stale_edge.id, None)

if self.last_chat_node_id == node_id:
# The active branch continues from wherever it now ends: the
Expand Down Expand Up @@ -515,11 +494,7 @@ def regenerate_response(self, node_id: str) -> tuple[SceneNode, str]:
SceneError; the WS-intent wrapper in register_canvas catches it and
shows ONE friendly notification for all three cases - see that wrapper
for why."""
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)
parent_edge = self._branch_parent_edge(node_id)
if parent_edge is None:
raise SceneError(f"node has no parent and cannot be regenerated: {node_id}")
Expand Down Expand Up @@ -616,10 +591,6 @@ def collapse_branch(self, node_id: str, collapsed: bool) -> None:
so a node that had previously been individually expanded/collapsed
differently loses that distinction the first time this runs - an
accepted, stated tradeoff, not solved here."""
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}")
require_node(self.nodes, node_id, "chat", ChatState) # validation only
for nid in self._chat_subtree_ids(node_id):
self.nodes[nid].is_collapsed = bool(collapsed)
21 changes: 13 additions & 8 deletions backend/domain/commands.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ def is_noop(self) -> bool:
self.node_before or self.node_after or self.edge_before or self.edge_after
) and (self.pin_before is None or self.pin_before == self.pin_after)

def invert(self, document: object) -> None:
def invert(self, document: SceneDocumentParts) -> None:
"""Restores document state to exactly how it was before this
command's mutator ran. Goes through the *_before snapshots only -
never re-invokes the original domain method, which is the whole
Expand All @@ -185,7 +185,7 @@ def invert(self, document: object) -> None:
# per-id scoping doesn't apply to pins the way it does nodes).
document.pins.reset(list(self.pin_before))

def apply(self, document: object) -> None:
def apply(self, document: SceneDocumentParts) -> None:
"""The mirror of invert() - restores to the *_after state. Not
needed for a first-time forward mutation (that already happened
for real before this Command was constructed); this exists for
Expand Down Expand Up @@ -283,13 +283,18 @@ def _restore(live: dict, snapshot: dict) -> None:
# only case where trusting the snapshot is correct. Mirrored
# below for HarnessState.harness_activity, same rationale.
current = live.get(key)
if isinstance(getattr(restored, "state", None), PlanState) and isinstance(
getattr(current, "state", None), PlanState,
):
# `current is not None` is redundant at runtime - getattr's own
# default already makes the isinstance fail - and is spelled out
# so the reads below are checkable. getattr stays because this
# helper also restores edges and image assets, which have no
# .state at all.
if current is not None and isinstance(
getattr(restored, "state", None), PlanState,
) and isinstance(getattr(current, "state", None), PlanState):
restored.state.builder_activity = current.state.builder_activity
if isinstance(getattr(restored, "state", None), HarnessState) and isinstance(
getattr(current, "state", None), HarnessState,
):
if current is not None and isinstance(
getattr(restored, "state", None), HarnessState,
) and isinstance(getattr(current, "state", None), HarnessState):
restored.state.harness_activity = current.state.harness_activity
live[key] = restored

Expand Down
9 changes: 7 additions & 2 deletions backend/domain/graph.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
SceneError,
SceneNode,
)
from backend.domain.node_access import is_node_of, require_node
from backend.domain.node_states import (
ArtifactState,
ChartState,
Expand Down Expand Up @@ -631,7 +632,11 @@ def move_node(self, node_id: str, x: float, y: float) -> SceneNode:
# the live bbox stay in agreement. See _recompute_group_bounds
# for how this anchor is unioned with live content so it still
# can never clip a member.
node.state.group_manual_x, node.state.group_manual_y = node.x, node.y
# Cannot raise: the node is present and its kind was just
# checked. Re-fetched under FrameState purely so the two
# frame-only writes below are checkable.
frame = require_node(self.nodes, node_id, "frame", FrameState)
frame.state.group_manual_x, frame.state.group_manual_y = node.x, node.y
self._recompute_group_bounds(node_id)
# R6.1: keep every frame/container this node is a member of enclosing
# it - a node is a member of at most one frame AND at most one
Expand Down Expand Up @@ -672,7 +677,7 @@ def move_nodes(self, positions: list[tuple[str, float, float]]) -> None:
continue
node.x, node.y = float(x), float(y)
moved_ids.add(node_id)
if node.kind == "frame":
if is_node_of(node, "frame", FrameState):
node.state.group_manual_x, node.state.group_manual_y = node.x, node.y
affected_groups: set[str] = set()
for moved_id in moved_ids:
Expand Down
Loading
Loading