← all branches

fix/dup-repair

needs attentionviewing older commit
8e6343e · incrementalPR #306reviewed 2026-07-14 04:45 UTC0H · 1M · 3L · 1I
The branch
Purpose
One-shot operational data-remediation script to repair duplicate-contract damage from the June-2026 CFE cross-account contamination (#231) — 67 contamination twins deposing real contracts, 18 inverted replacements, and 12 foreign XML bills left unrepaired after the #231 bill deletion.
Goal
Repair 3 classes of duplicate-contract damage by applying the newest-CFE-evidence rule: flip the evidence holder to active, soft-delete bill-less window-created twins, terminate real-history rows, re-point SUCs + subscriptions, and sweep 12 foreign bills. Idempotent re-runs confirmed on staging (3/3 groups).
Sub-goals
  • SG-1: Pure classification core (repair-dup-contracts-core.ts) — decide verdict per dup group, unit-testable
  • SG-2: Shell script (repair-dup-contracts.ts) — dry-run default, typed confirmation, per-group FOR UPDATE transactions, verify pass
  • SG-3: Foreign-bill sweep — delete 12 #231-residue xml bills, emit S3 manifest for follow-up aws s3 rm
  • SG-4: Test suite polish — round-4 reason/holder assertions + active-only SKIP_MANUAL case + Number() coercion guard
The changes (whole branch)
What
Round-4 polish commit: (1) added reason and holder assertions to the existing 'SKIP_MANUAL: two active rows' test for stronger classifier coverage; (2) added new 'SKIP_MANUAL: active-only group (zero terminated rows)' test case covering the terminated.length===0 branch of classify(); (3) added Number() coercion to twinBillCount query to guard against postgres.js returning count(*) as a string.
Why
The reason/holder assertions pin the exact early-return shape so a future classifier refactor can't silently drop them. The active-only case was a missing branch in the test matrix (the guard catches 3 conditions but only 2 were tested). The Number() coercion fixes a latent runtime bug where postgres.js surfaces count(*) as a string and downstream arithmetic would silently NaN.
Areas
scripts/__tests__/repair-dup-contracts-core.test.ts+10−0scripts/repair-dup-contracts.ts+3−0
Blast
3 files total on the branch (+887/−0 lines); pure scripts/ — no domain code, no API surface, no migrations. Zero blast radius on running systems; one-shot script not yet executed on prod.
one-shot operational script — retire after prod run requires PR #298 live before prod execution
➖ci· CI checks not accessible via current token➖coderabbit· no .coderabbit.yaml present

Findings · 5

conventions1

low

Multi-line comment block (3 lines) violates comment convention

scripts/repair-dup-contracts.ts:303

Lines 303–305 form a three-line consecutive comment block. Project conventions allow single-line comments only when the WHY is non-obvious, but prohibit multi-line comment blocks. Could be collapsed to one line, e.g. `// {rows} vs bare-array + Number() guards string-typed count(*) (same pattern as findDupGroups)`.

tests2

low

Active-only SKIP_MANUAL test omits holder-undefined assertion

scripts/__tests__/repair-dup-contracts-core.test.ts:168

The existing 'SKIP_MANUAL: malformed group (two active rows)' test now asserts both reason and that holder is undefined, but the new active-only test only asserts verdict and reason. For symmetry, it should also assert plan.holder is undefined — confirming the early-return shape is correct.

info

No unit test for Number() coercion of string count from postgres.js

scripts/repair-dup-contracts.ts:303

The Number() guard for count(*) returned as a string is documented but untested. Acceptable for a one-shot operational script. If a unit harness is ever added for repairGroup helpers, a test with n: '3' (string) confirming twinBillCount === 3 (number) would close the gap.

improvement2

medium

Driver-compat extraction duplicated rather than centralized

scripts/repair-dup-contracts.ts:303

The comment itself says 'same dual-path handling as findDupGroups' — the dual-path normalization (rows ?? bare-array + Number() coercion) is now a copy-paste pattern in at least two places. Extracting a small helper like extractCount(result) would eliminate the duplication and the need to repeat the explanation comment at each call site. Low urgency given the one-shot nature of this script.

low

Reason string literal duplicated across two test cases

scripts/__tests__/repair-dup-contracts-core.test.ts:161

Both the existing 'SKIP_MANUAL: multiple active rows' test and the new 'SKIP_MANUAL: active-only group' test assert the same reason string 'group is not 1-active + N-terminated'. A shared constant (e.g. NOT_1_ACTIVE_REASON) would make the coupling explicit and reduce drift if the string changes.

History · 8 commits

  1. 80e0f62safeincremental0H · 0M · 1L2026-07-16 19:20
  2. d513853needs attentionincremental0H · 2M · 3L2026-07-16 19:13
  3. 3de8c23safeincremental0H · 1M · 3L2026-07-14 14:23
  4. 8af2fabsafeincremental0H · 0M · 2L2026-07-14 14:16
  5. 8e6343eneeds attentionincremental0H · 1M · 3L2026-07-14 04:45current
  6. 57eb1ebneeds attentionincremental0H · 2M · 4L2026-07-14 02:41
  7. a29f8f4needs attentionincremental0H · 4M · 6L2026-07-14 02:33
  8. 27179efneeds attentionfull5H · 4M · 3L2026-07-14 02:15