From 64c7d19d807abf0e9170967d8cf8ef9425e5ccd6 Mon Sep 17 00:00:00 2001 From: dovvnloading Date: Sun, 16 Aug 2026 17:54:48 -0400 Subject: [PATCH 1/6] Add the Builder's activity log, completion notifications, and anchored output placement The Builder's tool-use loop had no durable record of what a build did: every tool call and its result vanished into the loop's in-memory message list. A build that paused or failed announced nothing unless the user happened to be looking at the canvas. New output (parentless node creations) landed near the canvas origin rather than near the build that produced it. This adds: - An activity log on the plan node (PlanState.builder_activity): one row per tool call the loop invokes, written at its single registry.invoke() choke point, so every call - including the four in-band control tools - is recorded in order with its outcome and timing. A capped ring buffer bounds its growth; it rides the existing whole-node scene patch mechanism, so no new wire plumbing was needed. It is deliberately untouched by undo: this is run telemetry, not document content, and the record of what happened must survive reverting what happened. Persisted with the rest of the plan node's state so it survives an app restart. - Notifications when a build lands done, paused, or failed (success/ warning/error respectively). "stopped" is excluded - the user just clicked Stop and is necessarily present. - Anchored placement: a build's parentless node creations (a note, a from-scratch chat or code node) now land near the plan node that launched them instead of scattering near the canvas origin, reusing the run context's existing plan-node reference rather than adding a new one. - builder/deleteRecipe, rounding out the recipe lifecycle - saveRecipe already existed with no way to remove what it saved. Every change here is confined to the loop, its wire contract, and session persistence; the loop's control flow, budget enforcement, and approval routing are unchanged. --- backend/api/intents_builder.py | 26 ++ backend/builder.py | 75 ++++- backend/domain/graph.py | 13 + backend/domain/node_states.py | 15 + backend/session_load.py | 11 + backend/session_save.py | 6 + backend/tests/test_builder.py | 257 +++++++++++++++++- backend/tools_graph.py | 59 +++- contracts/graphlink_scene_payload.py | 16 ++ tests/test_node_state_migration.py | 9 +- tests/test_undo_classification_gate.py | 9 +- tests/undo_classification.py | 1 + .../generated/scene-state.schema.json | 32 +++ .../lib/bridge-core/generated/scene-state.ts | 44 +++ 14 files changed, 542 insertions(+), 31 deletions(-) diff --git a/backend/api/intents_builder.py b/backend/api/intents_builder.py index 472de4f..ea385e0 100644 --- a/backend/api/intents_builder.py +++ b/backend/api/intents_builder.py @@ -152,6 +152,31 @@ async def save_recipe(node_id, name): await bus.publish("notification") return clean_name + async def delete_recipe(name): + """stage 8.7: rounds out the recipe lifecycle - saveRecipe already + creates one from a finished build, but nothing could ever remove + one. Mirrors save_recipe's own name-guard against built-ins and its + own settings.get_recipes()/set_recipes() replace-the-whole-list + posture.""" + from backend.builder import BUILT_IN_RECIPES + + clean_name = str(name or "").strip() + if any(r["name"] == clean_name for r in BUILT_IN_RECIPES): + notifications.show(f'"{clean_name}" is a built-in recipe - it cannot be deleted.', "warning") + await bus.publish("notification") + return False + settings = agent_dispatcher._settings_manager + existing = settings.get_recipes() + remaining = [r for r in existing if r["name"] != clean_name] + if len(remaining) == len(existing): + notifications.show(f'No saved recipe named "{clean_name}".', "info") + await bus.publish("notification") + return False + settings.set_recipes(remaining) + notifications.show(f'Deleted recipe "{clean_name}".', "info") + await bus.publish("notification") + return True + async def start_execution(node_id): node = document.nodes.get(node_id) if node is None or not isinstance(node.state, PlanState): @@ -202,4 +227,5 @@ async def set_plan_steps(node_id, steps): bus.register_intent("builder", "denyTool", deny_tool) bus.register_intent("builder", "listRecipes", list_recipes) bus.register_intent("builder", "saveRecipe", save_recipe) + bus.register_intent("builder", "deleteRecipe", delete_recipe) bus.register_intent("scene", "setPlanSteps", set_plan_steps) diff --git a/backend/builder.py b/backend/builder.py index 7276aff..ef4757f 100644 --- a/backend/builder.py +++ b/backend/builder.py @@ -74,6 +74,19 @@ _APPROVAL_SUMMARY_CAP = 400 +# stage 8.7: the activity log's own caps. A build is bounded at 50 steps x +# _STEP_TURN_CAP turns, so nothing else bounds how many rows a pathological +# replan loop could otherwise append to a plan node's wire dict (a whole- +# node diff per graph.py's take_dirty_patch_ops) - the ring buffer is the +# backstop. The summary cap is tighter than approval's own 400: approval +# summaries are shown one at a time, activity rows are shown many at once. +_ACTIVITY_CAP = 100 +_ACTIVITY_SUMMARY_CAP = 200 + +# stage 8.7: which terminal/pause statuses notify, and with what severity - +# keyed by the exact status string _land() writes to builder_status. +_LAND_NOTIFICATION_KINDS = {"done": "success", "failed": "error", "paused": "warning"} + PLAN_SCHEMA = { "type": "object", "properties": { @@ -368,11 +381,42 @@ def plan_steps_for_goal(goal: str, *, runtime=None, settings_manager=None) -> li # -- the executor loop ------------------------------------------------------- +def _truncate(text: str, cap: int) -> str: + return text if len(text) <= cap else text[:cap] + "…" + + def _approval_summary(call: ToolCall) -> str: args = json.dumps(call.arguments, sort_keys=True, ensure_ascii=False) - if len(args) > _APPROVAL_SUMMARY_CAP: - args = args[:_APPROVAL_SUMMARY_CAP] + "…" - return f"{call.name} {args}" + return f"{call.name} {_truncate(args, _APPROVAL_SUMMARY_CAP)}" + + +def _activity_summary(call: ToolCall, result: ToolResult) -> str: + """What a build's activity row shows for one invoked call. An error + (a real failure OR an approval denial - invoke() returns denial as + ToolResult(is_error=True, content="...was denied approval.") and the + two are not otherwise distinguishable here) shows the tool's own + result text, since that already says exactly what happened. A success + shows the call's arguments instead - the result of, say, + graph.read_subgraph is the interesting part to the MODEL, not to a + human scanning what the build did.""" + text = result.content if result.is_error else json.dumps(call.arguments, sort_keys=True, ensure_ascii=False) + return _truncate(text, _ACTIVITY_SUMMARY_CAP) + + +def _log_activity(node, *, tool: str, summary: str, outcome: str, step_id: str, elapsed_ms: int) -> None: + """Appends one row to the plan node's activity log - see PlanState's + own docstring for why this is deliberately untouched by undo. Trims + from the front (oldest first) once the ring buffer's cap is exceeded.""" + activity = node.state.builder_activity + activity.append({ + "tool": tool, + "summary": summary, + "outcome": outcome, + "stepId": step_id, + "elapsedMs": elapsed_ms, + }) + if len(activity) > _ACTIVITY_CAP: + del activity[: len(activity) - _ACTIVITY_CAP] def _rough_token_count(text: str) -> int: @@ -532,6 +576,16 @@ async def _land(status: str, detail: str) -> None: if node.pending_request_id == request_id: node.pending_request_id = None await bus.publish("scene") + # stage 8.7: a build that lands while the user is elsewhere on the + # canvas (or the app is unfocused) must not announce nothing. + # "stopped" is excluded - the user just clicked Stop and is + # necessarily present, so a notification for an action they took + # themselves is noise. "interrupted" never reaches this function + # (it is a session_load-time normalization, not a run outcome). + kind = _LAND_NOTIFICATION_KINDS.get(status) + if kind is not None and notifications is not None and detail: + notifications.show(detail, kind) + await bus.publish("notification") node.state.builder_status = "running" node.state.builder_status_detail = "" @@ -657,7 +711,16 @@ async def _land(status: str, detail: str) -> None: else: await _land("paused", breach + " Raise the budget and resume to continue.") return + call_started = time.monotonic() result = await registry.invoke(call, ctx) + # stage 8.7: every invoked call, including the four + # in-band control tools - hiding those would make the + # log lie about how many turns the build actually took. + _log_activity( + node, tool=call.name, summary=_activity_summary(call, result), + outcome="error" if result.is_error else "ok", + step_id=step["id"], elapsed_ms=max(0, round((time.monotonic() - call_started) * 1000)), + ) messages.append({ "role": "tool", "tool_call_id": call.id, "name": call.name, "content": result.content, @@ -715,10 +778,10 @@ async def _land(status: str, detail: str) -> None: # the timeout path above so resume doesn't skip the in-flight step. if step is not None and step.get("status") == "running": step["status"] = "pending" + # _land("failed", ...) now sends this failure's own notification + # (see _LAND_NOTIFICATION_KINDS) - a second one here would be a + # duplicate banner for the same event. await _land("failed", f"Build failed: {exc} — resume to retry.") - if notifications is not None: - notifications.show(f"Build failed: {exc}", "error") - await bus.publish("notification") def _apply_replan(document, node, controls: BuilderControls, run_id: str) -> None: diff --git a/backend/domain/graph.py b/backend/domain/graph.py index 8aaadc7..aac0b6b 100644 --- a/backend/domain/graph.py +++ b/backend/domain/graph.py @@ -2346,6 +2346,19 @@ def _node_wire(self, n: SceneNode) -> dict[str, Any]: ] if isinstance(n.state, PlanState) else [] ), + # ADR-008 stage 8.7: the run's own activity log - see PlanState's + # docstring for why this is untouched by undo. + "builderActivity": ( + [ + { + "tool": a["tool"], "summary": a["summary"], + "outcome": a["outcome"], "stepId": a["stepId"], + "elapsedMs": a["elapsedMs"], + } + for a in n.state.builder_activity + ] + if isinstance(n.state, PlanState) else [] + ), "builderStatus": n.state.builder_status if isinstance(n.state, PlanState) else "", "builderMode": n.state.builder_mode if isinstance(n.state, PlanState) else "", "builderRunId": n.state.builder_run_id if isinstance(n.state, PlanState) else "", diff --git a/backend/domain/node_states.py b/backend/domain/node_states.py index 0d62b08..ac02530 100644 --- a/backend/domain/node_states.py +++ b/backend/domain/node_states.py @@ -774,6 +774,20 @@ class PlanState(NodeState): own precedent directly above: the wire layer owns the typed row (PlanStepRow in contracts/), the domain keeps the flexible shape. + `activity` (stage 8.7) items are plain dicts {"tool": str, "summary": + str, "outcome": "ok"|"error", "stepId": str, "elapsedMs": int} - one + row per tool the loop invoked, in call order, written by builder.py's + _log_activity at its single registry.invoke() choke point. This is the + build's own visible record of WHAT IT DID - distinct from undo (undo_run + reverts document MUTATIONS; a build's activity log is run telemetry and + is deliberately left untouched by an undo, so the record of what + happened survives reverting what happened) and distinct from a chat + node's tool_invocations (that field holds one TURN's calls for a + completed reply; this holds a whole BUILD's calls across every turn and + every step). A capped ring buffer (see builder.py's _ACTIVITY_CAP) - + a build is bounded at 50 steps x 8 turns, so nothing else bounds this + list's growth. + `builder_status` is the loop's own state machine: draft -> planning -> awaiting_start -> running <-> awaiting_approval running -> paused (budget breach; resumable) @@ -797,6 +811,7 @@ class PlanState(NodeState): plan_goal: str = "" plan_steps: list[dict[str, Any]] = field(default_factory=list) + builder_activity: list[dict[str, Any]] = field(default_factory=list) builder_status: str = "draft" builder_mode: str = "copilot" # "copilot" | "autopilot" builder_run_id: str = "" diff --git a/backend/session_load.py b/backend/session_load.py index 82e2bbd..6a9e30d 100644 --- a/backend/session_load.py +++ b/backend/session_load.py @@ -713,6 +713,16 @@ def _restore_plan_payload(payload: dict[str, Any]) -> SceneNode: "detail": str(raw.get("detail", "")), }) mode = str(payload.get("builder_mode", "copilot") or "copilot") + activity = [] + for raw in payload.get("activity") or []: + if isinstance(raw, dict) and raw.get("tool"): + activity.append({ + "tool": str(raw.get("tool", "")), + "summary": str(raw.get("summary", "")), + "outcome": str(raw.get("outcome", "ok")), + "stepId": str(raw.get("stepId", "")), + "elapsedMs": int(raw.get("elapsedMs", 0) or 0), + }) return SceneNode( id="", x=x, y=y, title=f"Build: {goal[:40]}" if goal else "Build", @@ -721,6 +731,7 @@ def _restore_plan_payload(payload: dict[str, Any]) -> SceneNode: state=PlanState( plan_goal=goal, plan_steps=steps, + builder_activity=activity, builder_status=status, builder_mode=mode if mode in ("copilot", "autopilot") else "copilot", builder_run_id=str(payload.get("builder_run_id", "")), diff --git a/backend/session_save.py b/backend/session_save.py index 79e23bc..a0d4281 100644 --- a/backend/session_save.py +++ b/backend/session_save.py @@ -403,6 +403,12 @@ def _serialize_plan_node(node: SceneNode) -> dict[str, Any]: "node_type": "plan", "goal": node.state.plan_goal, "steps": [dict(s) for s in node.state.plan_steps], + # stage 8.7: the run's activity log - persisted like every other + # PlanState field so it survives restart alongside the resume + # point, and (unlike the LIVE-run fields below) it describes past + # calls rather than an in-flight RunHandle, so nothing here needs + # load-time normalization. + "activity": [dict(a) for a in node.state.builder_activity], "builder_status": node.state.builder_status, "builder_mode": node.state.builder_mode, "builder_run_id": node.state.builder_run_id, diff --git a/backend/tests/test_builder.py b/backend/tests/test_builder.py index e11c64c..4815b33 100644 --- a/backend/tests/test_builder.py +++ b/backend/tests/test_builder.py @@ -18,13 +18,16 @@ import api_provider from backend import builder as builder_module +from backend import tools_graph as tools_graph_module from backend.builder import run_build from backend.domain.graph import SceneDocument +from backend.domain.model import MESSAGE_VERTICAL_SPACING +from backend.notifications import NotificationState from backend.providers.base import ToolCall from backend.run_lifecycle import RunRegistry from backend.tests.test_canvas import make_bus_with_dispatcher from backend.tools import ToolRegistry -from backend.tools_graph import register_graph_tools, register_run_node_tool +from backend.tools_graph import _place_child, register_graph_tools, register_run_node_tool from backend.builder import register_builder_control_tools @@ -99,7 +102,7 @@ def seed_plan(document, steps, *, mode="copilot", **budgets): return node -async def drive_build(document, dispatcher, registry, bus, node, *, approve=True, deny_first=False): +async def drive_build(document, dispatcher, registry, bus, node, *, approve=True, deny_first=False, notifications=None): """Runs run_build while a driver coroutine plays the approving human. Returns (approvals_seen, denials_issued).""" cancel_event = threading.Event() @@ -111,7 +114,7 @@ async def run(): try: await run_build( document=document, dispatcher=dispatcher, registry=registry, - bus=bus, notifications=None, plan_node_id=node.id, + bus=bus, notifications=notifications, plan_node_id=node.id, request_id=handle.request_id, handle=handle, cancel_event=cancel_event, ) finally: @@ -829,3 +832,251 @@ def boom_turn(task, messages, tools=(), **kwargs): "a transient provider fault must not permanently kill a build " "whose goal/checklist/spent budgets are still on the canvas" ) + + +class TestActivityLog: + """stage 8.7: the build's own visible record of what it did.""" + + def test_activity_rows_are_written_for_ok_and_error_calls_including_control_tools(self, monkeypatch): + document, dispatcher, registry, bus = make_harness() + node = seed_plan(document, ["one step"]) + scripted_turns(monkeypatch, [ + {"tool_calls": [ + call("c1", "graph.create_node", kind="note", content="x"), + # pycoder requires parent_id - this call errors. + call("c2", "graph.create_node", kind="pycoder"), + ]}, + {"tool_calls": [call("c3", "builder.complete_step", summary="done")]}, + ], []) + + asyncio.run(drive_build(document, dispatcher, registry, bus, node)) + + rows = node.state.builder_activity + tools = [r["tool"] for r in rows] + assert tools == ["graph.create_node", "graph.create_node", "builder.complete_step"], ( + "every invoked call is logged in order, including the control tool" + ) + assert rows[0]["outcome"] == "ok" + assert rows[1]["outcome"] == "error" + assert rows[2]["outcome"] == "ok" + assert all(r["stepId"] == "s1" for r in rows) + assert all(isinstance(r["elapsedMs"], int) and r["elapsedMs"] >= 0 for r in rows) + + def test_a_denied_call_logs_as_error_with_the_denial_text(self, monkeypatch): + document, dispatcher, registry, bus = make_harness() + node = seed_plan(document, ["one step"]) + scripted_turns(monkeypatch, [ + {"tool_calls": [call("c1", "graph.create_node", kind="note", content="first try")]}, + {"tool_calls": [call("c2", "builder.complete_step", summary="ok, done without it")]}, + ], []) + + asyncio.run(drive_build(document, dispatcher, registry, bus, node, deny_first=True)) + + rows = node.state.builder_activity + assert rows[0]["tool"] == "graph.create_node" + assert rows[0]["outcome"] == "error" + assert "denied" in rows[0]["summary"] + + def test_activity_is_a_capped_ring_buffer(self, monkeypatch): + document, dispatcher, registry, bus = make_harness() + # autopilot: 120 graph.mutate calls with no approval round-trip each. + node = seed_plan(document, ["one step"], mode="autopilot", max_tokens=10_000_000, max_wall_seconds=10_000) + calls = [call(f"c{i}", "graph.create_node", kind="note", content=str(i)) for i in range(120)] + calls.append(call("cfinal", "builder.complete_step", summary="done")) + scripted_turns(monkeypatch, [{"tool_calls": calls}], []) + + asyncio.run(drive_build(document, dispatcher, registry, bus, node)) + + assert node.state.builder_status == "done" + assert len(node.state.builder_activity) == builder_module._ACTIVITY_CAP + # Oldest dropped first: the newest row logged is always last. + assert node.state.builder_activity[-1]["tool"] == "builder.complete_step" + + def test_undo_run_leaves_the_activity_log_intact(self, monkeypatch): + document, dispatcher, registry, bus = make_harness() + node = seed_plan(document, ["one step"]) + scripted_turns(monkeypatch, [ + {"tool_calls": [call("c1", "graph.create_node", kind="note", content="x")]}, + {"tool_calls": [call("c2", "builder.complete_step", summary="done")]}, + ], []) + + asyncio.run(drive_build(document, dispatcher, registry, bus, node)) + logged = len(node.state.builder_activity) + assert logged > 0 + + document.undo_run(node.state.builder_run_id) + + assert len(node.state.builder_activity) == logged, ( + "activity is run telemetry, not document content - undo_run must not touch it" + ) + + def test_activity_round_trips_through_session_save_and_load(self): + from backend.session_load import _restore_plan_payload + from backend.session_save import _serialize_plan_node + + document = SceneDocument() + node = document.add_plan_node(0, 0, "the goal") + node.state.builder_activity = [ + {"tool": "graph.create_node", "summary": "{}", "outcome": "ok", "stepId": "s1", "elapsedMs": 42}, + ] + + restored = _restore_plan_payload(_serialize_plan_node(node)) + + assert restored.state.builder_activity == node.state.builder_activity + + +class TestLandNotifications: + """stage 8.7: a build that lands while the user is elsewhere must not + announce nothing.""" + + def test_done_notifies_success(self, monkeypatch): + document, dispatcher, registry, bus = make_harness() + node = seed_plan(document, ["one step"]) + notifications = NotificationState() + scripted_turns(monkeypatch, [ + {"tool_calls": [call("c1", "builder.finish_build", summary="all done")]}, + ], []) + + asyncio.run(drive_build(document, dispatcher, registry, bus, node, notifications=notifications)) + + assert notifications.msg_type == "success" + assert notifications.message == "all done" + + def test_paused_notifies_warning(self, monkeypatch): + document, dispatcher, registry, bus = make_harness() + node = seed_plan(document, ["one step"], max_tokens=1) + notifications = NotificationState() + scripted_turns(monkeypatch, [ + { + "tool_calls": [call("c1", "graph.create_node", kind="note", content="x")], + "usage": {"prompt_tokens": 5, "completion_tokens": 5}, + }, + ], []) + + asyncio.run(drive_build(document, dispatcher, registry, bus, node, notifications=notifications)) + + assert notifications.msg_type == "warning" + assert "budget" in notifications.message.lower() + + def test_failed_notifies_error(self, monkeypatch): + document, dispatcher, registry, bus = make_harness() + node = seed_plan(document, ["a step"]) + notifications = NotificationState() + + def boom_turn(task, messages, tools=(), **kwargs): + raise RuntimeError("rate limited") + + monkeypatch.setattr(api_provider, "chat_turn_with_tools", boom_turn) + + asyncio.run(drive_build(document, dispatcher, registry, bus, node, notifications=notifications)) + + assert notifications.msg_type == "error" + assert "rate limited" in notifications.message + + def test_stopped_does_not_notify(self): + document, dispatcher, registry, bus = make_harness() + node = seed_plan(document, ["one step"]) + notifications = NotificationState() + cancel_event = threading.Event() + cancel_event.set() # pre-cancelled: the loop's own top-of-loop check fires first + handle = dispatcher._runs.claim("builder", node_id=node.id, cancel_event=cancel_event) + + async def run(): + try: + await run_build( + document=document, dispatcher=dispatcher, registry=registry, + bus=bus, notifications=notifications, plan_node_id=node.id, + request_id=handle.request_id, handle=handle, cancel_event=cancel_event, + ) + finally: + dispatcher._runs.release(handle.request_id) + + asyncio.run(run()) + + assert node.state.builder_status == "stopped" + assert notifications.message == "", ( + "Stop is user-initiated - a notification for an action the user just took is noise" + ) + + +class TestAnchoredPlacement: + """stage 8.7: a build's parentless creates land near its plan node + instead of scattering at the canvas origin.""" + + def test_a_parentless_create_anchors_near_the_plan_node(self, monkeypatch): + document, dispatcher, registry, bus = make_harness() + node = seed_plan(document, ["one step"]) + node.x, node.y = 500.0, 300.0 + scripted_turns(monkeypatch, [ + {"tool_calls": [call("c1", "graph.create_node", kind="note", content="x")]}, + {"tool_calls": [call("c2", "builder.complete_step", summary="done")]}, + ], []) + + asyncio.run(drive_build(document, dispatcher, registry, bus, node)) + + created = next(n for n in document.nodes.values() if n.kind == "note") + assert created.x == node.x + assert created.y == node.y + MESSAGE_VERTICAL_SPACING + + def test_multiple_parentless_creates_fan_out_along_the_anchor_row(self, monkeypatch): + document, dispatcher, registry, bus = make_harness() + node = seed_plan(document, ["one step"]) + node.x, node.y = 100.0, 100.0 + scripted_turns(monkeypatch, [ + {"tool_calls": [ + call("c1", "graph.create_node", kind="note", content="a"), + call("c2", "graph.create_node", kind="note", content="b"), + ]}, + {"tool_calls": [call("c3", "builder.complete_step", summary="done")]}, + ], []) + + asyncio.run(drive_build(document, dispatcher, registry, bus, node)) + + notes = sorted((n for n in document.nodes.values() if n.kind == "note"), key=lambda n: n.x) + assert len(notes) == 2 + assert notes[0].x == node.x + assert notes[1].x == node.x + tools_graph_module._SIBLING_HORIZONTAL_SPACING + assert notes[0].y == notes[1].y == node.y + MESSAGE_VERTICAL_SPACING + + def test_no_anchor_falls_back_to_the_origin_drop(self): + document = SceneDocument() + assert _place_child(document, None, None) == (80.0, 80.0) + + +class TestDeleteRecipe: + def test_deletes_a_saved_recipe(self): + async def run(): + bus, document, recorder, dispatcher = make_bus_with_dispatcher() + source = document.add_plan_node(0, 0, "goal") + source.state.plan_steps = [{"id": "s1", "title": "step", "status": "done", "detail": ""}] + await bus.dispatch_intent("builder", "saveRecipe", [source.id, "My recipe"]) + + result = await bus.dispatch_intent("builder", "deleteRecipe", ["My recipe"]) + assert result is True + + listing = await bus.dispatch_intent("builder", "listRecipes", []) + names = [r["name"] for r in listing["recipes"]] + assert "My recipe" not in names + assert "Research and summarize" in names, "built-ins are untouched" + + asyncio.run(run()) + + def test_refuses_to_delete_a_built_in(self): + async def run(): + bus, document, recorder, dispatcher = make_bus_with_dispatcher() + result = await bus.dispatch_intent("builder", "deleteRecipe", ["Research and summarize"]) + assert result is False + + listing = await bus.dispatch_intent("builder", "listRecipes", []) + names = [r["name"] for r in listing["recipes"]] + assert "Research and summarize" in names + + asyncio.run(run()) + + def test_deleting_an_unknown_name_is_a_no_op_not_an_error(self): + async def run(): + bus, document, recorder, dispatcher = make_bus_with_dispatcher() + result = await bus.dispatch_intent("builder", "deleteRecipe", ["Nonexistent"]) + assert result is False + + asyncio.run(run()) diff --git a/backend/tools_graph.py b/backend/tools_graph.py index 1c86134..2407805 100644 --- a/backend/tools_graph.py +++ b/backend/tools_graph.py @@ -28,7 +28,10 @@ Placement: the model never picks coordinates. New nodes land relative to their parent using the same MESSAGE_VERTICAL_SPACING convention send_message's own reply placement uses (backend/domain/model.py), with a -horizontal fan-out for siblings so parallel children don't stack. +horizontal fan-out for siblings so parallel children don't stack. A +parentless create (stage 8.7) instead anchors near the run's plan node, if +one exists, so a build's output lands where the user just looked rather +than at the canvas origin - see _place_child's own doc. """ from __future__ import annotations @@ -178,21 +181,49 @@ def _run_id_of(ctx: RunContext) -> str | None: return getattr(ctx, "run_id", None) -def _place_child(document: SceneDocument, parent_id: str | None) -> tuple[float, float]: +def _anchor_id_of(ctx: RunContext) -> str | None: + """stage 8.7: BuilderRunContext already carries plan_node_id (builder.py) + for run attribution - reused here as a PLACEMENT reference, not graph + parentage, so a build's parentless creates (a note, a from-scratch + chat/code node) land near the plan node the user just launched instead + of scattered near the canvas origin. Same duck-typed degradation as + _run_id_of: a bare RunContext has none, and placement falls through to + the origin-drop fallback exactly as before this existed.""" + return getattr(ctx, "plan_node_id", None) + + +def _place_child( + document: SceneDocument, parent_id: str | None, anchor_id: str | None = None, +) -> tuple[float, float]: """Parent-relative placement, model-free: directly below the parent, fanning right one slot per existing child so parallel children of the same parent land side by side instead of stacked.""" - if parent_id is None or parent_id not in document.nodes: - # Free-floating (note, parentless chat/code): drop near the origin - # offset by node count so repeated creations don't perfectly overlap. - n = len(document.nodes) - return 80.0 + (n % 5) * 40.0, 80.0 + (n % 7) * 40.0 - parent = document.nodes[parent_id] - existing_children = sum(1 for e in document.edges.values() if e.source == parent_id) - return ( - parent.x + existing_children * _SIBLING_HORIZONTAL_SPACING, - parent.y + MESSAGE_VERTICAL_SPACING, - ) + if parent_id is not None and parent_id in document.nodes: + parent = document.nodes[parent_id] + existing_children = sum(1 for e in document.edges.values() if e.source == parent_id) + return ( + parent.x + existing_children * _SIBLING_HORIZONTAL_SPACING, + parent.y + MESSAGE_VERTICAL_SPACING, + ) + if anchor_id is not None and anchor_id in document.nodes: + # stage 8.7: same fan-out shape as the parented branch above, but + # the anchor (a plan node) never gains an EDGE to what it builds - + # it is a placement reference only - so there is no edge count to + # read a sibling index off. A node already sitting on the anchor's + # own placement row is this same anchor's own earlier creation + # (nothing else places there), so counting THOSE stands in for + # "existing children" without needing a persisted counter. + anchor = document.nodes[anchor_id] + row_y = anchor.y + MESSAGE_VERTICAL_SPACING + row_siblings = sum( + 1 for n in document.nodes.values() if n.x >= anchor.x and abs(n.y - row_y) < 1.0 + ) + return anchor.x + row_siblings * _SIBLING_HORIZONTAL_SPACING, row_y + # Free-floating with no anchor either (a bare RunContext, e.g. a future + # non-builder caller): drop near the origin offset by node count so + # repeated creations don't perfectly overlap. + n = len(document.nodes) + return 80.0 + (n % 5) * 40.0, 80.0 + (n % 7) * 40.0 def _error(message: str) -> ToolResult: @@ -215,7 +246,7 @@ async def handler(call: ToolCall, ctx: RunContext) -> ToolResult: if kind == "document" and not title: return _error("kind 'document' requires a title.") - x, y = _place_child(document, parent_id) + x, y = _place_child(document, parent_id, _anchor_id_of(ctx)) run_id = _run_id_of(ctx) def mutator(): diff --git a/contracts/graphlink_scene_payload.py b/contracts/graphlink_scene_payload.py index ea59de1..7c20ab1 100644 --- a/contracts/graphlink_scene_payload.py +++ b/contracts/graphlink_scene_payload.py @@ -292,6 +292,21 @@ class PlanStepRow: detail: str = "" +@dataclass +class BuilderActivityRow: + """ADR-008 stage 8.7: one row of a Builder run's activity log - the + typed wire shape of PlanState.builder_activity's own {"tool","summary", + "outcome","stepId","elapsedMs"} dicts (backend/domain/node_states.py). + outcome is "ok"|"error" - pinned as a plain string for the same + additive-evolution reason PlanStepRow's own status is.""" + + tool: str + summary: str + outcome: str = "ok" + stepId: str = "" + elapsedMs: int = 0 + + @dataclass class ChartFlowRow: """One Sankey flow - the shape canonicalize_chart_data() (graphlink_ @@ -553,6 +568,7 @@ class SceneNodeRow: # contract these fields serialize. planGoal: str = "" planSteps: list["PlanStepRow"] = field(default_factory=list) + builderActivity: list["BuilderActivityRow"] = field(default_factory=list) builderStatus: str = "" builderMode: str = "" builderRunId: str = "" diff --git a/tests/test_node_state_migration.py b/tests/test_node_state_migration.py index 647596e..0b1ffe7 100644 --- a/tests/test_node_state_migration.py +++ b/tests/test_node_state_migration.py @@ -110,7 +110,7 @@ # candidates - goal/steps/mode/run_id - collide with Command.run_id # and friends all over the command layer). "plan": [ - "plan_goal", "plan_steps", "builder_status", "builder_mode", + "plan_goal", "plan_steps", "builder_activity", "builder_status", "builder_mode", "builder_run_id", "builder_max_steps", "builder_max_tokens", "builder_max_wall_seconds", "builder_spent_steps", "builder_spent_tokens", "builder_spent_wall_seconds", @@ -343,9 +343,10 @@ def test_scene_node_core_field_count(): "completionTokens", "promptTokens", "researchResult", "researchStage", "researchTotal", "responseIncomplete", "synthesisInstructions", "title", "toolCalls", "x", "y", - # ADR-008 stage 8.3: the Builder plan node's 15 fields. - "planGoal", "planSteps", "builderStatus", "builderMode", "builderRunId", - "builderMaxSteps", "builderMaxTokens", "builderMaxWallSeconds", + # ADR-008 stage 8.3: the Builder plan node's 15 fields, +1 (builderActivity) + # when stage 8.7 added the run's own activity log. + "planGoal", "planSteps", "builderActivity", "builderStatus", "builderMode", + "builderRunId", "builderMaxSteps", "builderMaxTokens", "builderMaxWallSeconds", "builderSpentSteps", "builderSpentTokens", "builderSpentWallSeconds", "builderAwaitingToolApproval", "builderApprovalToolName", "builderApprovalSummary", "builderStatusDetail", diff --git a/tests/test_undo_classification_gate.py b/tests/test_undo_classification_gate.py index 63e6da9..dc4327c 100644 --- a/tests/test_undo_classification_gate.py +++ b/tests/test_undo_classification_gate.py @@ -255,11 +255,12 @@ def test_the_scan_finds_the_real_population_of_registered_intents(): # archiveWorkspace sextet, 165 -> 166 when ADR-020 stage 20.3 added # app-chat-library's own setWorkspaceDefaultModel, 166 -> 168 when # ADR-020 stage 20.4 added app-chat-library's own loadGraphAndFocusNode - # plus the new "globalSearch" topic's own search intent, and 168 -> 169 - # when ADR-020 stage 20.5 added app-chat-library's own exportWorkspace. + # plus the new "globalSearch" topic's own search intent, 168 -> 169 + # when ADR-020 stage 20.5 added app-chat-library's own exportWorkspace, + # and 169 -> 170 when ADR-008 stage 8.7 added builder's own deleteRecipe. real = _collect_real_registrations() - assert len(real) == 169, ( - f"expected exactly 169 real registered intents, found {len(real)} - " + assert len(real) == 170, ( + f"expected exactly 170 real registered intents, found {len(real)} - " "either the scan broke, or the app's registered-intent surface " "genuinely changed and tests/undo_classification.py's own count " "comment (and this assertion) need a deliberate update alongside it" diff --git a/tests/undo_classification.py b/tests/undo_classification.py index b957635..b250876 100644 --- a/tests/undo_classification.py +++ b/tests/undo_classification.py @@ -248,6 +248,7 @@ class Classified: Classified("builder", "denyTool", "B", "security: builder tool-call approval gate"), Classified("builder", "listRecipes", "B", "read-only: returns the recipe list"), Classified("builder", "saveRecipe", "B", "preference: writes the settings store's recipe list, not document state"), + Classified("builder", "deleteRecipe", "B", "preference: writes the settings store's recipe list, not document state - same posture as saveRecipe"), Classified("scene", "setPlanSteps", "A", "content: the plan checklist is document state"), # -- backend/api/intents_settings_general.py (app-settings) ------------- diff --git a/web_ui/src/lib/bridge-core/generated/scene-state.schema.json b/web_ui/src/lib/bridge-core/generated/scene-state.schema.json index 31ef5d2..fa95db0 100644 --- a/web_ui/src/lib/bridge-core/generated/scene-state.schema.json +++ b/web_ui/src/lib/bridge-core/generated/scene-state.schema.json @@ -65,6 +65,37 @@ "branchStatus": { "type": "string" }, + "builderActivity": { + "items": { + "additionalProperties": false, + "properties": { + "elapsedMs": { + "type": "integer" + }, + "outcome": { + "type": "string" + }, + "stepId": { + "type": "string" + }, + "summary": { + "type": "string" + }, + "tool": { + "type": "string" + } + }, + "required": [ + "tool", + "summary", + "outcome", + "stepId", + "elapsedMs" + ], + "type": "object" + }, + "type": "array" + }, "builderApprovalSummary": { "type": "string" }, @@ -769,6 +800,7 @@ "indexIntoKnowledge", "planGoal", "planSteps", + "builderActivity", "builderStatus", "builderMode", "builderRunId", diff --git a/web_ui/src/lib/bridge-core/generated/scene-state.ts b/web_ui/src/lib/bridge-core/generated/scene-state.ts index 6f082fb..16e7893 100644 --- a/web_ui/src/lib/bridge-core/generated/scene-state.ts +++ b/web_ui/src/lib/bridge-core/generated/scene-state.ts @@ -98,6 +98,7 @@ export interface SceneNodeRow { indexIntoKnowledge: boolean; planGoal: string; planSteps: PlanStepRow[]; + builderActivity: BuilderActivityRow[]; builderStatus: string; builderMode: string; builderRunId: string; @@ -194,6 +195,14 @@ export interface PlanStepRow { detail: string; } +export interface BuilderActivityRow { + tool: string; + summary: string; + outcome: string; + stepId: string; + elapsedMs: number; +} + export interface SceneEdgeRow { id: string; source: string; @@ -714,6 +723,12 @@ function checkSceneNodeRow(value: unknown, path: string, errors: string[]): void else { if (!Array.isArray(fieldValue)) errors.push(`${path}.planSteps` + ": expected array"); else (fieldValue as unknown[]).forEach((item, i) => { checkPlanStepRow(item, `${path}.planSteps` + `[${i}]`, errors); }); } } + { + const fieldValue = value["builderActivity"]; + if (fieldValue === undefined || fieldValue === null) errors.push(`${path}.builderActivity: missing required field`); + else { if (!Array.isArray(fieldValue)) errors.push(`${path}.builderActivity` + ": expected array"); + else (fieldValue as unknown[]).forEach((item, i) => { checkBuilderActivityRow(item, `${path}.builderActivity` + `[${i}]`, errors); }); } + } { const fieldValue = value["builderStatus"]; if (fieldValue === undefined || fieldValue === null) errors.push(`${path}.builderStatus: missing required field`); @@ -1085,6 +1100,35 @@ function checkPlanStepRow(value: unknown, path: string, errors: string[]): void } } +function checkBuilderActivityRow(value: unknown, path: string, errors: string[]): void { + if (!isRecord(value)) { errors.push(`${path}: expected object`); return; } + { + const fieldValue = value["tool"]; + if (fieldValue === undefined || fieldValue === null) errors.push(`${path}.tool: missing required field`); + else { if (typeof fieldValue !== "string") errors.push(`${path}.tool` + ": expected string"); } + } + { + const fieldValue = value["summary"]; + if (fieldValue === undefined || fieldValue === null) errors.push(`${path}.summary: missing required field`); + else { if (typeof fieldValue !== "string") errors.push(`${path}.summary` + ": expected string"); } + } + { + const fieldValue = value["outcome"]; + if (fieldValue === undefined || fieldValue === null) errors.push(`${path}.outcome: missing required field`); + else { if (typeof fieldValue !== "string") errors.push(`${path}.outcome` + ": expected string"); } + } + { + const fieldValue = value["stepId"]; + if (fieldValue === undefined || fieldValue === null) errors.push(`${path}.stepId: missing required field`); + else { if (typeof fieldValue !== "string") errors.push(`${path}.stepId` + ": expected string"); } + } + { + const fieldValue = value["elapsedMs"]; + if (fieldValue === undefined || fieldValue === null) errors.push(`${path}.elapsedMs: missing required field`); + else { if (typeof fieldValue !== "number") errors.push(`${path}.elapsedMs` + ": expected number"); } + } +} + function checkSceneEdgeRow(value: unknown, path: string, errors: string[]): void { if (!isRecord(value)) { errors.push(`${path}: expected object`); return; } { From 6260a26e0c09d577c60cd86e1c5f19ca9538ff2c Mon Sep 17 00:00:00 2001 From: dovvnloading Date: Sun, 16 Aug 2026 18:07:13 -0400 Subject: [PATCH 2/6] Render the Builder's activity log on the plan node The plan node had no way to show the activity log added in the previous commit. This wires PlanState.builder_activity through SceneCanvas into the plan node's data and renders it as a collapsed-by-default disclosure, reusing ChatNodeView's existing "an assistant turn's tool calls" pattern (.chat-node-tool-invocations) rather than a new widget, since the content is the same shape - a tool name, an outcome, one block of detail text - just scoped to a whole build instead of a single turn, with a bounded, independently scrollable list to hold the larger row count. The log auto-scrolls to its newest row while the build is running and the disclosure is open. Verified against a real running backend (a scripted provider driving a real build through the real WS/document/wire path, approval gate included): the activity count, error count, per-row tool name, outcome tint, and summary text all matched the calls made; the completion notification carried the build's own finish summary; and the two created notes landed exactly where the previous commit's anchored-placement fan- out predicts, relative to the plan node. --- web_ui/src/app/canvas/PlanNodeView.test.tsx | 52 +++++++++++++++++ web_ui/src/app/canvas/PlanNodeView.tsx | 58 +++++++++++++++++++ .../canvas/SceneCanvas.pinSearchJump.test.tsx | 1 + web_ui/src/app/canvas/SceneCanvas.test.tsx | 1 + web_ui/src/app/canvas/SceneCanvas.tsx | 1 + .../SceneCanvas.virtualization.test.tsx | 1 + .../src/app/canvas/renderCountGate.test.tsx | 1 + web_ui/src/app/canvas/sceneStore.test.ts | 1 + web_ui/src/app/styles.css | 19 ++++++ 9 files changed, 135 insertions(+) diff --git a/web_ui/src/app/canvas/PlanNodeView.test.tsx b/web_ui/src/app/canvas/PlanNodeView.test.tsx index c556d01..f2a82a3 100644 --- a/web_ui/src/app/canvas/PlanNodeView.test.tsx +++ b/web_ui/src/app/canvas/PlanNodeView.test.tsx @@ -15,6 +15,7 @@ function makeData(overrides: Partial = {}): PlanNodeData { { id: "s2", title: "Write summary", status: "running", detail: "" }, { id: "s3", title: "Chart it", status: "pending", detail: "" }, ], + builderActivity: [], builderStatus: "running", builderMode: "copilot", builderRunId: "run-1", @@ -183,4 +184,55 @@ describe("PlanNodeView", () => { expect(screen.getByRole("alert")).toHaveTextContent("the model aborted"); expect(screen.getByText("autopilot")).toBeInTheDocument(); }); + + describe("activity log", () => { + it("stays hidden entirely when the build has no activity yet", () => { + renderPlan(makeData({ builderActivity: [] })); + expect(screen.queryByText(/activity entr/)).not.toBeInTheDocument(); + }); + + it("shows the count, error count, and every row's tool/summary/elapsed - collapsed by default", () => { + const data = makeData({ + builderActivity: [ + { tool: "graph.create_node", summary: '{"kind":"note"}', outcome: "ok", stepId: "s1", elapsedMs: 12 }, + { + tool: "graph.create_node", + summary: "Tool call 'graph.create_node' was denied approval.", + outcome: "error", stepId: "s1", elapsedMs: 0, + }, + ], + }); + renderPlan(data); + + const summary = screen.getByText("2 activity entries · 1 error"); + const details = summary.closest("details"); + expect(details).not.toHaveAttribute("open"); + expect(screen.getAllByText("graph.create_node")).toHaveLength(2); + expect(screen.getByText('{"kind":"note"}')).toBeInTheDocument(); + expect(screen.getByText(/was denied approval/)).toBeInTheDocument(); + expect(screen.getByText("12ms")).toBeInTheDocument(); + }); + + it("tints an error row distinctly from an ok row", () => { + renderPlan(makeData({ + builderActivity: [ + { tool: "graph.create_node", summary: "ok call", outcome: "ok", stepId: "s1", elapsedMs: 5 }, + { tool: "run_node", summary: "boom", outcome: "error", stepId: "s1", elapsedMs: 3 }, + ], + })); + + expect(screen.getByText("ok call").closest(".chat-node-tool-invocation")).not.toHaveClass("error"); + expect(screen.getByText("boom").closest(".chat-node-tool-invocation")).toHaveClass("error"); + }); + + it("singular-cases one entry and omits the error count when nothing failed", () => { + renderPlan(makeData({ + builderActivity: [ + { tool: "builder.complete_step", summary: "{}", outcome: "ok", stepId: "s1", elapsedMs: 1 }, + ], + })); + expect(screen.getByText("1 activity entry")).toBeInTheDocument(); + expect(screen.queryByText(/error/)).not.toBeInTheDocument(); + }); + }); }); diff --git a/web_ui/src/app/canvas/PlanNodeView.tsx b/web_ui/src/app/canvas/PlanNodeView.tsx index 34c61fc..5bad6d5 100644 --- a/web_ui/src/app/canvas/PlanNodeView.tsx +++ b/web_ui/src/app/canvas/PlanNodeView.tsx @@ -27,9 +27,18 @@ export interface PlanStepData { detail: string; } +export interface BuilderActivityRowData { + tool: string; + summary: string; + outcome: string; + stepId: string; + elapsedMs: number; +} + export interface PlanNodeData extends Record { planGoal: string; planSteps: PlanStepData[]; + builderActivity: BuilderActivityRowData[]; builderStatus: string; builderMode: string; builderRunId: string; @@ -96,6 +105,21 @@ function PlanNodeViewInner({ data, selected }: NodeProps) { const resumable = RESUMABLE.has(data.builderStatus); const startLabel = data.builderStatus === "awaiting_start" ? "Start build" : "Resume"; const denyButtonRef = useRef(null); + const activityDetailsRef = useRef(null); + const activityListRef = useRef(null); + const activityErrorCount = data.builderActivity.filter((row) => row.outcome !== "ok").length; + + // Keeps the log pinned to its newest row while the build is actively + // producing more of them - only while the disclosure is actually open + // (a native
's own `open` attribute IS the expand/collapse + // state here, so no separate React state exists to gate this on) and + // only while running, so a landed build's log stops moving under the + // user once there is nothing left to follow. + useEffect(() => { + if (running && activityDetailsRef.current?.open) { + activityListRef.current?.scrollTo({ top: activityListRef.current.scrollHeight }); + } + }, [data.builderActivity, running]); // The tool-approval panel mounts fresh each time the Builder pauses for // approval (see the block-level comment above); Deny is the safe default, @@ -166,6 +190,40 @@ function PlanNodeViewInner({ data, selected }: NodeProps) { + {/* stage 8.7: the build's own visible record of what it did - real + backend state (PlanState.builder_activity), not a debug aid. + Reuses ChatNodeView's own "an assistant turn's tool calls, + disclosed" pattern (.chat-node-tool-invocations) rather than a + parallel widget, since the content is the same shape (a tool + name, an outcome, one block of detail text) - just scoped to a + whole BUILD instead of one turn, and potentially many more rows, + which is the one thing that needs its own scrollable container. */} + {data.builderActivity.length > 0 && ( +
+ + {data.builderActivity.length === 1 + ? "1 activity entry" + : `${data.builderActivity.length} activity entries`} + {activityErrorCount > 0 && + ` · ${activityErrorCount} error${activityErrorCount === 1 ? "" : "s"}`} + +
+ {data.builderActivity.map((row, index) => ( +
+
+ {row.tool} + {row.elapsedMs}ms +
+ 
{row.summary}
+
+ ))} +
+
+ )} + {data.builderAwaitingToolApproval && (
diff --git a/web_ui/src/app/canvas/SceneCanvas.pinSearchJump.test.tsx b/web_ui/src/app/canvas/SceneCanvas.pinSearchJump.test.tsx index 83044bb..ce37363 100644 --- a/web_ui/src/app/canvas/SceneCanvas.pinSearchJump.test.tsx +++ b/web_ui/src/app/canvas/SceneCanvas.pinSearchJump.test.tsx @@ -131,6 +131,7 @@ function chatRow(id: string, x: number, y: number, title = id): SceneNodeRow { indexIntoKnowledge: false, planGoal: "", planSteps: [], + builderActivity: [], builderStatus: "", builderMode: "", builderRunId: "", diff --git a/web_ui/src/app/canvas/SceneCanvas.test.tsx b/web_ui/src/app/canvas/SceneCanvas.test.tsx index 4d82168..bfa01bb 100644 --- a/web_ui/src/app/canvas/SceneCanvas.test.tsx +++ b/web_ui/src/app/canvas/SceneCanvas.test.tsx @@ -141,6 +141,7 @@ function baseNode(overrides: Partial = {}): SceneNodeRow { indexIntoKnowledge: false, planGoal: "", planSteps: [], + builderActivity: [], builderStatus: "", builderMode: "", builderRunId: "", diff --git a/web_ui/src/app/canvas/SceneCanvas.tsx b/web_ui/src/app/canvas/SceneCanvas.tsx index a08a915..6b67e0f 100644 --- a/web_ui/src/app/canvas/SceneCanvas.tsx +++ b/web_ui/src/app/canvas/SceneCanvas.tsx @@ -1439,6 +1439,7 @@ export function toFlowNodes( data: { planGoal: n.planGoal, planSteps: n.planSteps, + builderActivity: n.builderActivity, builderStatus: n.builderStatus, builderMode: n.builderMode, builderRunId: n.builderRunId, diff --git a/web_ui/src/app/canvas/SceneCanvas.virtualization.test.tsx b/web_ui/src/app/canvas/SceneCanvas.virtualization.test.tsx index 063c67e..e99edf5 100644 --- a/web_ui/src/app/canvas/SceneCanvas.virtualization.test.tsx +++ b/web_ui/src/app/canvas/SceneCanvas.virtualization.test.tsx @@ -150,6 +150,7 @@ function chatRow(id: string, x: number, y = 0): SceneNodeRow { indexIntoKnowledge: false, planGoal: "", planSteps: [], + builderActivity: [], builderStatus: "", builderMode: "", builderRunId: "", diff --git a/web_ui/src/app/canvas/renderCountGate.test.tsx b/web_ui/src/app/canvas/renderCountGate.test.tsx index c29663e..bb4c22b 100644 --- a/web_ui/src/app/canvas/renderCountGate.test.tsx +++ b/web_ui/src/app/canvas/renderCountGate.test.tsx @@ -149,6 +149,7 @@ function chatRow(id: string, x: number): SceneNodeRow { indexIntoKnowledge: false, // ADR-017 stage 17.5 planGoal: "", planSteps: [], + builderActivity: [], builderStatus: "", builderMode: "", builderRunId: "", diff --git a/web_ui/src/app/canvas/sceneStore.test.ts b/web_ui/src/app/canvas/sceneStore.test.ts index 40b7ce9..7dd83c4 100644 --- a/web_ui/src/app/canvas/sceneStore.test.ts +++ b/web_ui/src/app/canvas/sceneStore.test.ts @@ -186,6 +186,7 @@ function validScenePayload(overrides: Record = {}) { indexIntoKnowledge: false, planGoal: "", planSteps: [], + builderActivity: [], builderStatus: "", builderMode: "", builderRunId: "", diff --git a/web_ui/src/app/styles.css b/web_ui/src/app/styles.css index 8900b96..82a055c 100644 --- a/web_ui/src/app/styles.css +++ b/web_ui/src/app/styles.css @@ -7351,6 +7351,25 @@ mark.document-view-search-match-current { color: var(--gl-surface-text-muted); flex-wrap: wrap; } + +/* stage 8.7: the activity log. .chat-node-tool-invocations already supplies + the disclosure surface's border/padding/summary styling and its per-row + .chat-node-tool-invocation(.error)/-name/-arguments treatment - this adds + only what a build's log needs beyond one turn's handful of calls: a + bounded, independently-scrollable list (up to 100 rows) and an elapsed- + time readout per row. */ +.plan-node-activity-list { + max-height: 160px; + overflow-y: auto; + display: flex; + flex-direction: column; +} + +.plan-node-activity-elapsed { + float: right; + font-weight: 400; + color: var(--gl-surface-text-muted); +} .plan-node-approval { border: 1px solid var(--gl-semantic-status-warning); border-radius: 6px; From 07b70ae5da720622a98e985e88deafe5ba6339bd Mon Sep 17 00:00:00 2001 From: dovvnloading Date: Sun, 16 Aug 2026 18:18:11 -0400 Subject: [PATCH 3/6] Rebuild the Builder launch dialog and focus the viewport on a new build The launch dialog was the last surface in the app still using a bare native . + */ +import { ReactFlowProvider, type useReactFlow as UseReactFlowType } from "@xyflow/react"; import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; -import { describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { SceneStore } from "../canvas/sceneStore"; + +type SetCenterCall = [number, number, { zoom?: number; duration?: number } | undefined]; +const setCenterCalls: SetCenterCall[] = []; + +vi.mock("@xyflow/react", async (importOriginal) => { + const original = await importOriginal(); + return { + ...original, + useReactFlow: (...args: Parameters) => { + const real = original.useReactFlow(...args); + return { + ...real, + setCenter: (x: number, y: number, options?: { zoom?: number; duration?: number }) => { + setCenterCalls.push([x, y, options]); + return real.setCenter(x, y, options); + }, + }; + }, + }; +}); + import { BuilderLaunchDialog } from "./BuilderLaunchDialog"; import { OverlayProvider, useOverlays } from "../overlays/overlays"; import type { WsTransport } from "../../lib/ws/transport"; @@ -20,6 +52,15 @@ function makeTransport() { return { transport, intents, request }; } +// Only getScene() is exercised (reading the newly-created node's position +// for the post-launch setCenter call) - a minimal fake, the same posture +// makeTransport() takes for WsTransport above. +function makeStore(nodes: Array<{ id: string; x: number; y: number }> = []) { + return { + getScene: () => ({ nodes }), + } as unknown as SceneStore; +} + function OpenBuilderButton() { const overlays = useOverlays(); return ( @@ -29,30 +70,44 @@ function OpenBuilderButton() { ); } -async function setup(startResult: unknown = "n42", recipes: unknown[] = []) { +async function setup(startResult: unknown = "n42", recipes: unknown[] = [], nodes: Array<{ id: string; x: number; y: number }> = []) { const user = userEvent.setup(); const fake = makeTransport(); + const store = makeStore(nodes); // The dialog fires listRecipes on open - dispatch by intent so each // test's start-result expectation is never consumed by the list call. fake.request.mockImplementation((topic: string, intent: string) => intent === "listRecipes" ? Promise.resolve({ recipes }) - : startResult instanceof Error - ? Promise.reject(startResult) - : Promise.resolve(startResult), + : intent === "deleteRecipe" + ? Promise.resolve(true) + : startResult instanceof Error + ? Promise.reject(startResult) + : Promise.resolve(startResult), ); render( - - + + + + , ); await user.click(screen.getByRole("button", { name: "open builder" })); return { user, ...fake }; } +async function pickRecipe(user: ReturnType, name: string) { + await user.click(screen.getByRole("button", { name: "Recipe" })); + await user.click(screen.getByRole("button", { name: new RegExp("^" + name) })); +} + describe("BuilderLaunchDialog", () => { - it("submits the goal with mode and default budgets through builder/start", async () => { + beforeEach(() => { + setCenterCalls.length = 0; + }); + + it("submits the goal with mode and default (Standard) budgets through builder/start", async () => { const { user, intents } = await setup(); await user.type(screen.getByLabelText(/what should the builder construct/i), "chart solar trends"); @@ -65,12 +120,10 @@ describe("BuilderLaunchDialog", () => { it("selecting a recipe enables launch without a typed goal and passes the name", async () => { const { user, intents } = await setup("n9", [ - { name: "Research and summarize", description: "d", builtIn: true }, + { name: "Research and summarize", description: "d", steps: [], mode: "copilot", builtIn: true }, ]); - await user.selectOptions( - screen.getByLabelText("Recipe"), "Research and summarize", - ); + await pickRecipe(user, "Research and summarize"); await user.click(screen.getByRole("button", { name: "Start from recipe" })); expect(intents).toContainEqual([ @@ -107,16 +160,88 @@ describe("BuilderLaunchDialog", () => { expect(copilot.closest("label")).not.toHaveClass("selected"); }); - it("shows the selected recipe's own description", async () => { + it("shows the selected recipe's own description and step list", async () => { const { user } = await setup("n9", [ - { name: "Research and summarize", description: "Researches a topic, then writes a summary.", builtIn: true }, + { + name: "Research and summarize", + description: "Researches a topic, then writes a summary.", + steps: ["Create a web research node", "Write the summary note"], + mode: "copilot", + builtIn: true, + }, ]); - await user.selectOptions(screen.getByLabelText("Recipe"), "Research and summarize"); + await pickRecipe(user, "Research and summarize"); expect( await screen.findByText("Researches a topic, then writes a summary."), ).toBeInTheDocument(); + expect(screen.getByText("Create a web research node")).toBeInTheDocument(); + expect(screen.getByText("Write the summary note")).toBeInTheDocument(); + }); + + it("offers to delete a saved (non-built-in) recipe, and refreshes the list on success", async () => { + const { user, intents, request } = await setup("n9", [ + { name: "My recipe", description: "d", steps: ["one"], mode: "copilot", builtIn: false }, + ]); + + await pickRecipe(user, "My recipe"); + expect(screen.getByRole("button", { name: "Delete this recipe" })).toBeInTheDocument(); + + request.mockImplementation((topic: string, intent: string) => + intent === "listRecipes" ? Promise.resolve({ recipes: [] }) : Promise.resolve(true), + ); + await user.click(screen.getByRole("button", { name: "Delete this recipe" })); + + expect(intents).toContainEqual(["builder", "deleteRecipe", ["My recipe"]]); + // The list is re-fetched and the selection clears back to from-scratch. + expect(await screen.findByText("Describe a build yourself, or pick a saved recipe.")).toBeInTheDocument(); + }); + + it("hides the delete affordance for a built-in recipe", async () => { + const { user } = await setup("n9", [ + { name: "Research and summarize", description: "d", steps: [], mode: "copilot", builtIn: true }, + ]); + + await pickRecipe(user, "Research and summarize"); + + expect(screen.queryByRole("button", { name: "Delete this recipe" })).not.toBeInTheDocument(); + }); + + it("defaults to the Standard budget preset, and a preset click updates the submitted budgets", async () => { + const { user, intents } = await setup(); + + expect(screen.getByRole("button", { name: "Standard" })).toHaveAttribute("aria-pressed", "true"); + expect(screen.getByText(/12 steps · 150k tokens · 15 min/)).toBeInTheDocument(); + + await user.click(screen.getByRole("button", { name: "Quick" })); + expect(screen.getByRole("button", { name: "Quick" })).toHaveAttribute("aria-pressed", "true"); + expect(screen.getByRole("button", { name: "Standard" })).toHaveAttribute("aria-pressed", "false"); + expect(screen.getByText(/6 steps · 50k tokens · 5 min/)).toBeInTheDocument(); + + await user.type(screen.getByLabelText(/what should the builder construct/i), "goal"); + await user.click(screen.getByRole("button", { name: "Plan the build" })); + + expect(intents).toContainEqual(["builder", "start", ["goal", "copilot", 6, 50_000, 300, null]]); + }); + + it("clamps an out-of-range Advanced max-steps value on blur rather than silently reverting a typed 0", async () => { + const { user, intents } = await setup(); + + await user.click(screen.getByText("Advanced")); + const stepsInput = screen.getByLabelText("Max steps"); + await user.clear(stepsInput); + await user.type(stepsInput, "0"); + // Mid-typing, the raw value is preserved (not silently coerced back to + // the default the moment the field reads 0 or empty). + expect(stepsInput).toHaveValue(0); + await user.tab(); // blur + + expect(stepsInput).toHaveValue(1); // clamped to MIN_STEPS, not reverted to 12 + + await user.type(screen.getByLabelText(/what should the builder construct/i), "goal"); + await user.click(screen.getByRole("button", { name: "Plan the build" })); + expect(intents).toContainEqual(["builder", "start", ["goal", "copilot", 1, 150_000, 900, null]]); }); it("a null start result (backend refused) shows an error instead of closing", async () => { @@ -126,10 +251,11 @@ describe("BuilderLaunchDialog", () => { await user.click(screen.getByRole("button", { name: "Plan the build" })); expect(await screen.findByRole("alert")).toHaveTextContent(/could not start/i); + expect(setCenterCalls).toHaveLength(0); }); - it("a successful start closes the dialog", async () => { - const { user } = await setup("n7"); + it("a successful start closes the dialog and centers the viewport on the new plan node", async () => { + const { user } = await setup("n7", [], [{ id: "n7", x: 300, y: 200 }]); await user.type(screen.getByLabelText(/what should the builder construct/i), "goal"); await user.click(screen.getByRole("button", { name: "Plan the build" })); @@ -138,5 +264,10 @@ describe("BuilderLaunchDialog", () => { await screen.findByRole("button", { name: "open builder" }), ).toBeInTheDocument(); expect(screen.queryByLabelText(/what should the builder construct/i)).not.toBeInTheDocument(); + + expect(setCenterCalls).toHaveLength(1); + const [x, y] = setCenterCalls[0]; + expect(x).toBe(300 + 180); + expect(y).toBe(200 + 90); }); }); diff --git a/web_ui/src/app/chrome/BuilderLaunchDialog.tsx b/web_ui/src/app/chrome/BuilderLaunchDialog.tsx index 2373020..bf43e30 100644 --- a/web_ui/src/app/chrome/BuilderLaunchDialog.tsx +++ b/web_ui/src/app/chrome/BuilderLaunchDialog.tsx @@ -1,41 +1,93 @@ import { useEffect, useRef, useState } from "react"; +import { useReactFlow } from "@xyflow/react"; import type { WsTransport } from "../../lib/ws/transport"; +import type { SceneStore } from "../canvas/sceneStore"; +import { motionDuration } from "../reducedMotion"; import { Dialog, useOverlays } from "../overlays/overlays"; +import { CustomSelect } from "./CustomSelect"; /** - * ADR-008 stage 8.3: the Builder's launcher - goal, oversight mode, and + * ADR-008 stage 8.3/8.7: the Builder's launcher - goal, oversight mode, and * the three hard budgets, submitted through `builder/start` (a * value-returning intent: it answers with the new plan node's id, so * transport.request(), the DiagnosticsDialog/KnowledgeSearchDialog - * precedent, not fireIntent). On success the dialog closes and the plan - * node - the visible, editable checklist - takes over on the canvas. + * precedent, not fireIntent). On success the dialog closes, the viewport + * centers on the new plan node (stage 8.7 - it previously landed wherever + * the scene's own extent happened to place it, off-screen as often as not), + * and the plan node - the visible, editable checklist - takes over on the + * canvas. * * Autopilot is a per-run, disclosed choice (the ADR's own decision #3): * selecting it surfaces the disclosure sentence inline, before launch - * graph edits and code execution will proceed WITHOUT per-step approval * (resource caps still apply); network access always still asks. + * + * stage 8.7 rebuild: this was the last dialog in the app still using a bare + * `: CustomSelect is not a native + form control with an id to associate via htmlFor, and it + already carries its own accessible name via ariaLabel below - + the same "visible section text + CustomSelect's own ariaLabel, + no separate
@@ -169,41 +280,76 @@ export function BuilderLaunchDialog({ transport }: { transport: WsTransport }) {
Budgets

Hard limits - a breach pauses the build.

-
- - - +
+ {BUDGET_PRESETS.map((preset) => ( + + ))}
+

{formatBudgetLine(maxSteps, maxTokens, maxWallSeconds)}

+ +
+ Advanced +
+ + + +
+
{error && ( diff --git a/web_ui/src/app/styles.css b/web_ui/src/app/styles.css index 82a055c..d959960 100644 --- a/web_ui/src/app/styles.css +++ b/web_ui/src/app/styles.css @@ -7425,10 +7425,10 @@ mark.document-view-search-match-current { .builder-launch-label { font-size: 13px; font-weight: 600; } .builder-launch-hint { margin: 0; font-size: 11px; color: var(--gl-surface-text-muted); } -/* One shared control box for all three input kinds - the number inputs - previously had no styling at all beyond a width, so they rendered as - stark default-white browser fields inside a dark dialog. */ -.builder-launch-select, +/* stage 8.7: the recipe picker is CustomSelect now (its own component owns + its styling) - this shared control box is what remains for the goal + textarea and the Advanced number fields. Previously also styled the bare +