Skip to content

Commit b75eb1d

Browse files
tadani3Tommaso Adanigithub-code-quality[bot]
authored
CI: Narrow affected-models detection by resolving __init__.py re-exports (#182)
## CI: Narrow affected-models detection by resolving __init__.py re-exports ### Changes 1. **`src/mobius/tasks/` reclassified from `shared_infra` → `traceable`.** Task files no longer trigger `run_all` unconditionally; they are now traced through the import graph. 2. **Re-export resolution for `__init__.py` hubs.** A new `_build_reexport_map()` walks every `__init__.py` and builds a `(package, symbol) → source_module` map. `_parse_imports()` resolves `from mobius.components import Foo` to the actual source module (`mobius.components._attention`) instead of recording a dependency on the package itself. The package name is only added when a symbol can't be resolved (wildcards, missing entries). The result: changes to a re-export hub (e.g. `components/__init__.py`) no longer invalidate every importer — only models whose actually-used symbol's source changed are affected. ### Before vs. after (Gemma4 + Qwen test scenario) Same input file list: | Detector | Affected models | `run_all` | |----------------|-----------------|-----------| | Before | **193** | `false` | | After | **2** (`gemma4`, `gemma4_text`) | `false` | ### Verification ```bash # Gemma4 + Qwen test scenario cat <<'EOF' | python scripts/detect_affected_models.py --stdin src/mobius/components/__init__.py src/mobius/components/_gemma4_audio.py src/mobius/models/gemma4.py EOF # → {"affected": ["gemma4", "gemma4_text"], "run_all": false} # Attention component → all causal-LMs / VLMs that use shared Attention echo "src/mobius/components/_attention.py" | python scripts/detect_affected_models.py --stdin # → 154 models (vision-only / audio-only models correctly excluded) # MoE component → only MoE architectures echo "src/mobius/components/_moe.py" | python scripts/detect_affected_models.py --stdin # → 33 models (Mixtral, DeepSeek V2/V3, Qwen MoE, Granite MoE, Jamba, ...) ``` All 49 tests in `scripts/detect_affected_models_test.py` pass. ### Caveat > Tasks aren't directly imported by model files, so a task change currently > produces an empty affected set (no `run_all`). If you want task changes to > trigger specific models, a task → model_type mapping would need to be added. --------- Signed-off-by: Tommaso Adani <83273681+tadani3@users.noreply.github.com> Co-authored-by: Tommaso Adani <tommasoadani@microsoft.com> Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
1 parent 8be8155 commit b75eb1d

2 files changed

Lines changed: 251 additions & 13 deletions

File tree

scripts/detect_affected_models.py

Lines changed: 88 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -48,15 +48,15 @@
4848
"src/mobius/models/__init__.py",
4949
)
5050

51-
# Task files are resolved by string-based lookup at runtime (not Python
52-
# imports), so the import graph cannot trace task → model dependencies.
53-
# Keep tasks/ as shared_infra until a task→model_type mapping exists.
54-
_SHARED_INFRA_PREFIXES = ("src/mobius/tasks/",)
51+
_SHARED_INFRA_PREFIXES: tuple[str, ...] = ()
5552

5653
# Traceable infrastructure: component files that are analyzed via the
5754
# import graph to find which models they actually affect, rather than
5855
# triggering run_all unconditionally.
59-
_TRACEABLE_PREFIXES = ("src/mobius/components/",)
56+
_TRACEABLE_PREFIXES = (
57+
"src/mobius/components/",
58+
"src/mobius/tasks/",
59+
)
6060

6161

6262
def classify_file(path: str) -> str:
@@ -108,12 +108,24 @@ def classify_file(path: str) -> str:
108108
# ----------------------------------------------------------------
109109

110110

