claude/tarifas-missing-data-2026-d0ddc5

Finalize state has no Catch — failed finalize leaves job stuck in `running`

infra/cdk/src/stacks/services/utility/tariffs/cfe/stepfunctions.stack.ts

The Finalize Task state (which invokes the transition lambda with type='finalize') has no Catch block. Every other state that can fail routes to FailJob. If Finalize throws — DB error, malformed results payload — the SFN execution terminates with States.TaskFailed and the job row stays permanently in `running`. The 3-hour reaper is the only recovery. Fix: add Catch: [{ ErrorEquals: ['States.ALL'], Next: 'FailJob', ResultPath: '$.error' }] to Finalize.

Year/month validation skips silently when dropdown selector returns 0 options

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

Both guards use `if (availableX.length > 0 && ...)`. If CFE changes the dropdown selector so optionValues() returns [], validation is bypassed and the form POST proceeds. Zero options is itself a page-structure change that should fail loudly — the guard should throw when the selector is present but empty, or at minimum log a warning.

divisionsDiscovered=0 with empty failures masks misconfigured division names

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

When event.divisions is a non-null array matching no configured division, resolveTargets() returns [], zoneCount([]) * rateTypes.length = 0, and the collector returns { divisionsDiscovered:0, failures:[] }. The job eventually fails via decideFinalizeJob but the failures array is empty — no diagnostic for the operator. A configuration typo is indistinguishable from a real scrape failure.

alreadyFailed flag leaks control-flow semantics into the decision type

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

FailTariffJobDecision.alreadyFailed: boolean is a control-flow discriminator telling the shell to skip the DB write and event emission. The canonical-form rule says decision types describe WHAT TO WRITE — they are commands, not outcome flags. The idiomatic pattern is two distinct decision variants: one for the write path (FailTariffJobDecision) and one for the no-op path (e.g. AlreadyFailedDecision), discriminated by a `_tag` field, so the shell uses an exhaustive switch rather than an `if (decision.value.alreadyFailed)` branch.

alreadyFailed shell path returns synthetic empty event id: ''

domains/utility/src/tariff-job/tariff-job.shells.ts

When alreadyFailed is true, the shell returns event: { id: '', eventType: ... } to satisfy the ShellResult type. Any future caller that logs or forwards event.id silently processes a sentinel value. Cleaner: make event nullable (event: T | null) or use a dedicated no-op result type so callers cannot treat the phantom event as real.

resolveTargetYearMonth throws instead of returning Result

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

resolveTargetYearMonth is a pure function (no async, no I/O) that validates format and applies a rule — the exact profile of a domain decision function. The throw pattern is intentional at the service layer (so SFN Catch routes to FailJob), but the function is not tested precisely because catching in a test requires try/catch. Result would make it testable inline. Low priority: service-layer exception usage is documented.

resolveTargetYearMonth not tested — silent monthsBack coercion on money path

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

resolveTargetYearMonth() has two behavioral branches: (a) explicit yearMonth validated against YEAR_MONTH_RE — an invalid value throws; (b) monthsBack defaulting to 0 when the value is a float, negative, or non-integer — this is a SILENT coercion that targets the current month instead of failing. A misconfigured EventBridge rule sending monthsBack:-1 would quietly scrape this month. Neither branch has a test. Given that scraping the wrong month is the exact failure mode this pipeline was built to prevent, the coercion-to-0 path deserves explicit coverage.

aggregatePersistResults not tested — fan-out totals could be miscounted silently

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

aggregatePersistResults() reduces the per-rate-type Map output into a single finalize payload. Three behaviors are untested: (1) null/undefined counter fields from a failed branch (the ?? 0 coercion) — a NaN would make decideFinalizeJob classify the job status incorrectly; (2) failures[] flatMap from a branch that returns failures:undefined rather than [] (flatMap throws); (3) the new finalize path accepting results[] rather than payload. The function is pure and trivially exportable — there is no reason for zero tests on this money-correctness aggregation.

MONTH_ALT selector path (historical-year scraping) unreachable from test suite

services/utility/tariffs/cfe/__tests__/collector.test.ts

resolveMonthSelector() has two branches: MONTH (current-year pages) and MONTH_ALT (historical-year pages that render a different dropdown id). The stub always renders MONTH; the 'scrapes a published past month' test hits MONTH on a page that claims to serve 2025-12. A regression breaking MONTH_ALT resolution would look exactly like: test passes, production backfill scrapes current month's data under the historical key. The stub needs an option to emit MONTH_ALT when a past year is requested.

Year-not-published guard not tested (month-guard asymmetry)

services/utility/tariffs/cfe/__tests__/collector.test.ts

scrapeRatePage() validates the year dropdown before validating the month, but no test exercises the year-guard failure path. The stub always includes 2026/2025/2024. A regression silencing the year check would make the collector post an unpublished year and persist whatever CFE's default renders under the requested yearMonth — the exact failure mode the month test covers at the next step.

failTariffJobShell alreadyFailed no-op path lacks integration test

domains/utility/src/tariff-job/tariff-job.shells.ts

The decision-layer test verifies alreadyFailed:true for an already-failed job, but the shell's early-return (no DB write, no outbox event) has no integration test. If the shell is refactored to call update() or emit() on the alreadyFailed path, a duplicate outbox event would be written for every reaper-vs-concurrent-execution race, and the decision test would not catch it.

YEAR_MONTH_RE duplicated between lambda and domain decision

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

YEAR_MONTH_RE appears identically in tariff-transition.lambda.ts and domains/utility/src/tariff-job/tariff-job.decisions.ts. The two can drift independently. The lambda already imports from @batu/utility-domain — it could re-export the constant from the domain, or delegate yearMonth validation to decideCreateJob. A second validation site means the lambda can reject a month the domain accepts, or vice versa, silently.

PersistResultPayload = Partial<FinalizePayload> too permissive

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

FinalizePayload has several required numeric fields. Making PersistResultPayload a Partial of that means a branch returning {} is type-correct. aggregatePersistResults defends with ?? 0, but the type is semantically imprecise. A discriminated union (e.g. FinalizePayload | { failures: ...; ratesPersisted: 0; ... }) would express success vs failed-branch structurally, eliminating the ?? 0 defensive defaults and making impossible states unrepresentable.

resolveTargetYearMonth silently coerces invalid monthsBack to 0

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

A float, negative, or non-integer monthsBack is silently coerced to 0 (current month). The yearMonth path throws on malformed input — consistency demands the same loud-fail posture here. The CreateForCurrentMonth SFN door already handles the 'no monthsBack' case, so the coercion-to-0 is unnecessary defensiveness that masks caller bugs.

MaxConcurrency: 3 is a magic number

infra/cdk/src/stacks/services/utility/tariffs/cfe/stepfunctions.stack.ts

The comment explains the reasoning (each branch runs its own Imperva clearance against the same CFE host) but the value 3 is inlined. A named constant like TARIFF_FAN_OUT_CONCURRENCY = 3 with the rationale attached would make the dependency explicit and searchable when the proxy pool or rate-type count changes.