feat/peer-price
safed44d0c5 · incrementalPR #345reviewed 2026-07-23 21:28 UTC0H · 0M · 1L · 2I- Purpose
- Extends the $-layer (monetary impact estimates) to the peer_benchmark_consumption detector, completing dollar pricing for all three energy-optimization gap-detectors (D1 capacity · D2b + peer_benchmark indicative energy).
- Goal
- Price the peer_benchmark excess with the marginal energy rate (COALESCE(NULLIF(genCost,0), genBCost)) rather than averageFee, across all tariffs with a pricing_zone_id.
- Sub-goals
- SG-19: org-scoped rate fetch via fetchContractEnergyRatesForOrg spans all tariff types (not GDMTH-only)
- SG-rate: marginal rate = genCost or genBCost fallback (not averageFee, which is 3-4× inflated for GDMTH/PDBT)
- SG-version: detector version bumped peer_benchmark_consumption@1 → @2
- SG-fix: PR #7 loop-review fixes — parallel fetch refactor + warning-band test
- What
- Fix commit (d44d0c59): (1) converts 6 sequential await fetches to Promise.all for concurrency in detectFindingsForOrgShell; (2) adds a test asserting detectPeerBenchmark fires 'warning' when deviation is between +30pp and +50pp (35pp case).
- Why
- The prior review flagged sequential reads where all 6 are independent. The warning-band test fills a coverage gap (the detection fires at two severity thresholds; only the critical band was tested before).
- Areas
- docs/energia+313−0domains/utility/src/consumption-series+862−0domains/utility/src/finding+1571−375
- Blast
- 14 files, +3188/−375 across 2 areas (docs/energia, domains/utility). Net-additive branch — no deletions outside finding refactor. Detector-only: no API surface, schema, or migration changes.
Findings · 3
conventions1
Multi-line WHY comment exceeds single-line maximum
domains/utility/src/finding/finding.shells.ts:163
The new comment spans 3 lines ('All six reads are independent (read-only, no ordering / dependency), so they run concurrently rather than serially.'). Project convention caps WHY comments at one line. Suggest: `// All six reads are independent — run concurrently via Promise.all.`
tests1
Warning-band test omits estimatedImpact assertion
domains/utility/src/finding/__tests__/finding.detectors.test.ts:387
The new test validates severity='warning' and deviationPct=35 but does not assert estimatedImpact. Since no energyRates are passed, the value is null — an explicit expect(found[0].estimatedImpact).toBeNull() would make the intent unambiguous and mirror the pattern in the existing critical-band test.
improvement1
Comment embeds a fragile fetch count
domains/utility/src/finding/finding.shells.ts:163
Saying 'All six reads' will silently become wrong when a 7th fetch is added. Count-free wording ('All reads are independent — run concurrently') is maintenance-free.