111-
def _parse_imports(filepath: Path) -> set[str]:
111+
def _parse_imports(
112+
filepath: Path,
113+
reexport_map: dict[tuple[str, str], str] | None = None,
114+
) -> set[str]:
112115
"""Extract imported module names from a Python file using AST.
113116
114117
Returns a set of dotted module names that appear in import
115118
statements. Only collects imports from within the
116119
mobius package.
120+
121+
When ``reexport_map`` is provided, ``from pkg import sym`` statements
122+
are resolved through the re-export map to the underlying source
123+
module that defines ``sym``. This avoids spurious dependencies on
124+
re-export hubs like ``mobius.components/__init__.py``: a model that
125+
imports ``Attention`` from ``mobius.components`` is recorded as
126+
depending on ``mobius.components._attention`` (the actual source),
127+
not on the package itself. Symbols not found in the re-export map
128+
fall back to recording the package name.
117129
"""
118130
try:
119131
source = filepath.read_text(encoding="utf-8")
@@ -129,10 +141,77 @@ def _parse_imports(filepath: Path) -> set[str]:
129141
imports.add(alias.name)
130142
elif isinstance(node, ast.ImportFrom):
131143
if node.module and node.module.startswith("mobius"):
132-
imports.add(node.module)
144+
unresolved = False
145+
for alias in node.names:
146+
if alias.name == "*":
147+
# Wildcard imports can't be resolved — fall back
148+
# to depending on the package itself.
149+
unresolved = True
150+
continue
151+
source_mod = (
152+
reexport_map.get((node.module, alias.name))
153+
if reexport_map is not None
154+
else None
155+
)
156+
if source_mod:
157+
imports.add(source_mod)
158+
else:
159+
unresolved = True
160+
# Only record the package itself when at least one
161+
# imported symbol could not be resolved through the
162+
# re-export map. This avoids spurious dependencies on
163+
# re-export hubs like ``mobius.components/__init__.py``.
164+
if unresolved:
165+
imports.add(node.module)
133166
return imports
134167

135168

169+
def _build_reexport_map(search_dir: Path) -> dict[tuple[str, str], str]:
170+
"""Build a (package, symbol) → source_module map from ``__init__.py`` files.
171+
172+
Parses each ``__init__.py`` in the source tree for ``from .submodule
173+
import Symbol`` and ``from mobius.pkg.submodule import Symbol``
174+
statements. The resulting map lets us resolve re-exported symbols
175+
back to their defining module so changes to a re-export hub don't
176+
spuriously invalidate every importer.
177+
"""
178+
reexport: dict[tuple[str, str], str] = {}
179+
for init_file in search_dir.rglob("__init__.py"):
180+
package = _module_name_from_path(init_file)
181+
if not package:
182+
continue
183+
try:
184+
source = init_file.read_text(encoding="utf-8")
185+
tree = ast.parse(source, filename=str(init_file))
186+
except (SyntaxError, UnicodeDecodeError):
187+
continue
188+
for node in ast.walk(tree):
189+
if not isinstance(node, ast.ImportFrom):
190+
continue
191+
# Resolve relative imports like ``from . import x`` or
192+
# ``from .sub import X`` against the current package.
193+
if node.level:
194+
base_parts = package.split(".") if package else []
195+
# ``from .`` keeps us at the same package; ``from ..`` goes up.
196+
if node.level - 1 > len(base_parts):
197+
continue
198+
base = ".".join(base_parts[: len(base_parts) - (node.level - 1)])
199+
if node.module:
200+
src_module = f"{base}.{node.module}" if base else node.module
201+
else:
202+
src_module = base
203+
else:
204+
src_module = node.module or ""
205+
if not src_module.startswith("mobius"):
206+
continue
207+
for alias in node.names:
208+
if alias.name == "*":
209+
continue
210+
exported_name = alias.asname or alias.name
211+
reexport[(package, exported_name)] = src_module
212+
return reexport
213+
214+
136215
def _module_name_from_path(filepath: Path) -> str | None:
137216
"""Convert a file path to a dotted module name.
138217
@@ -159,12 +238,13 @@ def _build_import_graph(
159238
modules it directly imports.
160239
"""
161240
graph: dict[str, set[str]] = {}
241+
reexport_map = _build_reexport_map(search_dir)
162242
for pyfile in search_dir.rglob("*.py"):
163243
if pyfile.name.endswith("_test.py"):
164244
continue
165245
mod_name = _module_name_from_path(pyfile)
166246
if mod_name:
167-
graph[mod_name] = _parse_imports(pyfile)
247+
graph[mod_name] = _parse_imports(pyfile, reexport_map)
168248
return graph
169249

170250

scripts/detect_affected_models_test.py

Lines changed: 163 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,11 @@
2222
_SRC_ROOT,
2323
_build_class_to_source_module,
2424
_build_import_graph,
25+
_build_reexport_map,
2526
_build_registry_class_to_types,
2627
_build_source_module_to_types,
2728
_find_reverse_dependents,
29+
_parse_imports,
2830
classify_file,
2931
detect_affected_models,
3032
)
@@ -46,7 +48,7 @@ def test_component_file(self):
4648
assert classify_file("src/mobius/components/_attention.py") == "traceable"
4749

