← all branches

fix/tariff-cov

needs attention
1ed035e · incrementalpre-PRreviewed 2026-08-11 02:03 UTC0H · 2M · 5L · 1I
The branch
Purpose
Fix the CFE tariff rate collection pipeline: silent data loss, wrong zone pairing, and pipeline stranding bugs accumulated across several incidents
Goal
Restore correct CFE tariff rates in the DB so billing is computed on live prices, not stale pre-drop values; prevent pipeline stalling that holds the month slot until the 3h reaper
Sub-goals
  • SG-1: Fix cross-month component drop: write+warn instead of refuse (prevents permanent zone freeze)
  • SG-2: Fix Valle de México zone pairing: name-based not positional (prevents NORTE/CENTRO/SUR transposition)
  • SG-3: Fix sanitizePipelineError O(n²) backtracking: truncate first (prevents transition Lambda timeout stranding job running)
  • SG-4: Move pipeline-definition.ts to infra/cdk: remove CDK→service import violation, enable structural ASL tests
  • SG-5: Add test suite coverage: decisions, ASL structure, real CFE page fixture, sanitizer edge cases
The changes (whole branch)
What
tariff-rate.decisions.ts decideScrapedRateWrite now distinguishes same-month (refuse) vs cross-month (write+warn) component drops; pairTablesToZones added to tariff-collector.lambda.ts for heading-based zone pairing; sanitizePipelineError moves truncation first and adds IPv6/AWS-key/user redaction; pipeline-definition.ts moved from service package to infra/cdk with a new vitest test runner; comprehensive test suite added
Why
Three separate silent failures were observed in production: billing on stale pre-drop rates (zone frozen by cross-month refuse), wrong zone rates (DL/DM billed on Sur's prices), and job stranded running after the transition Lambda timed out redacting a large stack trace
Areas
services/utility/tariffs/cfe+2148233infra/cdk/src/stacks/services/utility/tariffs/cfe+660291domains/utility/src/tariff-rate+66899domains/utility/src/tariff-job+27725apps/platform/src/api+1504packages/api/src+1074.claude/rules+1650
Blast
38 files, +5377/-503 across CFE tariff service, infra CDK, utility domain, platform API. No DB migrations, no API shape changes. All changes internal to the tariff pipeline.
no-pr-yet pre-pr-branch
typecheck· CI not run on pre-PR branchtests· CI not run on pre-PR branchcoderabbit· No .coderabbit.yaml configured

Findings · 8

correctness2

medium

pairTablesToZones silently falls back to positional pairing on partial labelling

services/utility/tariffs/cfe/src/handlers/tariff-collector.lambda.ts

When SOME (not all) tables have <h3> headings, labelled.length < tables.length so the name-based branch is skipped silently and positional pairing runs. This is the original transposition hazard — if CFE reorders tables or partially migrates its template, two of three Valle de México zones get each other's prices with no failure recorded. The code comment documents only the all-or-nothing expectation; partial labelling is not handled.

low

describeSkip warning message names the wrong month

services/utility/tariffs/cfe/src/handlers/tariff-persist.lambda.ts

The wrote:true message reads 'CFE published ${skip.baselineYearMonth} WITHOUT X' — but baselineYearMonth is the PRIOR month (the baseline). The intended meaning is 'the NEW month's publication dropped X relative to the prior month'. An operator triaging a RATE_SHAPE_CHANGED alert at 3am reads this as 'the old month is missing X' and investigates the wrong period.

security1

low

sanitizePipelineError does not redact /var/runtime/ Lambda paths

services/utility/tariffs/cfe/src/handlers/tariff-transition.lambda.ts

The path pattern /(var\/task|opt|tmp)\/\S+/g does not cover /var/runtime/ (appears in every Lambda stack trace as /var/runtime/index.mjs:NNN). These paths are written to tariff_jobs.result JSONB visible to tenants. No credentials exposed, but runtime file paths are leaked. Fix: extend to (var\/task|var\/runtime|opt|tmp).

tests1

low

valle-real-page.test.ts does not cover partial-labelling branch

services/utility/tariffs/cfe/__tests__/valle-real-page.test.ts

pairTablesToZones has three reachable branches: (1) all tables labelled → name-match, (2) zero tables labelled → positional, (3) some tables labelled — falls through to positional silently. The test covers (1) and (2) but not (3). The partial-labelling case is exactly the scenario where name-based pairing would have been useful but is skipped. A synthetic fixture with mixed labelling would close this gap.

improvement4

medium

ScrapedRateSkip reused as warning type for rows that WERE written

domains/utility/src/tariff-rate/tariff-rate.decisions.ts

ScrapedRateWrite.warning is typed as ScrapedRateSkip | null, and PersistScrapedRatesResult.warnings is ScrapedRateSkip[]. A type named 'Skip' used for anomalous writes is semantically wrong — wrote:boolean is a compensating patch jammed into a refusal type. Consider a separate ScrapedRateAnomaly type for the ComponentRegression warning path so skips are always non-writes and anomalies are always writes.

low

auditedYearMonth parameter name diverges from pipeline vocabulary

domains/utility/src/tariff-rate/tariff-rate.decisions.ts

The pipeline consistently uses yearMonth for 'the month being processed'. The new parameter is named auditedYearMonth — 'audited' is a finance/QA term not used elsewhere. The shell passes input.yearMonth; the ASL uses yearMonth. Consider renaming to yearMonth or targetYearMonth for consistency with the surrounding code.

low

2000-char intermediate slice security rationale undocumented

services/utility/tariffs/cfe/src/handlers/tariff-transition.lambda.ts

The comment justifies the 2000-char pre-truncation only via performance (prevents O(n²) backtracking). The security reason is also load-bearing: a credential at positions 450–530 would survive the 500-char final slice but would be missed if truncation ran first. The 2000-char window ensures redaction runs over the credential before it is cut off. The comment should explain both reasons.

info

PERSIST_RESULT_KEYS coupling to PersistScrapedRatesResult undocumented

infra/cdk/src/stacks/services/utility/tariffs/cfe/pipeline-definition.ts

PERSIST_RESULT_KEYS is hand-maintained alongside the ASL. Its values must match PersistScrapedRatesResult field names in tariff-rate.shells.ts — but infra/cdk cannot import service types, so this is a manual constraint. A comment naming the shell type as the source of truth would catch renames at code-review time without requiring a structural change.

History · 14 commits

  1. 1ed035eneeds attentionincremental0H · 2M · 5L2026-08-11 02:03current
  2. fcbe80dblockedfull6H · 12M · 14L2026-08-10 22:02
  3. 92353bdblockedincremental1H · 9M · 7L2026-08-10 19:32
  4. 5f2213eneeds attentionincremental1H · 6M · 10L2026-08-09 05:35
  5. c30da44needs attentionincremental1H · 4M · 2L2026-08-09 04:54
  6. 5940f56needs attentionincremental0H · 3M · 7L2026-08-07 19:15
  7. f222512needs attentionfull2H · 9M · 15L2026-08-07 18:58
  8. eec3b04needs attentionincremental2H · 1M · 4L2026-08-07 18:27
  9. 0b43396needs attentionincremental0H · 2M · 6L2026-08-07 01:39
  10. 3eb9789needs attentionincremental0H · 3M · 5L2026-08-06 18:30
  11. 87fc06aneeds attentionincremental0H · 4M · 9L2026-08-06 01:21
  12. 76bcc76needs attentionincremental0H · 1M · 5L2026-08-06 00:51
  13. 8948608needs attentionincremental0H · 3M · 4L2026-08-05 23:49
  14. e44f6bbneeds attentionfull3H · 6M · 9L2026-08-05 19:29