Fix silent startup crash when the saved session contains navigation pins

graphlink_app/graphlink_connections.py

@@ -350,11 +350,11 @@ def remove_pin(self, pin):
     def clear(self):
         """
         Clears all pins from the connection. (Note: This is a partial implementation
-        and seems to be a remnant, as the main scene clear handles most cleanup).
+        and seems to be a remnant, as the main scene clear handles most cleanup.
+        The legacy pin_overlay.clear_pins() call was removed with the PinOverlayHost
+        migration - that method no longer exists, and scene.pin_store is the
+        overlay's reactive source of truth.)
         """
-        if self.window and hasattr(self.window, 'pin_overlay'):
-            self.window.pin_overlay.clear_pins()
-
         self.pins.clear()
 
         self.nodes.clear()

graphlink_app/graphlink_session/deserializers.py

@@ -540,9 +540,15 @@ def _load_notes(self, scene, notes_data):
         return notes_map
 
     def _load_pins(self, scene, pins_data):
-        if self.window and hasattr(self.window, "pin_overlay"):
-            self.window.pin_overlay.clear_pins()
-
+        # No imperative pin_overlay sync here: the legacy PinOverlay's
+        # clear_pins()/add_pin_button() calls this method used to make do not
+        # exist on PinOverlayHost (Phase 5), and are unnecessary with the
+        # store-based design - restore_chat's scene.clear() already emptied
+        # scene.pin_store, add_navigation_pin() below registers each restored
+        # pin in it, and the overlay follows the store reactively. The stale
+        # calls crashed the ENTIRE chat restore (AttributeError) the first
+        # time a saved session actually contained pins - the app then exited
+        # with no window, looking like a silent startup hang.
         valid_records = []
         for index, pin_data in enumerate(pins_data or []):
             try:
@@ -552,7 +558,7 @@ def _load_pins(self, scene, pins_data):
 
         for record in sorted(valid_records, key=lambda item: item.sort_order):
             try:
