Skip to content

refactor(batch): collapse the batch family to one implementation over the store seam - #108

Merged
andrei-hasna merged 1 commit into
mainfrom
feat/9e1ea854-batch
Jul 27, 2026
Merged

refactor(batch): collapse the batch family to one implementation over the store seam#108
andrei-hasna merged 1 commit into
mainfrom
feat/9e1ea854-batch

Conversation

@andrei-hasna

@andrei-hasna andrei-hasna commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

What this is

OPE-MODE-P4, one family: src/lib/batch. Both arms deleted, one implementation left
behind the unchanged facade path.

src/lib/batch.ts was a nine-line facade that read the process-wide deployment mode and
handed batchSend to one of two modules. batch.local.ts (90 lines) rendered a template
per CSV row, checked suppression, sent through the provider adapters and wrote the
sent-mail ledger. batch.remote.ts (33 lines) threw. Same name, same signature, opposite
meanings — an environment variable decided whether the operation existed at all. That is
the axis this program deletes, and it is now gone from this family.

What replaces the arm choice is the store's own answer. The template is read through
EmailStore.templates with a name equality filter. The local SQLite store filters and
serves the row. A store that cannot answer "the template named X" refuses, and
batchSend throws that refusal with its code, status and message. A refusal is never
reported as { total: n, sent: 0, failed: 0, errors: [] }, because that is the correct
answer for an empty CSV and must not also be the answer for "this installation could not
be asked".

What this PR deliberately does NOT do

Each of these would either rebuild the axis under a new name or do a later phase's work:

  1. Nothing asks which store it holds. src/store-resolution.ts states that
    construction is the only place that answer is visible and that its plan union is not a
    runtime label for callers to branch on
    . A branch here on the store kind, or on
    descriptor, would be the same switch respelled — so there is none.
  2. The send does not go through the seam. SendIntentsRepository is gated on the
    idempotency-fenced ledger and both stores declare that capability false
    (src/store-sqlite/index.ts, src/store-http/index.ts); outboundPolicy is false on
    both as well. Routing the send there would refuse in every configuration and break
    batch sending outright. One send service is phase 8.
  3. Suppression, the send counters and the template placeholder grammar are not
    reimplemented.
    ContactsRepository/TemplatesRepository are generic
    ResourceRepository<ResourceRow> CRUD; copying the canonical-address logic or the
    {{name}} syntax into batch would create a second source of truth for who may be
    mailed while src/db/contacts and src/db/templates are still phase-4 families of
    their own.

For (2) and (3) batch imports each family's facade, never an arm — reaching into a
.local module is exactly what the deleted batch.local.ts did to four separate
families. Stated plainly so no reviewer has to infer it: those facades still dispatch on
the axis internally, so this module's dependency on the axis is transitive, not gone.

What is gone is batch's own mode decision. This PR does not touch the axis module, the
dispatch layer, the curl bridge, or any *.local.* mode-gated branch — all of which are
phase 9's, and only once the ratchet reads zero.

Changes outside my family — flagged loudly, as required

Two consumers reached into the arm rather than the facade, so deleting the arm breaks
them. Both are one-line import-specifier swaps and nothing else:

  • src/cli/commands/misc.local.ts:375 — production. The emails batch command's
    dynamic import moves from ../../lib/batch.local.js to ../../lib/batch.js. This file
    is the misc family (phase 5), not mine and not a sibling agent's. Its printed
    BatchResult fields are unchanged.
  • src/cli/commands/send-suppression.test.ts:413 — the same swap in a test.

No other file outside src/lib/batch.* is touched. src/store/ is byte-identical to
main
(git diff origin/main -- src/store/ produces zero lines).

batchSend also stops threading an explicit database handle into the contacts, send and
ledger calls. In a local configuration that is behaviour-identical — the facades default
to the same memoised connection — and it stops this module from opening a local SQLite
database under a configuration that names an API.

One additive public-type change: BatchSendOptions is now a named export of
src/lib/batch.ts (previously the options were an inline type on the function, re-exported
from the arm via export type *). BatchResult and the runtime surface (batchSend,
parseCsv) are unchanged, as is the facade's import path.

(a) Ratchet delta — all eleven metrics

Measured over the real git ls-files corpus, before on origin/main (2142932) and
after on the staged tree, using scripts/mode-axis-ratchet-lib.mjs itself:

Metric before after delta
twoArmFamilies 43 42 −1
remoteArmModules 43 42 −1
routedFacadeDefinitions 30 30 0
routedCallExpressions 293 293 0
selfHostedResourceBranches 47 47 0
selfHostedResourceReferences 203 203 0
isSelfHostedModeReferences 70 70 0
getEmailsModeReferences 78 76 −2
resolveEmailsModeReferences 73 73 0
normalizeEmailsModeReferences 16 16 0
emailsModeEnvReferences 239 239 0

