[Architecture] refactor: per-domain modules with service layers — pure motion, zero new deps (stacked on #294)

[amal66]

Row amal66#42 of the reference index (#205) — the backend reorganization the index says to take last. Stacked on #294 (durable queues): that PR rewrote routes/tabular.ts, so this one builds on it rather than conflicting with it; merge #294 first and this diff reduces to the reorganization alone.

Design (ADR)

Context. The route files are monoliths — routes/tabular.ts (1,649 lines), routes/documents.ts (1,504), routes/projects.ts (1,139), routes/user.ts (1,132) — each interleaving HTTP parsing, auth/access checks, storage IO, and business logic inline in a dozen unrelated handlers. Handlers can't be unit-tested without a live HTTP stack, and every change lands in a file where the blast radius is unclear.

Decision. Decompose each route file into a module under src/modules/<domain>/: a thin <name>.routes.ts (HTTP concerns: parse, validate, map typed results onto status codes) delegating to <name>.service.ts functions (business logic + data access, explicit db parameter, typed results — discriminated unions, never req/res). Large domains split the service into topic files behind a named-re-export facade so intra-module helpers can't leak: documents (access/upload/versions/download/edits), projects (crud/folders/documents/chats), user (profile/mfa/apiKeys/mcp/account/export), tabular (reviews/rows/extract/extractRow/generate/generateStream/chats). Streaming endpoints keep their SSE loops in the routes file — stream lifetime and client-abort handling are HTTP concerns; only their non-streaming prepare/persist logic moved. src/lib/ keeps cross-domain infrastructure only (storage, llm, chat, queue, access…); the tabular extraction files from #294 move from lib/tabular/ into modules/tabular/ so the domain lives in one place.

Consequences. Reviewability (each concern is a few-hundred-line file with one job), testability (service functions run against a fake db, no server), and a stable seam: the async extraction worker and the SSE route already call the same service functions, and future features get a module, not a bigger monolith. Cost: many more files and a big one-time diff — which is why this row is sequenced last and stacked.

Alternatives considered. Status quo (rejected: the monoliths are where changes go to be risky); splitting only the worst files (rejected: two conventions coexisting indefinitely); adopting the fork's full stack including pino/zod/OTel in the same change (rejected: organization should be reviewable as pure motion; each new dependency deserves its own decision).

Summary

src/routes/*.ts (11 files, 7,853 lines) → src/modules/{chat, project-chat, projects, documents, tabular, user, workflows, library, downloads, case-law, models}/ (56 files changed; small route files are recognized by git as renames). Endpoint surface, behavior, and dependencies are unchanged.

Changes

• 11 domain modules, each *.routes.ts + services as above; downloads, case-law, models are routes-only moves (too small for a service split — git shows them as 84–91% renames).
• app.ts — only the 11 import lines change; middleware, limiter, and mount order untouched.
• modules/tabular/ absorbs lib/tabular/ from [Architecture] feat: durable job queues (BullMQ) for conversion and tabular extraction — off by default, no Redis unless enabled #294 (git-tracked renames, 96–100% similarity) plus the route file's remaining inline helpers: tabular.reviews.ts (row building/rebuild/cell sync) and tabular.chats.ts (citation parsing + chat message building). The extraction worker's imports follow.
• Within-domain DRY only: helpers duplicated across handlers of the same domain now have one copy in their service (normalizeSharedWith in projects, the doc-fetch + access-guard sequence in documents, findSystemWorkflow in workflows). Similar-but-not-identical code was left alone rather than force-merged.
• Zero new dependencies, no logging/validation/observability frameworks, no behavior changes.

Why

For an open-source repo, file organization is contributor UX: small files with one concern are the difference between a drive-by contributor shipping a fix and giving up. This also completes the seam #294 introduced — the worker and the route provably share one extraction core because both import the same module functions.

Testing

Pure motion, verified three ways:

• Endpoint parity: the method+path multiset extracted from both trees is byte-identical — 68 endpoints (67 original + the GET /single-documents/:documentId added by [Architecture] feat: durable job queues (BullMQ) for conversion and tabular extraction — off by default, no Redis unless enabled #294's second commit), same registration order and middleware chains per module (order matters: /create before /:chatId, /ids and /prompt before /:reviewId, both verified).
• npm run build (tsc) clean.
• npx vitest run → 42 files, 523 tests passing — unchanged from the base, including the 11 route-level integration suites that exercise the real express app end-to-end, which now run through the module tree.

Handler bodies moved verbatim; the only rewrites are the mechanical seam (res.status(...) inside moved code became typed returns mapped back to the identical status/JSON in the route — all user-visible strings preserved, several verified by string-literal diff).

Provenance

Re-derived from the fork's service-layer refactor (amal66#42, running in the amal66 fork): its module boundaries, routes/service contract, and facade pattern are followed; every body comes from this repo's current code (post-#294), not the fork. Deliberately not ported from the fork's version: pino logging + request-id context, zod validation schemas, OTel hooks, magic-byte upload validation, and its supertest suites (this repo already has its own integration suites, which pass unchanged). Where fork shape and current code disagreed, current code won (kept: ?include=documents batching, spotlight-nonce chat flow, get_chats_overview RPC, manifest-signing export endpoint — none exist in the fork's version).

Sequencing note

This is the index's "merge last" row made concrete: open route-touching PRs (#247, #267/#268, #271, #285/#290, #292) will need re-porting onto the module layout once this lands — the handler bodies they touch are unchanged, so each re-port is a file-move, and I'm happy to do all of them myself within 48h of merge. If you'd rather not adopt the module layout, closing this costs nothing — nothing else depends on it.

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