diff --git a/docs/fiqh-basis.md b/docs/fiqh-basis.md index 0234239..44d6288 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 70fb360..27a7c30 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 0e76006..5be7869 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 02747e7..7a2c9d3 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 63cce7e..d80b10e 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 4dbd77f..480c136 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 5a831d1..5e42043 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 6705338..bd8bdda 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 15d0004..b78f264 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 067e3bc..c0e7d5e 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 0000000..8c3dfcf --- /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")