[BuildLibrary] fix auto-rename during merge

masque/library.py

@@ -167,19 +167,14 @@ def _plan_source_names(
 
     for name in source_order:
         visible = name
-        renamed = False
         if rename_when == 'always':
             assert rename_theirs is not None
             visible = rename_theirs(target, name)
-            renamed = True
         elif visible in existing_names or visible in visible_names:
             if rename_theirs is None:
                 raise LibraryError(f'Conflicting name while adding source: {name!r}')
             visible = rename_theirs(target, name)
-            renamed = True
         if visible in existing_names or visible in visible_names:
-            if not renamed:
-                raise LibraryError(f'Conflicting name while adding source: {name!r}')
             raise LibraryError(f'Unresolved duplicate key encountered while adding source: {name!r} -> {visible!r}')
         source_to_visible[name] = visible
         visible_names.add(visible)
@@ -1572,10 +1567,9 @@ class BuildLibrary(ILibrary):
     def __init__(self) -> None:
         self.cells = BuildCellsView(self)
         self._frozen = False
-        self._declarations: dict[str, 'Pattern | _BuildRecipe'] = {}
+        self._declarations: dict[str, Pattern | _BuildRecipe] = {}
         self._sources: list[tuple[ILibraryView, dict[str, str]]] = []
-        self._names: set[str] = set()
+        self._names: dict[str, None] = {}
-        self._order: list[str] = []
 
     def _active_session(self) -> '_BuildSessionLibrary | None':
         sessions = _ACTIVE_BUILD_SESSIONS.get()
@@ -1600,7 +1594,7 @@ class BuildLibrary(ILibrary):
         session = self._active_session()
         if session is not None:
             return iter(session)
-        return iter(self._order)
+        return iter(self._names)
 
     def __len__(self) -> int:
         session = self._active_session()
@@ -1620,7 +1614,7 @@ class BuildLibrary(ILibrary):
     def __setitem__(
             self,
             key: str,
-            value: 'Pattern | _BuildRecipe | Callable[[], Pattern]',
+            value: 'Pattern | _BuildRecipe',
             ) -> None:
         session = self._active_session()
         if session is not None:
@@ -1639,8 +1633,7 @@ class BuildLibrary(ILibrary):
             declaration = value
 
         self._declarations[key] = declaration
-        self._names.add(key)
+        self._names[key] = None
-        self._order.append(key)
 
     def __delitem__(self, key: str) -> None:
         session = self._active_session()
@@ -1652,8 +1645,7 @@ class BuildLibrary(ILibrary):
         if key not in self._declarations:
             raise KeyError(key)
         del self._declarations[key]
-        self._names.remove(key)
+        del self._names[key]
-        self._order.remove(key)
 
     def _merge(self, key_self: str, other: Mapping[str, 'Pattern'], key_other: str) -> None:
         session = self._active_session()
@@ -1668,10 +1660,77 @@ class BuildLibrary(ILibrary):
             rename_theirs: Callable[['ILibraryView', str], str] = _rename_patterns,
             mutate_other: bool = False,
             ) -> dict[str, str]:
+        from .pattern import map_targets        # noqa: PLC0415
+
         session = self._active_session()
         if session is not None:
             return session.add(other, rename_theirs=rename_theirs, mutate_other=mutate_other)
