[Architecture] refactor: per-domain modules with service layers, enforced by an architecture test - pure motion, zero new deps
From the PR description
Row amal66#42 of the reference index (#205) - the backend reorganization the index says to take last. Rebuilt on 2026-09-03 on top of main (its former base #294 has merged), and finished: the previous revision stated conventions the tree did not meet; this one meets them and enforces them.
Summary
The Express backend becomes domain modules over a shared kernel. Every HTTP surface lives in backend/src/modules/<domain>/ as a thin *.routes.ts (parse, call, map status) over a service layer (explicit db: Db, typed results, no req/res), reachable from outside only through a named-re-export facade. backend/src/routes/ is gone. A fitness test makes the layering rules fail CI when crossed. Zero new dependencies; 222 endpoints byte-identical to main including registration order.
main |
this PR | |
|---|---|---|
| HTTP handler files | 17 in routes/, 14,826 lines |
18 *.routes.ts in 16 modules, 6,105 lines (HTTP layer only) |
| Largest routes file | routes/tabular.ts 2,389 |
tabular.routes.ts 779 |
| Inline DB queries in routes files | everywhere | 0 (enforced) |
| Modules with a facade + service layer | 0 | 16 / 16 |
type Db = redeclarations |
30 | 0 (one export in lib/supabase.ts) |
| Backend test files | 105 | 115 (+ ~60 service-level unit tests, fake db, no server) |
| Coverage floors (stmts/branches/fns/lines) | 52/46/53/54 over lib/ only |
55/47/57/57 over lib/ + modules/ |
| Layering enforcement | none | src/__tests__/architecture.test.ts, 7 rules |
Start with docs/backend-architecture.md - it is the reference for everything below.
What changed
1. One Db type, one result contract (lib/supabase.ts, lib/serviceResult.ts). Db is exported once and every service takes it first. ServiceResult<T> / ServiceFailure name why an operation failed (validation→400, forbidden→403, not_found→404, conflict→409, unavailable→503, error→500 through sendInternalError, so raw errors are logged with the request id and never sent). sendServiceFailure(res, f) is the single status-code policy. Modules that predate it keep local kinds where a response needs a status the table does not name (422, 502).
2. The motion, completed. Sixteen modules: audit, auth, chat, documents, downloads, library, models, project-chat, projects, quick-actions, source-documents, tabular, uploads, user, word-chat, workflows. Compared with the previous revision: the six "not yet modularized" route files are modularized; uploadSessions.ts (new on main) became modules/uploads together with lib/uploadProcessing + lib/uploadSessions; models gained the facade it lacked; tabular.routes.ts went from 2,035 lines / 46 inline queries to 779 / 0; chat and project-chat routes are at zero too. Domain logic that lived in lib/ moved into its module when nothing that stays in lib/ imports it (userSettings, userApiKeys, projectsOverview, workflowsOverview, workflowName, tabularReviewsOverview, chatTitle, the upload pair). SSE loops stay in routes files; tabular.generateStream.ts is the one sanctioned service file that takes res, and its header says so.
3. The rules are executable. backend/src/__tests__/architecture.test.ts walks every import under src/ and fails on: lib/→modules/ (one allowlisted edge), any cross-module import that is not the facade (except app.ts mounting *.routes), express or res.* outside *.routes.ts (one allowlisted exception), any .from(/.rpc( in a routes file (ratchet, baseline empty), export * in a facade, middleware/→modules/, and the existence of src/routes/. Each allowlist entry carries the reason. No lint plugin, no dependency-cruiser; it runs with npm test.
4. Coverage ratchet widened and raised. Include is src/lib/** + src/modules/**; the new service tests lifted the measurement, so floors go up to 55/47/57/57.
5. Docs. docs/backend-architecture.md (module anatomy, service contract with example, the seven rules, what stays in lib/ and why, named follow-ups, how to add a domain); AGENTS.md Backend Structure rewritten; docs index and testing-coverage updated.
Why
The monoliths were where changes went to be risky, and the earlier revision of this PR proved that a convention without enforcement does not hold: it documented "routes never query the database" while shipping 46 inline queries in one file. A codebase is only as modular as its worst module and only as consistent as its CI makes it. This revision closes both gaps in the same change so the next contributor inherits a layout that the test suite defends.
Replication - proving it is pure motion
No user-visible behavior changes; the thing to verify is that only the layout changed.
- Endpoint parity, runtime proof. From
backend/on each ref, withSUPABASE_URL/SUPABASE_ANON_KEY/SUPABASE_SERVICE_ROLE_KEYset to dummies andDB_JOBS_ENABLED=false, importsrc/app.tsundertsx, walkapp._router.stackrecursively, printMETHOD pathper layer.mainand this tip both print 222 lines,diffempty (order included -/createmust beat/:chatId). - Rebase fidelity. Every hunk
mainapplied to the moved files since this branch's previous base was re-homed: the direct-upload session protocol replacing multipart uploads (documents, projects, library, workflows), workflow reference files → document-backed assets,transparent_tablesin the profile cascade,workflow_id+canEditgates on document access, the streamingdownload-zipwith folder selection.main's integration tests for each (documentsUpload,uploadSessions*,workflows,workflowAddons,user) pass unchanged. - Build + tests.
npx tsc --noEmitclean at every commit;npm test --prefix backend→ 1,209 passed | 34 skipped;npm run test:coverage --prefix backendpasses the raised floors (55.54 / 47.47 / 57.94 / 57.94 measured). - Architecture test.
npm test --prefix backend -- src/__tests__/architecture.test.ts→ 8/8. To see it bite, adddb.from("chats")to any routes file, orimport "../modules/chat/chat.title"from alib/file: the failure names the rule and the file.
The five commits
Each compiles on its own and each message is a self-contained explainer of its concept (result contracts, service layers, fitness functions, ratchets).
refactor(backend): one Db type and one service-result contract for every modulerefactor(backend): decompose route monoliths into per-domain modules with service layerstest(architecture): make the module layering rules executabletest(coverage): widen the ratchet to src/modules and re-floor to the measured treedocs: the module layout is the backend convention
Tradeoffs & design decisions (flagged)
- One big motion commit rather than a commit per module. A per-module series would not compile in between (cross-module facades) and would multiply the review surface without adding reviewable meaning. The commit message and
docs/backend-architecture.mdcarry the map. lib/still holds domain-flavored code (access,audit,documentTypes,documentVersions,modelSelection,routerModels,userLookup,workflowCatalog*,sourceDocuments,lib/chat/,lib/dbq/handlers.ts). It stays because the durable-job handler registry and the assistant engine import it and rule 1 forbidslib/→modules/. Moving them means first moving the job handlers andlib/chatinto modules - named follow-ups, kept out of this PR because several open PRs editlib/chatheavily.- Two result vocabularies coexist. New services use
lib/serviceResult.ts; modules that predate it keep localkinds with identical mappings. Migrating them is mechanical and safer module by module than in one sweep. - The parity proof covers method + path + order, not status codes or bodies. That boundary is unchanged from the previous revision; the seam is covered by the untouched integration suites and by reviewer inspection. One deliberate byte-level detail: the shared mapper emits
codebeforedetail, the order the pre-existing handlers used. app.tsimports*.routesdirectly, not the facade. A facade must stay HTTP-agnostic (rule 3); the routes file is a module's second, HTTP-only entry point and onlyapp.tsmay use it (rule 2 encodes exactly that).- Known pre-existing bug, deliberately not fixed here: the project/library delete paths do not enqueue the extracted-text cache object for cleanup (the documents path does). Verified present on
main; needs its own PR.
Sequencing note
Open route-touching PRs will need re-porting onto the module layout once this lands: #247, #267/#268/#363, #346, #385, #401, #341, and #356 (which was stacked on the previous revision of this branch and must be re-stacked). The handler bodies they touch are unchanged, so each re-port is a file move plus import re-rooting; git rename detection handles most of it. I'm happy to do all of them within 48h of merge.
Maintenance
Same commitment as the other index rows: I maintain what I land - 48h triage on anything this breaks; if I go dark for 60 days, revert freely.
🤖 Generated with Claude Code
Our analysis
Split route monoliths into domain service layers — read the full analysis →
Think the analysis missed something the PR description covers?
Capture this PR into my fork
Download a Markdown prompt that tells Claude how to port every
commit in this PR into your working tree. Run it via
claude -p < capture-pull-295.md from
inside the repo you want the changes in.