Corpus: 670 tracked / 669 scanned / 8,369,059 chars before; 668 / 667 / 8,387,638 after.
Both are well clear of the 500-file and 5,000,000-char floors.

No metric rose. The two required structural drops are both present. The dispatch-helper
counters do not move because batch was one of the thirteen facades that dispatched
inline rather than through the helper, so it never held a helper definition or a helper
call site; the −2 on the process-wide mode read is batch.ts's import plus its single call.

(b) New tests that FAIL on unmodified main: 9 of 17

Verified the required way: a pristine origin/main worktree with only
src/lib/batch.test.ts copied in and nothing else changed, then
bun test src/lib/batch.test.ts9 fail / 8 pass.

The nine:

  1. ships no second implementation arm beside the facade
  2. reaches past no facade into an arm, and reads no deployment-mode module
  3. no longer reaches into an arm from the CLI command that runs a batch
  4. throws the store's refusal instead of reporting a batch that sent nothing
  5. does not mail anything when the store refuses
  6. refuses to guess when a filtered read answers with a row of another name
  7. refuses an ambiguous name rather than picking one of the matches
  8. refuses a stored subject that is not text rather than mailing its coercion
  9. refuses to run when the configuration names both a local database and an API

The other eight are declared parity coverage, not change detectors: the five
parseCsv cases, "renders the stored template and mails every row over the configured
store", "skips a suppressed row and counts it", and "reports a template the store does not
hold as not found". They exist to prove the collapse did not lose the local arm's
behaviour, and they are labelled as such rather than counted as evidence of change.

Case 4 is the one that matters most: it asserts the result is not a BatchResult at
all, because a refusal reported as zero sends and zero errors is indistinguishable from an
empty CSV. Case 5 additionally asserts nothing was mailed and nothing was ledgered.

(c) Suite totals

before (origin/main 2142932) after (this branch)
bun run test 2714 pass / 143 skip / 0 fail — 2857 tests, 236 files 2725 pass / 143 skip / 0 fail — 2868 tests, 236 files
src/lib/batch.test.ts 6 tests 17 tests

Also clean on this branch: bunx tsc --noEmit (silent), bun run no-cloud:source
(26 pass / 0 fail), bun run build then bun run no-cloud:pack (786 text files /
13,055,367 bytes scanned, no markers — well above the pack guard's 100-file and
1,000,000-byte floors).

Adversarial self-review — what it found

Run before opening, hunting the four named classes. It was not empty:

  • A test that would still pass with the production change reverted — FOUND AND FIXED.
    "refuses to guess when a filtered read answers with a row of another name" originally
    passed on main. With _store injected but ignored (as main ignores it), the local arm
    looked tpl up in SQLite, found nothing, and threw the very same
    Template not found: tpl the test asserted. It now seeds a real, usable tpl row in
    the database while the injected store answers with another name's row: on main the batch
    finds the real template and sends, so the rejection assertion fails. That moved the
    fail-on-main count from 8 to 9.
  • A guard I nearly weakened to get green — FOUND AND FIXED PROPERLY. My first
    structural assertion banned every *.local.js import from batch.ts and immediately
    went red on ./sent-ledger.local.js. That module has no facade and no second arm, so
    importing it bypasses no dispatch. Rather than exempt it by name — which would have been
    weakening the guard to make the test pass — the predicate now resolves each suffixed
    specifier and only flags one whose family also ships a facade, i.e. the actual hazard of
    skipping a dispatch other callers go through. It carries positive controls in both
    directions (./batch.local.js and ../db/contacts.local.js must be flagged;
    ./sent-ledger.local.js and ./csv.js must not), checked against the predicate rather
    than against repo state, per trap feat: add BrowserPlan Mailery address contract #2 in the plan.
  • An operation returning empty instead of refusing — none found. Every non-ok
    outcome from the seam throws; there is no path from a refusal to a BatchResult. A
    non-string subject_template throws rather than being coerced, so a stored value that
    is not text can never be mailed as its String(...) rendering.
  • A collapsed path that silently picks one arm's behaviour — checked, and the remote
    arm's meaning survives without a mode read.
    Under an API configuration the HTTP
    store reads the service's published contract, finds that /v1/templates accepts no
    name filter, and refuses rather than answering with the unfiltered list — so batch
    fails loudly before any send is attempted. The wording changed (it now names the
    missing route instead of saying "not available in the self-hosted client"); nothing in
    the tree matched the old string, checked by grep.
  • A consumer fixed by widening a type — none. The two arm-importing consumers were
    repointed to the facade, not adapted. src/index.ts's Parameters<BatchModule["batchSend"]>
    re-export still resolves; src/cli/commands/sequences.ts's parseCsv import is
    untouched; the three module-list assertions in src/index.test.ts and the two
    startup-contract tests reference the facade path, which is unchanged.
  • First-run failure worth recording rather than hiding: the seam test initially failed
    with FOREIGN KEY constraint failed because emails.provider_id is a real foreign key
    and my provider id was invented. The provider is now minted through the seam. That
    failure was the loop's per-row error path working correctly.

