Skip to content

Repo-wide abstraction and naming (backend campaign 12) - #104

Closed
theobong wants to merge 21 commits into
chore/backend-campaign-11-sql-repositoriesfrom
chore/backend-campaign-12-abstraction-naming
Closed

theobong wants to merge 21 commits into
chore/backend-campaign-11-sql-repositoriesfrom
chore/backend-campaign-12-abstraction-naming

Conversation

@theobong

Copy link
Copy Markdown
Member

What changed

A repo-wide consolidation and naming pass, behavior-neutral. Duplicated helpers now have one home (cursors, hashing, bounded concurrency, sleep, time units, page limits, timeouts, the capped body reader, Postgres error codes, queue names, LIKE escaping, path-over-body merging, the chat/DM message core and the room fan-out wiring), the PR 11 delegate modules are gone, files and factories follow one naming convention, repository contracts live in <name>-repository.ts beside their implementations, and the 22 in-memory repositories that only tests use moved from src to test/helpers. Nothing a user sees should change.

Before you start

  • Where: staging (civfix.dev, admin.civfix.dev, the staging mobile build) after this merges to main
  • Sign in as: your own accounts, A (host) and B (attendee), with emailed codes; a throwaway account for the delete step; an operator session on admin.civfix.dev
  • Stacked on SQL into repositories (backend campaign 11) #100 (SQL into repositories). Read only this PR's diff against that branch, commit by commit.
  • Size: 13468 counted lines across 723 files. Campaign PR; the author approved PRs over the 400-line cap for this cleanup. Most of it is mechanical: 60 renames, the in-memory move (22 files plus 88 test import paths) and import-path rewrites that follow each consolidation.
  • Reading order: stage by stage, one commit per step: the cursor module; the shared lib helpers and their adoption; the timeout, capped-body, chat-core and fan-out commit; the queue-name, error-code, delegate and audit-rename commit; nine naming commits; the in-memory move; the four required worker methods; review fixes (comment accuracy, and the organization and team rules moved out of their contract files into organization-rules.ts and host-team-rules.ts).

Verify

Nothing new to verify: every step below is a regression check.

Regression

Signing in and out — [Web] [Mobile]

  1. Click "Sign in", "Continue with email", type your email and click "Send me a code"; enter the code and click "Verify code" — Expect: signed in
  2. Open your profile, the "Settings" gear, then "Sign out" — Expect: signed out

Photo reports — [Web] [Mobile]

  1. As A click "Report", add a photo, place the pin, pick a category, type a title and click "Drop pin" — Expect: "Pin dropped. We're on it."; the photo shows on the report after about 30 seconds

Chat and direct messages — [Web] [Mobile]

  1. A: open B's profile, click "Message", send a message with a photo, reply to one of B's messages and react with a like — Expect: B sees each live; the photo, reply and like chip show for both
  2. A: edit a message and delete another ("Delete message") — Expect: "(edited)" and "Message removed" for both
  3. Scroll up in a long conversation, then open "Messages" — Expect: older messages load in order; the conversation list shows the latest message and unread count

Hosting — [Web] [Mobile]

  1. A: "Host dashboard", then "Make announcement", send it — Expect: "Announcement sent." and B gets it
  2. A (web): on the "Attendees" tab click "Export", then the attendees export, then "Download" when ready — Expect: the roster CSV downloads
  3. A (web): open the event's "Check-in" tab, type B's ticket code and click "Check in" — Expect: "Checked in"
  4. A: "Host dashboard", "Team", then "Invite"; invite B as co-host with "Send the invite" — Expect: "Invite sent"

Your data and your account — [Web]

  1. Open "Settings", then "Account", then "Request my data" (once only) — Expect: "We emailed a copy of your data to ." and the email arrives
  2. With the throwaway account: "Delete account", "Email me a code", then "Permanently delete" — Expect: signed out; the account's posts show as "Deleted User"

Admin — [Admin]

  1. On the Dashboard click "Open reports", open a report and click "Send to city" — Expect: "Sent to "
  2. Click "Open moderation", approve an item — Expect: it leaves the queue
  3. Click "Review accounts", open a user and page through a long list — Expect: each tab loads; the next page continues where the previous ended

Findings addressed

Campaign PR 12 (repo-wide abstraction and naming): the GLOBAL DUP families from HELPERS.md and the ledger (isUuid, sha256, limit clamps, parseBool, sleep, bounded concurrency, time units, trailing slashes, attachment filenames, the SMTP timeout default, one cursor system, timeout races, the capped body reader, the chat/DM message core, fan-out wiring, the threads nudge, the analytics panel mapper, queue names, Postgres error codes, personViewOf, the PR 11 delegates, writeAudit, like helpers, mergeParams/mergeQuery, fragment aliases), the naming items (-repo vs -repository, .types.ts vs -types.ts, db/schema file names, the worker jobs module, make* vs build*, one place for repository interfaces, the PR 9 part files) and the in-memory repository move.

