← all branches

feat/kwh-price

safeviewing older commit
87725ff · incrementalPR #344reviewed 2026-07-23 16:02 UTC0H · 1M · 2L
The branch
Purpose
Add dollar-impact magnitudes to CFE energy-findings detectors so they can be prioritized alongside capacity findings
Goal
$-layer: price D2b overnight_load_factor with genBCost (estimatedImpact = gap×avgBaseKw×12×genBCost)
Sub-goals
  • SG-1: Extend ContractCapacityRate → ContractRate (add baseCostMxnPerKwh)
  • SG-2: Query fetchContractRatesForOrg resolves both capacityCost and genBCost off same latest tariff_rates row
  • SG-3: detectPerPeriodLoadFactor attaches estimatedImpact when base rate resolves
  • SG-4: detectPuntaManagement and detectPerPeriodLoadFactor share one ContractRate[] input
  • SG-5 (this commit): Loop-review fixes — LATERAL anchoring, D1 > 0 guard, type narrowing
The changes (whole branch)
What
Removed SQL predicate anchoring LATERAL on capacityCost presence (fixes potential stale-row bug where genBCost could come from an older row); added D1 > 0 guard mirroring D2b; replaced as-number casts with type predicates; added test for D2b asymmetric guard.
Why
Prior loop-review on b9685093 flagged that the LATERAL could pick different row vintages for capacityCost vs genBCost when anchored on one field's presence. This commit makes both fields read off the same unconditional latest snapshot, then validates each in TS independently.
Areas
domains/utility/src/finding+3518domains/utility/src/consumption-series+5880docs/energia+2910
Blast
~15 files, +2763/−376 cumulative vs main; all within domains/utility and docs/energia. No API contract changes, no schema migrations, no infra.
stacked PR — retarget to main after #343 lands
ci· GraphQL status check access unavailable on this tokencoderabbit· No .coderabbit.yaml present

Findings · 3

conventions1

low

Change-attribution tag `(loop-review)` in inline comment belongs in commit history

domains/utility/src/finding/finding.queries.ts

The comment ends with '(loop-review)' as attribution. Conventions require comments to explain WHY (non-obvious constraint), not who caught it — that's the commit message's job. The invariant itself ('anchoring on one field's presence could pull an older row and stale the other') stands cleanly on its own.

tests2

medium

Missing mirror test: D1 cap=0, base>0 should fall through to the default rate

domains/utility/src/finding/__tests__/finding.detectors.test.ts

The new test covers D2b's asymmetric guard (cap>0, base=0 → D2b stays unpriced). The symmetric gap is D1: when capacityCostMxnPerKw=0 but baseCostMxnPerKwh>0, detectPuntaManagement must NOT store the zero — it must fall through to DEFAULT_PUNTA_CAPACITY_RATE_MXN_PER_KW. No test covers this. The existing 'fallback' test uses an empty rates array, not a zero-cap case. A regression here is silent — removing the > 0 guard in D1 would still pass the full suite, because all existing D1 tests pass positive capacityCostMxnPerKw values. Consequence: a base-only-zone contract would price avoidable demand at $0/kW instead of $380, producing a zero-dollar finding that either fails the materiality gate silently or surfaces as a $0 critical.

low

D2b asymmetric guard test doesn't assert details.baseCostMxnPerKwh is absent

domains/utility/src/finding/__tests__/finding.detectors.test.ts:604

The new test asserts estimatedImpact=null and impactBasis=undefined, but does not check that details.baseCostMxnPerKwh is also absent. If the > 0 guard were broken and baseCostMxnPerKwh: 0 leaked into details, the existing assertions would still pass. Adding `expect(found[0]!.details.baseCostMxnPerKwh).toBeUndefined()` makes this a tighter regression guard.

History · 3 commits

  1. fcf7919safeincremental0H · 0M · 1L2026-07-23 21:00
  2. 87725ffsafeincremental0H · 1M · 2L2026-07-23 16:02current
  3. b968509needs attentionfull0H · 4M · 3L2026-07-23 15:48