4850
def test_task_file(self):
49-
assert classify_file("src/mobius/tasks/_causal_lm.py") == "shared_infra"
51+
assert classify_file("src/mobius/tasks/_causal_lm.py") == "traceable"
5052

5153
def test_configs_file(self):
5254
assert classify_file("src/mobius/_configs.py") == "shared_infra"
@@ -181,10 +183,16 @@ def test_component_change_traces_affected_models(self):
181183
# _attention.py is imported by many models — should find affected types
182184
assert len(result["affected"]) > 0
183185

184-
def test_task_change_triggers_run_all(self):
185-
"""Task files use string-based lookup, not imports — must trigger run_all."""
186+
def test_task_change_does_not_trigger_run_all(self):
187+
"""Task files are traceable but produce an empty affected set.
188+
189+
No model imports ``mobius.tasks`` directly (tasks are looked up at
190+
runtime by string keys), so tracing through the import graph finds
191+
no dependents. Documented limitation — see PR description.
192+
"""
186193
result = detect_affected_models(["src/mobius/tasks/_causal_lm.py"])
187-
assert result["run_all"] is True
194+
assert result["run_all"] is False
195+
assert result["affected"] == []
188196

189197
def test_configs_change_triggers_run_all(self):
190198
result = detect_affected_models(["src/mobius/_configs.py"])
@@ -354,10 +362,160 @@ def test_traceable_result_is_subset_of_all_models(self):
354362

355363

356364
# ----------------------------------------------------------------
357-
# CLI tests
365+
# Re-export resolution tests
366+
#
367+
# These tests use synthetic source trees in a temp directory so they
368+
# are isolated from the real mobius package layout.
358369
# ----------------------------------------------------------------
359370

360371

