feat/ui-impact

portfolioDiD monthly sums not comparable when store coverage (n) varies

apps/platform/src/app/[locale]/(dashboard)/energia/comparativo/page.tsx:243

Each monthly bucket sums kWh/día across however many stores have an expected value that month. If January has 8 stores and June has 3, the June sum is structurally smaller — not because those stores used less energy. Time-averaging these unequal-n sums (lines 246–248) produces mActual/mEsperado that are neither a per-store average nor a stable portfolio total; they are an average of unequally-weighted monthly sums. The dev% may be directionally correct but will be biased toward months with highest coverage. Fix: divide each monthly sum by its n before time-averaging (per-store mean per month, then average across months), or weight the average by n.

Math.round applied to monthly sums before time-averaging — rounding error accumulates

apps/platform/src/app/[locale]/(dashboard)/energia/comparativo/page.tsx:243

Line 243: actual: Math.round(actual) and esperado: Math.round(esperado) round monthly portfolio SUMS before they are stored in series. Lines 246–247 then sum these already-rounded values again. With ~20 stores and ~12 months the accumulated rounding error before the display fmtNorm can reach ±6 kWh/día. Round only at the display layer (fmtNorm already calls toFixed(0)); remove the Math.round calls from the intermediate series construction.

Division-by-zero in storeChart when store series has dias === 0

apps/platform/src/app/[locale]/(dashboard)/energia/comparativo/page.tsx:184

Line 184: Math.round(p.kwh / p.dias). If any MonthPoint has dias: 0, the result is Infinity, which recharts renders as a broken chart. Baked-in production data does not appear to contain dias: 0 records, so risk is low. A guard (p.dias > 0 ? Math.round(p.kwh / p.dias) : null) would be safe and honest.

URL param ?tienda= validated against allowlist before use — no XSS risk

apps/platform/src/app/[locale]/(dashboard)/energia/comparativo/page.tsx:141

window.location.search is read and the 'tienda' param is immediately validated against STORES.some(s => s.id === t). Value is never rendered as HTML. Positive signal — no change needed.

New Tooltip formatter (v: any) is safe — value is recharts-internal from baked-in data

apps/platform/src/app/[locale]/(dashboard)/energia/comparativo/page.tsx:460

The new portfolio norm chart Tooltip formatter at line 460 uses (v: any) => fmtNorm(Number(v)). Value originates from portfolioDiD.series (static baked-in data via analyzeStore). No user-controlled data flows in. Matches pre-existing pattern on lines 422 and 487.

as number cast guarded by null check (line 239)

apps/platform/src/app/[locale]/(dashboard)/energia/comparativo/page.tsx:239

esperado += pt.expected as number is preceded by p.expected != null in the find predicate. All data is static baked-in STORES. Safe cast, no injection risk.

fmtNorm on portfolio sum misleads — P4 one language violated

apps/platform/src/app/[locale]/(dashboard)/energia/comparativo/page.tsx:380

fmtNorm appends 'kWh/día' to portfolioDiD.actual, but the value is the time-average of monthly SUMS of per-store kWh/día (e.g. 30 stores × 800 = 24,000 kWh/día). The sub-label 'kWh/día promedio' reads as 'per-store daily average' in natural Spanish. In every other context on this page, 'kWh/día' means one store's normalized daily consumption (sucursal KPIs, cohort ranking). P4 requires one vocabulary: either label this as a portfolio aggregate explicitly, or normalize by the store count so the unit is genuinely comparable.

connectNulls on esperado line silently bridges unknown-coverage gaps — P5 violated

apps/platform/src/app/[locale]/(dashboard)/energia/comparativo/page.tsx:428

storeChart sets esperado: null for months lacking cohort data (line 185). With connectNulls, Recharts draws an interpolated segment across those null months, implying a continuous expected trend where no value was computed. P5: 'Null → dash; synthetic is labeled.' The bridged line is synthetic and unlabeled. The callout text at lines 402–405 partially mitigates this at page level, but the chart itself has no visual gap or annotation at bridged segments. Fix: remove connectNulls from the esperado Line (let recharts show the honest gap), or add a distinct strokeDasharray and annotation to mark the interpolated region.

Em dash used as separator in new user-facing UI copy — Copy Style rule violated

apps/platform/src/app/[locale]/(dashboard)/energia/comparativo/page.tsx:380

Commit 95a101ec introduced at least two new hardcoded UI strings with em dashes as separators: line 380 KPI label 'Portafolio — real' and line 441 CardTitle 'Portafolio — real vs. su tendencia esperada · kWh/día'. ui-patterns.md Copy Style states: 'No em dashes in user-facing copy.' The honest-absence glyph exception covers '—' for absent data values, not separator usage in titles. Fix: replace with ':', '·', or a comma — e.g. 'Portafolio · real' and 'Portafolio: real vs su tendencia esperada · kWh/día'.

portfolioDiD.series includes n field passed into recharts LineChart data

apps/platform/src/app/[locale]/(dashboard)/energia/comparativo/page.tsx:243

