fix(scripts): enforce relative-import scope in check_layer_imports

[ethan-scitix]

Type

• fix — bug fix or alignment correction

Summary

• CLAUDE.md ## Import Policy and CONTRIBUTING.md:59 both state "same package: relative imports; cross-package: absolute imports", but no enforcer covered the second half — a from ..parent import X passed every check. This adds _check_relative_scope: a relative import with level >= 2 escapes its own package, so it is a cross-package import written relatively. Flagged, with the resolved absolute module offered as the fix.
• Closes a hole in the private-access check: its carve-out comment claimed relative imports are "same-package by construction" but was implemented as level > 0, so a from .._x import _foo slipped past both the private-name and protected-module rules.
• Relative imports are now resolved to absolute before those two rules run. Without that, node.module is the bare tail ("ir") and every rule short-circuits on the sieval. prefix test — i.e. narrowing the carve-out alone would have been dead code.

Found while reviewing #45, whose sieval/core/models/transports/*.py are currently the only from .. imports in the tree. This PR is deliberately independent of that review: it lands the enforcement on main, which is clean today.

Second commit — two chained holes in the layer check

Self-review surfaced that check 1 never got the resolve-to-absolute treatment that check 2 did, and that the omission chained into a second, older hole:

• _check_layer_imports matched on node.module alone. For a relative import that is only the bare tail ("tasks" for from ...tasks import x), so a cross-layer import written relatively short-circuited on the sieval. prefix test and was reported solely as an import-style error — whose suggested fix is itself a layer violation.
• from sieval import tasks was not reported at all (exit 0, pre-existing). It names the layer as an imported alias rather than in the module path, so the len(parts) >= 2 test never fired.
• The two chained: _check_relative_scope offers from sieval import ... as the fix for from ... import tasks, steering authors toward the one shape the layer check could not see. Removing the and node.module guard also made that shape newly reachable via relative syntax, so the alias branch is load-bearing for this change rather than adjacent cleanup.

Implementation: _absolute_module is now the single relative→absolute normalization that both _check_layer_imports and _check_private_access read through — it replaced the latter's inline branch, so the contract lives in one place instead of two sites that must agree.

Third commit — the narrowed carve-out was still wider than same-package

Code review of the second commit caught that level > 0 → level == 1 did not go far enough. level == 1 is same-package only when the module is undotted; a dotted level-1 module walks DOWN into a child subpackage:

sieval/tasks/foo.py:  from .sub._hidden import X             -> NO ERRORS      (before)
sieval/tasks/foo.py:  from sieval.tasks.sub._hidden import X -> flagged, out-of-subtree

from .sub._hidden import X resolves to sieval.tasks.sub._hidden, owned by subtree sieval.tasks.sub. The importer at sieval.tasks is an ancestor, not a descendant, so per CLAUDE.md that access is out-of-subtree and forbidden — yet it escaped while the identical absolute spelling was flagged. Same semantic import, two verdicts depending only on how it was written: the same class of hole as the first two commits, mirrored (downward escape instead of upward).

Predicate is now node.level == 1 and "." not in (node.module or "").

Residual limit, recorded in a comment + test rather than left as an apparent oversight: from .sub import _priv is cross-package when sub is a package, but is syntactically identical to a sibling module (from .mod import _priv). Separating them needs a filesystem lookup, which would make a lint verdict depend on checkout completeness. Left exempt on purpose — the dotted form is where a private module segment can actually appear mid-path.

Rule 3 deliberately stays quiet on level-1 dotted descent: it reads as a local descent and is idiomatic enough that banning it buys little, and checks 1 and 2 resolve through it either way, so layer boundaries and private-module protection are unaffected. Now stated in the _check_relative_scope docstring instead of left for the next reader to re-derive.

sieval/community/ scope divergence — now documented in code, still deferred. check_preflight.check_imports carried a comment claiming its file set "must match the pre-commit hook's files: filter". It matches files: but not the effective set: pre-commit also applies the global exclude: ^(sieval/community/|vendor/), so it skips community/ while preflight checks it — exactly what .claude/rules/engineering-infra.md warns about ("compute the actual file set: hook files: × global exclude:, not the regex alone"). Inert today (all 6 relative imports under community/ are bare level-1 from . import x, verified), but a future vendored drop using from ..x import y would pass pre-commit and fail preflight, with the only offered fix being to edit code kept byte-identical to upstream. The comment now names the divergence and why fixing it is a design call — hoisting the exemption into _check_file would also drop check 2's coverage of community/. Behavior unchanged; the design decision remains open.

