diff --git a/backend/api/intents_builder.py b/backend/api/intents_builder.py index 472de4fa..ea385e01 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 7276aff1..248b2802 100644 --- a/backend/builder.py +++ b/backend/builder.py @@ -74,6 +74,27 @@ _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 +# review-fix: a tool CALL's name has no upstream length validation - it is +# whatever a provider's tool-call parsing extracted from the model's own +# output verbatim (e.g. providers/ollama_provider.py's _extract_tool_calls), +# so a malformed/hallucinating turn (most reachable with a local model) can +# emit an arbitrarily large one. Every OTHER field this row stores is +# capped; leaving `tool` uncapped would let exactly that turn defeat the +# same wire/session-size bound the ring buffer and summary cap exist for. +_ACTIVITY_TOOL_NAME_CAP = 80 + +# 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 +389,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": _truncate(tool, _ACTIVITY_TOOL_NAME_CAP), + "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 +584,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 +719,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 +786,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/commands.py b/backend/domain/commands.py index ebb753dd..b93bb8c1 100644 --- a/backend/domain/commands.py +++ b/backend/domain/commands.py @@ -236,6 +236,31 @@ def _restore(live: dict, snapshot: dict) -> None: restored.state.builder_awaiting_tool_approval = False restored.state.builder_approval_tool_name = "" restored.state.builder_approval_summary = "" + # review-fix (stage 8.7): builder_activity is documented as run + # TELEMETRY, not reversible document content (PlanState's own + # docstring: "deliberately left untouched by an undo, so the + # record of what happened survives reverting what happened") - + # unlike plan_steps, which genuinely IS reversible content + # (scene/setPlanSteps is A-classified for exactly that reason). + # A command recorded MID-RUN (builderReplan fires on every + # builder.replan call, not just once at the run's start) + # snapshots the node with whatever activity existed at THAT + # instant; restoring that snapshot verbatim would silently erase + # every row logged afterward the moment the command is inverted + # - the same phantom-state class of hazard pending_request_id's + # unconditional reset above already guards against, just for a + # different field. Carries the CURRENT node's activity log + # forward instead of trusting the snapshot's stale copy - `live` + # still holds the pre-restore node at this point, one line + # before it is replaced. A node that does not currently exist + # (this restore is recreating a deleted one) has no "current" to + # preserve, so the snapshot's own activity is used as-is - the + # only case where trusting the snapshot is correct. + current = live.get(key) + if isinstance(getattr(restored, "state", None), PlanState) and isinstance( + getattr(current, "state", None), PlanState, + ): + restored.state.builder_activity = current.state.builder_activity live[key] = restored diff --git a/backend/domain/graph.py b/backend/domain/graph.py index 8aaadc77..aac0b6bd 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 0d62b08a..ac02530e 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 82e2bbdf..d815fdb7 100644 --- a/backend/session_load.py +++ b/backend/session_load.py @@ -713,6 +713,25 @@ 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"): + # review-fix: elapsedMs is untrusted input from a saved file + # (hand-edited, or written by an older/different format) - a + # non-numeric value must degrade to 0, the same tolerance every + # other field on this row already gets via str(), not crash the + # whole session load the way a bare int() would. + try: + elapsed_ms = int(raw.get("elapsedMs", 0) or 0) + except (TypeError, ValueError): + elapsed_ms = 0 + activity.append({ + "tool": str(raw.get("tool", "")), + "summary": str(raw.get("summary", "")), + "outcome": str(raw.get("outcome", "ok")), + "stepId": str(raw.get("stepId", "")), + "elapsedMs": elapsed_ms, + }) return SceneNode( id="", x=x, y=y, title=f"Build: {goal[:40]}" if goal else "Build", @@ -721,6 +740,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 79e23bcc..a0d42814 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 e11c64ca..111252c0 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,402 @@ 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 + + def test_session_load_drops_malformed_activity_entries_instead_of_crashing(self): + from backend.session_load import _restore_plan_payload + + payload = { + "goal": "g", + "activity": [ + {"tool": "graph.create_node", "summary": "ok row", "outcome": "ok", "stepId": "s1", "elapsedMs": 5}, + "not a dict", + {"summary": "missing the tool key entirely"}, + {"tool": "run_node", "summary": "s", "outcome": "ok", "stepId": "s1", "elapsedMs": "not a number"}, + ], + } + + restored = _restore_plan_payload(payload) + + assert len(restored.state.builder_activity) == 2, ( + "the non-dict entry and the entry missing 'tool' are dropped, " + "matching plan_steps' own malformed-entry tolerance" + ) + assert restored.state.builder_activity[0]["tool"] == "graph.create_node" + assert restored.state.builder_activity[1]["elapsedMs"] == 0, ( + "a non-numeric elapsedMs coerces to 0 rather than crashing session load" + ) + + def test_review_fix_undo_run_does_not_revert_activity_logged_after_a_mid_run_replan(self, monkeypatch): + """A command recorded mid-run (builderReplan fires on every + builder.replan call, not just once) snapshots the plan node with + whatever activity existed at that instant. Reverting that command - + via undo_run, which walks the whole run's commands - must not + silently erase rows logged afterward; see commands.py's own + review-fix in _restore for the mechanism.""" + document, dispatcher, registry, bus = make_harness() + node = seed_plan(document, ["s1", "s2", "s3"]) + scripted_turns(monkeypatch, [ + {"tool_calls": [call("c1", "builder.complete_step", summary="s1 done")]}, + # The replan's own command snapshots the node right after this + # call's own activity row is appended. + {"tool_calls": [call("c2", "builder.replan", steps=["new s3"], reason="r")]}, + # Everything below is logged AFTER that snapshot. + {"tool_calls": [call("c3", "graph.create_node", kind="note", content="after replan")]}, + {"tool_calls": [call("c4", "builder.complete_step", summary="s2 done")]}, + {"tool_calls": [call("c5", "builder.complete_step", summary="new s3 done")]}, + ], []) + + asyncio.run(drive_build(document, dispatcher, registry, bus, node)) + + assert node.state.builder_status == "done" + activity_before_undo = list(node.state.builder_activity) + assert len(activity_before_undo) == 5, "one row per invoked call" + + document.undo_run(node.state.builder_run_id) + + assert node.state.builder_activity == activity_before_undo, ( + "undo_run inverts the builderReplan command among the run's " + "others - its mid-run snapshot must not erase rows logged after it" + ) + + def test_activity_is_a_capped_ring_buffer_at_the_exact_boundary(self, monkeypatch): + """review-fix: the existing ring-buffer test only checks aggregate + length and the newest row after a large overshoot; this pins the + exact trim at the boundary - one run under the cap, one exactly at + it, one one-over - and which specific rows survive.""" + document, dispatcher, registry, bus = make_harness() + node = seed_plan(document, ["one step"], mode="autopilot", max_tokens=10_000_000, max_wall_seconds=10_000) + # _ACTIVITY_CAP + 1 create_node calls, then complete_step - the cap + # is breached by exactly one row. + calls = [ + call(f"c{i}", "graph.create_node", kind="note", content=str(i)) + for i in range(builder_module._ACTIVITY_CAP + 1) + ] + calls.append(call("cfinal", "builder.complete_step", summary="done")) + scripted_turns(monkeypatch, [{"tool_calls": calls}], []) + + asyncio.run(drive_build(document, dispatcher, registry, bus, node)) + + activity = node.state.builder_activity + assert len(activity) == builder_module._ACTIVITY_CAP + # The trim runs after EVERY append that exceeds the cap, not once + # at the end: 101 create_node calls (content "0".."100") first push + # the log to 101 rows, dropping content "0"; the trailing + # complete_step then pushes it to 101 again, dropping content "1" + # too - "2" is the oldest of the two content rows to survive both + # trims. + assert activity[0]["summary"] == '{"content": "2", "kind": "note"}' + assert activity[-1]["tool"] == "builder.complete_step" + + def test_activity_tool_name_and_summary_are_both_truncated_when_over_cap(self, monkeypatch): + document, dispatcher, registry, bus = make_harness() + node = seed_plan(document, ["one step"]) + long_tool_name = "graph." + "x" * 200 + long_content = "y" * 500 + + from backend.providers.base import ToolSpec + from backend.tools import GRAPH_READ, ToolResult + + async def oversized_handler(call, ctx): + return ToolResult(content="ok") + + registry.register( + ToolSpec(name=long_tool_name, description="d", input_schema={"type": "object"}), + oversized_handler, scopes={GRAPH_READ}, approval="auto", + ) + scripted_turns(monkeypatch, [ + {"tool_calls": [call("c1", long_tool_name, content=long_content)]}, + {"tool_calls": [call("c2", "builder.complete_step", summary="done")]}, + ], []) + + asyncio.run(drive_build(document, dispatcher, registry, bus, node)) + + row = node.state.builder_activity[0] + # Pinned explicitly: an error result would ALSO produce a long, + # truncated summary (the error text embeds the oversized tool + # name), so without this the assertions below could pass for the + # wrong reason - the success path never actually being exercised. + assert row["outcome"] == "ok" + # _truncate appends a trailing ellipsis character on top of the cap, + # so the truncated length is cap + 1, not cap itself. + assert len(row["tool"]) == builder_module._ACTIVITY_TOOL_NAME_CAP + 1 + assert row["tool"].endswith("…") + assert len(row["summary"]) == builder_module._ACTIVITY_SUMMARY_CAP + 1 + assert row["summary"].endswith("…") + + +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) + + def test_review_fix_an_explicit_parent_id_equal_to_the_anchor_does_not_overlap_an_anchor_placed_sibling(self): + """The executor prompt tells the model the plan node's own id + (builder.py: "The plan node's id is {plan_node_id}."), and nothing + stops it passing that back as parent_id for a chat/code node - + which used to count siblings by EDGE (invisible to a note the + anchor branch placed with no edge at all), landing the two nodes on + top of each other. Both branches must now agree on the same row.""" + document = SceneDocument() + plan = document.add_plan_node(200.0, 200.0, "goal") + + x1, y1 = _place_child(document, None, plan.id) # parentless -> anchor branch + document.add_note(x1, y1) + x2, y2 = _place_child(document, plan.id, plan.id) # parent_id IS the anchor + + assert (x2, y2) != (x1, y1), "must not collide with the anchor-placed sibling" + assert (x2, y2) == (x1 + tools_graph_module._SIBLING_HORIZONTAL_SPACING, y1) + + def test_review_fix_a_real_parent_distinct_from_the_anchor_keeps_its_own_edge_based_counting(self): + """The unification fix must be scoped to parent_id == anchor_id + only - a normal parent (anything else) keeps exactly its prior, + unrelated-to-the-builder edge-based placement.""" + document = SceneDocument() + plan = document.add_plan_node(0.0, 0.0, "goal") + parent = document.add_chat_node(500.0, 500.0, "p", True) + + x, y = _place_child(document, parent.id, plan.id) + + assert (x, y) == (parent.x, parent.y + MESSAGE_VERTICAL_SPACING) + + +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 1c861348..7b468d8b 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,64 @@ 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, - ) + same parent land side by side instead of stacked. + + review-fix (stage 8.7): the plan node can be reached by EITHER path - + as an explicit parent_id (the executor prompt tells the model the plan + node's own id, and nothing stops it passing that id back as parent_id + for a chat/code node) or as the implicit anchor for a parentless create. + The two branches below used to count siblings differently (edges vs. + position), so a parentless create landing via the anchor branch was + invisible to the parent branch's edge count and vice versa - two nodes + from the two different paths could land on the exact same coordinates. + `parent_id != anchor_id` routes that one specific case (parent_id IS the + anchor) into the position-based branch below, which sees every node in + the row regardless of which path placed it. A normal parent - anything + other than the anchor - is completely unaffected.""" + if parent_id is not None and parent_id in document.nodes and parent_id != anchor_id: + 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, + ) + reference_id = parent_id or anchor_id + if reference_id is not None and reference_id in document.nodes: + # 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 + # reference's own placement row is this same reference's own + # earlier creation (nothing else places there), so counting THOSE + # stands in for "existing children" without needing a persisted + # counter - and, per the docstring above, catches siblings placed + # via EITHER path. + reference = document.nodes[reference_id] + row_y = reference.y + MESSAGE_VERTICAL_SPACING + row_siblings = sum( + 1 for n in document.nodes.values() if n.x >= reference.x and abs(n.y - row_y) < 1.0 + ) + return reference.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 +261,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 ea59de1f..7c20ab1a 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 647596e3..0b1ffe72 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 63e6da92..dc4327c7 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 b9576358..b2508760 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/app/App.tsx b/web_ui/src/app/App.tsx index 0381c797..0372d14a 100644 --- a/web_ui/src/app/App.tsx +++ b/web_ui/src/app/App.tsx @@ -505,7 +505,7 @@ function App() { - + diff --git a/web_ui/src/app/canvas/PlanNodeView.test.tsx b/web_ui/src/app/canvas/PlanNodeView.test.tsx index c556d011..b2e3479a 100644 --- a/web_ui/src/app/canvas/PlanNodeView.test.tsx +++ b/web_ui/src/app/canvas/PlanNodeView.test.tsx @@ -1,3 +1,4 @@ +import type { ReactElement } from "react"; import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { ReactFlowProvider } from "@xyflow/react"; @@ -15,6 +16,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", @@ -42,8 +44,8 @@ function makeData(overrides: Partial = {}): PlanNodeData { }; } -function renderPlan(data: PlanNodeData) { - return render( +function planElement(data: PlanNodeData) { + return ( - , + ); } +function renderPlan(data: PlanNodeData) { + return render(planElement(data)); +} + +// ReactFlowProvider remounting on every rerender would reset internal state +// unrelated to this test - reusing the exact same element shape as +// renderPlan keeps rerender() a like-for-like prop update instead. +function rerenderPlan(rerender: (ui: ReactElement) => void, data: PlanNodeData) { + rerender(planElement(data)); +} + describe("PlanNodeView", () => { it("renders the goal, every step with its status, and the budget line", () => { renderPlan(makeData()); @@ -183,4 +196,84 @@ 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(); + }); + + it("review-fix: only auto-scrolls while running AND the disclosure is open - not collapsed, not landed", () => { + const scrollToSpy = vi.fn(); + const originalScrollTo = Element.prototype.scrollTo; + Element.prototype.scrollTo = scrollToSpy; + const row = (n: number) => ({ tool: `tool-${n}`, summary: "{}", outcome: "ok", stepId: "s1", elapsedMs: n }); + + try { + const { rerender } = renderPlan(makeData({ builderStatus: "running", builderActivity: [row(1)] })); + const details = document.querySelector(".plan-node-activity") as HTMLDetailsElement; + expect(details.open).toBe(false); // collapsed by default + + rerenderPlan(rerender, makeData({ builderStatus: "running", builderActivity: [row(1), row(2)] })); + expect(scrollToSpy).not.toHaveBeenCalled(); // still collapsed - must not scroll a hidden panel + + details.open = true; // the same native toggle a real click on performs + rerenderPlan(rerender, makeData({ builderStatus: "running", builderActivity: [row(1), row(2), row(3)] })); + expect(scrollToSpy).toHaveBeenCalledTimes(1); // running + open - follows the newest row + + scrollToSpy.mockClear(); + rerenderPlan(rerender, makeData({ + builderStatus: "done", pendingRequestId: null, + builderActivity: [row(1), row(2), row(3), row(4)], + })); + expect(scrollToSpy).not.toHaveBeenCalled(); // landed - stops following, doesn't yank a spot the user scrolled to + } finally { + Element.prototype.scrollTo = originalScrollTo; + } + }); + }); }); diff --git a/web_ui/src/app/canvas/PlanNodeView.tsx b/web_ui/src/app/canvas/PlanNodeView.tsx index 34c61fc8..5bad6d55 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 83044bb5..ce37363c 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 4d821689..bfa01bbe 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 a08a915f..6b67e0ff 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 063c67eb..e99edf5f 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 c29663e0..bb4c22b4 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 40b7ce95..7dd83c49 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/chrome/BuilderLaunchDialog.test.tsx b/web_ui/src/app/chrome/BuilderLaunchDialog.test.tsx index 92ba3927..d85d826b 100644 --- a/web_ui/src/app/chrome/BuilderLaunchDialog.test.tsx +++ b/web_ui/src/app/chrome/BuilderLaunchDialog.test.tsx @@ -1,6 +1,38 @@ +/** + * stage 8.7 rebuild. Mirrors GlobalSearchDialog.test.tsx's own useReactFlow + * wrapping (every real pan/zoom export stays functional; only setCenter is + * intercepted) for asserting the post-launch focus call's exact arguments, + * and CustomSelect.test.tsx's own "click the trigger by its ariaLabel, then + * click the option by its label" interaction shape for the recipe picker - + * userEvent.selectOptions no longer applies now that this is not a native + * ` (CustomSelect exists precisely to retire that - see its own + * module doc) and three unlabelled-scale number spinners with no sense of + * what they cost. Now: CustomSelect for the recipe picker, a live preview + * of a selected recipe's own steps (the payload always carried them - + * nothing rendered them), named budget TIERS with a plain-language summary + * line in place of raw numbers, and the exact numbers moved behind an + * "Advanced" disclosure for whoever wants them. Recipe deletion (builder/ + * deleteRecipe) rounds out the lifecycle saveRecipe started. */ const DEFAULT_MAX_STEPS = 12; const DEFAULT_MAX_TOKENS = 150_000; const DEFAULT_MAX_WALL_SECONDS = 900; +// Mirrors intents_builder.py's own _MIN/_MAX_*_BUDGET constants exactly - +// the backend clamps to these regardless, but the field should visually +// settle on the value it is actually about to submit. +const MIN_STEPS = 1; +const MAX_STEPS = 50; +const MIN_TOKENS = 1_000; +const MAX_TOKENS = 2_000_000; +const MIN_WALL_SECONDS = 30; +const MAX_WALL_SECONDS = 7_200; + +// Named tiers in place of three bare numbers - "150000 tokens" means +// nothing on its own sight-read; a tier name plus a plain-language budget +// line does. Standard matches the launcher's own prior hardcoded defaults +// exactly, so an existing habit of "just launch" is unaffected. +const BUDGET_PRESETS = [ + { id: "quick", label: "Quick", maxSteps: 6, maxTokens: 50_000, maxWallSeconds: 300 }, + { id: "standard", label: "Standard", maxSteps: DEFAULT_MAX_STEPS, maxTokens: DEFAULT_MAX_TOKENS, maxWallSeconds: DEFAULT_MAX_WALL_SECONDS }, + { id: "extended", label: "Extended", maxSteps: 25, maxTokens: 400_000, maxWallSeconds: 1_800 }, +] as const; + +function clamp(value: number, low: number, high: number, fallback: number): number { + if (!Number.isFinite(value)) return fallback; + return Math.min(Math.max(value, low), high); +} + +function formatBudgetLine(steps: number, tokens: number, seconds: number): string { + const minutes = Math.max(1, Math.round(seconds / 60)); + return `${steps} step${steps === 1 ? "" : "s"} · ${Math.round(tokens / 1000).toLocaleString()}k tokens · ${minutes} min`; +} + interface RecipeRow { name: string; description: string; + goal: string; + steps: string[]; + mode: "copilot" | "autopilot"; builtIn: boolean; } -export function BuilderLaunchDialog({ transport }: { transport: WsTransport }) { +export function BuilderLaunchDialog({ transport, store }: { transport: WsTransport; store: SceneStore }) { const overlays = useOverlays(); + const reactFlow = useReactFlow(); const [goal, setGoal] = useState(""); const [mode, setMode] = useState<"copilot" | "autopilot">("copilot"); - const [maxSteps, setMaxSteps] = useState(DEFAULT_MAX_STEPS); - const [maxTokens, setMaxTokens] = useState(DEFAULT_MAX_TOKENS); - const [maxWallSeconds, setMaxWallSeconds] = useState(DEFAULT_MAX_WALL_SECONDS); + const [maxSteps, setMaxSteps] = useState(DEFAULT_MAX_STEPS); + const [maxTokens, setMaxTokens] = useState(DEFAULT_MAX_TOKENS); + const [maxWallSeconds, setMaxWallSeconds] = useState(DEFAULT_MAX_WALL_SECONDS); const [recipes, setRecipes] = useState([]); const [recipe, setRecipe] = useState(""); const [starting, setStarting] = useState(false); + const [deletingRecipe, setDeletingRecipe] = useState(false); const [error, setError] = useState(null); const latestRequestId = useRef(0); const open = overlays.isOpen("builder-launch"); @@ -73,6 +125,18 @@ export function BuilderLaunchDialog({ transport }: { transport: WsTransport }) { if (nodeId != null) { setGoal(""); overlays.close(); + // The launcher has no canvas anchor of its own, so the plan node + // can land anywhere the scene's own extent happens to place it - + // often off the current viewport entirely. intents_builder.start() + // already awaited publish_scene() before answering with this id, + // so the node's position is already in the store by the time this + // callback fires. Center is approximate (the node's real rendered + // size isn't known until React Flow measures it) - this only + // needs to bring it into view, not frame it exactly. + const node = store.getScene().nodes.find((n) => n.id === nodeId); + if (node) { + reactFlow.setCenter(node.x + 180, node.y + 90, { duration: motionDuration(300) }); + } } else { setError("The build could not start - see the notification for details."); } @@ -86,34 +150,91 @@ export function BuilderLaunchDialog({ transport }: { transport: WsTransport }) { }); } - // The selected recipe's own description - listRecipes has always - // returned it, but nothing ever rendered it, so the picker gave no clue - // what a recipe actually builds until after launching it. + function deleteSelectedRecipe() { + if (!recipe || deletingRecipe) return; + setDeletingRecipe(true); + transport + .request("builder", "deleteRecipe", [recipe]) + .then((deleted) => { + // A resolved `false` (unknown name, or a refused built-in) is NOT a + // rejected promise - deleteRecipe's own backend already surfaced + // why via its own notification (backend/api/intents_builder.py). + // Refreshing the list and clearing the selection here regardless + // would silently claim success on a call that changed nothing. + if (!deleted) return; + return transport.request("builder", "listRecipes", []).then((value) => { + const payload = value as { recipes: RecipeRow[] }; + setRecipes(payload.recipes ?? []); + setRecipe(""); + }); + }) + .catch(() => { + setError("The recipe could not be deleted - see graphlink.log for details."); + }) + .finally(() => setDeletingRecipe(false)); + } + + // The selected recipe's own description AND steps - listRecipes has + // always returned both, but nothing ever rendered either, so the picker + // gave no clue what a recipe actually builds until after launching it. const selectedRecipe = recipes.find((r) => r.name === recipe) ?? null; + const selectedPresetId = + BUDGET_PRESETS.find( + (preset) => + preset.maxSteps === maxSteps && + preset.maxTokens === maxTokens && + preset.maxWallSeconds === maxWallSeconds, + )?.id ?? null; return (
- - -