Each datum in portfolioDiD.series has { mes, actual, esperado, n }. No <Line dataKey='n'> exists; recharts ignores the field. n is useful for future tooltip coverage signal, but currently appears in recharts' internal payload processing as a spurious entry. Consider stripping n from the chart series (compute .filter(r => r.n > 0) using a local var and emit only { mes, actual, esperado }) or surface it in the Tooltip formatter for honest coverage signaling.

useMemo dependency arrays are correct — no missing deps

apps/platform/src/app/[locale]/(dashboard)/energia/comparativo/page.tsx:228

portfolioDiD [inRange], portfolioStats [inRange], portfolioChart [inRange], storeChart [store, a, metric, inRange], storeHasExpected [storeChart] — all correct. STORES, analyzeStore, ALL_YMS, shortMonth are module-level stable references and correctly omitted.

Kpi accent coloring with dev ?? undefined null-guard is correct

apps/platform/src/app/[locale]/(dashboard)/energia/comparativo/page.tsx:382

pct={portfolioDiD.dev ?? undefined} maps null → undefined when dev is null. Kpi suppresses color when pct === undefined; renders '—' value without coloring. When dev===0, pct=0 triggers rose color for '+0%' — consistent with sucursal KPI at line 375, pre-existing pattern.

portfolioDiD non-trivial math has no unit tests

apps/platform/src/app/[locale]/(dashboard)/energia/comparativo/page.tsx

portfolioDiD (lines 228–250) computes: per-month sums of actual+expected across qualifying stores, filters months with n=0, time-averages, derives dev%. Three untested edge cases: (1) covered=0 path when no stores have expected values in range; (2) dev=null when mEsperado=0; (3) variable-n bias described above. The logic should be extracted into a pure function (e.g. _lib/portfolioDiD.ts) and covered with Vitest — identical pattern to analyzeStore in anomaly.ts.

storeChart empty state (storeChart.length === 0) has no test coverage

apps/platform/src/app/[locale]/(dashboard)/energia/comparativo/page.tsx:413

The new empty state guard at line 413 is reachable via a 'custom' preset range that falls outside any store's data window. A refactor of the range filter could silently break this path. A unit test for the storeChart memo (or for the extracted pure function) with a range that returns no matching series would cover this.

portfolioDiD.dev=null display path ('—') is untested

apps/platform/src/app/[locale]/(dashboard)/energia/comparativo/page.tsx:382

Line 382 renders '—' when portfolioDiD.dev == null. This path is reached when mEsperado === 0 or series is empty. A future change to the null-coalescing expression could silently turn null into '0%'. Covered by a test that sets a range where no store has expected values.

analyzeStore (anomaly.ts) has no unit tests despite being shared computation core

apps/platform/src/app/[locale]/(dashboard)/energia/comparativo/_lib/anomaly.ts

analyzeStore contains multi-step DiD math (baseline anchoring, cohort median, breakout detection) and is called by three memos. No __tests__ directory exists under comparativo/. Pre-existing gap, but the three-call pattern now makes a defect here impact three places.

E2E coverage not warranted — all logic is pure computation on static data

apps/platform/src/app/[locale]/(dashboard)/energia/comparativo/page.tsx

The page uses baked-in anonymized data with no API calls. Unit tests for extracted pure functions (portfolioDiD, analyzeStore) provide better signal than browser-level E2E at lower cost.

analyzeStore called three times independently across memos — redundant computation

apps/platform/src/app/[locale]/(dashboard)/energia/comparativo/page.tsx:221

portfolioStats (line 221), portfolioDiD (line 229), and ranking (line 255) each call STORES.map(s => analyzeStore(s.id)). analyzeStore runs O(stores × months × peers) logic per call (~8k iterations per sweep). Three independent sweeps = ~24k iterations per inRange change. Hoist to a single useMemo with an empty dep array (inputs are stable module-level constants): const allResults = useMemo(() => STORES.map(s => analyzeStore(s.id)).filter(Boolean as ...), []).

storeHasExpected memo could fold into storeChart — no memoization advantage to separate memo

apps/platform/src/app/[locale]/(dashboard)/energia/comparativo/page.tsx:192

storeHasExpected depends solely on storeChart and always recomputes with it. Returning { data, hasExpected } from the storeChart memo (renamed storeChartResult) eliminates the extra memo and the extra dep declaration while keeping the derivation co-located.

Portfolio KPI sub-label 'kWh/día promedio' risks per-store misread

apps/platform/src/app/[locale]/(dashboard)/energia/comparativo/page.tsx:380

portfolioDiD.actual is the time-average of monthly portfolio totals — not a per-store average. 'kWh/día promedio' reads naturally as the per-store daily average. Sharpening to 'kWh/día total · promedio mensual' or a similar phrasing makes the aggregation direction unambiguous, consistent with the chart description at line 444 which correctly says 'Suma del consumo real de tus tiendas'.

Recharts boilerplate duplication across three chart cards is tolerable at this scale

apps/platform/src/app/[locale]/(dashboard)/energia/comparativo/page.tsx:416

Three charts share the same scaffold (ResponsiveContainer/LineChart/CartesianGrid/XAxis/YAxis/Tooltip/Legend) with identical styling but different dataKeys, units, conditional Lines, and formatters. An abstraction's props surface would approach the JSX it replaces. At three instances this is acceptable; flag if a fourth variant is added.