fix/col-rename
needs attentionviewing older commit39b9e17 · fullPR #284reviewed 2026-07-08 23:15 UTC3H · 5M · 6L · 5I- Purpose
- Production bug fix: user-renamed line-item columns rendered blank in the recibos table and CSV export for Energía Real (436 rows affected).
- Goal
- Ensure transformBillToPresentation keys line-item values under the same name the column header uses.
- Sub-goals
- SG-1: Fix the key lookup in the line-item for-loop (bill.shells.ts)
- SG-2: Export the pure helper for direct unit testing
- SG-3: Add regression tests covering the bug scenario and the no-override fallback
- What
- Single for-loop block changed to look up effective publicName via conceptToPublicName.get(lineItem.conceptName) with fallback to lineItem.conceptPublicName; function exported; two regression tests added.
- Why
- The header column name uses the override-aware conceptToPublicName map, but old value-keying used raw lineItem.conceptPublicName — mismatch causing blank cells for renamed columns.
- Areas
- domains/utility/src/bill/bill.shells.ts+9−2domains/utility/src/bill/__tests__/bill.shells.test.ts+87−1
- Blast
- 2 files, +96/−3. Strictly corrective: non-renamed columns byte-identical; no API/wire-shape change.
Findings · 15
correctness2
Uncovered but safe: conceptName non-null yet absent from map
Uncovered but safe: both conceptName and conceptPublicName null
security4
No length cap on displayName — keys replicated O(bills_per_page) times (pre-existing)
No denylist for JS property names (constructor, toString) as object keys (pre-existing)
CSV formula injection in displayName header cells (pre-existing)
billFieldPublicNames guard bypassable by whitespace-differing rename (pre-existing)
conventions3
Pure function belongs in bill.decisions.ts, not bill.shells.ts
domains/utility/src/bill/bill.shells.ts:467
transformBillToPresentation is a pure function — no async, no I/O — yet lives in the shell file whose job is transaction orchestration. Move to bill.decisions.ts alongside synthesizeAverageFeeFromCommand etc.
Export justified by 'for unit testing' — prohibited by canonical-form
domains/utility/src/bill/bill.shells.ts:466
Export comment names the caller (test file) — belongs in PR description
domains/utility/src/bill/bill.shells.ts:466
tests5
Missing test: conceptName === null → falls back to conceptPublicName
domains/utility/src/bill/__tests__/bill.shells.test.ts
EnrichedLineItem.conceptName is string | null. XML bills with unmapped concepts have null conceptName. No test covers this fallback path.
Missing test: conceptName present but absent from map → falls back to conceptPublicName
domains/utility/src/bill/__tests__/bill.shells.test.ts
A line item whose concept wasn't in the column-config pool misses the map. The ?? lineItem.conceptPublicName fallback is untested.
Missing test: multi-line-item bill with mixed renamed/non-renamed columns
Missing test: both conceptName and conceptPublicName null → item skipped
Test 2 passes on old code — not a true regression test for the bug
improvement1
Ternary guard redundant — Map.get() handles null input natively
domains/utility/src/bill/bill.shells.ts:523