372+
class TestReexportResolution:
373+
"""Tests for _parse_imports + _build_reexport_map.
374+
375+
The resolver must record dependencies on the *source* module that
376+
actually defines a symbol, not on re-export hubs like
377+
``components/__init__.py``. Wildcard and unknown symbols fall back
378+
to depending on the hub package.
379+
"""
380+
381+
@staticmethod
382+
def _write_pkg(tmp_path: Path, monkeypatch) -> Path:
383+
"""Create a synthetic ``src/mobius`` tree.
384+
385+
Layout::
386+
387+
src/mobius/__init__.py
388+
src/mobius/components/__init__.py # re-exports Attention, MLP
389+
src/mobius/components/_attention.py # defines Attention
390+
src/mobius/components/_mlp.py # defines MLP
391+
"""
392+
src = tmp_path / "src" / "mobius"
393+
(src / "components").mkdir(parents=True)
394+
(src / "__init__.py").write_text("")
395+
(src / "components" / "__init__.py").write_text(
396+
"from ._attention import Attention\nfrom ._mlp import MLP\n"
397+
)
398+
(src / "components" / "_attention.py").write_text("class Attention: ...\n")
399+
(src / "components" / "_mlp.py").write_text("class MLP: ...\n")
400+
# _module_name_from_path uses _PROJECT_ROOT to resolve dotted names;
401+
# point it at our synthetic tree for the duration of the test.
402+
monkeypatch.setattr("detect_affected_models._PROJECT_ROOT", tmp_path)
403+
return src
404+
405+
def test_resolves_symbol_to_source_module(self, tmp_path: Path, monkeypatch) -> None:
406+
"""``from mobius.components import Attention`` → depends on _attention."""
407+
src = self._write_pkg(tmp_path, monkeypatch)
408+
importer = tmp_path / "importer.py"
409+
importer.write_text("from mobius.components import Attention\n")
410+
411+
reexport = _build_reexport_map(src)
412+
imports = _parse_imports(importer, reexport)
413+
414+
assert "mobius.components._attention" in imports
415+
# The hub package itself is NOT recorded when every symbol resolves.
416+
assert "mobius.components" not in imports
417+
418+
def test_resolves_multiple_symbols_from_same_hub(
419+
self, tmp_path: Path, monkeypatch
420+
) -> None:
421+
"""Each symbol in a multi-import resolves to its own source module."""
422+
src = self._write_pkg(tmp_path, monkeypatch)
423+
importer = tmp_path / "importer.py"
424+
importer.write_text("from mobius.components import Attention, MLP\n")
425+
426+
reexport = _build_reexport_map(src)
427+
imports = _parse_imports(importer, reexport)
428+
429+
assert "mobius.components._attention" in imports
430+
assert "mobius.components._mlp" in imports
431+
assert "mobius.components" not in imports
432+
433+
def test_unknown_symbol_falls_back_to_package(self, tmp_path: Path, monkeypatch) -> None:
434+
"""Symbols not in the re-export map fall back to the hub package."""
435+
src = self._write_pkg(tmp_path, monkeypatch)
436+
importer = tmp_path / "importer.py"
437+
importer.write_text("from mobius.components import NotExported\n")
438+
439+
reexport = _build_reexport_map(src)
440+
imports = _parse_imports(importer, reexport)
441+
442+
# Unknown symbol → conservative fallback on the package itself
443+
assert "mobius.components" in imports
444+
# And no spurious source-module resolution
445+
assert "mobius.components._attention" not in imports
446+
447+
def test_wildcard_import_falls_back_to_package(self, tmp_path: Path, monkeypatch) -> None:
448+
"""``from mobius.components import *`` cannot be resolved — fall back."""
449+
src = self._write_pkg(tmp_path, monkeypatch)
450+
importer = tmp_path / "importer.py"
451+
importer.write_text("from mobius.components import *\n")
452+
453+
reexport = _build_reexport_map(src)
454+
imports = _parse_imports(importer, reexport)
455+
456+
assert "mobius.components" in imports
457+
458+
def test_mixed_resolved_and_unresolved_records_both(
459+
self, tmp_path: Path, monkeypatch
460+
) -> None:
461+
"""Mix of known + unknown symbols records resolved sources AND the hub."""
462+
src = self._write_pkg(tmp_path, monkeypatch)
463+
importer = tmp_path / "importer.py"
464+
importer.write_text("from mobius.components import Attention, NotExported\n")
465+
466+
reexport = _build_reexport_map(src)
467+
imports = _parse_imports(importer, reexport)
468+
469+
assert "mobius.components._attention" in imports
470+
# Unresolved symbol keeps the hub as a conservative dependency
471+
assert "mobius.components" in imports
472+
473+
def test_plain_import_statement_records_module(self, tmp_path: Path, monkeypatch) -> None:
474+
"""``import mobius.components`` (no ``from``) records the module directly."""
475+
src = self._write_pkg(tmp_path, monkeypatch)
476+
importer = tmp_path / "importer.py"
477+
importer.write_text("import mobius.components\n")
478+
479+
reexport = _build_reexport_map(src)
480+
imports = _parse_imports(importer, reexport)
481+
482+
assert "mobius.components" in imports
483+
484+
def test_no_reexport_map_records_module(self, tmp_path: Path, monkeypatch) -> None:
485+
"""When no map is passed, the hub is always recorded (legacy behavior)."""
486+
self._write_pkg(tmp_path, monkeypatch)
487+
importer = tmp_path / "importer.py"
488+
importer.write_text("from mobius.components import Attention\n")
489+
490+
imports = _parse_imports(importer, reexport_map=None)
491+
492+
assert "mobius.components" in imports
493+
assert "mobius.components._attention" not in imports
494+
495+
def test_non_mobius_imports_ignored(self, tmp_path: Path, monkeypatch) -> None:
496+
"""Imports outside the mobius package are not recorded."""
497+
src = self._write_pkg(tmp_path, monkeypatch)
498+
importer = tmp_path / "importer.py"
499+
importer.write_text(
500+
"import os\nfrom typing import Any\nfrom mobius.components import Attention\n"
501+
)
502+
503+
reexport = _build_reexport_map(src)
504+
imports = _parse_imports(importer, reexport)
505+
506+
assert imports == {"mobius.components._attention"}
507+
508+
def test_reexport_map_built_from_relative_import(
509+
self, tmp_path: Path, monkeypatch
510+
) -> None:
511+
"""``from .sub import X`` in __init__.py produces correct (pkg, X) entry."""
512+
src = self._write_pkg(tmp_path, monkeypatch)
513+
reexport = _build_reexport_map(src)
514+
515+
assert reexport[("mobius.components", "Attention")] == ("mobius.components._attention")
516+
assert reexport[("mobius.components", "MLP")] == "mobius.components._mlp"
517+
518+
361519
class TestCLI:
362520
def test_json_output(self):
363521
result = subprocess.run(

0 commit comments

Comments
 (0)