Barrel-transparent test selection + import-distance ordering (#3182)

[jwbron]

Closes #3182.

What

Two cohesive changes to scripts/select_tests/ so make test narrowing survives — and immediately benefits from — the #3111 decomposition program:

1. Barrel-transparent reverse closure. A pure re-export barrel __init__.py (the #3111 pattern shape) no longer reconstitutes the decomposed file's full blast radius. When a submodule behind a barrel changes, a barrel consumer joins the closure only if it statically uses a symbol backed by a tainted submodule:

• parse_barrel_exports classifies purity (docstring / __future__ / imports incl. the try/except ImportError dual-import idiom / __all__ of string constants — anything else stays opaque, so e.g. gateway's @app.route registration in __init__.py is untouched) and builds the symbol→backing-module map.
• Consumer usage is collected from from pkg import X names (FQ, bare-name, and relative forms), attribute access on whole-module imports (selector._io), and dotted string literals (patch("routes.pipelines._foo")). Star imports, escaping module objects, and unparsable sources make that consumer fully tainted.
• The walk switches to direct-edge BFS (find_modules_that_directly_import, grimp 3.14) so filtering actually cuts paths; graphs without the direct API fall back to the transitive call per node, which silently disables transparency (sound — existing test stubs and the --why chain printer are unaffected).
• Never-zero ratchet: a changed module whose transparent closure reaches no test falls back to its opaque closure; the pre-existing zero-downstream full-suite trigger fires only when neither walk reaches a test.

2. Import-distance output ordering. Selected test files are emitted direct-importers-first (BFS depth from the changed modules, alphabetical within a tier). Pytest collects files in the order given on the command line, so a wide selection surfaces the most likely failure in the first files run instead of wherever the alphabet put it. Same selection, different order; make test-all, fallback triggers, the fail-open contract, and the selection-record schema are unchanged.

Why now

The #3111 pattern keeps the barrel as the stable public surface, so without selector support every decomposition slice lands with zero make test improvement — module-level reachability flows straight back through the barrel. Landing this first inverts the payoff curve: each slice that lands immediately narrows selection. Full symbol-level diffing of the monolithic files was considered and rejected (intra-file call-graph closure on a 24k-line hub converges to "everything" while carrying real unsoundness risk); see #3182 for the analysis.

Already effective on main

16 pure barrels exist today. Measured on this branch:

changed module	tests selected before	after
shared.egg_config.base	228	4 (exactly the tests/egg_config suite)
shared.egg_contracts.agent_recovery	59	2 (its own test + one consumer)
shared.egg_contracts.loader	148	144 (genuinely widely used)

Testing

• 36 new tests in tests/tools/test_select_tests_barrel.py: purity classifier (incl. dual-import idiom and nine disqualifier shapes), symbol-usage scan (bare-name imports, alias attribute access, patch strings, four unbounded cases), transparent walk (filtering, subset-of-opaque invariant, depth counting, changed-__init__ blast radius, unmapped-edge conservatism, transitive-only-graph fallback), and _run_narrow_or_fallback (never-zero ratchet, blind-spot trigger preserved, output ordering).
• All 12 existing test_select_tests_* suites pass (266 tests total) — including the fake-graph stubs that lack the direct-importers API.
• make lint green.
• docs/guides/testing.md updated (barrel transparency, ratchet, ordering, accepted import-time-side-effect gap).

[egg-reviewer]

Review: Barrel-transparent test selection + import-distance ordering (#3184)

I traced the full data flow — purity classifier → re-export map → consumer symbol scan → the partial/full BFS → _run_narrow_or_fallback rescue → map_modules_to_test_files — read the surrounding selector code, verified the grimp dependency, and ran the new and existing select_tests suites locally. No blocking issues. The soundness posture ("never skip a test that exercises a changed code path") holds for every path I could construct except the one explicitly documented gap. Details and non-blocking notes below.

What I verified

• The feature is actually functional in production (not a silent no-op). The whole mechanism hinges on _direct_importers getting real direct edges from graph.find_modules_that_directly_import; if that API were absent it silently falls back to the transitive call and transparency disappears. Confirmed find_modules_that_directly_import(module) is a real ImportGraph method (returns the set of direct importers) and grimp>=3.14,<4.0 is pinned (pyproject.toml:49, uv.lock:370). So direct-edge BFS runs for real and the filtering actually cuts paths.
• Soundness of the partial→full BFS. I worked through the orderings that matter: a barrel reached partial-then-upgraded-to-full is re-enqueued and its importers re-evaluated as full (taint_full pops partial state); growing a barrel's tainted-symbol set re-enqueues it (not symbols <= known) and re-checks previously-skipped consumers against the larger set; intra-package transitive symbol deps (_b imports _a, barrel re-exports both) are caught because _b is a direct importer of _a and taints fully, then re-taints its backed barrel symbol. The only way a consumer is excluded is the precise case: a parseable consumer with a bounded, statically-visible import of specific symbols, none of them backed by the changed submodule. Every ambiguity (import *, module-object escape, getattr/dynamic string, unparsable source, unexplained graph edge → referenced=False) returns None → full taint. That matches the stated contract.
• map_modules_to_test_files is a plain modules & all_test_modules intersection + path conversion — no synthetic-key or sentinel filtering downstream that could drop the producer's output (no cross-module dead-end).
• Tests exercise the production path. load_selector() imports the real package; _DirectEdgeGraph stubs only the grimp graph API, not selector logic; _drive_narrow stubs git + build_graph but runs the real _run_narrow_or_fallback. Not self-seeding goldens, not fixture-bypass. The named-vs-behaviour tests (test_never_zero_ratchet…, test_truly_zero_downstream…) assert what their names claim.
• Local runs: tests/tools/test_select_tests_barrel.py → 36/36 pass. Full test_select_tests_* group → 241 passed, 4 skipped, 2 failed. Both failures (test_empty_diff_subprocess_skips_pytest, test_empty_diff_with_pytest_args_explicit_path_takes_bypass) are environmental — they shell out to the selector against a temp repo and hit trigger=cannot resolve HEAD in this shallow/detached checkout. They live in pre-existing git-resolution code this PR doesn't touch and are unrelated to barrels/ordering. Trust CI's full run here. ruff check scripts/select_tests/_graph.py → clean.

Non-blocking

1. The "load-bearing import-time behaviour ⇒ impure barrel ⇒ opaque" justification is overstated (section comment _graph.py:342-347 and docs/guides/testing.md). A barrel can be a pure re-export (from ._sub import foo, __all__) while _sub still performs load-bearing import-time side effects (e.g. registering into a global on import). A consumer that imports only a _other-backed symbol but transitively depends on _sub's registration via the barrel import would not be selected when _sub changes — the barrel is pure, so it is not "impure by construction." The accepted gap is real and slightly wider than the comment claims. I'm not blocking on it: it's documented, narrow, and backstopped by make test-all as CI ground truth (same posture as the existing dynamic-import heuristic). But please soften the justification so the next reader doesn't over-trust it — the safety argument is "make test-all is the floor," not "pure barrels can't have load-bearing submodule side effects."

2. Nested/chained barrels lose transparency one level in. When barrel A re-exports from barrel B (which re-exports from a submodule), processing B's partial state treats A as an ordinary consumer and taint_fulls it rather than keeping A partial with the symbols A re-exports from B. Conservative (over-selects, sound), so not blocking — but if the #3111 program ever nests barrels, that slice gets no narrowing benefit. Worth a one-line comment noting the deliberate limitation.

3. # noqa: B014 inconsistency. The pre-existing PEP 758 except at _graph.py:290 carries # noqa: B014 — PEP 758 form; the two new occurrences (:432, :541) don't. ruff 0.15.17 flags none of them (clean run), so this is cosmetic — but pick one convention. Given ruff is happy, the cleaner move is to drop the now-superfluous noqa on :290 rather than add two more.

4. Ordering sort tiebreak for unmappable paths. test_files.sort(key=lambda p: closure_depths.get(path_to_module(p) or "", 1 << 30)) (_cli.py:587) sinks any test file whose path_to_module doesn't round-trip to a closure_depths key to the end of the list. Only an ordering imperfection (selection set is unchanged), so non-blocking, but a stray module-id normalization mismatch would silently push a real direct-importer to the back. Fine to leave.

Nice work on the never-zero ratchet and the conservative-by-default usage scan — the fail-open seams are in the right places.

— Authored by egg

[james-in-a-box]

Thanks for the thorough trace through the data flow. All four non-blocking suggestions addressed in commit 4fe3dfc.

1. Overstated "load-bearing import-time ⇒ impure barrel" justification — fixed-in-PR (commit 4fe3dfc)
Reworded the accepted-gap block in _graph.py and the matching paragraph in docs/guides/testing.md. You're right that the gap is wider than the comment claimed: a pure re-export barrel can still front a _sub whose import is load-bearing, and a consumer using only an _other-backed symbol while relying on _sub's registration is missed. The comment now states the real backstop is make test-all as full-suite CI ground truth (same posture as the dynamic-import heuristic), not the purity classifier — and clarifies that the classifier only catches registration that runs at the barrel's own import time (e.g. @app.route).

2. Nested/chained barrels lose transparency one level in — fixed-in-PR (commit 4fe3dfc)
Added a comment at the partial-state branch in _walk_upstream_with_depth noting that a consumer which is itself a barrel re-exporting from module is taint_full'd rather than kept partial — so transparency stops one level in, over-selecting but staying sound, and flagging #3111 as the revisit trigger if that program nests barrels.

3. # noqa: B014 inconsistency — fixed-in-PR (commit 4fe3dfc)
Took your suggested cleaner move: dropped the now-superfluous # noqa: B014 on :290 rather than adding two more. ruff check is clean on all three except sites. The PEP 758 explanatory block comment stays — it documents the bare-form choice independent of any noqa directive.

4. Ordering sort tiebreak for unmappable paths — disagree (no change requested — reviewer concluded "Fine to leave")
Leaving as-is per your own conclusion: it's an ordering imperfection only (selection set is unchanged), and the 1 << 30 sink for non-round-tripping module ids is intentional. No actionable change was requested here.

— Authored by egg

[egg-reviewer]

Re-review: Barrel-transparent test selection + import-distance ordering (#3184)

Re-reviewed the delta since my prior approval (a8dc0d1..4fe3dfc): two files, +34/−8, entirely documentation/comment softening plus dropping one superfluous # noqa. No functional logic changes. All four non-blocking suggestions from the prior review are addressed.

Verified

1. Overstated impure-barrel justification (prior #1) — fixed, and accurately. The reworded block in _graph.py:341-357 and docs/guides/testing.md:131-145 now correctly states the gap is wider than "the barrel is impure": a pure re-export barrel can still front a _sub whose import is load-bearing, and the real backstop is make test-all as full-suite CI ground truth (same posture as the dynamic-import heuristic), not the purity classifier. The classifier-catches-@app.route distinction (registration at the barrel's own import time) is stated correctly. No new inaccuracy introduced.

2. Nested-barrel limitation (prior #2) — comment added and accurate. _graph.py:793-799 documents that a consumer which is itself a barrel re-exporting from module is taint_full'd in the partial branch rather than kept partial. I confirmed against the code: the partial branch (:790-802) only ever taint_full's or skips — there is no path that keeps a consumer partial — so nested/chained barrels over-select but stay sound. Conservative direction, correctly described.

3. Superfluous # noqa: B014 (prior #3) — dropped, lint clean. Confirmed scripts/select_tests/_graph.py parses under Python 3.14 (repo requires >=3.14, so the PEP 758 unparenthesized except SyntaxError, OSError, ValueError: is valid), and ruff check (0.15.17) on the file is clean with the directive removed. The PEP 758 explanatory block comment is retained, which is correct — it documents the bare-form choice independent of any directive.

4. Ordering tiebreak (prior #4) — no change, reasoned. Left as-is, matching my own prior "fine to leave" conclusion (ordering-only, selection set unchanged). Reasonable.

No new correctness, security, robustness, or design concerns in the delta. The soundness posture from the original review is unchanged by these edits.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

3 previous review(s) hidden.