fix/split-live
needs attentionb22f9f0 · fullPR #286reviewed 2026-07-09 02:13 UTC0H · 3M · 2L · 2I- Purpose
- One-shot data remediation to split sites incorrectly merged by legacy name-based dedup (feat/sites-1n razón social bug). Follow-up to PR #285 to tighten the split predicate before the prod execution.
- Goal
- Exclude terminated (historical tariff-change) contract links from the split predicate so that 69 corpse-only sites are not accidentally split into junk sites holding dead contracts.
- Sub-goals
- SG-1: Add AND uc.status <> 'terminated' to findSuspiciousSites WHERE clause so corpse-only sites are excluded from targets
- SG-2: Add same filter to splitSite inner transaction so ordering (oldest LIVE link keeps site) is consistent with detection
- SG-3: Update doc comments and runbook header to reflect LIVE-only predicate
- What
- Added AND uc.status <> 'terminated' in two SQL queries (detection + per-site split transaction). Updated doc comments. No structural changes.
- Why
- Prod analysis showed 382 raw multi-contract sites but 69 are multi-RPU only because a terminated corpse link was left behind by the pipeline's terminate-and-replace pattern. Without the filter, those 69 would be split into junk sites holding dead contracts, and a corpse that happened to be the oldest link would evict every live RPU from the original site.
- Areas
- scripts/split-merged-sites.ts+14−2
- Blast
- 1 file, +14/-2 lines. Script-only change; no domain code, no migrations, no API surface affected.
Findings · 7
correctness2
name_matches_service subquery missing terminated filter — D-org comms list may be incomplete
scripts/split-merged-sites.ts:157
The EXISTS subquery at line 157-161 that computes name_matches_service (used to assign category A/B/C/D) joins utility_contracts without AND ucn.status <> 'terminated'. A site whose name matches only a terminated contract's service_name will be classified A or C instead of B or D. Since all categories split, this doesn't affect which sites split — but it skews the census category counts and the D-org comms list (category D = Pilgrim's/Neto-style labeled sites that get a customer heads-up in runbook step 4). A D-org miscategorized as B is silently omitted from that list. Fix: add AND ucn.status <> 'terminated' inside the EXISTS WHERE clause.
UNION ALL + LIMIT 1 for site_locations copy is non-deterministic with multiple source rows
scripts/split-merged-sites.ts:241
The site_locations INSERT (lines 241-247) uses UNION ALL of (copy from source) and (fallback Mexico City row) with a bare LIMIT 1, no ORDER BY on the first branch. If a source site has more than one location row, which row is copied to the new site is undefined. In practice only timezone/country matter (address is intentionally excluded) and most sites likely have exactly one location, so real impact is probably zero. Adding ORDER BY sl.id ASC LIMIT 1 to the first UNION branch would make the copy deterministic.
security1
No security issues — hardcoded literal and parameterized CLI args are safe
scripts/split-merged-sites.ts
The new 'terminated' string is a developer-controlled literal emitted verbatim by Drizzle's sql tag — not user input, no injection risk. Existing ORG_FILTER/SITE_FILTER/LIMIT are parameterized as bind variables. The change is scope-narrowing (fewer rows touched). No new attack surface introduced.
conventions1
New SQL comments are accurate and the status literal matches the canonical enum
scripts/split-merged-sites.ts:173
CONTRACT_STATUSES in packages/database includes 'terminated' — the filter value is correct. The block comment (lines 173-179) accurately explains the terminate-and-replace pipeline pattern, the 69-site data point, and the split-from ordering hazard. Script structure is appropriate for a one-off CLI tool (FCIS/ADR-016 applies to domain entities, not scripts).
improvement3
status <> 'terminated' admits pending and suspended contracts — broader than 'LIVE' implies
scripts/split-merged-sites.ts:180
CONTRACT_STATUSES is ['pending', 'active', 'suspended', 'terminated']. The predicate AND uc.status <> 'terminated' treats pending and suspended contracts as live. Pending contracts (the DB default) are created by the CFE pipeline before CFE confirms the credential — they may never activate. A site with one active RPU + one pending RPU will be split today, creating a new site holding a pending (potentially never-to-activate) contract. The comment says 'LIVE links only' but the predicate is 'not terminated'. If the intent is strictly active contracts, change to AND uc.status = 'active'. If pending/suspended are intentionally included, the comment should say so.
No census output or snapshot for the 69 excluded corpse-only sites
scripts/split-merged-sites.ts
The PR states 69 sites are multi-RPU only because of terminated corpse links. The census log and snapshot only show the 313 targets that will be split. Operators reviewing the dry-run output have no built-in cross-check that the filter is working correctly. A second read-only query (same as findSuspiciousSites but counting raw multi-contract sites vs live-only multi-contract sites) and logging the delta would give the operator the '382 raw → 313 live, 69 excluded' confirmation in the tool's own output rather than requiring manual mental arithmetic.
Snapshot audit record omits excluded corpse-only sites
scripts/split-merged-sites.ts:332
The snapshot JSON is described as the durable audit record and rollback complement. It captures all 313 target sites but not the 69 sites evaluated and deliberately excluded. If an operator later asks whether a specific site was considered during remediation, there is no answer in the artifact. Adding a corpseOnlyCount (integer) to the totals block would close this gap with minimal overhead.