HR-ATS-Portal/docs/architecture/adr/0015-module-boundary-enforcement.md

# 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`.

```mermaid
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:

```ini
[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 |

---

## Related

- **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 `GRANT`s 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.