#350 ci(imports): forbid the objective-rules hot path from the subjective ML chain

[Anonymous]

Originally created by: Alishah634

Slice

• Feature / Slice: CI enhancement — import-linter contract for issue [#324]

What this PR does

Adds an import-linter forbidden contract that keeps the objective-rules hot path (doberman.engine.rules, doberman.engine.objective, doberman.engine.trifecta) rebuilt by the host hooks on every tool call, in a cold-start CLI process from ever importing the subjective layer's heavy ML import chain (numpy/scipy/river).

Prior to this change, that boundary holds only by convention: PR [#323] deliberately extracted trifecta_fires into doberman.engine.trifecta, a models-only leaf, specifically so engine.rules.trifecta_floor could share the lethal-trifecta predicate without pulling in engine.subjective (-> engine.detectors / engine.registry -> river/numpy/scipy). Nothing enforced that convention in CI, so a future rule that did from doberman.engine.subjective import ... would silently add ~2s of import cost to every tool call, and only review would catch it (assuming the reviewer notices). This PR makes it a CI-checked invariant, enforcing the rule.

[[tool.importlinter.contracts]]
name = "Objective-rules hot path must not depend on the subjective ML import chain"
type = "forbidden"
source_modules = ["doberman.engine.rules", "doberman.engine.trifecta", "doberman.engine.objective"]
forbidden_modules = ["doberman.engine.subjective", "doberman.engine.detectors", "numpy", "scipy", "river"]

Design choices

• numpy/scipy/river are forbidden directly, not just doberman.subjective.
  import-linter's forbidden contract type checks indirect/transitive import chains by default (allow_indirect_imports defaults to False). Naming the actual ML libraries means the contract also catches a future path into them (e.g. a rule importing some other helper module that happens to import river), not only today's engine.subjective route.

• doberman.engine.registry is deliberately not forbidden.
  The issue asked to "confirm the exact forbidden set doesn't over-constrain a legitimate import" i.e engine.objective legitimately imports engine.registry.discover_rules() to run plugin rules alongside the built-ins. I traced registry.py's import graph, including its several function-local/lazy imports (discover_algebra_adapters, discover_adjudicators, discover_egress_brokers, discover_policy_sources, discover_auth_providers, discover_audit_sinks, discover_drift_observers, discover_cost_observers) and none of them reach numpy/scipy/river, even indirectly. registry.py stays a thin, ML-free entry-point loader, so it's
  safe to leave out of forbidden_modules.

• doberman.engine.detectors is forbidden even though it currently contains no ML imports.
  (just TokenChannelDetector, which only touches doberman.models/doberman.tokens). This is a forward-looking guard, and not a fix for a present violation. The package's own docstring designates it as where advanced/behavioral detectors are meant to land (built-in or plugin), so keeping it forbidden now means a future ML-heavy detector added there is automatically blocked from the objective hot path, without anyone needing to remember to update the contract at that time.

Tests added (run in CI)

tests/unit/test_import_boundaries.py:

• test_contract_is_declared_with_the_exact_source_and_forbidden_sets parses the contract out of pyproject.toml via importlinter.api.read_configuration() and asserts the exact source_modules/forbidden_modules sets, so the
  contract can't be silently narrowed later (raise-only applies to CI guards too).
• test_contract_is_kept_against_the_real_codebase builds the real import graph with grimp.build_graph("doberman", include_external_packages=True) and asserts the contract is kept, i.e. this is a pytest-visible regression test of the same property lint-imports checks in CI, not just a manual
  one-off confirmation.
• test_contract_catches_a_direct_forbidden_import synthetic graph where a hypothetical doberman.engine.rules.leaky module imports numpy directly; asserts the contract reports kept is False.
• test_contract_catches_an_indirect_forbidden_import_chain synthetic graph where a rule module imports an unrelated helper module that itself imports river; asserts the contract still catches this transitive chain (proving
  the contract isn't only a direct-import check).
• test_contract_permits_the_shared_registry_dependency synthetic graph mirroring engine.objective -> engine.registry; asserts the contract stays kept, documenting why engine.registry is intentionally absent from
  forbidden_modules.

Security checklist

• [x] Fails closed on error / uncertainty - N/A, no runtime decision path changed
• [x] No secret, full file, or unredacted prompt logged or committed
• [x] Any guardrail/learning change is raise-only (no silent loosening) - this
  PR only adds a CI check; it cannot loosen anything
• [x] Every BLOCK/AUTH carries reason codes + a human explanation - N/A, no
  runtime behavior changed

Edge cases covered / Minor Notes

• Verified locally (fresh .venv, pip install -e ".[dev]") that lint-imports
  reports all 3 contracts KEPT, ruff check . / ruff format --check . pass,
  and the new test file passes.
• CI-guard only, as scoped in the issue - no runtime behavior change, so
  README/CONTRIBUTING weren't touched (neither of the two pre-existing
  import-linter contracts is documented there either; this follows the same
  convention).

[Anonymous]

Ticket changed by: fu351

• status: open --> closed

[Anonymous]

Originally posted by: fu351

This is the right way to close out [#324]: turning a review-only convention into a contract CI actually enforces, so the objective rules can't drift back into pulling numpy/scipy/river onto the cold-start hot path. What I appreciate most is that you didn't trust lint-imports as a black box — your tests exercise the contract directly, including the indirect-import chain (a rule importing an innocent-looking helper that itself imports river) and a guard against the contract silently narrowing. That last case matters: a narrowed contract still passes lint-imports, so without your test a regression would slip through unnoticed. Merging.

For a next one, [#335] (the Codex host fails open when a hook crashes) suits you well — it's a fail-closed correctness bug, and you clearly care about invariants staying enforced rather than just documented. Want me to assign it?

If you're not already in the Discord, we're here: https://discord.gg/Sfy5XGNqty

Thanks, Alishah634.

[Anonymous]

Originally posted by: fu351

@Alishah634 Your import-linter contract from this PR is still standing guard: the objective hot path stays clean of subjective imports, and that fence keeps paying rent.

Quick community roll-call: are you in the Doberman Discord? If yes, drop your handle here (or DM me there) so I can match the GitHub side of your work to your Discord name. If not, you're warmly invited: https://discord.gg/Sfy5XGNqty