23 KiB

Raw Blame History

ADR 0015 — Module boundaries enforced mechanically by `import-linter` layered contracts


Status	Accepted — 2026-07-30
Scope	How the modular monolith's module boundaries are made real: the tier model, the dependency rules, the mechanism that fails a build on violation, and the two escape hatches that are allowed. Does not choose the module inventory itself (`_decisions.md` Part 1, module lists)
Owner	Talha Ahmed owns `.importlinter`, the tier assignment of every module and the review of any contract change. Ahmed Mujtaba owns the `service.py`/`dto.py` facade scaffolding per module and the boundary-violation test — including the deliberately-violating fixture that proves the gate fails — with a Talha review checkpoint
Consistent with	`_decisions.md` Part 1 → Module boundary enforcement and dependency rules (all five rules), Architecture style, AI boundary: how explainability, reviewability and 'never auto-reject' are enforced structurally, Testing, CI and delivery process
Related ADRs	0001 (the monolith whose boundaries these are), 0011 (rule 3 is what makes "AI never auto-rejects" structural), 0009 (`identity` as an ambient dependency and the single authorization chokepoint), 0016 (the CI that runs the contracts), 0013 (the frontend analogue of the same argument)

Context

A modular monolith without mechanical enforcement becomes a big ball of mud in about six months. That is a general claim, so here is the specific one: this repository already demonstrates the failure mode at 4,779 lines of JavaScript.

Prototype fact	Evidence	What it produced
Every module is a global on `window` — `DB`, `UI`, `Charts`, `App`, `Router`, `Views`, plus per-module globals. 22 ordered `<script>` tags, no module system	`index.html:264-285`, `_repo-findings.md` §C	Any file can reach any other file's internals. There is no such thing as a private function
All data lives in one flat shared structure that every view reads directly	`js/data.js`	The flat candidate array carrying `jobId`, `jobTitle`, `stage`, `aiScore`, `recruiter` (`js/data.js:117-127`) — the exact modelling error Phase 1 has to undo
Scoring logic is duplicated across layers: `aiScore` is generated in the data layer, and a separate client-side "relevance" blend exists in the view layer	`js/data.js:123`, `js/candidates.js:18`	Two competing scores with no owner. This is what "no module owns its domain" looks like in practice

The flat candidate array is the important evidence. It is not a data-modelling mistake that happened to coexist with a missing module system — it is what a missing module system produces. When every view can read every field, the cheapest place to put a field is on the object the view already has.

What the enforcement has to protect

Four of the platform's non-negotiable constraints are, on inspection, statements about the dependency graph:

Constraint	The graph property that makes it true
AI must never auto-reject	No `intelligence`-tier module may import a core-domain module's write path. If `scoring` physically cannot call `application.transition()`, auto-rejection is impossible rather than forbidden
The chatbot must never bypass access controls	`assistant` reaches data only through `ai_orchestration` and `identity`; it has no import path to a repository or a model
Raw intake must exist before candidate creation	`intake` owns the chain and no other module may write `candidate` directly, so there is one code path to audit
ATS scores are per-application and append-only	Only `scoring` writes `ats_result`; no module reads another module's tables

Each of those is currently a sentence in a design document. This ADR is about converting them into a build failure.

The constraint that actually decides the mechanism

One senior developer is the only reviewer, for both developers' work, including his own. Review discipline does not scale in that shape — not because the reviewer is careless, but because there is no second reviewer for the reviewer, and boundary violations are exactly the kind of change that looks locally reasonable in a diff. "Ahmed imported candidate.models in a worklist view because he needed one field" is a two-line diff that passes any reasonable review and permanently deletes a boundary.

So the mechanism must be a tool, and the violation must be a failed build, not a review argument.

Options considered

Option A — `import-linter` layered contracts plus a forbidden-import contract (chosen)

A declarative .importlinter config: one layers contract expressing the tier order, one forbidden contract per hard rule, run in CI.

