diff --git a/backend/domain/model.py b/backend/domain/model.py index 78bdb1ce..9abf61da 100644 --- a/backend/domain/model.py +++ b/backend/domain/model.py @@ -155,13 +155,25 @@ class SceneNode: y: float title: str kind: str = "placeholder" - # R3.1 (doc/QT_REMOVAL_PLAN.md): the chat node's real persisted shape - - # graphlink_session/serializers.py's raw_content/is_user/is_collapsed, - # minus everything Qt-only (paint state, scroll position, docked-child - # widgets). Unused (default) for every other kind. - # R3.17: also reused verbatim as the html node's raw HTML source string - - # no separate field, same reuse pattern as R3.5's code text and R3.13's - # thinking text living in this same field. + # The node's single free-text body. Originally R3.1 + # (doc/QT_REMOVAL_PLAN.md), the chat node's persisted shape - + # graphlink_session/serializers.py's raw_content, minus everything + # Qt-only (paint state, scroll position, docked-child widgets). + # + # It is NOT chat-only and has not been for a long time. This comment used + # to end "Unused (default) for every other kind" while a later line in the + # same block already contradicted it, and the list has kept growing since. + # Verified 2026-09-04 against the SceneNode(...) constructions in + # backend/domain/graph.py and backend/session_load.py, TEN kinds populate + # it: artifact, chat, document, harness, html, image, note, plan, + # thinking, web_research. Pinned by tests/test_shared_node_field_docs.py, + # so this list fails the build rather than rotting again. + # + # That shared use is deliberate, and it is why `content` did not move to a + # per-kind class in the ADR-002 stage 2.5 migration: a field ten kinds + # write would have to be duplicated across ten state classes to live + # there, which is worse than one core field. Treat it as core, like + # title - not as a kind-specific leftover. content: str = "" is_collapsed: bool = False # R3.13 (doc/QT_REMOVAL_PLAN.md): the ThinkingNode/docked-child increment - @@ -170,14 +182,24 @@ class SceneNode: # time from this flag (scan nodes whose parent edge points at it), never # stored on the parent itself. Unused (default) for every other kind. is_docked: bool = False - # R3.25 (doc/QT_REMOVAL_PLAN.md): the ConversationNode's real persisted - # shape - graphlink_conversation_node.py's conversation_history, a - # growing list of {"role": "user"|"assistant", "content": text} dicts - # rendered as multiple bubbles inside one node card. This is the one R3 - # kind whose OWN field is a LIST rather than a scalar - every prior kind - # (chat/code/document/thinking/html/image) stores one scalar value per - # node; a conversation node instead owns a whole message history. Unused - # (default empty list) for every other kind. + # A node's own message history: a growing list of + # {"role": "user"|"assistant", "content": text} dicts. Originally R3.25 + # (doc/QT_REMOVAL_PLAN.md), the ConversationNode's persisted shape, ported + # from graphlink_conversation_node.py's conversation_history. + # + # It is NOT conversation-only and has not been since the plugin kinds + # landed. This comment used to say "Unused (default empty list) for every + # other kind"; verified 2026-09-04 against the restorers in + # backend/domain/graph.py and backend/session_load.py, SEVEN kinds + # populate it: artifact, chat, code_sandbox, conversation, gitlink, html, + # web_research. Any kind that holds a back-and-forth with a model keeps it + # here. Pinned by tests/test_shared_node_field_docs.py. + # + # Shared for the same reason `content` is, and left on SceneNode by the + # ADR-002 stage 2.5 migration for the same reason - see that field's own + # note above. backend/domain/node_states.py's docstring is right to list + # `conversation` among the kinds with no state class: its history is not a + # kind-specific field, it is this shared one. history: list[dict[str, str]] = field(default_factory=list) # R4.3 (doc/QT_REMOVAL_PLAN.md): transient per-node in-flight-request # marker - the id of the AgentDispatcher request currently generating a diff --git a/tests/test_shared_node_field_docs.py b/tests/test_shared_node_field_docs.py new file mode 100644 index 00000000..b27b0a96 --- /dev/null +++ b/tests/test_shared_node_field_docs.py @@ -0,0 +1,83 @@ +"""`content` and `history` are SHARED fields; their comments have to say so. + +Both live on SceneNode rather than in a per-kind class in +backend/domain/node_states.py, and both were documented as belonging to one +kind - `content` to chat, `history` to conversation - with an explicit +"Unused for every other kind" line. Neither was true any more. A 2026-09-04 +audit measured ten kinds writing `content` and seven writing `history`, +and in `content`'s case a later line in the SAME comment block already +contradicted the earlier one. + +That is a recurring shape in this codebase rather than a one-off: a comment +asserts a closed set, the set grows, and nothing fails. The same audit found +backend/api/_shared.py naming its two callers by hand and being wrong within +one commit of being written. This gate is the cheap answer for the two cases +where the claim is load-bearing - it is the reason these fields were left on +SceneNode, so a reader deciding whether to "finish the migration" needs the +list to be right. + +Derived from the kinds that actually construct a SceneNode with each field, +across the two modules that create nodes: backend/domain/graph.py (live +creation) and backend/session_load.py (restore from a saved chat). + +Same posture as tests/undo_classification.py: a hand-authored expectation +plus a gate that fails when the code and the expectation diverge. If this +fails because a new kind legitimately started using the field, update BOTH +the set below and the field's comment in backend/domain/model.py - that +pairing is the whole point. +""" + +from __future__ import annotations + +import ast +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[1] +NODE_CREATING_MODULES = ( + REPO_ROOT / "backend" / "domain" / "graph.py", + REPO_ROOT / "backend" / "session_load.py", +) + +# Hand-authored, and deliberately duplicated in backend/domain/model.py's own +# field comments. The gate exists to keep the two copies honest. +CONTENT_KINDS = { + "artifact", "chat", "document", "harness", "html", + "image", "note", "plan", "thinking", "web_research", +} +HISTORY_KINDS = {"artifact", "chat", "code_sandbox", "conversation", "gitlink", "html", "web_research"} + + +def _kinds_constructing_with(field_name: str) -> set[str]: + """Kinds passed to a SceneNode(...) call that also passes `field_name`.""" + kinds: set[str] = set() + for path in NODE_CREATING_MODULES: + tree = ast.parse(path.read_text(encoding="utf-8")) + for node in ast.walk(tree): + if not isinstance(node, ast.Call): + continue + func = node.func + name = func.id if isinstance(func, ast.Name) else getattr(func, "attr", None) + if name != "SceneNode": + continue + passed = {kw.arg for kw in node.keywords if kw.arg} + if field_name not in passed: + continue + for kw in node.keywords: + if kw.arg == "kind" and isinstance(kw.value, ast.Constant): + kinds.add(str(kw.value.value)) + return kinds + + +def test_content_is_written_by_the_kinds_its_comment_names(): + assert _kinds_constructing_with("content") == CONTENT_KINDS + + +def test_history_is_written_by_the_kinds_its_comment_names(): + assert _kinds_constructing_with("history") == HISTORY_KINDS + + +def test_the_scan_finds_a_sane_population(): + """Guards the guard: a renamed class or a changed construction style would + make both checks above compare two empty sets and pass.""" + assert len(_kinds_constructing_with("content")) >= 5 + assert len(_kinds_constructing_with("history")) >= 5