From ee660a99f0111d976477efdc59cd4cfe08887a24 Mon Sep 17 00:00:00 2001 From: Swathi Date: Tue, 21 Apr 2026 00:21:46 +0530 Subject: [PATCH] fix(dhan): paginated statement API + history row shape Discovered while running the first real sync: the Dhan v2 Statement API doesn't match what I originally guessed. Actual shape: GET /trades/{from}/{to}/{page} - Page number is a REQUIRED third path segment (not optional). - Page size is ~20, newest-first. Paginate until empty list. - History rows are aggregated per-order-per-day, so exchangeTradeId='0'. Falling back to orderId so UNIQUE(broker, broker_trade_id) holds. - Timestamps come in two formats: 'YYYY-MM-DD HH:MM:SS' (today's /trades) and 'YYYY-MM-DDTHH:MM:SS' (history pages). Parser handles both. - createTime/updateTime can be literal 'NA' on history rows. - customSymbol on history has spaces ('NIFTY 21 APR 24300 PUT'); underlying extractor strips trailing whitespace. Adds tests/fixtures/dhan/{trades_today,trades_history_page0}.json and tests/test_dhan_mapper.py (10 cases) covering every fix above. Fixtures use synthetic IDs only; no real account data. Verified against a live account: 103 executions over 30 days, 18 round-trip trades reconstructed cleanly. --- khata/adapters/dhan/adapter.py | 13 +- khata/adapters/dhan/client.py | 10 +- khata/adapters/dhan/mapper.py | 29 +++-- tests/fixtures/dhan/trades_history_page0.json | 60 +++++++++ tests/fixtures/dhan/trades_today.json | 62 +++++++++ tests/test_dhan_mapper.py | 123 ++++++++++++++++++ 6 files changed, 282 insertions(+), 15 deletions(-) create mode 100644 tests/fixtures/dhan/trades_history_page0.json create mode 100644 tests/fixtures/dhan/trades_today.json create mode 100644 tests/test_dhan_mapper.py diff --git a/khata/adapters/dhan/adapter.py b/khata/adapters/dhan/adapter.py index 71b2b05..aac1f53 100644 --- a/khata/adapters/dhan/adapter.py +++ b/khata/adapters/dhan/adapter.py @@ -38,16 +38,21 @@ def fetch_trades(self, session: Session, since: datetime) -> list[CanonicalExecu since_date = since.astimezone(UTC).date() rows: list[dict] = [] - # For today only → /trades (cheaper, more fields). if since_date >= today: rows.extend(client.get_trades()) else: - # Statement API for the backfill range, then /trades for today. - # Dhan caps statement queries at ~90 days — chunk if needed. + # Statement API: paginate until empty. Dhan caps history queries at + # ~90 days per call, so chunk date ranges too. cur = since_date while cur < today: chunk_end = min(cur + timedelta(days=89), today - timedelta(days=1)) - rows.extend(client.get_trades_range(cur, chunk_end)) + page = 0 + while page < 100: # safety bound + chunk = client.get_trades_range(cur, chunk_end, page=page) + if not chunk: + break + rows.extend(chunk) + page += 1 cur = chunk_end + timedelta(days=1) rows.extend(client.get_trades()) diff --git a/khata/adapters/dhan/client.py b/khata/adapters/dhan/client.py index 5886bbd..f4dcc90 100644 --- a/khata/adapters/dhan/client.py +++ b/khata/adapters/dhan/client.py @@ -49,9 +49,13 @@ def get_trades(self) -> list[dict]: """Today's executions (trade book).""" return self._get("/trades") - def get_trades_range(self, from_date: date, to_date: date) -> list[dict]: - """Historical trades via Statement API. Dhan convention: YYYY-MM-DD path params.""" - path = f"/trades/{from_date.isoformat()}/{to_date.isoformat()}" + def get_trades_range(self, from_date: date, to_date: date, page: int = 0) -> list[dict]: + """Historical trades via Statement API. + + Path: GET /trades/{YYYY-MM-DD}/{YYYY-MM-DD}/{pageNumber} + Page size is ~20, newest-first. Caller paginates until an empty list. + """ + path = f"/trades/{from_date.isoformat()}/{to_date.isoformat()}/{page}" return self._get(path) def get_positions(self) -> list[dict]: diff --git a/khata/adapters/dhan/mapper.py b/khata/adapters/dhan/mapper.py index 9a08c63..481b454 100644 --- a/khata/adapters/dhan/mapper.py +++ b/khata/adapters/dhan/mapper.py @@ -65,10 +65,15 @@ def _parse_ts(raw: str | None) -> datetime: - if not raw: + """Parse Dhan timestamps. IST-local, no tz suffix. Two formats observed: + - '/trades' (today): 'YYYY-MM-DD HH:MM:SS' + - '/trades/.../{p}' (history): 'YYYY-MM-DDTHH:MM:SS' + The literal string 'NA' appears on some history rows. + """ + if not raw or raw.strip() in ("", "NA"): return datetime.now(UTC) - # Dhan timestamps are IST, "YYYY-MM-DD HH:MM:SS" - dt = datetime.strptime(raw.strip(), "%Y-%m-%d %H:%M:%S").replace(tzinfo=IST) + s = raw.strip().replace("T", " ") + dt = datetime.strptime(s, "%Y-%m-%d %H:%M:%S").replace(tzinfo=IST) return dt.astimezone(UTC) @@ -82,13 +87,14 @@ def _parse_date(raw: str | None) -> date | None: def _underlying_from_symbol(trading_symbol: str | None, custom_symbol: str | None) -> str | None: - """Best-effort underlying extraction. e.g. 'NIFTY25APR25350CE' → 'NIFTY'.""" + """Best-effort underlying extraction. + Handles both 'NIFTY25APR25350CE' and 'NIFTY 21 APR 24300 PUT'. + """ src = custom_symbol or trading_symbol or "" - # Strip digits-and-suffix tail for i, ch in enumerate(src): if ch.isdigit(): - return src[:i] or None - return src or None + return src[:i].strip() or None + return src.strip() or None def map_trade(row: dict, broker: str = "dhan") -> CanonicalExecution: @@ -114,9 +120,16 @@ def map_trade(row: dict, broker: str = "dhan") -> CanonicalExecution: ipft_paise=rupees_to_paise(row.get("ipft")), ) + # History rows return exchangeTradeId='0' (orders are aggregated per-day, not per-fill). + # Fall back to orderId in that case so our UNIQUE(broker, broker_trade_id) holds. + raw_trade_id = str(row.get("exchangeTradeId") or "").strip() + broker_trade_id = ( + raw_trade_id if raw_trade_id and raw_trade_id != "0" else str(row.get("orderId") or "") + ) + return CanonicalExecution( broker=broker, - broker_trade_id=str(row.get("exchangeTradeId") or row.get("orderId") or ""), + broker_trade_id=broker_trade_id, broker_order_id=str(row.get("orderId") or "") or None, symbol=row.get("customSymbol") or row.get("tradingSymbol") or "", underlying=_underlying_from_symbol(row.get("tradingSymbol"), row.get("customSymbol")), diff --git a/tests/fixtures/dhan/trades_history_page0.json b/tests/fixtures/dhan/trades_history_page0.json new file mode 100644 index 0000000..2c60b77 --- /dev/null +++ b/tests/fixtures/dhan/trades_history_page0.json @@ -0,0 +1,60 @@ +[ + { + "dhanClientId": "TEST_CLIENT_001", + "orderId": "TEST_HIST_ORDER_0001", + "exchangeOrderId": "TEST_HIST_EXCH_0001", + "exchangeTradeId": "0", + "transactionType": "BUY", + "exchangeSegment": "NSE_FNO", + "productType": "INTRADAY", + "orderType": null, + "customSymbol": "NIFTY 21 APR 24300 PUT", + "securityId": "00000", + "tradedQuantity": 1755, + "tradedPrice": 189.94, + "isin": "", + "instrument": "OPTIDX", + "sebiTax": 0.33, + "stt": 0.0, + "brokerageCharges": 20.0, + "serviceTax": 24.97, + "exchangeTransactionCharges": 118.43, + "stampDuty": 0.0, + "ipft": 0.0, + "createTime": "NA", + "updateTime": "NA", + "exchangeTime": "2026-04-17T10:48:02", + "drvExpiryDate": "2026-04-21", + "drvOptionType": "PUT", + "drvStrikePrice": 24300.0 + }, + { + "dhanClientId": "TEST_CLIENT_001", + "orderId": "TEST_HIST_ORDER_0002", + "exchangeOrderId": "TEST_HIST_EXCH_0002", + "exchangeTradeId": "0", + "transactionType": "SELL", + "exchangeSegment": "NSE_FNO", + "productType": "INTRADAY", + "orderType": null, + "customSymbol": "BANKNIFTY 30 APR 51000 CALL", + "securityId": "00001", + "tradedQuantity": 30, + "tradedPrice": 142.5, + "isin": "", + "instrument": "OPTIDX", + "sebiTax": 0.01, + "stt": 0.64, + "brokerageCharges": 20.0, + "serviceTax": 3.78, + "exchangeTransactionCharges": 1.89, + "stampDuty": 0.0, + "ipft": 0.0, + "createTime": "NA", + "updateTime": "NA", + "exchangeTime": "2026-04-15T11:22:09", + "drvExpiryDate": "2026-04-30", + "drvOptionType": "CALL", + "drvStrikePrice": 51000.0 + } +] diff --git a/tests/fixtures/dhan/trades_today.json b/tests/fixtures/dhan/trades_today.json new file mode 100644 index 0000000..a328aaf --- /dev/null +++ b/tests/fixtures/dhan/trades_today.json @@ -0,0 +1,62 @@ +[ + { + "dhanClientId": "TEST_CLIENT_001", + "orderId": "TEST_ORDER_0001", + "exchangeOrderId": "TEST_EXCH_ORDER_0001", + "exchangeTradeId": "TEST_EXCH_TRADE_0001", + "transactionType": "BUY", + "exchangeSegment": "NSE_FNO", + "productType": "INTRADAY", + "orderType": "MARKET", + "tradingSymbol": "NIFTY24APR2624300PE", + "customSymbol": null, + "securityId": "00000", + "tradedQuantity": 75, + "tradedPrice": 100.25, + "isin": "", + "instrument": "OPTIDX", + "sebiTax": 0.01, + "stt": 0.0, + "brokerageCharges": 20.0, + "serviceTax": 4.22, + "exchangeTransactionCharges": 3.76, + "stampDuty": 0.15, + "ipft": 0.0, + "createTime": "2026-04-21 09:35:12", + "updateTime": "2026-04-21 09:35:12", + "exchangeTime": "2026-04-21 09:35:12", + "drvExpiryDate": "2026-04-24", + "drvOptionType": "PUT", + "drvStrikePrice": 24300.0 + }, + { + "dhanClientId": "TEST_CLIENT_001", + "orderId": "TEST_ORDER_0002", + "exchangeOrderId": "TEST_EXCH_ORDER_0002", + "exchangeTradeId": "TEST_EXCH_TRADE_0002", + "transactionType": "SELL", + "exchangeSegment": "NSE_FNO", + "productType": "INTRADAY", + "orderType": "MARKET", + "tradingSymbol": "NIFTY24APR2624300PE", + "customSymbol": null, + "securityId": "00000", + "tradedQuantity": 75, + "tradedPrice": 112.5, + "isin": "", + "instrument": "OPTIDX", + "sebiTax": 0.01, + "stt": 4.22, + "brokerageCharges": 20.0, + "serviceTax": 4.22, + "exchangeTransactionCharges": 3.76, + "stampDuty": 0.0, + "ipft": 0.0, + "createTime": "2026-04-21 09:58:47", + "updateTime": "2026-04-21 09:58:47", + "exchangeTime": "2026-04-21 09:58:47", + "drvExpiryDate": "2026-04-24", + "drvOptionType": "PUT", + "drvStrikePrice": 24300.0 + } +] diff --git a/tests/test_dhan_mapper.py b/tests/test_dhan_mapper.py new file mode 100644 index 0000000..fffdb54 --- /dev/null +++ b/tests/test_dhan_mapper.py @@ -0,0 +1,123 @@ +"""Regression tests for the Dhan → canonical mapper. + +Covers the two response shapes observed in the wild: + - /trades (today's book): per-fill, real exchangeTradeId, space-separated timestamps + - /trades/{from}/{to}/{page} (history): aggregated per-order-per-day, + exchangeTradeId='0', ISO-T timestamps, createTime/updateTime='NA' +""" + +from __future__ import annotations + +import json +from datetime import UTC, datetime +from pathlib import Path + +import pytest + +from khata.adapters.dhan.mapper import map_trade +from khata.core.adapter import InstrumentType, OptionType, Side + +FIXTURES = Path(__file__).parent / "fixtures" / "dhan" + + +def _load(name: str) -> list[dict]: + return json.loads((FIXTURES / name).read_text()) + + +# ── today's endpoint (per-fill) ──────────────────────────────────────── +def test_today_uses_exchange_trade_id_as_canonical_id(): + row = _load("trades_today.json")[0] + e = map_trade(row) + assert e.broker_trade_id == "TEST_EXCH_TRADE_0001" + assert e.broker_order_id == "TEST_ORDER_0001" + + +def test_today_parses_space_separated_timestamp_as_ist_to_utc(): + row = _load("trades_today.json")[0] # "2026-04-21 09:35:12" IST + e = map_trade(row) + # 09:35:12 IST = 04:05:12 UTC + assert e.ts == datetime(2026, 4, 21, 4, 5, 12, tzinfo=UTC) + + +def test_today_maps_fields_into_canonical(): + rows = _load("trades_today.json") + buy = map_trade(rows[0]) + sell = map_trade(rows[1]) + + assert buy.side == Side.BUY + assert sell.side == Side.SELL + assert buy.instrument_type == InstrumentType.OPT + assert buy.option_type == OptionType.PE + assert buy.strike_paise == 24300 * 100 + assert buy.qty == 75 + assert buy.price_paise == 10025 # ₹100.25 + assert buy.exchange == "NFO" + assert buy.segment == "NSE_FNO" + + # Fees recorded in paise (rounded from rupees float) + assert buy.fees.brokerage_paise == 2000 # ₹20.00 + assert sell.fees.stt_paise == 422 # ₹4.22 + + +# ── history endpoint (aggregated, shape differs) ─────────────────────── +def test_history_falls_back_to_order_id_when_trade_id_is_zero(): + """Without the fallback the DB's UNIQUE(broker, broker_trade_id) would + collapse every history row to one — this is the critical fix.""" + row = _load("trades_history_page0.json")[0] + assert row["exchangeTradeId"] == "0" + e = map_trade(row) + assert e.broker_trade_id == "TEST_HIST_ORDER_0001" + assert e.broker_order_id == "TEST_HIST_ORDER_0001" + + +def test_history_parses_iso_t_timestamp_as_ist_to_utc(): + row = _load("trades_history_page0.json")[0] # "2026-04-17T10:48:02" IST + e = map_trade(row) + # 10:48:02 IST = 05:18:02 UTC + assert e.ts == datetime(2026, 4, 17, 5, 18, 2, tzinfo=UTC) + + +def test_history_handles_na_literal_in_create_time(): + """History rows set createTime='NA'. Mapper must not crash; exchangeTime + is the authoritative source so we prefer it anyway.""" + row = _load("trades_history_page0.json")[0] + assert row["createTime"] == "NA" + e = map_trade(row) + assert e.ts.tzinfo == UTC # parsed from exchangeTime, not createTime + + +def test_history_custom_symbol_extracts_underlying_without_trailing_space(): + """'NIFTY 21 APR 24300 PUT' must yield 'NIFTY', not 'NIFTY ' (trailing + space would break joins with the positions/quote feed later).""" + row = _load("trades_history_page0.json")[0] + e = map_trade(row) + assert e.underlying == "NIFTY" + assert e.symbol == "NIFTY 21 APR 24300 PUT" + + row2 = _load("trades_history_page0.json")[1] + e2 = map_trade(row2) + assert e2.underlying == "BANKNIFTY" + + +# ── shared behaviours ────────────────────────────────────────────────── +@pytest.mark.parametrize("fixture_name", ["trades_today.json", "trades_history_page0.json"]) +def test_every_fixture_row_produces_a_canonical_execution(fixture_name): + rows = _load(fixture_name) + assert rows, f"fixture {fixture_name} is empty" + for row in rows: + e = map_trade(row) + assert e.broker == "dhan" + assert e.broker_trade_id # never empty — this is the dedup key + assert e.qty > 0 + assert e.price_paise > 0 + assert e.ts.tzinfo is UTC + + +def test_mapper_handles_missing_na_exchange_time(): + """If exchangeTime itself is 'NA', we fall back to now(UTC) not crash.""" + row = _load("trades_today.json")[0].copy() + row["exchangeTime"] = "NA" + row["updateTime"] = "NA" + row["createTime"] = "NA" + e = map_trade(row) + assert e.ts.tzinfo is UTC # defaulted to now, no exception