Adopt directory-based module structure

docs/adr-006-directory-based-module-layout.md

@@ -0,0 +1,164 @@
+# Architectural decision record (ADR) 006: directory-based module layout
+
+## Status
+
+Accepted (2026-10-16): Adopt `mod.rs`-rooted directory modules, and extract an
+inline test module that outgrows the 400-line budget into a sibling file wired
+with `#[cfg(test)] #[path = "..."] mod ...;`.
+
+## Date
+
+2026-10-16.
+
+## Context and problem statement
+
+`AGENTS.md` states that no single code file may be longer than 400 lines. The
+rule is a readability constraint rather than a compile-time one: a module of
+that length has stopped being reviewable in one sitting, so growth past the
+limit is answered by decomposition, not by raising the threshold.
+
+Rust offers two ways to turn a module file into a directory. The first is a
+directory whose root file is named `mod.rs`. The second is a *self-named
+sibling*: `install_flow.rs` sits next to an `install_flow/` directory that
+holds its children. Both are legal, and the choice is not visible to the
+compiler's semantics — only to the reader who has to find a module's source.
+
+A second and larger source of growth is the inline test module. Whitaker
+colocates unit tests with the code they exercise, and a
+`#[cfg(test)] mod tests { … }` block grows faster than the production code
+above it, because every edge case adds an arm. Several installer modules had
+reached the point where the test block alone carried the file past the budget,
+which means the budget could not be met without either weakening the tests or
+moving them.
+
+The decision is how to structure modules at the point they pass the budget, and
+how to move a test module without disturbing the tests themselves.
+
+## Decision drivers
+
+- Keep every module within the 400-line budget that `AGENTS.md` sets.
+- Keep a structural change purely structural, so the diff can be reviewed as a
+  move rather than re-read as behaviour.
+- Preserve each test module's path, so `super::` and `crate::` imports inside it
+  and `cargo test` filter expressions that name it continue to resolve.
+- Choose one directory-module form and apply it consistently.
+- Keep the extracted file a real module, subject to the same documentation rules
+  as the code it came from.
+
+## Options considered
+
+### Option A: keep test modules inline and raise the threshold
+
+Rejected. The budget is the reason to decompose, so exempting the largest files
+from it inverts the rule. It also leaves the production code and its tests in
+the same file with no mechanism to separate them later.
+
+### Option B: extract tests but use the self-named sibling form
+
+Rejected. The repository's convention is the `mod.rs` form: 46 `mod.rs` files
+are tracked outside `vendor/`, and no path in the tree has both `name.rs` and
+`name/mod.rs`, so no module is ambiguous between the two. Two file modules
+deviate and reach a child through a same-named directory:
+`installer/src/cli.rs` resolves `mod source_policy;` to `cli/source_policy.rs`,
+and `crates/conditional_max_n_branches/src/driver.rs` resolves `mod numeric;` to
+`driver/numeric.rs`. Both are single-child outliers, and neither is a
+convention anyone is expected to follow. Adopting the form deliberately would
+make a module's location depend on which convention its author happened to use.
+
+### Option C: `mod.rs` directory modules with `#[path]` sibling test files
+
+Accepted.
+
+## Decision outcome
+
+A module that has grown submodules becomes a directory whose root file is
+`mod.rs`. Directory modules nest, so `installer/src/toolchain/tests/` is itself
+a `tests/` directory module with its own `mod.rs`.
+
+An inline test module that outgrows the budget moves to a sibling file. The
+declaration keeps the module's name, and an explicit `#[path]` attribute points
+it at the new file:
+
+```rust
+#[cfg(test)]
+#[path = "list_tests.rs"]
+mod tests;
+```
+
+A module's name and its file are separate things. `mod tests;` fixes the name,
+and with it the module path `crate::list::tests`; `#[path]` selects the file
+that backs it. The attribute is what makes the move reviewable: the test bodies
+stay byte-identical across it, so the extraction can be checked as a move
+rather than re-read as behaviour.
+
+The attribute also supplies the placement. Left to the default location, a bare
+`mod tests;` in `list.rs` resolves to `list/tests.rs`, inside a `list/`
+directory of its own. Naming the declaration after the file, as
+`mod list_tests;`, would rename the module to `crate::list::list_tests` — and
+still resolve to `list/list_tests.rs`, not to a sibling. Only the attribute
+puts the file beside `list.rs` while keeping the module named `tests`.
+
+Renaming the module would not, on its own, break the tests: `super::` keeps
+working, because the module remains a child of `list`. What breaks is absolute
+paths such as `crate::list::tests::…` and `cargo test` filter expressions that
+name `tests`.
+
+Three shapes occur, and the rule that distinguishes them is whether anything
+above the extracted module already restricts it to test builds:
+
+- *File-module sibling.* The extracted file sits beside the module file and is
+  wired with `#[cfg(test)]` plus `#[path = "<name>_tests.rs"]`.
+- *Directory-module `tests.rs`.* Inside a directory module the attribute is
+  unnecessary, because Rust already resolves a bare `#[cfg(test)] mod tests;` to
+  `tests.rs` in the same directory.
+- *Nested tests directory.* When the tests need helpers and submodules of their
+  own, they become a `tests/` directory module with a `mod.rs`.
+
+`installer/src/deps/tests.rs` shows the variation that follows from the rule.
+It is declared as `#[cfg(test)] mod tests;` by `installer/src/deps/mod.rs`, so
+the whole file is already test-only and its children omit `#[cfg(test)]` for
+that reason alone. They still carry `#[path]`, because there is no
+`deps/tests/` directory for a bare `mod` to resolve into.
+
+An extracted test file still opens with a `//!` module-level doc comment, so it
+satisfies the repository rule that every module documents its purpose.
+
+## Known risks and limitations
+
+- `#[path]` is a manual anchor. Renaming or moving an extracted test file is not
+  checked by the compiler until the module is built, so the attribute can go
+  stale in a way that a conventional module path cannot.
+- The attribute's *meaning* moves with the module's form. In a file module
+  `name.rs`, `#[path = "p"]` resolves relative to the containing directory; in
+  a directory module `name/mod.rs` it resolves relative to `name/` itself.
+  Converting a file module to a directory module therefore doubles the module's
+  own segment in any `#[path]` that already opens with it, as
+  `install_flow/mod.rs` did when a rebase renamed `install_flow.rs` around a
+  pre-existing anchor. A bare `mod` in a directory module carries no such
+  hazard, which is why the decision outcome above confines the attribute to the
+  file-module case.
+- The 400-line budget is a repository convention that this decision serves, and
+  it is **not** yet machine-enforced in this repository. The `module_max_lines`
+  dylint lint implements the check and its own UI tests cover it, but the
+  workspace does not run Whitaker's lints over itself: `make lint` runs
+  `cargo doc` and `cargo clippy` only, and the repository's own `dylint.toml`
+  configures `no_std_fs_operations` alone. Self-linting is a separate layer of
+  work, and until it lands the budget is held by review.
+- The extraction adds a file per module, so a directory listing no longer shows
+  a module and its tests together. The `#[path]` attribute is the link between
+  them, which is why it is written explicitly rather than inferred.
+
+## Architectural rationale
+
+The decision keeps two properties that matter more than the specific file
+names. The first is that structure and behaviour stay separable: because the
+declaration keeps the module name and `#[path]` only redirects the file, a
+decomposition commit can be reviewed as a move, and a behaviour change can
+never hide inside one. The second is that the budget is answered by
+decomposition rather than exemption, so the rule continues to describe the
+codebase instead of describing the exceptions to itself.
+
+Both properties follow from treating the module layout as a repository-wide
+convention rather than a per-module judgement. 65 tracked `.rs` files already
+carry a `#[path = "` anchor, so the mechanism this record standardizes was
+already the repository's practice; the record fixes which cases use it and why.