For: declarative and reviewable as a diff — the architecture is a file, not a wiki page. Zero runtime cost (static analysis of the import graph). layers contracts express exactly the tier model already decided. Failure output names the offending import chain, which is what makes it actionable for a junior. Mature, single-purpose, tiny dependency surface.
Against: static import analysis only. It cannot see importlib, dynamic attribute access, Django's string-based app/model references (ForeignKey('candidate.Candidate')), or a service-locator pattern. It also does not stop a module reading another module's tables if it does so with raw SQL rather than an import.
Verdict: chosen. Its blind spots are real and are closed by the two supplementary gates below, not ignored.

Option B — Convention documented in a README, enforced in review

For: zero tooling, zero false positives, complete flexibility.
Against: unenforceable at one reviewer. This is what the prototype had — implicitly — and js/data.js:117-127 is the result.
Verdict: rejected. This is the option this ADR exists to reject explicitly, so nobody proposes it as "pragmatic" in month three.

Option C — Separate Python packages (or a monorepo with per-module distributions) with real dependency declarations

Each module becomes an installable package; the dependency graph is enforced by what is in each package's pyproject.toml.

For: the strongest possible enforcement short of network boundaries — a module genuinely cannot import what it does not depend on. Forces facade design.
Against: 25 packages, 25 version numbers, 25 build steps and a local-development install dance, for two developers. Cross-module refactors become multi-package version bumps. It also makes Django's app registry awkward. This is service-oriented ceremony without service-oriented benefit.
Verdict: rejected on team size. Revisit at T3 below.

Option D — Runtime enforcement: an import hook or a module-level `getattr` that raises on cross-tier access

For: catches dynamic access, which static analysis cannot.
Against: a violation becomes a production exception rather than a build failure — the failure moves from CI to a recruiter's screen. Fragile against Django's own import machinery, and the debugging story for a junior facing an import hook's traceback is bad.
Verdict: rejected. Enforcement belongs in CI, not at runtime.

Option E — Full hexagonal architecture: ports and adapters per module

For: the boundary is a type, not a rule. Testability is excellent.
Against: ceremony a junior will fight, and 25 modules × (port + adapter + DTO mapping) is a large permanent tax to pay uniformly when only two modules genuinely face external systems.
Verdict: rejected as a blanket pattern. Adopted selectively for the two modules where the abstraction earns its keep: ai_orchestration's AiProvider port (ADR 0011) and integrations_inbound's mail-source port (ADR 0005).

Option F — Custom AST checks in a `flake8`/`ruff` plugin

For: exactly the rules we want, no third-party semantics to learn.
Against: writing and maintaining an AST plugin is a side project owned by the one person who is already the bottleneck. import-linter already does this correctly.
Verdict: rejected. Do not build the tool that exists.

Decision

Five tiers, a one-way dependency rule, and import-linter contracts that fail the build.

1. The tier model and the one-way rule

surfaces → core domain → platform, and intelligence → core domain (read) + platform.

graph TD
  SURF["Surfaces<br/>analytics, assistant, integrations_inbound,<br/>integrations_outbound, worklist"]
  INTEL["Intelligence<br/>ai_orchestration, scoring,<br/>fairness_evaluation, document_parsing"]
  DOMAIN["Core domain<br/>intake, candidate, duplicate_review, requisition,<br/>application, pipeline, assignment, interview,<br/>assessment, offer, talent_pool"]
  API["API layer<br/>(cross-cutting, may import any service facade)"]
  PLAT["Platform (ambient)<br/>identity, audit, files, config, notifications"]

  SURF --> INTEL
  SURF --> DOMAIN
  SURF --> PLAT
  INTEL -->|"read only"| DOMAIN
  INTEL --> PLAT
  DOMAIN --> PLAT
  API --> SURF
  API --> INTEL
  API --> DOMAIN
  API --> PLAT