-        return super().add(other, rename_theirs=rename_theirs, mutate_other=mutate_other)
+
+        self._assert_editable()
+
+        source_backed = isinstance(other, ILibraryView) and not isinstance(other, Library | LibraryView)
+        if source_backed:
+            if mutate_other:
+                raise BuildError('BuildLibrary.add(..., mutate_other=True) is not supported for source-backed inputs.')
+            return self.add_source(
+                other,
+                rename_theirs = rename_theirs,
+                rename_when = 'conflict',
+                )
+
+        source_order = tuple(other.keys())
+        source_to_visible = _plan_source_names(
+            self,
+            source_order,
+            self._names,
+            rename_theirs = rename_theirs,
+            rename_when = 'conflict',
+            )
+        rename_map = _source_rename_map(source_to_visible)
+
+        if mutate_other:
+            temp = other
+        else:
+            temp = Library(copy.deepcopy(dict(other)))
+
+        for source_name in source_order:
+            visible_name = source_to_visible[source_name]
+            pattern = temp[source_name]
+            if rename_map:
+                pattern.refs = map_targets(
+                    pattern.refs,
+                    lambda target: cast('dict[str | None, str | None]', rename_map).get(target, target),
+                    )
+            self[visible_name] = pattern
+
+        return rename_map
+
+    def __lshift__(self, other: TreeView) -> str:
+        session = self._active_session()
+        if session is not None:
+            return session << other
+
+        self._assert_editable()
+        if len(other) == 1:
+            name = next(iter(other))
+        elif isinstance(other, ILibraryView) and not isinstance(other, Library | LibraryView):
+            source_order = other.source_order()
+            child_graph = other.child_graph(dangling='include')
+            referenced = set().union(*child_graph.values()) if child_graph else set()
+            tops = [candidate for candidate in source_order if candidate not in referenced]
+            if len(tops) != 1:
+                raise LibraryError(f'Asked for the single topcell, but found the following: {pformat(tops)}')
+            name = tops[0]
+        else:
+            return super().__lshift__(other)
+
+        rename_map = self.add(other)
+        return rename_map.get(name, name)
+
+    def __le__(self, other: Mapping[str, 'Pattern']) -> Abstract:
+        if self._active_session() is not None:
+            return super().__le__(other)
+        raise BuildError('BuildLibrary.__le__() is only available while validate() or build() is running.')
 
     def rename(
             self,
@@ -1761,8 +1820,7 @@ class BuildLibrary(ILibrary):
         self._sources.append((view, dict(source_to_visible)))
         for source_name in source_order:
             visible = source_to_visible[source_name]
-            self._names.add(visible)
+            self._names[visible] = None
-            self._order.append(visible)
         return _source_rename_map(source_to_visible)
 
     def validate(
@@ -1849,12 +1907,9 @@ class _BuildSessionLibrary(ILibrary):
 
         self._builder = builder
         self._overlay = OverlayLibrary()
-        self._states: dict[str, Literal['unbuilt', 'building', 'built']] = {
+        self._built: set[str] = set()
-            name: 'unbuilt' for name in builder._declarations
-            }
         self._declared_stack: list[str] = []
-        self._names = set(builder._names)
+        self._names = dict(builder._names)
-        self._order = list(builder._order)
         self._provenance: dict[str, CellProvenance] = {}
         self._dependency_graph: defaultdict[str, set[str]] = defaultdict(set)
         self._install_sources()
@@ -1897,7 +1952,7 @@ class _BuildSessionLibrary(ILibrary):
                     )
 
     def __iter__(self) -> Iterator[str]:
-        return (name for name in self._order if name in self._names)
+        return iter(self._names)
 
     def __len__(self) -> int:
         return len(self._names)
@@ -1905,11 +1960,6 @@ class _BuildSessionLibrary(ILibrary):
     def __contains__(self, key: object) -> bool:
         return key in self._names
 
-    def _touch_name(self, key: str) -> None:
-        if key not in self._names:
-            self._names.add(key)
-            self._order.append(key)
-
     def _current_declared(self) -> str | None:
         if not self._declared_stack:
             return None
@@ -1947,11 +1997,10 @@ class _BuildSessionLibrary(ILibrary):
             raise LibraryError(f'"{new_name}" already exists in the library.')
 
         self._overlay.rename(old_name, new_name, move_references=move_references)
-        self._names.discard(old_name)
+        self._names = {
-        self._names.add(new_name)
+            new_name if name == old_name else name: None
-        if old_name in self._order:
+            for name in self._names
-            idx = self._order.index(old_name)
+            }
-            self._order[idx] = new_name
 
         provenance = self._provenance.pop(old_name)
         self._provenance[new_name] = provenance
@@ -1976,7 +2025,7 @@ class _BuildSessionLibrary(ILibrary):
 
         pattern = value() if callable(value) else value
         self._overlay[key] = pattern
-        self._touch_name(key)
+        self._names.setdefault(key, None)
 
         kind: Literal['declared', 'helper']
         if current is not None and key == current:
@@ -2000,9 +2049,7 @@ class _BuildSessionLibrary(ILibrary):
         self._guard_mutable_output_name(key, operation='delete')
         if key in self._overlay:
             del self._overlay[key]
-        self._names.discard(key)
+        self._names.pop(key, None)
-        if key in self._order:
-            self._order.remove(key)
         self._provenance.pop(key, None)
 
     def _merge(self, key_self: str, other: Mapping[str, 'Pattern'], key_other: str) -> None:
@@ -2046,15 +2093,13 @@ class _BuildSessionLibrary(ILibrary):
     def _ensure_declared(self, name: str) -> None:
         from .pattern import Pattern        # noqa: PLC0415
 
-        state = self._states[name]
+        if name in self._built:
-        if state == 'built':
             return
-        if state == 'building':
+        if name in self._declared_stack:
             chain = ' -> '.join(self._declared_stack + [name])
             raise BuildError(f'Cycle detected while building declared cells: {chain}')
 
         declaration = self._builder._declarations[name]
-        self._states[name] = 'building'
         self._declared_stack.append(name)
         try:
             if isinstance(declaration, _BuildRecipe):
@@ -2073,9 +2118,8 @@ class _BuildSessionLibrary(ILibrary):
                         )
             else:
                 self[name] = pattern
-            self._states[name] = 'built'
+            self._built.add(name)
         except Exception as exc:
-            self._states[name] = 'unbuilt'
             raise self._wrap_error(name, exc) from exc
         finally:
             self._declared_stack.pop()

masque/test/test_build_library.py

@@ -1,19 +1,47 @@
+from collections.abc import Iterator
+
 import pytest
 
 from ..builder import Pather
 from ..error import BuildError