docs/contents.md

@@ -94,6 +94,10 @@
 - [Architectural decision record (ADR) 005: adopt the shared CV-005 contract
   library](adr-005-adopt-the-shared-cv005-contract-library.md) records the move
   from a local copy of the CV-005 contract to the pinned shared check.
+- [Architectural decision record (ADR) 006: directory-based module
+  layout](adr-006-directory-based-module-layout.md) records the `mod.rs` module
+  convention and the extraction of oversized test modules into sibling files,
+  and states plainly that the 400-line budget is not yet machine-enforced here.
 
 ## Requests for comments
 

docs/developers-guide.md

@@ -1404,6 +1404,145 @@ make check-fmt  # Verify formatting
 make fmt        # Apply formatting
 ```
 
+## Module layout and test placement
+
+### The 400-line module budget
+
+`AGENTS.md` states that no single code file may be longer than 400 lines. The
+limit has a machine implementation — `module_max_lines`, one of Whitaker's own
+dylint lints, measures the source lines occupied by a module and warns when the
+count exceeds a configurable `max_lines` threshold, which defaults to 400 — but
+that implementation does not yet run against this repository. See
+[Where the budget is checked](#where-the-budget-is-checked) for what does and
+does not execute.
+
+The budget is a readability rule. A module of that length has stopped being
+reviewable in one sitting, so growth past the limit is answered by decomposing
+the module, not by raising the threshold.
+
+The reasoning behind the layout rules below, including the alternatives that
+were rejected, is recorded in
+[ADR 006](./adr-006-directory-based-module-layout.md).
+
+### Directory modules
+
+A module that has grown into submodules becomes a directory whose root file is
+`mod.rs`, for example `installer/src/install_flow/mod.rs` and
+`installer/src/toolchain/mod.rs`. The self-named-sibling form, where
+`install_flow.rs` sits next to an `install_flow/` directory, is not the
+convention here: 46 `mod.rs` files are tracked outside `vendor/`, and no path
+has both `name.rs` and `name/mod.rs`. Two file modules do reach a child that
+way — `installer/src/cli.rs` and
+`crates/conditional_max_n_branches/src/driver.rs` — but they are single-child
+outliers, not the pattern to copy.
+
+Directory modules nest. `installer/src/toolchain/tests/` is a `tests/`
+directory module with its own `mod.rs`, holding `test_helpers.rs` and a further
+`failure_mocks/` subdirectory.
+
+### Sibling test modules
+
+An inline `#[cfg(test)] mod tests { … }` block that grows large enough to
+threaten the 400-line budget moves to a sibling file. The declaration keeps the
+module's name, and an explicit `#[path]` attribute points it at the file:
+
+```rust
+#[cfg(test)]
+#[path = "list_tests.rs"]
+mod tests;
+```
+
+Three shapes of this appear in the tree.
+
+- **File-module sibling.** The extracted file is a sibling of the module file,
+  wired with `#[cfg(test)]` plus `#[path = "<name>_tests.rs"]`.
+  `installer/src/list.rs` gains `installer/src/list_tests.rs`,
+  `installer/src/dependency_binaries/install/downloader.rs` gains
+  `downloader_tests.rs`, and `installer/src/deps/tests.rs` gains
+  `policy_tests.rs` and `tests_repository.rs`.
+- **Directory-module `tests.rs`.** Inside a directory module the attribute is
+  unnecessary, because Rust already resolves it, so a bare
+  `#[cfg(test)] mod tests;` resolves to `tests.rs` in the same directory.
+  `installer/src/toolchain/mod.rs` ends with that declaration, and
+  `installer/src/install_flow/mod.rs` reaches `dependency_check_tests.rs` the
+  same way, with a bare `#[cfg(test)] mod dependency_check_tests;`.
+- **Nested tests directory.** When the tests themselves need helpers and
+  submodules, they become a `tests/` directory module with a `mod.rs`.
+
+The `#[path]` form is preferred for the file-module case because `mod tests;`
+keeps the test module's name, `tests`, and therefore its module path unchanged,
+while the attribute places the file beside the module rather than in a
+directory of its own. Left to the default location, a bare `mod tests;` in
+`list.rs` resolves to `list/tests.rs`; naming the declaration after the file, as
+`mod list_tests;`, would rename the module to `crate::list::list_tests` and
+still resolve to `list/list_tests.rs`, not to a sibling. Only the attribute
+puts the file beside `list.rs` while keeping the module named `tests`.
+
+Renaming the module would not break `super::`, since the module stays a child of
+`list`; what breaks is `crate::` paths such as `crate::list::tests::…` and
+`cargo test` filters naming `tests`. With the attribute the production file
+drops back under the budget without folding a behaviour change into a purely
+structural edit. A test file extracted this way still carries a `//!`
+module-level doc comment, matching the repository rule that every module begins
+with one.
+
+`installer/src/deps/tests.rs` shows the one variation on this. It is not a
+module file in its own right: `installer/src/deps/mod.rs` declares it as
+`#[cfg(test)] mod tests;`, so the whole file is already test-only. Its own
+children therefore omit `#[cfg(test)]`, which would be redundant. They still
+carry `#[path]`, because no `deps/tests/` directory exists for a bare `mod` to
+resolve into:
+
+```rust
+#[path = "policy_tests.rs"]
+mod policy;
+
+#[path = "tests_repository.rs"]
+mod repository;
+```
+
+Add `#[cfg(test)]` to an extracted test module only when nothing above it
+already restricts it to test builds.
+
+The convention is repository-wide rather than installer-only: 65 tracked `.rs`
+files contain a `#[path = "` anchor.
+
+### Where the budget is checked
+
+Be precise about this, because the current answer is narrower than it looks.
+
+`make lint` runs
+`cargo clippy --workspace --all-targets --all-features -- -D warnings`,
+preceded by a `cargo doc` pass. Neither runs the 400-line budget. The budget is
+implemented by `module_max_lines`, one of Whitaker's own dylint lints, and a
+dylint lint is not a rustc or Clippy built-in: it is a separate compiler plugin
+that runs only when a workspace loads it deliberately. This workspace does not.
+`rg 'dylint run' Makefile scripts/` finds nothing, and the root `dylint.toml`
+configures `[no_std_fs_operations]` alone. The `[workspace.lints.rust]` and
+`[workspace.lints.rustdoc]` tables in `Cargo.toml` do not change this: they
+configure rustc and rustdoc lints, and neither names `module_max_lines`. Within
+the CI workflow the dylint host tools are restored for the `make publish-check`
+step alone, and that step's single dylint invocation is
+`dylint list --no-metadata --no-build` — it enumerates built libraries to
+verify packaging, and lints nothing.
+
+So the budget is **not** currently enforced automatically against this
+repository's own source. The lint itself is real and tested — its UI fixtures
+under `crates/module_max_lines/ui/` pin both the failing and passing cases —
+but nothing runs it over these crates. Running Whitaker's lints over Whitaker
+is the final layer of the lint-hardening stack, and it has not landed yet.
+
+Until it does, the budget is held by review. That is a weaker guarantee than a
+gate, and the difference matters: a module that drifts past 400 lines will not
+turn a check red.
+
+The threshold, when self-linting arrives, is set as TOML under
+`[module_max_lines]`, not by a compiler flag. The shape is documented in
+[`docs/whitaker-cli-design.md`](./whitaker-cli-design.md).
+
+The layout convention itself is recorded in
+[ADR 006](./adr-006-directory-based-module-layout.md).
+
 ## Python script interpreter convention
 
 Every Python script that a workflow or the `Makefile` runs by path carries the