Add ARCHITECTURE.md: agent-facing reference doc

[judyks]

Summary

Adds ARCHITECTURE.md, a reference doc structured for an agent to consult mid-task rather than a human onboarding narrative: a task index ("I need to... -> touch these files"), a module dependency diagram, the full actual adr-bot data flow, core data structure shapes, the three extension-seam traits, coding conventions, and known pitfalls/constraints.

Scope grew during review: fixing the doc surfaced two existing docs that contradict it, one of them with a live bug. Those are fixed here too, so the repo is self-consistent when this lands. Four files, all markdown — no Rust source is touched.

Pass 1: initial source audit

The doc was cross-checked against the current source after an initial draft turned up real inaccuracies:

• The module dependency diagram claimed no cycles. In reality config cycles back through cli (config/dotpath.rs calls into cli::commands::models), and both runner and generation cycle back through tailoring (they consume TailoringOutput, which tailoring also depends on them for). Diagram redrawn to show tailoring as a dependency hub with the real edges, and the false "no cycles" claim removed.
• The TerminalIO extension-seam snippet was missing two methods (select_files, select_one) that have no default implementation, an agent following the doc alone would have hit a compile error implementing a new terminal backend.
• The Quick Reference coverage command (cargo llvm-cov --workspace) would have pulled in crates/tui-test's own tests and tests/tui_e2e.rs, which CI's own comments say hangs under instrumentation. Replaced with a pointer to the real invocation in coverage.yml.
• A Conventions bullet had the anyhow/Result<_, ActualError> split backwards; corrected and named the real exception (analysis::signals).
• tailoring/'s file table pointed at the wrong file for filter_minor_changes; fixed and disambiguated from the similarly-named pre_filter_rejected.
• Added two modules that were declared in lib.rs but never documented: branding/ and model_cache.rs (the latter is a real, sizeable dependency of config/dotpath.rs and cli/commands/models.rs).

Pass 2: review fixes

A second audit caught two claims that were still wrong, and traced them outward:

ARCHITECTURE.md

• TerminalIO::select_one was documented as taking default: usize; the real trait takes Option<usize>, and both impls agree. Pass 1 added the method precisely so a backend could be implemented from the doc alone — with the wrong type, that compile error was still there.
• The login.rs note said it was "excluded from the coverage ignore-filename-regex". coverage.yml puts cli/commands/login.rs inside that regex — it is excluded from coverage by it. The sentence asserted the opposite of what CI does.
• Reframed the tests/common/mod.rs vs src/testutil.rs duplication as forced rather than an oversight: testutil is #[cfg(test)] + pub(crate), so integration-test binaries can't reach it. As written it invited someone to "fix" it.
• Explained why ADR 29aaf503's benches/ requirement is a stale artifact (the source ADR cites benches/analysis_baseline.rs; Cargo.toml still carries autobenches = false; .gitignore still ignores benches/*_results*.json) and warned off both wrong reactions — creating benches/, or hand-editing the generated .claude/rules/ file.
• Replaced hardcoded line counts with a rounded figure so they don't rot on every commit to pipeline.rs.
• Cross-referenced docs/LOCAL_DEVELOPMENT.md.

CONTRIBUTING.md and docs/LOCAL_DEVELOPMENT.md — both contradicted the new doc, and both were the stale side:

• Their --ignore-filename-regex omitted cli/commands/login.rs, which CI's has. Anyone following either doc got a false coverage failure on that file. All three occurrences now match coverage.yml byte for byte.
• "Prefer anyhow::Result in binary code" didn't match the code: anyhow appears only in the five analysis/signals files, and in neither main.rs nor lib.rs. Updated to describe the actual convention.

Discoverability — ARCHITECTURE.md was referenced by no file in the repo. Linked from README.md and CONTRIBUTING.md. Deliberately not from the generated CLAUDE.md/AGENTS.md, since adr-bot overwrites those.

Test plan