Upward imports do not exist. A core-domain module importing scoring is a failed build; so is candidate importing application.

2. The five rules, and how each is enforced

#	Rule	Enforcement
1	Every module is a Python package whose only public entry point is `service.py`. Cross-module imports may touch `<module>.service` and `<module>.dto` only — never `models`, `views`, `selectors`, `tasks` or `repositories`	`forbidden` contracts: for every module M, `M.models`, `M.views`, `M.selectors`, `M.repositories` are forbidden as import targets from everything outside M
2	No module reads or writes another module's tables. Sole exception: `analytics`, which owns read-only SQL views declared in migrations	Rule 1's contract covers the import path. The SQL path is covered by supplementary gate (b) below
3	Core domain must never import `intelligence` or `surfaces`. AI results are attached by the domain module accepting a suggestion	The `layers` contract. This is the load-bearing rule
4	`identity`, `audit` and `config` are ambient — importable from every tier, importing nothing above platform	Declared as an independent bottom layer; `forbidden` contracts stop them importing upward
5	Violations fail CI	`lint-imports` is a required step in the one required workflow (ADR 0016)

3. The contract file

.importlinter at the repository root, reviewed by Talha on every change:

[importlinter]
root_packages = ats
include_external_packages = True

[importlinter:contract:tiers]
name = Tier layering is one-way
type = layers
layers =
    ats.api
    ats.surfaces
    ats.intelligence
    ats.domain
    ats.platform
containers = ats

[importlinter:contract:domain-never-imports-ai]
name = Core domain must never import intelligence or surfaces
type = forbidden
source_modules =
    ats.domain.*
forbidden_modules =
    ats.intelligence
    ats.surfaces
    ats.api

[importlinter:contract:facades-only]
name = Cross-module imports touch service and dto only
type = forbidden
source_modules =
    ats.domain.*
    ats.intelligence.*
    ats.surfaces.*
forbidden_modules =
    ats.domain.*.models
    ats.domain.*.selectors
    ats.domain.*.repositories
    ats.domain.*.views
    ats.domain.*.tasks
ignore_imports =
    ats.domain.*.* -> ats.domain.*.models

[importlinter:contract:ambient-imports-nothing-upward]
name = Platform modules import nothing above platform
type = forbidden
source_modules =
    ats.platform.*
forbidden_modules =
    ats.domain
    ats.intelligence
    ats.surfaces
    ats.api

[importlinter:contract:assistant-reaches-data-only-via-orchestration]
name = The assistant has no direct data path
type = forbidden
source_modules =
    ats.surfaces.assistant
forbidden_modules =
    ats.domain.*.models
    ats.domain.*.repositories
    ats.domain.*.selectors

The ignore_imports line in facades-only is the one deliberate exemption: a module may import its own models. Everything else is closed.

4. Two supplementary gates, because static import analysis is not sufficient

import-linter cannot see raw SQL or dynamic access. Both blind spots matter here, so both get their own gate:

(a) Grant-based enforcement at the database, for the rules that must not depend on Python at all. The application role has UPDATE/DELETE revoked on ats_result, audit_event and the six *_version tables (ADR 0002, ADR 0007). A module that bypasses every Python boundary and issues raw SQL still cannot mutate an append-only row. This is the real backstop, and it is deliberately in a different layer from the linter: one is a build-time convention check, the other is a runtime permission the process does not hold.

