From eb81101672368bae6b9d16da8c09772d9f84c83c Mon Sep 17 00:00:00 2001 From: Jan Petykiewicz Date: Mon, 14 Sep 2026 21:57:07 -0700 Subject: [PATCH] [OverlayLibrary] allow rename into a name we previously renamed out of --- masque/library/overlay.py | 33 ++++++++++----------------------- masque/test/test_library.py | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 23 deletions(-) diff --git a/masque/library/overlay.py b/masque/library/overlay.py index 529684b..43e201e 100644 --- a/masque/library/overlay.py +++ b/masque/library/overlay.py @@ -273,7 +273,6 @@ class OverlayLibrary(ILibrary, IMaterializable, IBorrowing): self._layers: list[_SourceLayer] = [] self._entries: dict[str, Pattern | _SourceEntry] = {} self._order: list[str] = [] - self._target_remap: dict[str, str] = {} def __iter__(self) -> Iterator[str]: return (name for name in self._order if name in self._entries) @@ -345,6 +344,10 @@ class OverlayLibrary(ILibrary, IMaterializable, IBorrowing): source_target_map=dict(source_to_visible), child_graph=child_graph, ) + # Include dangling targets so each source tracks current names directly. + for children in child_graph.values(): + for child in children: + layer.source_target_map.setdefault(child, child) layer_index = len(self._layers) self._layers.append(layer) @@ -371,6 +374,7 @@ class OverlayLibrary(ILibrary, IMaterializable, IBorrowing): entry = self._entries.pop(old_name) self._entries[new_name] = entry + self._order = [name for name in self._order if name != new_name] idx = self._order.index(old_name) self._order[idx] = new_name @@ -378,28 +382,13 @@ class OverlayLibrary(ILibrary, IMaterializable, IBorrowing): self.move_references(old_name, new_name) return self - def _resolve_target(self, target: str) -> str: - seen: set[str] = set() - current = target - while current in self._target_remap: - if current in seen: - raise LibraryError(f'Cycle encountered while resolving target remap for {target!r}') - seen.add(current) - current = self._target_remap[current] - return current - - def _set_target_remap(self, old_target: str, new_target: str) -> None: - resolved_new = self._resolve_target(new_target) - if resolved_new == old_target: - raise LibraryError(f'Ref target remap would create a cycle: {old_target!r} -> {new_target!r}') - self._target_remap[old_target] = resolved_new - for key in list(self._target_remap): - self._target_remap[key] = self._resolve_target(self._target_remap[key]) - def move_references(self, old_target: str, new_target: str) -> OverlayLibrary: if old_target == new_target: return self - self._set_target_remap(old_target, new_target) + for layer in self._layers: + for source_target, current_target in layer.source_target_map.items(): + if current_target == old_target: + layer.source_target_map[source_target] = new_target for entry in list(self._entries.values()): if isinstance(entry, Pattern) and old_target in entry.refs: entry.refs[new_target].extend(entry.refs[old_target]) @@ -407,8 +396,7 @@ class OverlayLibrary(ILibrary, IMaterializable, IBorrowing): return self def _effective_target(self, layer: _SourceLayer, target: str) -> str: - visible = layer.source_target_map.get(target, target) - return self._resolve_target(visible) + return layer.source_target_map.get(target, target) def _remap_source_pattern(self, layer: _SourceLayer, source_pat: Pattern) -> Pattern: def remap(target: str | None) -> str | None: @@ -528,7 +516,6 @@ class OverlayLibrary(ILibrary, IMaterializable, IBorrowing): ] new._order = [name for name in self._order if name in keep and name in self._entries] new._entries = {name: self._entries[name] for name in new._order} - new._target_remap = dict(self._target_remap) return new def find_refs_local( diff --git a/masque/test/test_library.py b/masque/test/test_library.py index adc6f3c..323bcf2 100644 --- a/masque/test/test_library.py +++ b/masque/test/test_library.py @@ -47,6 +47,42 @@ def test_writable_libraries_are_restricted_mappings( assert not lib +@pytest.mark.parametrize('cached', [False, True]) +def test_overlay_reuses_names_and_keeps_new_sources_independent(cached: bool) -> None: + source = Library({'a': Pattern(), 'parent': Pattern().ref('a')}) + overlay = OverlayLibrary() + overlay.add_source(source) + if cached: + overlay['parent'] + overlay.rename('a', 'b', move_references=True) + overlay.rename('b', 'a', move_references=True) + assert overlay.child_graph()['parent'] == {'a'} + assert set(overlay.materialize('parent', persist=False).refs) == {'a'} + overlay.rename('a', 'b', move_references=True) + overlay.add_source(Library({'a': Pattern(), 'new_parent': Pattern().ref('a')})) + assert overlay.child_graph()['parent'] == {'b'} + assert overlay.child_graph()['new_parent'] == {'a'} + assert set(overlay['new_parent'].refs) == {'a'} + assert set(source['parent'].refs) == {'a'} + + +def test_overlay_reuses_dangling_targets_and_preserves_failed_rename() -> None: + overlay = OverlayLibrary() + overlay.add_source(Library({'parent': Pattern().ref('missing')})) + overlay.move_references('missing', 'other') + overlay.move_references('other', 'missing') + assert set(overlay['parent'].refs) == {'missing'} + overlay['used'] = Pattern() + before = list(overlay) + with pytest.raises(LibraryError, match='already exists'): + overlay.rename('parent', 'used', move_references=True) + assert list(overlay) == before + assert set(overlay['parent'].refs) == {'missing'} + del overlay['used'] + overlay.rename('parent', 'used') + assert list(overlay) == ['used'] + + def test_library_tops() -> None: lib = Library() lib["child"] = Pattern()