From a4416952b2b2bd872d4f16d9eb4799cc0b6f3fbc Mon Sep 17 00:00:00 2001 From: Matthew Wesney Date: Wed, 15 Jul 2026 17:54:55 -0400 Subject: [PATCH] Refactor canvas utility tools for production reliability --- .../graphlink_canvas_frame.py | 33 ++- .../graphlink_canvas/graphlink_canvas_note.py | 63 +++++- graphlink_app/graphlink_scene.py | 114 +++++++--- .../graphlink_session/deserializers.py | 41 +++- .../graphlink_session/serializers.py | 13 ++ graphlink_app/graphlink_utility.py | 214 ++++++++++++++++++ graphlink_app/graphlink_window.py | 10 +- graphlink_app/graphlink_window_actions.py | 158 ++++++++++--- graphlink_app/tests/test_utility_tools.py | 152 +++++++++++++ pyproject.toml | 1 + 10 files changed, 719 insertions(+), 80 deletions(-) create mode 100644 graphlink_app/graphlink_utility.py create mode 100644 graphlink_app/tests/test_utility_tools.py diff --git a/graphlink_app/graphlink_canvas/graphlink_canvas_frame.py b/graphlink_app/graphlink_canvas/graphlink_canvas_frame.py index e63f95b..926efa3 100644 --- a/graphlink_app/graphlink_canvas/graphlink_canvas_frame.py +++ b/graphlink_app/graphlink_canvas/graphlink_canvas_frame.py @@ -45,10 +45,12 @@ def __init__(self, nodes, parent=None): self.setAcceptHoverEvents(True) # Load icons for the lock/unlock button. - self.lock_icon = qta.icon('fa.lock', color='#ffffff') - self.unlock_icon = qta.icon('fa.unlock-alt', color='#ffffff') - self.lock_icon_hover = qta.icon('fa.lock', color=get_semantic_color("status_info").name()) - self.unlock_icon_hover = qta.icon('fa.unlock-alt', color=get_semantic_color("status_success").name()) + # QtAwesome's legacy ``fa`` prefix is not installed in current releases; + # use the Font Awesome 5 solid set used by the rest of the canvas. + self.lock_icon = qta.icon('fa5s.lock', color='#ffffff') + self.unlock_icon = qta.icon('fa5s.unlock-alt', color='#ffffff') + self.lock_icon_hover = qta.icon('fa5s.lock', color=get_semantic_color("status_info").name()) + self.unlock_icon_hover = qta.icon('fa5s.unlock-alt', color=get_semantic_color("status_success").name()) # State attributes self.is_locked = True @@ -82,6 +84,7 @@ def __init__(self, nodes, parent=None): self.resizing = False self.resize_start_rect = None self.resize_start_pos = None + self._user_resized = False # Animation for the "unlocked" state outline. self.outline_animation = QVariantAnimation() @@ -284,9 +287,10 @@ def updateGeometry(self): self.rect = QRectF(0, 0, 320, 180) self._update_title_editor_geometry() return - old_rect = QRectF(self.rect) new_rect = self.calculate_minimum_size() - final_rect = old_rect.united(new_rect) if old_rect.isValid() else new_rect + final_rect = new_rect + if self._user_resized and self.rect.isValid(): + final_rect = self.rect.united(new_rect) if final_rect != self.rect: self.prepareGeometryChange() @@ -299,6 +303,13 @@ def updateGeometry(self): if parent and isinstance(parent, Container): parent.updateGeometry() + def fit_to_content(self): + """Reset manual bounds to the current membership bounds.""" + self._user_resized = False + self.updateGeometry() + self._update_child_connections() + self.update() + def boundingRect(self): """Returns the bounding rectangle of the item.""" return self.rect @@ -456,6 +467,7 @@ def mouseMoveEvent(self, event): if new_rect != self.rect: self.prepareGeometryChange() self.rect = new_rect + self._user_resized = True self._update_title_editor_geometry() self._update_child_connections() @@ -585,6 +597,15 @@ def keyPressEvent(self, event): event.accept() return + if ( + event.key() == Qt.Key.Key_F + and event.modifiers() & Qt.KeyboardModifier.ControlModifier + and event.modifiers() & Qt.KeyboardModifier.ShiftModifier + ): + self.fit_to_content() + event.accept() + return + return super().keyPressEvent(event) def paint(self, painter, option, widget=None): diff --git a/graphlink_app/graphlink_canvas/graphlink_canvas_note.py b/graphlink_app/graphlink_canvas/graphlink_canvas_note.py index 6095e91..aeb6c6a 100644 --- a/graphlink_app/graphlink_canvas/graphlink_canvas_note.py +++ b/graphlink_app/graphlink_canvas/graphlink_canvas_note.py @@ -2,6 +2,7 @@ import markdown import qtawesome as qta +from uuid import uuid4 from PySide6.QtWidgets import QDialog, QGraphicsItem, QApplication, QMessageBox from PySide6.QtCore import Qt, QRectF, QPointF, QTimer @@ -26,6 +27,7 @@ class Note(QGraphicsItem): DEFAULT_WIDTH = 200 DEFAULT_HEIGHT = 150 MAX_HEIGHT = 500 + MAX_CONTENT_LENGTH = 100_000 CONTROL_GUTTER = 25 SCROLLBAR_PADDING = 5 @@ -46,6 +48,12 @@ def __init__(self, pos, parent=None): self.setAcceptHoverEvents(True) self.is_system_prompt = False self.is_summary_note = False + self.persistent_id = uuid4().hex + self.note_role = "manual" + self.source_ids = [] + self.operation_id = "" + self.source_revisions = {} + self.provider_snapshot = {} # Geometry and appearance self.width = self.DEFAULT_WIDTH @@ -58,6 +66,8 @@ def __init__(self, pos, parent=None): self.edit_text = "" self.cursor_pos = 0 self.cursor_visible = True + self._edit_history = [] + self._edit_redo = [] # State for text selection self.selection_start = 0 @@ -94,11 +104,14 @@ def content(self, new_content): Sets the note's content. If not in editing mode, it immediately updates the QTextDocument for rendering. """ + new_content = str(new_content or "")[:self.MAX_CONTENT_LENGTH] if self._content != new_content: self._content = new_content if not self.editing: self._setup_document() self.update() + if self.scene() and hasattr(self.scene(), "_schedule_scene_changed"): + self.scene()._schedule_scene_changed() def _setup_document(self): """ @@ -387,6 +400,7 @@ def mousePressEvent(self, event): elif self.color_button_rect.contains(event.pos()): self.show_color_picker() event.accept() + return if event.button() == Qt.MouseButton.LeftButton and self.scene(): self.scene().is_dragging_item = True @@ -420,6 +434,8 @@ def mouseDoubleClickEvent(self, event): if not self.editing: self.editing = True self.edit_text = self.content + self._edit_history = [] + self._edit_redo = [] char_pos = self.get_char_pos_at_x(event.pos().x(), event.pos().y()) text = self.edit_text @@ -448,12 +464,17 @@ def keyPressEvent(self, event): elif event.key() == Qt.Key.Key_V: self.paste_text(); return elif event.key() == Qt.Key.Key_X: self.cut_selection(); return elif event.key() == Qt.Key.Key_A: self.select_all(); return + elif event.key() == Qt.Key.Key_Z: + self.undo_edit(); return + elif event.key() == Qt.Key.Key_Y: + self.redo_edit(); return if event.key() == Qt.Key.Key_Return and event.modifiers() & Qt.KeyboardModifier.ControlModifier: self.finishEditing() elif event.key() == Qt.Key.Key_Escape: self.editing = False; self.cursor_timer.stop(); self.update() elif event.key() in (Qt.Key.Key_Backspace, Qt.Key.Key_Delete): + self._push_edit_snapshot() if self.selection_start != self.selection_end: self.delete_selection() elif event.key() == Qt.Key.Key_Backspace and self.cursor_pos > 0: self.edit_text = self.edit_text[:self.cursor_pos-1] + self.edit_text[self.cursor_pos:] @@ -492,13 +513,15 @@ def keyPressEvent(self, event): self.update() elif event.key() == Qt.Key.Key_Return: # Insert newline. + self._push_edit_snapshot() if self.selection_start != self.selection_end: self.delete_selection() self.edit_text = self.edit_text[:self.cursor_pos] + '\n' + self.edit_text[self.cursor_pos:] self.cursor_pos += 1; self.selection_start = self.selection_end = self.cursor_pos; self.update() elif len(event.text()) and event.text().isprintable(): # Insert typed character. + self._push_edit_snapshot() if self.selection_start != self.selection_end: self.delete_selection() - self.edit_text = self.edit_text[:self.cursor_pos] + event.text() + self.edit_text[self.cursor_pos:] + self.edit_text = (self.edit_text[:self.cursor_pos] + event.text() + self.edit_text[self.cursor_pos:])[:self.MAX_CONTENT_LENGTH] self.cursor_pos += 1; self.selection_start = self.selection_end = self.cursor_pos; self.update() def wheelEvent(self, event): @@ -525,6 +548,29 @@ def update_scroll_position(self, value): self.scroll_value = value; self.update() # --- Text manipulation methods --- + def _push_edit_snapshot(self): + if not self._edit_history or self._edit_history[-1] != self.edit_text: + self._edit_history.append(self.edit_text) + self._edit_redo.clear() + + def _restore_edit_text(self, text): + self.edit_text = str(text or "")[:self.MAX_CONTENT_LENGTH] + self.cursor_pos = min(self.cursor_pos, len(self.edit_text)) + self.selection_start = self.selection_end = self.cursor_pos + self.update() + + def undo_edit(self): + if not self._edit_history: + return + self._edit_redo.append(self.edit_text) + self._restore_edit_text(self._edit_history.pop()) + + def redo_edit(self): + if not self._edit_redo: + return + self._edit_history.append(self.edit_text) + self._restore_edit_text(self._edit_redo.pop()) + def copy_selection(self): if self.selection_start != self.selection_end: start, end = min(self.selection_start, self.selection_end), max(self.selection_start, self.selection_end) @@ -532,17 +578,20 @@ def copy_selection(self): def cut_selection(self): if self.selection_start != self.selection_end: + self._push_edit_snapshot() self.copy_selection(); self.delete_selection() def paste_text(self): text = QApplication.clipboard().text() if text: + self._push_edit_snapshot() if self.selection_start != self.selection_end: self.delete_selection() - self.edit_text = self.edit_text[:self.cursor_pos] + text + self.edit_text[self.cursor_pos:] + self.edit_text = (self.edit_text[:self.cursor_pos] + text + self.edit_text[self.cursor_pos:])[:self.MAX_CONTENT_LENGTH] self.cursor_pos += len(text); self.selection_start = self.selection_end = self.cursor_pos; self.update() def delete_selection(self): if self.selection_start != self.selection_end: + self._push_edit_snapshot() start, end = min(self.selection_start, self.selection_end), max(self.selection_start, self.selection_end) self.edit_text = self.edit_text[:start] + self.edit_text[end:] self.cursor_pos = self.selection_start = self.selection_end = start @@ -568,10 +617,14 @@ def hoverLeaveEvent(self, event): def show_color_picker(self): """Opens the color picker dialog.""" - dialog = ColorPickerDialog(self.scene().views()[0]) + scene = self.scene() + if scene is None or not scene.views(): + return + view = scene.views()[0] + dialog = ColorPickerDialog(view) note_pos = self.mapToScene(self.color_button_rect.topRight()) - view_pos = self.scene().views()[0].mapFromScene(note_pos) - global_pos = self.scene().views()[0].mapToGlobal(view_pos) + view_pos = view.mapFromScene(note_pos) + global_pos = view.mapToGlobal(view_pos) dialog.move(global_pos.x() + 10, global_pos.y()) if dialog.exec() == QDialog.DialogCode.Accepted: color, color_type = dialog.get_selected_color() diff --git a/graphlink_app/graphlink_scene.py b/graphlink_app/graphlink_scene.py index c141886..1dee9f5 100644 --- a/graphlink_app/graphlink_scene.py +++ b/graphlink_app/graphlink_scene.py @@ -279,7 +279,8 @@ def find_items(self, text): matches = [] all_nodes = self._all_layout_nodes() - for node in all_nodes: + utility_items = self.notes + self.frames + self.containers + for node in all_nodes + utility_items: content = "" if isinstance(node, ChatNode): content = node.text @@ -303,6 +304,12 @@ def find_items(self, text): content = node.get_prompt() + "\n" + node.get_requirements() + "\n" + node.get_code() + "\n" + node.output_display.toPlainText() elif isinstance(node, ChartItem): content = node.to_context_text() if hasattr(node, "to_context_text") else str(node.data) + elif isinstance(node, Note): + content = getattr(node, "content", "") + elif isinstance(node, Frame): + content = getattr(node, "note", "") + elif isinstance(node, Container): + content = getattr(node, "title", "") if text in content.lower(): matches.append(node) @@ -318,7 +325,7 @@ def update_search_highlight(self, matched_nodes): Args: matched_nodes (list): A list of nodes that should be highlighted. """ - all_nodes = self._all_layout_nodes() + all_nodes = self._all_layout_nodes() + self.notes + self.frames + self.containers for node in all_nodes: is_match = node in matched_nodes if getattr(node, 'is_search_match', False) != is_match: @@ -576,6 +583,54 @@ def add_chart(self, data, pos, parent_content_node=None, source_node=None): self.scene_changed.emit() return chart + def _detach_item_from_groups(self, item, remove_empty=True): + """Detach an item through one ownership path and repair stale indexes.""" + parent = item.parentItem() + known_parents = [] + if isinstance(parent, (Frame, Container)): + known_parents.append(parent) + for group in list(self.frames) + list(self.containers): + members = group.nodes if isinstance(group, Frame) else group.contained_items + if item in members and group not in known_parents: + known_parents.append(group) + + scene_pos = item.scenePos() + if isinstance(parent, (Frame, Container)): + item.setParentItem(None) + item.setPos(scene_pos) + + for group in known_parents: + members = group.nodes if isinstance(group, Frame) else group.contained_items + while item in members: + members.remove(item) + if group.scene() == self and members: + group.updateGeometry() + elif remove_empty and group.scene() == self and not members: + if isinstance(group, Frame): + self.deleteFrame(group) + else: + self.deleteContainer(group) + return known_parents + + def validate_group_invariants(self): + """Return human-readable group/index violations for tests and diagnostics.""" + violations = [] + memberships = {} + for group in self.frames: + for item in group.nodes: + memberships.setdefault(item, []).append(group) + if item.parentItem() is not group: + violations.append(f"{type(group).__name__} member has wrong parent") + for group in self.containers: + for item in group.contained_items: + memberships.setdefault(item, []).append(group) + if item.parentItem() is not group: + violations.append(f"{type(group).__name__} member has wrong parent") + for groups in memberships.values(): + if len(groups) > 1: + violations.append("item appears in multiple groups") + return violations + def createFrame(self): """Creates a Frame around the currently selected nodes.""" selected_nodes = [item for item in self.selectedItems() @@ -584,21 +639,8 @@ def createFrame(self): if not selected_nodes: return - # If a selected node is already in a frame, un-parent it first. for node in selected_nodes: - if node.parentItem() and isinstance(node.parentItem(), Frame): - old_frame = node.parentItem() - scene_pos = node.scenePos() - node.setParentItem(None) - node.setPos(scene_pos) - old_frame.nodes.remove(node) - # If the old frame is now empty, remove it. - if not old_frame.nodes: - self.removeItem(old_frame) - if old_frame in self.frames: - self.frames.remove(old_frame) - else: - old_frame.updateGeometry() + self._detach_item_from_groups(node) frame = Frame(selected_nodes) self.addItem(frame) @@ -619,21 +661,8 @@ def createContainer(self): if not selected_items: return - # Un-parent selected items from any existing containers or frames. for item in selected_items: - if item.parentItem() and isinstance(item.parentItem(), (Frame, Container)): - old_parent = item.parentItem() - scene_pos = item.scenePos() - item.setParentItem(None) - item.setPos(scene_pos) - - # Clean up the old parent if it becomes empty. - if isinstance(old_parent, Frame): - old_parent.nodes.remove(item) - if not old_parent.nodes: self.deleteFrame(old_parent) - elif isinstance(old_parent, Container): - old_parent.contained_items.remove(item) - if not old_parent.contained_items: self.deleteContainer(old_parent) + self._detach_item_from_groups(item) container = Container(selected_items) self.addItem(container) @@ -668,7 +697,9 @@ def deleteFrame(self, frame): """ if hasattr(frame, 'dispose'): frame.dispose() - release_parent = frame.parentItem() + release_parent = frame.parentItem() if isinstance(frame.parentItem(), Container) else None + if isinstance(frame.parentItem(), Container) and frame in frame.parentItem().contained_items: + frame.parentItem().contained_items.remove(frame) # Un-parent all nodes from the frame, restoring their scene positions. for node in frame.nodes: @@ -677,6 +708,8 @@ def deleteFrame(self, frame): node.setParentItem(release_parent) if release_parent: node.setPos(release_parent.mapFromScene(scene_pos)) + if node not in release_parent.contained_items: + release_parent.contained_items.append(node) else: node.setPos(scene_pos) node.setVisible(True) @@ -687,6 +720,8 @@ def deleteFrame(self, frame): self.removeItem(frame) if frame in self.frames: self.frames.remove(frame) + if release_parent and release_parent.scene() == self: + release_parent.updateGeometry() self.scene_changed.emit() def deleteContainer(self, container): @@ -698,16 +733,23 @@ def deleteContainer(self, container): """ if hasattr(container, 'dispose'): container.dispose() - for item in container.contained_items: + release_parent = container.parentItem() if isinstance(container.parentItem(), Container) else None + if release_parent and container in release_parent.contained_items: + release_parent.contained_items.remove(container) + for item in list(container.contained_items): scene_pos = item.scenePos() - item.setParentItem(None) - item.setPos(scene_pos) + item.setParentItem(release_parent) + item.setPos(release_parent.mapFromScene(scene_pos) if release_parent else scene_pos) + if release_parent and item not in release_parent.contained_items: + release_parent.contained_items.append(item) item.setVisible(True) self.nodeMoved(item) self.removeItem(container) if container in self.containers: self.containers.remove(container) + if release_parent and release_parent.scene() == self: + release_parent.updateGeometry() self.scene_changed.emit() def keyPressEvent(self, event): @@ -798,7 +840,7 @@ def _set_branch_focus_state(self, item, is_active): def selectAllNodes(self): """Selects all node-like items in the scene, including plugins and charts.""" - for node in self._all_layout_nodes(): + for node in self._all_layout_nodes() + self.notes + self.frames + self.containers: node.setSelected(True) def register_transient_layout_item(self, item): @@ -1308,6 +1350,8 @@ def deleteSelectedItems(self): Deletes all currently selected items, handling each type appropriately. """ for item in list(self.selectedItems()): + if not isinstance(item, (Frame, Container)): + self._detach_item_from_groups(item, remove_empty=False) if isinstance(item, ChatNode): self.delete_chat_node(item) elif isinstance(item, CodeNode): for conn in self.content_connections[:]: diff --git a/graphlink_app/graphlink_session/deserializers.py b/graphlink_app/graphlink_session/deserializers.py index 1b26657..69f0d01 100644 --- a/graphlink_app/graphlink_session/deserializers.py +++ b/graphlink_app/graphlink_session/deserializers.py @@ -405,6 +405,8 @@ def deserialize_frame(self, data, scene, all_nodes_map): nodes = [all_nodes_map[index] for index in frame_item_indices if index in all_nodes_map] frame = Frame(nodes) + if data.get("id"): + frame.persistent_id = data["id"] frame.setPos(data["position"]["x"], data["position"]["y"]) frame.note = data["note"] @@ -415,19 +417,32 @@ def deserialize_frame(self, data, scene, all_nodes_map): if "size" in data: frame.rect.setWidth(data["size"]["width"]) frame.rect.setHeight(data["size"]["height"]) + rect_data = data.get("rect") + if rect_data: + frame.rect = QRectF(rect_data["x"], rect_data["y"], rect_data["width"], rect_data["height"]) + frame._user_resized = True + expanded_data = data.get("expanded_rect") + if expanded_data: + frame.expanded_rect = QRectF(expanded_data["x"], expanded_data["y"], expanded_data["width"], expanded_data["height"]) scene.addItem(frame) scene.frames.append(frame) frame.setZValue(-2) - if not data.get("is_locked", True): - frame.toggle_lock() - if data.get("is_collapsed", False): - frame.toggle_collapse() + frame.is_locked = data.get("is_locked", True) + frame.is_collapsed = data.get("is_collapsed", False) + frame._apply_lock_state() + for node in frame.nodes: + node.setVisible(not frame.is_collapsed) + if frame.is_collapsed: + frame.rect = QRectF(0, 0, frame.COLLAPSED_WIDTH, frame.COLLAPSED_HEIGHT) + frame._update_title_editor_geometry() return frame def deserialize_container(self, data, scene, all_items_map): items = [all_items_map[index] for index in data["items"] if index in all_items_map] container = Container(items) + if data.get("id"): + container.persistent_id = data["id"] container.setPos(data["position"]["x"], data["position"]["y"]) container.title = data.get("title", "Container") container.color = data.get("color", "#3a3a3a") @@ -438,9 +453,16 @@ def deserialize_container(self, data, scene, all_items_map): container.expanded_rect = QRectF( rect_data["x"], rect_data["y"], rect_data["width"], rect_data["height"] ) + rect_data = data.get("rect") + if rect_data: + container.rect = QRectF(rect_data["x"], rect_data["y"], rect_data["width"], rect_data["height"]) - if data.get("is_collapsed", False): - container.toggle_collapse() + container.is_collapsed = data.get("is_collapsed", False) + for item in container.contained_items: + item.setVisible(not container.is_collapsed) + if container.is_collapsed: + container.rect = QRectF(0, 0, container.COLLAPSED_WIDTH, container.COLLAPSED_HEIGHT) + container._update_title_editor_geometry() scene.addItem(container) scene.containers.append(container) @@ -485,6 +507,13 @@ def _load_notes(self, scene, notes_data): note.header_color = note_data["header_color"] note.is_system_prompt = note_data.get("is_system_prompt", False) note.is_summary_note = note_data.get("is_summary_note", False) + if note_data.get("id"): + note.persistent_id = note_data["id"] + note.note_role = note_data.get("role", "manual") + note.source_ids = list(note_data.get("source_ids", [])) + note.operation_id = note_data.get("operation_id", "") + note.source_revisions = dict(note_data.get("source_revisions", {})) + note.provider_snapshot = dict(note_data.get("provider_snapshot", {})) note.content = note_data["content"] notes_map[index] = note return notes_map diff --git a/graphlink_app/graphlink_session/serializers.py b/graphlink_app/graphlink_session/serializers.py index 13b1beb..92318c8 100644 --- a/graphlink_app/graphlink_session/serializers.py +++ b/graphlink_app/graphlink_session/serializers.py @@ -303,13 +303,17 @@ def _serialize_node_with_identity(self, node, all_nodes_list): def serialize_frame(self, frame, frame_items_map): return { + "id": self._node_persistent_id(frame), "items": [frame_items_map[item] for item in frame.nodes if item in frame_items_map], + "item_ids": [self._node_persistent_id(item) for item in frame.nodes], "position": {"x": frame.pos().x(), "y": frame.pos().y()}, "note": frame.note, "size": { "width": frame.rect.width(), "height": frame.rect.height(), }, + "rect": {"x": frame.rect.x(), "y": frame.rect.y(), "width": frame.rect.width(), "height": frame.rect.height()}, + "expanded_rect": {"x": frame.expanded_rect.x(), "y": frame.expanded_rect.y(), "width": frame.expanded_rect.width(), "height": frame.expanded_rect.height()}, "is_locked": frame.is_locked, "is_collapsed": frame.is_collapsed, "color": frame.color, @@ -318,7 +322,9 @@ def serialize_frame(self, frame, frame_items_map): def serialize_container(self, container, all_items_map): return { + "id": self._node_persistent_id(container), "items": [all_items_map[item] for item in container.contained_items], + "item_ids": [self._node_persistent_id(item) for item in container.contained_items], "position": {"x": container.pos().x(), "y": container.pos().y()}, "title": container.title, "is_collapsed": container.is_collapsed, @@ -330,10 +336,12 @@ def serialize_container(self, container, all_items_map): "width": container.expanded_rect.width(), "height": container.expanded_rect.height(), }, + "rect": {"x": container.rect.x(), "y": container.rect.y(), "width": container.rect.width(), "height": container.rect.height()}, } def serialize_note(self, note): return { + "id": self._node_persistent_id(note), "content": note.content, "position": {"x": note.pos().x(), "y": note.pos().y()}, "size": {"width": note.width, "height": note.height}, @@ -341,6 +349,11 @@ def serialize_note(self, note): "header_color": note.header_color, "is_system_prompt": getattr(note, "is_system_prompt", False), "is_summary_note": getattr(note, "is_summary_note", False), + "role": getattr(note, "note_role", "manual"), + "source_ids": list(getattr(note, "source_ids", [])), + "operation_id": getattr(note, "operation_id", ""), + "source_revisions": dict(getattr(note, "source_revisions", {})), + "provider_snapshot": dict(getattr(note, "provider_snapshot", {})), } def serialize_chart(self, chart, all_nodes_list): diff --git a/graphlink_app/graphlink_utility.py b/graphlink_app/graphlink_utility.py new file mode 100644 index 0000000..01ce575 --- /dev/null +++ b/graphlink_app/graphlink_utility.py @@ -0,0 +1,214 @@ +"""Shared lifecycle and context primitives for canvas utility operations.""" + +from dataclasses import dataclass, field +from enum import Enum +from threading import Event +from uuid import uuid4 + +from PySide6.QtCore import QObject, Signal + + +class UtilityKind(str, Enum): + TAKEAWAY = "takeaway" + EXPLAINER = "explainer" + GROUP_SUMMARY = "group_summary" + + +class UtilityOperationState(str, Enum): + PREPARING = "preparing" + RUNNING = "running" + CANCELLING = "cancelling" + SUCCEEDED = "succeeded" + FAILED = "failed" + CANCELLED = "cancelled" + STALE = "stale" + + +@dataclass(frozen=True) +class UtilitySourceSnapshot: + source_id: str + source_type: str + text: str + x: float = 0.0 + y: float = 0.0 + revision: str = "" + + +@dataclass(frozen=True) +class UtilityContextSnapshot: + operation_id: str + chat_epoch: int + sources: tuple[UtilitySourceSnapshot, ...] + rendered_context: str + estimated_tokens: int + omitted_source_ids: tuple[str, ...] = () + + +@dataclass(frozen=True) +class UtilityResult: + operation_id: str + kind: UtilityKind + content: str + context: UtilityContextSnapshot + provider_snapshot: dict = field(default_factory=dict) + + +@dataclass +class _Operation: + operation_id: str + kind: UtilityKind + context: UtilityContextSnapshot + state: UtilityOperationState = UtilityOperationState.PREPARING + cancel_event: Event = field(default_factory=Event) + + +class UtilityOperationController(QObject): + """Own operation identity, cancellation and stale-result guards.""" + + operation_started = Signal(object) + operation_state_changed = Signal(str, str) + operation_finished = Signal(object) + operation_failed = Signal(str, str) + operation_cancelled = Signal(str) + operation_stale = Signal(str) + + def __init__(self, parent=None): + super().__init__(parent) + self._operations = {} + + def begin(self, kind, sources, chat_epoch=0, rendered_context="", estimated_tokens=0, omitted_source_ids=()): + operation_id = uuid4().hex + context = UtilityContextSnapshot( + operation_id=operation_id, + chat_epoch=int(chat_epoch), + sources=tuple(sources), + rendered_context=rendered_context, + estimated_tokens=int(estimated_tokens), + omitted_source_ids=tuple(omitted_source_ids), + ) + operation = _Operation(operation_id, UtilityKind(kind), context) + self._operations[operation_id] = operation + self.operation_started.emit(operation) + return operation_id + + def _set_state(self, operation_id, state): + operation = self._operations.get(operation_id) + if operation is None: + return False + operation.state = UtilityOperationState(state) + self.operation_state_changed.emit(operation_id, operation.state.value) + return True + + def mark_running(self, operation_id): + return self._set_state(operation_id, UtilityOperationState.RUNNING) + + def is_current(self, operation_id, chat_epoch=None): + operation = self._operations.get(operation_id) + if operation is None: + return False + if operation.state in { + UtilityOperationState.CANCELLED, + UtilityOperationState.FAILED, + UtilityOperationState.STALE, + UtilityOperationState.SUCCEEDED, + }: + return False + return chat_epoch is None or operation.context.chat_epoch == chat_epoch + + def cancellation_requested(self, operation_id): + operation = self._operations.get(operation_id) + return bool(operation and operation.cancel_event.is_set()) + + def cancel(self, operation_id): + operation = self._operations.get(operation_id) + if operation is None or operation.state in { + UtilityOperationState.SUCCEEDED, + UtilityOperationState.FAILED, + UtilityOperationState.CANCELLED, + UtilityOperationState.STALE, + }: + return False + operation.cancel_event.set() + self._set_state(operation_id, UtilityOperationState.CANCELLED) + self.operation_cancelled.emit(operation_id) + return True + + def complete(self, operation_id, content, provider_snapshot=None): + operation = self._operations.get(operation_id) + if operation is None or not self.is_current(operation_id): + return None + result = UtilityResult( + operation_id=operation_id, + kind=operation.kind, + content=str(content or ""), + context=operation.context, + provider_snapshot=dict(provider_snapshot or {}), + ) + self._set_state(operation_id, UtilityOperationState.SUCCEEDED) + self.operation_finished.emit(result) + return result + + def fail(self, operation_id, message): + operation = self._operations.get(operation_id) + if operation is None or operation.state in { + UtilityOperationState.SUCCEEDED, + UtilityOperationState.CANCELLED, + UtilityOperationState.STALE, + }: + return False + self._set_state(operation_id, UtilityOperationState.FAILED) + self.operation_failed.emit(operation_id, str(message)) + return True + + def mark_stale(self, operation_id): + if not self._set_state(operation_id, UtilityOperationState.STALE): + return False + self.operation_stale.emit(operation_id) + return True + + def get(self, operation_id): + return self._operations.get(operation_id) + + def active_operations(self): + terminal = { + UtilityOperationState.SUCCEEDED, + UtilityOperationState.FAILED, + UtilityOperationState.CANCELLED, + UtilityOperationState.STALE, + } + return tuple(operation for operation in self._operations.values() if operation.state not in terminal) + + +def ensure_persistent_id(item): + persistent_id = getattr(item, "persistent_id", None) + if not persistent_id: + persistent_id = uuid4().hex + item.persistent_id = persistent_id + return persistent_id + + +def source_snapshot(item, text, revision=""): + position = item.scenePos() if hasattr(item, "scenePos") else None + return UtilitySourceSnapshot( + source_id=ensure_persistent_id(item), + source_type=type(item).__name__, + text=str(text or ""), + x=float(position.x()) if position is not None else 0.0, + y=float(position.y()) if position is not None else 0.0, + revision=str(revision or ""), + ) + + +def render_context(sources, max_chars=24000): + """Render deterministic, bounded utility context and report omitted sources.""" + rendered = [] + omitted = [] + used = 0 + for index, source in enumerate(sources, 1): + block = f"Source {index} ({source.source_type}):\n{source.text.strip()}\n" + if rendered and used + len(block) > max_chars: + omitted.extend(item.source_id for item in sources[index - 1:]) + break + rendered.append(block) + used += len(block) + return "\n".join(rendered).strip(), tuple(omitted) diff --git a/graphlink_app/graphlink_window.py b/graphlink_app/graphlink_window.py index 1ef86e6..b1db817 100644 --- a/graphlink_app/graphlink_window.py +++ b/graphlink_app/graphlink_window.py @@ -49,6 +49,7 @@ from graphlink_paths import asset_path from graphlink_crash import mark_clean_exit from graphlink_composer import ComposerController +from graphlink_utility import UtilityOperationController class ChatWindow(QMainWindow, WindowActionsMixin, WindowNavigationMixin): def __init__(self, settings_manager): @@ -90,6 +91,8 @@ def __init__(self, settings_manager): self._main_request_cancel_pending = False self._main_request_cancel_callback = None self.composer_controller = ComposerController(self) + self.utility_operation_controller = UtilityOperationController(self) + self.utility_threads = {} self.container = QWidget() container_layout = QVBoxLayout(self.container) @@ -464,6 +467,9 @@ def _iter_shutdown_threads(self): if worker is not None: yield label, worker, (lambda name=attr_name: setattr(self, name, None)) + for operation_id, worker in list(getattr(self, "utility_threads", {}).items()): + yield "canvas utility operation", worker, (lambda op=operation_id: self.utility_threads.pop(op, None)) + chat_view = getattr(self, "chat_view", None) scene = chat_view.scene() if chat_view is not None else None if scene is not None: @@ -632,7 +638,9 @@ def show_library(self): def keyPressEvent(self, event): if event.key() == Qt.Key.Key_Escape: - if self.pending_attachments: self.clear_attachment() + if self.cancel_latest_utility_operation(): + event.accept() + elif self.pending_attachments: self.clear_attachment() elif event.modifiers() & Qt.KeyboardModifier.ControlModifier: if event.key() == Qt.Key.Key_N: view_pos = self.chat_view.mapFromGlobal(QCursor.pos()); scene_pos = self.chat_view.mapToScene(view_pos); self.chat_view.scene().add_note(scene_pos) diff --git a/graphlink_app/graphlink_window_actions.py b/graphlink_app/graphlink_window_actions.py index b1fb0fa..0ebb616 100644 --- a/graphlink_app/graphlink_window_actions.py +++ b/graphlink_app/graphlink_window_actions.py @@ -11,6 +11,7 @@ from graphlink_node import ChatNode, CodeNode from graphlink_canvas_items import Note from graphlink_connections import GroupSummaryConnectionItem +from graphlink_utility import UtilityKind, render_context, source_snapshot from graphlink_pycoder import PyCoderMode, PyCoderNode from graphlink_plugins.graphlink_plugin_code_sandbox import CodeSandboxNode from graphlink_web import WebNode @@ -37,6 +38,94 @@ ) class WindowActionsMixin: + def _utility_chat_epoch(self): + return int(getattr(getattr(self, "session_manager", None), "_context_epoch", 0)) + + def _utility_bounded_text(self, node): + snapshot = source_snapshot(node, getattr(node, "text", "")) + return render_context([snapshot])[0] + + def _utility_bounded_texts(self, nodes): + snapshots = [source_snapshot(node, getattr(node, "text", "")) for node in nodes] + rendered, omitted = render_context(snapshots) + omitted_ids = set(omitted) + return [snapshot.text for snapshot in snapshots if snapshot.source_id not in omitted_ids] + + def _utility_start(self, kind, source_nodes, worker, finished_factory): + snapshots = [source_snapshot(node, getattr(node, "text", "")) for node in source_nodes] + rendered_context, omitted = render_context(snapshots) + operation_id = self.utility_operation_controller.begin( + kind, snapshots, chat_epoch=self._utility_chat_epoch(), + rendered_context=rendered_context, + estimated_tokens=max(1, len(rendered_context) // 4), + omitted_source_ids=omitted, + ) + self.utility_operation_controller.mark_running(operation_id) + self.utility_threads[operation_id] = worker + worker.finished.connect(finished_factory(operation_id)) + worker.error.connect(lambda message, op=operation_id: self._utility_failed(op, message)) + worker.finished.connect(worker.deleteLater) + worker.error.connect(worker.deleteLater) + worker.start() + return operation_id + + def _utility_failed(self, operation_id, message): + if self.utility_operation_controller.cancellation_requested(operation_id): + self._utility_cleanup(operation_id) + self._clear_loading_animation() + return + self.utility_operation_controller.fail(operation_id, message) + self._utility_cleanup(operation_id) + self.handle_error(message) + + def _utility_cleanup(self, operation_id): + self.utility_threads.pop(operation_id, None) + + def _utility_result(self, operation_id, source_nodes, response): + controller = self.utility_operation_controller + scene = self.chat_view.scene() + if not controller.is_current(operation_id, self._utility_chat_epoch()): + controller.mark_stale(operation_id) + self._utility_cleanup(operation_id) + self._clear_loading_animation() + return None + if any(node.scene() != scene for node in source_nodes): + controller.mark_stale(operation_id) + self._utility_cleanup(operation_id) + self._clear_loading_animation() + return None + result = controller.complete(operation_id, response) + self._utility_cleanup(operation_id) + return result + + def cancel_latest_utility_operation(self): + active = self.utility_operation_controller.active_operations() + if not active: + return False + operation = active[-1] + self.utility_operation_controller.cancel(operation.operation_id) + worker = self.utility_threads.get(operation.operation_id) + if worker is not None and hasattr(worker, "stop"): + worker.stop() + self._utility_cleanup(operation.operation_id) + self._clear_loading_animation() + self.notification_banner.show_message("Utility generation cancelled.", 2500, "info") + return True + + def _decorate_utility_note(self, note, result, role, source_nodes): + note.note_role = role.value + note.operation_id = result.operation_id + note.source_ids = [source.source_id for source in result.context.sources] + note.source_revisions = {source.source_id: source.revision for source in result.context.sources if source.revision} + note.provider_snapshot = result.provider_snapshot + note.is_summary_note = role == UtilityKind.GROUP_SUMMARY + scene = self.chat_view.scene() + for source_node in source_nodes: + conn = GroupSummaryConnectionItem(source_node, note) + scene.addItem(conn) + scene.group_summary_connections.append(conn) + scene.register_connection(conn) + def _graphics_item_dimensions(self, item): if item is None: return 0.0, 0.0 @@ -493,22 +582,28 @@ def handle_regenerated_response(self, new_assistant_message, old_node, parent_hi def generate_takeaway(self, node): try: self._show_loading_animation(anchor_node=node) - self.takeaway_thread = KeyTakeawayWorkerThread(KeyTakeawayAgent(), node.text, node.scenePos()) - self.takeaway_thread.finished.connect(self.handle_takeaway_response) - self.takeaway_thread.error.connect(self.handle_error) - self.takeaway_thread.finished.connect(self.takeaway_thread.deleteLater) - self.takeaway_thread.error.connect(self.takeaway_thread.deleteLater) - self.takeaway_thread.start() + self.takeaway_thread = KeyTakeawayWorkerThread(KeyTakeawayAgent(), self._utility_bounded_text(node), node.scenePos()) + self._utility_start( + UtilityKind.TAKEAWAY, [node], self.takeaway_thread, + lambda operation_id: lambda response, node_pos: self.handle_takeaway_response( + operation_id, response, node_pos, [node] + ), + ) except Exception as e: self.handle_error(f"Error generating takeaway: {str(e)}") - def handle_takeaway_response(self, response, node_pos): + def handle_takeaway_response(self, operation_id, response, node_pos, source_nodes): try: + result = self._utility_result(operation_id, source_nodes, response) + if result is None: + return note_pos = QPointF(node_pos.x() + 400, node_pos.y()) note = self.chat_view.scene().add_note(note_pos) note.width, note.content = 400, response note.color, note.header_color = get_current_palette().FRAME_COLORS["Mid Gray"]["color"], get_semantic_color("status_info").name() + self._decorate_utility_note(note, result, UtilityKind.TAKEAWAY, source_nodes) note._recalculate_geometry() + self.save_chat() except Exception as e: self.handle_error(f"Error creating takeaway note: {str(e)}") finally: @@ -521,7 +616,10 @@ def generate_group_summary(self): if len(selected_nodes) < 2: self.notification_banner.show_message("Please select two or more chat nodes to summarize.", 5000, "warning") return - texts = [node.text for node in selected_nodes] + texts = self._utility_bounded_texts(selected_nodes) + if len(texts) < 2: + self.notification_banner.show_message("Selected context exceeds the utility limit; select fewer sources.", 5000, "warning") + return avg_x, max_x, avg_y = 0, 0, 0 for node in selected_nodes: pos = node.scenePos() @@ -531,27 +629,27 @@ def generate_group_summary(self): note_pos = QPointF(max_x + 100, avg_y / len(selected_nodes)) self._show_loading_animation(scene_pos=QPointF(note_pos.x() - 50, note_pos.y())) self.group_summary_thread = GroupSummaryWorkerThread(GroupSummaryAgent(), texts, note_pos, selected_nodes) - self.group_summary_thread.finished.connect(self.handle_group_summary_response) - self.group_summary_thread.error.connect(self.handle_error) - self.group_summary_thread.finished.connect(self.group_summary_thread.deleteLater) - self.group_summary_thread.error.connect(self.group_summary_thread.deleteLater) - self.group_summary_thread.start() + self._utility_start( + UtilityKind.GROUP_SUMMARY, selected_nodes, self.group_summary_thread, + lambda operation_id: lambda response, result_pos, result_sources: self.handle_group_summary_response( + operation_id, response, result_pos, result_sources + ), + ) except Exception as e: self.handle_error(f"Error generating group summary: {str(e)}") - def handle_group_summary_response(self, response, note_pos, source_nodes): + def handle_group_summary_response(self, operation_id, response, note_pos, source_nodes): try: + result = self._utility_result(operation_id, source_nodes, response) + if result is None: + return scene = self.chat_view.scene() note = scene.add_note(note_pos) note.content, note.color, note.header_color = response, get_current_palette().FRAME_COLORS["Mid Gray"]["color"], get_semantic_color("status_warning").name() note.width, note.is_summary_note = 450, True + self._decorate_utility_note(note, result, UtilityKind.GROUP_SUMMARY, source_nodes) note._recalculate_geometry() - for source_node in source_nodes: - if source_node.scene() == scene: - conn = GroupSummaryConnectionItem(source_node, note) - scene.addItem(conn) - scene.group_summary_connections.append(conn) - scene.register_connection(conn) + self.save_chat() except Exception as e: self.handle_error(f"Error creating summary note: {str(e)}") finally: @@ -560,22 +658,28 @@ def handle_group_summary_response(self, response, note_pos, source_nodes): def generate_explainer(self, node): try: self._show_loading_animation(anchor_node=node) - self.explainer_thread = ExplainerWorkerThread(ExplainerAgent(), node.text, node.scenePos()) - self.explainer_thread.finished.connect(self.handle_explainer_response) - self.explainer_thread.error.connect(self.handle_error) - self.explainer_thread.finished.connect(self.explainer_thread.deleteLater) - self.explainer_thread.error.connect(self.explainer_thread.deleteLater) - self.explainer_thread.start() + self.explainer_thread = ExplainerWorkerThread(ExplainerAgent(), self._utility_bounded_text(node), node.scenePos()) + self._utility_start( + UtilityKind.EXPLAINER, [node], self.explainer_thread, + lambda operation_id: lambda response, node_pos: self.handle_explainer_response( + operation_id, response, node_pos, [node] + ), + ) except Exception as e: self.handle_error(f"Error generating explanation: {str(e)}") - def handle_explainer_response(self, response, node_pos): + def handle_explainer_response(self, operation_id, response, node_pos, source_nodes): try: + result = self._utility_result(operation_id, source_nodes, response) + if result is None: + return note_pos = QPointF(node_pos.x() + 400, node_pos.y() + 100) note = self.chat_view.scene().add_note(note_pos) note.width, note.content = 400, response note.color, note.header_color = get_current_palette().FRAME_COLORS["Mid Gray"]["color"], get_semantic_color("status_info").name() + self._decorate_utility_note(note, result, UtilityKind.EXPLAINER, source_nodes) note._recalculate_geometry() + self.save_chat() except Exception as e: self.handle_error(f"Error creating explainer note: {str(e)}") finally: diff --git a/graphlink_app/tests/test_utility_tools.py b/graphlink_app/tests/test_utility_tools.py new file mode 100644 index 0000000..78ae61c --- /dev/null +++ b/graphlink_app/tests/test_utility_tools.py @@ -0,0 +1,152 @@ +"""Focused regression coverage for canvas utilities and group invariants.""" + +from unittest.mock import MagicMock + +from PySide6.QtCore import QPointF, QRectF +from PySide6.QtGui import QKeyEvent +from PySide6.QtCore import QEvent, Qt + +from graphlink_canvas_items import Container, Frame, Note +from graphlink_scene import ChatScene +from graphlink_session.serializers import SceneSerializer +from graphlink_utility import ( + UtilityKind, + UtilityOperationController, + UtilityOperationState, + render_context, + source_snapshot, +) + + +def make_scene(): + window = MagicMock() + scene = ChatScene(window) + window.chat_view.scene.return_value = scene + return window, scene + + +def test_frame_construction_uses_installed_qtawesome_icons(): + _, scene = make_scene() + node = scene.add_chat_node("A") + frame = Frame([node]) + scene.addItem(frame) + scene.frames.append(frame) + assert frame.lock_icon is not None + assert frame.unlock_icon is not None + + +def test_group_moves_remove_stale_membership_and_preserve_invariants(): + _, scene = make_scene() + first = scene.add_chat_node("first") + second = scene.add_chat_node("second") + first.setSelected(True) + second.setSelected(True) + scene.createContainer() + container = scene.containers[-1] + assert scene.validate_group_invariants() == [] + + scene.clearSelection() + first.setSelected(True) + scene.createFrame() + frame = scene.frames[-1] + + assert first in frame.nodes + assert first not in container.contained_items + assert second in container.contained_items + assert scene.validate_group_invariants() == [] + + +def test_nested_container_delete_reparents_children_without_dangling_parent_entry(): + _, scene = make_scene() + node = scene.add_chat_node("child") + inner = Container([node]) + scene.addItem(inner) + scene.containers.append(inner) + outer = Container([inner]) + scene.addItem(outer) + scene.containers.append(outer) + + scene.deleteContainer(inner) + + assert inner not in outer.contained_items + assert node in outer.contained_items + assert node.parentItem() is outer + assert scene.validate_group_invariants() == [] + + +def test_utility_operation_controller_guards_completion_after_cancel(): + controller = UtilityOperationController() + source = source_snapshot(MagicMock(scenePos=lambda: QPointF(10, 20)), "source") + operation_id = controller.begin(UtilityKind.TAKEAWAY, [source], chat_epoch=3) + controller.mark_running(operation_id) + assert controller.cancel(operation_id) + assert controller.get(operation_id).state == UtilityOperationState.CANCELLED + assert controller.complete(operation_id, "late result") is None + + +def test_utility_context_is_bounded_and_reports_omitted_sources(): + source_a = source_snapshot(MagicMock(scenePos=lambda: QPointF()), "a" * 20) + source_b = source_snapshot(MagicMock(scenePos=lambda: QPointF()), "b" * 20) + rendered, omitted = render_context([source_a, source_b], max_chars=35) + assert "Source 1" in rendered + assert source_b.source_id in omitted + + +def test_note_provenance_and_exact_frame_geometry_are_serialized(): + window, scene = make_scene() + node = scene.add_chat_node("source") + note = scene.add_note(QPointF(30, 40)) + note.content = "generated" + note.note_role = "explainer" + note.operation_id = "operation-1" + note.source_ids = [node.persistent_id if hasattr(node, "persistent_id") else "source-1"] + frame = Frame([node]) + scene.addItem(frame) + scene.frames.append(frame) + frame.rect = QRectF(2, 3, 410, 220) + frame.expanded_rect = QRectF(4, 5, 510, 320) + + payload = SceneSerializer(window).serialize_chat_data() + note_payload = payload["notes_data"][0] + frame_payload = payload["frames"][0] + assert note_payload["role"] == "explainer" + assert note_payload["operation_id"] == "operation-1" + assert frame_payload["rect"]["width"] == 410 + assert frame_payload["expanded_rect"]["height"] == 320 + + +def test_utility_items_are_searchable_and_note_picker_is_safe_without_a_view(): + _, scene = make_scene() + note = scene.add_note(QPointF()) + note.content = "utility provenance" + note.show_color_picker() + assert note in scene.find_items("provenance") + + +def test_note_editor_supports_bounded_content_and_undo_redo(): + note = Note(QPointF()) + note.editing = True + note.edit_text = "before" + note.cursor_pos = len(note.edit_text) + note.selection_start = note.selection_end = note.cursor_pos + note._push_edit_snapshot() + note.edit_text += " after" + note.undo_edit() + assert note.edit_text == "before" + note.redo_edit() + assert note.edit_text == "before after" + note.content = "x" * (note.MAX_CONTENT_LENGTH + 10) + assert len(note.content) == note.MAX_CONTENT_LENGTH + + +def test_frame_can_shrink_again_after_manual_resize(): + _, scene = make_scene() + node = scene.add_chat_node("small") + frame = Frame([node]) + scene.addItem(frame) + scene.frames.append(frame) + frame.rect = QRectF(0, 0, 1200, 900) + frame._user_resized = True + frame.fit_to_content() + assert frame.rect.width() < 1200 + assert frame.rect.height() < 900 diff --git a/pyproject.toml b/pyproject.toml index 4453c10..2438271 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -94,6 +94,7 @@ py-modules = [ "graphlink_styles", "graphlink_system_dialogs", "graphlink_ui_components", + "graphlink_utility", "graphlink_update", "graphlink_version", "graphlink_view",