From 883f0142c7c58f3acc868eb6415bc3892bde849e Mon Sep 17 00:00:00 2001 From: Chris Troutner Date: Fri, 28 Aug 2026 10:16:20 -0700 Subject: [PATCH] Review and harden ZMQ DB backups: centralize backup decision, kill mutation survivor By architect. --- docs/reviews/zmq-db-backups-summary.md | 48 +++++++++++++++++++ psf-memo-indexer/psf-memo-block-indexer.js | 7 ++- .../specs/zmq-mode-db-backups.feature | 4 ++ psf-memo-indexer/src/use-cases/backup-db.js | 4 ++ .../test/unit/use-cases/backup-db.unit.js | 8 ++++ 5 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 docs/reviews/zmq-db-backups-summary.md diff --git a/docs/reviews/zmq-db-backups-summary.md b/docs/reviews/zmq-db-backups-summary.md new file mode 100644 index 0000000..c4727a6 --- /dev/null +++ b/docs/reviews/zmq-db-backups-summary.md @@ -0,0 +1,48 @@ +# zmq-db-backups — Architect Review Summary + +**By architect.** + +## Task and commits reviewed +- Task: `zmq-db-backups` (refactorer handoff, `merge_and_process refactorer 89731964de`) +- Merged `swarmforge-refactorer` (fast-forward) — commits: + - `edcb8c8` Spec ZMQ-mode DB backups every epoch + - `2d193c6` Implement ZMQ-mode DB backups every epoch blocks (coder) + - `8973196` Refactor ZMQ DB backups: add property coverage and constructor test (refactorer) + +## Architectural findings and fixes +- **Cohesion / DRY (fixed):** In `psf-memo-block-indexer.js` the IBD catch-up path + duplicated the backup decision (`if (nextBlockHeight % epoch === 0)`) that the + `BackupDb.maybeBackupDb` use case already encapsulates, while the ZMQ live path + correctly delegated the decision. Removed the redundant guard and now use the + use case's boolean return to decide whether to log. The decision now lives in + exactly one place. +- **Boundaries (confirmed good):** `BackupDb` (use case) is a pure decision + function depending only on the `dbCtrl.backupDb` adapter interface; the HTTP + IO lives in `src/adapters/backup-db.js`. The use case is fully testable without + IO. No framework/persistence structures leak across the boundary. +- **Hardening (added):** Added a unit test for the `height === 1` boundary to kill + a surviving mutation (`height > 0` → `height > 1`). + +## Verification results +- **Language mutation** (`mutate4javascript`, `--max-workers 8`): `src/use-cases/backup-db.js` + — **6 killed, 0 survived, 0 uncovered** (after adding the `height === 1` test). +- **CRAP** (`crap4javascript`): `BackupDb.maybeBackupDb` CC=3, 100% cov, CRAP=3.0 + (threshold 8.0). Pass. +- **DRY** (`dry4javascript`): no new duplication from this task. Pre-existing + duplicates in `rpc.js` and `set-name/set-profile/set-profile-pic` are out of + scope for this handoff. +- **Soft Gherkin acceptance mutation** (`gherkin-mutator --level soft`): 11 killed, + **4 survived — all documented equivalents**. The survivors (m8, m9, m14, m15) + mutate height/epoch values in the two "0 backup" example rows to other + non-multiple values; the observable result (0 backups) is unchanged, so the + scenarios cannot distinguish them. No action needed. + +## Suite status +- `psf-memo-indexer` unit: **54 passing** (was 53; +1 new boundary test) +- `psf-memo-indexer` property: **4 passing** +- `psf-memo-indexer` acceptance: **3 generated files, all passing** +- `psf-memo-indexer` lint (`standard`): clean + +## Handoffs sent +- `git_handoff` to coder and refactorer (`priority: 00`) with the review commit + for follow-up review. diff --git a/psf-memo-indexer/psf-memo-block-indexer.js b/psf-memo-indexer/psf-memo-block-indexer.js index 9a0fc35..fc4a138 100644 --- a/psf-memo-indexer/psf-memo-block-indexer.js +++ b/psf-memo-indexer/psf-memo-block-indexer.js @@ -48,9 +48,12 @@ async function start () { process.exit(1) } - if (nextBlockHeight % config.dbBackupEpoch === 0) { + const backedUp = await useCases.backupDb.maybeBackupDb( + nextBlockHeight, + config.dbBackupEpoch + ) + if (backedUp) { console.log(`Creating DB backup at block ${nextBlockHeight}`) - await useCases.backupDb.maybeBackupDb(nextBlockHeight, config.dbBackupEpoch) } biggestBlockHeight = await queue.addToQueue(adapters.rpc.getBlockCount, {}) diff --git a/psf-memo-indexer/specs/zmq-mode-db-backups.feature b/psf-memo-indexer/specs/zmq-mode-db-backups.feature index bb1ad72..91bf9f2 100644 --- a/psf-memo-indexer/specs/zmq-mode-db-backups.feature +++ b/psf-memo-indexer/specs/zmq-mode-db-backups.feature @@ -1,3 +1,7 @@ +# acceptance-mutation-manifest-begin +# {"version":1,"tested_at":"2026-08-28T17:15:12.126220806Z","feature_name":"ZMQ mode DB backups","feature_path":"/home/trout/work/psf-memo/.worktrees/architect/psf-memo-indexer/specs/zmq-mode-db-backups.feature","background_hash":"dbd9928c8a005b3b4846976d2144892926158c50a05868292fc9cdc0a03ef735","implementation_hash":"unknown","scenarios":[]} +# acceptance-mutation-manifest-end + # Scenarios: ZMQ mode DB backups - 1 Feature: ZMQ mode DB backups diff --git a/psf-memo-indexer/src/use-cases/backup-db.js b/psf-memo-indexer/src/use-cases/backup-db.js index 830d689..dcdb6dc 100644 --- a/psf-memo-indexer/src/use-cases/backup-db.js +++ b/psf-memo-indexer/src/use-cases/backup-db.js @@ -25,3 +25,7 @@ class BackupDb { } export default BackupDb + +// mutate4javascript-manifest-begin +// {"version":1,"tested_at":"2026-08-28T17:14:22.022Z","module_hash":"5bb4a36a5e28a1be19c5e544518576bafd81b30df24db30b8ca64690f300c27c","functions":[{"id":"func/BackupDb.constructor","name":"BackupDb.constructor","line":6,"end_line":12,"hash":"50c4a5ffabb1fe5848ab489c49aeebca4e8a808fd773ab9e603fc55d85fff08d"},{"id":"func/BackupDb.maybeBackupDb","name":"BackupDb.maybeBackupDb","line":14,"end_line":24,"hash":"0fc3ba6a7f6cfc52b05103e7ff164318d0a11f56372d307a2e6f3c5220df0a59"}]} +// mutate4javascript-manifest-end diff --git a/psf-memo-indexer/test/unit/use-cases/backup-db.unit.js b/psf-memo-indexer/test/unit/use-cases/backup-db.unit.js index 034f833..83008df 100644 --- a/psf-memo-indexer/test/unit/use-cases/backup-db.unit.js +++ b/psf-memo-indexer/test/unit/use-cases/backup-db.unit.js @@ -56,6 +56,14 @@ describe('#BackupDb', () => { assert.equal(adapters.dbCtrl.backupDb.callCount, 0) }) + it('should request a backup at height 1 with epoch 1', async () => { + const result = await uut.maybeBackupDb(1, 1) + + assert.equal(result, true) + assert.equal(adapters.dbCtrl.backupDb.callCount, 1) + assert.deepEqual(adapters.dbCtrl.backupDb.firstCall.args, [1, 1]) + }) + it('should support a configurable epoch smaller than 1000', async () => { const result = await uut.maybeBackupDb(500, 500)