--- id: module-layout title: Where code goes is a compile error, not a convention status: shipped repos: [atgc] dependsOn: [] exitCriterion: > Nothing outside cmd/ imports cmd/, no module reaches sideways for a neighbour's helper, and the two rules visibility cannot express have tests. --- # module-layout Six folders under `src/`: `cmd/` for the command families, `clients/` for everything that talks to a counterpart (git included), `lexicon/` for the pure record and identifier conventions, and `config/`, `logging/` and `term/` for the rest. `docs/module-layout.md` is the contract, and it landed first so the moves could be read against it. The rules are enforced where they can be. Everything in `cmd/` is `pub(super)`, `pub(in crate::cmd)` or — for the entry points `main.rs` dispatches through — `pub(crate)`; no bare `pub` is left. The two rules visibility cannot express, because `clients` has to be `pub(crate)` for `cmd/` to call it, are pinned by `tests/module_layout.rs` instead. Both had been violated in the tree at some point, and both were found by grep, which is the argument for the test. The three-copies-of-a-body-reader entry this file used to carry is gone rather than restated: `term/body.rs` is that function, and what is left at the three call sites are named wrappers whose doc comments say what each one's nested `Option` means. ## What it needs - [ ] Not to be merged with it: `newest_state` and `state_of` look like the same pair and are deliberately not. The issue half orders on a parsed instant because it merges the author's records with the acting account's and two accounts write `…Z` and `…+03:00`; the pull half reads one PDS and orders on the string. Written down because the two are a page apart and the obvious cleanup is wrong — see `StateEvent::instant`, which argues it. If the pull half ever merges a second account's statuses it needs the instant compare, and that is the change to make, not the sharing ## Done - [x] The rule behind a run of defects is written down, and it is not a missing type. `Evidence` and `Listing` were already the right shape and said so; what went wrong was flattening them at a boundary, which `RepoRows` did and no longer does. The other three carriers — `RecordsAbout`, `web::Backfill`, `web::pulls::Coverage` — were surveyed and left alone deliberately: they answer different questions (did I reach a floor, how many did I never open, did I see every numbered pull), each is consumed by every caller that needs it, and forcing one type over them would be unification for its own sake. Two call sites read `truncated` directly and are right to, because they ask about the page cap rather than about completeness. What was actually missing was the shared obligation, stated nowhere: a walk that can stop early has to say so, and a caller that concludes from absence has to ask. `docs/architecture.md` carries it now, with the three shipped failures of that shape as evidence and the four carriers named as answering one question in different shapes - [x] `RepoRows` carries the whole `Evidence` rather than a `truncated` bool. Every stack command asked that bool whether the rows were safe to draw a conclusion from, and it answered correctly only by accident: `repo_rows` asks for `Reach::Whole`, so "did not hit the page cap" and "is the whole collection" coincide. Narrow the reach and they come apart in silence, taking with them every refusal that exists because a chain read off a short listing has members missing and no sign that any are. The question is `complete()` now, which is both facts, and a test pins the case that tells them apart - [x] Two doc comments in the read half were attached to the wrong item, both found by the split that moved them and both moved as they were rather than fixed inside a move. `append_backfilled`'s paragraph — the one describing an append and saying why `indexed` stays false — sat on top of `apply_backfill_states`, whose own doc ran on underneath it in the same block, so the function it described had none. Each is over its own function now. The other was a half-sentence, "The observation this whole change is built on, as it stood on", stranded above `builds_a_repo_url_from_the_label_it_already_had` in `labels.rs`: a fragment of the `-- the staleness evidence --` section's opening, whose separator went to `sources.rs` with the tests it heads and reads completely without it. Deleted rather than reunited, since there is nothing left for it to open - [x] One backwards walk over a subject's records, not two. `cmd/issue/read.rs`'s `state_events_for` and `cmd/pr/write.rs`'s `list_statuses` were the same function — page `listRecords` newest-first, keep the records naming this subject, stop once a page ends below the subject's own rkey — differing in the NSID, the subject field, the state field, the struct built, and a 50-page cap spelled twice under two names. They are `pds::records_about` now, and the strict comparison that keeps a backfilled record written under the subject's own key visible is stated once, in `page_ended_below`, with the test that would fail if somebody tidied it to `<=`. The move turned up what the duplication was hiding. Neither copy said whether it had reached the bottom, so both read the page cap as the end of the collection — and the absence of a record is not nothing here, it is what "still open" is read off. A `merged` status one page past the cap therefore read as `open`, and `pr close` would write over it without the merged-is-terminal refusal ever firing. `records_about` reports `complete`; the write path refuses on a short walk and the issue listing says so and carries on, which is the difference between a state that decides a write and a state that fills a column - [x] `html/` — a folder for the documents atgc serves a browser, sibling to `term/`, and `art.rs` at the root for the field of DNA both media draw. The OAuth callback pages left `clients/atproto/oauth/`, where markup and SVG sat in the middle of a module about tokens; the field left `cmd/about.rs`, which is how a *client* came to import a **command** module for `field_html` — the one edge in the tree running against docs/module-layout.md. `respond_html` stays with the client: the line falls at the `TcpStream`, since building a page opens nothing and writing one to a socket is talking to a counterpart. A move and not a rewrite: `atgc about` is byte-identical across 16 frame sizes, and the two renderings now share `Canvas::rows` rather than keeping a `rposition` call each - [x] The module layout is enforced rather than described. Everything in `cmd/` is now `pub(super)` (119), `pub(in crate::cmd)` (41) or — for the entry points `main.rs` dispatches through, the clap types reachable from them, and the JSON shapes docs/output.md links to by path — `pub(crate)` (125). No bare `pub` is left. That makes "nothing outside `cmd/` imports `cmd/`" a compile error for everything except those entry points, which is as far as visibility can go inside one crate - [x] `tests/module_layout.rs` for the two rules visibility cannot express — `lexicon/` importing `clients/`, and a client asking `config/` who we are. Neither is expressible because `clients` has to be `pub(crate)` for `cmd/` to call it, and `pub(crate)` is visible to `lexicon/` too; only a workspace split would do it. Both were violated in the tree at some point and both were found by grep, which is the argument for the test - [x] The one edge that pointed the wrong way: `clients/atproto/oauth/ pages.rs` called `cmd::about::field_html`. It was named as the single allowed exception in `tests/module_layout.rs`, and the `html/`+`art.rs` move deleted both the call and the allowlist entry — the page and the field of DNA now live in `html/` and `art.rs`, and `pages.rs` is gone. Nothing outside `cmd/` imports `cmd/` any more, in code or in the allowlist - [x] `auth.rs` split into `clients/atproto/oauth/{client,sessions,store, login}.rs` and `cmd/auth.rs`, which is the piece the folder move deliberately left behind. A pure move: no printed string and no step of the OAuth flow changed. The seam is the `config/` constraint rather than the verbs — `login` and `agent_for_did` read `crate::config::account` to pick a DID and so stay in `cmd/`, while everything they call takes that DID as an argument - [x] `cmd/pr/read.rs` was past the split rule at 4,287 lines, and the seam was not the verbs. It is a directory now, along the line the module doc had been drawing all along. `sources.rs` is the source reconciliation — `Source`, `Reach`, `pds_pulls`, `merge`, `missing_from_bobbin`, `warn_stale`, `gather`, `settle_own_open` — which is to say deciding what the set of pulls *is*; `mod.rs` is the arguments, the two listing verbs, `pr view` and the `--json` shapes, which is deciding how to say it. A third file fell out rather than being planned: `labels.rs`, the handle, repo-name and appview-number lookups, which neither half owns and both need — the columns want them and so does the backfill, for a repo's web root. `Listed` is the currency between all three, so there is no arrangement without a back edge; three files at least name what each one is. Re-exports keep every `crate::cmd::pr::read::…` path other modules already use, so nothing outside the directory moved, and the only visibility that widened is private → `pub(super)` on the items a listing reads off a gathered row. The three entries this was said to be waiting behind did not block it in the end: the concurrency and ordering ones landed, and the `"?"` sentinel one is a change to `State`, `label()` and `known_state`, which now sit closer together than they did - [x] `record::put` — one `putRecord` with a `swapRecord` precondition, for every command that edits a record rather than creating one. `pr resubmit` and `pr edit` had the only copy, inside `pr/write.rs`, and `repo edit` needed the same thing; jacquard's own helper hardcodes `swapRecord: None` and cannot express the precondition at all - [x] One module per remote, instead of a URL built wherever a command needed one: `crate::pds` owns the `{pds}/xrpc/…` reads — getRecord, listRecords and its paging, blob downloads — and `crate::bobbin` owns the appview's base URL and its three endpoints. `pr`, `repo`, `review` and `ssh` all went through their own copy before, so a paging or error-shape fix landed in one caller and not the rest. docs/architecture.md says which commands reach which - [x] `pr` split into a directory at 3,141 lines: `pr/read.rs` for `list`, `status` and `view`, `pr/write.rs` for `create`, `resubmit`, `edit`, `close`, `reopen` and `comment`. A move rather than a refactor — the two halves share one function between them, and `pr/mod.rs` re-exports the same names at the same paths, so `main.rs` did not change - [x] Six folders under `src/` instead of thirty siblings: `cmd/` for the command families, `clients/` for everything that talks to a counterpart (git included), `lexicon/` for the pure record and identifier conventions, and `config/`, `logging/` and `term/` for the rest. docs/module-layout.md is the contract, and it lands first so the moves can be read against it. Landing as a stack, one folder per PR, with the splits that fall out of it: `auth.rs` four ways, `review.rs`'s appview scrape out to `clients/tangled/web/`, every git invocation under `clients/git/`, `repo.rs` into read/write/branch/checkout, and clap inlined into `cmd/` so the 31 mirror arg structs go away. Landed as a fifteen-pull stack. Two pieces are deliberately left: `auth.rs` still holds both the OAuth client and the `auth` verbs, and wants splitting into `clients/atproto/oauth/{client,session}.rs` and `cmd/auth.rs`; and `write_git_identity` stayed in `cmd/repo/checkout.rs` as policy over the `clients/git/config.rs` primitives rather than moving wholesale - [x] `cmd/issue/read.rs` reached sideways for `cmd::pr::read::ellipsize`, which is the shape rule 2 in docs/module-layout.md exists to stop. Done since: it and `day` live in `term/column.rs`, and `issue`, `repo`, `search` and `pr` all import them from there - [x] `new_body`/`comment_body` in `cmd/issue/write.rs` were the third near-copy of "a body from a flag, a file, or stdin" in the tree, after `cmd/pr/write.rs` and `cmd/report.rs` — three copies serving six commands. Now `term/body.rs`: `read` for the clearable case, whose nested `Option` is what lets an edit tell "leave it alone" from "clear it", and `required` for a comment, which has no second meaning for empty. Below `cmd/` because rule 2 says so, and beside `term/noinput` because both are about what is fed to a terminal. The one real difference between the copies was `report` trimming its result, which is kept at that caller with the reason written down: a report body is prose on a public board, while a pull body's indentation is content - [x] `model/`, the layer the tree did not have: `lexicon/` says what a record *is* on the wire, `cmd/` acts on one, and nothing owned what a record *means*. So every verb re-derived it, and the copies drifted — six readings of a pull's target branch, one of which defaulted to `main` on the merge path; five of which state record wins, three of them deciding state and ordering it three ways; two round counts disagreeing about an empty array; `Standing` written once for pulls and once for issues. `model/pull.rs` and `model/record.rs` hold the single version of each, and rule 4 in docs/module-layout.md says where the next one goes. `cmd/stack`'s chain reader is the same shape arrived at by accident and is the evidence it works: chain rules stopped diverging once there was an obvious place to put them, and it wants moving here as `model/chain.rs` when the stack work settles