Repository navigation
chore(OMN-14216): install import-linter + encode intra-core dependency-layering oracle (CI + pre-commit) - #1407
Conversation
…y-layering oracle Install import-linter (dev dep) and add `.importlinter` encoding the DIRECTION of imports between omnibase_core's architectural sub-packages as `forbidden` contracts (direct-import mode). This is the enforcement oracle later decoupling work verifies against: new illegal back-edges fail `lint-imports`; current known back-edges are frozen as a minimal, annotated allowlist to burn down under OMN-3210. Enforcement wired in the same PR (Rule #5): a `lint-imports` CI job in the Phase-1 quality gate + a `lint-imports` pre-commit hook. A transitive `layers` contract is deferred (documented in .importlinter): core's graph is densely bridged through hub modules (utils/mixins/protocols), so a direct-import contract is the honest, green-today formulation. Closes OMN-14216 Refs OMN-3210
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 42 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…>models) Adds a 4th forbidden contract: protocols (the seam layer) must not import the domain/behavioral layers (models/services/validation/nodes). protocols sits low (everything depends on protocol interfaces), so this strengthens the oracle by catching any NEW protocols->services/validation/nodes edge (currently 0). The existing protocols->models direction (~65 protocol I/O type references) is frozen as burn-down debt under OMN-3210; the reverse models->protocols is left permitted. Per graph-pass steer from the lead. lint-imports: 4 kept, 0 broken. Refs OMN-14216 OMN-3210
Independent verification (leaf-0 verifier, not the implementing agent)Reproduced against both the original 3-contract head
The gate genuinely gates (negative tests, each reverted):
Ignore-list honesty (no padding): removing any one of the 8 frozen ignore families (types→models, errors→models, constants→models, types→validation, models→validation, models→nodes, validation→nodes, protocols→models) turns exactly one contract BROKEN — each ignore is load-bearing. A bogus unmatched ignore errors ( Also independently confirmed: Evidence-Source OCC#3768 (merged) carries the independent-verify receipt for |
What & why
omnibase_coredocuments an intended internal dependency layering (ADR-001, ADR-005,docs/architecture/DEPENDENCY_INVERSION.md) but nothing mechanically enforces the direction of imports between its sub-packages. Existing guards (scripts/check_transport_imports.py,scripts/validation/validate-no-infra-imports.py,tests/unit/test_no_circular_imports.py) cover cross-repo/transport boundaries and circularity — not intra-core sub-package layering.This PR is ordinary architecture hygiene: it installs import-linter and encodes the intra-core directional layering as a deterministic, CI- and pre-commit-enforced contract. New illegal back-edges fail the gate; the current known back-edges are frozen as an explicit, annotated allowlist to burn down under OMN-3210 (Platform Tech Debt Reduction).
The contract (
.importlinter)Doctrine layering (high → low):
nodes→ (services|validation) →models→ (constants|enums|errors|types), withprotocolsas the low seam layer. Encoded as fourforbiddencontracts (direct-import mode):models/services/validation/nodesmodelsmust not importservices/validation/nodesservices/validationmust not importnodesprotocols(seam) must not importmodels/services/validation/nodesWhy
forbidden(direct) and not a transitivelayerscontractRe-derived independently with
grimp(import-linter's own engine, 4078 modules): core's graph is densely bridged through hub modules (utils,mixins,protocols), so a transitivelayerscontract can't be expressed today without an unmaintainable bridge allowlist. The contracts therefore enforce direct imports (allow_indirect_imports = True). Tightening to a transitive contract is a follow-up gated on slimming those hub modules.protocols ↔ modelsis a real cycle: theprotocols → modelsdirection (~65 protocol I/O type refs) is frozen debt; the reversemodels → protocolsis left permitted.Frozen back-edges (allowlist, burn down under OMN-3210)
The doctrine's clean layer story does not fully hold. Direct back-edges frozen (8 classes / ~124 edges), each annotated in
.importlinter:protocols → modelstypes → modelsmodels → validationerrors → modelsmodels → nodesconstants → modelstypes → validationvalidation → nodesunmatched_ignore_imports_alertingis left at its ERROR default, so when a class is fully removed its stale ignore line forces its own deletion (self-cleaning ratchet).Enforcement (same PR, Rule #5)
lint-importsjob (Import Layering Oracle) wired into the Phase-1quality-gate(needs + gate condition + summary row).lint-importshook (pre-commit stage; grimp caches the graph so re-runs are sub-second)..import_linter_cache/added to.gitignore.Follow-up recommendation for the operator: add the
lint-importscontext todevbranch-protectionrequired_status_checksonce it has run green for a few PRs.dod_evidence
Local verification (worktree off
origin/dev):lint-importsexits 1 (gate genuinely gates).uv run ruff format --check src/ tests/→ clean;uv run ruff check src/ tests/→ All checks passeduv run mypy src/omnibase_core/→ Success: no issues found in 4079 source filesuv run pytest tests/unit/test_no_circular_imports.py→ 20 passeduv run pre-commit run --files <changed files>→ all Passed/Skipped (yamlfmt, ruff, import-linter, gitignore-baseline included)uv lock --check→ consistentDiff is config/CI/deps only — no
src/ortests/Python changed. Full suite runs in CI on this PR.Closes OMN-14216
Refs OMN-3210
Evidence-Source: OCC#3768
Evidence-Ticket: OMN-14216