fix/bill-parity
needs attention0868341 · incrementalpre-PRreviewed 2026-08-05 22:31 UTC0H · 1M · 2L- Purpose
- Reproduce legacy CFE billing behavior exactly — a bill-parity pass to match outputs to the legacy Python pipeline
- Goal
- Fix two divergences from legacy: (1) blank-string billHeader overrides rendering empty PDF headers; (2) cross-midnight 15-min demand windows being counted when legacy drops them
- Sub-goals
- fix(bill-pdf): embed Energía Real brand as the default
- fix(bill-pdf): add Y-axis to the PDBT monthly chart
- fix(bill-pdf): legacy header + chart-format parity for the render worker
- fix(cross-domain): classify max_demand_15min by whole-window-in-band (legacy parity)
- fix(cross-domain): drop cross-midnight demand windows for full legacy per-day parity
- fix(bill-pdf): backfill billHeader `??` → `||` blank-string fallback
- test(cross-domain): lock billHeader precedence + last-of-day demand-window boundary
- What
- This incremental review covers 4 files: (1) `bill-pdf-view-model.ts` — switched 4 fields from `??` to `||` so blank backfilled strings fall through to derived values; (2) `site-energy-metrics.shells.ts` — extended the classify cache to return `{period, day}` and added `start.day === end.day` guard in the max_demand_15min loop; (3+4) two new test files pinning the header precedence logic and the midnight-boundary behavior.
- Why
- Legacy `daily-aggregation.py` groups avgDemand15min per `local_date` and masks windows with `time_minutes + 15 <= end (≤ 1440)`, so a cross-midnight window is dropped even if both instants classify to the same TOU band. The previous port missed this per-day grouping. Similarly, `billHeader` fields may be blank strings in the DB (backfilled from legacy) — `??` treats `''` as a real value, `||` treats it as fallback, matching intended behavior.
- Areas
- domains/cross-domain/src+148−8packages/bill-pdf/src+35−24packages/bill-pdf/scripts+0−0
- Blast
- 11 files, +309/−32 across the branch. Cross-domain only in this increment (4 files, +148/−8). No schema changes, no API surface changes, no shell I/O changes — pure logic and tests.
Findings · 3
tests2
`tariffReference` blank-fallback behavior is unasserted
domains/cross-domain/src/__tests__/bill-pdf-view-model.test.ts:46
The blank-override test (line 46) and partial-override test (line 55) both pass `tariffReference: ''` but neither test asserts the resulting value. With `ctx=null` and `bh.tariffReference=''`, the `||` chain evaluates to `undefined`. A regression back to `??` would return `''` instead — and this test would not catch it. Add `expect(m.tariffReference).toBeUndefined()` to both cases.
Last-of-day boundary test has no `max_demand` assertion
domains/cross-domain/src/__tests__/site-energy-metrics-breakdown.test.ts:211
The 'keeps the last-of-day window ending exactly at local midnight' test only checks `max_demand_15min`. The first cross-midnight test correctly pairs a `max_demand` assertion to confirm the same-day guard doesn't suppress instantaneous peak. The second test should mirror this pattern to guard against regressions that accidentally drop raw demand too.
improvement1
Single-use `res` variable in `classify` is noise
domains/cross-domain/src/site-energy-metrics.shells.ts
The `const res = { period, day }` is assigned and immediately stored and returned. Inline `{ period, day }` into both `classifyByTs.set(key, ...)` and the return to remove the indirection — or capture the set-then-return in one step.