- {selectedRecipe?.description || "Describe a build yourself, or pick a saved recipe."} -

+ options={[ + { id: "", label: "Start from scratch" }, + ...recipes.map((r) => ({ + id: r.name, + label: r.builtIn ? `${r.name} (built-in)` : r.name, + description: r.description || undefined, + })), + ]} + onChange={setRecipe} + ariaLabel="Recipe" + /> + {selectedRecipe ? ( +
+ {selectedRecipe.description && ( +

{selectedRecipe.description}

+ )} + {selectedRecipe.steps.length > 0 && ( +
    + {selectedRecipe.steps.map((title, index) => ( +
  1. {title}
    2. + ))} +
+ )} + {!selectedRecipe.builtIn && ( + + )} +
+ ) : ( +

Describe a build yourself, or pick a saved recipe.

+ )}
@@ -169,41 +290,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/chrome/OnboardingDialog.tsx b/web_ui/src/app/chrome/OnboardingDialog.tsx index ca332778..949a5d40 100644 --- a/web_ui/src/app/chrome/OnboardingDialog.tsx +++ b/web_ui/src/app/chrome/OnboardingDialog.tsx @@ -122,7 +122,9 @@ export function OnboardingDialog({ transport, store }: { transport: WsTransport;

Graphlink is a visual AI workspace: every message becomes a node on a canvas, and you can branch a - conversation from any earlier node instead of only ever continuing the last one. + conversation from any earlier node instead of only ever continuing the last one. When a task needs more + than one step, the Builder can plan and construct a whole branch for you, supervised at whatever level + you choose.

This short wizard checks your provider and offers a small sample workspace, then gets out of the way.

diff --git a/web_ui/src/app/chrome/help-data/sections.ts b/web_ui/src/app/chrome/help-data/sections.ts index 09e1ed28..f9b8febd 100644 --- a/web_ui/src/app/chrome/help-data/sections.ts +++ b/web_ui/src/app/chrome/help-data/sections.ts @@ -329,6 +329,54 @@ export const HELP_SECTIONS: HelpSection[] = [ } ] }, + { + "name": "Builder", + "description": "An agent that plans a checklist for a goal, then constructs it on the canvas one supervised step at a time - the same tools you could use by hand, run for you.", + "subsections": [ + { + "title": "How a Build Works", + "items": [ + { + "action": "Goal, Plan, Build", + "description": "Give the Builder a goal, or pick a saved recipe, and it drafts a short checklist first. The plan is a real node on the canvas - review it, edit it if you want, and only then start the build." + }, + { + "action": "What It Can Build", + "description": "Create and edit chat, note, code, document, and web research nodes; execute Python; generate charts and assistant replies from a node's content; run web research and search your knowledge base - the same actions available to you, driven for you." + }, + { + "action": "Oversight: Co-pilot vs. Autopilot", + "description": "Co-pilot asks you to approve every mutating step. Autopilot runs to completion within its budgets, creating and editing nodes and executing code without asking - network access still asks every time, in either mode." + }, + { + "action": "Budgets Are Hard Limits", + "description": "Steps, tokens, and time are capped before a build starts. A budget breach pauses the build with its state intact rather than losing progress - raise the limit and resume to pick up exactly where it stopped." + } + ] + }, + { + "title": "The Plan Node", + "items": [ + { + "action": "Watch It Build", + "description": "The plan node shows each step's status live, plus an expandable activity log of every tool call the build made and whether it succeeded." + }, + { + "action": "Pause, Stop, and Resume", + "description": "A build can be stopped at any time. A paused, stopped, or failed build resumes from exactly where it left off - even after restarting the app, since the plan node itself is the resume point." + }, + { + "action": "Undo a Build", + "description": "Once a build finishes or stops, Undo Build reverts everything it did in one action, the same as undoing any other change." + }, + { + "action": "Save and Reuse Recipes", + "description": "Turn a finished build's plan into a reusable recipe from its own node's Save as Recipe action. A saved recipe can be deleted from the launcher when you no longer need it - built-in recipes cannot be removed." + } + ] + } + ] + }, { "name": "Settings & Models", "description": "How runtime modes, providers, model routing, and quality-of-life settings shape the behavior of the application.", diff --git a/web_ui/src/app/styles.css b/web_ui/src/app/styles.css index 8900b964..58b5cab1 100644 --- a/web_ui/src/app/styles.css +++ b/web_ui/src/app/styles.css @@ -7351,6 +7351,36 @@ 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; +} + +/* review-fix: flexbox rather than float, matching the "label + trailing + value" idiom used everywhere else in the app (ViewPopover's own + .view-field-row and 20+ others) - the only float in this stylesheet + otherwise. Scoped to this activity section specifically (not a bare + .chat-node-tool-invocation-name override) so ChatNodeView's own use of + that class, which has no trailing value to align, is unaffected. */ +.plan-node-activity .chat-node-tool-invocation-name { + display: flex; + justify-content: space-between; + gap: 8px; +} + +.plan-node-activity-elapsed { + font-weight: 400; + color: var(--gl-surface-text-muted); +} .plan-node-approval { border: 1px solid var(--gl-semantic-status-warning); border-radius: 6px; @@ -7406,10 +7436,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 +