csv-parser: keep LLM-mapped tickers; don't pass them through T212 mapping
The route's resolve-slice loop is T212-specific — it looks tickers up against the InstrumentMap, which only has T212's universe. For the LLM path the ticker is already Yahoo-ready (e.g. VOD.L, ASML.AS), so sending it through resolve_slice produced spurious "could not be resolved" warnings and dropped the positions. Fix: ParsedPie gains a ``tickers_resolved`` flag (default False for T212 backward-compat); _apply_mapping in the LLM path sets it True and also extracts currency from the LLM-mapped currency_col into a new ``ParsedPosition.currency`` field. The route branches on the flag: LLM-path positions are kept verbatim with a best-effort InstrumentMap lookup for nicer name/currency overrides, never dropped. Integration test tightened to assert all 5 IBKR fixture positions round-trip with the right currencies (USD / GBP / EUR). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
parent
b8ebba9503
commit
bc55ab7d26
4 changed files with 74 additions and 13 deletions
|
|
@ -225,19 +225,42 @@ async def parse_portfolio(
|
||||||
unmapped: list[str] = []
|
unmapped: list[str] = []
|
||||||
|
|
||||||
for p in pie.positions:
|
for p in pie.positions:
|
||||||
|
if pie.tickers_resolved:
|
||||||
|
# LLM path: ``p.slice`` is already a Yahoo-ready ticker. We
|
||||||
|
# still do a best-effort InstrumentMap lookup so we can use
|
||||||
|
# the canonical name + currency when we happen to have one;
|
||||||
|
# but unlike the T212 path we never *drop* a position just
|
||||||
|
# because resolve_slice missed.
|
||||||
|
yahoo_ticker = p.slice.strip().upper()
|
||||||
|
if not yahoo_ticker:
|
||||||
|
unmapped.append(p.name or "?")
|
||||||
|
continue
|
||||||
|
resolved = await resolve_slice(session, yahoo_ticker)
|
||||||
|
name = (resolved.name if resolved else None) or p.name
|
||||||
|
currency = (
|
||||||
|
(resolved.currency if resolved else None)
|
||||||
|
or p.currency or "USD"
|
||||||
|
)
|
||||||
|
else:
|
||||||
|
# T212 path: ``p.slice`` is a shortcode that MUST round-trip
|
||||||
|
# through the InstrumentMap. Drop unmapped positions — the
|
||||||
|
# warnings block surfaces them to the user.
|
||||||
resolved = await resolve_slice(session, p.slice)
|
resolved = await resolve_slice(session, p.slice)
|
||||||
if resolved is None or not resolved.yahoo_ticker:
|
if resolved is None or not resolved.yahoo_ticker:
|
||||||
unmapped.append(p.slice or p.name or "?")
|
unmapped.append(p.slice or p.name or "?")
|
||||||
continue
|
continue
|
||||||
|
yahoo_ticker = resolved.yahoo_ticker
|
||||||
|
name = resolved.name or p.name
|
||||||
|
currency = resolved.currency
|
||||||
positions_out.append({
|
positions_out.append({
|
||||||
"yahoo_ticker": resolved.yahoo_ticker,
|
"yahoo_ticker": yahoo_ticker,
|
||||||
"t212_slice": p.slice,
|
"t212_slice": p.slice,
|
||||||
"name": resolved.name or p.name,
|
"name": name,
|
||||||
"qty": p.quantity,
|
"qty": p.quantity,
|
||||||
"avg_cost": p.average_price, # @property — no call parens
|
"avg_cost": p.average_price, # @property — no call parens
|
||||||
"currency": resolved.currency,
|
"currency": currency,
|
||||||
})
|
})
|
||||||
yahoo_tickers.append(resolved.yahoo_ticker)
|
yahoo_tickers.append(yahoo_ticker)
|
||||||
|
|
||||||
# Synchronous upsert: bypass the Redis buffer so the dashboard has
|
# Synchronous upsert: bypass the Redis buffer so the dashboard has
|
||||||
# live prices immediately. The buffer + flush machinery remains for
|
# live prices immediately. The buffer + flush machinery remains for
|
||||||
|
|
|
||||||
|
|
@ -37,7 +37,10 @@ _REQUIRED_FIELDS = ("slice", "quantity")
|
||||||
|
|
||||||
@dataclass(frozen=True)
|
@dataclass(frozen=True)
|
||||||
class ParsedPosition:
|
class ParsedPosition:
|
||||||
slice: str # T212 shortcode, e.g. "SGLN"
|
slice: str # T212 shortcode (e.g. "SGLN") or a
|
||||||
|
# Yahoo-ready ticker (e.g. "VOD.L")
|
||||||
|
# when produced by the LLM path —
|
||||||
|
# see ParsedPie.tickers_resolved.
|
||||||
name: str
|
name: str
|
||||||
invested_value: float | None
|
invested_value: float | None
|
||||||
current_value: float | None
|
current_value: float | None
|
||||||
|
|
@ -46,6 +49,10 @@ class ParsedPosition:
|
||||||
dividends_gained: float | None = None
|
dividends_gained: float | None = None
|
||||||
dividends_cash: float | None = None
|
dividends_cash: float | None = None
|
||||||
dividends_reinvested: float | None = None
|
dividends_reinvested: float | None = None
|
||||||
|
currency: str | None = None # Populated by the LLM path from the
|
||||||
|
# mapped currency_col; the T212 path
|
||||||
|
# leaves it None and gets currency
|
||||||
|
# from the InstrumentMap row.
|
||||||
|
|
||||||
@property
|
@property
|
||||||
def average_price(self) -> float | None:
|
def average_price(self) -> float | None:
|
||||||
|
|
@ -67,6 +74,11 @@ class ParsedPie:
|
||||||
invested: float | None # totals from the Total row
|
invested: float | None # totals from the Total row
|
||||||
value: float | None
|
value: float | None
|
||||||
result: float | None
|
result: float | None
|
||||||
|
tickers_resolved: bool = False # True when ``slice`` on each position
|
||||||
|
# is already a Yahoo-ready ticker
|
||||||
|
# (LLM path). False (default) means
|
||||||
|
# tickers must still be resolved via
|
||||||
|
# the T212 InstrumentMap.
|
||||||
|
|
||||||
|
|
||||||
def _normalise_header(h: str) -> str:
|
def _normalise_header(h: str) -> str:
|
||||||
|
|
|
||||||
|
|
@ -250,6 +250,7 @@ def _apply_mapping(
|
||||||
qty_col = mapping["qty_col"]
|
qty_col = mapping["qty_col"]
|
||||||
name_col = mapping.get("name_col")
|
name_col = mapping.get("name_col")
|
||||||
cost_col = mapping.get("cost_col")
|
cost_col = mapping.get("cost_col")
|
||||||
|
currency_col = mapping.get("currency_col")
|
||||||
|
|
||||||
positions: list[ParsedPosition] = []
|
positions: list[ParsedPosition] = []
|
||||||
invested_total = 0.0
|
invested_total = 0.0
|
||||||
|
|
@ -279,6 +280,9 @@ def _apply_mapping(
|
||||||
name = row[idx[name_col]].strip()
|
name = row[idx[name_col]].strip()
|
||||||
if not name:
|
if not name:
|
||||||
name = ticker
|
name = ticker
|
||||||
|
currency: str | None = None
|
||||||
|
if currency_col is not None and idx[currency_col] < len(row):
|
||||||
|
currency = row[idx[currency_col]].strip() or None
|
||||||
positions.append(ParsedPosition(
|
positions.append(ParsedPosition(
|
||||||
slice=ticker,
|
slice=ticker,
|
||||||
name=name,
|
name=name,
|
||||||
|
|
@ -286,6 +290,7 @@ def _apply_mapping(
|
||||||
current_value=None,
|
current_value=None,
|
||||||
result=None,
|
result=None,
|
||||||
quantity=qty,
|
quantity=qty,
|
||||||
|
currency=currency,
|
||||||
))
|
))
|
||||||
|
|
||||||
return ParsedPie(
|
return ParsedPie(
|
||||||
|
|
@ -294,6 +299,7 @@ def _apply_mapping(
|
||||||
invested=(invested_total if invested_seen else None),
|
invested=(invested_total if invested_seen else None),
|
||||||
value=None,
|
value=None,
|
||||||
result=None,
|
result=None,
|
||||||
|
tickers_resolved=True,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -494,6 +494,20 @@ async def test_parse_portfolio_route_falls_through_to_llm(tmp_path, monkeypatch)
|
||||||
)
|
)
|
||||||
monkeypatch.setattr(market_mod, "fetch", _fake_fetch)
|
monkeypatch.setattr(market_mod, "fetch", _fake_fetch)
|
||||||
|
|
||||||
|
# ticker_universe.upsert_tickers uses MySQL ON DUPLICATE KEY UPDATE
|
||||||
|
# which SQLite doesn't compile. Mock the two universe-side effects;
|
||||||
|
# neither contributes to the JSON contract we're testing here.
|
||||||
|
from app.services import ticker_universe as tu_mod
|
||||||
|
|
||||||
|
async def _fake_upsert(session, tickers):
|
||||||
|
return len(list(tickers))
|
||||||
|
|
||||||
|
async def _fake_buffer(tickers):
|
||||||
|
return len(list(tickers))
|
||||||
|
|
||||||
|
monkeypatch.setattr(tu_mod, "upsert_tickers", _fake_upsert)
|
||||||
|
monkeypatch.setattr(tu_mod, "buffer_tickers", _fake_buffer)
|
||||||
|
|
||||||
raw = open("tests/fixtures/ibkr_sample.csv", "rb").read()
|
raw = open("tests/fixtures/ibkr_sample.csv", "rb").read()
|
||||||
upload = UploadFile(filename="ibkr.csv", file=BytesIO(raw))
|
upload = UploadFile(filename="ibkr.csv", file=BytesIO(raw))
|
||||||
|
|
||||||
|
|
@ -502,12 +516,18 @@ async def test_parse_portfolio_route_falls_through_to_llm(tmp_path, monkeypatch)
|
||||||
result = await parse_portfolio(file=upload, session=session)
|
result = await parse_portfolio(file=upload, session=session)
|
||||||
|
|
||||||
assert result["base_currency"] == "GBP"
|
assert result["base_currency"] == "GBP"
|
||||||
# At least the AAPL/MSFT/NVDA rows should be present; resolve_slice may
|
# All 5 IBKR positions should round-trip — the LLM path trusts the
|
||||||
# filter some if there's no InstrumentMap row, which is fine for this
|
# Yahoo-ready tickers from the file and does NOT drop on a
|
||||||
# test — we just want to confirm the LLM fallback ran end-to-end.
|
# resolve_slice miss (that's the T212 path's behaviour).
|
||||||
assert isinstance(result["positions"], list)
|
tickers = {p["yahoo_ticker"] for p in result["positions"]}
|
||||||
|
assert tickers == {"AAPL", "MSFT", "NVDA", "VOD.L", "ASML.AS"}
|
||||||
# LLM was called exactly once (cache miss).
|
# LLM was called exactly once (cache miss).
|
||||||
assert mod.call_llm.await_count == 1
|
assert mod.call_llm.await_count == 1
|
||||||
|
# Currency comes from the LLM-mapped currency_col, falling back to
|
||||||
|
# USD only when neither InstrumentMap nor the file specified one.
|
||||||
|
by_t = {p["yahoo_ticker"]: p["currency"] for p in result["positions"]}
|
||||||
|
assert by_t["VOD.L"] == "GBP"
|
||||||
|
assert by_t["ASML.AS"] == "EUR"
|
||||||
|
|
||||||
|
|
||||||
def test_parse_portfolio_route_requires_paid():
|
def test_parse_portfolio_route_requires_paid():
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue