fix: stop reporting a partial-coverage sum as net worth - #97
Closed
KenTaniguchi-R wants to merge 1 commit into
Closed
KenTaniguchi-R wants to merge 1 commit into
KenTaniguchi-R wants to merge 1 commit into
Conversation
The MCP server SDK was pinned to the 2.0.0-beta.5 pre-release. Stable 2.0.0 has since shipped, so move off the beta. No API drift: createMcpHandler and McpServer in src/lib/mcp/server.ts are unchanged, and the bundled protocol revisions stay the same (2026-07-28 current, 2025-11-25 legacy fallback).
Owner
Author
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.
Fixes #86.
The bug
The dashboard read:
Nothing earned that. Five accounts holding $51,703 — both Robinhood accounts, the IRA, Crypto, the Wells Fargo card — have their first-ever balance snapshot on 2026-08-28. Every point before that counted them as $0, so the chart plotted the credit cards alone and labelled the result net worth, then jumped $55K when the rest appeared.
Coverage on the real household steps:
Carry-forward (#68) fixed gaps between an account's snapshots. It cannot fix a gap before the first one — there is nothing to carry backward. So this is a presentation problem, not a reconstruction one, and per the #67 precedent the fix does not invent the missing history; it says which stretch is missing.
What changed
Query —
getNetWorthHistoryemitscoveredAccounts/totalAccountsper point.lastBalanceByAccount.sizeis already exactly the number of accounts we hold any balance for as of that date, so this is free; the synthetic today point counts accounts reporting a live balance.Delta — new
coveredTrendDeltameasures across the fully covered span only and returnsnullwith fewer than two such points. A delta from a partial baseline reports accounts appearing, not money arriving.Chart — splits at the boundary: muted dashed line plus a hatched band for the partial span, solid area after, boundary rule between. The boundary point belongs to both dataKeys or the segments wouldn't meet. The tooltip drops the null half so a date isn't listed twice.
Caption — names the shortfall in words ("2–5 of 10 accounts had balance history before Aug 28, so it is not yet net worth"), so the treatment is never color-alone.
Zero deltas lose the arrow — "↑ $0.00" pointed somewhere the number didn't. This case is live right now: the real household has exactly two fully-covered points at the same value.
Design
Three treatments were mocked against the real 84-point series before building; this is option B. A (trim to full coverage) is the most literal reading of "net worth" but collapses 6M/1Y/All to two points and discards three months of accurate credit-card history. C (suppress the percentage only) is two lines but leaves a line diving to −$13,456 with nothing on screen explaining why.
Typing
NetWorthPointsplits intoNetWorthSeriesPoint(date/assets/liabilities/netWorth) plus the coverage-carrying dashboard shape. The reports series has the same leading-gap exposure and keeps the base type — surfacing coverage there is separate work, andcoverageBoundarytreats absent coverage fields as "fully covered" rather than guessing.Known limitation
Nothing distinguishes "the account didn't exist yet" from "we have no data yet."
accounts.createdAtis when Ledgr learned of an account, not when it opened. So a genuinely new account is also marked partial. That is the conservative direction, but if an "opened on" field is ever added, the band should respect it.Verification
Full suite green: 113/113 test files. New tests cover the boundary helper (7 cases incl. never-completes, no-coverage-data, and the exact −$2,271 → $52,942 series that produced 2430.7%) and three integration tests on coverage reporting, including seeding from before the window.
Verified in the running app that a fully-covered household (the demo seed) shows no band and keeps its normal range label — the negative case doesn't regress.
🤖 Generated with Claude Code