-from ..library import BuildLibrary, Library, cell
+from ..library import BuildLibrary, BuildReport, ILibraryView, Library, cell, dangling_mode_t
 from ..pattern import Pattern
 from ..ports import Port
 
 
-def _owned_by(report, owner: str) -> set[str]:
+def _owned_by(report: BuildReport, owner: str) -> set[str]:
     return {
         name for name, prov in report.provenance.items()
         if prov.owner_declared_name == owner
     }
 
 
+class _MetadataSource(ILibraryView):
+    def __init__(self, mapping: dict[str, Pattern], child_graph: dict[str, set[str]]) -> None:
+        self.mapping = mapping
+        self._child_graph = child_graph
+        self.loads = 0
+
+    def __getitem__(self, key: str) -> Pattern:
+        self.loads += 1
+        return self.mapping[key]
+
+    def __iter__(self) -> Iterator[str]:
+        return iter(self.mapping)
+
+    def __len__(self) -> int:
+        return len(self.mapping)
+
+    def __contains__(self, key: object) -> bool:
+        return key in self.mapping
+
+    def source_order(self) -> tuple[str, ...]:
+        return tuple(self.mapping)
+
+    def child_graph(self, dangling: dangling_mode_t = 'error') -> dict[str, set[str]]:     # noqa: ARG002
+        return self._child_graph
+
+
 def test_build_library_traces_declared_dependencies_out_of_order() -> None:
     builder = BuildLibrary()
 
@@ -59,6 +87,36 @@ def test_build_library_tracks_helper_provenance_and_tree_merge_renames() -> None
     assert any(name != prov.requested_name for name, prov in helpers)
 
 
+def test_build_library_authoring_tree_merge_renames_repeated_single_use_names() -> None:
+    builder = BuildLibrary()
+    tree = Library({"_helper": Pattern()})
+
+    name_a = builder << tree
+    name_b = builder << tree
+    built, report = builder.build()
+
+    assert name_a == "_helper"
+    assert name_b != "_helper"
+    assert name_a in built
+    assert name_b in built
+    assert report.provenance[name_b].requested_name == name_b
+
+
+def test_build_library_authoring_tree_merge_remaps_internal_refs() -> None:
+    builder = BuildLibrary()
+    builder["_helper"] = Pattern()
+    helper = Pattern()
+    top = Pattern()
+    top.ref("_helper")
+
+    top_name = builder << Library({"_helper": helper, "top": top})
+    built, _report = builder.build()
+
+    assert top_name == "top"
+    assert "_helper" not in built[top_name].refs
+    assert any(name != "_helper" for name in built[top_name].refs)
+
+
 def test_build_library_requires_build_session_for_reads_and_freezes_after_build() -> None:
     builder = BuildLibrary()
     builder["leaf"] = Pattern()
@@ -206,6 +264,70 @@ def test_build_library_add_source_can_rename_every_source_cell() -> None:
     assert report.provenance["mapped_child"].requested_name == "child"
 
 
+def test_build_library_authoring_tree_merge_keeps_source_view_lazy() -> None:
+    child = Pattern()
+    top = Pattern()
+    top.ref("child")
+    source = _MetadataSource(
+        {"child": child, "top": top},
+        {"child": set(), "top": {"child"}},
+    )
+
+    builder = BuildLibrary()
+    top_name = builder << source
+    built, _report = builder.build()
+
+    assert top_name == "top"
+    assert "top" in built
+    assert source.loads == 0
+
+
+def test_build_library_authoring_source_tree_merge_returns_renamed_top() -> None:
+    existing = Pattern()
+    source_top = Pattern()
+    source = _MetadataSource(
+        {"_helper": source_top},
+        {"_helper": set()},
+    )
+
+    builder = BuildLibrary()
+    builder["_helper"] = existing
+    top_name = builder << source
+    built, _report = builder.build()
+
+    assert top_name != "_helper"
+    assert top_name in built
+    assert source.loads == 0
+
+
+def test_build_library_authoring_source_tree_merge_remaps_renamed_child_on_materialization() -> None:
+    source_helper = Pattern()
+    source_top = Pattern()
+    source_top.ref("_helper")
+    source = _MetadataSource(
+        {"_helper": source_helper, "top": source_top},
+        {"_helper": set(), "top": {"_helper"}},
+    )
+
+    builder = BuildLibrary()
+    builder["_helper"] = Pattern()
+    top_name = builder << source
+    built, _report = builder.build(output="library")
+
+    assert top_name == "top"
+    assert "_helper" not in built[top_name].refs
+    assert source.loads == 2
+
+
+def test_build_library_rejects_authoring_tree_le_before_mutating() -> None:
+    builder = BuildLibrary()
+
+    with pytest.raises(BuildError, match="__le__"):
+        _abstract = builder <= Library({"leaf": Pattern()})
+
+    assert list(builder) == []
+
+
 def test_build_library_rejects_source_cells_added_after_add_source() -> None:
     source = Library({"src": Pattern()})
     builder = BuildLibrary()