Not done on purpose

docs/PLAN-MODE-REMOVAL.md §4.3 still lists 13 two-arm families under src/lib/
(it is now 12). The doc states its numbers were measured on one commit and to re-measure
before trusting them, and three sibling agents are collapsing three other families
concurrently — a four-way edit of the same tables would be conflict for no accuracy gain.
Worth a single follow-up that re-measures the whole table once the parallel P4 batch has
landed.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

… the store seam

`src/lib/batch.ts` was a facade that read the process-wide deployment mode and
handed `batchSend` to one of two modules. `batch.local.ts` rendered a template
per CSV row, checked suppression, sent through the provider adapters and wrote
the sent-mail ledger. `batch.remote.ts` threw. Same name, same signature,
opposite meanings, with an environment variable deciding whether the operation
existed at all. Both arms are deleted and one implementation remains behind the
unchanged facade path.

What replaces the arm choice is the store's own answer, not a mode word: the
template is read through `EmailStore.templates` with a `name` equality filter.
The local SQLite store filters and serves the row; a store that cannot answer
that question refuses, with the typed refusal's code, status and message, and
`batchSend` throws it loudly. A refusal is never reported as a batch that sent
nothing and hit no errors, because that is the correct answer for an empty CSV.

Deliberately NOT done here, since each would rebuild the axis or do another
phase's work:

  * nothing asks WHICH store it holds. `src/store-resolution.ts` says
    construction is the only place that answer is visible and that its plan
    union is not a runtime label to branch on; a branch on the store kind or
    the descriptor would be the same switch respelled.
  * the SEND does not go through the seam. `SendIntentsRepository` is gated on
    the idempotency-fenced ledger and both stores declare that capability
    false, so routing the send there would refuse in every configuration. One
    send service is phase 8.
  * suppression, the send counters and the template placeholder grammar are not
    reimplemented. They stay with the contacts and templates families, each a
    phase-4 migration of its own, and are reached through their FACADES rather
    than their arms — which is what the deleted `batch.local.ts` did to four
    separate families.

The dependency on the axis is therefore transitive, not gone. What is gone is
batch's own mode decision.

Two consumers reached into the arm rather than the facade and are repointed:
`src/cli/commands/misc.local.ts` (the `emails batch` command) and
`src/cli/commands/send-suppression.test.ts`. Both are one-line specifier swaps.

`batchSend` no longer threads an explicit database handle into the contacts,
send and ledger calls. In a local configuration that is behaviour-identical —
the facades default to the same memoised connection — and it stops this module
from opening a local database under a configuration that names an API.

Ratchet, measured over the real `git ls-files` corpus before and after staging
(670/669 files and 8,369,059 chars before; 668/667 and 8,387,638 after, both
well above the floors):

  twoArmFamilies                43 -> 42  (-1)
  remoteArmModules              43 -> 42  (-1)
  routedFacadeDefinitions       30 -> 30
  routedCallExpressions        293 -> 293
  selfHostedResourceBranches    47 -> 47
  selfHostedResourceReferences 203 -> 203
  isSelfHostedModeReferences    70 -> 70
  getEmailsModeReferences       78 -> 76  (-2)
  resolveEmailsModeReferences   73 -> 73
  normalizeEmailsModeReferences 16 -> 16
  emailsModeEnvReferences      239 -> 239

No metric rose. The dispatch-helper counters do not move because batch was one
of the thirteen facades that dispatched inline and never held the helper.

`src/store/` is byte-identical to main.
@andrei-hasna
andrei-hasna merged commit 535ec70 into main Jul 27, 2026
3 checks passed
andrei-hasna added a commit that referenced this pull request Jul 27, 2026
`<=` leaves slack a regression can walk back through: a ceiling above the
live count is a licence to re-add exactly as many branches as the gap is
wide with the guard still green. The delivery-doctor collapse lowers six
counts, and two earlier collapses left slack behind — #108 left two
structural metrics one wide each and the process-wide-read metric two
wide, and #83's note already recorded the tree-wide variable metric three
under its ceiling. All eleven ceilings are now exactly the count measured
over this commit's git ls-files corpus.

Verified two ways, because a ceiling that does not bite is worse than
none: the added prose moves no count (the identifiers are named by role
and file:line, as the plan requires of anything inside this corpus), and
staging a single tracked occurrence of the deployment variable turns the
ratchet red at 240 against 239.
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.

1 participant