fix/bill-parity
needs attentionviewing older commit2b602ef · fullpre-PRreviewed 2026-07-31 17:41 UTC2H · 3M · 6L · 2I- Purpose
- Restore legacy-parity in the bill PDF header for mall-tenant sites where relational fields (site_locations, org) are null or partial — the exact property name, full address (incl. postal), recipient-with-unit, and rate_division string (including the THOR provider suffix) live in sites.metadata.billHeader as verbatim backfilled strings.
- Goal
- Read billHeader from sites.metadata JSONB and use its fields as overrides for the PDF header, falling back to derived relational values for new sites that have no override. Also add YAxis ticks and data-point circles to the hourly area chart, and fix XLabels alignment for TOU/PDBT charts.
- Sub-goals
- SG-1: Expose BillHeaderOverride via readSiteBillingMeta query (JSONB -> operator extraction)
- SG-2: Apply override precedence in buildBillPdfRichMeta (bh?.field ?? derived)
- SG-3: Add YAxis + Circle data points to hourly area chart
- SG-4: Fix XLabels width binding for TOU and PDBT monthly charts
- What
- Three files changed: scheduled-billing.queries.ts adds BillHeaderOverride interface and billHeader field to SiteBillingMeta (read via sql JSONB extraction); bill-pdf-view-model.ts applies override precedence for all four header fields; bill-document.tsx adds Circle import, wraps hourly chart in chartFrame with YAxis, and fixes XLabels alignment.
- Why
- Legacy Smarter/THOR simulated bills carried exact verbatim strings (property name, postal address, tenant unit in recipient, THOR provider in rate_division) that cannot be derived from the relational schema alone for mall-tenant sites. Backfilling these into sites.metadata.billHeader and reading them at render time restores pixel-parity.
- Areas
- domains/cross-domain+24−5packages/bill-pdf+15−5
- Blast
- 3 files, +39/−10 lines across 2 areas (cross-domain queries+view-model, bill-pdf renderer). Changes are additive (new interface field, new override logic, new chart elements). No schema migration needed — reads from existing sites.metadata JSONB. Both callers (platform handler + batch render worker) are affected by the view-model change.
Findings · 13
security3
Unsafe type assertion on JSONB sub-key — no runtime validation of BillHeaderOverride
domains/cross-domain/src/scheduled-billing.queries.ts:113
sql<BillHeaderOverride | null>`${sites.metadata} -> 'billHeader'` is a TypeScript-only assertion; Drizzle passes the Postgres JSONB value verbatim with zero runtime shape validation. Maliciously crafted or malformed content in clientName, address, recipientName, tariffReference renders verbatim in the PDF. Extremely long strings can cause layout corruption / resource exhaustion in the react-pdf render worker. Adding a Zod parse at the query boundary would surface bad data early.
billHeader sub-key has no access control — writable by any org member with site-create permission
domains/cross-domain/src/scheduled-billing.queries.ts:90
Any org member who can call POST /sites can store arbitrary content under metadata.billHeader. There is no admin-only gate on this sub-key. Low severity under the current trust model (same org) but worth flagging if self-service site creation is ever opened to less-trusted actors.
No length cap on BillHeaderOverride string fields — potential DoS via oversized PDF text
domains/cross-domain/src/bill-pdf-view-model.ts:79
propertyName, address, recipientName, and tariffReference are passed verbatim into react-pdf Text elements without any length truncation. A very long string (tens of kB) could cause excessive memory use or a crash in the render worker.
conventions3
JSDoc block misattributed — wrong interface gets wrong doc
domains/cross-domain/src/scheduled-billing.queries.ts:74
Two consecutive JSDoc blocks appear before the two interfaces. The first block ('Human-facing site context for the bill PDF header...') was SiteBillingMeta's doc but now sits immediately above BillHeaderOverride, making it BillHeaderOverride's doc in tooling. SiteBillingMeta ends up documented by 'Exact legacy bill-header strings...' — the inverse of intent. Move the 'Human-facing...' block to immediately precede SiteBillingMeta.
BillHeaderOverride interface defined in a queries file rather than a type file
domains/cross-domain/src/scheduled-billing.queries.ts:75
Per canonical form, domain interfaces belong in {entity}.type.ts. BillHeaderOverride is a pure data shape with no query logic; it belongs in a companion type file (e.g. scheduled-billing.types.ts) and imported by the queries file.
sql<T> JSONB `->` operator is correct but cast accuracy should be verified
domains/cross-domain/src/scheduled-billing.queries.ts:113
The `->` operator returns the JSONB sub-object (correct — pg driver deserializes it as JS object). `->>` would return text. The cast is accurate. No action needed; noted for awareness.
tests4
No unit tests for buildBillPdfRichMeta — billHeader override paths completely untested
domains/cross-domain/src/bill-pdf-view-model.ts:62
buildBillPdfRichMeta is a pure function with three distinct branches: (a) billHeader present → all four fields overridden; (b) billHeader null → derived from siteMeta/ctx; (c) partial billHeader → mixed override+derived. None of these branches have any unit tests — no test file references buildBillPdfRichMeta or BillHeaderOverride. This function is shared by the platform handler and the batch render worker; a silent regression in the priority logic would affect both callers.
Partial billHeader override (some fields present) not tested — empty-string edge case
domains/cross-domain/src/bill-pdf-view-model.ts:79
The override uses ?? per-field independently. A billHeader with clientName set but address absent should yield a mixed result. An empty-string clientName ('') passes through as propertyName (not nullish). Neither behavior is documented by a test.
BillHeaderOverride fields are optional — misspelled JSONB keys silently fall through to derived values
domains/cross-domain/src/scheduled-billing.queries.ts:78
If stored JSONB has wrong key casing (e.g. 'client_name' vs 'clientName'), the cast succeeds but reads as undefined, silently using the derived fallback. No runtime parse surfaces this mismatch. A test simulating wrong-key JSONB would document the failure mode.
No test coverage on bill-pdf package — PDBT XLabels width-binding change unverifiable
packages/bill-pdf/src/bill-document.tsx:1
The bill-pdf package has zero test files. The XLabels width binding fix (<View style={{ width: CHART_W + 22 }}>) cannot be regression-tested. Low severity — react-pdf layout tests are unconventional — but noted as a gap.
improvement3
`?? null` on `siteMeta?.billHeader` is redundant
domains/cross-domain/src/bill-pdf-view-model.ts:75
`siteMeta?.billHeader` is already `BillHeaderOverride | null | undefined`. The `?? null` collapses undefined→null, but bh is only ever used via optional chaining which handles both. `const bh = siteMeta?.billHeader;` is sufficient.
`n <= 48` circle threshold in AreaLine is vacuously true for hourly data
packages/bill-pdf/src/bill-document.tsx:106
AreaLine is only ever called with hourly (24 points). The guard is always true. Either document it as a forward-compat guard for possible future callers, or replace with `n > 0` if circles are always desired.
Hourly chart container (232 pt) vs inner content (22 + 208 = 230 pt) — 2 pt slack
packages/bill-pdf/src/bill-document.tsx:225
Enclosing View is 232 pt, YAxis + AreaLine = 230 pt. The 2 pt gap is harmless flex trailing space but may indicate a copy-paste approximation from the prior 230 pt width. Harmless unless flush alignment is required.