• Every file path, trait signature, and struct field referenced in the doc checked against current source
• Mermaid diagram syntax reviewed (balanced brackets/quotes, no unquoted special characters in edge labels)
• All three --ignore-filename-regex occurrences byte-compared against .github/workflows/coverage.yml
• select_one snippet byte-compared against src/cli/ui/terminal.rs
• Every relative markdown link in the four changed files resolves on disk
• git diff --name-only confirms four markdown files changed and no src/ paths
• Reviewer: skim-read for anything that reads confusingly out of context (this doc has no automated test coverage of its own)

---

Generated by Claude Code

[judyks]

The Coverage Enforcement failure on this run is a pre-existing test flake, unrelated to this PR:

cli::commands::auth::tests::test_check_auth_async_no_json_timeout
expected RunnerTimeout in no-json path, got: Err(RunnerFailed { message: "failed to spawn claude: Text file busy (os error 26)", stderr: "" })

This is an ETXTBSY subprocess-spawn race (spawning a freshly-written fake claude binary can transiently report "text file busy"). A retry helper for exactly this error class already exists (spawn_with_etxtbsy_retry in src/runner/util.rs), but check_auth_async_no_json in src/cli/commands/auth.rs spawns directly without going through it.

This PR only touches ARCHITECTURE.md, no Rust source, so it can't be the cause. The same coverage job has failed-then-passed-on-retry recently on several unrelated branches (atomic-writes-fixes, oauth-exact-loopback-guard, feat/mint-token-jwt-bearer), consistent with an intermittent flake rather than a regression.

I don't have permission to trigger a job rerun from here (403 on the rerun-failed-jobs API). A maintainer re-running the job, or a new push through the merge queue, should clear it. Happy to open a separate fix applying spawn_with_etxtbsy_retry to the auth check path if that's wanted, just say the word.

---

Generated by Claude Code

[judyks]

Review: ARCHITECTURE.md

I checked the doc's claims against the source at the head commit rather than skimming it. It's unusually accurate — the hard, checkable claims all hold (exact struct fields, the 13-variant exit-code count, all three dependency cycles, the 81% test-block figure, the 9 coverage --test binaries). Two things are wrong, and merging as-is leaves the repo's docs contradicting each other.

Non-blocking — the doc is a clear net win.

---

Must fix

1. TerminalIO::select_one signature is wrong (line 302)

-    fn select_one(&self, prompt: &str, items: &[String], default: usize) -> Result<usize, ActualError>;
+    fn select_one(&self, prompt: &str, items: &[String], default: Option<usize>) -> Result<usize, ActualError>;

Source is default: Option<usize> (src/cli/ui/terminal.rs). This one matters more than a normal typo: the PR description says select_files/select_one were added to §7 because "an agent following the doc alone would have hit a compile error implementing a new terminal backend." With the wrong parameter type, that compile error is still there — the fix didn't quite land.

2. The login.rs sentence says the opposite of what CI does (line 364)

cli/commands/login.rs is excluded from the coverage ignore-filename-regex for a different reason

.github/workflows/coverage.yml:75 puts cli/commands/login\.rs inside the --ignore-filename-regex. It's excluded from coverage by that regex — not excluded from the regex. Suggested:

cli/commands/login.rs is excluded from coverage via that same --ignore-filename-regex, for a different reason: its real browser OAuth + loopback listener can't run in CI.

---

The new doc is right, but two existing docs are now stale

ARCHITECTURE.md is correct in both cases below. The problem is that shipping it makes three files disagree.

3. Coverage instructions conflict — and one is a live bug

Line 33 says don't run cargo llvm-cov --workspace. But CONTRIBUTING.md:52-60 and docs/LOCAL_DEVELOPMENT.md:98-120 both instruct exactly that, and their regex omits cli/commands/login\.rs, which CI's has. A contributor following either existing doc gets a false coverage failure on login.rs today.

Fix is one entry, in 3 places (CONTRIBUTING.md ×1, docs/LOCAL_DEVELOPMENT.md ×2):

