Skip to content

Type the persistence layer, and guard what it writes #414

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

Type the persistence layer, and guard what it writes #414

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
25 changes: 25 additions & 0 deletions backend/domain/node_access.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,31 @@ def optional_node(
return node # type: ignore[return-value]


def with_state(node: SceneNode, state_cls: type[_S]) -> "_NodeWith[_S]":
"""A node whose kind has already been decided, narrowed by its state.

require_node's form for code that is HANDED a node instead of looking one
up. backend/session_save.py's per-kind serializers are the case this
exists for: each is reached through a kind-keyed dispatch table, so the
kind is settled before the function is entered - what the function needs
is not another kind check but a way to say which state it is about to
read, in a form a checker can follow.

Unlike is_node_of this returns the node rather than a bool, because these
callers have no wrong-kind branch to take: a serializer handed the wrong
node cannot produce a correct payload, and writing a half-built one into
a save file is worse than failing. The isinstance check is redundant on
every live path for the same reason require_node's is - and, exactly like
require_node's, it is the difference between a clear error and a
confusing one if a future dispatch table ever disagrees with itself.
"""
if not isinstance(node.state, state_cls):
raise SceneError(
f"node {node.id} is {node.kind} but has no {state_cls.__name__}"
)
return node # type: ignore[return-value]


def is_node_of(
node: SceneNode | None, kind: str | tuple[str, ...], state_cls: type[_S],
) -> TypeGuard["_NodeWith[_S]"]:
Expand Down
64 changes: 42 additions & 22 deletions backend/session_load.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,10 @@
_content_codec,
_placeholder_chart_data,
)
from backend.domain.node_access import with_state
# Not re-exported by backend.canvas, unlike the kinds above - imported
# from the domain package directly, as session_save.py does.
from backend.domain.node_states import ChartState, FrameState, NoteState
from backend.plugin_sdk import NodeKindSpec, PluginRegistry, discover_plugins
from graphlink_chart_data import ChartDataError, canonicalize_chart_data
from graphlink_navigation_pins import NavigationPinRecord
Expand Down Expand Up @@ -469,7 +473,7 @@ def _restore_chat_payload(payload: dict[str, Any]) -> SceneNode:
is_branch_synthesis=bool(payload.get("is_branch_synthesis", False)),
synthesis_instructions=str(payload.get("synthesis_instructions", "") or ""),
branch_status=(
payload.get("branch_status")
str(payload.get("branch_status"))
if payload.get("branch_status") in SceneDocument.BRANCH_STATUS_VALUES
else "active"
),
Expand Down Expand Up @@ -538,7 +542,10 @@ def _restore_image_payload(payload: dict[str, Any], document: SceneDocument) ->
import uuid as _uuid

x, y = _position(payload)
node = SceneNode(id="", x=x, y=y, title="Image", kind="image", state=ImageState())
node = with_state(
SceneNode(id="", x=x, y=y, title="Image", kind="image", state=ImageState()),
ImageState,
)
asset_store = _ACTIVE_ASSET_STORE.get()

