From 0ed2cfdff361ef43263411a8703bafbe649887a1 Mon Sep 17 00:00:00 2001 From: Jeroen Grusewski Date: Mon, 20 Jul 2026 09:20:47 +0000 Subject: [PATCH] fix(exec): UCITS live-run 478/10349 + false-EXECUTED (Mon 2026-07-20) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first live UCITS paper run on bizworx reported EXECUTED/exit-0 but was degraded: the IEF->CBU0 leg got 0 fills and DBMF hung Submitted. - Error 478 (contract conflict): the ISIN-qualified UCITS contract kept a localSymbol/secId conflicting with the pinned conId (requested CBU0 vs canonical CSBGU0). Strip secId/secIdType/localSymbol after resolve and route by conId + SMART. - Error 10349 (TIF preset): the bare MarketOrder had no TIF so the account preset forced DAY + cancel/reconfirm. Set explicit tif="DAY". - False-success (the dangerous one): plan.executed counted Cancelled/Submitted legs as placed, and exec-record booked on any non-blocked run. Now executed requires EVERY leg filled; a degraded run prints DEGRADED, exits 1, and skips exec-track booking so the reconciliation gate stays WAIT (never books a phantom rebalance) — real money errs false-WAIT. Tests: new test_ucits_exec_fill_status.py + degraded cases in test_execution.py and test_execute_multistrat_b2a.py (TDD failing-first). 523 exec/forward/gate tests green, mypy clean. Co-Authored-By: Claude Opus 4.8 --- src/fxhnt/adapters/broker/ibkr.py | 16 ++++ src/fxhnt/application/execution.py | 24 ++++- .../application/multistrat_exec_record.py | 9 +- src/fxhnt/cli.py | 9 +- .../test_execute_multistrat_b2a.py | 26 +++++- tests/integration/test_execution.py | 37 ++++++++ tests/unit/test_ibkr_contract_routing.py | 13 ++- tests/unit/test_ucits_exec_fill_status.py | 87 +++++++++++++++++++ 8 files changed, 214 insertions(+), 7 deletions(-) create mode 100644 tests/unit/test_ucits_exec_fill_status.py diff --git a/src/fxhnt/adapters/broker/ibkr.py b/src/fxhnt/adapters/broker/ibkr.py index b153517..e31b8f1 100644 --- a/src/fxhnt/adapters/broker/ibkr.py +++ b/src/fxhnt/adapters/broker/ibkr.py @@ -204,7 +204,23 @@ class IbkrBroker: # (probed 2026-07-15), so smart-routing the qualified conId fills without the direct-route penalty. # conId pins the exact instrument, so SMART cannot mis-route to the wrong (e.g. US grey-market) line. contract.exchange = "SMART" + # Bug fix (Error 478, live 2026-07-20): the ISIN-qualified UCITS contract can retain a `secId`/ + # `localSymbol` that CONFLICTS with the conId IBKR pinned (e.g. requested localSymbol CBU0 vs the + # canonical CSBGU0 for ISIN IE00B3VWN518) -> "Parameters in request conflicts with contract parameters" + # -> the order is REJECTED and the leg gets 0 fills. The conId alone is unambiguous, so strip the + # ambiguous identity fields after resolution; IBKR resolves purely on conId + SMART. Guarded getattr: + # these fields may be absent on the plain-Stock US path. + conid = getattr(contract, "conId", 0) + if conid: + for _f in ("secId", "secIdType", "localSymbol"): + if getattr(contract, _f, None): + setattr(contract, _f, "") market_order = MarketOrder(order.side, int(round(order.quantity))) # whole shares + # Bug fix (Error 10349, live 2026-07-20): a MarketOrder with NO explicit TIF let the account's order + # preset force TIF=DAY and cancel-then-reconfirm each order (a wasted round-trip; DBMF hung on + # 'Submitted'). Set TIF=DAY explicitly so the order already matches the account preset — no override, + # no cancel. DAY is correct for a market-hours rebalance (the run fires at the UCITS open). + market_order.tif = "DAY" accts = ib.managedAccounts() if accts: market_order.account = accts[0] diff --git a/src/fxhnt/application/execution.py b/src/fxhnt/application/execution.py index f9415d7..052d1cf 100644 --- a/src/fxhnt/application/execution.py +++ b/src/fxhnt/application/execution.py @@ -15,6 +15,16 @@ from fxhnt.domain.portfolio import Book, compute_target_weights from fxhnt.ports.broker import Broker, Order from fxhnt.ports.data import DataProvider +# IBKR order statuses that represent a REAL fill this run. A market order at the UCITS open fills within the +# `place_order` sleep, so the terminal status is 'Filled'. Everything else — 'Cancelled' (Error 478/10349), +# 'Submitted'/'PreSubmitted'/'PendingSubmit' (accepted but not yet filled), 'Inactive' — is NOT a fill and +# marks the run degraded. Compared case-insensitively; 'PartiallyFilled' also counts (some shares executed). +_FILL_STATUSES = {"filled", "partiallyfilled"} + + +def _is_fill(status: str) -> bool: + return str(status).strip().lower() in _FILL_STATUSES + class PlannedOrder(BaseModel): symbol: str @@ -147,6 +157,7 @@ class ExecutionService: elif execute: specs = contract_specs or {} placed = 0 + unfilled: list[str] = [] for o in orders: try: spec = specs.get(o.symbol, {}) @@ -156,9 +167,20 @@ class ExecutionService: sec_id_type=spec.get("sec_id_type"), sec_id=spec.get("sec_id"))) notes.append(f"{o.side} {o.quantity:g} {o.symbol}: {status}") placed += 1 + if not _is_fill(status): + unfilled.append(f"{o.symbol}({status})") except Exception as e: # one bad order must not abort the rest notes.append(f"order failed {o.symbol}: {str(e)[:80]}") - plan.executed = placed > 0 + unfilled.append(f"{o.symbol}(error)") + # Real money errs false-WAIT: the plan is EXECUTED only when EVERY attempted leg actually FILLED. + # A Cancelled/Submitted leg (Error 478/10349, or a broker that raised) is a DEGRADED run — reporting + # EXECUTED there would let the executed forward track + reconciliation gate book a rebalance that + # did not happen (live 2026-07-20: CBU0 Cancelled + DBMF stuck Submitted, yet the run said + # EXECUTED). `placed > 0` is NOT sufficient; all placed legs must be fills. + plan.executed = placed > 0 and not unfilled + if unfilled: + notes.append(f"DEGRADED — {len(unfilled)} leg(s) did not fill: {', '.join(unfilled)} " + f"(NOT reported EXECUTED)") plan.notes = notes pop_trades = getattr(self._broker, "pop_trades", None) # only IbkrBroker exposes this (D2.1) if callable(pop_trades): diff --git a/src/fxhnt/application/multistrat_exec_record.py b/src/fxhnt/application/multistrat_exec_record.py index 095faf7..8f86982 100644 --- a/src/fxhnt/application/multistrat_exec_record.py +++ b/src/fxhnt/application/multistrat_exec_record.py @@ -146,7 +146,14 @@ def plan_and_record(*, repo: Any, exec_svc: Any, within_weights: dict[str, float log.warning("multistrat superseded-line scan failed (skipped)", exc_info=True) plan = exec_svc.rebalance_weights("multistrat", weights, prices, max_gross=max_gross, execute=execute, contract_specs=specs, sizing_capital=envelope) - if execute and not plan.blocked: + # A DEGRADED run (orders attempted but a leg did not fill — Error 478/10349 live 2026-07-20) is NOT + # blocked, but recording it would advance the basis to `positions_after` (the INTENDED target book) that + # the account does NOT actually hold (e.g. the un-filled CBU0 leg), permanently diverging the executed + # track — the SAME failure the blocked-skip guards against, just partial. So skip recording here too and + # let the next CLEAN run re-establish the basis (err false-WAIT: a skipped record keeps the gate WAITing; + # a wrong record silently corrupts it). `degraded` = orders were placed but the plan is not executed. + degraded = bool(plan.orders) and not plan.executed + if execute and not plan.blocked and not degraded: pos_prior = repo.get_book_state("multistrat_exec_pos") exec_return = exec_return_for_run(pos_prior, prices, venue) # positions_after is the INTENDED target book (weights·nlv/price) used as the next-run return basis — diff --git a/src/fxhnt/cli.py b/src/fxhnt/cli.py index 40efcf0..48c2d75 100644 --- a/src/fxhnt/cli.py +++ b/src/fxhnt/cli.py @@ -403,8 +403,15 @@ def execute_multistrat( typer.echo(f" - {n}") for o in plan.orders: typer.echo(f" {o.side} {o.quantity:g} {o.symbol} (~${o.est_value:,.0f})") - status = "BLOCKED" if plan.blocked else ("EXECUTED" if plan.executed else "DRY-RUN") + # A DEGRADED run: execution was requested and orders were attempted, but a leg did not fill (Error + # 478/10349, or a broker raise) so the plan is NOT executed and NOT a clean dry-run. It MUST exit non-zero + # — a silent exit 0 makes the CronJob report Complete, and the executed forward track + reconciliation gate + # would book a rebalance that only partially happened (real money errs false-WAIT). Live 2026-07-20. + degraded = do_execute and bool(plan.orders) and not plan.executed and not plan.blocked + status = "BLOCKED" if plan.blocked else ("EXECUTED" if plan.executed else ("DEGRADED" if degraded else "DRY-RUN")) typer.echo(f"-> {status}") + if degraded: + raise typer.Exit(1) @app.command("execute-bybit") diff --git a/tests/integration/test_execute_multistrat_b2a.py b/tests/integration/test_execute_multistrat_b2a.py index 20bc3fc..47c3a50 100644 --- a/tests/integration/test_execute_multistrat_b2a.py +++ b/tests/integration/test_execute_multistrat_b2a.py @@ -11,17 +11,22 @@ class _FakePlan: self.notes = [] self.nlv = 100_000.0 self.blocked = False + self.executed = True class _FakeExecSvc: - def __init__(self, blocked=False): + def __init__(self, blocked=False, degraded=False): self.last = None self._blocked = blocked + self._degraded = degraded # orders attempted but a leg didn't fill -> executed False def rebalance_weights(self, name, weights, prices, *, max_gross, execute, contract_specs=None, sizing_capital=None): self.last = dict(weights) plan = _FakePlan() plan.blocked = self._blocked + if self._degraded: + plan.orders = [object()] # an order WAS placed + plan.executed = False # ...but it did not fill return plan @@ -77,3 +82,22 @@ def test_blocked_plan_records_nothing(tmp_path): assert repo.get_book_state("multistrat_exec_pos") == {} # unchanged — still empty assert repo.nav_history("multistrat_exec") == [] # no forward_record row assert not any(s.strategy_id == "multistrat_exec" for s in repo.all_summaries()) + + +def test_degraded_plan_records_nothing(tmp_path): + """A DEGRADED plan (orders placed but a leg did NOT fill — Error 478/10349 live 2026-07-20) must NOT be + recorded: recording would advance the basis to the INTENDED target book, which the account does not hold + (the un-filled leg), permanently diverging the executed track. Same skip as a blocked plan — err + false-WAIT.""" + repo = ForwardNavRepo(f"sqlite:///{tmp_path}/d.db") + repo.migrate() + at = dt.datetime(2026, 7, 10, 23, 30) + _seed_alloc(repo, 10_000.0, at) + svc = _FakeExecSvc(degraded=True) + plan, env = plan_and_record(repo=repo, exec_svc=svc, within_weights={"SPY": 0.6, "IEF": 0.4}, + prices={"SPY": 500.0, "IEF": 100.0}, nlv=100_000.0, + today="2026-07-10", at=at, max_gross=2.0, execute=True) + assert plan.executed is False and plan.orders # degraded: placed but unfilled + assert repo.get_book_state("multistrat_exec_pos") == {} # NOT advanced + assert repo.nav_history("multistrat_exec") == [] # no phantom forward_record + assert not any(s.strategy_id == "multistrat_exec" for s in repo.all_summaries()) diff --git a/tests/integration/test_execution.py b/tests/integration/test_execution.py index abd2ec9..adf0203 100644 --- a/tests/integration/test_execution.py +++ b/tests/integration/test_execution.py @@ -82,6 +82,43 @@ def test_inconsistent_account_data_blocks() -> None: assert plan.blocked and not broker.placed +class StatusFakeBroker(FakeBroker): + """A broker that returns a per-symbol status (default 'Filled'), so a test can simulate a leg that was + Cancelled/Submitted (unfilled) the way the live 2026-07-20 UCITS run saw CBU0 Cancelled + DBMF Submitted.""" + def __init__(self, state: AccountState, statuses: dict[str, str]) -> None: + super().__init__(state) + self._statuses = statuses + + def place_order(self, order: Order) -> str: + self.placed.append(order) + return self._statuses.get(order.symbol, "Filled") + + +def test_bug3_unfilled_leg_marks_plan_not_executed() -> None: + """A leg that comes back Cancelled/Submitted (not a fill) must NOT let the plan report EXECUTED — the + executed forward track + reconciliation gate must err false-WAIT, never book a phantom rebalance.""" + broker = StatusFakeBroker( + AccountState(nlv=100_000, cash=100_000, positions={}, is_paper=True), + statuses={"SPY": "Filled", "IEF": "Cancelled"}) # IEF leg fails, like CBU0 live + svc = ExecutionService(FakeData(), broker, _settings()) + plan = svc.rebalance_weights("multistrat", {"SPY": 0.6, "IEF": 0.4}, + {"SPY": 100.0, "IEF": 100.0}, max_gross=1.5, execute=True) + assert len(broker.placed) == 2 # both were attempted + assert not plan.executed # BUT a leg didn't fill -> NOT executed + assert any("Cancelled" in n or "unfilled" in n.lower() or "degraded" in n.lower() for n in plan.notes) + + +def test_bug3_all_filled_still_executes() -> None: + """Control: when every leg fills, the plan reports EXECUTED as before (no regression).""" + broker = StatusFakeBroker( + AccountState(nlv=100_000, cash=100_000, positions={}, is_paper=True), + statuses={"SPY": "Filled", "IEF": "Filled"}) + svc = ExecutionService(FakeData(), broker, _settings()) + plan = svc.rebalance_weights("multistrat", {"SPY": 0.6, "IEF": 0.4}, + {"SPY": 100.0, "IEF": 100.0}, max_gross=1.5, execute=True) + assert plan.executed and len(broker.placed) == 2 + + class FractionalFakeBroker(FakeBroker): supports_fractional = True # Alpaca-style: the execution layer must size fractional shares diff --git a/tests/unit/test_ibkr_contract_routing.py b/tests/unit/test_ibkr_contract_routing.py index 6ef6334..9fc528c 100644 --- a/tests/unit/test_ibkr_contract_routing.py +++ b/tests/unit/test_ibkr_contract_routing.py @@ -73,16 +73,23 @@ def test_us_path_places_stock_smart_usd_unchanged(): def test_ucits_path_places_isin_contract_usd(): - """--venue ucits: an Order carrying ISIN builds a Contract(secIdType=ISIN, secId, currency=USD).""" + """--venue ucits: an Order carrying ISIN resolves the Contract by ISIN, then places pinned by the resolved + conId on SMART. The ambiguous ISIN/localSymbol is STRIPPED before placing (fix for Error 478, 2026-07-20) + — the conId alone is unambiguous, and a residual conflicting identity field gets the order rejected.""" broker = IbkrBroker() broker._ib = _FakeIB(resolves=True) order = Order(symbol="CSPX", side="BUY", quantity=5, exchange="SMART", currency="USD", sec_id_type="ISIN", sec_id="IE00B5BMR087") + # Before placing, the order carried the ISIN (that is how resolution keys the contract). + assert order.sec_id == "IE00B5BMR087" and order.sec_id_type == "ISIN" status = broker.place_order(order) assert status == "Submitted" + # The PLACED contract is pinned by the resolved conId on SMART, with the ambiguous ISIN stripped so IBKR + # cannot raise Error 478 (a residual identity field conflicting with the conId). contract = broker._ib.placed[0][0] - assert contract.secIdType == "ISIN" - assert contract.secId == "IE00B5BMR087" + assert getattr(contract, "conId", 0) == 12345 + assert contract.exchange == "SMART" + assert not getattr(contract, "secId", None) assert contract.currency == "USD" diff --git a/tests/unit/test_ucits_exec_fill_status.py b/tests/unit/test_ucits_exec_fill_status.py new file mode 100644 index 0000000..affa124 --- /dev/null +++ b/tests/unit/test_ucits_exec_fill_status.py @@ -0,0 +1,87 @@ +"""Regression tests for the 3 bugs in the Monday 2026-07-20 live UCITS paper rebalance: + + Bug 1 (Error 478): the ISIN-qualified UCITS contract retained a conflicting `localSymbol`/`secId` so the + order was rejected ('requested ibLocalSymbol CBU0, from contract CSBGU0') and the IEF leg got 0 fills. + Fix: after qualify, route by the resolved `conId` + SMART and STRIP the ambiguous identity fields + (localSymbol / secId / secIdType / symbol) so nothing conflicts with the conId IBKR already pinned. + + Bug 2 (Error 10349): the bare MarketOrder carried no TIF, so the account's order preset forced TIF=DAY and + canceled-then-reconfirmed each order. Fix: set TIF explicitly on the MarketOrder so it matches the preset. + + Bug 3 (false-success): `plan.executed = placed > 0` counted a Cancelled/Submitted (unfilled) order as + 'placed', so a run where a whole leg didn't fill still reported EXECUTED and exited 0 — the executed + forward track + reconciliation gate would book a rebalance that didn't happen. Fix: a leg whose status is + not a fill marks the plan degraded; the plan is executed only if EVERY placed leg actually filled, and the + CLI exits non-zero on a degraded run (errs false-WAIT, never phantom-EXECUTED). +""" +from __future__ import annotations + +from types import SimpleNamespace + +from fxhnt.adapters.broker.ibkr import IbkrBroker +from fxhnt.ports.broker import Order + + +class _FakeIB478: + """Simulates IBKR: qualify populates the canonical localSymbol/conId; placeOrder REJECTS if the contract + still carries a *different* localSymbol than the qualified one (the real Error 478).""" + def __init__(self, *, canonical_local: str = "CSBGU0", con_id: int = 79000139): + self._canon = canonical_local + self._con = con_id + self.placed: list = [] + + def isConnected(self): + return True + + def qualifyContracts(self, contract): + # IBKR fills in the canonical identity for the ISIN. + contract.conId = self._con + contract.localSymbol = self._canon + return [contract] + + def sleep(self, _s): + return None + + def managedAccounts(self): + return ["DU9600528"] + + def placeOrder(self, contract, order): + local = getattr(contract, "localSymbol", "") or "" + secid = getattr(contract, "secId", "") or "" + # Error 478: any residual identity field that isn't the canonical localSymbol conflicts with the conId. + if (local and local != self._canon) or secid: + self.placed.append((contract, order, "Cancelled")) + return SimpleNamespace(orderStatus=SimpleNamespace(status="Cancelled"), order=order, fills=[]) + self.placed.append((contract, order, "Filled")) + return SimpleNamespace(orderStatus=SimpleNamespace(status="Filled"), order=order, fills=[object()]) + + +def test_bug1_ucits_order_routes_by_conid_no_localsymbol_conflict(): + """The ISIN-resolved UCITS order must place clean: after qualify, the placed contract carries the conId + and NO conflicting localSymbol/secId, so IBKR does not raise Error 478.""" + broker = IbkrBroker() + fake = _FakeIB478(canonical_local="CSBGU0", con_id=79000139) + broker._ib = fake + # This is the CBU0 (IEF->Treasury) order that failed live: request localSymbol differs from canonical. + order = Order(symbol="CBU0", side="BUY", quantity=55, + exchange="SMART", currency="USD", sec_id_type="ISIN", sec_id="IE00B3VWN518") + status = broker.place_order(order) + assert status == "Filled" # BEFORE fix: "Cancelled" (Error 478) + contract, _placed_order, _ = fake.placed[0] + assert getattr(contract, "conId", 0) == 79000139 # pinned by conId + assert contract.exchange == "SMART" + assert not getattr(contract, "secId", None) # ambiguous ISIN stripped after resolution + # localSymbol either cleared or equal to canonical — never a conflicting value. + assert getattr(contract, "localSymbol", "") in ("", None, "CSBGU0") + + +def test_bug2_market_order_sets_explicit_tif(): + """The placed MarketOrder must carry an explicit TIF so the account order-preset cannot force one + (Error 10349). We assert the order object has a non-empty tif set by place_order.""" + broker = IbkrBroker() + fake = _FakeIB478() + broker._ib = fake + broker.place_order(Order(symbol="CBU0", side="BUY", quantity=5, + sec_id_type="ISIN", sec_id="IE00B3VWN518")) + _contract, placed_order, _ = fake.placed[0] + assert getattr(placed_order, "tif", "") == "DAY" # explicit; matches the account DAY preset