-'(src/main\.rs|tests/|real_terminal\.rs|sync_kb_poller\.rs|tui/renderer\.rs|pty\.rs|session\.rs|test_support\.rs)'
+'(src/main\.rs|tests/|real_terminal\.rs|sync_kb_poller\.rs|tui/renderer\.rs|pty\.rs|session\.rs|test_support\.rs|cli/commands/login\.rs)'

One nit on the framing: "skips the file exclusions CI relies on" is a little unfair to those docs — they do pass a regex, it's just one entry out of date. The substance is right.

4. anyhow guidance conflicts with CONTRIBUTING.md

CONTRIBUTING.md:91 says "Prefer anyhow::Result in binary code, typed errors in library code." That's not what the code does — anyhow appears in exactly five files, all under src/analysis/signals/, and in neither main.rs nor lib.rs. §8 of the new doc describes the real convention; CONTRIBUTING.md should be corrected to match.

Recommendation: fold both into this PR. Four one-line doc edits, zero code risk, no CI impact. Merging a doc that contradicts two others — one with a regex that actively misfires — trades a slightly wider PR for a repo that's self-inconsistent the moment it lands.

---

Discoverability

Nothing in the repo references ARCHITECTURE.md. For an agent-facing reference that's the difference between useful and dead weight.

Note the obvious targets are traps: CLAUDE.md and AGENTS.md are generated from docs/adr/ by this tool (per the doc's own §10), so a link added there gets overwritten on the next adr-bot run. Link from the hand-maintained files instead — README.md (~line 183, beside the existing CONTRIBUTING.md link) and CONTRIBUTING.md §Project Structure.

---

Nitpicks

• Line 370 frames the tests/common/mod.rs ↔ src/testutil.rs duplication as avoidable ("instead of sharing one implementation"). It isn't: src/lib.rs:18 declares #[cfg(test)] pub(crate) mod testutil;, so integration-test binaries — a separate crate, compiled without cfg(test) — can't reach actual_cli::testutil at all. Calling it forced duplication is the more useful note; as written it invites someone to try to merge them.
• Lines 356-357 hardcode volatile numbers. They're exact today (#[cfg(test)] starts at line 1408 → 5,960 of 7,367 = 80.9%; 16 top-level fns), but they rot on every commit to that file, and there's no CI guard — no markdown lint or link check in any workflow. "Roughly 80% is its own test module" would age better.
• §10 omits docs/LOCAL_DEVELOPMENT.md, the closest sibling doc. Project layout is now documented in three places.
• Lines 353-359 tell agents an ADR rule's accept criteria are wrong. True — benches/ doesn't exist and rule 29aaf503 runs test -d benches — but CLAUDE.md says "Compliance is not optional," so a standing exception in a doc nothing links to is the weaker fix. Correcting the rule itself seems better.

---

Verified correct

Listing what was actually checked, not assumed:

• All three dependency cycles are real and correctly described: config→cli (src/config/dotpath.rs:3), runner↔tailoring (all 5 backends), generation↔tailoring (both directions confirmed).
• Exactly 5 production TailoringRunner impls, as listed. Trait signature exact.
• §6: every struct field matches source — RepoAnalysis, Project, ToolMatch, CanonicalIR, TailoringOutput, FileOutput, AdrSection, Adr, is_v2().
• error.rs: exactly 13 variants map to exit code 2; the ClaudeNotFound hint string is verbatim.
• anyhow confined to exactly the five named analysis/signals files.
• DEFAULT_API_URL duplicated identically in both named files.
• SERVICE_KEY hardcoded and documented as a write-only counter token — there's even a test asserting it never reaches log output.
• The integration feature gates exactly one test, and it is #[ignore]d.
• coverage.yml uses --lib plus exactly 9 --test binaries with a 100% per-file gate.
• pipeline.rs is the largest file in the repo; the 81% figure is exact.
• All TOC anchors resolve against the doc's own headers.

CI is green across all 10 checks on the head commit — the coverage flake you documented has cleared.

---

Generated by Claude Code