Fix layer rename reverting to default; auto-index duplicate names (#103)

src/optiverse/core/models.py

@@ -263,6 +263,9 @@ class SourceParams:
     x_mm: float = -400.0
     y_mm: float = 0.0
     angle_deg: float = 0.0
+    # Custom layer-panel name (None = fall back to the "Source" type label).
+    # Without this field a source rename had nowhere to be stored and reverted.
+    name: str | None = None
     size_mm: float = 10.0
     n_rays: int = 9
     ray_length_mm: float = 1000.0

src/optiverse/objects/annotations/text_note_item.py

@@ -267,6 +267,7 @@ def clone(self, offset_mm: tuple[float, float] = (20.0, 20.0)) -> TextNoteItem:
 
         new_item = TextNoteItem(self.toPlainText())
 
+        new_item.display_name = self.display_name  # keep custom layer name for indexing
         new_item.setDefaultTextColor(self.defaultTextColor())
         new_item.setFont(self.font())
         new_item.setZValue(self.zValue())
@@ -290,6 +291,8 @@ def to_dict(self) -> dict[str, Any]:
         }
         if self._locked:
             d["locked"] = True
+        if self.display_name:
+            d["display_name"] = self.display_name
         if self.owner_uuid is not None:
             d["owner_uuid"] = self.owner_uuid
             d["owner_offset_x"] = float(self._owner_offset.x())
@@ -319,6 +322,8 @@ def from_dict(d: dict[str, Any]) -> TextNoteItem:
             item.setZValue(float(d["z_value"]))
         if d.get("locked"):
             item.set_locked(True)
+        if d.get("display_name"):
+            item.display_name = str(d["display_name"])
 
         # Restore autolabel link
         owner_uuid = d.get("owner_uuid")

src/optiverse/objects/naming.py

@@ -0,0 +1,91 @@
+"""Shared helpers for the display names shown in the layer panel.
+
+The label for a scene item is resolved the same way in three places (the layer
+model, the rename command, and duplicate auto-indexing), so the logic lives here
+once: ``display_name`` → ``params.name`` → prettified ``type_name``.
+"""
+
+from __future__ import annotations
+
+from typing import TYPE_CHECKING
+
+if TYPE_CHECKING:
+    from PyQt6 import QtWidgets
+
+
+def item_label(item: object) -> str:
+    """Return the label shown for *item* in the layer panel.
+
+    Mirrors ``LayerItemModel._get_item_name``: a custom ``display_name`` wins,
+    then ``params.name``, then the item's ``type_name`` (prettified).
+    """
+    display_name = getattr(item, "display_name", None)
+    if display_name:
+        return str(display_name)
+    params = getattr(item, "params", None)
+    name = getattr(params, "name", None) if params is not None else None
+    if name:
+        return str(name)
+    type_name = getattr(item, "type_name", None)
+    return type_name.replace("_", " ").title() if type_name else "Item"
+
+
+def set_item_label(item: object, name: str) -> None:
+    """Write *name* to the item's rename target (``display_name`` or ``params.name``)."""
+    if hasattr(item, "display_name"):
+        item.display_name = name
+        return
+    params = getattr(item, "params", None)
+    if params is not None and hasattr(params, "name"):
+        params.name = name
+
+
+def _is_nameable(item: object) -> bool:
+    """True for user-facing layer rows.
+
+    Excludes autolabels (a ``TextNoteItem`` with an ``owner_uuid``), which show
+    their owner's optical property and are not independently renameable — they
+    must not take part in duplicate indexing or be counted as taken names.
+    """
+    return hasattr(item, "item_uuid") and getattr(item, "owner_uuid", None) is None
+
+
+def assign_unique_name(
+    scene: QtWidgets.QGraphicsScene, item: object, *, reserved: set[str] | None = None
+) -> str:
+    """Give *item* a duplicate-free label: ``Lens`` → ``Lens 2`` → ``Lens 3``.
+
+    Compares the item's default label against every other named item already in
+    *scene* (plus any names in *reserved*, used to keep a multi-item paste batch
+    internally unique). The first element of a kind keeps its bare name; only
+    collisions get a numeric suffix. Call on freshly created items *before*
+    adding them to the scene. Returns the final label.
+    """
+    base = item_label(item)
+    if not _is_nameable(item):
+        return base
+    existing = {
+        item_label(other)
+        for other in scene.items()
+        if other is not item and _is_nameable(other)
+    }
+    if reserved:
+        existing |= reserved
+    if base not in existing:
+        return base
+    n = 2
+    while f"{base} {n}" in existing:
+        n += 1
+    new_name = f"{base} {n}"
+    set_item_label(item, new_name)
+    return new_name
+
+
+def assign_unique_names(scene: QtWidgets.QGraphicsScene, items: list[object]) -> None:
+    """Auto-index a batch (e.g. a paste/duplicate) so it stays unique against the
+    scene *and* against the rest of the batch. Autolabels are skipped."""
+    taken: set[str] = set()
+    for item in items:
+        if not _is_nameable(item):
+            continue
+        taken.add(assign_unique_name(scene, item, reserved=taken))

