Scope a positions snapshot to its own broker - #5
Merged
Conversation
Prices were never updated except by a broker sync, and nothing said so. `app/navs.py` fetches AMFI's daily NAV file — public, no key, nothing personal sent — and marks every mutual fund position. This matters most for Paytm, which reports no NAV at all: those funds were marked at the NAV of their last transaction, and one was 24 days stale. Refreshing corrected India from 97,31,673 to 97,20,028. Matching refuses to guess. A NAV on the wrong fund is worse than a stale one, because a stale price is visibly stale and a wrong one is not. Funds with an ISIN match on it. Paytm funds have none, so the scheme name must reduce to exactly one Direct/Growth scheme after filler words are dropped — which correctly matched "HDFC Index Fund - BSE Sensex Plan" to AMFI's "HDFC BSE Sensex Index Fund" despite the word order, and refuses anything ambiguous. Regular plans and IDCW options are excluded, since an IDCW NAV would permanently understate a growth holding. Resolved ISINs are stored so a name is matched once and can be audited. "Data through" measured the wrong thing. It reported the last transaction date, so a trade this morning made the header look healthy while fund NAVs were a month old. Price age is now reported per asset class as the OLDEST mark in each group, not the newest — one fund priced today does not make the fund holdings current — shown beside the transaction date and bannered past a week. The quotes table was dead: written by upsert_quotes, read by nothing, 0 rows. It now backs pricing. A quote is used only when dated later than the broker snapshot, so a stale quote can never override a fresh sync. Writing the tests found a real bug. results() had a second pricing path for holdings with no transactions — which is every mutual fund — that read positions.price directly and ignored quotes entirely. So the funds this feature exists to reprice were the one group it would not have reached. Both paths now go through _positions(). 22 new tests: feed parsing, the matcher's refusals, and that a stale quote loses to a fresh broker price. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A sync of one market deleted every holding belonging to the others. upsert_positions issued `DELETE FROM positions` with no filter. That was correct when Robinhood was the only source, and became silent data loss the moment a second market existed — the snapshot is authoritative for its own broker and says nothing about anyone else's. It now deletes only the rows it is replacing, and stamps broker and currency on what it writes, which it also was not doing. Nothing caught this because the figures checked after a US sync were the US figures, and those stayed perfectly correct. The other market simply had no positions left, which reads as an empty market rather than an error. Three regression tests: a snapshot from one broker leaves the others intact, it does still replace its own rows, and broker and currency are stamped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
A sync of one market deleted every holding belonging to the others.
The bug
Correct when Robinhood was the only source. Silent data loss the moment a second market
existed: a US sync wiped all Zerodha and Paytm positions.
It also never stamped
brokerorcurrencyon the rows it inserted, so anything it wrotedefaulted to the schema's
robinhood/USD.Why nothing caught it
The figures checked after a US sync were the US figures, and those stayed perfectly
correct. The other market simply had no positions left — which renders as an empty market,
not as an error. Every test and every manual check looked at the market that still worked.
The fix
Delete only the rows being replaced, and stamp broker and currency on the inserts.
Tests
Three regressions in
tests/test_ingest.py:Recovery note for anyone who hit this
Positions are a snapshot, not history — transactions are untouched, so nothing is
permanently lost. Re-sync the affected market to repopulate. Paytm positions are derived
from transactions and rebuild with
ingest._rebuild_paytm_positions().