# ADR-009 stage 9.5: READ BOTH SHAPES. A chat saved with an asset store
Expand Down Expand Up @@ -616,16 +623,19 @@ def _restore_web_payload(payload: dict[str, Any]) -> SceneNode:
# R6.4 translation: legacy node_type "web" -> backend kind
# "web_research" (confirmed distinct strings, not a typo).
x, y = _position(payload)
node = SceneNode(
id="", x=x, y=y, title="Web Research", kind="web_research",
content=str(payload.get("query", "")),
history=_restore_history(payload.get("conversation_history")),
is_collapsed=bool(payload.get("is_collapsed", False)),
state=WebResearchState(
# ADR-021 stage 21.5: absent in every pre-21.5 row, which is
# exactly the False default Web Research has always behaved as.
research_retain_to_knowledge=bool(payload.get("retain_to_knowledge", False)),
node = with_state(
SceneNode(
id="", x=x, y=y, title="Web Research", kind="web_research",
content=str(payload.get("query", "")),
history=_restore_history(payload.get("conversation_history")),
is_collapsed=bool(payload.get("is_collapsed", False)),
state=WebResearchState(
# ADR-021 stage 21.5: absent in every pre-21.5 row, which is
# exactly the False default Web Research has always behaved as.
research_retain_to_knowledge=bool(payload.get("retain_to_knowledge", False)),
),
),
WebResearchState,
)
research_result = payload.get("research_result")
if isinstance(research_result, dict) and research_result:
Expand Down Expand Up @@ -870,7 +880,7 @@ def _restore_plan_payload(payload: dict[str, Any]) -> SceneNode:
goal = str(payload.get("goal", ""))
raw_status = str(payload.get("builder_status", "draft") or "draft")
status = raw_status if raw_status in _BUILDER_TERMINAL_STATUSES + ("draft",) else "interrupted"
steps = []
steps: list[dict[str, Any]] = []
for raw in payload.get("steps") or []:
if isinstance(raw, dict) and raw.get("title"):
steps.append({
Expand Down Expand Up @@ -1240,10 +1250,13 @@ def _restore_notes(document: SceneDocument, notes_data: list) -> dict[int, str]:
continue
try:
x, y = _position(note_payload)
note = document.add_note(
x, y,
is_system_prompt=bool(note_payload.get("is_system_prompt", False)),
is_summary_note=bool(note_payload.get("is_summary_note", False)),
note = with_state(
document.add_note(
x, y,
is_system_prompt=bool(note_payload.get("is_system_prompt", False)),
is_summary_note=bool(note_payload.get("is_summary_note", False)),
),
NoteState,
)
document.set_note_content(note.id, str(note_payload.get("content", "")))
# Rows saved before the forced-default fix carry
Expand Down Expand Up @@ -1308,7 +1321,12 @@ def _restore_charts(
chart_payload, "parent_node_id", "parent_node_index", nodes_by_id, all_nodes_map,
)
x, y = _position(chart_payload)
chart = document.add_chart_node(x, y, parent_id, chart_type, chart_data, chart_error=chart_error)
chart = with_state(
document.add_chart_node(
x, y, parent_id, chart_type, chart_data, chart_error=chart_error,
),
ChartState,
)
# Aspect-lock MUST be applied before any resize: resize_chart's
# own aspect-preserving re-derivation reads chart_aspect_locked
# at call time, and a freshly-created chart always starts locked
Expand Down Expand Up @@ -1348,7 +1366,7 @@ def _restore_frames(
member_ids = [frame_source_map[i] for i in item_indices if i in frame_source_map]
if not member_ids:
continue
frame = document.create_frame(member_ids)
frame = with_state(document.create_frame(member_ids), FrameState)
document.set_group_label(frame.id, str(frame_payload.get("note", "") or ""))
document.set_group_color(frame.id, frame_payload.get("color"), frame_payload.get("header_color"))
if bool(frame_payload.get("is_locked", True)) != frame.state.is_locked:
Expand Down Expand Up @@ -1681,7 +1699,7 @@ def _restore_branch_provenance_item_ids(
def _restore_pins(document: SceneDocument, pins_data: list) -> None:
if not isinstance(pins_data, list) or not pins_data:
return
records = []
records: list[NavigationPinRecord] = []
for pin_payload in pins_data:
if not isinstance(pin_payload, dict):
continue
Expand Down Expand Up @@ -1863,12 +1881,14 @@ def _restore_chat_into_document(
by_payload_id = dict(nodes_by_id)
if isinstance(notes_data, list):
for note_index, note_payload in enumerate(notes_data):
note_new_id = notes_map.get(note_index)
if not isinstance(note_payload, dict) or note_new_id is None:
# Not `note_new_id`: that name is bound as a plain str by the
# loop over notes_map.items() earlier in this function.
mapped_note_id = notes_map.get(note_index)
if not isinstance(note_payload, dict) or mapped_note_id is None:
continue
note_payload_id = note_payload.get("id")
if note_payload_id:
by_payload_id[str(note_payload_id)] = note_new_id
by_payload_id[str(note_payload_id)] = mapped_note_id
by_payload_id.update(charts_by_id)

if not _restore_flat_edges(document, chat_data, by_payload_id):
Expand Down
67 changes: 51 additions & 16 deletions backend/session_save.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,25 @@
from typing import Any

from backend.canvas import SceneDocument, SceneNode, _content_codec
from backend.domain.node_access import with_state
from backend.domain.node_states import (
ArtifactState,
ChartState,
ChatState,
CodeReviewState,
CodeSandboxState,
CodeState,
ContainerState,
DocumentState,
FrameState,
GitlinkState,
HarnessState,
HtmlState,
ImageState,
NoteState,
PlanState,
WebResearchState,
)
from backend.plugin_sdk import NodeKindSpec, PluginRegistry, discover_plugins
from graphlink_settings_store import SettingsManager

Expand Down Expand Up @@ -217,7 +236,8 @@ def _serialize_history(history: list[dict[str, Any]]) -> list[dict[str, Any]]:
# _classify_edges) - these functions never reference `document` or other
# nodes at all, mirroring session_load.py's equivalent restorers.

def _serialize_chat_node(node: SceneNode) -> dict[str, Any]:
def _serialize_chat_node(raw_node: SceneNode) -> dict[str, Any]:
node = with_state(raw_node, ChatState)
if node.state.content_parts is not None:
raw_content = _content_codec.process_content_for_serialization(node.state.content_parts)
else:
Expand Down Expand Up @@ -269,11 +289,13 @@ def _serialize_chat_node(node: SceneNode) -> dict[str, Any]:
}


def _serialize_code_node(node: SceneNode) -> dict[str, Any]:
def _serialize_code_node(raw_node: SceneNode) -> dict[str, Any]:
node = with_state(raw_node, CodeState)
return {"node_type": "code", "code": node.state.code, "language": node.state.language}


def _serialize_document_node(node: SceneNode) -> dict[str, Any]:
def _serialize_document_node(raw_node: SceneNode) -> dict[str, Any]:
node = with_state(raw_node, DocumentState)
return {
"node_type": "document",
"title": node.title,
Expand All @@ -290,7 +312,7 @@ def _serialize_document_node(node: SceneNode) -> dict[str, Any]:


def _serialize_image_node(
node: SceneNode, document: SceneDocument, asset_store: Any | None = None
raw_node: SceneNode, document: SceneDocument, asset_store: Any | None = None
) -> dict[str, Any]:
"""ADR-009 stage 9.5: writes the image's bytes to the content-addressed
asset store when one is supplied, emitting only a ref - so autosave
Expand All @@ -303,6 +325,7 @@ def _serialize_image_node(
shape, so a chat saved by an older build keeps loading untouched and no
row ever has to be rewritten to make this safe. The inline path is
what a future cleanup deletes, once no old rows remain in the wild."""
node = with_state(raw_node, ImageState)
asset = document.image_assets.get(node.state.image_asset_id)
image_bytes = asset[0] if asset is not None else b""
mime_type = asset[1] if asset is not None else "image/png"
Expand Down Expand Up @@ -347,7 +370,8 @@ def _serialize_conversation_node(node: SceneNode) -> dict[str, Any]:
}


def _serialize_html_node(node: SceneNode) -> dict[str, Any]:
def _serialize_html_node(raw_node: SceneNode) -> dict[str, Any]:
node = with_state(raw_node, HtmlState)
return {
"node_type": "html",
"html_content": node.content,
Expand All @@ -357,7 +381,8 @@ def _serialize_html_node(node: SceneNode) -> dict[str, Any]:
}


def _serialize_web_node(node: SceneNode) -> dict[str, Any]:
def _serialize_web_node(raw_node: SceneNode) -> dict[str, Any]:
node = with_state(raw_node, WebResearchState)
research_result = _camel_to_snake_deep(node.state.research_result) if node.state.research_result else {}
return {
# R6.5 translation (inverse of R6.4's own): backend kind
Expand All @@ -374,7 +399,8 @@ def _serialize_web_node(node: SceneNode) -> dict[str, Any]:
}


def _serialize_artifact_node(node: SceneNode) -> dict[str, Any]:
def _serialize_artifact_node(raw_node: SceneNode) -> dict[str, Any]:
node = with_state(raw_node, ArtifactState)
return {
"node_type": "artifact",
"instruction": node.content,
Expand All @@ -384,7 +410,8 @@ def _serialize_artifact_node(node: SceneNode) -> dict[str, Any]:
}


def _serialize_gitlink_node(node: SceneNode) -> dict[str, Any]:
def _serialize_gitlink_node(raw_node: SceneNode) -> dict[str, Any]:
node = with_state(raw_node, GitlinkState)
return {
"node_type": "gitlink",
"task_prompt": node.state.gitlink_task_prompt,
Expand All @@ -408,12 +435,13 @@ def _serialize_gitlink_node(node: SceneNode) -> dict[str, Any]:
}


def _serialize_code_review_node(node: SceneNode) -> dict[str, Any]:
def _serialize_code_review_node(raw_node: SceneNode) -> dict[str, Any]:
# NOTE (ADR-002 stage 2.5 gate): every field below is read as
# node.state.<field>, never via a `state = node.state` alias -
# tests/test_node_state_migration.py's bare-attribute ban only
# recognizes the `X.state.<field>` shape, so an alias would fail the
# build (the _serialize_gitlink_node precedent reads the same way).
node = with_state(raw_node, CodeReviewState)
return {
"node_type": "code_review",
"pr_url": node.state.code_review_pr_url,
Expand Down Expand Up @@ -456,7 +484,8 @@ def _serialize_code_review_node(node: SceneNode) -> dict[str, Any]:
}


def _serialize_code_sandbox_node(node: SceneNode) -> dict[str, Any]:
def _serialize_code_sandbox_node(raw_node: SceneNode) -> dict[str, Any]:
node = with_state(raw_node, CodeSandboxState)
return {
"node_type": "code_sandbox",
"prompt": node.state.code_sandbox_prompt,
Expand All @@ -470,14 +499,15 @@ def _serialize_code_sandbox_node(node: SceneNode) -> dict[str, Any]:
}


def _serialize_plan_node(node: SceneNode) -> dict[str, Any]:
def _serialize_plan_node(raw_node: SceneNode) -> dict[str, Any]:
"""ADR-008 stage 8.3: the Builder plan node. NEW-app-only kind (the
legacy app never had a Builder) - a legacy load silently skips it, the
same documented tolerant behavior every post-legacy kind gets. The
LIVE-run fields (awaiting approval + its summary) are deliberately NOT
persisted: they describe a RunHandle that cannot survive a restart;
session_load's restorer likewise normalizes a non-terminal
builder_status to "interrupted" (see PlanState's own docstring)."""
node = with_state(raw_node, PlanState)
return {
"node_type": "plan",
"goal": node.state.plan_goal,
Expand All @@ -502,14 +532,15 @@ def _serialize_plan_node(node: SceneNode) -> dict[str, Any]:
}


def _serialize_harness_node(node: SceneNode) -> dict[str, Any]:
def _serialize_harness_node(raw_node: SceneNode) -> dict[str, Any]:
"""PLAN-2026-08-24 H1: the harness node. NEW-app-only kind, same
tolerant legacy-skip posture as the plan node. Deliberately small:
conversation history lives in the workspace transcript, not here (see
HarnessState's own docstring) - what persists is the render surface
plus the two durable identities (workspace id, last run id).
session_load normalizes a non-terminal harness_status to
"interrupted", the exact PlanState treatment."""
node = with_state(raw_node, HarnessState)
return {
"node_type": "harness",
"goal": node.state.harness_goal,
Expand Down Expand Up @@ -637,7 +668,8 @@ def _serialize_plugin_node(
}


def _serialize_note(node: SceneNode) -> dict[str, Any]:
def _serialize_note(raw_node: SceneNode) -> dict[str, Any]:
node = with_state(raw_node, NoteState)
return {
"id": node.id,
"content": node.content,
Expand Down Expand Up @@ -684,7 +716,8 @@ def _serialize_pin(record) -> dict[str, Any]:
}


def _serialize_frame(node: SceneNode, frame_source_index: dict[str, int]) -> dict[str, Any]:
def _serialize_frame(raw_node: SceneNode, frame_source_index: dict[str, int]) -> dict[str, Any]:
node = with_state(raw_node, FrameState)
item_indices = [frame_source_index[i] for i in node.item_ids if i in frame_source_index]
# Technical-debt audit finding: group_width/group_height is the frame's
# CURRENT effective size, and reading it unconditionally is CORRECT
Expand Down Expand Up @@ -735,7 +768,8 @@ def _serialize_frame(node: SceneNode, frame_source_index: dict[str, int]) -> dic
}


def _serialize_container(node: SceneNode, all_items_index: dict[str, int]) -> dict[str, Any]:
def _serialize_container(raw_node: SceneNode, all_items_index: dict[str, int]) -> dict[str, Any]:
node = with_state(raw_node, ContainerState)
item_indices = [all_items_index[i] for i in node.item_ids if i in all_items_index]
width = node.state.group_width if node.state.group_width is not None else 0.0
height = node.state.group_height if node.state.group_height is not None else 0.0
Expand All @@ -754,8 +788,9 @@ def _serialize_container(node: SceneNode, all_items_index: dict[str, int]) -> di


def _serialize_chart(
node: SceneNode, nodes_index: dict[str, int], parent_id: str | None,
raw_node: SceneNode, nodes_index: dict[str, int], parent_id: str | None,
) -> dict[str, Any]:
node = with_state(raw_node, ChartState)
parent_index = nodes_index.get(parent_id) if parent_id is not None else None
return {
"id": node.id,
Expand Down
Loading
Loading