Skip to content

Commit e664104

Browse files
Zain Dana Harperclaude
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 977b7f0 commit e664104

3 files changed

Lines changed: 59 additions & 0 deletions

File tree

CHANGELOG.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ latest
1010
* Tweak Just commands for running version-specific Python tests.
1111
* Remove `typing-extensions` as a dependency.
1212
* Make package FIPS compatible by marking blake2 hashing with `usedforsecurity=False`.
13+
* Raise a descriptive `ValueError` instead of an unhelpful panic when the same module is assigned to
14+
more than one layer in `find_illegal_dependencies_for_layers`.
15+
(https://github.com/python-grimp/grimp/issues/165)
1316

1417
3.14 (2025-12-10)
1518
-----------------

src/grimp/application/graph.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,9 +433,11 @@ def find_illegal_dependencies_for_layers(
433433
lower layer (the 'upstream') to the higher layer (the 'downstream').
434434
435435
Raises:
436+
ValueError: if the same module is assigned to more than one layer.
436437
NoSuchContainer: if the container is not a module in the graph.
437438
"""
438439
layers = _parse_layers(layers)
440+
_check_layers_do_not_share_modules(layers)
439441
try:
440442
result = self._rustgraph.find_illegal_dependencies_for_layers(
441443
layers=tuple(
@@ -518,6 +520,27 @@ def _parse_layers(layers: Sequence[Layer | str | set[str]]) -> tuple[Layer, ...]
518520
return tuple(out_layers)
519521

520522

523+
def _check_layers_do_not_share_modules(layers: tuple[Layer, ...]) -> None:
524+
"""
525+
Raise ValueError if any module is assigned to more than one layer.
526+
527+
A module cannot sit at two different levels at once. Without this guard a
528+
repeated module surfaces as an unhelpful panic from the underlying Rust
529+
extension rather than a clear error.
530+
"""
531+
seen: set[str] = set()
532+
duplicates: set[str] = set()
533+
for layer in layers:
534+
for module in layer.module_tails:
535+
if module in seen:
536+
duplicates.add(module)
537+
seen.add(module)
538+
if duplicates:
539+
formatted = ", ".join(repr(module) for module in sorted(duplicates))
540+
msg = f"Modules can only belong to one layer, but the following appear in more than one: {formatted}."
541+
raise ValueError(msg)
542+
543+
521544
def _dependencies_from_tuple(
522545
rust_package_dependency_tuple: tuple[_RustPackageDependency, ...],
523546
) -> set[PackageDependency]:

tests/unit/application/graph/test_layers.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -840,6 +840,39 @@ 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+
843876
# TODO: move test to within Rust.
844877
@pytest.mark.skip(reason="This only passes if run on its own, due to pyo3_log caching.")
845878
class TestLogging:

0 commit comments

Comments
 (0)