-                pin = scene.add_navigation_pin(
+                scene.add_navigation_pin(
                     QPointF(record.position[0], record.position[1]),
                     title=record.title,
                     note=record.note,
@@ -565,8 +571,6 @@ def _load_pins(self, scene, pins_data):
                     "warning",
                 )
                 continue
-            if self.window and hasattr(self.window, "pin_overlay"):
-                self.window.pin_overlay.add_pin_button(pin)
 
     def _restore_view_state(self, chat_data):
         view_state = chat_data.get("view_state")
@@ -602,8 +606,11 @@ def _handle_load_error(self, scene, error):
             self.window.message_input.setPlaceholderText("Type your message...")
             self.window.update_title_bar()
             self.window.reset_token_counter()
-            if hasattr(self.window, "pin_overlay") and self.window.pin_overlay:
-                self.window.pin_overlay.clear_pins()
+            # No pin_overlay.clear_pins() here: the method was the legacy
+            # PinOverlay's - PinOverlayHost has no such facade, so this
+            # RECOVERY path itself crashed with the same AttributeError it
+            # was recovering from. scene.clear() above already emptied
+            # scene.pin_store, which the overlay follows reactively.
 
     def restore_chat(self, chat, notes_data, pins_data):
         scene = self._scene()

graphlink_app/graphlink_window.py

@@ -1418,6 +1418,8 @@ def new_chat(self, parent_for_dialog=None):
                 self._set_main_request_state(active=False)
                 self._clear_loading_animation()
                 self._clear_pending_response_preview()
-            if hasattr(self, 'pin_overlay') and self.pin_overlay: self.pin_overlay.clear_pins()
+            # No pin_overlay.clear_pins() (a legacy-PinOverlay method that does
+            # not exist on PinOverlayHost): scene.clear() below empties
+            # scene.pin_store, which the overlay follows reactively.
             self.session_manager.mark_context_switch(); self.session_manager.current_chat_id = None; scene.clear(); self.current_node = None; self.message_input.clear(); self.clear_attachment(); self.message_input.set_context_anchor(None); self.message_input.setPlaceholderText("Type your message..."); self.update_title_bar(); self.reset_token_counter(); return True
         return False

graphlink_app/tests/test_pins_restore_regression.py

@@ -0,0 +1,93 @@
+"""Regression for the silent startup crash the first time a saved session
+contained navigation pins.
+
+The session deserializer's _load_pins (and its _handle_load_error recovery
+path, plus new_chat and a connections-module remnant) still called the
+LEGACY PinOverlay's clear_pins()/add_pin_button() - methods that do not
+exist on PinOverlayHost (Phase 5's replacement, wired as window.pin_overlay).
+The AttributeError aborted the ENTIRE chat restore at launch; the recovery
+handler then crashed on the same missing method, and the app exited with no
+window - presenting as "the app sits spinning and never opens." It was
+data-dependent: nothing surfaced until a user actually created pins and
+saved, which is why empty-session test drives never caught it.
+
+The store-based design makes those imperative calls unnecessary: scene.clear()
+empties scene.pin_store (the overlay's reactive source of truth) and
+scene.add_navigation_pin() registers restored pins in it. These tests pin the
+contract down with a pin_overlay object that has NO legacy methods at all -
+any reintroduced legacy call fails loudly here.
+"""
+
+import sys
+import types
+from pathlib import Path
+from unittest.mock import MagicMock
+
+sys.path.insert(0, str(Path(__file__).resolve().parents[1]))
+
+from PySide6.QtWidgets import QApplication
+
+_APP = QApplication.instance() or QApplication([])
+
+from graphlink_scene import ChatScene
+from graphlink_session.deserializers import SceneDeserializer
+
+
+def _make_window_and_scene():
+    window = MagicMock()
+    # The decisive detail: a pin_overlay with NO methods at all. The real
+    # PinOverlayHost has no clear_pins/add_pin_button either - a MagicMock
+    # would silently absorb the legacy calls and hide the regression.
+    window.pin_overlay = types.SimpleNamespace()
+    scene = ChatScene(window=window)
+    window.chat_view.scene.return_value = scene
+    return window, scene
+
+
+_PIN_PAYLOAD = [
+    {
+        "id": "pin-1",
+        "title": "First waypoint",
+        "note": "remember this",
+        "position": {"x": 120.0, "y": 340.0},
+        "sort_order": 0,
+    },
+    {
+        "id": "pin-2",
+        "title": "Second waypoint",
+        "note": "",
+        "position": {"x": -40.0, "y": 12.5},
+        "sort_order": 1,
+    },
+]
+
+
+class TestLoadPinsAgainstTheRealHostSurface:
+    def test_restoring_pins_does_not_touch_pin_overlay_and_lands_in_the_store(self):
+        window, scene = _make_window_and_scene()
+        deserializer = SceneDeserializer(window)
+
+        deserializer._load_pins(scene, _PIN_PAYLOAD)  # must not raise
+
+        assert len(scene.pin_store.records) == 2
+        titles = sorted(record.title for record in scene.pin_store.records)
+        assert titles == ["First waypoint", "Second waypoint"]
+
+    def test_restoring_zero_pins_is_a_no_op(self):
+        window, scene = _make_window_and_scene()
+        deserializer = SceneDeserializer(window)
+
+        deserializer._load_pins(scene, [])
+        deserializer._load_pins(scene, None)
+
+        assert len(scene.pin_store.records) == 0
+
+    def test_load_error_recovery_does_not_touch_pin_overlay(self):
+        # The recovery path itself crashed on the same missing legacy method
+        # it was recovering from - the app then died with no window at all.
+        window, scene = _make_window_and_scene()
+        deserializer = SceneDeserializer(window)
+
+        deserializer._handle_load_error(scene, RuntimeError("boom"))  # must not raise
+
+        assert window.current_node is None