Metrics (api and media-worker src, #100's head -> this PR): source lines 123,080 -> 114,522 (about 8,600 of it the test doubles leaving src); functions 8,191 -> 7,267; functions over 100 lines 124 -> 123.

Decisions for the reviewer

  • Behavior-neutral, proven three ways: every commit typechecks on its own; the unit suites pass at the tip; and the offline SQL transcripts (the recorder from Backend test safety net: SQL transcripts, CSRF pairing, authz and characterization tests #92, run locally) show no statement change. After the consolidation stage, six transcripts differed only because the case generator picked a different member of a status union as a synthetic input; re-run with the baseline's exact inputs they are byte-identical. Every removed or renamed case has a byte-identical transcript under its new key.
  • Merges that differ only on unreachable inputs: the shared page-limit clamp now floors fractions and maps NaN or values below 1 to the default for the chat history, group member, announcement and volunteer-hours lists. Every one of those limits is validated by its request schema as an integer in range before it reaches the service, so no request can observe the difference.
  • Kept apart on purpose (they change for different reasons or differ in behavior): the push providers' prune rules, the SMS client's timeout, the truncation helpers, the two date cursor parsers (one accepts a rolled-over calendar date), the org in-memory pager (an unknown cursor restarts instead of ending), the discovery numeric cursor, the admin routes' id-validating body parser, the certificate and slot conflict checks (they also match on the error detail), and the geocoder's distance function (equal to the shared one only up to float rounding against 40/60 m thresholds).
  • Not renamed: the *-repository.drizzle.ts family (the files are raw postgres.js, not Drizzle). Renaming it touches 344 files on a repo many people push to, and the makeDrizzle* factory names would still say Drizzle; CLAUDE.md already documents the misnomer.
  • Package subpaths: every existing @civfix/api/* key the worker uses keeps working (only some targets moved); new ones: queue-names, time, concurrency, env-parsers, timeout, capped-body, and three type-only contract subpaths.
  • The in-memory move is proven by the bundles: the api and worker tsup bundles built before and after the move are byte-identical. dm-repository.memory.ts and conversation-hides-repository.memory.ts stay in src because the no-database fake mode builds them.
  • Stage 4 is one commit that carries seven non-overlapping changes (queue names, error codes, delegate removal, the audit rename, the LIKE helpers, path-over-body helpers, fragment aliases), applied by three parallel agents; it is reviewable by file but not split per change.
  • A confirmed bug found during planning is not fixed here (this PR is behavior-neutral): the admin system-health probe never clears its timeout timer, so 2 seconds after every probe it sends a Postgres cancel to whichever statement that pooled connection is running then. No screen calls the endpoint today. It is logged for a follow-up with a one-line fix and a test.
  • Naming leftovers, recorded: the message attachment, mention and reaction factories are makeMessage*Repository without the Drizzle prefix (the optional prefix pass was skipped); admin/mail-repository.ts still holds a few runtime helpers next to its contract, as it did before this PR; six build* functions read from the database while assembling a value, so the convention is recorded as "build* assembles a value and never returns a wired object" rather than "pure".
  • Left for later, listed: the audit error text still names writeAudit (the transcripts pin it); three src exports now used only by test helpers; a few test-side leftovers (?./! on the now-required worker methods, a hand copy of the mute binder in one test).

User-visible copy changes

None.

Tests changed

Only to follow moved paths or renamed symbols; no assertion changed.

  • Import paths in about 150 test files: the in-memory repositories under test/helpers (87 files), the queue names, the error codes, the renamed modules and contracts, and the removed delegates.
  • Renamed symbols: makeServer, makeContainer, makeAuthServices, makeWorker, makeJobs, makeSeams, insertAuditRow, parseKeysetCursor (two precision tests pass their requireUuid flag in the options object), makeDrizzle*Repository factories, InMemoryMediaWorkerRepository.
  • dm-pg.test.ts calls the user-search repository directly now that the social-repository wrapper is gone (same arguments, same assertion).
  • One integration test's assertion message names makeSeams instead of buildSeams.

Verification

  • Every commit typechecks on its own (api and media worker).
  • At this branch: pnpm lint, prettier check, check:sql clean; api unit suite by explicit path 428 files, 7282 passed, 3 skipped; media-worker unit 300 passed, 2 failed (the known arm64 ffprobe helper issue fixed in Backend test safety net: SQL transcripts, CSRF pairing, authz and characterization tests #92).
  • The media worker builds; its bundles contain no oslo or argon2, and the sandboxed image lane still imports nothing from the api beyond the inlined timeout module.
  • An adversarial review (five passes, one per stage) found no behavior, security or performance change; its fixes (comment accuracy, runtime rules out of two contract files, the README subpath list) are in the last two commits.
  • The integration suite was not run (it needs Docker); CI runs it once this PR is retargeted to main. CI runs only on PRs based on main, so it starts here when the PRs below merge.

Staging checks

  • https://api.civfix.dev/readyz answers ready (no migration in this PR).
  • The media worker picks up photo reports (its queue names and shared queue policy now come from one module).

Not covered

  • The admin system-health endpoint has no screen; its unchanged probes are covered by unit tests only.
  • Nightly jobs (retention, reminders, metrics rollup) are not triggered by hand; their queue names and schedules are unchanged and covered by unit tests.

🤖 Generated with Claude Code

… insertAuditRow everywhere, like helpers in db, path-over-body helpers
@byteful
byteful added this pull request to stack #122 September 24, 2026 17:53
@byteful byteful closed this Sep 24, 2026
@byteful
byteful deleted the chore/backend-campaign-12-abstraction-naming branch September 24, 2026 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants