From 8f9a68266f9ec06a72324279ee9958cd0d16d092 Mon Sep 17 00:00:00 2001 From: dovvnloading Date: Tue, 28 Jul 2026 19:25:25 -0400 Subject: [PATCH] R8a: restore the Key Takeaway and Explainer Note agents Both chat-node menu items rendered as disabled stubs whose tooltip read "AI note generation isn't available yet". That tooltip had been wrong since R4: the agent layer it blamed is the same one Regenerate Response, Generate Image and Generate Chart have used ever since. The real cause was that KeyTakeawayAgent and ExplainerAgent lived in graphlink_agents_core.py and were deleted with graphlink_app/ in the R7.6b cutover, and nobody ported them. Nothing was blocked. graphlink_note_agent.py restores both, carrying over the prompts, the clean_agent_markdown_response post-processor and the one-chat-call shape verbatim so output matches what the Qt app produced. Two divergences, both because the machinery they relied on is gone: - No text bounding. Legacy passed source text through render_context/ source_snapshot, which died with the Qt app and has no successor. Every surviving agent path feeds unbounded text today, so this matches them rather than reintroducing a single bounded path. - No QThread workers. The dispatcher owns concurrency now, so the legacy worker classes have no counterpart. AgentDispatcher.start_note_generation follows start_chart_generation's directly-awaited shape for the same reason: the result is a brand new note node, so the caller needs it in the same round trip and there is no existing node to hang a spinner on. One guard covers both agents - they are the same gesture differing only in prompt, and running them concurrently would race two notes onto overlapping positions. An empty agent response is treated as a failure rather than becoming a blank note. The note is placed 400px to the right of its source node, tinted with the frontend palette's Mid Gray body and Blue header - the closest surviving equivalents to legacy's Mid Gray + status_info pairing, since no status_info token exists in the new stack. Legacy's note width of 400 is not ported: note width is CSS-driven and not a modeled field. Source text is the node's OWN content, not the branch history generateChart uses - widening it would change what the feature summarises. The frontend test that asserted both items were disabled is inverted rather than deleted: it was the guard encoding the stub as correct, so it now encodes the opposite. Co-Authored-By: Claude Opus 5 --- backend/agents.py | 104 +++++++++++ backend/canvas.py | 87 +++++++++ backend/tests/test_agent_layer_qt_free.py | 10 + backend/tests/test_agents.py | 121 ++++++++++++ backend/tests/test_canvas.py | 126 +++++++++++++ graphlink_note_agent.py | 194 ++++++++++++++++++++ web_ui/src/app/canvas/ChatNodeView.test.tsx | 51 ++++- web_ui/src/app/canvas/ChatNodeView.tsx | 43 ++++- web_ui/src/app/canvas/SceneCanvas.tsx | 5 + web_ui/src/app/canvas/sceneStore.ts | 14 ++ 10 files changed, 745 insertions(+), 10 deletions(-) create mode 100644 graphlink_note_agent.py diff --git a/backend/agents.py b/backend/agents.py index bfbfc98..739087c 100644 --- a/backend/agents.py +++ b/backend/agents.py @@ -74,6 +74,7 @@ from graphlink_artifact_agent import ArtifactAgent from graphlink_chart_agent import ChartDataAgent from graphlink_chat_agent import ChatAgent +from graphlink_note_agent import ExplainerAgent, KeyTakeawayAgent from graphlink_licensing import SettingsManager # type hint only from graphlink_plugins.common.github_client import GitHubRestClient from graphlink_plugins.gitlink.agent import GitlinkAgent, _fingerprint_changes, _is_repo_text_path @@ -343,6 +344,19 @@ def __init__(self, settings_manager: SettingsManager): # no checkpoint to insert one at, and its own legacy caller, # ChartWorkerThread, has no stop() method either). self._chart_requests: dict[str, bool] = {} + # R8a: a TENTH independent in-flight-request GUARD, same directly- + # awaited shape (and therefore same dict-of-sentinels rather than + # dict-of-tasks) as self._chart_requests above, for the same reason: + # Key Takeaway / Explainer Note each create a brand new note node, so + # the caller needs the result back in the same round trip and there is + # no pre-existing node to hang a spinner on. ONE guard covers both + # agents deliberately - they are the same user-facing gesture + # ("summarise this node into a note") differing only in prompt, and + # letting a takeaway and an explainer run concurrently would race two + # notes onto overlapping canvas positions for no benefit. + # request_id -> True; no task/cancel_event to store, since the legacy + # workers these replace had no stop() either. + self._note_requests: dict[str, bool] = {} # R5.4: Py-Coder's REPL subprocess outlives any single run (state # persists between calls, same as legacy's own PyCoderReplManager - # see that class's own docstring in graphlink_plugins/pycoder/domain.py @@ -2204,6 +2218,96 @@ async def _invoke(fn, *a): finally: self._chart_requests.pop(request_id, None) + async def start_note_generation( + self, + *, + bus: SessionBus, + notifications_state, + node_id: str, + note_kind: str, + source_text: str, + on_success, + on_failure, + ) -> None: + """R8a: Key Takeaway / Explainer Note generation. + + DIRECTLY AWAITED by its caller rather than scheduled via + asyncio.create_task, the same shape as start_chart_generation above + and for the same reason: the result is a brand new note node, so the + caller needs it back in the same round trip and there is no + pre-existing node to attach a spinner to. + + `note_kind` selects the agent ("takeaway" | "explainer"). One method + rather than two near-identical ones because the two differ ONLY in + which agent class runs and how failures are worded - the guard, + timeout, callback and cleanup logic are identical, and duplicating + them would be two places to fix every future bug in. + """ + label = NOTE_AGENT_LABELS.get(note_kind, "Note") + + if self._note_requests: + notifications_state.show(f"A {label.lower()} is already being generated.", "info") + await bus.publish("notification") + return + + request_id = uuid.uuid4().hex + self._note_requests[request_id] = True + + async def _invoke(fn, *a): + if inspect.iscoroutinefunction(fn): + await fn(*a) + else: + fn(*a) + + try: + text = await asyncio.wait_for( + asyncio.to_thread(_call_note_agent, note_kind, source_text), + timeout=WATCHDOG_TIMEOUT_SECONDS, + ) + if not str(text or "").strip(): + # An agent that returns nothing usable must not silently + # create an empty note - that reads as a broken feature. + message = f"{label} generation returned an empty response. Please try again." + await _invoke(on_failure, message) + notifications_state.show(message, "error") + await bus.publish("notification") + else: + await _invoke(on_success, text) + except asyncio.TimeoutError: + message = ( + f"{label} generation stopped responding before the request completed. " + "Please try again." + ) + await _invoke(on_failure, message) + notifications_state.show(message, "error") + await bus.publish("notification") + except Exception as exc: + logger.exception("%s generation dispatch failed (source node %s)", label, node_id) + message = f"{label} generation failed: {exc}" + await _invoke(on_failure, message) + notifications_state.show(message, "error") + await bus.publish("notification") + finally: + self._note_requests.pop(request_id, None) + + +# R8a: the two note agents, keyed by the `note_kind` start_note_generation +# takes. Kept as data rather than an if/elif so adding a third note agent is +# one entry, not another branch in three places. +NOTE_AGENT_LABELS = {"takeaway": "Key takeaway", "explainer": "Explainer note"} +_NOTE_AGENTS = {"takeaway": KeyTakeawayAgent, "explainer": ExplainerAgent} + + +def _call_note_agent(note_kind: str, source_text: str) -> str: + """Runs inside asyncio.to_thread - the blocking driver for + start_note_generation above, mirroring _call_chart_agent's own shape + (fresh agent instance per call). Returns the agent's already-cleaned + text; unlike the chart agent there is no JSON to parse.""" + agent_cls = _NOTE_AGENTS.get(note_kind) + if agent_cls is None: + raise ValueError(f"unknown note kind: {note_kind}") + return agent_cls().get_response(source_text) + def _call_pycoder_execution_agent(conversation_history, user_prompt) -> str: """Runs inside asyncio.to_thread. Reuses PyCoderExecutionAgent.get_response diff --git a/backend/canvas.py b/backend/canvas.py index 30619d4..46c9b1e 100644 --- a/backend/canvas.py +++ b/backend/canvas.py @@ -187,6 +187,17 @@ def _decode_image_bytes(data): # find_branch_position packing algorithm (a later refinement). MESSAGE_VERTICAL_SPACING = 160 +# R8a: where a generated Key Takeaway / Explainer Note lands relative to its +# source chat node, and how it is tinted. 400px clears a chat node's own +# width (~292px) with room to spare, matching the legacy offset. The colours +# are hex because the backend never resolves colour NAMES (see SceneNode's +# own comment) - these two are the frontend palette's "Mid Gray" body and +# "Blue" header, the closest surviving equivalents to legacy's Mid Gray + +# status_info pairing (there is no status_info token in the new stack). +NOTE_AGENT_X_OFFSET = 400 +NOTE_AGENT_BODY_COLOR = "#7a7a7a" +NOTE_AGENT_HEADER_COLOR = "#3f7dc9" + # R6.1: Notes/Frames/Containers - legacy canvas decorations, ported for the # first time. _recompute_group_bounds (below, on SceneDocument) is plain # server-side math, NOT a React Flow extent/parentId feature - it computes a @@ -3636,6 +3647,78 @@ def _on_failure(message): ) return result_holder.get("node_id") + async def _generate_note_from_node(source_node_id, note_kind, x_offset, y_offset): + """R8a: shared path for generateKeyTakeaway and generateExplainerNote. + + Both take one chat node, run its text through an agent, and drop the + result into a new note beside it - identical except for the agent and + the note's offset, so they share one implementation rather than two + that can drift. + + Source text is the node's OWN content, not the branch history that + generate_chart uses: legacy's takeaway/explainer passed a single + node's text, and widening that to the whole branch would change what + the feature summarises. + """ + if not source_node_id or source_node_id not in document.nodes: + notifications.show("Please select a valid node first.", "warning") + await bus.publish("notification") + return None + + source = document.nodes[source_node_id] + if source.kind != "chat": + notifications.show("This node can't be summarised into a note.", "warning") + await bus.publish("notification") + return None + if not source.content or not source.content.strip(): + notifications.show("The selected node has no text to summarise.", "warning") + await bus.publish("notification") + return None + + result_holder: dict[str, str] = {} + + async def _on_success(text): + if source_node_id not in document.nodes: + # Deleted mid-flight - silent no-op, same posture as + # _dispatch_image's own liveness check. + return + note = document.add_note(source.x + x_offset, source.y + y_offset) + document.set_note_content(note.id, text) + # Legacy tinted these notes "Mid Gray" with an info-coloured + # header. Both values come from the frontend's own palette + # (GroupColorPicker's GROUP_MONO_COLORS/GROUP_NAMED_COLORS) since + # the backend stores hex and never resolves a colour name. The + # legacy note width of 400 is NOT ported: note width is not a + # modeled field here (it is CSS-driven), so there is nothing to + # set it on. + document.set_group_color(note.id, NOTE_AGENT_BODY_COLOR, NOTE_AGENT_HEADER_COLOR) + result_holder["node_id"] = note.id + await bus.publish("scene") + + def _on_failure(message): + # start_note_generation already surfaced the notification. + pass + + await agent_dispatcher.start_note_generation( + bus=bus, + notifications_state=notifications, + node_id=source_node_id, + note_kind=note_kind, + source_text=source.content, + on_success=_on_success, + on_failure=_on_failure, + ) + return result_holder.get("node_id") + + async def generate_key_takeaway(source_node_id): + return await _generate_note_from_node(source_node_id, "takeaway", NOTE_AGENT_X_OFFSET, 0) + + async def generate_explainer_note(source_node_id): + # Offset vertically as well as horizontally so a takeaway and an + # explainer generated from the same node don't land on top of each + # other - the same 100px stagger legacy used. + return await _generate_note_from_node(source_node_id, "explainer", NOTE_AGENT_X_OFFSET, 100) + async def resize_chart(node_id, width, height): document.resize_chart(node_id, width, height) await publish_scene() @@ -4138,6 +4221,10 @@ async def set_view_state(zoom_factor, scroll_x, scroll_y): # R6.2: Chart - a single combined create+generate action, unlike every # node-creation flow above - see generate_chart's own docstring. bus.register_intent("scene", "generateChart", generate_chart) + # R8a: the two note agents restored from the deleted Qt app - see + # graphlink_note_agent.py's own docstring for why they were dead stubs. + bus.register_intent("scene", "generateKeyTakeaway", generate_key_takeaway) + bus.register_intent("scene", "generateExplainerNote", generate_explainer_note) bus.register_intent("scene", "resizeChart", resize_chart) bus.register_intent("scene", "toggleChartAspectLock", toggle_chart_aspect_lock) bus.register_intent("scene", "moveNode", move_node) diff --git a/backend/tests/test_agent_layer_qt_free.py b/backend/tests/test_agent_layer_qt_free.py index 7edc0a4..a42b298 100644 --- a/backend/tests/test_agent_layer_qt_free.py +++ b/backend/tests/test_agent_layer_qt_free.py @@ -98,6 +98,16 @@ def test_chart_agent_imports_qt_free(): _assert_import_is_qt_free("graphlink_chart_agent") +def test_note_agent_imports_qt_free(): + # R8a: graphlink_note_agent.py restores KeyTakeawayAgent/ExplainerAgent, + # which were lost when R7.6b deleted graphlink_app/ (they lived in + # graphlink_agents_core.py alongside QThread workers). Their menu items + # had been disabled stubs ever since. backend/agents.py imports this + # module at module scope, so it must stay Qt-free or it re-taints the + # entire backend import graph. + _assert_import_is_qt_free("graphlink_note_agent") + + def test_chart_rendering_imports_qt_free(): # R6.2: graphlink_chart_rendering.py ports the legacy ChartItem's # Matplotlib rendering (already Qt-free upstream: matplotlib.use("Agg") diff --git a/backend/tests/test_agents.py b/backend/tests/test_agents.py index 561bdb5..0736c10 100644 --- a/backend/tests/test_agents.py +++ b/backend/tests/test_agents.py @@ -4466,3 +4466,124 @@ async def run(): assert notifications.msg_type == "error" asyncio.run(run()) + + +# -- R8a: note agents (Key Takeaway / Explainer Note) ------------------------- +# +# These two were implemented in the deleted Qt app and never ported, leaving +# their menu items as disabled stubs. Mocking follows the chart/artifact seam +# (patch the agent CLASS's get_response), not the api_provider.chat seam, +# because agents.py constructs a fresh agent instance per call. + + +def _make_note_env(): + bus = SessionBus("agents-note-test") + notifications = NotificationState() + bus.register_topic("notification", notifications.payload) + bus.register_topic("scene", lambda: {}) + dispatcher = AgentDispatcher(_FakeSettingsManager()) + return bus, notifications, dispatcher + + +def test_start_note_generation_takeaway_calls_on_success_then_clears_the_slot(monkeypatch): + monkeypatch.setattr( + agents_module.KeyTakeawayAgent, "get_response", + lambda self, text: f"Key Takeaway\n\nMain Points:\n• from {text}", + ) + + async def run(): + bus, notifications, dispatcher = _make_note_env() + successes, failures = [], [] + await dispatcher.start_note_generation( + bus=bus, notifications_state=notifications, node_id="n1", + note_kind="takeaway", source_text="the source node's text", + on_success=successes.append, on_failure=failures.append, + ) + assert successes == ["Key Takeaway\n\nMain Points:\n• from the source node's text"] + assert failures == [] + assert dispatcher._note_requests == {} + assert notifications.visible is False + + asyncio.run(run()) + + +def test_start_note_generation_explainer_uses_the_explainer_agent(monkeypatch): + # note_kind must actually select the agent - a regression here would + # silently produce takeaways for both menu items. + monkeypatch.setattr(agents_module.KeyTakeawayAgent, "get_response", lambda self, text: "TAKEAWAY") + monkeypatch.setattr(agents_module.ExplainerAgent, "get_response", lambda self, text: "EXPLAINER") + + async def run(): + bus, notifications, dispatcher = _make_note_env() + got = [] + await dispatcher.start_note_generation( + bus=bus, notifications_state=notifications, node_id="n1", + note_kind="explainer", source_text="x", + on_success=got.append, on_failure=lambda m: None, + ) + assert got == ["EXPLAINER"] + + asyncio.run(run()) + + +def test_start_note_generation_rejects_a_second_concurrent_run(monkeypatch): + monkeypatch.setattr(agents_module.KeyTakeawayAgent, "get_response", lambda self, text: "ok") + + async def run(): + bus, notifications, dispatcher = _make_note_env() + dispatcher._note_requests["already-running"] = True + successes = [] + await dispatcher.start_note_generation( + bus=bus, notifications_state=notifications, node_id="n1", + note_kind="takeaway", source_text="x", + on_success=successes.append, on_failure=lambda m: None, + ) + assert successes == [], "the busy guard must not run a second agent" + assert notifications.visible is True + assert notifications.msg_type == "info" + # The pre-existing sentinel must survive - the guard rejects, it + # must never clear someone else's in-flight slot. + assert dispatcher._note_requests == {"already-running": True} + + asyncio.run(run()) + + +def test_start_note_generation_empty_response_fails_instead_of_creating_a_blank_note(monkeypatch): + monkeypatch.setattr(agents_module.KeyTakeawayAgent, "get_response", lambda self, text: " ") + + async def run(): + bus, notifications, dispatcher = _make_note_env() + successes, failures = [], [] + await dispatcher.start_note_generation( + bus=bus, notifications_state=notifications, node_id="n1", + note_kind="takeaway", source_text="x", + on_success=successes.append, on_failure=failures.append, + ) + assert successes == [], "an empty agent response must not become a note" + assert len(failures) == 1 + assert notifications.msg_type == "error" + assert dispatcher._note_requests == {} + + asyncio.run(run()) + + +def test_start_note_generation_agent_exception_surfaces_and_clears_the_slot(monkeypatch): + def _boom(self, text): + raise RuntimeError("model exploded") + + monkeypatch.setattr(agents_module.KeyTakeawayAgent, "get_response", _boom) + + async def run(): + bus, notifications, dispatcher = _make_note_env() + successes, failures = [], [] + await dispatcher.start_note_generation( + bus=bus, notifications_state=notifications, node_id="n1", + note_kind="takeaway", source_text="x", + on_success=successes.append, on_failure=failures.append, + ) + assert successes == [] + assert "model exploded" in failures[0] + assert notifications.msg_type == "error" + assert dispatcher._note_requests == {}, "the slot must not leak after a failure" + + asyncio.run(run()) diff --git a/backend/tests/test_canvas.py b/backend/tests/test_canvas.py index b448c91..bcf9e6a 100644 --- a/backend/tests/test_canvas.py +++ b/backend/tests/test_canvas.py @@ -18,6 +18,9 @@ from backend.canvas import ( DRAG_FACTOR_MAX, DRAG_FACTOR_MIN, + NOTE_AGENT_BODY_COLOR, + NOTE_AGENT_HEADER_COLOR, + NOTE_AGENT_X_OFFSET, SceneDocument, SceneEmptyPromptError, SceneError, @@ -6038,3 +6041,126 @@ async def run(): assert recorder.topics_seen().count("scene") == 1 asyncio.run(run()) + + +# -- R8a: generateKeyTakeaway / generateExplainerNote round trips -------------- +# +# Both agents were lost with the R7.6b Qt cutover and their menu items sat +# disabled ever since. These cover the intent layer: validation, the note that +# gets created, and where it lands. + + +def test_generate_key_takeaway_creates_a_tinted_note_beside_the_source_node(monkeypatch): + monkeypatch.setattr( + agents_module.KeyTakeawayAgent, "get_response", + lambda self, text: "Key Takeaway\n\nMain Points:\n• it works", + ) + + async def run(): + bus, document, recorder, _ = make_bus_with_dispatcher() + chat = document.add_chat_node(100, 200, "a long assistant answer", False) + scene_publishes_before = recorder.topics_seen().count("scene") + + note_id = await bus.dispatch_intent("scene", "generateKeyTakeaway", [chat.id]) + + assert note_id is not None + note = document.nodes[note_id] + assert note.kind == "note" + assert note.content == "Key Takeaway\n\nMain Points:\n• it works" + # Offset to the RIGHT of the source, clearing the chat node's width. + assert (note.x, note.y) == (100 + NOTE_AGENT_X_OFFSET, 200) + assert note.color == NOTE_AGENT_BODY_COLOR + assert note.header_color == NOTE_AGENT_HEADER_COLOR + assert recorder.topics_seen().count("scene") > scene_publishes_before + + asyncio.run(run()) + + +def test_generate_explainer_note_staggers_below_a_takeaway_from_the_same_node(monkeypatch): + # Both generated from ONE node must not land on top of each other. + monkeypatch.setattr(agents_module.KeyTakeawayAgent, "get_response", lambda self, text: "TAKEAWAY") + monkeypatch.setattr(agents_module.ExplainerAgent, "get_response", lambda self, text: "EXPLAINER") + + async def run(): + bus, document, _, _ = make_bus_with_dispatcher() + chat = document.add_chat_node(0, 0, "source text", False) + + takeaway_id = await bus.dispatch_intent("scene", "generateKeyTakeaway", [chat.id]) + explainer_id = await bus.dispatch_intent("scene", "generateExplainerNote", [chat.id]) + + takeaway, explainer = document.nodes[takeaway_id], document.nodes[explainer_id] + assert takeaway.content == "TAKEAWAY" + assert explainer.content == "EXPLAINER" + assert takeaway.x == explainer.x + assert explainer.y > takeaway.y, "the two must not overlap" + + asyncio.run(run()) + + +def test_generate_key_takeaway_rejects_an_empty_node_without_calling_the_agent(monkeypatch): + calls = [] + monkeypatch.setattr( + agents_module.KeyTakeawayAgent, "get_response", + lambda self, text: calls.append(text) or "should not happen", + ) + + async def run(): + bus, document, _, _ = make_bus_with_dispatcher() + blank = document.add_chat_node(0, 0, " ", True) + node_count_before = len(document.nodes) + + result = await bus.dispatch_intent("scene", "generateKeyTakeaway", [blank.id]) + + assert result is None + assert calls == [], "an empty node must never reach the model" + assert len(document.nodes) == node_count_before, "no note should be created" + + asyncio.run(run()) + + +def test_generate_key_takeaway_rejects_a_non_chat_node(monkeypatch): + calls = [] + monkeypatch.setattr( + agents_module.KeyTakeawayAgent, "get_response", + lambda self, text: calls.append(text) or "x", + ) + + async def run(): + bus, document, _, _ = make_bus_with_dispatcher() + note = document.add_note(0, 0) + result = await bus.dispatch_intent("scene", "generateKeyTakeaway", [note.id]) + assert result is None + assert calls == [] + + asyncio.run(run()) + + +def test_generate_key_takeaway_rejects_an_unknown_node_id(): + async def run(): + bus, document, _, _ = make_bus_with_dispatcher() + result = await bus.dispatch_intent("scene", "generateKeyTakeaway", ["does-not-exist"]) + assert result is None + + asyncio.run(run()) + + +def test_generate_key_takeaway_sends_the_nodes_own_text_not_the_branch_history(monkeypatch): + # Legacy summarised ONE node. Widening this to the branch history (as + # generateChart does) would change what the feature actually summarises. + seen = [] + monkeypatch.setattr( + agents_module.KeyTakeawayAgent, "get_response", + lambda self, text: seen.append(text) or "Key Takeaway", + ) + + async def run(): + bus, document, _, _ = make_bus_with_dispatcher() + parent = document.add_chat_node(0, 0, "the parent question", True) + child = document.add_chat_node(0, 160, "the child answer", False, parent_id=parent.id) + + await bus.dispatch_intent("scene", "generateKeyTakeaway", [child.id]) + + assert seen == ["the child answer"] + assert "the parent question" not in seen[0] + + asyncio.run(run()) diff --git a/graphlink_note_agent.py b/graphlink_note_agent.py new file mode 100644 index 0000000..541565f --- /dev/null +++ b/graphlink_note_agent.py @@ -0,0 +1,194 @@ +"""Qt-free note-agent core: Key Takeaway and Explainer Note (R8a). + +Both agents were implemented in the Qt app's graphlink_agents_core.py and +were lost to the R7.6b cutover, leaving their two chat-node menu items +rendered as disabled stubs whose tooltip blamed a missing agent layer. That +blocker had in fact been gone since R4 - the same dispatcher that already +drives Regenerate Response, Generate Image and Generate Chart. Nothing was +blocked; the two agents had simply never been ported. This module restores +them. + +The prompts, the `clean_agent_markdown_response` post-processor and the +"one chat call, no tools, no history" shape are carried over verbatim from +the deleted implementation, so output matches what the Qt app produced. + +Two deliberate divergences from that original, both because the machinery +it depended on no longer exists: + + - No text bounding. Legacy passed source text through + `render_context(source_snapshot(...))`, which died with the Qt app and + has no successor. Every surviving agent path (chart, image, chat) feeds + unbounded text today, so this matches them rather than reintroducing a + single bounded path. If context limits ever need enforcing, that is a + systemic concern for all agents, not one this feature should solve + alone. + - No QThread workers. The dispatcher owns concurrency now + (backend/agents.py's start_note_generation), so the legacy + `KeyTakeawayWorkerThread`/`ExplainerWorkerThread` classes have no + counterpart and are not ported. + +This file must stay Qt-free forever - it exists to be importable from +backend/, which test_no_qt_anywhere.py holds to zero tolerance. +""" + +import api_provider + +# graphlink_task_config (NOT graphlink_config) - the R4.1 Qt-free split of +# task/provider/model config. graphlink_config chains to PySide6.QtGui, so +# importing it here would silently re-taint this module with Qt. +import graphlink_task_config as config + + +def clean_agent_markdown_response( + text, + required_title, + section_markers, + reset_bullet_state_on_section_header=False, +): + """Strip common markdown noise and normalize bullets/section spacing for a + structured agent response. + + Ported verbatim from the deleted graphlink_agents_core.py, including the + `reset_bullet_state_on_section_header` flag: the legacy GroupSummaryAgent + set it and the two agents here did not, and that difference is preserved + rather than normalized away so their output is unchanged. (Group Summary + itself is not ported - its menu entry was conditional on a multi-select + model the new stack does not have.) + + Args: + text (str): The raw text from the AI model. + required_title (str): Header line to prepend if the first cleaned line + doesn't already contain it. + section_markers (list[str]): Line substrings (e.g. "Key Parts:") that + get an extra blank line before them. + reset_bullet_state_on_section_header (bool): Whether a section-marker + line resets bullet-run tracking. + + Returns: + str: The cleaned and formatted text. + """ + replacements = [ + ('```', ''), + ('`', ''), + ('**', ''), + ('__', ''), + ('*', ''), + ('_', ''), + ('•', '•'), + ('→', '->'), + ('\n\n\n', '\n\n'), + ] + + cleaned = str(text or "") + for old, new in replacements: + cleaned = cleaned.replace(old, new) + + cleaned_lines = [] + for line in cleaned.split('\n'): + line = line.strip() + if line: + if line.lstrip().startswith('-'): + line = '• ' + line.lstrip('- ') + cleaned_lines.append(line) + + formatted = '' + in_bullet_list = False + + for i, line in enumerate(cleaned_lines): + if i == 0 and required_title not in line: + formatted += f"{required_title}\n" + + if line.startswith('•'): + if not in_bullet_list: + formatted += '\n' if formatted else '' + in_bullet_list = True + formatted += line + '\n' + elif any(marker in line for marker in section_markers): + formatted += '\n' + line + '\n' + if reset_bullet_state_on_section_header: + in_bullet_list = False + else: + in_bullet_list = False + formatted += line + '\n' + + return formatted.strip() + + +class KeyTakeawayAgent: + """Extracts key takeaways from a block of text.""" + + def __init__(self): + self.system_prompt = """You are a key takeaway generator. Format your response exactly like this: + +Key Takeaway +[1-2 sentence overview] + +Main Points: +• [First key point] +• [Second key point] +• [Third key point if needed] + +Keep total output under 150 words. Be direct and focused on practical value. +No markdown formatting, no special characters.""" + + def clean_text(self, text): + return clean_agent_markdown_response( + text, + required_title="Key Takeaway", + section_markers=['Main Points:'], + ) + + def get_response(self, text): + messages = [ + {'role': 'system', 'content': self.system_prompt}, + {'role': 'user', 'content': f"Generate key takeaways from this text: {text}"}, + ] + response = api_provider.chat(task=config.TASK_CHAT, messages=messages) + return self.clean_text(response['message']['content']) + + +class ExplainerAgent: + """Simplifies complex topics into plain language.""" + + def __init__(self): + self.system_prompt = """You are an expert at explaining complex topics in simple terms. Follow these principles in order: + +1. Simplification: Break down complex ideas into their most basic form +2. Clarification: Remove any technical jargon or complex terminology +3. Distillation: Extract only the most important concepts +4. Breakdown: Present information in small, digestible chunks +5. Simple Language: Use everyday words and short sentences + +Always use: +- Analogies: Connect ideas to everyday experiences +- Metaphors: Compare complex concepts to simple, familiar things + +Format your response exactly like this: + +Simple Explanation +[2-3 sentence overview using everyday language] + +Think of it Like This: +[Add one clear analogy or metaphor that a child would understand] + +Key Parts: +• [First simple point] +• [Second simple point] +• [Third point if needed] + +Remember: Write as if explaining to a curious 5-year-old. No technical terms, no complex words.""" + + def clean_text(self, text): + return clean_agent_markdown_response( + text, + required_title="Simple Explanation", + section_markers=['Think of it Like This:', 'Key Parts:'], + ) + + def get_response(self, text): + messages = [ + {'role': 'system', 'content': self.system_prompt}, + {'role': 'user', 'content': f"Explain this in simple terms: {text}"}, + ] + response = api_provider.chat(task=config.TASK_CHAT, messages=messages) + return self.clean_text(response['message']['content']) diff --git a/web_ui/src/app/canvas/ChatNodeView.test.tsx b/web_ui/src/app/canvas/ChatNodeView.test.tsx index a6d6a27..aa5b7ce 100644 --- a/web_ui/src/app/canvas/ChatNodeView.test.tsx +++ b/web_ui/src/app/canvas/ChatNodeView.test.tsx @@ -31,6 +31,8 @@ function renderChatNode(overrides: Partial = {}) { const onRegenerate = vi.fn(); const onGenerateImage = vi.fn(); const onGenerateChart = vi.fn(); + const onGenerateKeyTakeaway = vi.fn(); + const onGenerateExplainerNote = vi.fn(); const onScrollChange = vi.fn(); const props = { id: "n0", @@ -47,6 +49,8 @@ function renderChatNode(overrides: Partial = {}) { onRegenerate, onGenerateImage, onGenerateChart, + onGenerateKeyTakeaway, + onGenerateExplainerNote, onScrollChange, ...overrides, }, @@ -57,7 +61,11 @@ function renderChatNode(overrides: Partial = {}) { , ); - return { onToggleCollapse, onDelete, onUndockChild, onRegenerate, onGenerateImage, onGenerateChart, onScrollChange, container }; + return { + onToggleCollapse, onDelete, onUndockChild, onRegenerate, onGenerateImage, + onGenerateChart, onGenerateKeyTakeaway, onGenerateExplainerNote, + onScrollChange, container, + }; } describe("ChatNodeView", () => { @@ -97,10 +105,14 @@ describe("ChatNodeView", () => { expect(hideBranches).toHaveAttribute("title", "Branch visibility isn't built yet"); const docView = screen.getByRole("menuitem", { name: "Open Document View" }); expect(docView).toBeDisabled(); + // R8a: these two were disabled stubs until their agents were ported + // back from the deleted Qt app. This assertion is deliberately inverted + // rather than removed - it was the guard that encoded the stub as + // correct, so it has to now encode the opposite. for (const name of ["Generate Key Takeaway", "Generate Explainer Note"]) { const item = screen.getByRole("menuitem", { name }); - expect(item).toBeDisabled(); - expect(item).toHaveAttribute("title", "AI note generation isn't available yet"); + expect(item).not.toBeDisabled(); + expect(item).not.toHaveAttribute("title"); } expect(screen.getByRole("menuitem", { name: "Generate Image" })).not.toBeDisabled(); expect(screen.getByRole("menuitem", { name: "Generate Chart" })).not.toBeDisabled(); @@ -169,6 +181,39 @@ describe("ChatNodeView", () => { expect(screen.queryByRole("menu")).toBeNull(); // onClose fires after onGenerateImage }); + it("Generate Key Takeaway calls onGenerateKeyTakeaway then closes the menu", async () => { + const user = userEvent.setup(); + const { onGenerateKeyTakeaway, onGenerateExplainerNote } = renderChatNode({ isUser: false }); + + fireEvent.contextMenu(screen.getByText("Assistant")); + await user.click(screen.getByRole("menuitem", { name: "Generate Key Takeaway" })); + + expect(onGenerateKeyTakeaway).toHaveBeenCalledOnce(); + // The two sit adjacent in the menu - assert the other one did NOT fire, + // so a future edit can't silently wire both buttons to one handler. + expect(onGenerateExplainerNote).not.toHaveBeenCalled(); + expect(screen.queryByRole("menu")).toBeNull(); + }); + + it("Generate Explainer Note calls onGenerateExplainerNote then closes the menu", async () => { + const user = userEvent.setup(); + const { onGenerateExplainerNote, onGenerateKeyTakeaway } = renderChatNode({ isUser: false }); + + fireEvent.contextMenu(screen.getByText("Assistant")); + await user.click(screen.getByRole("menuitem", { name: "Generate Explainer Note" })); + + expect(onGenerateExplainerNote).toHaveBeenCalledOnce(); + expect(onGenerateKeyTakeaway).not.toHaveBeenCalled(); + expect(screen.queryByRole("menu")).toBeNull(); + }); + + it("both note agents are offered on a USER node too, matching legacy's unconditional enablement", () => { + renderChatNode({ isUser: true }); + fireEvent.contextMenu(screen.getByText("You")); + expect(screen.getByRole("menuitem", { name: "Generate Key Takeaway" })).not.toBeDisabled(); + expect(screen.getByRole("menuitem", { name: "Generate Explainer Note" })).not.toBeDisabled(); + }); + it("Generate Chart is a real, enabled item with aria-haspopup that starts collapsed (no submenu items in the DOM yet)", () => { renderChatNode({ isUser: true }); fireEvent.contextMenu(screen.getByText("You")); diff --git a/web_ui/src/app/canvas/ChatNodeView.tsx b/web_ui/src/app/canvas/ChatNodeView.tsx index f39cc94..e7c22c3 100644 --- a/web_ui/src/app/canvas/ChatNodeView.tsx +++ b/web_ui/src/app/canvas/ChatNodeView.tsx @@ -15,8 +15,7 @@ import { NodeMenu } from "./NodeMenu"; * honest disabled+title label rather than a fake action or a silent drop * (an R3.4 live-drive audit found several legacy ChatNode menu items had * been dropped with zero acknowledgment - fixed here): Regenerate (assistant - * nodes only, needs the R4 agent layer), Key Takeaway/Explainer Note - * generation (R4, same agent-layer blocker), Open Document View (the + * nodes only, needs the R4 agent layer), Open Document View (the * document-viewer island isn't wired into the SPA overlay system yet), and * Hide Other Branches (the legacy scene's * branch-visibility toggle has no backend/frontend equivalent at all yet - @@ -41,9 +40,13 @@ import { NodeMenu } from "./NodeMenu"; * real click-to-expand submenu (CHART_TYPE_OPTIONS below) offering the 5 * chart types in legacy's own menu order (Bar/Line/Histogram/Pie/Sankey), * each dispatching the real generateChart intent with this node as the - * parent. Key Takeaway/Explainer Note remain honestly deferred (still no - * agent-layer support of their own) - the stale "Chart" mention in their old - * shared R4-blocker note above has been removed accordingly. "Export" is + * parent. "Generate Key Takeaway" and "Generate Explainer Note" are + * likewise no longer deferred as of R8a: their agents were lost with the + * R7.6b Qt cutover and never ported, and the tooltip blaming a missing + * agent layer had been stale since R4 - the very layer Regenerate/Image/ + * Chart already use. Both now dispatch real intents that drop the agent's + * output into a new note beside this node (graphlink_note_agent.py). + * "Export" is * likewise no longer deferred as of R7.5a: it downloads the node's raw * content (not the rendered markdown) as a .md file via downloadTextFile - * frontend-only, no backend involved, since the content is already in @@ -71,6 +74,8 @@ export interface ChatNodeData extends Record { onRegenerate: () => void; onGenerateImage: () => void; onGenerateChart: (chartType: string) => void; + onGenerateKeyTakeaway: () => void; + onGenerateExplainerNote: () => void; onScrollChange: (value: number) => void; } @@ -107,6 +112,8 @@ function ChatNodeMenu({ onRegenerate, onGenerateImage, onGenerateChart, + onGenerateKeyTakeaway, + onGenerateExplainerNote, onClose, }: { position: MenuPosition; @@ -121,6 +128,8 @@ function ChatNodeMenu({ onRegenerate: () => void; onGenerateImage: () => void; onGenerateChart: (chartType: string) => void; + onGenerateKeyTakeaway: () => void; + onGenerateExplainerNote: () => void; onClose: () => void; }) { const [chartMenuOpen, setChartMenuOpen] = useState(false); @@ -186,10 +195,28 @@ function ChatNodeMenu({ - - {/* R6.2: a real click-to-expand submenu (not disabled) - same @@ -359,6 +386,8 @@ export function ChatNodeView({ id, data, selected }: NodeProps) { onRegenerate={data.onRegenerate} onGenerateImage={data.onGenerateImage} onGenerateChart={data.onGenerateChart} + onGenerateKeyTakeaway={data.onGenerateKeyTakeaway} + onGenerateExplainerNote={data.onGenerateExplainerNote} onClose={() => setMenuPosition(null)} /> )} diff --git a/web_ui/src/app/canvas/SceneCanvas.tsx b/web_ui/src/app/canvas/SceneCanvas.tsx index 9d1bba3..394c0c3 100644 --- a/web_ui/src/app/canvas/SceneCanvas.tsx +++ b/web_ui/src/app/canvas/SceneCanvas.tsx @@ -239,6 +239,11 @@ export function toFlowNodes(scene: SceneState, store: SceneStore): SceneFlowNode // onGenerateImage above - the new chart node arrives through the // next scene snapshot. onGenerateChart: (chartType: string) => store.generateChart(n.id, chartType), + // R8a: the two note agents, restored from the deleted Qt app. Same + // fire-and-forget posture as onGenerateImage above - the new note + // arrives through the next scene snapshot. + onGenerateKeyTakeaway: () => store.generateKeyTakeaway(n.id), + onGenerateExplainerNote: () => store.generateExplainerNote(n.id), // R6.3: the node's own scroll position within its content area - // read on mount by ChatNodeView (restore) and reported (debounced) // via the new setChatScrollValue intent on every scroll. Defaults diff --git a/web_ui/src/app/canvas/sceneStore.ts b/web_ui/src/app/canvas/sceneStore.ts index ecaf89c..be73624 100644 --- a/web_ui/src/app/canvas/sceneStore.ts +++ b/web_ui/src/app/canvas/sceneStore.ts @@ -671,6 +671,20 @@ export class SceneStore { this.transport.intent("scene", "generateChart", [parentNodeId, chartType]); } + // R8a: the Key Takeaway / Explainer Note agents, restored from the deleted + // Qt app (their menu items had been disabled stubs blaming a missing agent + // layer that had in fact shipped in R4). Both take only the source chat + // node's id - the backend reads its own .content, exactly like + // generateImage - and the resulting note arrives on the next scene + // snapshot, so neither needs the new node id back here. + generateKeyTakeaway(sourceNodeId: string): void { + this.transport.intent("scene", "generateKeyTakeaway", [sourceNodeId]); + } + + generateExplainerNote(sourceNodeId: string): void { + this.transport.intent("scene", "generateExplainerNote", [sourceNodeId]); + } + resizeChart(nodeId: string, width: number, height: number): void { this.transport.intent("scene", "resizeChart", [nodeId, width, height]); }