(b) A table-ownership check in CI. A small script parses db/migrations/*.sql for table definitions, maps each table to its owning module from a declared TABLE_OWNERS map, and greps module source for raw SQL referencing tables it does not own. Exemptions: the analytics read-only views (rule 2) and the migration files themselves. This is a heuristic, honestly labelled as one — it catches the obvious violation, not a determined one, and gate (a) is what makes the determined one harmless.

5. The boundary-violation test — the gate must be proven to fail

A test fixture deliberately containing a forbidden import, asserted to make lint-imports exit non-zero. This is Ahmed's task, and it is the interesting half of the work: a gate that has never been observed failing is a gate nobody knows is wired up. It runs as its own CI step against a fixture tree, not against ats/.

The same pattern applies to gate (a): a test asserting that UPDATE ats_result raises InsufficientPrivilege (ADR 0007, and 08-requirements-traceability.md AC1.7).

Justification

The mechanism is chosen from the team shape, and the tier model is chosen from the constraints. Those are two separate arguments and both matter.

On the mechanism: with one reviewer, a rule that is checked by a person is a rule that holds until that person is tired, on leave, or reviewing his own code. import-linter moves the check to a place that does not get tired and does not review its own work. The cost is a declarative config file and occasional friction when the config is wrong; the benefit is that the architecture in this document is the architecture in the repository, verifiably, in month eighteen.

On the tier model: rule 3 is the one that justifies the whole apparatus. "AI must never auto-reject" is a governance requirement (BRD §7.1) that most systems implement as a policy statement plus a code review habit. Here, scoring has no import path to application.service.transition(), so a rejection cannot be emitted by an intelligence module — not because it is forbidden, but because the function is unreachable. That converts a promise into a graph property, and a graph property is testable. 05-security-rbac-ai-governance.md §5.3 and 08-requirements-traceability.md §5.7 both cite this contract as one of three independent layers enforcing the same requirement, and this ADR is the one that makes it mechanical.

On why the facade is also the test seam. Rule 1 forces every cross-module interaction through service.py, which means service.py is simultaneously the public API, the mock-free test target and the audit surface. _decisions.md names service-level tests on each module facade as the primary test layer, and that layering only works because rule 1 guarantees the facade is the only door. Boundary enforcement and testability are the same investment.

The tradeoff, stated plainly

We are accepting real friction — some genuinely reasonable imports will be blocked, some will need a facade method that feels like ceremony for one field, and the config will occasionally be wrong in ways that block a merge for reasons unrelated to the change. In exchange, the four constraints listed in the Context become properties of the import graph rather than promises in a document, and the prototype's demonstrated failure (js/data.js:117-127) cannot recur silently. With 25 modules, two developers and one reviewer, the friction is the cheaper side of that trade — and the friction is informative: being blocked usually means the facade is missing a method it should have.

Consequences

Positive

The architecture is a file in the repository, diffable and reviewable, rather than a diagram that decays.
"AI never auto-rejects" and "the chatbot never bypasses access control" become graph properties with a failing build behind them.
The facade requirement gives every module a mock-free test seam, which is what makes the service-level test layer in _decisions.md viable.
Boundary violations are caught in seconds by a tool that names the offending import chain — a far better teaching signal for a junior than a review comment days later.
Extraction of a service later (if a split trigger in ADR 0001 fires) is mechanical rather than archaeological, because the seam already exists and is already enforced.
identity as an ambient bottom layer means there is exactly one authorization chokepoint (ADR 0009), reachable from everywhere and dependent on nothing.

Negative — the costs being accepted

Friction on legitimate work. Needing one field from another module means adding a facade method. Sometimes that is right; sometimes it is ceremony, and it will feel like ceremony either way.
False confidence is possible. import-linter sees imports, not behaviour. A module can respect every contract and still be badly coupled through shared table access or an over-broad DTO. Gates (a) and (b) narrow this; they do not close it.
Blind spots are real: importlib, Django's string-based model references, service locators, and raw SQL. Stated here rather than discovered later.
Config maintenance. Every new module needs a tier assignment, and a wrong ignore_imports line silently reopens a boundary. Talha reviews all contract changes for exactly this reason.
A junior can be blocked by the tool without understanding why. Mitigated by the failure output naming the chain, and by the facade pattern being the same in every module.
Gate (b) is a heuristic and will have both false positives and false negatives. It is labelled as such; gate (a) is the enforcement that does not depend on parsing source code.

Risks

#	Risk	Likelihood	Impact	Mitigation
R1	Contracts are progressively weakened (`ignore_imports` grows) to unblock delivery	Medium — this is the realistic decay path	High	Every `.importlinter` change requires a Talha review with a stated reason in the PR body. `ignore_imports` entries carry an inline comment. Quarterly read-through of the file
R2	A module respects every contract and is still tightly coupled through a fat DTO or shared tables	Medium	Medium	DTOs reviewed at module-facade design time; gate (b) on table ownership; gate (a) makes the worst coupling (writing another module's append-only tables) impossible regardless
R3	Dynamic access (`importlib`, service locator) bypasses the linter entirely	Low	Medium–High	A `ruff` rule banning `importlib.import_module` in `ats/` outside a reviewed allowlist; code review; gate (a) as the runtime backstop
R4	Django's string-based `ForeignKey('candidate.Candidate')` references create real cross-module coupling the linter cannot see	Medium — this is idiomatic Django and will happen	Medium	Cross-module FKs are a deliberate design decision, listed per relationship in `03-database-design.md`, and reviewed there rather than left to the linter. The linter's job is code coupling, not the physical schema
R5	The gate is misconfigured and silently passes everything	Low	High	The deliberately-violating fixture test (§5) exists precisely to detect this, and it runs as its own CI step
R6	Tier assignment for a genuinely ambiguous module (`worklist`, `duplicate_review`) is argued repeatedly	Medium	Low	Tier assignment is recorded in `.importlinter` and in `02-system-architecture.md` §4; a change is a reviewed diff, which ends the argument by making it concrete

Revisit conditions

#	Condition	Expected move
T1	`ignore_imports` exceeds ~5 entries, or any contract is disabled	Stop and re-examine the tier model. A contract fighting the code usually means the tiers are wrong, not that the rule is wrong
T2	A split trigger in ADR 0001 fires and a module is extracted	The facade becomes a network boundary. The contract for the extracted module is replaced by a client package; the remaining contracts are unchanged
T3	Headcount reaches four or more developers, or merge contention on shared modules becomes routine	Revisit Option C (separate installable packages). At four developers the per-package ceremony starts paying for itself
T4	Raw-SQL access outside `analytics` is found in production code more than once	Promote gate (b) from a heuristic grep to a real check — route all SQL through a single audited helper that asserts table ownership at call time
T5	`import-linter` is unmaintained or cannot express a rule we need	Re-evaluate Option F (a `ruff` plugin) — but only then, and only for the rules that cannot be expressed

ADR 0001 — the modular monolith. Its module tiers and one-way dependency rule are what this ADR enforces; without enforcement, 0001's central claim ("boundaries are logical, not network") is unverifiable.
ADR 0011 — AI provider abstraction. Rule 3 is the mechanism that makes "AI never auto-rejects" and "suggestions never write domain state" structural rather than procedural.
ADR 0009 — permission enforcement. identity as an ambient bottom layer is what gives the platform exactly one can() chokepoint.
ADR 0010 — the chatbot's controlled query surface; the assistant forbidden-import contract is the import-graph half of that guarantee.
ADR 0007 and ADR 0002 — the column-level GRANTs that form supplementary gate (a), the backstop that does not depend on static analysis.
ADR 0016 — the CI pipeline that runs lint-imports and the violating-fixture test as required steps.
ADR 0013 — the same argument applied to the frontend: with one reviewer, the rule has to be a tool (stylelint tokens, react/no-danger).
_decisions.md Part 1 → Module boundary enforcement and dependency rules (the five rules and the module graph).
_repo-findings.md §C, §H, and js/data.js:117-127 — the demonstrated failure mode this ADR exists to prevent.

23 KiB Raw Blame History Unescape Escape

ADR 0015 — Module boundaries enforced mechanically by import-linter layered contracts