fix(pulse/finances): non-USD amounts parse as zero, plus two adjacent data-loss bugs - #1801
Open
prafed wants to merge 2 commits into
Open
fix(pulse/finances): non-USD amounts parse as zero, plus two adjacent data-loss bugs#1801prafed wants to merge 2 commits into
prafed wants to merge 2 commits into
Conversation
…d sample-vendor matching Three defects in the Finances data layer, each of which produces a plausible-looking but wrong dashboard rather than a visible failure. 1. parseCurrencyCell stripped $ and commas but not £ or €, so a cell like "£1,234" survived cleaning as "£1234", failed the leading-digit match and returned 0. Every row on a non-USD dashboard silently became zero. Also accepts the Unicode minus (U+2212) so negative rows parse rather than being dropped by the amount > 0 filter. USD behaviour is unchanged. 2. Obligation scope was hardcoded "personal" in two places — once in the YAML normalizer and again in the resolver — so an obligations.yaml declaring scope: business was ignored, and fixing only the first has no visible effect. 3. knownLabels included vendors and obligations resolving to zero. Every row of the shipped sample vendors.yaml is zero-value and "unconfigured", and the substring matcher let those sample names suppress real EXPENSES.md rows. A user who fills in EXPENSES.md without clearing the sample file loses those expenses from the outbound total with no warning. Only entries that actually contribute spend can now suppress a row, which is the only case where suppression prevents genuine double-counting.
…a hard-coded $ The dashboard formatted every figure with a literal $. Read state.currency from the API payload and pick the symbol from it, falling back to $ when the field is absent so existing installs are unaffected. Pairs with the parseCurrencyCell fix: without both, a GBP install shows $0 everywhere.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three defects in the Pulse Finances data layer, plus the frontend change needed to see the fix. Each bug produces a plausible-looking but wrong dashboard rather than a visible failure, which is why they're easy to miss.
Found while populating the Finances tab with real GBP data on a fresh install.
1. Non-USD amounts parse as zero
parseCurrencyCellstrips$and commas but not£or€:"£1,234"survives as"£1234", failscleaned.match(/^[\d.]+/), and returns0. Every row on a non-USD dashboard silently becomes zero — the tab renders normally and shows a total of 0.Also now accepts the Unicode minus (U+2212), which typographic pipelines emit, so negative rows parse rather than being dropped by the
amount > 0filter inparseCurrencyTable.USD behaviour is unchanged; regression cases below.
2. Obligation
scopeis hardcoded in two placesobligations.yamlcan declarescope: business, but the value is overwritten with"personal"both in the YAML normalizer and again in the resolver. Fixing only the first has no visible effect, which makes this awkward to debug. Every business-paid obligation renders as personal.3. Sample vendors silently delete real expenses
knownLabelsis built from all vendors and obligations, then substring-matched againstEXPENSES.mdrows to avoid double-counting:Every row of the shipped
install/USER/FINANCES/vendors.yamlis a zero-valueunconfiguredsample ("Sample Employer Inc.","Sample Landlord", …). A user who fills inEXPENSES.mdbut leaves the sample file alone — it looks inert — has those sample names suppress their genuine expense rows.On my install this removed roughly 40% of real expenses from the outbound total with no warning: the tab showed £32k against a true £55k.
A zero-value entry has nothing to double-count, so suppression is pure data loss. Only entries that actually contribute spend can now suppress a row.
4. Frontend: currency symbol from
state.jsonfmtHero/fmtExacthardcode$. They now readstate.currency(already present in the API payload) and fall back to$when absent, so existing installs are unaffected. Without this the parser fix is invisible: a GBP install shows$0everywhere, then$98K.Verification
Parser, including USD regression:
£1,234**£31,570.56**−£6,408.47€2,500$1,234~$500$12K£1.2M-$50""/n/aEnd to end on a real GBP install: income and expense totals correct, obligations splitting business from personal correctly, and the Expenses tab total moving from the wrong £32k to the correct £55k once sample vendors stopped matching.
Happy to split this into separate PRs if you'd prefer them reviewed independently.