Related Issues

Refs #45

Test Plan

Automated

• Lint/format clean (ruff check && ruff format --check)
• Type check clean (ty check)
• Mirror tests: 102 passed (86 → 97 → 102; +5 in the third commit covering the dotted level-1 hole, spelling-equivalence, no-over-fire on public descents, and the documented residual limit)
• tests/unit/scripts/ as a whole: 210 passed
• Full preflight green, including check_imports

Manual

• sieval/ + scripts/ clean under all three checks: git ls-files '*.py' | grep -E '^(sieval|scripts)/' | python scripts/check_layer_imports.py --stdin → exit 0

• The carve-out tightening is a no-op on the current tree — AST-scanned every tracked .py under sieval/ + scripts/ for level-1 relative imports with a dotted module: 0 hits. This closes a latent hole rather than changing behavior, so there is no false-positive risk.

• Verified the check fires with a correct fix suggestion on the feat(models): capability-based Model IR + Transport frontends (RFC #25) #45 pattern:

  .../transports/openai_chat.py:23: cross-package relative import '..capabilities' —
    relative imports are for the same package only, use absolute across packages;
    use `from sieval.core.models.capabilities import ...`
  

• _resolve_relative cross-checked against CPython's own resolver: 75 (package, level, module) combinations against importlib.util.resolve_name, 0 mismatches. The above-root guard strip >= len(parts) is equivalent to CPython's len(bits) < level. _file_package also verified correct for __init__.py (drops the filename, so __package__ semantics match for both __init__ and regular modules).

• Layer-check behaviour, before → after, in a scratch tree rooted at a core/ file:

  import	before	after
  from sieval import tasks	exit 0 (missed)	core/ must not import tasks/ (sieval.tasks)
  from sieval import tasks, datasets	exit 0 (missed)	both layers reported
  from sieval import tasks as T	exit 0 (missed)	reported (asname handled)
  from ... import tasks	style error only	layer violation + style error
  from ...tasks import registry	style error only	layer violation + style error
  from .sibling import helper	clean	clean (level 1 = same layer)
  from sieval import __version__	clean	clean (real pattern in core/)
  from sieval import settings	clean	clean (non-layer public name)

• Private-access behaviour, before → after, from sieval/tasks/foo.py:

  import	before	after
  from .sub._hidden import X	clean (missed)	import from private module 'sieval.tasks.sub._hidden' outside its subtree
  from sieval.tasks.sub._hidden import X	flagged	flagged (unchanged — now equal to the relative spelling)
  from .sub.mod import helper	clean	clean (no private segment; no over-fire)
  from ._arc import _helper	clean	clean (same-package sibling carve-out intact)
  from .sub import _priv	clean	clean (documented residual limit)

• Mutation-checked for discriminating power — 11 targeted mutations, all caught. Rule 3: revert the carve-out to level > 0; threshold >= 2 → >= 3; guard >= → >; unwire from _check_file. Layer check: un-resolve back to node.module; delete the imported-alias branch; _absolute_module returning "" for level 0; returning None instead of "" above root. Third commit's predicate: revert to level == 1, widen to level > 0, flip and → or — all three killed by test_dotted_level_1_matches_its_absolute_spelling, which asserts the spelling-equivalence invariant directly rather than a specific message.

• .claude/rules/engineering-infra.md chain walked: pre-commit files: scope unchanged (^(sieval|scripts)/); hook name: updated to mention the third check; check_preflight.py wrapper key and message strings still accurate, and its file-set comment corrected; mirror test extended; CLAUDE.md / CONTRIBUTING.md wording already described this rule, so no doc change was needed — only the enforcement was missing. The files: × global exclude: interaction is now written up in check_preflight.py itself as a deferred item.

Checklist

Required (all PRs)

• PR title follows conventional format (type(scope): description)
• No internal paths, credentials, or personal info in committed files
• AI-generated code has AI-Generated Code - <model> (<provider>) in module docstring
• No new upper-layer dependencies added to core/
• Deleted code verified — no remaining call sites depend on it

🤖 Generated with Claude Code