src/optiverse/ui/controllers/component_operations.py

@@ -105,6 +105,11 @@ def on_drop_component(self, rec: dict, scene_pos: QtCore.QPointF):
         # Connect signals
         self._connect_item_signals(item)
 
+        # Auto-index duplicate names (e.g. second "Lens" becomes "Lens 2")
+        from ...objects.naming import assign_unique_name
+
+        assign_unique_name(self.scene, item)
+
         # Add to scene with undo support
         cmd = AddItemCommand(self.scene, item, self._layer_state)
         self.undo_stack.push(cmd)
@@ -329,6 +334,12 @@ def paste_items(self, target_pos: QtCore.QPointF | None = None):
                 f"Successfully pasted {len(pasted_items)} item(s)", LogCategory.COPY_PASTE
             )
 
+            # Auto-index duplicate names across the scene and within this batch
+            # (a pasted/duplicated "Lens" becomes "Lens 2", two at once "Lens 2"/"Lens 3")
+            from ...objects.naming import assign_unique_names
+
+            assign_unique_names(self.scene, pasted_items)
+
             cmd = PasteItemsCommand(self.scene, pasted_items, self._layer_state)
             self.undo_stack.push(cmd)
 
@@ -424,6 +435,11 @@ def _paste_deserialized_items(
             it.setPos(it.pos().x() + dx, it.pos().y() + dy)
             self._connect_item_signals(it)
 
+        # Auto-index duplicate names against the scene and within this batch
+        from ...objects.naming import assign_unique_names
+
+        assign_unique_names(self.scene, items)
+
         cmd = PasteItemsCommand(self.scene, items, self._layer_state)
         self.undo_stack.push(cmd)
 

src/optiverse/ui/models/layer_item_model.py

@@ -440,14 +440,12 @@ def _prune_orphan_items(self) -> None:
             self._layer_state.remove_item(uid, emit=False)
 
     def _get_item_name(self, uuid: str) -> str:
+        from ...objects.naming import item_label
+
         item = self._get_live_item(uuid)
         if not item:
             return "Item"
-        if hasattr(item, "display_name") and item.display_name:
-            return str(item.display_name)
-        if hasattr(item, "params") and hasattr(item.params, "name") and item.params.name:
-            return str(item.params.name)
-        return item.type_name.replace("_", " ").title() if hasattr(item, "type_name") else "Item"
+        return item_label(item)
 
     def _uuids_from_indexes(self, indexes: list[QtCore.QModelIndex]) -> list[str]:
         uuids: list[str] = []

src/optiverse/ui/views/placement_handler.py

@@ -405,6 +405,11 @@ def _place_component(self, scene_pos: QtCore.QPointF) -> None:
         if component_type not in ("text", "rectangle"):
             self._connect_item_signals(placed_item)
 
+        # Auto-index duplicate names (e.g. second "Lens" becomes "Lens 2")
+        from ...objects.naming import assign_unique_name
+
+        assign_unique_name(self.scene, placed_item)
+
         # Add to scene with undo support
         cmd = AddItemCommand(self.scene, placed_item, self._layer_state)
         self.undo_stack.push(cmd)

src/optiverse/ui/widgets/layer_panel.py

@@ -171,6 +171,12 @@ def refresh(self) -> None:
     def _do_refresh(self) -> None:
         if not self._scene:
             return
+        # Never reset the model while the user is renaming a layer: a
+        # beginResetModel/endResetModel tears down the open inline editor and the
+        # typed name is lost. Defer until the edit finishes.
+        if self._tree.state() == QtWidgets.QAbstractItemView.State.EditingState:
+            self._refresh_timer.start(100)
+            return
         self._model.set_context(
             scene=self._scene,
             layer_state=self._layer_state,
@@ -280,6 +286,11 @@ def sync_from_scene_selection(self) -> None:
     def _do_sync_from_scene_selection(self) -> None:
         if not self._scene:
             return
+        # Selecting a layer for rename triggers this sync; clearing/reselecting
+        # the tree's selection mid-edit closes the inline editor before the name
+        # is committed. Skip while editing — a later selection change re-syncs.
+        if self._tree.state() == QtWidgets.QAbstractItemView.State.EditingState:
+            return
         uuids = {
             item.item_uuid for item in self._scene.selectedItems() if hasattr(item, "item_uuid")
         }

tests/core/test_naming.py

@@ -0,0 +1,144 @@
+"""Unit tests for layer-name resolution and duplicate auto-indexing (issue #103).
+
+These exercise ``optiverse.objects.naming`` with lightweight duck-typed fakes so
+they run without a QApplication (and therefore in the headless CI suite, which
+skips ``tests/ui``). The Qt model/view and serialization integration is covered
+separately in ``tests/ui/test_layer_rename_and_autoindex.py``.
+"""
+
+from __future__ import annotations
+
+from types import SimpleNamespace
+
+from optiverse.objects.naming import (
+    assign_unique_name,
+    assign_unique_names,
+    item_label,
+    set_item_label,
+)
+
+_uid = 0
+
+
+def _next_uuid() -> str:
+    global _uid
+    _uid += 1
+    return f"uuid-{_uid}"
+
+
+class FakeScene:
+    """Minimal stand-in for QGraphicsScene exposing ``items()``."""
+
+    def __init__(self, items=()):
+        self._items = list(items)
+
+    def items(self):
+        return list(self._items)
+
+    def add(self, item):
+        self._items.append(item)
+        return item
+
+
+def component(name=None):
+    """Component-like item: label comes from ``params.name`` (falls back to type)."""
+    return SimpleNamespace(
+        item_uuid=_next_uuid(),
+        params=SimpleNamespace(name=name),
+        type_name="component",
+        owner_uuid=None,
+    )
+
+
+def text(display_name=None, owner_uuid=None):
+    """Text-like item: label comes from ``display_name`` (falls back to type)."""
+    return SimpleNamespace(
+        item_uuid=_next_uuid(),
+        display_name=display_name,
+        type_name="text",
+        owner_uuid=owner_uuid,
+    )
+
+
+# --- label resolution -------------------------------------------------------
+def test_item_label_resolution_order():
+    assert item_label(text(display_name="Custom")) == "Custom"
+    assert item_label(component(name="Achromat")) == "Achromat"
+    assert item_label(component(name=None)) == "Component"  # type_name fallback
+    assert item_label(SimpleNamespace(type_name="beam_source")) == "Beam Source"
+
+
+def test_set_item_label_targets_the_right_field():
+    t = text()
+    set_item_label(t, "Note A")
+    assert t.display_name == "Note A"
+
+    c = component(name="Lens")
+    set_item_label(c, "Lens 2")
+    assert c.params.name == "Lens 2"
+
+
+# --- single-add auto-indexing ----------------------------------------------
+def test_first_of_a_kind_keeps_bare_name():
+    scene = FakeScene()
+    c = component("Lens")
+    assert assign_unique_name(scene, c) == "Lens"
+    assert c.params.name == "Lens"  # unchanged
+
+
+def test_sequential_adds_increment():
+    scene = FakeScene()
+    labels = []
+    for _ in range(3):
+        c = component("Lens")
+        assign_unique_name(scene, c)
+        scene.add(c)
+        labels.append(item_label(c))
+    assert labels == ["Lens", "Lens 2", "Lens 3"]
+
+
+def test_gap_is_refilled():
+    scene = FakeScene()
+    kept = []
+    for _ in range(3):  # Lens, Lens 2, Lens 3
+        c = component("Lens")
+        assign_unique_name(scene, c)
+        scene.add(c)
+        kept.append(c)
+    scene._items.remove(kept[1])  # drop "Lens 2"
+
+    c = component("Lens")
+    assign_unique_name(scene, c)
+    assert item_label(c) == "Lens 2"
+
+
+# --- batch (paste / duplicate) auto-indexing --------------------------------
+def test_batch_is_unique_against_scene_and_itself():
+    scene = FakeScene([component("Lens")])  # scene already has "Lens"
+    batch = [component("Lens"), component("Lens")]
+    assign_unique_names(scene, batch)
+    assert [item_label(b) for b in batch] == ["Lens 2", "Lens 3"]
+
+
+def test_batch_of_distinct_kinds():
+    scene = FakeScene([component("Lens"), component("Mirror")])
+    batch = [component("Lens"), component("Mirror")]
+    assign_unique_names(scene, batch)
+    assert [item_label(b) for b in batch] == ["Lens 2", "Mirror 2"]
+
+
+# --- autolabels are excluded ------------------------------------------------
+def test_autolabel_not_counted_as_taken():
+    scene = FakeScene([text(owner_uuid="owner-1")])  # autolabel resolves to "Text"
+    note = text()
+    assign_unique_name(scene, note)
+    assert item_label(note) == "Text"  # not bumped to "Text 2"
+
+
+def test_autolabel_in_batch_is_left_untouched():
+    scene = FakeScene()
+    autolabel = text(owner_uuid="owner-2")
+    note = text()
+    assign_unique_names(scene, [autolabel, note])
+    assert autolabel.display_name is None  # never renamed
+    assert item_label(note) == "Text"