From f3fbc179b0e63a7ab970fe68976b95f6f245cf28 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 1 Sep 2026 13:35:12 +0000 Subject: [PATCH] fix(kis): collect all parent candidates before child evaluation; boundary case tests Co-authored-by: genichin <4282227+genichin@users.noreply.github.com> --- tests/adapters/broker/test_kis_cancel.py | 142 ++++++++++++++++++++++- trex_core/adapters/broker/kis.py | 106 ++++++++++------- 2 files changed, 201 insertions(+), 47 deletions(-) diff --git a/tests/adapters/broker/test_kis_cancel.py b/tests/adapters/broker/test_kis_cancel.py index 15e70b5..8494f0a 100644 --- a/tests/adapters/broker/test_kis_cancel.py +++ b/tests/adapters/broker/test_kis_cancel.py @@ -7,7 +7,7 @@ pre-seeded, httpx.get faked) and assert the FillStatus mapping. from __future__ import annotations -from datetime import datetime, timedelta, timezone +from datetime import UTC, datetime, timedelta from trex_core.adapters.broker import kis as kis_mod from trex_core.adapters.broker.kis import KISBroker @@ -23,7 +23,7 @@ def _broker(): base_url="https://openapivts.koreainvestment.com:29443", # mock endpoint ) b._token = "tok" - b._token_exp = datetime.now(tz=timezone.utc) + timedelta(hours=1) + b._token_exp = datetime.now(tz=UTC) + timedelta(hours=1) return b @@ -52,6 +52,7 @@ def _row(**kw): "rjct_qty": "0", "ord_dt": "20260608", "ord_gno_brno": "06010", + "pdno": "005930", } base.update(kw) return {"rt_cd": "0", "output1": [base]} @@ -509,3 +510,140 @@ def test_correction_child_wrong_identity_is_not_treated_as_correction(monkeypatc assert fs.state == "accepted", f"expected accepted (unrelated correction), got {fs.state}" raw = fs.raw if isinstance(fs.raw, dict) else {} assert raw.get("reason") != "correction_child_present_unresolved" + + +# ── Regression: parent identity boundary cases (missing / wrong-first / ambiguous) ─ + +def test_parent_missing_identity_fields_returns_unknown(monkeypatch): + """Parent row with missing pdno and ord_dt → treated as identity mismatch (fail-closed).""" + parent_no_identity = { + "odno": ODNO, + "orgn_odno": "", + "ord_qty": "10", + "tot_ccld_qty": "0", + "avg_prvs": "0", + "cncl_yn": "Y", # would be cancelled if accepted + "rjct_qty": "0", + # ord_dt, ord_gno_brno, pdno intentionally absent + } + monkeypatch.setattr( + kis_mod.httpx, "get", _fake_get({"rt_cd": "0", "output1": [parent_no_identity]}) + ) + fs = _broker().poll_fill( + f"06010:{ODNO}", side="buy", symbol="225190", order_date="20260901" + ) + # Missing identity → fail-closed, not terminal + assert fs.state == "unknown", f"expected unknown (missing identity), got {fs.state}" + assert not fs.is_terminal_reject + + +def test_child_cancel_missing_identity_fields_not_accepted_as_cancel(monkeypatch): + """Cancel child with missing identity fields must NOT be accepted as matching child.""" + parent = { + "odno": ODNO, + "orgn_odno": "", + "ord_qty": "10", + "tot_ccld_qty": "0", + "avg_prvs": "0", + "cncl_yn": "", + "rjct_qty": "0", + "ord_dt": "20260901", + "ord_gno_brno": "06010", + "pdno": "225190", + } + child_no_identity = { + "odno": "0000123461", + "orgn_odno": ODNO, + "ord_qty": "10", + "tot_ccld_qty": "0", + "avg_prvs": "0", + "cncl_yn": "Y", + "rjct_qty": "0", + # ord_dt, ord_gno_brno, pdno intentionally absent + } + monkeypatch.setattr( + kis_mod.httpx, + "get", + _fake_get({"rt_cd": "0", "output1": [parent, child_no_identity]}), + ) + fs = _broker().poll_fill( + f"06010:{ODNO}", side="buy", symbol="225190", order_date="20260901" + ) + # Child has missing identity → not a matching cancel; parent has no fill → accepted + assert fs.state == "accepted", f"expected accepted (child missing identity), got {fs.state}" + + +def test_wrong_parent_first_exact_parent_later_finds_cancel(monkeypatch): + """When the wrong-branch parent comes first, the exact parent+cancel child later must win.""" + wrong_parent = { + "odno": ODNO, + "orgn_odno": "", + "ord_qty": "10", + "tot_ccld_qty": "0", + "avg_prvs": "0", + "cncl_yn": "", + "rjct_qty": "0", + "ord_dt": "20260901", + "ord_gno_brno": "99999", # wrong branch — comes first in list + "pdno": "225190", + } + exact_parent = { + "odno": ODNO, + "orgn_odno": "", + "ord_qty": "10", + "tot_ccld_qty": "0", + "avg_prvs": "0", + "cncl_yn": "", + "rjct_qty": "0", + "ord_dt": "20260901", + "ord_gno_brno": "06010", # correct branch — comes second + "pdno": "225190", + } + child = { + "odno": "0000123462", + "orgn_odno": ODNO, + "ord_qty": "10", + "tot_ccld_qty": "0", + "avg_prvs": "0", + "cncl_yn": "Y", + "rjct_qty": "0", + "ord_dt": "20260901", + "ord_gno_brno": "06010", + "pdno": "225190", + } + monkeypatch.setattr( + kis_mod.httpx, + "get", + _fake_get({"rt_cd": "0", "output1": [wrong_parent, exact_parent, child]}), + ) + fs = _broker().poll_fill( + f"06010:{ODNO}", side="buy", symbol="225190", order_date="20260901" + ) + # Must find the exact parent + cancel child even though wrong parent comes first + assert fs.state == "cancelled", f"expected cancelled (exact parent later), got {fs.state}" + + +def test_ambiguous_exact_parents_returns_unknown(monkeypatch): + """Two rows with identical exact identity → ambiguous parent → unknown.""" + parent1 = { + "odno": ODNO, + "orgn_odno": "", + "ord_qty": "10", + "tot_ccld_qty": "0", + "avg_prvs": "0", + "cncl_yn": "", + "rjct_qty": "0", + "ord_dt": "20260901", + "ord_gno_brno": "06010", + "pdno": "225190", + } + parent2 = {**parent1} # exact duplicate + monkeypatch.setattr( + kis_mod.httpx, "get", _fake_get({"rt_cd": "0", "output1": [parent1, parent2]}) + ) + fs = _broker().poll_fill( + f"06010:{ODNO}", side="buy", symbol="225190", order_date="20260901" + ) + assert fs.state == "unknown", f"expected unknown (ambiguous parents), got {fs.state}" + raw = fs.raw if isinstance(fs.raw, dict) else {} + assert raw.get("reason") == "parent_identity_ambiguous" diff --git a/trex_core/adapters/broker/kis.py b/trex_core/adapters/broker/kis.py index d5a7182..32dadcd 100644 --- a/trex_core/adapters/broker/kis.py +++ b/trex_core/adapters/broker/kis.py @@ -418,15 +418,51 @@ class KISBroker: request_brno = broker_order_id.split(":")[0] if ":" in broker_order_id else "" request_date = inquiry_date # == order_date request_symbol = symbol - row = next( - ( - r - for r in rows - if (str(r.get("odno") or "").strip().lstrip("0") or "0") == normalized_odno - ), - None, - ) - if row is None: + + def _row_odno_normalized(r: dict) -> str: + return str(r.get("odno") or "").strip().lstrip("0") or "0" + + def _row_matches_request_identity(r: dict) -> bool: + """True when a row's identity fields are present and agree with the + authoritative request identity. An absent field in the row is + treated as a mismatch (fail-closed), not as a wildcard.""" + row_brno = str(r.get("ord_gno_brno") or "").strip() + row_dt = str(r.get("ord_dt") or "").strip() + row_pdno = str(r.get("pdno") or "").strip() + # brno: required only when request carries one; if request has no + # brno (legacy odno-only format) skip the check to preserve + # backwards compatibility. + if request_brno and (not row_brno or row_brno != request_brno): + return False + # date and symbol: always required — these never legitimately vary + # for the same order on the same inquiry date. + if not row_dt or row_dt != request_date: + return False + return bool(row_pdno) and row_pdno == request_symbol + + # Collect ALL rows whose odno matches. We must then require exactly + # one that also passes the full identity check. Using next() here + # would accept the first odno-match regardless of identity and stop + # the search, missing authoritative rows that come later and ignoring + # the ambiguous-parent edge case. + odno_matches = [ + r for r in rows if _row_odno_normalized(r) == normalized_odno + ] + exact_parent_matches = [r for r in odno_matches if _row_matches_request_identity(r)] + + if len(exact_parent_matches) == 0: + # No row with the exact request identity. If there are odno + # matches they all failed identity validation → unknown (not grace). + if odno_matches: + return FillStatus( + state="unknown", + raw={ + "reason": "parent_identity_mismatch", + "order_visible": True, + "inquiry_date": inquiry_date, + }, + ) + # Truly invisible — apply the submission visibility grace window. visible_grace = ( submitted_at is None or (self._clock().astimezone(UTC) - submitted_at.astimezone(UTC)) @@ -445,40 +481,20 @@ class KISBroker: "inquiry_date": inquiry_date, }, ) - # Validate that the matched parent row's identity agrees with the - # authoritative request identity. KIS can return stale rows for - # reused odno values; a branch, date, or symbol mismatch means this - # row does not belong to our order → fail-closed as unknown. - row_brno = str(row.get("ord_gno_brno") or "").strip() - row_dt = str(row.get("ord_dt") or "").strip() - row_pdno = str(row.get("pdno") or "").strip() - if request_brno and row_brno and row_brno != request_brno: + + if len(exact_parent_matches) > 1: + # More than one row claims the same exact identity — ambiguous. return FillStatus( state="unknown", raw={ - "reason": "parent_identity_mismatch_brno", - "order_visible": True, - "inquiry_date": inquiry_date, - }, - ) - if row_dt and row_dt != request_date: - return FillStatus( - state="unknown", - raw={ - "reason": "parent_identity_mismatch_date", - "order_visible": True, - "inquiry_date": inquiry_date, - }, - ) - if row_pdno and row_pdno != request_symbol: - return FillStatus( - state="unknown", - raw={ - "reason": "parent_identity_mismatch_symbol", + "reason": "parent_identity_ambiguous", "order_visible": True, "inquiry_date": inquiry_date, }, ) + + row = exact_parent_matches[0] + # dcy-013 ext: KIS sometimes records an external cancel as a separate # child row with orgn_odno= and cncl_yn="Y" rather than # setting cncl_yn on the original row. Scan sibling rows to detect @@ -486,24 +502,24 @@ class KISBroker: # # Safety requirements: # - Child must share the AUTHORITATIVE request identity (branch, date, - # symbol from the caller), NOT the row values. Using row values as - # ground truth would let a mismatched row pass its own wrong values - # back as the filter reference. + # symbol from the caller). All identity fields must be present and + # equal — absent fields are NOT treated as wildcards (fail-closed). # - Ambiguous (> 1 matching cancel) → fail-closed "unknown". # - Correction (정정) children (cncl_yn != "Y") that match the same # request identity are also fail-closed → "unknown". - # - Unrelated rows that only share orgn_odno but differ in branch/date/ - # symbol are excluded from both candidate lists. + # - Unrelated rows whose identity fields differ from the request are + # excluded from both candidate lists. def _child_matches_request(r: dict) -> bool: - """True when a child row is bound to the same order identity as the request.""" + """True when a child row's identity fields are all present and + equal to the authoritative request identity. Missing = mismatch.""" child_brno = str(r.get("ord_gno_brno") or "").strip() child_dt = str(r.get("ord_dt") or "").strip() child_pdno = str(r.get("pdno") or "").strip() - if request_brno and child_brno and child_brno != request_brno: + if request_brno and (not child_brno or child_brno != request_brno): return False - if child_dt and child_dt != request_date: + if not child_dt or child_dt != request_date: return False - return not (child_pdno and child_pdno != request_symbol) + return bool(child_pdno) and child_pdno == request_symbol child_cancel_candidates = [ r