frontend/src/dashboard.ts formats dollar values with 5-6 independent .toLocaleString() calls inside Chart.js tooltip and axis-tick callbacks, none of which pin a locale. This is the same defect #1728 fixed in formatCurrency, in a file that fix did not reach.
Exact line numbers are listed in PR #1732's body.
Why it was scoped out of #1732
Deliberately, and correctly. Those call sites render to canvas, not the DOM; no test exercises them; and none of #1728's eight failures originated there. Widening a one-line locale fix into untested chart-rendering code would have been scope growth into the least verifiable part of the frontend.
The question worth answering first
Should these call formatCurrency instead of formatting money ad hoc?
If they can, that is the better fix: the underlying defect is that money formatting exists in more than one place, which is exactly what allowed the locale divergence. Consolidating removes the class rather than pinning six more instances of it. formatCurrency already pins 'en-US' after #1732 and already hardcodes the $ prefix, so a caller gets both behaviours for free.
If they genuinely cannot — different precision for axis ticks, abbreviated forms like $1.2M, or non-currency numbers mixed in — then pin each explicitly and record why consolidation was rejected, so the next person does not re-litigate it.
Resolve that before writing any code. The answer determines whether this is a 1-line change or a 6-line change.
Verification
Same standard as #1732: run under at least two locales (LANG=en_US.UTF-8 and LANG=de_DE.UTF-8). Since no test covers these paths, a locale run alone proves nothing — render the dashboard and look at the chart under a non-US locale before and after. If that is impractical, say so plainly in the PR rather than implying coverage that does not exist.
Do not fabricate tests for chart rendering to make this look verified. An honest "mechanical change, untested path, here is the visual check" is worth more than a test written to satisfy a checklist.
Context
Found while fixing #1728.
frontend/src/dashboard.tsformats dollar values with 5-6 independent.toLocaleString()calls inside Chart.js tooltip and axis-tick callbacks, none of which pin a locale. This is the same defect #1728 fixed informatCurrency, in a file that fix did not reach.Exact line numbers are listed in PR #1732's body.
Why it was scoped out of #1732
Deliberately, and correctly. Those call sites render to canvas, not the DOM; no test exercises them; and none of #1728's eight failures originated there. Widening a one-line locale fix into untested chart-rendering code would have been scope growth into the least verifiable part of the frontend.
The question worth answering first
Should these call
formatCurrencyinstead of formatting money ad hoc?If they can, that is the better fix: the underlying defect is that money formatting exists in more than one place, which is exactly what allowed the locale divergence. Consolidating removes the class rather than pinning six more instances of it.
formatCurrencyalready pins'en-US'after #1732 and already hardcodes the$prefix, so a caller gets both behaviours for free.If they genuinely cannot — different precision for axis ticks, abbreviated forms like
$1.2M, or non-currency numbers mixed in — then pin each explicitly and record why consolidation was rejected, so the next person does not re-litigate it.Resolve that before writing any code. The answer determines whether this is a 1-line change or a 6-line change.
Verification
Same standard as #1732: run under at least two locales (
LANG=en_US.UTF-8andLANG=de_DE.UTF-8). Since no test covers these paths, a locale run alone proves nothing — render the dashboard and look at the chart under a non-US locale before and after. If that is impractical, say so plainly in the PR rather than implying coverage that does not exist.Do not fabricate tests for chart rendering to make this look verified. An honest "mechanical change, untested path, here is the visual check" is worth more than a test written to satisfy a checklist.
Context
formatCurrency; fixed by pinning'en-US', matching theformatDate/formatDateTimeconvention already established in that file.formatCurrencyalready hardcodes$, so following the host locale for number grouping was an oversight rather than an intent.Found while fixing #1728.