fix(execution): bind parent row to request identity before child evaluation; add parent mismatch and race tests
Co-authored-by: genichin <4282227+genichin@users.noreply.github.com>
This commit is contained in:
co-authored by
genichin
parent
65df10542b
commit
584e08eb36
@@ -391,3 +391,121 @@ def test_ambiguous_multiple_child_cancel_rows_return_unknown(monkeypatch):
|
|||||||
assert fs.state == "unknown", f"expected unknown (ambiguous children), got {fs.state}"
|
assert fs.state == "unknown", f"expected unknown (ambiguous children), got {fs.state}"
|
||||||
raw = fs.raw if isinstance(fs.raw, dict) else {}
|
raw = fs.raw if isinstance(fs.raw, dict) else {}
|
||||||
assert raw.get("reason") == "child_cancel_ambiguous"
|
assert raw.get("reason") == "child_cancel_ambiguous"
|
||||||
|
|
||||||
|
|
||||||
|
# ── Regression: parent row identity mismatch with request ────────────────────
|
||||||
|
|
||||||
|
def test_parent_row_wrong_branch_returns_unknown(monkeypatch):
|
||||||
|
"""Parent row with wrong ord_gno_brno (branch) relative to broker_order_id → unknown."""
|
||||||
|
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 — request has 06010
|
||||||
|
"pdno": "225190",
|
||||||
|
}
|
||||||
|
monkeypatch.setattr(
|
||||||
|
kis_mod.httpx, "get", _fake_get({"rt_cd": "0", "output1": [parent]})
|
||||||
|
)
|
||||||
|
# broker_order_id encodes branch 06010, but row has 99999
|
||||||
|
fs = _broker().poll_fill(
|
||||||
|
f"06010:{ODNO}", side="buy", symbol="225190", order_date="20260901"
|
||||||
|
)
|
||||||
|
assert fs.state == "unknown", f"expected unknown (wrong parent branch), got {fs.state}"
|
||||||
|
raw = fs.raw if isinstance(fs.raw, dict) else {}
|
||||||
|
assert "mismatch" in raw.get("reason", "")
|
||||||
|
|
||||||
|
|
||||||
|
def test_parent_row_wrong_date_returns_unknown(monkeypatch):
|
||||||
|
"""Parent row with wrong ord_dt relative to order_date arg → unknown."""
|
||||||
|
parent = {
|
||||||
|
"odno": ODNO,
|
||||||
|
"orgn_odno": "",
|
||||||
|
"ord_qty": "10",
|
||||||
|
"tot_ccld_qty": "0",
|
||||||
|
"avg_prvs": "0",
|
||||||
|
"cncl_yn": "",
|
||||||
|
"rjct_qty": "0",
|
||||||
|
"ord_dt": "20260831", # wrong date — request has 20260901
|
||||||
|
"ord_gno_brno": "06010",
|
||||||
|
"pdno": "225190",
|
||||||
|
}
|
||||||
|
monkeypatch.setattr(
|
||||||
|
kis_mod.httpx, "get", _fake_get({"rt_cd": "0", "output1": [parent]})
|
||||||
|
)
|
||||||
|
fs = _broker().poll_fill(
|
||||||
|
f"06010:{ODNO}", side="buy", symbol="225190", order_date="20260901"
|
||||||
|
)
|
||||||
|
assert fs.state == "unknown", f"expected unknown (wrong parent date), got {fs.state}"
|
||||||
|
raw = fs.raw if isinstance(fs.raw, dict) else {}
|
||||||
|
assert "mismatch" in raw.get("reason", "")
|
||||||
|
|
||||||
|
|
||||||
|
def test_parent_row_wrong_symbol_returns_unknown(monkeypatch):
|
||||||
|
"""Parent row with wrong pdno (symbol) relative to symbol arg → unknown."""
|
||||||
|
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": "000660", # wrong symbol — request has 225190
|
||||||
|
}
|
||||||
|
monkeypatch.setattr(
|
||||||
|
kis_mod.httpx, "get", _fake_get({"rt_cd": "0", "output1": [parent]})
|
||||||
|
)
|
||||||
|
fs = _broker().poll_fill(
|
||||||
|
f"06010:{ODNO}", side="buy", symbol="225190", order_date="20260901"
|
||||||
|
)
|
||||||
|
assert fs.state == "unknown", f"expected unknown (wrong parent symbol), got {fs.state}"
|
||||||
|
raw = fs.raw if isinstance(fs.raw, dict) else {}
|
||||||
|
assert "mismatch" in raw.get("reason", "")
|
||||||
|
|
||||||
|
|
||||||
|
def test_correction_child_wrong_identity_is_not_treated_as_correction(monkeypatch):
|
||||||
|
"""Correction child with wrong branch/date/symbol must not trigger correction_child_present_unresolved."""
|
||||||
|
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",
|
||||||
|
}
|
||||||
|
unrelated_correction = {
|
||||||
|
"odno": "0000199990",
|
||||||
|
"orgn_odno": ODNO,
|
||||||
|
"ord_qty": "10",
|
||||||
|
"tot_ccld_qty": "0",
|
||||||
|
"avg_prvs": "0",
|
||||||
|
"cncl_yn": "N",
|
||||||
|
"rjct_qty": "0",
|
||||||
|
"ord_dt": "20260831", # different date → not related to our request
|
||||||
|
"ord_gno_brno": "99999", # different branch
|
||||||
|
"pdno": "000660", # different symbol
|
||||||
|
}
|
||||||
|
monkeypatch.setattr(
|
||||||
|
kis_mod.httpx,
|
||||||
|
"get",
|
||||||
|
_fake_get({"rt_cd": "0", "output1": [parent, unrelated_correction]}),
|
||||||
|
)
|
||||||
|
fs = _broker().poll_fill(
|
||||||
|
f"06010:{ODNO}", side="buy", symbol="225190", order_date="20260901"
|
||||||
|
)
|
||||||
|
# Unrelated correction must not cause correction_child_present_unresolved
|
||||||
|
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"
|
||||||
|
|||||||
@@ -225,3 +225,69 @@ def test_timeout_cancel_first_attempt_with_retry_before_none():
|
|||||||
)
|
)
|
||||||
execution.run_once() # should trigger timeout cancel
|
execution.run_once() # should trigger timeout cancel
|
||||||
assert broker.cancel_calls == 1
|
assert broker.cancel_calls == 1
|
||||||
|
|
||||||
|
|
||||||
|
# ── Regression: concurrent timeout/operator cancel race — broker call count 1 ─
|
||||||
|
|
||||||
|
def test_timeout_operator_race_broker_called_exactly_once():
|
||||||
|
"""Timeout cancel loop and operator cancel_pending_slot race to the same first
|
||||||
|
reservation. The repository CAS guarantees exactly one winner; broker must
|
||||||
|
receive cancel_order exactly once regardless of scheduling order.
|
||||||
|
|
||||||
|
Uses a threading barrier so both paths enter reserve_cancel_attempt
|
||||||
|
simultaneously, making the race deterministic.
|
||||||
|
"""
|
||||||
|
import threading
|
||||||
|
|
||||||
|
broker = ProgrammableBroker()
|
||||||
|
repo, _, intents, execution, _, _ = _wire(broker)
|
||||||
|
sid = _buy_to_holding(intents, execution)
|
||||||
|
intents.submit_sell(SellIntent(intent_id="s1", slot_id=sid))
|
||||||
|
broker.responder = lambda side: FillStatus(
|
||||||
|
state="accepted",
|
||||||
|
raw={"order_open_confirmed": True},
|
||||||
|
)
|
||||||
|
execution.run_once() # dispatch sell
|
||||||
|
|
||||||
|
# Backdate so timeout_cancel fires in the next run_once()
|
||||||
|
s = repo.fetch_slot(sid)
|
||||||
|
repo._slots[sid] = replace(
|
||||||
|
s,
|
||||||
|
order_submitted_at=datetime.now(tz=timezone.utc) - timedelta(seconds=120),
|
||||||
|
order_date="20260901",
|
||||||
|
timeout_sec=30,
|
||||||
|
)
|
||||||
|
|
||||||
|
barrier = threading.Barrier(2)
|
||||||
|
original_reserve = repo.reserve_cancel_attempt
|
||||||
|
|
||||||
|
def _barrier_reserve(slot_id, broker_order_id, *, retry_before=None):
|
||||||
|
barrier.wait(timeout=5)
|
||||||
|
return original_reserve(slot_id, broker_order_id, retry_before=retry_before)
|
||||||
|
|
||||||
|
repo.reserve_cancel_attempt = _barrier_reserve
|
||||||
|
|
||||||
|
errors = []
|
||||||
|
|
||||||
|
def _operator():
|
||||||
|
try:
|
||||||
|
execution.cancel_pending_slot(sid)
|
||||||
|
except Exception as exc: # noqa: BLE001
|
||||||
|
errors.append(exc)
|
||||||
|
|
||||||
|
t = threading.Thread(target=_operator)
|
||||||
|
t.start()
|
||||||
|
|
||||||
|
# Execution loop's timeout cancel is the second concurrent caller.
|
||||||
|
try:
|
||||||
|
execution.run_once()
|
||||||
|
except Exception as exc: # noqa: BLE001
|
||||||
|
errors.append(exc)
|
||||||
|
|
||||||
|
t.join(timeout=5)
|
||||||
|
assert not errors, f"thread errors: {errors}"
|
||||||
|
|
||||||
|
# CAS ensures exactly one winner — broker cancel must be called once.
|
||||||
|
assert broker.cancel_calls == 1, (
|
||||||
|
f"expected exactly 1 broker cancel call, got {broker.cancel_calls}"
|
||||||
|
)
|
||||||
|
|||||||
@@ -413,6 +413,11 @@ class KISBroker:
|
|||||||
self._fill_poll_not_before[broker_order_id] = monotonic() + _FILL_POLL_MIN_INTERVAL_SECONDS
|
self._fill_poll_not_before[broker_order_id] = monotonic() + _FILL_POLL_MIN_INTERVAL_SECONDS
|
||||||
rows = payload.get("output1") or []
|
rows = payload.get("output1") or []
|
||||||
normalized_odno = odno.lstrip("0") or "0"
|
normalized_odno = odno.lstrip("0") or "0"
|
||||||
|
# Canonical request identity from the caller — these are authoritative
|
||||||
|
# and must not be overridden by whatever the response row contains.
|
||||||
|
request_brno = broker_order_id.split(":")[0] if ":" in broker_order_id else ""
|
||||||
|
request_date = inquiry_date # == order_date
|
||||||
|
request_symbol = symbol
|
||||||
row = next(
|
row = next(
|
||||||
(
|
(
|
||||||
r
|
r
|
||||||
@@ -440,40 +445,81 @@ class KISBroker:
|
|||||||
"inquiry_date": inquiry_date,
|
"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:
|
||||||
|
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",
|
||||||
|
"order_visible": True,
|
||||||
|
"inquiry_date": inquiry_date,
|
||||||
|
},
|
||||||
|
)
|
||||||
# dcy-013 ext: KIS sometimes records an external cancel as a separate
|
# dcy-013 ext: KIS sometimes records an external cancel as a separate
|
||||||
# child row with orgn_odno=<original> and cncl_yn="Y" rather than
|
# child row with orgn_odno=<original> and cncl_yn="Y" rather than
|
||||||
# setting cncl_yn on the original row. Scan sibling rows to detect
|
# setting cncl_yn on the original row. Scan sibling rows to detect
|
||||||
# this case.
|
# this case.
|
||||||
#
|
#
|
||||||
# Safety requirements:
|
# Safety requirements:
|
||||||
# - Child must share the parent's canonical identity:
|
# - Child must share the AUTHORITATIVE request identity (branch, date,
|
||||||
# ord_gno_brno (branch), ord_dt (date), pdno (symbol).
|
# symbol from the caller), NOT the row values. Using row values as
|
||||||
# Mismatched or ambiguous children → fail-closed as "unknown".
|
# ground truth would let a mismatched row pass its own wrong values
|
||||||
# - Correction (정정) children have cncl_yn != "Y"; they are not
|
# back as the filter reference.
|
||||||
# treated as cancels here. If a correction child is present with
|
# - Ambiguous (> 1 matching cancel) → fail-closed "unknown".
|
||||||
# a new odno the evidence is ambiguous → "unknown" so the slot is
|
# - Correction (정정) children (cncl_yn != "Y") that match the same
|
||||||
# not incorrectly unlocked.
|
# request identity are also fail-closed → "unknown".
|
||||||
parent_brno = str(row.get("ord_gno_brno") or "").strip()
|
# - Unrelated rows that only share orgn_odno but differ in branch/date/
|
||||||
parent_dt = str(row.get("ord_dt") or "").strip()
|
# symbol are excluded from both candidate lists.
|
||||||
parent_pdno = str(row.get("pdno") or "").strip()
|
def _child_matches_request(r: dict) -> bool:
|
||||||
|
"""True when a child row is bound to the same order identity as the request."""
|
||||||
|
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:
|
||||||
|
return False
|
||||||
|
if child_dt and child_dt != request_date:
|
||||||
|
return False
|
||||||
|
return not (child_pdno and child_pdno != request_symbol)
|
||||||
|
|
||||||
child_cancel_candidates = [
|
child_cancel_candidates = [
|
||||||
r
|
r
|
||||||
for r in rows
|
for r in rows
|
||||||
if (str(r.get("orgn_odno") or "").strip().lstrip("0") or "0") == normalized_odno
|
if (str(r.get("orgn_odno") or "").strip().lstrip("0") or "0") == normalized_odno
|
||||||
and str(r.get("cncl_yn") or "").strip().upper() == "Y"
|
and str(r.get("cncl_yn") or "").strip().upper() == "Y"
|
||||||
and str(r.get("ord_gno_brno") or "").strip() == parent_brno
|
and _child_matches_request(r)
|
||||||
and str(r.get("ord_dt") or "").strip() == parent_dt
|
|
||||||
and str(r.get("pdno") or "").strip() == parent_pdno
|
|
||||||
]
|
]
|
||||||
|
|
||||||
# Correction children: same orgn_odno but cncl_yn != "Y". If present,
|
# Correction children: same orgn_odno + matching identity but cncl_yn != "Y".
|
||||||
# the situation is ambiguous — a replacement order may be live.
|
|
||||||
correction_candidates = [
|
correction_candidates = [
|
||||||
r
|
r
|
||||||
for r in rows
|
for r in rows
|
||||||
if (str(r.get("orgn_odno") or "").strip().lstrip("0") or "0") == normalized_odno
|
if (str(r.get("orgn_odno") or "").strip().lstrip("0") or "0") == normalized_odno
|
||||||
and str(r.get("cncl_yn") or "").strip().upper() != "Y"
|
and str(r.get("cncl_yn") or "").strip().upper() != "Y"
|
||||||
|
and _child_matches_request(r)
|
||||||
]
|
]
|
||||||
|
|
||||||
if len(child_cancel_candidates) > 1:
|
if len(child_cancel_candidates) > 1:
|
||||||
|
|||||||
Reference in New Issue
Block a user