From f588462433cab5d926d4be02c258e0574874f392 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Tue, 1 Sep 2026 07:05:41 -0400 Subject: [PATCH] fix(execution): a SELL is clamped to what the venue holds, and refused when it holds nothing (#667) `_sell_base_size` quantized the exit down to the venue's increment and did nothing else. No SELL path read a base balance -- rail 13's own comment recorded the asymmetry: "SELL intents never fetch a balance (the rail exempts them)". So keel sold the quantity its ledger believed it held, and the ledger runs high in three ways it cannot see from its own records: a venue that takes its taker fee out of the received base leaves 0.9985 where the order said 1.0000; a partial fill leaves less still, and `orders.filled_quantity` is written only when the venue's post-fill status was observable; an operator who moves coins out tells keel nothing at all. On a cash account each is a rejected exit -- a position that wanted out, still in, unprotected. On a margin-enabled account (#666) the venue fills the difference by opening a short. That is *bay' ma la yamlik*, arrived at by arithmetic rather than by anyone's intent, and reachable without a single line of the strategy layer proposing one. TWO MECHANISMS, DELIBERATELY SPLIT. `_clamp_to_held` shrinks an order that is too big for a position that really exists -- down only, never up, never to zero, and unchanged when the holding is unreadable. Rail 21 refuses the one case the clamp will not touch: a venue affirmatively reporting nothing held while the ledger expects something. `_build_intent` already returns None when the LEDGER says zero, so reaching rail 21 at all means the two disagree -- and refusing sells nothing and traps nothing, because there is nothing there to trap. `Balance.total`, NEVER `Balance.available`, and this is the load-bearing line. `available` excludes base committed to resting orders, and keel's own protective bracket commits the ENTIRE position -- so for exactly the products keel is protecting, `available` reads ~0. A clamp on that number would shrink every exit to nothing and rail 21 would then veto it, refusing every stop roll and every exit keel has ever placed. `total` is available + hold, and ownership rather than encumbrance is what the prohibition asks about. It also makes `_roll_stop`'s cancel-then-place ordering irrelevant to the number, which `available` would not. RAIL 21 FAILS OPEN ON UNKNOWN -- the deliberate inverse of rails 12/13/17, and the one place in this engine where "unknown is a rejection" would do more harm than the hole it closes. A refused BUY costs nothing; a refused SELL strands a position that wanted out. `_sell_base_size`'s asymmetry table, applied to a balance instead of an increment. Do not fix this into consistency. This is NOT the auto-resize `_record_observed_fill_quantity` declines to do, and that refusal is well argued and still stands unchanged. What it refuses is CANCELLING A PROTECTIVE ORDER on the strength of a settling snapshot. Nothing here cancels anything; it only declines to ask for more than is there, which no snapshot ambiguity can make wrong. Also: `filled_quantity` and `average_filled_price` were coupled by one guard that belongs to the price alone, so a venue answering with a filled size and no average price lost the size too -- and the size is the half that measures the drift. The quantity observation now runs first. Drift is recorded under `balance_drift:` and surfaced by `keel doctor` as `balance.drift` (WARN, not FAIL -- every cause is legitimate). A log line alone requires an operator to know to grep for it, and the drift outlives the order that discovered it. 21 mutants, 21 killed. One survivor was worth the exercise: every rail-21 test built its intent by hand, so dropping `available_base=` from the intent the executor actually builds left all of them green. The end-to-end tests through `execute`, `place_bracket`, `scale_out` and `roll_to_break_even` exist because of that survivor. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NzuKAe2RVrPt9acVAWjRyL --- docs/fiqh-basis.md | 39 +- docs/glossary.md | 2 +- docs/go-live-runbook.md | 2 +- docs/launch.md | 2 +- docs/operator-runbook.md | 2 +- keel/commands/doctor.py | 54 +++ keel/execution/executor.py | 166 ++++++++- keel/execution/guards.py | 68 +++- tests/commands/test_doctor.py | 72 ++++ tests/execution/test_executor.py | 25 +- tests/execution/test_sell_clamp.py | 549 +++++++++++++++++++++++++++++ 11 files changed, 961 insertions(+), 20 deletions(-) create mode 100644 tests/execution/test_sell_clamp.py diff --git a/docs/fiqh-basis.md b/docs/fiqh-basis.md index 02342395..44d6288f 100644 --- a/docs/fiqh-basis.md +++ b/docs/fiqh-basis.md @@ -138,10 +138,10 @@ account level is the operator's obligation, listed first in ### The remaining rails — prudential, not fiqh -Nineteen rails exist (1–14, 16, 17, 18, 19, 20 — there is no rail 15). Of these, only rail 17 -encodes a fiqh ruling, and rails 1/18/19 enforce what the screen and the charter admit. The -rest are PRUDENTIAL — risk and discipline, justified by trading evidence, carrying no -religious claim: +Twenty rails exist (1–14, 16–21 — there is no rail 15). Of these, rails 17 and 21 encode a +fiqh ruling, and rails 1/18/19 enforce what the screen and the charter admit. The rest are +PRUDENTIAL — risk and discipline, justified by trading evidence, carrying no religious +claim: | rail | what it does | basis | | --- | --- | --- | @@ -155,6 +155,37 @@ religious claim: | 13, 14 | spend only the settled quote currency; monthly allowance cap | operational safety | | 20 | trade-scope veto on live entries for a venue credential nobody has attested for trading | operational safety | +**Rail 21 is the second fiqh rail, and it was added because the ruling had a hole under it +(#667).** keel's refusal to go short is structural — no rule can express a short and the engine +builds every entry as a BUY — but structure governs what keel DECIDES, not the quantity that +reaches the venue. A SELL was sized from keel's own ledger, and the ledger runs high: a venue +that takes its taker fee out of the received base leaves less than the order said, a partial +fill leaves less still, and an operator who moves coins out of the account tells keel nothing. +Ask a venue for base that is not there and a cash account rejects it — but a margin-enabled one +fills the difference by opening a short. + +That is *bay' ma la yamlik* (بيع ما لا يملك), "do not sell what you do not possess" — Ayub +Ch 6.5.1 (§65.4), with Ch 5.4.2 (§65.11) recording that "short-selling has been prohibited by +almost all scholars" because the subject matter must be "capable of ownership/title, capable of +delivery/possession". It is a more direct anchor than riba for this particular failure, and the +difference is load-bearing: the oversell is impermissible before any interest is charged, so a +riba-framed defence does not reach it at all. + +Two mechanisms, deliberately split. `executor._clamp_to_held` reduces an order that is too big +for a position that really exists — down only, never up. Rail 21 refuses the one case the clamp +will not touch: a venue that affirmatively reports holding nothing while the ledger expects +something. Neither cancels a protective order, which is why +`_record_observed_fill_quantity`'s refusal to auto-resize still stands beside them unchanged. + +⚠️ The rail fails **OPEN** on an unknown holding, the deliberate inverse of rails 12/13/17. A +refused BUY costs nothing; a refused SELL strands a position that wanted out. An unreadable +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. + +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. + Beside the rails — not among them, and not numbered — sits one routing-time check with the same prudential character: the **max-spread entry gate** (#350, `keel/execution/executor.py`) refuses a live BUY whose previewed book shows `(best_ask − best_bid) / mid` at or beyond diff --git a/docs/glossary.md b/docs/glossary.md index 70fb3609..27a7c30f 100644 --- a/docs/glossary.md +++ b/docs/glossary.md @@ -23,7 +23,7 @@ Each entry is a `## term` heading, a definition, and a `Source:` line. ## rail One of keel's numbered hard guards that every order passes through -- spend caps, drawdown -breakers, the allowlist, settlement-currency, spot-shape and trade-scope checks. Nineteen exist +breakers, the allowlist, settlement-currency, spot-shape and trade-scope checks. Twenty exist (1-14, 16, 17, 18, 19, 20 -- there is no rail 15); each is un-overridable and audit-logged. Source: keel's own vocabulary -- docs/fiqh-basis.md's rails table (the prudential rails 2-14, 16, 20) plus its prose sections for rails 1, 17, 18 and 19 diff --git a/docs/go-live-runbook.md b/docs/go-live-runbook.md index 0e760064..5be78691 100644 --- a/docs/go-live-runbook.md +++ b/docs/go-live-runbook.md @@ -138,7 +138,7 @@ exists to accumulate the live evidence the promotion gate demands and cannot its Demoting these rules would end that experiment without putting anything in its place. **What bounds the risk instead.** Not the promotion gate, which never ran. The caps -(`max_exposure_usd` 200 total at once, `max_per_order_usd` 100), the nineteen un-overridable +(`max_exposure_usd` 200 total at once, `max_per_order_usd` 100), the twenty un-overridable `guards.py` rails, rail 1's allowlist, and rail 14's monthly allowance. **The bypass is of the evidence gate, not the safety rails** — separate mechanisms, and only the first was skipped. diff --git a/docs/launch.md b/docs/launch.md index 02747e76..7a2c9d3c 100644 --- a/docs/launch.md +++ b/docs/launch.md @@ -72,7 +72,7 @@ the Arabic README. > I built an open-source Shariah-compliance **engine** for spot crypto trading — not a > trading bot with a halal coat of paint, but the compliance machinery: allowlist > admission where Shariah classifications are attested with a source and never inferred -> from market data, a fails-closed screen, nineteen un-overridable safety rails including +> from market data, a fails-closed screen, twenty un-overridable safety rails including > §65.4 *qabd* (constructive possession) encoded as an executable check, and an > audit trail of who attested what. > diff --git a/docs/operator-runbook.md b/docs/operator-runbook.md index 63cce7e9..d80b10e2 100644 --- a/docs/operator-runbook.md +++ b/docs/operator-runbook.md @@ -532,7 +532,7 @@ half it can: The gate is BUY-only (exits must execute — the same principle that makes rail 17 halt entries, not exits), **fails closed** (a live BUY whose preview carries no readable bid/ask is refused -with a distinct `book_unreadable` reason, never guessed past), and lives beside the nineteen +with a distinct `book_unreadable` reason, never guessed past), and lives beside the twenty rails rather than among them: `guards.check` is broker-less by design, and the book exists only in the preview the executor just fetched. diff --git a/keel/commands/doctor.py b/keel/commands/doctor.py index 4dbd77f4..480c136e 100644 --- a/keel/commands/doctor.py +++ b/keel/commands/doctor.py @@ -768,6 +768,51 @@ def partial_fill_findings(orders: list[dict[str, Any]]) -> list[Finding]: +def balance_drift_findings(records: dict[str, Any]) -> list[Finding]: + """Products where the venue held less base than keel's ledger expected (#667). + + Written by `executor._clamp_to_held` whenever a SELL had to be reduced. The clamp already + kept the order honest -- keel asked for what was there, not what it remembered -- so this is + not a report of an order that went wrong. It is a report that the BOOKS and the ACCOUNT + disagree, which the clamp handles per-order and nobody reconciles. + + WARN, not FAIL, and for a specific reason: every cause is legitimate. A venue took its fee + out of the base leg, a fill came in short, or the operator moved coins. None of those is a + fault in the deployment; all of them make the ledger's idea of the position wrong until a + human decides which it was. A FAIL would demand action on a state that may be entirely + intended. + + Surfaced HERE rather than left to the log line the clamp also writes, because the drift + outlives the order that discovered it: the next exit will be clamped by the same amount, and + an operator who never greps for `executor.sell_clamped_to_held` would never learn why. + """ + drifts = sorted((p, r) for p, r in records.items() if isinstance(r, dict)) + if not drifts: + return [ + Finding( + "balance.drift", + OK, + "no ledger/venue divergence recorded", + "every SELL went out at the quantity the ledger expected", + "-", + ) + ] + described = ", ".join( + f"{product}: ledger {record.get('ordered')} vs venue {record.get('held')} " + f"(short {record.get('drift')})" + for product, record in drifts + ) + return [ + Finding( + "balance.drift", + WARN, + f"{len(drifts)} product(s) held less than the ledger expected", + f"{described} -- the SELL was clamped to the held quantity, so nothing oversold", + "reconcile the position: a base-leg fee, a short fill, or an out-of-band transfer", + ) + ] + + def unbooked_exit_findings( open_positions: list[dict[str, Any]], orders: list[dict[str, Any]] ) -> list[Finding]: @@ -938,6 +983,7 @@ def gather_findings(repo: Any, config: Any, log_lines: Iterable[str], now_ts: in from keel import agent from keel.commands import fetch from keel.commands._products import _default_sim_products + from keel.execution import executor as executor_mod from keel.execution import guards findings: list[Finding] = [] @@ -983,6 +1029,14 @@ def gather_findings(repo: Any, config: Any, log_lines: Iterable[str], now_ts: in findings += veto_findings(log_lines, since_ts=now_ts - 7 * 86_400) # A repo read, like every other check -- pinned read-only by the same change-counter test. findings += partial_fill_findings(repo.get_orders(mode="live")) + # Read here rather than inside the finding, so the finding stays a pure function of data + # like every other one in this module and the read stays where the repo already is. + findings += balance_drift_findings( + { + key[len(executor_mod.BALANCE_DRIFT_PREFIX) :]: repo.get_state(key) + for key in repo.get_state_keys(executor_mod.BALANCE_DRIFT_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/executor.py b/keel/execution/executor.py index 5a831d11..5e420438 100644 --- a/keel/execution/executor.py +++ b/keel/execution/executor.py @@ -122,7 +122,7 @@ coerce_cancel_outcome, ) from keel_core.credential_identity import current_credential_fingerprint -from keel_core.products import quote_currency_of +from keel_core.products import parse_spot_product_id, quote_currency_of from keel_core.telemetry import current_venue, log_event, log_exception, log_venue_failure from keel_core.trade_scope import TradeScopeState, VenueTradeScope @@ -478,6 +478,127 @@ def _fetch_available_quote(broker: Any, quote_currency: str | None) -> Decimal | return None +#: `agent_state` key prefix for a recorded divergence between keel's ledger and the venue's own +#: holding (#667). Written whenever a SELL is clamped, read by `keel doctor` -- the drift is a +#: fact about the ACCOUNT, so it outlives the order that discovered it and must not live only in +#: a log line an operator has to know to grep for. +BALANCE_DRIFT_PREFIX = "balance_drift:" + + +def _held_base(broker: Any, product_id: str) -> Decimal | None: + """What the venue says the account HOLDS of `product_id`'s base leg, or `None` if unknown. + + ⚠️ **`Balance.total`, never `Balance.available`, and this is the load-bearing line of #667.** + `available` excludes base committed to resting orders, and keel's own protective bracket + commits the ENTIRE position -- so for exactly the products keel is protecting, `available` + reads ~0. Clamping or vetoing on that number would refuse every stop roll and every exit + keel has ever placed, which is the one outcome worse than the oversell being fixed here. + `total` is `available + hold` on the venue that trades live (see the coinbase adapter), and + ownership -- not encumbrance -- is what *bay' ma la yamlik* asks about. + + The STRICT parse, unlike `guards._asset`: this string is matched against the venue's own + currency code, and a loose reduction of a futures id (`ADA-28AUG26-CDE` -> `ADA`) would match + a real balance for an instrument rail 19 refuses. A product id that is not a spot pair has no + base balance to read, and says so by returning `None`. + + Never raises -- every failure is the same answer, UNKNOWN, which does not veto and does not + clamp (rail 21, `_clamp_to_held`). An exit must not become less likely to go out because a + balance endpoint was slow. + """ + parsed = parse_spot_product_id(product_id) + if parsed is None: + return None + base = parsed[0] + if broker is None: + # Paper mode passes no broker; expected, not an error -- the same reasoning + # `_fetch_available_quote` and `_base_increment_for` both record. + return None + try: + balances = broker.get_balances() + except Exception: + log_venue_failure(logger, "executor.held_base_fetch_failed", product=product_id) + return None + for balance in balances or []: + if balance.currency.upper() == base.upper(): + return balance.total + # No account row for this currency at all. Deliberately UNKNOWN rather than zero: a venue + # that omits empty accounts and a venue that reports `0` are saying different things, and + # only the second is an affirmative statement about the holding. Rail 21 vetoes on the + # second; guessing it from the first would refuse exits on a venue whose only fault is a + # sparse response. + return None + + +def _clamp_to_held( + qty: Decimal, held: Decimal | None, repo: Repository, product_id: str, now_ts: int +) -> Decimal: + """`qty`, reduced to what the account actually holds. Never raises, never RAISES the quantity. + + Three inputs, three answers, and the asymmetry is the point: + + | venue says | answer | + |---|---| + | unknown (`None`) | `qty` unchanged -- an unreadable balance must not strand an exit | + | at least `qty` | `qty` unchanged -- never rounds UP to the balance | + | less than `qty` | the holding -- sell what is there, not what the ledger remembers | + + A zero-or-less holding is deliberately NOT clamped to zero and NOT refused here: rail 21 + owns that refusal, so it arrives as a `GuardResult` violation with an audit trail like every + other veto, instead of as a malformed zero-size order or a second refusal path in the sizing + code. This function's job is to shrink a real order, not to decide there is no order. + + **Why the ledger drifts, which is the whole of #667.** A venue that takes its taker fee out + of the received base leaves 0.9985 where the order said 1.0000; a partial fill leaves less + still, and `orders.filled_quantity` is only written when the venue's post-fill status was + observable; an operator who moves coins out of the account tells keel nothing at all. Each + one makes the ledger say more than the account holds, and a SELL sized from the ledger then + asks the venue for base that is not there. On a cash account that is a rejected exit; on a + margin-enabled account (#666) the venue fills the difference by opening a short -- *bay' ma + la yamlik* arrived at by arithmetic rather than by anyone's intent. + + Clamping is NOT the auto-resize `_record_observed_fill_quantity` declines to do, and the + distinction matters because that refusal is well argued and still stands. What it refuses is + CANCELLING A PROTECTIVE ORDER on the strength of a settling snapshot. This cancels nothing. + It only declines to ask for more than is there, which is the one thing no snapshot ambiguity + can make wrong: whatever the true holding turns out to be, it is not larger than the + venue's own report of it plus keel's own hold. + """ + if held is None or held <= 0 or qty <= held: + return qty + log_event( + logger, + logging.WARNING, + "executor.sell_clamped_to_held", + product=product_id, + ordered=str(qty), + held=str(held), + drift=str(qty - held), + detail=( + "keel's ledger expected more base than the venue reports holding -- the SELL is " + "reduced to the held quantity. Fee taken in the base asset, a partial fill the " + "venue never reported, or an out-of-band transfer; reconcile the position" + ), + ) + repo.set_state( + f"{BALANCE_DRIFT_PREFIX}{product_id}", + {"ordered": str(qty), "held": str(held), "drift": str(qty - held), "observed_at": now_ts}, + ) + return held + + +def _clamped_sell_qty( + broker: Any, repo: Repository, product_id: str, qty: Decimal, now_ts: int +) -> tuple[Decimal, Decimal | None]: + """`(quantity to sell, what the venue says is held)` -- the one entry point every SELL uses. + + Returned as a pair because both halves travel onward and they answer different questions: + the clamped quantity sizes the order, and the raw holding rides on the intent for rail 21, + which refuses the case the clamp deliberately will not (see `_clamp_to_held`). + """ + held = _held_base(broker, product_id) + return _clamp_to_held(qty, held, repo, product_id, now_ts), held + + def _build_intent( signal: Signal, broker: Any, @@ -543,6 +664,10 @@ def _build_intent( return None entry = signal.setup.entry if signal.setup is not None else avg_cost + # #667. `qty` above is what the ORDERS AUDIT LOG says is held; this is what the VENUE says. + # They diverge through fees taken in the base asset, partial fills the venue never reported, + # and out-of-band transfers -- and the ledger is the one that runs high. + qty, held = _clamped_sell_qty(broker, repo, signal.product_id, qty, now_ts) return OrderIntent( product_id=signal.product_id, side=Side.SELL, @@ -556,6 +681,7 @@ def _build_intent( # #516. Fetched here, like `available_quote` above, so `_order_spec` stays a # pure function of the intent. `None` is fine and means "send unquantized". base_increment=_base_increment_for(broker, repo, signal.product_id, now_ts), + available_base=held, ) @@ -1046,12 +1172,19 @@ def _upgrade_to_observed_economics( log_exception(logger, "executor.observed_economics_unavailable", order_id=order_id) return + # #667: the QUANTITY observation goes first, and no longer sits behind the price guard. + # `filled_quantity` and `average_filled_price` are two independent facts the venue may + # report independently, and the `fill <= 0` test belongs to the second one -- it is there + # because overwriting a good estimate with a zero price makes the row worse. Behind it, a + # venue that answered with a filled size and no average price lost the size too, and the + # size is the one that tells `_clamp_to_held` how far the ledger has drifted. Recording it + # first costs nothing when both are present and keeps the more useful half when they are not. + _record_observed_fill_quantity(repo, order_id, observed, intent, now_ts) fill = observed.average_filled_price fees = observed.total_fees if not fill or fill <= 0: return repo.update_order(order_id, actual_fill=fill, fee=fees, updated_at=now_ts) - _record_observed_fill_quantity(repo, order_id, observed, intent, now_ts) _log_intent_divergence(order_id, intent, fill) @@ -1604,6 +1737,14 @@ def _order_spec(intent: OrderIntent) -> OrderSpec: Down, not nearest, on both sides: selling slightly less than held leaves dust, while selling more is rejected for insufficient funds anyway. + + **The quantity arriving here is already clamped to the venue's holding (#667).** That is a + change of INPUT, not of contract: this function still only quantizes, still never refuses, + and the sentence above still describes it exactly. What changed is upstream -- "rejected for + insufficient funds anyway" was the whole answer to overselling only for as long as every + account was a cash account, and on a margin-enabled one the venue fills the difference by + opening a short instead of rejecting it. `_clamp_to_held` closes that at intent-construction + time, where a repo and a broker are in hand; refusing an empty holding outright is rail 21's. """ if intent.side == Side.BUY: increment = sizing.quote_increment_for(intent.product_id) @@ -1870,6 +2011,13 @@ def place_bracket( now carries both prices -- rolling the stop means re-placing the bracket, and the target is no longer recoverable from a separate leg. """ + # #667, and this is the site the fee-dust case actually bites: `qty` here is the size the + # ENTRY was placed for, and a venue that takes its taker fee out of the received base leaves + # less than that in the account. A bracket for the ordered size is then a protective order + # able to sell more than is held -- the "oversized-bracket condition" + # `_record_observed_fill_quantity` names and declines to fix by cancelling. Sizing it down + # before it is ever placed cancels nothing. + qty, held = _clamped_sell_qty(broker, repo, product_id, qty, now_ts) intent = OrderIntent( product_id=product_id, side=Side.SELL, @@ -1882,6 +2030,7 @@ def place_bracket( notional=sizing.spend(qty, stop), is_dca=False, rule_kind=rule_name, + available_base=held, ) result = _run_order( intent, @@ -2011,6 +2160,12 @@ def scale_out( "position's rule ownership and levels" ), ) + # #667. `held` above is the LEDGER's holding; clamp the partial to the VENUE's before the + # remainder is derived from it, so the crash-ledger record and the log both describe the + # sequence that is actually about to run. Under no drift this changes nothing at all -- a + # scale-out is a fraction of the position by construction, so the clamp binds only when the + # books and the account genuinely disagree. + qty, venue_held = _clamped_sell_qty(broker, repo, product_id, qty, now_ts) remainder = held - qty stop = repo.get_state(f"open_stop:{product_id}") @@ -2082,6 +2237,7 @@ def scale_out( notional=sizing.spend(qty, exit_price), is_dca=False, rule_kind=rule_name, + available_base=venue_held, ) result = _run_order(intent, broker, repo, config, "autonomous", None, now_ts) @@ -2335,6 +2491,11 @@ def _roll_stop( _cancel_at_exchange(broker, repo, old_order) repo.update_order(old_stop_order_id, status="canceled", updated_at=now_ts) + # #667, read AFTER the cancel above -- which is free, and worth saying why it is free. The + # holding is `Balance.total`, so it does not move when a resting bracket is cancelled; had + # this used `Balance.available` the number would jump from ~0 to the full position across + # that line, and every roll would depend on which side of the cancel it was read. + qty, held = _clamped_sell_qty(broker, repo, product_id, qty, now_ts) intent = OrderIntent( product_id=product_id, side=Side.SELL, @@ -2347,6 +2508,7 @@ def _roll_stop( notional=sizing.spend(qty, new_stop), is_dca=False, rule_kind=rule_name, + available_base=held, ) result = _run_order( intent, diff --git a/keel/execution/guards.py b/keel/execution/guards.py index 6705338d..bd8bdda3 100644 --- a/keel/execution/guards.py +++ b/keel/execution/guards.py @@ -4,7 +4,7 @@ un-overridable safety-critical rails: 13/14 added by Issue #59 (USDC-funding + monthly-allowance), 16, the consecutive-loss circuit breaker (Task 4), 17, the withdrawal/`qabd` rail, 18, the settlement-currency rail, 19, the spot-instrument rail, and 20, the trade-scope rail (#233) — -nineteen in all, since there is no rail 15. They run before any order is placed, in every +twenty in all, since there is no rail 15. They run before any order is placed, in every `auto_trade` mode (confirm *and* autonomous) and for both rule-trading and DCA order classes. It never short-circuits: every violated rail is collected and reported so an operator (or the executor, Task 4) sees the full picture, not just the first trip-wire. @@ -231,18 +231,40 @@ class OrderIntent: # exit, which is worse than the imprecision being fixed. base_increment: Decimal | None = None + # #667: what the venue says the account actually HOLDS of this product's base currency. + # Fetched by the executor, like `available_quote` and `base_increment`, because guards has + # no broker of its own. Rail 21 reads it, and `executor._clamp_to_held` clamps the SELL + # quantity to it. + # + # ⚠️ This is `Balance.total`, NEVER `Balance.available`, and the distinction is the whole + # reason this field is safe to have. `available` EXCLUDES base committed to resting orders, + # and keel's own protective bracket commits the entire position -- so `available` reads + # ~0 for exactly the products keel is protecting, and clamping to it would veto every stop + # roll and every exit keel has ever placed. `total` (available + hold) is the ownership + # number, and ownership is the question *bay' ma la yamlik* asks. + # + # `None` means UNKNOWN and does NOT veto -- the deliberate opposite of rail 13's fail-closed + # posture, for the reason `_sell_base_size` gives: refusing a SELL strands a position that + # wanted out, and an unreadable balance is not evidence the position is gone. + available_base: Decimal | None = None + #: Rails whose inputs describe the LIVE ACCOUNT and therefore cannot be evaluated offline: #: rail 13 needs a broker-fetched quote balance, rail 17 needs the account's real withdrawal #: state, rail 20 needs the venue's own attested/confirmed trade-scope record. Paper trading has #: no live account, so these are SKIPPED there -- and RECORDED as skipped, never silently #: omitted, so a paper track record is honest about its own gaps. -LIVE_STATE_RAILS = ("usdc_funding", "withdrawal_capability", "trade_scope") +LIVE_STATE_RAILS = ( + "usdc_funding", + "withdrawal_capability", + "trade_scope", + "base_balance", +) @dataclass(frozen=True) class GuardResult: - """The outcome of running all nineteen rails: `ok` iff `violations` is empty.""" + """The outcome of running all twenty rails: `ok` iff `violations` is empty.""" ok: bool violations: list[str] @@ -446,13 +468,13 @@ def check( now_ts: int, offline: bool = False, ) -> GuardResult: - """Run all nineteen §14 (+ Issue #59, Task 4, #233) hard rails against `intent`. Never + """Run all twenty §14 (+ Issue #59, Task 4, #233, #667) hard rails against `intent`. Never short-circuits. Called before every order in every `auto_trade` mode (confirm *and* autonomous) -- un-overridable. - `offline=True` (paper trading only) skips `LIVE_STATE_RAILS` — the two rails whose inputs + `offline=True` (paper trading only) skips `LIVE_STATE_RAILS` — the rails whose inputs describe the real account, which a paper rehearsal has no access to. **Every other rail still runs**, because the promotion gate is scored on the paper track record: a rehearsal that skipped the rails would promote a strategy on evidence of trades live trading would have @@ -956,6 +978,42 @@ def check( f"--trading --venue {venue}` once the credential can place live orders." ) + # 21. Base-balance (#667) — a SELL may not be sent for a base the account does not hold. + # The mirror of rail 13, and deliberately NOT its mirror image. + # + # Rail 13 fails CLOSED: an unknown quote balance vetoes the BUY, because a refused BUY + # costs nothing. This one fails OPEN on unknown, because a refused SELL strands a + # position that wanted out -- `_sell_base_size`'s asymmetry table, applied to a balance + # instead of an increment. Do not "fix" this into consistency either. + # + # It therefore vetoes ONE case: the venue affirmatively reports a zero-or-negative + # holding while keel's ledger believes it holds something. That is not an exit to + # protect, it is a divergence between the books and the account -- and `_build_intent` + # already returns `None` when the LEDGER says zero, so reaching here at all means the + # two disagree. Sending it would be *bay' ma la yamlik* reached by arithmetic: on a cash + # account the venue rejects it, and on a margin-enabled one (#666) it fills as a short. + # Refusing sells nothing and traps nothing, because there is nothing there to trap. + # + # A holding that is merely SMALLER than the order is NOT vetoed here -- that is the + # clamp's job (`executor._clamp_to_held`), and vetoing it would refuse an exit that can + # still be made, for a position that really exists. + # + # ⚠️ `available_base` is `Balance.total`, not `Balance.available`. See `OrderIntent`: + # `available` excludes base committed to resting orders, and keel's own bracket commits + # the whole position, so this rail read on `available` would veto every exit keel places. + if not offline and not is_buy: + held = intent.available_base + # The loose parse, purely for the MESSAGE. The executor matched the venue balance on + # the strict one; naming a leg here costs nothing and never raises. + base = _asset(intent.product_id) + if held is not None and held <= 0: + violations.append( + f"base_balance: the venue reports a {base} holding of {held} for " + f"{intent.product_id} while keel's ledger expects {intent.qty} -- refusing to " + "sell what the account does not hold. Reconcile the position (an out-of-band " + "transfer, or an exit that already executed) before trading this product." + ) + for violation in violations: log_event( logger, diff --git a/tests/commands/test_doctor.py b/tests/commands/test_doctor.py index 15d00045..b78f2646 100644 --- a/tests/commands/test_doctor.py +++ b/tests/commands/test_doctor.py @@ -24,6 +24,7 @@ admissibility_findings, allowance_findings, attestation_findings, + balance_drift_findings, data_health_findings, doctor_exit_code, doctor_lines, @@ -580,6 +581,7 @@ def test_gather_findings_covers_every_check_over_a_seeded_db(tmp_path, valid_con "allowance.headroom", "veto.recent", "fill.partial", + "balance.drift", "ledger.unbooked_exit", "data.missing", "data.stale", @@ -762,3 +764,73 @@ def test_an_unbooked_LIVE_exit_warns_too() -> None: alike, and an unbooked LIVE exit is strictly worse than the paper one that found this.""" (finding,) = unbooked_exit_findings([_tranche()], [_sell(mode="live")]) assert finding.status == "warn" + + +# -- ledger/venue balance drift (#667) ----------------------------------------------------------- + + +def test_balance_drift_findings_is_ok_when_nothing_drifted() -> None: + """The clean state is REPORTED, not omitted. An absent row reads as a check that never ran.""" + (finding,) = balance_drift_findings({}) + + assert finding.name == "balance.drift" + assert finding.status == "ok" + + +def test_balance_drift_findings_warns_and_names_both_numbers() -> None: + """An operator cannot reconcile a divergence they are only told the size of. + + WARN rather than FAIL because every cause is legitimate -- a fee taken in the base leg, a + short fill, an operator's own transfer. None is a fault in the deployment; all of them leave + the ledger's idea of the position wrong until a human decides which it was. + """ + (finding,) = balance_drift_findings( + { + "BTC-USD": { + "ordered": "1.0", + "held": "0.9985", + "drift": "0.0015", + "observed_at": NOW, + } + } + ) + + assert finding.status == "warn" + assert "1.0" in finding.detail and "0.9985" in finding.detail + assert "clamped" in finding.detail, ( + "the detail must say the SELL was already reduced -- otherwise this reads as an order " + "that oversold, which is the outcome the clamp exists to prevent" + ) + + +def test_balance_drift_findings_ignores_a_malformed_record() -> None: + """State is written by code that can change; a bad row must not crash `doctor`. + + `doctor` is what an operator runs when something is already wrong. It is the one command + that must not fail on the state it exists to describe. + """ + (finding,) = balance_drift_findings({"BTC-USD": "not a record"}) + + assert finding.status == "ok" + + +def test_gather_findings_surfaces_a_recorded_drift(tmp_path, valid_config_path) -> None: + """The wiring, not just the function. A finding nothing calls reports nothing. + + Written the way the executor writes it -- `balance_drift:` -- so a rename on + either side fails here rather than silently retiring the check. + """ + from keel.execution.executor import BALANCE_DRIFT_PREFIX + + repo = _seeded_repo(tmp_path / "keel.db") + repo.set_state( + f"{BALANCE_DRIFT_PREFIX}BTC-USD", + {"ordered": "1.0", "held": "0.9985", "drift": "0.0015", "observed_at": NOW}, + ) + config = load_config(valid_config_path) + + findings = gather_findings(repo, config, [], NOW) + + (drift,) = [f for f in findings if f.name == "balance.drift"] + assert drift.status == "warn" + assert "BTC-USD" in drift.detail diff --git a/tests/execution/test_executor.py b/tests/execution/test_executor.py index 067e3bc5..c0e7d5ea 100644 --- a/tests/execution/test_executor.py +++ b/tests/execution/test_executor.py @@ -527,7 +527,11 @@ def test_execute_fetches_the_available_quote_balance_for_a_buy_and_places(repo): result = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) assert result.placed is True - assert broker.get_balances_calls == 1 + # TWO reads, and they are different questions about different currencies (#667). The first + # is rail 13's: how much QUOTE can this BUY spend. The second belongs to the exit bracket + # `execute` places once the entry has filled -- how much BASE does the account actually hold + # to protect. A bracket sized from the ordered quantity is the oversized-bracket condition. + assert broker.get_balances_calls == 2 def test_broker_balance_fetch_error_vetoes_the_buy_before_preview_or_place(repo): @@ -555,9 +559,15 @@ def test_insufficient_usdc_balance_vetoes_the_buy_before_preview_or_place(repo): assert repo.get_orders() == [] -def test_exit_signal_never_fetches_a_balance(repo): - """SELL is exempt from rail 13 -- the executor shouldn't even bother fetching a balance for - an EXIT, since it wouldn't be used.""" +def test_exit_signal_fetches_the_held_base_not_the_quote_balance(repo): + """A SELL is still exempt from rail 13, and since #667 it reads a balance anyway. + + This test asserted `get_balances_calls == 0` for six months, and the reasoning was sound + while the only balance question was rail 13's: an EXIT spends no quote, so fetching one + bought nothing. #667 asks a DIFFERENT question of the same endpoint -- not "can this order + be funded" but "does the account still hold what the ledger says it holds" -- and that one + an exit must ask, because the exit is the order that oversells when the answer is no. + """ _seed_open_position(repo, "BTC-USD", Decimal("0.1"), Decimal("50000")) broker = FakeBroker() signal = Signal( @@ -574,7 +584,12 @@ def test_exit_signal_never_fetches_a_balance(repo): result = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) assert result.placed is True - assert broker.get_balances_calls == 0 + assert broker.get_balances_calls == 1 + # And it is the BASE leg that was asked about: the default fake funds USD/USDC and carries + # no BTC row, so the holding reads UNKNOWN and the exit goes out at the ledger quantity -- + # unchanged, un-vetoed. An unreadable balance never strands a position that wanted out. + spec = broker.place_calls[-1]["spec"] + assert spec.base_size == Decimal("0.1") # -- autonomous mode: compliant -> placed without a prompt diff --git a/tests/execution/test_sell_clamp.py b/tests/execution/test_sell_clamp.py new file mode 100644 index 00000000..8c3dfcfc --- /dev/null +++ b/tests/execution/test_sell_clamp.py @@ -0,0 +1,549 @@ +"""#667 -- a SELL never asks the venue for more base than the account holds. + +The ledger and the account drift apart in three ways keel cannot see from its own records: a +venue that takes its taker fee out of the received base leaves 0.9985 where the order said +1.0000; a partial fill leaves less still, and `orders.filled_quantity` is written only when the +venue's post-fill status was observable; an operator who moves coins out tells keel nothing at +all. Every one of them makes the ledger say MORE than the account holds. + +Sizing a SELL from the ledger then asks for base that is not there. On a cash account that is a +rejected exit -- a position that wanted out, still in, unprotected. On a margin-enabled account +(#666) the venue fills the difference by opening a short: *bay' ma la yamlik*, arrived at by +arithmetic rather than by anyone's intent, and reachable without a single line of the strategy +layer proposing one. + +Two mechanisms, deliberately split, and the split is what these tests pin: + +* **the clamp** (`executor._clamp_to_held`) shrinks an order that is too big for a position that + really exists -- down only, never up, and never to zero; +* **rail 21** (`guards.check`) refuses the one case the clamp will not touch, a venue that + affirmatively reports holding nothing at all. + +Neither cancels a protective order. That distinction is the reason +`_record_observed_fill_quantity`'s refusal to auto-resize still stands beside this: what it +declines to do is cancel a resting bracket on the strength of a settling snapshot, and nothing +here cancels anything. +""" + +from __future__ import annotations + +from decimal import Decimal + +import pytest +from keel_broker_api.results import Balance + +from keel.execution import executor, guards +from keel.execution.executor import BALANCE_DRIFT_PREFIX, place_bracket, scale_out +from keel.execution.guards import LIVE_STATE_RAILS, OrderIntent +from keel.strategy.rules.base import Action, Signal +from keel.types import Side +from tests.execution.test_executor import ( + NOW_TS, + FakeBroker, + _config, + _seed_open_position, + repo, # noqa: F401 -- the shared in-memory Repository fixture +) + + +def _exit_signal(product_id: str = "BTC-USD") -> Signal: + return Signal( + rule_name="target_harvest", + product_id=product_id, + action=Action.EXIT, + side=Side.SELL, + setup=None, + cts_score=0, + entry_technique="market", + ts=NOW_TS, + ) + + +class HeldBroker(FakeBroker): + """A `FakeBroker` whose account carries a BASE holding as well as the quote legs. + + `available` and `total` are set INDEPENDENTLY, because the difference between them is the + single most dangerous line in #667 and every test that cares about it needs to be able to + make them disagree. + """ + + def __init__(self, base: str, available: Decimal, total: Decimal, **kwargs: object) -> None: + super().__init__(**kwargs) # type: ignore[arg-type] + self._base = base + self._base_available = available + self._base_total = total + + def get_balances(self) -> list[Balance]: + balances = super().get_balances() + balances.append( + Balance( + currency=self._base, available=self._base_available, total=self._base_total + ) + ) + return balances + + +# -- the clamp ----------------------------------------------------------------------------- + + +def test_a_sell_is_reduced_to_what_the_venue_says_is_held(repo): # noqa: F811 + """The fee-dust case, end to end: the ledger says 1.0, the account holds 0.9985.""" + _seed_open_position(repo, "BTC-USD", Decimal("1.0"), Decimal("50000")) + broker = HeldBroker("BTC", available=Decimal("0.9985"), total=Decimal("0.9985")) + + result = executor.execute( + _exit_signal(), broker, repo, _config(), mode="autonomous", now_ts=NOW_TS + ) + + assert result.placed is True + assert broker.place_calls[-1]["spec"].base_size == Decimal("0.9985") + + +def test_a_sell_is_never_raised_to_a_larger_holding(repo): # noqa: F811 + """Down-only. A venue holding MORE than the ledger expects does not enlarge the order. + + The asymmetry is the whole point: selling less than is held leaves dust, and selling more is + the violation. A clamp that moved in both directions would sell base the position's own + ledger never accounted for -- which is a different position, not a corrected one. + """ + _seed_open_position(repo, "BTC-USD", Decimal("0.1"), Decimal("50000")) + broker = HeldBroker("BTC", available=Decimal("5"), total=Decimal("5")) + + result = executor.execute( + _exit_signal(), broker, repo, _config(), mode="autonomous", now_ts=NOW_TS + ) + + assert result.placed is True + assert broker.place_calls[-1]["spec"].base_size == Decimal("0.1") + + +def test_an_unreadable_holding_sells_the_ledger_quantity(repo): # noqa: F811 + """UNKNOWN never strands an exit -- the deliberate opposite of rail 13's fail-closed posture. + + A venue with no account row for the base is saying nothing about the holding, and refusing + an exit on nothing said would replace "sometimes exits" with "never exits" the moment a + balance endpoint went quiet. + """ + _seed_open_position(repo, "BTC-USD", Decimal("0.1"), Decimal("50000")) + broker = FakeBroker() # funds USD/USDC only -- no BTC row at all + + result = executor.execute( + _exit_signal(), broker, repo, _config(), mode="autonomous", now_ts=NOW_TS + ) + + assert result.placed is True + assert broker.place_calls[-1]["spec"].base_size == Decimal("0.1") + + +def test_a_broker_that_raises_on_get_balances_still_exits(repo): # noqa: F811 + """A balance read must never become a new failure mode on the exit path.""" + _seed_open_position(repo, "BTC-USD", Decimal("0.1"), Decimal("50000")) + + class Outage(FakeBroker): + def get_balances(self) -> list[Balance]: + raise ConnectionError("simulated balance outage") + + result = executor.execute( + _exit_signal(), Outage(), repo, _config(), mode="autonomous", now_ts=NOW_TS + ) + + assert result.placed is True + + +def test_the_clamp_reads_total_and_not_available(repo): # noqa: F811 + """THE load-bearing test of #667. `Balance.available` is the wrong number here. + + `available` excludes base committed to resting orders, and keel's own protective bracket + commits the ENTIRE position -- so for every product keel is protecting, `available` reads + zero while the account holds the lot. A clamp on that number would shrink every exit to + nothing and rail 21 would then veto it, refusing every stop roll and every exit keel has + ever placed. `total` (available + hold) is the ownership number, and ownership is the + question *bay' ma la yamlik* asks. + + Modelled exactly as the venue reports it: `available=0`, `hold=0.1`, `total=0.1`. + """ + _seed_open_position(repo, "BTC-USD", Decimal("0.1"), Decimal("50000")) + broker = HeldBroker("BTC", available=Decimal("0"), total=Decimal("0.1")) + + result = executor.execute( + _exit_signal(), broker, repo, _config(), mode="autonomous", now_ts=NOW_TS + ) + + assert result.placed is True, ( + "an exit was refused for a position the venue reports holding in full -- the clamp is " + "reading `Balance.available`, which a resting bracket drives to zero" + ) + assert broker.place_calls[-1]["spec"].base_size == Decimal("0.1") + + +def test_a_clamped_sell_records_the_drift_for_doctor(repo): # noqa: F811 + """Silently absorbing the divergence would hide a real fact about the ACCOUNT. + + The log line alone is not enough: it requires an operator to know to grep for it. The drift + is a property of the account, so it outlives the order that discovered it. + """ + _seed_open_position(repo, "BTC-USD", Decimal("1.0"), Decimal("50000")) + broker = HeldBroker("BTC", available=Decimal("0.9985"), total=Decimal("0.9985")) + + executor.execute(_exit_signal(), broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) + + drift = repo.get_state(f"{BALANCE_DRIFT_PREFIX}BTC-USD") + assert drift is not None, "a clamped SELL must leave a record `doctor` can surface" + assert drift["held"] == "0.9985" + assert Decimal(drift["drift"]) == Decimal("0.0015") + + +def test_an_unclamped_sell_records_nothing(repo): # noqa: F811 + """No drift, no record. A key written on every ordinary exit would make the finding noise.""" + _seed_open_position(repo, "BTC-USD", Decimal("0.1"), Decimal("50000")) + broker = HeldBroker("BTC", available=Decimal("0.1"), total=Decimal("0.1")) + + executor.execute(_exit_signal(), broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) + + assert repo.get_state(f"{BALANCE_DRIFT_PREFIX}BTC-USD") is None + + +def test_the_bracket_placed_after_an_entry_is_sized_to_the_held_base(repo): # noqa: F811 + """The oversized-bracket condition, closed where it is created. + + `place_bracket` receives the quantity the ENTRY was placed for. A venue that took its fee + out of the received base holds less than that, so a bracket at the ordered size is a + protective order able to sell more than is held -- exactly what + `_record_observed_fill_quantity` names and declines to fix by cancelling. Sizing it down + before it is ever placed cancels nothing. + """ + broker = HeldBroker("BTC", available=Decimal("0.9985"), total=Decimal("0.9985")) + + place_bracket( + broker, + repo, + _config(), + "BTC-USD", + Decimal("1.0"), + Decimal("45000"), + Decimal("55000"), + "turtle_breakout", + NOW_TS, + ) + + assert broker.place_calls[-1]["spec"].base_size == Decimal("0.9985") + + +def test_a_scale_out_is_clamped_to_the_held_base(repo): # noqa: F811 + """A partial exit is a SELL like any other, and drift binds it the same way. + + Under no drift the clamp cannot bind here at all -- `scale_out` already refuses a `qty` at + or above the ledger's holding -- so this only fires when the books and the account really + disagree. + """ + _seed_open_position(repo, "BTC-USD", Decimal("1.0"), Decimal("50000")) + repo.set_state("open_stop:BTC-USD", Decimal("45000")) + repo.set_state("open_target:BTC-USD", Decimal("55000")) + broker = HeldBroker("BTC", available=Decimal("0.3"), total=Decimal("0.3")) + + scale_out( + broker, repo, _config(), "BTC-USD", Decimal("0.5"), Decimal("52000"), "target_1", NOW_TS + ) + + sells = [c["spec"] for c in broker.place_calls if getattr(c["spec"], "side", None) is Side.SELL] + assert sells, "scale_out must have placed the partial SELL" + assert sells[0].base_size == Decimal("0.3") + + +# -- rail 21 ------------------------------------------------------------------------------- + + +def _sell_intent(**overrides: object) -> OrderIntent: + fields: dict[str, object] = dict( + product_id="BTC-USD", + side=Side.SELL, + qty=Decimal("0.1"), + entry=Decimal("50000"), + stop=None, + notional=Decimal("5000"), + is_dca=False, + rule_kind="target_harvest", + ) + fields.update(overrides) + return OrderIntent(**fields) # type: ignore[arg-type] + + +def _rail21(result: guards.GuardResult) -> list[str]: + return [v for v in result.violations if v.startswith("base_balance")] + + +def test_rail_21_vetoes_a_sell_when_the_venue_reports_no_holding(repo): # noqa: F811 + """The books say a position exists; the account says it does not. That is not an exit. + + `_build_intent` already returns `None` when the LEDGER says zero, so reaching rail 21 at all + means the two disagree -- and refusing sells nothing and traps nothing, because there is + nothing there to trap. On a margin-enabled account, sending it is the short. + """ + result = guards.check( + _sell_intent(available_base=Decimal("0")), repo, _config(), NOW_TS + ) + + assert _rail21(result), "a SELL against an affirmatively empty holding must be vetoed" + assert "keel's ledger expects 0.1" in _rail21(result)[0] + + +def test_rail_21_does_not_veto_a_holding_merely_smaller_than_the_order(repo): # noqa: F811 + """A partial holding is a real position. Vetoing it would strand what could still be sold. + + This is the boundary between the two mechanisms: below the order but above zero belongs to + the clamp, and only zero-or-less belongs to the rail. + """ + result = guards.check( + _sell_intent(available_base=Decimal("0.05")), repo, _config(), NOW_TS + ) + + assert not _rail21(result) + + +def test_rail_21_does_not_veto_an_unknown_holding(repo): # noqa: F811 + """Fails OPEN on unknown -- the deliberate inverse of rail 13, and it must stay that way. + + Rail 13 vetoes a BUY on an unknown quote balance because a refused BUY costs nothing. A + refused SELL costs a position its exit. Do not make these two consistent. + """ + result = guards.check(_sell_intent(available_base=None), repo, _config(), NOW_TS) + + assert not _rail21(result) + + +def test_rail_21_never_touches_a_buy(repo): # noqa: F811 + """A BUY has rail 13. A base holding says nothing about whether an entry can be funded.""" + result = guards.check( + _sell_intent(side=Side.BUY, available_base=Decimal("0")), repo, _config(), NOW_TS + ) + + assert not _rail21(result) + + +def test_rail_21_is_skipped_and_reported_in_paper(repo): # noqa: F811 + """Paper has no account, so it cannot ask -- and must SAY it could not ask. + + A paper track record that silently skipped a live-state rail would score a strategy on + evidence live trading would have refused. That is what the promotion gate exists to prevent. + """ + result = guards.check( + _sell_intent(available_base=Decimal("0")), repo, _config(), NOW_TS, offline=True + ) + + assert not _rail21(result) + assert "base_balance" in result.skipped_rails + assert "base_balance" in LIVE_STATE_RAILS + + +@pytest.mark.parametrize("held", [Decimal("0"), Decimal("-1")]) +def test_rail_21_treats_a_negative_holding_as_empty(repo, held): # noqa: F811 + """A venue reporting a negative base is already short. Nothing about that permits a SELL.""" + result = guards.check(_sell_intent(available_base=held), repo, _config(), NOW_TS) + + assert _rail21(result) + + +# -- rail 21, reached through the real cycle ------------------------------------------------- +# +# Everything above this line builds an `OrderIntent` by hand, and that is not enough. A rail is +# only as real as the wiring that feeds it: drop `available_base=` from the intent the executor +# actually builds and every hand-built test above still passes, because none of them ever ran +# `execute`. These do. + + +def test_execute_vetoes_an_exit_the_venue_says_it_cannot_make(repo): # noqa: F811 + """Ledger says 0.1 BTC, venue says 0. The exit does not go out, and rail 21 is why.""" + _seed_open_position(repo, "BTC-USD", Decimal("0.1"), Decimal("50000")) + broker = HeldBroker("BTC", available=Decimal("0"), total=Decimal("0")) + + result = executor.execute( + _exit_signal(), broker, repo, _config(), mode="autonomous", now_ts=NOW_TS + ) + + assert result.placed is False + assert any(v.startswith("base_balance") for v in result.vetoed_by), ( + f"expected a base_balance veto, got {result.vetoed_by} -- the executor is not threading " + "the venue's holding onto the intent, so rail 21 can never see it" + ) + assert broker.place_calls == [] + + +def test_place_bracket_is_vetoed_when_the_venue_holds_nothing(repo): # noqa: F811 + """The same wiring, on the protective-order path. + + A bracket for a position the account no longer holds is the orphan #668 sweeps for. Refusing + to place one is the cheaper half of the same problem. + """ + broker = HeldBroker("BTC", available=Decimal("0"), total=Decimal("0")) + + result = place_bracket( + broker, + repo, + _config(), + "BTC-USD", + Decimal("1.0"), + Decimal("45000"), + Decimal("55000"), + "turtle_breakout", + NOW_TS, + ) + + assert result is None + assert broker.place_calls == [] + + +def test_scale_out_is_vetoed_when_the_venue_holds_nothing(repo): # noqa: F811 + """And on the partial-exit path, which reaches `_run_order` by a third route again.""" + _seed_open_position(repo, "BTC-USD", Decimal("1.0"), Decimal("50000")) + repo.set_state("open_stop:BTC-USD", Decimal("45000")) + repo.set_state("open_target:BTC-USD", Decimal("55000")) + broker = HeldBroker("BTC", available=Decimal("0"), total=Decimal("0")) + + result = scale_out( + broker, repo, _config(), "BTC-USD", Decimal("0.5"), Decimal("52000"), "target_1", NOW_TS + ) + + assert result.placed is False + assert any(v.startswith("base_balance") for v in result.vetoed_by), ( + f"expected a base_balance veto, got {result.vetoed_by}" + ) + + +def test_rolling_a_stop_is_vetoed_when_the_venue_holds_nothing(repo): # noqa: F811 + """The fourth route into `_run_order`, and the one with the sharpest ordering hazard. + + `_roll_stop` CANCELS the resting bracket before it places the replacement, so a holding read + on `Balance.available` would jump from ~0 to the full position across that single line and + the roll's fate would depend on which side of the cancel it was read from. Reading `total` + makes the cancel irrelevant to the number -- which is why this test can assert the veto + without caring about the order of operations at all. + """ + broker = HeldBroker("BTC", available=Decimal("0"), total=Decimal("0.01")) + stop_id = place_bracket( + broker, + repo, + _config(), + product_id="BTC-USD", + qty=Decimal("0.01"), + stop=Decimal("49000"), + target=Decimal("53000"), + rule_name="pullback_continuation", + now_ts=NOW_TS, + ) + assert stop_id is not None, "the bracket must place while the venue still reports the holding" + + # The position leaves the account out of band -- withdrawn, or an exit that already ran. + broker._base_total = Decimal("0") + + rolled = executor.roll_to_break_even( + broker, + repo, + _config(), + product_id="BTC-USD", + old_stop_order_id=stop_id, + entry_price=Decimal("50000"), + qty=Decimal("0.01"), + rule_name="pullback_continuation", + now_ts=NOW_TS + 100, + ) + + assert rolled is None, ( + "a stop was re-placed for a position the venue reports it no longer holds -- " + "`_roll_stop` is not threading the holding onto its intent" + ) + + +def test_a_rolled_stop_is_re_placed_at_the_held_size(repo): # noqa: F811 + """Not just vetoed on empty -- SIZED on partial. A roll re-places the whole position. + + `_roll_stop` re-places at the same quantity it cancelled, so it carries the ordered size + forward across every roll of a position's life. Left unclamped it would keep reasserting an + oversized protective order long after the drift that created it, at each new stop. + """ + broker = HeldBroker("BTC", available=Decimal("0"), total=Decimal("1.0")) + stop_id = place_bracket( + broker, + repo, + _config(), + product_id="BTC-USD", + qty=Decimal("1.0"), + stop=Decimal("49000"), + target=Decimal("53000"), + rule_name="pullback_continuation", + now_ts=NOW_TS, + ) + assert stop_id is not None + broker._base_total = Decimal("0.9985") + + rolled = executor.roll_to_break_even( + broker, + repo, + _config(), + product_id="BTC-USD", + old_stop_order_id=stop_id, + entry_price=Decimal("50000"), + qty=Decimal("1.0"), + rule_name="pullback_continuation", + now_ts=NOW_TS + 100, + ) + + assert rolled is not None + assert broker.place_calls[-1]["spec"].base_size == Decimal("0.9985") + + +# -- the observation the clamp depends on ---------------------------------------------------- + + +def test_a_filled_size_is_recorded_even_when_the_venue_reports_no_average_price(repo): # noqa: F811 + """`filled_quantity` and `average_filled_price` are independent facts (#667). + + They were coupled: one `fill <= 0` guard returned before the quantity was recorded, so a + venue that answered with a size and no average price lost the size too. The guard belongs to + the PRICE -- it is there so a zero does not overwrite a usable estimate -- and the quantity + is the half that measures how far the ledger has drifted. + """ + from keel_broker_api.results import OrderStatus, PlaceResult + + order_id = repo.insert_order( + dict( + mode="live", + product_id="BTC-USD", + side=Side.BUY.value, + order_type="market", + qty=Decimal("1.0"), + limit_price=Decimal("50000"), + status="filled", + fee=Decimal("0"), + expected_fill=Decimal("50000"), + actual_fill=Decimal("50000"), + raw_response=None, + confirmation="autonomous", + rule_id=None, + created_at=NOW_TS, + updated_at=NOW_TS, + ) + ) + + class Priceless: + """A venue that knows what executed but not at what average price.""" + + def get_order(self, order_id: str) -> OrderStatus: + return OrderStatus( + order_id=order_id, + status="FILLED", + filled_size=Decimal("0.9985"), + average_filled_price=Decimal("0"), + total_fees=Decimal("0"), + ) + + executor._upgrade_to_observed_economics( + Priceless(), + repo, + order_id, + PlaceResult(success=True, broker_order_id="venue-1"), + NOW_TS, + ) + + assert repo.get_order(order_id)["filled_quantity"] == Decimal("0.9985") + # The price is still declined, which is the half of the old guard that was always right. + assert repo.get_order(order_id)["actual_fill"] == Decimal("50000")