Skip to content

Commit d03c595

Browse files
Zain Dana Harperclaude
authored andcommitted
Raise ValueError when a module is assigned to more than one layer
find_illegal_dependencies_for_layers panicked with an opaque pyo3_runtime.PanicException ("no entry found for key") when the same module appeared in more than one layer. Add a Python-side guard that raises a descriptive ValueError naming the repeated module(s) before the call reaches the Rust extension, and cover it with tests. Fixes #165. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 4bb38dd commit d03c595

3 files changed

Lines changed: 84 additions & 0 deletions

File tree

CHANGELOG.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ latest
1313
* Raise `ModuleNotPresent` instead of an unclear error when `find_modules_that_directly_import` or
1414
`find_modules_directly_imported_by` is called with a module that is not in the graph.
1515
(https://github.com/python-grimp/grimp/issues/113)
16+
* Raise `ValueError` instead of pyo3_runtime.PanicException when a duplicate layer
17+
module is passed to `find_illegal_dependencies_for_layers`.
1618

1719
3.14 (2025-12-10)
1820
-----------------

src/grimp/application/graph.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,9 +446,11 @@ def find_illegal_dependencies_for_layers(
446446
lower layer (the 'upstream') to the higher layer (the 'downstream').
447447
448448
Raises:
449+
ValueError: if the same module is assigned to more than one layer.
449450
NoSuchContainer: if the container is not a module in the graph.
450451
"""
451452
layers = _parse_layers(layers)
453+
_check_layers_do_not_share_modules(layers)
452454
try:
453455
result = self._rustgraph.find_illegal_dependencies_for_layers(
454456
layers=tuple(
@@ -531,6 +533,30 @@ def _parse_layers(layers: Sequence[Layer | str | set[str]]) -> tuple[Layer, ...]
531533
return tuple(out_layers)
532534

533535

536+
def _check_layers_do_not_share_modules(layers: tuple[Layer, ...]) -> None:
537+
"""
538+
Raise ValueError if any module is assigned to more than one layer.
539+
540+
A module cannot sit at two different levels at once. Without this guard a
541+
repeated module surfaces as an unhelpful panic from the underlying Rust
542+
extension rather than a clear error.
543+
"""
544+
seen = set()
545+
duplicates = set()
546+
547+
flattened_modules = [module for layer in layers for module in layer.module_tails]
548+
for module in flattened_modules:
549+
if module in seen:
550+
duplicates.add(module)
551+
else:
552+
seen.add(module)
553+
554+
if duplicates:
555+
formatted = ", ".join(repr(module) for module in sorted(duplicates))
556+
msg = f"Modules can only belong to one layer, but the following appear in more than one: {formatted}."
557+
raise ValueError(msg)
558+
559+
534560
def _dependencies_from_tuple(
535561
rust_package_dependency_tuple: tuple[_RustPackageDependency, ...],
536562
) -> set[PackageDependency]:

tests/unit/application/graph/test_layers.py

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -840,6 +840,62 @@ def test_single_missing_container(self, missing_container: str):
840840
)
841841

842842

843+
class TestInvalidLayers:
844+
def test_module_in_multiple_layers_raises_value_error(self):
845+
graph = ImportGraph()
846+
graph.add_module("mypackage")
847+
graph.add_module("mypackage.foo")
848+
graph.add_module("mypackage.bar")
849+
850+
with pytest.raises(
851+
ValueError,
852+
match=re.escape(
853+
"Modules can only belong to one layer, but the following appear in "
854+
"more than one: 'mypackage.foo'."
855+
),
856+
):
857+
graph.find_illegal_dependencies_for_layers(
858+
layers=("mypackage.foo", "mypackage.bar", "mypackage.foo"),
859+
)
860+
861+
def test_module_shared_across_sibling_layers_raises_value_error(self):
862+
graph = ImportGraph()
863+
864+
with pytest.raises(
865+
ValueError,
866+
match=re.escape(
867+
"Modules can only belong to one layer, but the following appear in "
868+
"more than one: 'shared'."
869+
),
870+
):
871+
graph.find_illegal_dependencies_for_layers(
872+
layers=(Layer("high", "shared"), Layer("shared", "low")),
873+
)
874+
875+
def test_multiple_duplicate_modules_raises_value_error(self):
876+
graph = ImportGraph()
877+
878+
with pytest.raises(
879+
ValueError,
880+
match=re.escape(
881+
"Modules can only belong to one layer, but the following appear in "
882+
"more than one: 'blue', 'red', 'yellow'."
883+
),
884+
):
885+
graph.find_illegal_dependencies_for_layers(
886+
layers=(
887+
Layer("one", "blue"),
888+
Layer("blue", "two", "three", "yellow"),
889+
# Note that green doesn't cause an error as these are stored as a set.
890+
Layer("green", "four", "green"),
891+
Layer("five"),
892+
Layer("red"),
893+
Layer("yellow"),
894+
Layer("red"),
895+
),
896+
)
897+
898+
843899
# TODO: move test to within Rust.
844900
@pytest.mark.skip(reason="This only passes if run on its own, due to pyo3_log caching.")
845901
class TestLogging:

0 commit comments

Comments
 (0)