diff --git a/docs/fiqh-basis.md b/docs/fiqh-basis.md index 44d6288..b932445 100644 --- a/docs/fiqh-basis.md +++ b/docs/fiqh-basis.md @@ -182,6 +182,14 @@ refused BUY costs nothing; a refused SELL strands a position that wanted out. An balance is not evidence the position is gone, and this is the one place in the engine where "unknown is a rejection" would do more harm than the hole it closes. +Beside the rail, one sweep with the same warrant and no number: `sweep_orphan_brackets` +(#668) cancels a resting SELL whose position has left the account. It is not a rail because it +runs on the reconciliation pass rather than before an order, and it is order-driven where every +other sweep is position-driven -- a protective order whose position is gone has no tranche to be +found from. The trigger is the venue's own statement, never keel's ledger: cancelling a stop +over a position that really exists strips a live holding of its only protection, and the ledger +can be stale in exactly that direction. + What remains open at the venue boundary is #666: on a cash account every case above is a rejected order rather than a short, and keel has no cash-account posture check on Coinbase — `verify_cash_account` exists only on the Alpaca adapter. diff --git a/keel/agent.py b/keel/agent.py index 00bb1e1..875ed8e 100644 --- a/keel/agent.py +++ b/keel/agent.py @@ -1648,6 +1648,16 @@ def run_once( # row, so the pass above cannot see it (issue #195). reconcile.reconcile_unbracketed_positions(broker, repo, config, now_ts) + # Then cancel any resting SELL the account can no longer honour (#668). LAST of the + # three, and the order is load-bearing: the pass above heals a tranche that has no + # bracket, and for a tranche whose position left the account out of band it will place + # one -- against nothing. Sweeping afterwards makes this the cycle's final word, so + # nothing re-creates within the same cycle what it just cancelled. (#667's rail 21 stops + # that re-place at source, which makes the churn rare rather than making the order + # optional: the rail refuses to PLACE into an empty holding, and this cancels what is + # already resting. Neither subsumes the other.) + reconcile.sweep_orphan_brackets(broker, repo, now_ts) + # Rail 11's inputs, refreshed BEFORE any entry this cycle. `poll_once` above has already # written the candles, so every product's latest price is readable here -- and it has to # be done now, not after the loop: `guards.check` reads these scalars from inside diff --git a/keel/commands/doctor.py b/keel/commands/doctor.py index 480c136..64438d9 100644 --- a/keel/commands/doctor.py +++ b/keel/commands/doctor.py @@ -813,6 +813,45 @@ def balance_drift_findings(records: dict[str, Any]) -> list[Finding]: ] +def orphan_bracket_findings(records: dict[str, Any]) -> list[Finding]: + """Resting SELLs the orphan sweep cancelled because the account no longer held them (#668). + + The cancel already resolved the ORDER -- there is nothing left at the venue to trigger + against nothing. What it did not resolve is why keel was protecting a position the account + says is gone, and that question outlives the order: the tranche is still open in the ledger, + deliberately, because closing it would book a realized outcome at a price nobody observed. + + WARN. The sweep did the safe thing and did it automatically; this is the record that it had + to, which is a state a human should look at once rather than an ongoing fault. + """ + orphans = sorted((p, r) for p, r in records.items() if isinstance(r, dict)) + if not orphans: + return [ + Finding( + "bracket.orphan", + OK, + "no orphaned protective orders", + "every resting SELL stands against a position the venue confirms", + "-", + ) + ] + described = ", ".join( + f"{product}: order {record.get('order_id')} cancelled, venue held {record.get('held')}" + for product, record in orphans + ) + return [ + Finding( + "bracket.orphan", + WARN, + f"{len(orphans)} product(s) had a protective order with nothing behind it", + f"{described} -- cancelled before the market could trigger it; the tranche is " + "still open in the ledger", + "reconcile the position: an out-of-band sale or transfer, or an exit that " + "already executed at the venue", + ) + ] + + def unbooked_exit_findings( open_positions: list[dict[str, Any]], orders: list[dict[str, Any]] ) -> list[Finding]: @@ -985,6 +1024,7 @@ def gather_findings(repo: Any, config: Any, log_lines: Iterable[str], now_ts: in from keel.commands._products import _default_sim_products from keel.execution import executor as executor_mod from keel.execution import guards + from keel.execution import reconcile as reconcile_mod findings: list[Finding] = [] @@ -1037,6 +1077,12 @@ def gather_findings(repo: Any, config: Any, log_lines: Iterable[str], now_ts: in for key in repo.get_state_keys(executor_mod.BALANCE_DRIFT_PREFIX) } ) + findings += orphan_bracket_findings( + { + key[len(reconcile_mod.ORPHAN_BRACKET_PREFIX) :]: repo.get_state(key) + for key in repo.get_state_keys(reconcile_mod.ORPHAN_BRACKET_PREFIX) + } + ) # #639: modes are POOLED here, unlike the partial-fill sweep above -- the ledger # invariant belongs to `agent._open_tranche`, which writes it for paper and live alike. findings += unbooked_exit_findings(repo.get_open_positions(), repo.get_orders()) diff --git a/keel/execution/reconcile.py b/keel/execution/reconcile.py index 6053dad..7ae6c29 100644 --- a/keel/execution/reconcile.py +++ b/keel/execution/reconcile.py @@ -29,6 +29,14 @@ recorded before this change, which is the gap itself. That rarity changes the priority of auto-remediation, not the validity of recognizing the state. +ORPHANED PROTECTIVE ORDERS (#668) are swept here too, and by a different kind of pass. Every +other reconciliation in this module and in `executor` is POSITION-DRIVEN: it starts from a +tranche and asks what protects it. `sweep_orphan_brackets` runs the other way -- it starts from +keel's own resting SELLs and asks whether the account still holds what each one stands over -- +because a protective order whose position has left the account has no tranche to be walked from, +and nothing else in the engine can reach it. Left alone, the market reverses through the stop and +the venue is asked to sell an asset that is not there. + What is deliberately NOT done here: resizing or amending the bracket when a partially-filled entry leaves it oversized for what is held. The port migration is done and the cancel-and-replace is expressible (`BracketGTC` since #569; `executor._roll_stop` performs @@ -316,6 +324,149 @@ def reconcile_unbracketed_positions( return healed +#: `agent_state` key prefix for a resting SELL this sweep cancelled because the account no +#: longer held the product (#668). Read by `keel doctor`: the cancel resolves the ORDER, but the +#: divergence that produced it -- keel holding a protective order for something the venue says is +#: gone -- is a fact about the deployment that outlives the order and nobody else reports. +ORPHAN_BRACKET_PREFIX = "orphan_bracket:" + + +def sweep_orphan_brackets(broker: Any, repo: Repository, now_ts: int) -> list[int]: + """Cancel every resting SELL for a product the venue says the account no longer holds (#668). + + **The gap this closes is structural, not incidental.** `_clear_resting_bracket` fails closed + before an exit, and `reconcile_unbracketed_positions` heals a position with no bracket. Both + are POSITION-DRIVEN -- they start from a tranche and ask what protects it. A resting SELL + whose position is gone has no tranche to be walked from, so neither can reach it, and nothing + else looks. + + How one appears, since the shape is not obvious on a venue with native brackets: + + * the operator sells or transfers the asset at the venue themselves -- keel's bracket keeps + resting against inventory that left without telling it; + * `scale_out` and `_roll_stop` both cancel before they place, and a process that dies inside + that window can leave a replacement resting with the position already resolved; + * a venue without a native trigger-bracket needs two legs, and a filled target then leaves a + live stop behind (the classic spot-OCO collision -- not reachable on the venue that trades + live today, where `BracketGTC` is one order, and reachable the moment one is not). + + The market then reverses through the stop and the venue sells an asset that is not held. On a + cash account that is a rejection; on a margin-enabled one (#666) it is a short, produced + entirely by a missing cancel -- *bay' ma la yamlik* with nobody in the loop. + + **The VENUE decides, never the ledger, and that asymmetry is a safety property.** Cancelling + a protective order on a position that really exists strips a live holding of its stop, so the + trigger must be the account's own statement about itself. keel's ledger can be stale in the + dangerous direction -- it says zero while the venue holds the position -- and a sweep driven + from it would cancel exactly the brackets that were doing their job. An unreadable balance + therefore cancels NOTHING; the sweep simply did not run for that product this cycle. + + `Balance.total`, not `Balance.available`, for the same reason as #667's clamp: a resting SELL + holds the base it commits, so `available` reads ~0 for precisely the products this sweep is + looking at, and cancelling on it would cancel every bracket keel has ever placed. + + Dust below the venue's own `base_increment` counts as nothing held -- a residue the venue + cannot even express as a size is not a position, and a bracket protecting it is an orphan + with a rounding error attached. + + Returns the local ids it cancelled. **Never raises into the cycle**: an unreachable venue + means the sweep did not run, which is the honest state, and the next cycle retries. The + tranche is deliberately LEFT ALONE -- closing it would book a realized outcome at a price + nobody observed, and this function's warrant covers the order, not the ledger. + """ + cancelled: list[int] = [] + rows = [r for r in _polled_rows(repo) if str(r["side"]).upper() == Side.SELL.value.upper()] + if not rows: + return cancelled + + # One balance read per PRODUCT, not per row: a product with two resting legs asks the same + # question twice otherwise, and this runs on every cycle. + held_by_product: dict[str, Decimal | None] = {} + for row in rows: + product_id = row["product_id"] + if product_id not in held_by_product: + held_by_product[product_id] = _orphan_threshold_held(broker, repo, product_id, now_ts) + held = held_by_product[product_id] + if held is None: + continue + + try: + executor._cancel_at_exchange(broker, repo, row) + except executor.CancelPending: + # The venue took it and settles asynchronously. Not a failure and not a retry: + # `reconcile_open_orders` reads the terminal state next cycle, and this sweep will + # see the row gone from `_polled_rows` when it does. + log_event( + logger, + logging.INFO, + "reconcile.orphan_cancel_pending", + product=product_id, + order_id=row["id"], + ) + continue + except Exception: + # Includes `CancelUnavailable` -- an order the venue refuses to cancel may be one it + # already filled, which is a fact the next status poll will settle. Per-row + # isolation, like every other venue call in this module: one refusal must not + # abandon the remaining orphans. + log_exception( + logger, + "reconcile.orphan_cancel_failed", + product=product_id, + order_id=row["id"], + ) + continue + + repo.update_order(row["id"], status="canceled", updated_at=now_ts) + repo.set_state( + f"{ORPHAN_BRACKET_PREFIX}{product_id}", + {"order_id": row["id"], "held": str(held), "cancelled_at": now_ts}, + ) + cancelled.append(row["id"]) + log_event( + logger, + logging.WARNING, + "reconcile.orphan_bracket_cancelled", + product=product_id, + order_id=row["id"], + held=str(held), + detail=( + "a resting SELL was protecting a position the venue reports the account no " + "longer holds -- cancelled before the market could trigger it against nothing. " + "The tranche is left open deliberately: closing it would book an outcome at a " + "price nobody observed" + ), + ) + + return cancelled + + +def _orphan_threshold_held( + broker: Any, repo: Repository, product_id: str, now_ts: int +) -> Decimal | None: + """The venue's holding when it is small enough to orphan a bracket, else `None`. + + `None` is the DO-NOTHING answer and covers two different situations on purpose: the balance + could not be read (so nothing may be cancelled), and the account holds a real position (so + nothing should be). Collapsing them is safe precisely because the action is the same and the + dangerous mistake -- cancelling a live position's only stop -- is impossible from either. + """ + held = executor._held_base(broker, product_id) + if held is None: + return None + increment = executor._base_increment_for(broker, repo, product_id, now_ts) + if increment is not None and increment > 0: + # Dust the venue cannot express as a size is not a position. Strictly BELOW the + # increment: a holding of exactly one increment is the smallest real position there is, + # and a bracket over it is doing its job. + return held if held < increment else None + # No increment known, so there is no dust threshold to apply and none may be invented -- + # only an affirmative nothing counts. `_base_increment_for` returns None for a venue error + # as well as for a genuinely unknown product, and guessing a floor from either would cancel + # protective orders over a number nobody supplied. + return held if held <= 0 else None + + def _has_resting_bracket(repo: Repository, position: dict[str, Any]) -> bool: """Whether this tranche still has a bracket working at the exchange.""" bracket_id = position.get("bracket_order_id") diff --git a/tests/commands/test_doctor.py b/tests/commands/test_doctor.py index b78f264..c583578 100644 --- a/tests/commands/test_doctor.py +++ b/tests/commands/test_doctor.py @@ -29,6 +29,7 @@ doctor_exit_code, doctor_lines, gather_findings, + orphan_bracket_findings, partial_fill_findings, rail_state_findings, render_json, @@ -582,6 +583,7 @@ def test_gather_findings_covers_every_check_over_a_seeded_db(tmp_path, valid_con "veto.recent", "fill.partial", "balance.drift", + "bracket.orphan", "ledger.unbooked_exit", "data.missing", "data.stale", @@ -834,3 +836,50 @@ def test_gather_findings_surfaces_a_recorded_drift(tmp_path, valid_config_path) (drift,) = [f for f in findings if f.name == "balance.drift"] assert drift.status == "warn" assert "BTC-USD" in drift.detail + + +# -- orphaned protective orders (#668) ----------------------------------------------------------- + + +def test_orphan_bracket_findings_is_ok_when_none_were_swept() -> None: + (finding,) = orphan_bracket_findings({}) + + assert finding.name == "bracket.orphan" + assert finding.status == "ok" + + +def test_orphan_bracket_findings_warns_and_says_the_tranche_is_still_open() -> None: + """The cancel resolved the order. The ledger row it stood over is deliberately untouched. + + An operator reading only "cancelled" would assume the position was closed out; it was not, + and the detail has to say so or the finding is misleading in the direction that matters. + """ + (finding,) = orphan_bracket_findings( + {"BTC-USD": {"order_id": 7, "held": "0", "cancelled_at": NOW}} + ) + + assert finding.status == "warn" + assert "still open in the ledger" in finding.detail + + +def test_orphan_bracket_findings_ignores_a_malformed_record() -> None: + (finding,) = orphan_bracket_findings({"BTC-USD": "not a record"}) + + assert finding.status == "ok" + + +def test_gather_findings_surfaces_a_swept_orphan(tmp_path, valid_config_path) -> None: + """The wiring, keyed on the prefix the sweep actually writes.""" + from keel.execution.reconcile import ORPHAN_BRACKET_PREFIX + + repo = _seeded_repo(tmp_path / "keel.db") + repo.set_state( + f"{ORPHAN_BRACKET_PREFIX}BTC-USD", {"order_id": 7, "held": "0", "cancelled_at": NOW} + ) + config = load_config(valid_config_path) + + findings = gather_findings(repo, config, [], NOW) + + (orphan,) = [f for f in findings if f.name == "bracket.orphan"] + assert orphan.status == "warn" + assert "BTC-USD" in orphan.detail diff --git a/tests/execution/test_orphan_bracket.py b/tests/execution/test_orphan_bracket.py new file mode 100644 index 0000000..84dc8ba --- /dev/null +++ b/tests/execution/test_orphan_bracket.py @@ -0,0 +1,352 @@ +"""#668 -- a resting SELL whose position is gone is cancelled before the market can trigger it. + +`_clear_resting_bracket` fails closed before an exit. `reconcile_unbracketed_positions` heals a +position that has no bracket. Both are POSITION-DRIVEN: they start from a tranche and ask what +protects it. A protective order whose position has left the account has no tranche to be walked +from, so neither can reach it, and before this sweep nothing looked. + +The market then reverses through the stop and the venue is asked to sell an asset that is not +there. On a cash account that is a rejection; on a margin-enabled one (#666) it is a short -- +*bay' ma la yamlik* produced entirely by a missing cancel, with nobody in the loop. + +The trigger is the VENUE's own statement about the account, never keel's ledger, and that +asymmetry is the safety property these tests exist to hold: cancelling a protective order over a +position that really exists strips a live holding of its only stop, and keel's ledger can be +stale in exactly that direction. +""" + +from __future__ import annotations + +from decimal import Decimal + +from keel_broker_api.results import Balance, CancelOutcome, Instrument + +from keel.data.repository import Repository +from keel.execution.reconcile import ORPHAN_BRACKET_PREFIX, sweep_orphan_brackets +from keel.types import Side +from tests.execution.test_reconcile import ( + NOW, + PRODUCT, + repo, # noqa: F401 -- the shared in-memory Repository fixture +) + + +class SweepBroker: + """The three reads the sweep makes, and a cancel that records what it was asked to kill. + + `base` is the account's holding of the base leg as `Balance.total`; `available` is set + independently so a test can model the venue's real answer for a product with a resting + order against it -- `available` zero, the whole position on `hold`. + """ + + def __init__( + self, + *, + base: Decimal | None = Decimal("0"), + available: Decimal | None = None, + increment: Decimal | None = Decimal("0.00000001"), + cancel: object = CancelOutcome.CONFIRMED, + raise_on_balances: bool = False, + ) -> None: + self._base = base + self._available = base if available is None else available + self._increment = increment + self._cancel = cancel + self._raise_on_balances = raise_on_balances + self.cancelled: list[str] = [] + self.get_balances_calls = 0 + + def get_balances(self) -> list[Balance]: + self.get_balances_calls += 1 + if self._raise_on_balances: + raise ConnectionError("simulated balance outage") + balances = [Balance(currency="USD", available=Decimal("1000"), total=Decimal("1000"))] + if self._base is not None: + balances.append( + Balance(currency="BTC", available=self._available, total=self._base) + ) + return balances + + def get_instrument(self, product_id: str) -> Instrument | None: + if self._increment is None: + return None + return Instrument(product_id=product_id, base_increment=self._increment) + + def cancel_order(self, order_id: str) -> object: + self.cancelled.append(order_id) + if isinstance(self._cancel, Exception): + raise self._cancel + return self._cancel + + +def _resting_sell( + repo: Repository, # noqa: F811 + *, + native_id: str = "cb-orphan-1", + status: str = "pending", + product_id: str = PRODUCT, + side: str = Side.SELL.value, +) -> int: + """One resting protective order, as `place_bracket` leaves it.""" + return repo.insert_order( + dict( + mode="live", + product_id=product_id, + side=side, + order_type="market", + qty=Decimal("0.01"), + limit_price=None, + status=status, + fee=None, + expected_fill=Decimal("49000"), + actual_fill=None, + raw_response=f'{{"order_id": "{native_id}"}}', + created_at=NOW - 1000, + updated_at=NOW - 1000, + ) + ) + + +# -- what the sweep cancels ------------------------------------------------------------------- + + +def test_a_resting_sell_over_an_empty_account_is_cancelled(repo): # noqa: F811 + """The whole point: the venue holds nothing, so the protective order protects nothing.""" + order_id = _resting_sell(repo) + broker = SweepBroker(base=Decimal("0")) + + cancelled = sweep_orphan_brackets(broker, repo, NOW) + + assert cancelled == [order_id] + assert broker.cancelled == ["cb-orphan-1"] + assert repo.get_order(order_id)["status"] == "canceled" + + +def test_dust_below_the_base_increment_counts_as_nothing_held(repo): # noqa: F811 + """A residue the venue cannot express as a size is not a position. + + A bracket over it is an orphan with a rounding error attached: the venue would refuse the + order for being under its own minimum, so nothing is being protected either way. + """ + order_id = _resting_sell(repo) + broker = SweepBroker(base=Decimal("0.000000005"), increment=Decimal("0.00000001")) + + assert sweep_orphan_brackets(broker, repo, NOW) == [order_id] + + +def test_exactly_one_increment_is_a_real_position(repo): # noqa: F811 + """The boundary, and it belongs on the side of NOT cancelling. + + One increment is the smallest position the venue can express. A bracket over it is doing its + job, and the comparison must be strictly-below rather than at-or-below. + """ + _resting_sell(repo) + broker = SweepBroker(base=Decimal("0.00000001"), increment=Decimal("0.00000001")) + + assert sweep_orphan_brackets(broker, repo, NOW) == [] + assert broker.cancelled == [] + + +def test_a_partially_filled_bracket_is_swept_too(repo): # noqa: F811 + """`partially_filled` is RESTING (#446) -- its unfilled remainder still works at the venue. + + Sweeping only `pending` would leave exactly the leg that has already begun executing against + a position that is gone. + """ + order_id = _resting_sell(repo, status="partially_filled") + broker = SweepBroker(base=Decimal("0")) + + assert sweep_orphan_brackets(broker, repo, NOW) == [order_id] + + +# -- what the sweep must NOT cancel ----------------------------------------------------------- + + +def test_a_real_position_keeps_its_bracket(repo): # noqa: F811 + """The dangerous mistake, and the one this suite exists to make impossible.""" + _resting_sell(repo) + broker = SweepBroker(base=Decimal("0.01")) + + assert sweep_orphan_brackets(broker, repo, NOW) == [] + assert broker.cancelled == [] + + +def test_a_bracket_holding_its_own_base_keeps_it(repo): # noqa: F811 + """`Balance.total`, not `Balance.available` -- the same trap as #667's clamp. + + A resting SELL HOLDS the base it commits, so `available` reads zero for precisely the + products this sweep looks at. Reading it would cancel every protective order keel has ever + placed, on its first cycle, and call the result a fix. + + Modelled as the venue reports it: `available=0`, `hold=0.01`, `total=0.01`. + """ + _resting_sell(repo) + broker = SweepBroker(base=Decimal("0.01"), available=Decimal("0")) + + assert sweep_orphan_brackets(broker, repo, NOW) == [], ( + "a resting bracket's own hold was read as an empty account -- the sweep is reading " + "`Balance.available`" + ) + + +def test_an_unreadable_balance_cancels_nothing(repo): # noqa: F811 + """Unknown is the do-nothing answer here, not a licence to cancel. + + An unreachable venue means the sweep did not run, which is the honest state; the next cycle + retries. Failing closed in the other direction would strip protection over a network blip. + """ + _resting_sell(repo) + broker = SweepBroker(raise_on_balances=True) + + assert sweep_orphan_brackets(broker, repo, NOW) == [] + assert broker.cancelled == [] + + +def test_a_venue_with_no_account_row_cancels_nothing(repo): # noqa: F811 + """A venue that omits empty accounts is saying nothing, not saying zero. + + Only an affirmative report of a zero holding may cancel. Treating a sparse response as an + empty account would cancel brackets on every venue whose only fault is a terse balance list. + """ + _resting_sell(repo) + broker = SweepBroker(base=None) + + assert sweep_orphan_brackets(broker, repo, NOW) == [] + + +def test_an_unknown_increment_still_cancels_on_an_affirmative_zero(repo): # noqa: F811 + """No increment means no dust threshold -- and none may be invented. + + `_base_increment_for` returns None for a venue error as much as for an unknown product, so a + guessed floor would cancel protective orders over a number nobody supplied. An affirmative + zero needs no threshold to be unambiguous. + """ + order_id = _resting_sell(repo) + broker = SweepBroker(base=Decimal("0"), increment=None) + + assert sweep_orphan_brackets(broker, repo, NOW) == [order_id] + + +def test_an_unknown_increment_does_not_cancel_over_dust(repo): # noqa: F811 + """The other half of the same rule: without an increment, only zero counts.""" + _resting_sell(repo) + broker = SweepBroker(base=Decimal("0.000000005"), increment=None) + + assert sweep_orphan_brackets(broker, repo, NOW) == [] + + +def test_a_resting_buy_is_never_swept(repo): # noqa: F811 + """A resting BUY is not a protective order and holds no base to be orphaned from.""" + _resting_sell(repo, side=Side.BUY.value) + broker = SweepBroker(base=Decimal("0")) + + assert sweep_orphan_brackets(broker, repo, NOW) == [] + + +def test_a_terminal_order_is_never_swept(repo): # noqa: F811 + """Only orders still working at the venue can be cancelled; the rest are history.""" + _resting_sell(repo, status="filled") + broker = SweepBroker(base=Decimal("0")) + + assert sweep_orphan_brackets(broker, repo, NOW) == [] + assert broker.cancelled == [] + + +# -- failure behaviour -------------------------------------------------------------------------- + + +def test_a_refused_cancel_leaves_the_row_alone_and_continues(repo): # noqa: F811 + """A venue that refuses a cancel may have already filled the order. Local state must not lie. + + And the sweep keeps going: one refusal must not abandon the remaining orphans -- the same + per-item isolation every other venue call in this module has. + """ + first = _resting_sell(repo, native_id="cb-refused") + second = _resting_sell(repo, native_id="cb-ok") + + class PartlyRefusing(SweepBroker): + def cancel_order(self, order_id: str) -> object: + self.cancelled.append(order_id) + return CancelOutcome.REFUSED if order_id == "cb-refused" else CancelOutcome.CONFIRMED + + broker = PartlyRefusing(base=Decimal("0")) + cancelled = sweep_orphan_brackets(broker, repo, NOW) + + assert cancelled == [second] + assert repo.get_order(first)["status"] == "pending", ( + "a refused cancel was recorded locally as canceled while it may still be live" + ) + assert broker.cancelled == ["cb-refused", "cb-ok"] + + +def test_an_accepted_but_unsettled_cancel_is_not_recorded_as_done(repo): # noqa: F811 + """The venue took it and settles asynchronously. `reconcile_open_orders` reads the terminal + state next cycle; claiming it here would retire an order that is still working.""" + order_id = _resting_sell(repo) + broker = SweepBroker(base=Decimal("0"), cancel=CancelOutcome.ACCEPTED) + + assert sweep_orphan_brackets(broker, repo, NOW) == [] + assert repo.get_order(order_id)["status"] == "pending" + + +def test_a_raising_cancel_does_not_abort_the_sweep(repo): # noqa: F811 + """Never raises into the cycle. This runs at the top of every run and owns no failure.""" + _resting_sell(repo) + broker = SweepBroker(base=Decimal("0"), cancel=RuntimeError("venue exploded")) + + assert sweep_orphan_brackets(broker, repo, NOW) == [] + + +def test_the_balance_is_read_once_per_product_not_once_per_order(repo): # noqa: F811 + """Two legs on one product ask the venue one question. This runs on every cycle.""" + _resting_sell(repo, native_id="cb-a") + _resting_sell(repo, native_id="cb-b") + broker = SweepBroker(base=Decimal("0")) + + assert len(sweep_orphan_brackets(broker, repo, NOW)) == 2 + assert broker.get_balances_calls == 1 + + +def test_an_empty_book_touches_no_venue_at_all(repo): # noqa: F811 + """No resting SELLs, no balance read. The common case must cost nothing.""" + broker = SweepBroker(base=Decimal("0")) + + assert sweep_orphan_brackets(broker, repo, NOW) == [] + assert broker.get_balances_calls == 0 + + +# -- what it records ---------------------------------------------------------------------------- + + +def test_a_cancelled_orphan_is_recorded_for_doctor(repo): # noqa: F811 + """The cancel resolves the ORDER; it does not resolve why keel was protecting a ghost.""" + order_id = _resting_sell(repo) + + sweep_orphan_brackets(SweepBroker(base=Decimal("0")), repo, NOW) + + record = repo.get_state(f"{ORPHAN_BRACKET_PREFIX}{PRODUCT}") + assert record is not None + assert record["order_id"] == order_id + assert record["held"] == "0" + + +def test_the_tranche_is_left_open_on_purpose(repo): # noqa: F811 + """Closing it would book a realized outcome at a price nobody observed. + + The sweep's warrant covers the ORDER. What the position was worth when it left the account + is a question only an operator can answer, and inventing an answer is worse than the open + row -- `doctor`'s `ledger.unbooked_exit` and `bracket.orphan` both surface it. + """ + _resting_sell(repo) + position_id = repo.open_position( + product_id=PRODUCT, + rule_name="turtle_breakout", + opened_at=NOW - 1000, + qty=Decimal("0.01"), + entry_fill=Decimal("50000"), + entry_fee=Decimal("3"), + ) + + sweep_orphan_brackets(SweepBroker(base=Decimal("0")), repo, NOW) + + assert [p["id"] for p in repo.get_open_positions()] == [position_id] diff --git a/tests/test_agent.py b/tests/test_agent.py index bcd801c..51f2291 100644 --- a/tests/test_agent.py +++ b/tests/test_agent.py @@ -4550,3 +4550,42 @@ def test_a_fully_filled_exit_still_closes_everything_and_clears_the_state( assert len(repo.get_trade_outcomes()) == 1 assert repo.get_state(f"position_rule:{PRODUCT}") is None assert repo.get_state(f"open_stop:{PRODUCT}") is None + + +# -- the orphan-bracket sweep runs, and runs LAST (#668) ---------------------------------------- + + +def test_run_once_sweeps_orphan_brackets_after_the_rebracket_pass(repo, monkeypatch): + """The sweep is wired into the cycle, and its POSITION in the cycle is the point. + + A function nothing calls reports nothing, and no test above this one would notice: every + sweep test drives `sweep_orphan_brackets` directly. This asserts the wiring. + + The order matters as much as the presence. `reconcile_unbracketed_positions` heals a tranche + that has no bracket, and for a tranche whose position left the account out of band it will + PLACE one -- against nothing. Sweeping afterwards makes the cancel the cycle's final word, + so nothing re-creates within the same cycle what the sweep just removed. Running it first + would leave a fresh orphan resting for a full day, and this deployment cycles once per UTC + day. + """ + from keel.execution import reconcile + + calls: list[str] = [] + + def record(name): + def _fn(*args, **kwargs): + calls.append(name) + return [] + + return _fn + + monkeypatch.setattr(reconcile, "reconcile_open_orders", record("open_orders")) + monkeypatch.setattr(reconcile, "reconcile_unbracketed_positions", record("unbracketed")) + monkeypatch.setattr(reconcile, "sweep_orphan_brackets", record("orphan_sweep")) + repo.set_state("kill_switch", False) + + run_once(FakeBroker(), repo, _config(), now_ts=90_000) + + assert calls == ["open_orders", "unbracketed", "orphan_sweep"], ( + f"expected the three reconciliation passes in order, got {calls}" + )