From 8b3bbd557feecccc0033b27e5ad90a10e2400337 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Wed, 5 Aug 2026 18:14:45 -0400 Subject: [PATCH 1/2] feat(guards): add rail 19, the spot-instrument rail (R2) Rail 18 checks the settlement LEG; it does not check the instrument SHAPE. `quote_currency_of("BTC-PERP-USD")` is "USD" and `_asset` is "BTC", so a derivative-shaped id with a legitimate final segment passes rail 1 AND rail 18 on the live allowlist. Rail 19 is the only thing that stops it, which is asserted as a test. - `parse_spot_product_id` in keel-core: [A-Z0-9]{1,16}-[A-Z0-9]{2,10}, total, never raises. The quote bound is tied to config's _CURRENCY_CODE_RE so the two grammars cannot disagree. `quote_currency_of` is untouched -- rails 13/18 depend on its loose parse to name "CDE" in their messages. - Rail 19 `spot_instrument`: both sides, every mode, not in LIVE_STATE_RAILS, appends a violation and never raises. - `_asset` made total. `_open_exposure_by_asset` logs an unparseable history row and STILL COUNTS it -- a deliberate correction to the study's "skip-or-flag" wording, because skipping would reduce measured exposure and loosen rails 4/5/6. Overcounting is the closed direction. - `--products` validated where the operator types it, so `rules seed --products XLM-28AUG26-CDE --status live` now fails at the keyboard instead of seeding a row the agent polls and the rails veto forever. - `assets screen` is deliberately EXEMPT from that validation. Screening is the command that answers "may keel trade this, and why not"; a usage error would make the one tool whose job is to explain an inadmissible asset the one tool that cannot be asked about one. Pinned by a test. - BrokerCapabilities.asset_classes gets a vocabulary check and conformance assertions, but the engine still does not read it: the live path builds CoinbaseClient, which has no capabilities(), and paper passes broker=None, so such a gate would be dead code reading as a defence -- the same pattern already built and deleted once here. Deferred to the broker-port migration, as a load-time check. A chosen validator, not a classifier: a classifier must enumerate the shapes keel refuses and so fails OPEN on a shape the venue has not invented yet; a validator answers only "is this a well-formed spot id" and fails closed. Verified: all 936 live spot ids match the grammar; all 99 futures and all 1000 equity ids fail it. The six live deployment rules are unaffected. Co-Authored-By: Claude Opus 5 (1M context) --- ...-08-05-coinbase-asset-class-feasibility.md | 91 ++++++- keel/cli.py | 24 +- keel/commands/_products.py | 98 +++++++- keel/commands/rules.py | 26 +- keel/execution/executor.py | 2 +- keel/execution/guards.py | 127 ++++++++-- .../keel_broker_api/capabilities.py | 30 ++- .../keel_broker_api/conformance/suite.py | 19 +- packages/keel-core/keel_core/products.py | 65 +++++ tests/broker_api/test_capabilities.py | 52 ++++ tests/commands/test_products.py | 99 ++++++++ tests/compliance/test_assets_cli.py | 30 +++ tests/core/test_products.py | 111 +++++++- tests/execution/test_guards.py | 238 ++++++++++++++++++ tests/test_init_and_seed.py | 69 +++++ 15 files changed, 1047 insertions(+), 34 deletions(-) create mode 100644 tests/broker_api/test_capabilities.py create mode 100644 tests/commands/test_products.py diff --git a/docs/experiments/2026-08-05-coinbase-asset-class-feasibility.md b/docs/experiments/2026-08-05-coinbase-asset-class-feasibility.md index eaa6ceac..5c4eabec 100644 --- a/docs/experiments/2026-08-05-coinbase-asset-class-feasibility.md +++ b/docs/experiments/2026-08-05-coinbase-asset-class-feasibility.md @@ -664,7 +664,8 @@ crypto). Gold and silver are named prohibited in the KB. Do not start here. The non-USD/USDC **spot** rejection predicted above is real and was accepted: `BTC-EUR`, `ETH-GBP`, `*-USDT` and friends are vetoed by the shipped default. Blast radius on the deployment is nil, as measured here, and `settlement_currencies` is the escape hatch. -2. **R2 — Then make the spot-only assumption structural. (~2–4 days.)** +2. ✅ **R2 — Then make the spot-only assumption structural. (~2–4 days.) — SHIPPED 2026-08-05, see + "What R2 actually shipped" below.** R1 is a settlement-leg check, not an instrument model; it would not stop a hypothetical future product that happens to settle in USD. Turn `BrokerCapabilities.asset_classes` (`capabilities.py:20`) from a dead stub into a real gate — and price it honestly, because the @@ -684,6 +685,89 @@ crypto). Gold and silver are named prohibited in the KB. Do not start here. *On the estimate:* this is the `asset_classes`-gate half of **A6** (3–5 days), plus the A1 slice above and the CLI-entry rejection, minus A6's `AssetAttestation`-to-instruments extension. It is not additive on top of A6; if Path A is ever built, A6 subsumes it. + + **What R2 actually shipped.** **Rail 19, `spot_instrument`** (`keel/execution/guards.py`) — + eighteen rails now, still no rail 15 — plus a strict grammar, a total `_asset`, and rejection + at the keyboard. The framing that made it a day rather than the estimated 2–4 is that the + residual is answerable **without** an instrument model: rail 18 asks *what does this settle + in?* and rail 19 asks *what shape is this?*, and the second question is decided by the id + alone. + - **The residual, exactly.** `quote_currency_of("BTC-PERP-USD")` is `"USD"` — a configured + settlement currency — and `_asset` reduces it to the allowlisted `"BTC"`, so a + derivative-shaped id with a legitimate final segment passes rails 1 **and** 18. Verified by + execution before writing anything. Rail 19 is the only thing that stops it, and that is the + test: `test_rail19_a_usd_settled_derivative_shaped_id_is_vetoed` asserts `spot_instrument` + is in the violations while `settlement_currency` and `halal_allowlist` are not — the same + "both shipped defences pass it, that is the hole" pattern R1's test uses. + - **The grammar.** `parse_spot_product_id` (`packages/keel-core/keel_core/products.py`), + alongside `quote_currency_of` and leaving it untouched: a well-formed spot id is + `[A-Z0-9]{1,16}-[A-Z0-9]{2,10}`, returning `(base, quote)` or `None`, **total** — it never + raises, on any input including non-strings. The census above is its basis: all 936 spot ids + have exactly one hyphen, every futures id has two, and every equity id has none. The quote + leg's 2–10 bound is not a free choice — it is config's own `_CURRENCY_CODE_RE`, so a + well-formed spot id that no `settlement_currencies` set could ever name cannot exist. A + property test pins that agreement. + - **Every mode, both sides, DCA included**, deliberately not in `LIVE_STATE_RAILS`, for rail + 18's reason: it needs no broker and no account state, so paper cannot skip it. + - **No config field**, unlike rail 18's `settlement_currencies`. Spot-only is this agent's + charter, not an operator preference, and a knob whose only safe value is its default is a + liability. + - **CLI-entry rejection.** `validate_product_ids` / `parse_products_option` in + `keel/commands/_products.py` — the existing single source of truth for id construction, so + the derivation and the validation cannot disagree — asks both questions of a typed + `--products` list and raises `ValueError` naming *every* bad id; `keel/cli.py` and + `keel/commands/rules.py` wrap that in `click.BadParameter`. `keel rules seed --products + XLM-28AUG26-CDE --status live` now fails at the keyboard with the reason and seeds nothing, + where it used to write a row the agent would poll and rails 18/19 veto forever. A lowercase + id is **rejected with a "did you mean BTC-USD?" hint, never silently uppercased**: a product + id is a venue identifier, not free text, and guessing at one is how a typo becomes a + position. `rules seed` loads config unconditionally now (it needs the settlement set); + `--products` on `assets screen` is refused one layer earlier than the screen's own + settlement criterion, which is unchanged and still unit-tested. + - **`BrokerCapabilities.asset_classes` hardened, not wired.** `ASSET_CLASSES = {"spot", + "futures", "equity"}` with an `__post_init__` rejection of anything else, mirroring the + `ORDER_KINDS` check beside it, and two conformance assertions (`asset_classes` non-empty and + a subset; `quote_currencies` non-empty). Both first-party adapters pass. + + **What was deliberately NOT shipped**, and why each was a decision rather than an omission: + - **No engine read of `capabilities().asset_classes`** — the same finding that killed R1's + venue-declaration check, unchanged and re-verified: the live path constructs + `keel/data/cb_client.py`'s `CoinbaseClient`, which has **no `capabilities()` at all**, and + the paper path passes `broker=None`. A gate there is dead code on every real path that + reads as a defence. That pattern was built and deleted once in this codebase already; rail + 19 needs no broker handle, which is why it can be the gate today. The reconciliation belongs + with the broker-port migration, at **load** time, never as a per-order raise. + - **No `capabilities()` on `CoinbaseClient`.** Bolting one on to make the above reachable + would put a second, divergent declaration next to `keel_broker_coinbase.CoinbaseAdapter`'s — + two answers to one question, which is the failure the port exists to prevent. + - **No id→class classifier**, and so none of the A1 slice this entry priced in. The recommendation + assumed the gate had to be *"is this instrument's class in the declared set"*, which does + require classifying an id. Rail 19 asks the strictly weaker question *"is this a spot pair"*, + which is decidable from the id and is the only question a spot-only agent has. A classifier + that named the class of a product keel refuses either way would be inventory, not safety. + - **No config field for asset classes.** See above. + - **`OrderIntent` still carries no instrument class**, for the same reason. + + **One deliberate correction to this document's own wording.** The bullet above says of the + history walk: *"on history, skip-or-flag the row and keep going."* **Skipping is wrong, and + the shipped behaviour logs at WARNING (`guards.exposure_row_unparseable`) and STILL COUNTS the + row.** `_open_exposure_by_asset` feeds rails 4/5/6, which are **caps**: dropping a row + *reduces* measured exposure and therefore *loosens* all three. That is fail-**open** — a + malformed audit row would buy the agent headroom it has not got. Counting it can only + over-state exposure, which is the closed direction, and an over-stated cap refuses an order a + human can then look at. Tested both ways round + (`test_open_exposure_walk_survives_a_malformed_history_row`, and a companion asserting the row + still trips the exposure cap). + + **`_asset` was made total, not strict** — the second correction, and the smaller one. The + bullet asks for a *violation* on a non-`BASE-QUOTE` id; the violation is rail 19's, on the + intent, and `_asset` is a **grouping key**, not an admission check. Tightening the key would + have changed rail 1's verdict on `ADA-28AUG26-CDE` from pass to fail — destroying the "rail 1 + passes the contract, that is the hole" assertion this study's own R1 test records — and would + have split a derivative's exposure out of its root's bucket, under-stating the figure rails + 4/5/6 cap. So `_asset` returns exactly the string it always did, with a `str()` in front so + that `_asset(None)` yields a key instead of the `AttributeError` it used to raise. Totality + was the defect; strictness was not. 3. **R3 — Answer question 1 (cash-settled or delivered?) before spending anything else.** It is a documentation lookup plus, if needed, one email to Coinbase support. If CDE contracts settle in cash, §65.11 closes the entire futures family and Paths A and C are dead for ~zero @@ -694,7 +778,10 @@ crypto). Gold and silver are named prohibited in the KB. Do not start here. claim is not evidence (`screen.py:210`). 5. **R5 — Only after R3 and R4 both clear: build Path A**, dated contracts before perp-style (dated ones have no funding mechanic, so they carry strictly fewer open questions), and gate it - behind the R2 capability check from day one. + behind the R2 capability check from day one. **Amended by what R2 shipped:** there is no + capability check to gate behind. Rail 19 refuses every non-spot shape outright, so Path A's + first task is not adding a gate but deliberately *widening* one, in `guards.py`, where the + change is visible in the rails rather than in an adapter's declaration. 6. **R6 — Re-probe equities in ~6 months.** The venue gate is Coinbase's to remove and there is no signal it is imminent. Re-running `docs/experiments/2026-08-05-coinbase-asset-class-probe.py` (command at the top of this document; ~1 minute) answers it: if the `MARKET DATA -- equity …` diff --git a/keel/cli.py b/keel/cli.py index df498d8f..413bc822 100644 --- a/keel/cli.py +++ b/keel/cli.py @@ -79,7 +79,11 @@ _require_interactive_confirmation, with_disclaimer, ) -from keel.commands._products import _default_sim_products, _history_product +from keel.commands._products import ( + _default_sim_products, + _history_product, + parse_products_option, +) from keel.commands.autonomy import autonomy_group from keel.commands.db import db_group from keel.commands.insights import insights_group @@ -770,7 +774,10 @@ def assets_screen(ctx: click.Context, products: str | None) -> None: """ config = _load_cfg(ctx) repo = _open_repo(ctx) - product_list = _parse_products_option(products, config) + # Deliberately UNVALIDATED, unlike every other `--products` caller: screening is the command + # that answers "may keel trade this", and `screen_asset` rejects a cross-settled or malformed + # id with a reason. Validating here would turn that answer into a usage error. + product_list = parse_products_option(products, config, validate=False) admitted = 0 for product in product_list: @@ -1251,9 +1258,16 @@ def pnl(ctx: click.Context, asset: str | None, raw_marks: tuple[str, ...]) -> No def _parse_products_option(products: str | None, config: Config) -> list[str]: - if not products: - return _default_sim_products(config) - return [p.strip() for p in products.split(",") if p.strip()] + """`--products` for `fetch`/`screen`/`monitor`/`simulate`, refused here if keel cannot trade it. + + The parse itself lives in `commands._products` so `rules seed` uses the same one. This + wrapper exists only to turn its `ValueError` into a `click.BadParameter`, i.e. a usage error + naming the offending ids rather than a traceback (feasibility study R2). + """ + try: + return parse_products_option(products, config) + except ValueError as exc: + raise click.BadParameter(str(exc), param_hint="--products") from exc def _sim_asset(product_id: str) -> str: diff --git a/keel/commands/_products.py b/keel/commands/_products.py index f2e10339..59343aea 100644 --- a/keel/commands/_products.py +++ b/keel/commands/_products.py @@ -1,13 +1,19 @@ -"""Product-id derivation shared across CLI commands. +"""Product-id derivation and validation shared across CLI commands. `fetch`, `simulate`, `screen`, `holdings` and `rules seed` must all agree on which product id an allowlist asset means, in the deployment's settlement currency. Keeping that derivation in one leaf module (depended on by both `keel/cli.py` and the extracted command groups, importing neither) is what prevents them from disagreeing. + +The same module owns the check applied to an id the operator TYPES (`--products`), for the same +reason: a derivation and a validation that disagree about what a product id is would let the CLI +refuse an id keel itself constructs, or accept one it cannot trade. """ from __future__ import annotations +from keel_core.products import parse_spot_product_id, quote_currency_of + from keel.config import Config @@ -30,3 +36,93 @@ def _default_sim_products(config: Config) -> list[str]: cannot disagree about which product an asset means. """ return [_history_product(asset, config.quote_currency) for asset in config.allowlist] + + +def validate_product_ids(ids: list[str], settlement_currencies: frozenset[str]) -> list[str]: + """Return `ids` unchanged, or raise `ValueError` naming every id keel could not trade. + + The two questions here are the two the hard rails ask, deliberately and in the same order: + + 1. **Shape** (`parse_spot_product_id`, rail 19) -- is it a spot pair, `BASE-QUOTE`? + 2. **Settlement** (`quote_currency_of` vs `settlement_currencies`, rail 18) -- is its quote + leg one this deployment settles in? + + Asking them where the operator TYPES the id, rather than only where the agent trades it, is + the point (feasibility study R2). `keel rules seed --products XLM-28AUG26-CDE --status live` + otherwise writes a row that looks seeded, that the agent then polls every cycle and rails + 18/19 veto forever -- the reason visible only in a log line nobody is reading. The rails stay + exactly as they are: this is an ergonomics check standing in front of them, never a + replacement for them, and it runs on the operator's list only. Nothing reads it at order time. + + ⚠️ **A lowercase id is REJECTED, with a hint -- never silently uppercased.** `quote_currency_of` + case-folds because it is identifying the currency of an id that already exists; accepting + `btc-USD` here would mean the id the operator typed is not the id keel goes on to trade, and + a product id is a venue identifier, not free text. Guessing at one is how a typo becomes a + position. The hint costs a line and leaves the operator holding the fix. + + Reports EVERY bad id in one message. An operator fixing a list one error per invocation + learns it slowly and abandons it fast. Raises `ValueError` and nothing else, so callers can + wrap it in `click.BadParameter` and get a usage error rather than a traceback. + """ + reasons: list[str] = [] + for product_id in ids: + if parse_spot_product_id(product_id) is None: + # The hint fires only when case is the ONLY thing wrong, so it can never suggest an + # id that is itself inadmissible -- `xlm-28aug26-cde` gets the refusal, not advice. + hint = "" + if isinstance(product_id, str) and parse_spot_product_id(product_id.upper()): + hint = f" -- did you mean {product_id.upper()}?" + reasons.append( + f"{product_id!r} is not a spot product id (expected BASE-QUOTE, uppercase, " + f"exactly one hyphen; keel is spot-only, so futures BASE-DDMMMYY-CDE and equity " + f"hashes are refused){hint}" + ) + continue + settlement = quote_currency_of(product_id) + if settlement not in settlement_currencies: + reasons.append( + f"{product_id!r} settles in {settlement}, which is not one of this deployment's " + f"settlement_currencies {sorted(settlement_currencies)} -- rail 18 would veto " + f"every order for it" + ) + if reasons: + raise ValueError( + "unusable product id(s):\n" + "\n".join(f" - {reason}" for reason in reasons) + ) + return ids + + +def parse_products_option( + products: str | None, config: Config, *, validate: bool = True +) -> list[str]: + """A `--products` option value as a validated product list; the allowlist when it is absent. + + The one parse of that option, shared by `fetch`/`monitor`/`simulate` (via + `cli._parse_products_option`) and `rules seed`, which used to split it inline and so could + not have been given this check without growing a second copy of the derivation. + + `validate=False` is for `assets screen` ALONE, and is not a convenience. Screening is the + diagnostic that ANSWERS "may keel trade this, and why not" -- `screen_asset` has a settlement + criterion of its own and reports `REJECT` with a reason. Refusing the id at the option would + replace that reasoned verdict with a usage error, i.e. the one command whose entire job is to + explain an inadmissible asset would become the one command that cannot be asked about one. + Screening writes nothing and orders nothing; rails 18/19 stop the id if it ever reaches an + order by another route. + + Raises `ValueError` listing every unusable id. Callers are CLI commands and wrap it in + `click.BadParameter`, so the operator gets a usage error rather than a traceback. + + ⚠️ Validation applies to what the operator TYPED, not to the allowlist-derived default. The + default is `_history_product`'s output over `config.allowlist`, and its shape is config's + question, checked once at load (`load_config` already refuses a `quote_currency` outside + `settlement_currencies` for exactly this reason). Validating it here would mean `keel fetch` + -- which places no orders and needs no rail -- started refusing configs it has always + accepted, for a defect the trading path already reports. Rails 18/19 remain the backstop for + an id that reaches an order by any route, typed or derived. + """ + if not products: + return _default_sim_products(config) + ids = [p.strip() for p in products.split(",") if p.strip()] + if not validate: + return ids + return validate_product_ids(ids, config.settlement_currencies) diff --git a/keel/commands/rules.py b/keel/commands/rules.py index 999a559e..209e0282 100644 --- a/keel/commands/rules.py +++ b/keel/commands/rules.py @@ -18,7 +18,7 @@ from keel import agent from keel.commands._common import _load_cfg, _open_repo, with_disclaimer -from keel.commands._products import _default_sim_products +from keel.commands._products import parse_products_option from keel.data.repository import Repository from keel.strategy import backtest as backtest_mod from keel.strategy import promotion as promotion_mod @@ -272,6 +272,12 @@ def rules_seed( already has a rule row of any status, so it's safe to call repeatedly (e.g. from a setup script) without piling up duplicate candidates. `--force` inserts a fresh candidate anyway. + `--products` is validated before anything is written (`parse_products_option`): an id keel + could not trade -- a futures contract, an equity hash, a pair settling outside + `settlement_currencies`, a lowercase typo -- is refused here, naming it, and NO row is + seeded. Rails 18/19 would veto every order for such a rule anyway; the difference is that + the operator hears it now rather than reading it out of a log after the row is in the table. + Read-only w.r.t. the exchange: no network call, no confirmation gate -- it only ever writes local `rules` rows, exactly like `rules promote`/`demote`/`disable`. @@ -279,11 +285,19 @@ def rules_seed( repo = _open_repo(ctx) now_ts = int(time.time()) - if products: - product_list = [p.strip() for p in products.split(",") if p.strip()] - else: - config = _load_cfg(ctx) - product_list = _default_sim_products(config) + # Config is loaded UNCONDITIONALLY now, where it used to be loaded only on the + # allowlist-default branch: `parse_products_option` needs `settlement_currencies` to answer + # rail 18's question about a typed id, and a `--products` seed that skipped that check is + # exactly the case R2 exists to close -- `--products XLM-28AUG26-CDE --status live` wrote a + # row that looked seeded and that the agent then polled and vetoed on every cycle forever. + # The cost is that `rules seed --products ...` now needs a readable `config.yaml`, like every + # other command that touches products; the gain is that the operator hears "no" at the + # keyboard, with the reason, instead of in a log line nobody is reading. + config = _load_cfg(ctx) + try: + product_list = parse_products_option(products, config) + except ValueError as exc: + raise click.BadParameter(str(exc), param_hint="--products") from exc if kinds: kind_list = [k.strip() for k in kinds.split(",") if k.strip()] diff --git a/keel/execution/executor.py b/keel/execution/executor.py index 6c2cbb50..a5e90008 100644 --- a/keel/execution/executor.py +++ b/keel/execution/executor.py @@ -1,7 +1,7 @@ """The order executor (P3 Task 4) -- turns a `Signal` into a guarded live order. `execute()` is the only path from a strategy `Signal` to a real order: it sizes the candidate -(`execution.sizing`), runs the seventeen un-overridable §14 hard rails (`execution.guards.check`) +(`execution.sizing`), runs the eighteen un-overridable §14 hard rails (`execution.guards.check`) **before** anything reaches the broker, previews the order, honors the confirm/autonomous mode gate, places it, and writes a full audit trail to the `orders` table both before and after the broker call (so a crash mid-placement, or a broker-side rejection, still leaves a record). No path in diff --git a/keel/execution/guards.py b/keel/execution/guards.py index 9399d8bf..10b6fd3f 100644 --- a/keel/execution/guards.py +++ b/keel/execution/guards.py @@ -1,13 +1,13 @@ """THE HARD RAILS (§14) — enforced before every order, un-overridable. -`check()` runs the twelve safety rails from the main spec's §14, plus five later, equally +`check()` runs the twelve safety rails from the main spec's §14, plus six later, equally 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, and 18, the -settlement-currency rail — seventeen 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. +16, the consecutive-loss circuit breaker (Task 4), 17, the withdrawal/`qabd` rail, 18, the +settlement-currency rail, and 19, the spot-instrument rail — eighteen 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. Design notes on rails that need state this repo doesn't compute anywhere else yet (Task 3 lands before the executor/money_mgmt modules that would normally produce some of these numbers): @@ -81,6 +81,16 @@ EVERY mode and on BOTH sides, because it needs no broker and no live account state -- see the rail's own comment for why that is the whole point, and for the deliberate spot pairs it also excludes. + +Rail 19 (spot-instrument, safety-critical, un-overridable) closes rail 18's residual (the same +study's R2): rail 18 checks the settlement LEG, this one checks the instrument SHAPE. They are +not redundant. `quote_currency_of("BTC-PERP-USD")` is `"USD"` -- a configured settlement currency +-- and `_asset` reduces it to the allowlisted `"BTC"`, so a derivative-shaped id whose final +segment is legitimate passes rails 1 AND 18 and is stopped only here. Rail 18 catches the classes +Coinbase lists TODAY on their settlement legs; rail 19 makes spot-only structural rather than a +property of which suffixes the venue currently happens to use. Every mode, both sides, DCA +included, and no config field to widen -- spot-only is this agent's charter, not an operator +preference. """ from __future__ import annotations @@ -92,7 +102,7 @@ from decimal import Decimal from typing import Any -from keel_core.products import quote_currency_of +from keel_core.products import parse_spot_product_id, quote_currency_of from keel_core.subscription import SubscriptionStatus from keel_core.telemetry import log_event @@ -158,7 +168,7 @@ class OrderIntent: @dataclass(frozen=True) class GuardResult: - """The outcome of running all seventeen rails: `ok` iff `violations` is empty.""" + """The outcome of running all eighteen rails: `ok` iff `violations` is empty.""" ok: bool violations: list[str] @@ -166,8 +176,35 @@ class GuardResult: skipped_rails: list[str] = field(default_factory=list) -def _asset(product_id: str) -> str: - return product_id.split("-")[0] +def _asset(product_id: object) -> str: + """The base leg of `product_id`: the bucket key rails 1/4/5/6/8 group and compare by. + + **Total by contract** -- never raises, on any input, which is why the parameter is typed + `object`. That is the R2 fix and the only behaviour change here: `_asset(None)` used to raise + `AttributeError`. It matters because `_asset` runs over every historical filled order in + `_open_exposure_by_asset`, whose `product_id` column holds whatever the audit log happens to + hold, and one bad row crashing the agent cycle is strictly worse than the hole rail 19 closes. + + ⚠️ **Deliberately still the LOOSE parse, not `parse_spot_product_id`.** Admission is rail + 19's job, and it does that job on the intent before anything is placed; `_asset` is a + grouping key, and for a key the loose reduction is the *closed* direction on both rails that + read it: + + - **Rail 1.** Reducing `ADA-28AUG26-CDE` to `ADA` is what makes the allowlist pass a futures + contract -- the hole the feasibility study found, and the hole rail 19 exists to close. It + is asserted, as a hole, in `tests/execution/test_guards.py`. Keeping rail 1's verdict + unchanged keeps rail 19 the single, legible reason such an intent is refused, instead of + splitting the story across two rails whose messages disagree about what the asset even is. + - **Rails 4/5/6.** A derivative on an allowlisted root belongs in that root's exposure + bucket. Keying `ADA-28AUG26-CDE` under its own name instead would split ADA's measured + exposure in two, and the per-asset concentration cap would then admit an order the + combined figure refuses. Merging can only over-state a bucket; splitting under-states it. + + So: same string as before for every input that has one, and `str()` first so that inputs + which never had one (`None`, a stray `int`) produce a key rather than an exception. A key + that is not in the allowlist, which is the closed outcome for a value that should not exist. + """ + return str(product_id).split("-")[0] def _utc_day_bounds(ts: int) -> tuple[int, int]: @@ -188,10 +225,34 @@ def _order_notional(order: dict[str, Any]) -> Decimal: def _open_exposure_by_asset(repo: Repository) -> dict[str, Decimal]: - """Net at-risk notional per asset from filled live orders (BUY adds, SELL reduces).""" + """Net at-risk notional per asset from filled live orders (BUY adds, SELL reduces). + + ⚠️ A row whose `product_id` is not a well-formed spot id is LOGGED AND STILL COUNTED, under + whatever key `_asset` gives it -- never skipped. A deliberate correction to the feasibility + study's own "skip-or-flag the row and keep going" wording (R2), and the direction is the + whole argument: this figure feeds rails 4/5/6, which are CAPS, so dropping a row REDUCES + measured exposure and LOOSENS every one of them. That is fail-OPEN -- a malformed audit row + would buy the agent headroom it has not got. Counting it can only over-state exposure, which + is the closed direction, and an over-stated cap refuses an order a human can then look at. + The WARNING is how the operator finds out; it is not a substitute for counting the money. + + Such a row should be impossible going forward -- rail 19 vetoes the intent before it can be + written, and the live `orders` table held zero rows when rail 19 shipped -- but "impossible" + is what the study said about a futures SELL passing every rail. + """ exposure: dict[str, Decimal] = {} for order in repo.get_orders(mode="live", status="filled"): - asset = _asset(order["product_id"]) + product_id = order["product_id"] + if parse_spot_product_id(product_id) is None: + log_event( + logger, + logging.WARNING, + "guards.exposure_row_unparseable", + product=str(product_id), + order_id=order.get("id"), + side=order.get("side"), + ) + asset = _asset(product_id) amount = _order_notional(order) if order["side"] == Side.BUY.value: exposure[asset] = exposure.get(asset, Decimal("0")) + amount @@ -259,7 +320,7 @@ def check( now_ts: int, offline: bool = False, ) -> GuardResult: - """Run all seventeen §14 (+ Issue #59, Task 4) hard rails against `intent`. Never + """Run all eighteen §14 (+ Issue #59, Task 4) hard rails against `intent`. Never short-circuits. Called before every order in every `auto_trade` mode (confirm *and* autonomous) -- @@ -622,6 +683,44 @@ def check( f"Only spot products quoted in a configured currency may be traded." ) + # 19. Spot instrument shape — the product id must BE a spot pair, `BASE-QUOTE` + # (`parse_spot_product_id`). Rail 18 and this rail ask different questions about the same + # id and neither subsumes the other: + # + # rail 18 — *what does it settle in?* the LAST segment, vs `settlement_currencies` + # rail 19 — *what shape is it?* the WHOLE id, vs the spot grammar + # + # THE RESIDUAL THIS CLOSES (feasibility study R2, + # `docs/experiments/2026-08-05-coinbase-asset-class-feasibility.md`): a derivative-shaped + # id whose final segment is a legitimate settlement currency passes both shipped + # defences. `quote_currency_of("BTC-PERP-USD")` is `"USD"` -- configured -- so rail 18 + # admits it, and `_asset` reduces it to the allowlisted `"BTC"`, so rail 1 admits it too. + # Only the shape stops it. Coinbase lists no such product today; rail 18 catches the + # classes that DO exist (`CDE` futures, equity hashes) on their settlement legs. This + # rail is what makes spot-only structural rather than a property of which suffixes the + # venue currently happens to use. + # + # BOTH SIDES, EVERY MODE, DCA INCLUDED — deliberately not in `LIVE_STATE_RAILS`, for + # rail 18's reason: it needs no broker and no account state, so paper cannot skip it, and + # a rehearsal cannot build a track record on trades live trading would veto. + # + # Spot-only is this agent's CHARTER, not an operator preference, so there is no config + # field here to widen (unlike rail 18's `settlement_currencies`). Nor does this consult + # `BrokerCapabilities.asset_classes`: `guards.check` has no broker handle, the live path + # constructs `data.cb_client.CoinbaseClient` which has no `capabilities()` at all, and + # paper passes `broker=None` -- so such a gate would be dead code that reads as a + # defence. That exact pattern was built and deleted once already (R1's "what was + # deliberately NOT shipped"). It belongs with the broker-port migration. + # + # Returns a VIOLATION, never raises, on any input. `parse_spot_product_id` is total. + if parse_spot_product_id(intent.product_id) is None: + violations.append( + f"spot_instrument: {intent.product_id!r} is not a well-formed spot product id " + f"(BASE-QUOTE, uppercase, exactly one hyphen). keel is spot-only: futures " + f"(BASE-DDMMMYY-CDE), equities (an opaque 64-char hash) and any other instrument " + f"shape are refused here regardless of what they settle in." + ) + for violation in violations: log_event( logger, diff --git a/packages/keel-broker-api/keel_broker_api/capabilities.py b/packages/keel-broker-api/keel_broker_api/capabilities.py index 0c551508..ef5f880c 100644 --- a/packages/keel-broker-api/keel_broker_api/capabilities.py +++ b/packages/keel-broker-api/keel_broker_api/capabilities.py @@ -6,10 +6,33 @@ from keel_broker_api.orders import ORDER_KINDS +#: The instrument classes an adapter may declare -- the three the 2026-08-05 Coinbase +#: asset-class study enumerated at the venue (`SPOT`, `FUTURE`, `EQUITY`; +#: `docs/experiments/2026-08-05-coinbase-asset-class-feasibility.md`). A closed vocabulary for +#: the same reason `ORDER_KINDS` is one: a declaration checked against nothing is a comment with +#: a type annotation, and the near-misses (`SPOT`, `future`, `perp`) are exactly the values that +#: would sit in a set gating nothing. +ASSET_CLASSES: frozenset[str] = frozenset({"spot", "futures", "equity"}) + @dataclass(frozen=True) class BrokerCapabilities: - """An adapter's self-declaration. The conformance suite verifies it does not lie.""" + """An adapter's self-declaration. The conformance suite verifies it does not lie. + + ⚠️ `asset_classes` is **not** what keeps keel spot-only today, and no engine code reads it. + The spot gate on the live path is **rail 19 (`spot_instrument`)** in + `keel/execution/guards.py`, which checks the product id's shape and needs no broker handle. + That is deliberate, not an oversight: `guards.check` has no broker, the only broker the live + path constructs is `keel/data/cb_client.py`'s `CoinbaseClient` -- which has no + `capabilities()` at all -- and the paper path passes `broker=None`, so a gate built on this + field would be dead code on every real path while reading as a defence. That exact pattern + was built and deleted once already (R1's "what was deliberately NOT shipped"). + + This field's job until then is to keep the declaration honest and checkable, so the + broker-port migration that makes `capabilities()` reachable inherits a vocabulary rather + than a free-form set. At that point the reconciliation belongs at LOAD time, not as a + per-order raise -- a raise on the exit path can trap a position. + """ venue: str supported_orders: frozenset[str] @@ -23,6 +46,9 @@ def __post_init__(self) -> None: unknown = self.supported_orders - ORDER_KINDS if unknown: raise ValueError(f"unknown order kinds: {sorted(unknown)}") + unknown_classes = self.asset_classes - ASSET_CLASSES + if unknown_classes: + raise ValueError(f"unknown asset classes: {sorted(unknown_classes)}") @property def can_preview(self) -> bool: @@ -30,4 +56,4 @@ def can_preview(self) -> bool: return self.supports_native_preview or self.synthesizes_preview -__all__ = ["BrokerCapabilities"] +__all__ = ["ASSET_CLASSES", "BrokerCapabilities"] diff --git a/packages/keel-broker-api/keel_broker_api/conformance/suite.py b/packages/keel-broker-api/keel_broker_api/conformance/suite.py index 5d7b93be..bb3a88f4 100644 --- a/packages/keel-broker-api/keel_broker_api/conformance/suite.py +++ b/packages/keel-broker-api/keel_broker_api/conformance/suite.py @@ -28,7 +28,7 @@ def broker(self) -> MyVenueAdapter: import pytest from keel_core.types import Granularity, Side -from keel_broker_api.capabilities import BrokerCapabilities +from keel_broker_api.capabilities import ASSET_CLASSES, BrokerCapabilities from keel_broker_api.orders import ( ORDER_KINDS, LimitGTC, @@ -86,6 +86,23 @@ def test_supported_orders_is_a_subset_of_the_known_kinds(self) -> None: def test_venue_is_a_non_empty_string(self) -> None: assert self.broker().capabilities().venue + def test_asset_classes_is_non_empty_and_drawn_from_the_known_vocabulary(self) -> None: + """An adapter that declares nothing declares nothing checkable. + + `BrokerCapabilities.__post_init__` already refuses an unknown class, so this is the + subset assertion restated where a future adapter author will read it -- plus the + non-emptiness that a frozenset default would otherwise let through silently. + """ + caps: BrokerCapabilities = self.broker().capabilities() + assert caps.asset_classes + assert caps.asset_classes <= ASSET_CLASSES + + def test_quote_currencies_is_non_empty(self) -> None: + """Rail 18's default settlement set is derived FROM this declaration. An adapter that + declared none would be saying it settles in nothing, which cannot be true of a venue + that accepts orders.""" + assert self.broker().capabilities().quote_currencies + # --- capabilities cannot lie about orders --------------------------------------------- def test_every_declared_order_kind_is_actually_accepted(self) -> None: diff --git a/packages/keel-core/keel_core/products.py b/packages/keel-core/keel_core/products.py index c4b0b749..245f3df8 100644 --- a/packages/keel-core/keel_core/products.py +++ b/packages/keel-core/keel_core/products.py @@ -4,10 +4,41 @@ PRODUCT, not of global configuration.** `BTC-USD` settles in USD whatever `config.quote_currency` says. Conflating the two let rail 13 check a balance the order never touches, which could pass an order the account had no settled funds for -- the exact case that rail exists to prevent. + +Two questions, two functions, deliberately not merged: + +- `quote_currency_of` -- *what does this settle in?* A loose `rpartition` parse, because rails + 13/18 want an answer even for an id that is not a spot pair: it is what lets rail 18's message + say `ADA-28AUG26-CDE` "settles in CDE", naming the venue suffix an operator can then look up. +- `parse_spot_product_id` -- *is this the shape of a spot pair at all?* A strict grammar, which + rail 19 gates on. Tightening `quote_currency_of` into this one would have cost rail 18 its + message and bought nothing, since the rails ask both questions anyway. """ from __future__ import annotations +import re + +# A well-formed SPOT product id is exactly `BASE-QUOTE`: two non-empty uppercase-alphanumeric +# legs, one hyphen, nothing else. Empirical basis, from the 2026-08-05 Coinbase asset-class +# study (`docs/experiments/2026-08-05-coinbase-asset-class-feasibility.md`), which enumerated +# every product the venue lists: +# +# - **SPOT (936): all match.** Every one has EXACTLY ONE hyphen (measured), and every observed +# quote leg -- `USD`, `USDC`, `EUR`, `GBP`, `USDT`, `BTC`, `ETH`, `INR`, `SGD`, `CAD`, `AUD` +# -- is 3-4 uppercase characters. Base legs are venue ticker symbols, which may lead with a +# digit (`1INCH-USD`), hence `[A-Z0-9]` rather than `[A-Z]`. +# - **FUTURE (99): none match.** Every contract is `ROOT-DDMMMYY-CDE` -- two hyphens. +# - **EQUITY (1000, plus 813 `alias` ids): none match.** Each is an opaque 64-char hex hash +# with no separator at all. +# +# The bounds are an envelope around what was measured, not a claim about it: 1-16 on the base is +# room for a long ticker; **2-10 on the quote is not free choice** -- it is `config`'s own +# `_CURRENCY_CODE_RE`, so a settlement currency that regex admits and this grammar rejects (or +# the reverse) cannot exist. A well-formed spot id no `settlement_currencies` set could ever +# name would be vetoed by rail 18 forever with nothing saying why. +_SPOT_PRODUCT_ID_RE = re.compile(r"[A-Z0-9]{1,16}-[A-Z0-9]{2,10}") + def quote_currency_of(product_id: str | None) -> str | None: """The settlement leg of `product_id` (`"BTC-USD"` -> `"USD"`), uppercased. @@ -23,3 +54,37 @@ def quote_currency_of(product_id: str | None) -> str | None: if not separator or not base.strip() or not quote.strip(): return None return quote.strip().upper() + + +def parse_spot_product_id(product_id: object) -> tuple[str, str] | None: + """`(base, quote)` if `product_id` is a well-formed SPOT id, else `None`. + + `"BTC-USD"` -> `("BTC", "USD")`; `"ADA-28AUG26-CDE"` (futures), a 64-hex equity hash, and + `"BTC-PERP-USD"` (the derivative-shaped id whose settlement leg is legitimate) all -> `None`. + See `_SPOT_PRODUCT_ID_RE` above for the grammar and the census it rests on. + + This is a SHAPE check, not a settlement check, and not an existence check. It says nothing + about whether the venue lists the product, whether the base is allowlisted (rail 1), or + whether the quote is a settlement currency the operator configured (rail 18) -- `BTC-EUR` + parses cleanly and rail 18 still vetoes it. The single question it answers is the one no + other check asks: *is this the shape of a spot pair, or of something else?* + + **No normalisation, deliberately.** A lowercase id does not parse; it is not lowered first. + `quote_currency_of` uppercases because it is answering "which currency is this" about an id + that already exists, but silently accepting `btc-usd` here would mean an operator's typo + became a traded product, and the CLI's job (`keel.commands._products.validate_product_ids`) + is to say "did you mean BTC-USD?" instead. + + **Total by contract.** Never raises, on any input, including non-strings -- `product_id` is + typed `object` to say so. Callers include the historical-order walk in + `guards._open_exposure_by_asset`, which runs over whatever the audit log happens to hold; an + exception there would turn one bad row into a crashed agent cycle, strictly worse than the + hole rail 19 closes. + """ + if not isinstance(product_id, str): + return None + match = _SPOT_PRODUCT_ID_RE.fullmatch(product_id) + if match is None: + return None + base, _, quote = product_id.partition("-") + return base, quote diff --git a/tests/broker_api/test_capabilities.py b/tests/broker_api/test_capabilities.py new file mode 100644 index 00000000..6d68e52e --- /dev/null +++ b/tests/broker_api/test_capabilities.py @@ -0,0 +1,52 @@ +"""`BrokerCapabilities` -- an adapter's self-declaration, checked for sense at construction. + +`supported_orders` has been checked against `ORDER_KINDS` since the port landed; `asset_classes` +was a free-form set nobody validated. A declaration nothing checks is a comment with a type +annotation, and it is the field a future broker-port migration will gate spot-only on -- so the +vocabulary is pinned now, while the field is still cheap to change (feasibility study R2). +""" + +from __future__ import annotations + +import pytest +from keel_broker_api.capabilities import ASSET_CLASSES, BrokerCapabilities + + +def _caps(**overrides: object) -> BrokerCapabilities: + base: dict[str, object] = dict( + venue="test", + supported_orders=frozenset({"market_ioc_quote"}), + supports_native_preview=True, + synthesizes_preview=False, + supports_fee_summary=True, + quote_currencies=frozenset({"USD"}), + asset_classes=frozenset({"spot"}), + ) + base.update(overrides) + return BrokerCapabilities(**base) # type: ignore[arg-type] + + +def test_the_vocabulary_is_the_three_classes_the_venue_study_enumerated() -> None: + """`SPOT`, `FUTURE` and `EQUITY` are what Coinbase lists. A set that drifts from the + vocabulary adapters declare against is how a typo becomes a silently permissive gate.""" + assert ASSET_CLASSES == frozenset({"spot", "futures", "equity"}) + + +def test_a_declaration_of_spot_is_accepted() -> None: + assert _caps().asset_classes == frozenset({"spot"}) + + +@pytest.mark.parametrize("bogus", ["margin_spot", "SPOT", "perp", "", "future"]) +def test_an_unknown_asset_class_is_refused_at_construction(bogus: str) -> None: + """Mirrors the `supported_orders` check: an adapter cannot invent a class the engine has no + vocabulary for. `SPOT` and `future` are the near-misses that would otherwise pass silently.""" + with pytest.raises(ValueError) as excinfo: + _caps(asset_classes=frozenset({bogus})) + assert bogus in str(excinfo.value) + + +def test_the_unknown_order_kind_check_still_fires() -> None: + """The new check must not shadow the one that was already there.""" + with pytest.raises(ValueError) as excinfo: + _caps(supported_orders=frozenset({"iceberg"})) + assert "iceberg" in str(excinfo.value) diff --git a/tests/commands/test_products.py b/tests/commands/test_products.py new file mode 100644 index 00000000..46c50b67 --- /dev/null +++ b/tests/commands/test_products.py @@ -0,0 +1,99 @@ +"""`keel.commands._products.validate_product_ids` -- rejecting a bad id at the KEYBOARD. + +Rails 18 and 19 stop an inadmissible product where the agent trades it, which is the right place +for a safety rail and the wrong place for a typo. `keel rules seed --products XLM-28AUG26-CDE +--status live` used to write a row the agent would then poll every cycle and veto forever, with +the reason buried in a log line. These tests pin the two questions the CLI asks instead -- the +same two the rails ask, deliberately -- and the fact that it never repairs an id on the +operator's behalf. +""" + +from __future__ import annotations + +import pytest + +from keel.commands._products import validate_product_ids + +_SETTLEMENT = frozenset({"USD", "USDC"}) + + +def test_well_formed_settled_ids_are_returned_unchanged(): + ids = ["BTC-USD", "ETH-USD", "PAXG-USD", "ADA-USD", "XLM-USD", "BTC-USDC"] + assert validate_product_ids(ids, _SETTLEMENT) == ids + + +@pytest.mark.parametrize( + "bad", + [ + "XLM-28AUG26-CDE", # futures: two hyphens + "BTC-PERP-USD", # the R2 residual: derivative-shaped, USD-settled + "ac568fb9e6c5a67da94f065a49fb7b0c59b7b258cfdf0a3b1560849071c3b05e", # equity hash + "BTCUSD", + "BTC-", + "-USD", + "BTC--USD", + "BTC/USD", + ], +) +def test_an_id_that_is_not_a_spot_pair_is_rejected_on_SHAPE(bad): + with pytest.raises(ValueError) as excinfo: + validate_product_ids([bad], _SETTLEMENT) + assert bad in str(excinfo.value) + assert "not a spot product id" in str(excinfo.value) + + +def test_a_well_formed_pair_in_an_unconfigured_currency_is_rejected_on_SETTLEMENT(): + """`BTC-EUR` is a real Coinbase spot pair and passes the shape check. Rail 18 would veto it + on every cycle; the operator should hear that now, and hear WHICH check refused it.""" + with pytest.raises(ValueError) as excinfo: + validate_product_ids(["BTC-EUR"], _SETTLEMENT) + message = str(excinfo.value) + assert "BTC-EUR" in message + assert "settles in EUR" in message + assert "USD" in message and "USDC" in message + + +def test_the_settlement_set_is_the_operators_not_a_hardcode(): + """Widening `settlement_currencies` in config admits `-EUR` here too -- the CLI must ask the + same question rail 18 asks, of the same set, or the two disagree.""" + assert validate_product_ids(["BTC-EUR"], frozenset({"EUR"})) == ["BTC-EUR"] + with pytest.raises(ValueError): + validate_product_ids(["BTC-USD"], frozenset({"EUR"})) + + +@pytest.mark.parametrize("lower", ["btc-USD", "btc-usd", "BTC-usd", "Btc-Usd"]) +def test_a_lowercase_id_is_REJECTED_with_a_hint_never_silently_uppercased(lower): + """Silently repairing it would mean the id the operator typed is not the id keel trades. + + A product id is a venue identifier, not free text; guessing at one is how a typo becomes a + position. The hint costs one line and keeps the operator in charge of the fix. + """ + with pytest.raises(ValueError) as excinfo: + validate_product_ids([lower], _SETTLEMENT) + message = str(excinfo.value) + assert lower in message + assert f"did you mean {lower.upper()}" in message + + +def test_every_bad_id_is_reported_at_once_not_just_the_first(): + """An operator fixing a list one error per run learns the list slowly and gives up fast.""" + with pytest.raises(ValueError) as excinfo: + validate_product_ids(["BTC-USD", "XLM-28AUG26-CDE", "BTC-EUR", "eth-usd"], _SETTLEMENT) + message = str(excinfo.value) + assert "XLM-28AUG26-CDE" in message + assert "BTC-EUR" in message + assert "eth-usd" in message + + +def test_an_empty_list_is_not_an_error_here(): + """Emptiness is a different complaint, owned by the caller that knows what empty means for + it -- `_parse_products_option` falls back to the allowlist rather than validating nothing.""" + assert validate_product_ids([], _SETTLEMENT) == [] + + +def test_it_never_raises_anything_but_ValueError(): + """`click.BadParameter` wrapping at the call sites depends on this: an unexpected exception + type would surface as a traceback instead of a usage error.""" + for weird in ([None], [42], [""], [" "], [b"BTC-USD"]): + with pytest.raises(ValueError): + validate_product_ids(weird, _SETTLEMENT) diff --git a/tests/compliance/test_assets_cli.py b/tests/compliance/test_assets_cli.py index 33cd8016..ff940993 100644 --- a/tests/compliance/test_assets_cli.py +++ b/tests/compliance/test_assets_cli.py @@ -813,6 +813,36 @@ def test_the_settlement_criterion_still_catches_an_EXTERNALLY_supplied_product( assert "REJECT" in result.output +def test_screen_REPORTS_on_a_futures_id_rather_than_refusing_the_option( + tmp_path, valid_config_path +): + """`assets screen` is the ONE `--products` caller that does not validate its option, and this + pins that exception (feasibility study R2). + + Every other caller -- `fetch`, `monitor`, `simulate`, `rules seed` -- refuses an id keel + cannot trade at the keyboard. Screening must not, because screening is the command that + ANSWERS "may keel trade this, and why not". A usage error would make the one tool whose job + is to explain an inadmissible asset the one tool that cannot be asked about one. It writes + nothing and orders nothing; rails 18/19 stop the id if it ever reaches an order. + """ + db_path = tmp_path / "t.db" + repo = _repo_at(db_path) + _seed_history(repo, "ADA-28AUG26-CDE") + runner = CliRunner() + assert _attest(runner, db_path, valid_config_path, "ADA").exit_code == 0 + + result = runner.invoke( + cli, + ["--db", str(db_path), "--config", str(valid_config_path), + "assets", "screen", "--products", "ADA-28AUG26-CDE"], + ) + + assert result.exit_code == 0, "screening must report a verdict, not a usage error" + assert "REJECT" in result.output + # The verdict carries the REASON -- which is the whole point of not refusing the option. + assert "settlement" in result.output + + # -- assets propose ----------------------------------------------------------------------------- diff --git a/tests/core/test_products.py b/tests/core/test_products.py index 233312a1..a623973d 100644 --- a/tests/core/test_products.py +++ b/tests/core/test_products.py @@ -1,12 +1,19 @@ -"""`keel_core.products` -- deriving an order's settlement leg from its product id. +"""`keel_core.products` -- deriving an order's settlement leg, and its SHAPE, from its id. The currency an order spends is a property of the PRODUCT, never of global config. Getting this wrong let rail 13 guard a balance the order never touches. + +`parse_spot_product_id` answers the other half of the question -- not "what does it settle in" +but "is this the shape of a spot pair at all" -- which is what rail 19 gates on. The two are +deliberately separate functions: `quote_currency_of`'s loose `rpartition` parse is what lets rail +18 name `CDE` in its message, and tightening it would change that message for no gain. """ from __future__ import annotations -from keel_core.products import quote_currency_of +import pytest +from keel_core.config import _CURRENCY_CODE_RE +from keel_core.products import parse_spot_product_id, quote_currency_of def test_the_quote_leg_is_the_part_after_the_last_dash(): @@ -23,3 +30,103 @@ def test_a_malformed_product_id_returns_None_so_callers_fail_closed(): """`None` is what rail 13 already treats as 'unknown' and vetoes on.""" for bad in ("BTC", "", " ", "BTC-", "-USD", None): assert quote_currency_of(bad) is None, f"{bad!r} should not resolve to a currency" + + +# -- parse_spot_product_id (rail 19's grammar) ------------------------------------------------- + +#: The three instrument classes the 2026-08-05 Coinbase asset-class study enumerated, as real +#: ids from that study, each paired with the parse the spot grammar must produce. Only the SPOT +#: rows may parse: a futures contract is `ROOT-DDMMMYY-CDE` (two hyphens) and an equity is a +#: 64-char hex hash (no hyphen), so both fail on shape alone -- no instrument model needed. +_REAL_PRODUCT_IDS = [ + # SPOT -- every id the live deployment can construct, plus the settlement legs rail 18 + # rejects by default (still well-formed SPOT; shape and settlement are separate questions). + ("BTC-USD", ("BTC", "USD")), + ("ETH-USD", ("ETH", "USD")), + ("PAXG-USD", ("PAXG", "USD")), + ("ADA-USD", ("ADA", "USD")), + ("XLM-USD", ("XLM", "USD")), + ("BTC-USDC", ("BTC", "USDC")), + ("BTC-EUR", ("BTC", "EUR")), + ("ETH-USDT", ("ETH", "USDT")), + ("SOL-BTC", ("SOL", "BTC")), + ("1INCH-USD", ("1INCH", "USD")), + # FUTURE -- two hyphens. `quote_currency_of` reads the venue suffix `CDE` as a settlement + # leg (which is how rail 18 catches it); the shape grammar rejects it outright. + ("ADA-28AUG26-CDE", None), + ("BIT-28AUG26-CDE", None), + ("XLM-28AUG26-CDE", None), + # The residual R2 closes: a derivative-shaped id whose FINAL segment is a legitimate + # settlement currency, so rail 18 passes it and only the shape grammar stops it. + ("BTC-PERP-USD", None), + # EQUITY -- an opaque 64-char hex hash, no separator at all. + ("ac568fb9e6c5a67da94f065a49fb7b0c59b7b258cfdf0a3b1560849071c3b05e", None), + # ...and an equity `alias` id (the same share's other quote leg), equally opaque. + ("a4a295140e2f9a6dbb2fc9b0f0a6f6e0a1b2c3d4e5f60718293a4b5c6d7e6162", None), +] + + +@pytest.mark.parametrize(("product_id", "expected"), _REAL_PRODUCT_IDS) +def test_the_spot_grammar_admits_spot_and_rejects_every_other_class(product_id, expected): + assert parse_spot_product_id(product_id) == expected + + +@pytest.mark.parametrize( + "bad", + [ + "", + "-", + "BTC-", + "-USD", + "BTC--USD", + "BTC-U", # quote leg below the 2-char floor `_CURRENCY_CODE_RE` sets + "BTC-VERYLONGQUOTE", # ...and above its 10-char ceiling + "btc-usd", # lowercase is a TYPO, not an id -- never silently uppercased + "BTC-usd", + "BTC/USD", + "BTC_USD", + " BTC-USD", + "BTC-USD ", + "BTC USD", + "BTCUSD", + "BTC-US D", + "BTC-US.D", + ], +) +def test_a_malformed_id_does_not_parse(bad): + assert parse_spot_product_id(bad) is None + + +@pytest.mark.parametrize( + "weird", [None, 42, 3.5, b"BTC-USD", ["BTC-USD"], {"BTC": "USD"}, object()] +) +def test_the_parser_is_TOTAL_and_never_raises(weird): + """Rail 19 and `_asset` both run over historical audit rows, where one bad value must + produce a veto, never an exception that crashes the agent cycle.""" + assert parse_spot_product_id(weird) is None + + +@pytest.mark.parametrize(("product_id", "expected"), _REAL_PRODUCT_IDS) +def test_any_parsed_quote_leg_is_a_valid_currency_code_by_configs_own_grammar( + product_id, expected +): + """The two grammars cannot be allowed to disagree about what a currency code is. + + Rail 18 compares `quote_currency_of`'s output against `config.settlement_currencies`, whose + members are shape-checked at parse by `_CURRENCY_CODE_RE`. If this parser admitted a quote + leg that regex rejects, there would be a well-formed spot id no configurable settlement set + could ever name -- vetoed by rail 18 forever with nothing saying why. + """ + parsed = parse_spot_product_id(product_id) + if parsed is None: + return + assert _CURRENCY_CODE_RE.fullmatch(parsed[1]), parsed + + +def test_the_two_parsers_agree_on_the_quote_leg_of_a_well_formed_spot_id(): + """Where the shape grammar admits an id, `quote_currency_of` must read the same leg off it + -- rails 18 and 19 would otherwise be talking about different halves of the same product.""" + for product_id, expected in _REAL_PRODUCT_IDS: + if expected is None: + continue + assert quote_currency_of(product_id) == expected[1], product_id diff --git a/tests/execution/test_guards.py b/tests/execution/test_guards.py index f145d6b6..36711e0e 100644 --- a/tests/execution/test_guards.py +++ b/tests/execution/test_guards.py @@ -9,6 +9,7 @@ from __future__ import annotations +import logging from decimal import Decimal from typing import Any @@ -1121,6 +1122,243 @@ def test_rail18_violation_names_the_product_the_currency_and_the_allowed_set( assert "USD" in violation and "USDC" in violation +# -- rail 19: spot instrument shape (instrument admission, every mode, both sides) -------------- +# +# Rail 18 closed the CLASS hole the study found by execution. Its residual -- R2 in +# `docs/experiments/2026-08-05-coinbase-asset-class-feasibility.md` -- is that it checks the +# settlement LEG, not the instrument SHAPE. `quote_currency_of("BTC-PERP-USD")` returns `"USD"`, +# which is a configured settlement currency, and `_asset` returns the allowlisted `"BTC"`, so a +# derivative-shaped id with a legitimate final segment passes BOTH shipped defences. Rail 19 is +# the shape gate that was missing. + +#: The residual, as one id: derivative-shaped, USD-settled, allowlisted base. Coinbase does not +#: list this product today -- that is the point. Rail 18 is a check on the settlement leg and +#: cannot see the middle segment, so a venue that ever listed one would find keel's rails open. +DERIVATIVE_SHAPED_USD_ID = "BTC-PERP-USD" + + +def test_rail19_a_usd_settled_derivative_shaped_id_is_vetoed(repo: Repository) -> None: + """The R2 residual, stated as a test: SELL `BTC-PERP-USD` on the live allowlist. + + Both shipped defences pass it -- rail 1 because `_asset` reduces it to the allowlisted + `BTC`, rail 18 because its settlement leg really is `USD`. Assert BOTH of those, so a future + reader cannot mistake this for a duplicate of either, and only rail 19 stops it. + """ + intent = _intent(product_id=DERIVATIVE_SHAPED_USD_ID, side=Side.SELL) + result = check(intent, repo, _config(allowlist=LIVE_ALLOWLIST), NOW_TS) + + assert "spot_instrument" in _keys(result) + assert "settlement_currency" not in _keys(result), ( + "rail 18 passes it -- its settlement leg is genuinely USD; that is the residual" + ) + assert "halal_allowlist" not in _keys(result), "rail 1 passes it too -- `_asset` sees BTC" + assert result.ok is False + + +def test_rail19_is_vetoed_offline_too_and_is_never_skipped(repo: Repository) -> None: + """Like rail 18, this needs no broker and no live account state, so paper cannot skip it. + A rehearsal that admitted a derivative would prove a track record live trading would veto.""" + intent = _intent( + product_id=DERIVATIVE_SHAPED_USD_ID, + side=Side.SELL, + available_quote=None, + withdrawals_enabled=None, + ) + result = check(intent, repo, _config(allowlist=LIVE_ALLOWLIST), NOW_TS, offline=True) + + assert "spot_instrument" in _keys(result) + assert "spot_instrument" not in result.skipped_rails + assert "spot_instrument" not in LIVE_STATE_RAILS + + +def test_rail19_vetoes_both_sides_in_every_mode(repo: Repository) -> None: + for side in (Side.BUY, Side.SELL): + for offline in (False, True): + result = check( + _intent(product_id=DERIVATIVE_SHAPED_USD_ID, side=side), + repo, + _config(allowlist=LIVE_ALLOWLIST), + NOW_TS, + offline=offline, + ) + assert "spot_instrument" in _keys(result), (side, offline) + + +def test_rail19_vetoes_DCA_too(repo: Repository) -> None: + """DCA is exempt from rails 8 and 11, never from instrument admission (§12.6).""" + result = check( + _intent(product_id=DERIVATIVE_SHAPED_USD_ID, is_dca=True, rule_kind="dca"), + repo, + _config(allowlist=LIVE_ALLOWLIST), + NOW_TS, + ) + assert "spot_instrument" in _keys(result) + + +def test_rail19_vetoes_a_futures_contract_and_an_equity_hash(repo: Repository) -> None: + """The two classes rail 18 already stops are stopped here too -- belt and braces, and the + reason rail 19 can be read on its own without tracing what rail 18 happens to catch.""" + for product_id in (FUTURES_PRODUCT_ID, EQUITY_PRODUCT_ID): + result = check( + _intent(product_id=product_id), repo, _config(allowlist=LIVE_ALLOWLIST), NOW_TS + ) + assert "spot_instrument" in _keys(result), product_id + + +def test_rail19_never_raises_on_a_malformed_product_id(repo: Repository) -> None: + """A veto, never an exception -- same contract as rail 18, for the same reason: the rail + machinery also walks historical filled orders, where one bad row must not crash the cycle.""" + for product_id in ("", "-", "BTC-", "-USD", "BTC--USD", "btc-usd", " ", "BTCUSD"): + result = check(_intent(product_id=product_id), repo, _config(), NOW_TS) + assert "spot_instrument" in _keys(result), product_id + + +def test_rail19_passes_every_live_deployment_product(repo: Repository) -> None: + """The six rules in the live DB verbatim (five turtle + the BTC DCA rule). Rail 19 must be + invisible to the deployment as it stands -- blast radius nil, exactly as rail 18's was.""" + for product_id in ("BTC-USD", "ETH-USD", "PAXG-USD", "ADA-USD", "XLM-USD"): + result = check( + _intent(product_id=product_id), repo, _config(allowlist=LIVE_ALLOWLIST), NOW_TS + ) + assert "spot_instrument" not in _keys(result), product_id + + dca = check( + _intent(product_id="BTC-USD", is_dca=True, rule_kind="dca"), + repo, + _config(allowlist=LIVE_ALLOWLIST), + NOW_TS, + ) + assert "spot_instrument" not in _keys(dca) + + +def test_rail19_passes_a_well_formed_spot_pair_rail_18_rejects(repo: Repository) -> None: + """Shape and settlement are separate questions and must stay separately reported: `BTC-EUR` + is a perfectly well-formed spot pair, vetoed only by the settlement set.""" + result = check(_intent(product_id="BTC-EUR"), repo, _config(allowlist=LIVE_ALLOWLIST), NOW_TS) + + assert "spot_instrument" not in _keys(result) + assert "settlement_currency" in _keys(result) + + +def test_rail19_violation_names_the_product_and_says_what_shape_is_required( + repo: Repository, +) -> None: + """An operator must be able to act on the message without reading this module.""" + result = check( + _intent(product_id=DERIVATIVE_SHAPED_USD_ID), + repo, + _config(allowlist=LIVE_ALLOWLIST), + NOW_TS, + ) + violation = next(v for v in result.violations if v.startswith("spot_instrument")) + assert DERIVATIVE_SHAPED_USD_ID in violation + assert "BASE-QUOTE" in violation + + +# -- `_asset` and the history walk are total --------------------------------------------------- + + +def test__asset_is_total(repo: Repository) -> None: + """`_asset` runs over every historical filled order, so it must never raise on anything the + audit log can hold. `_asset(None)` used to raise `AttributeError`.""" + for weird in ( + None, + "", + "-", + "BTC-", + "-USD", + "BTC--USD", + "btc-usd", + " ", + EQUITY_PRODUCT_ID, + FUTURES_PRODUCT_ID, + 42, + 3.5, + b"BTC-USD", + ["BTC-USD"], + {"BTC": "USD"}, + ): + guards._asset(weird) # must not raise + + +def test__asset_still_returns_exactly_what_it_returned_before(repo: Repository) -> None: + """Totality is the ONLY behaviour change. `_asset` stays the loose parse on purpose. + + Tightening it to `parse_spot_product_id` would silently change rail 1's verdict on a futures + id -- destroying the "rail 1 passes the contract, that is the hole" assertion above -- and + would split a derivative's exposure out of its root's bucket, under-stating the figure rails + 4/5/6 cap. Rail 19 is where an unparseable id is refused; this is only a grouping key. + """ + assert guards._asset("BTC-USD") == "BTC" + assert guards._asset(FUTURES_PRODUCT_ID) == "ADA" + assert guards._asset(DERIVATIVE_SHAPED_USD_ID) == "BTC" + assert guards._asset(EQUITY_PRODUCT_ID) == EQUITY_PRODUCT_ID # no separator: the whole hash + assert guards._asset(None) == "None" # a key, not an AttributeError + + +def test_open_exposure_walk_survives_a_malformed_history_row( + repo: Repository, caplog: pytest.LogCaptureFixture +) -> None: + """One unparseable audit row must not crash the cycle -- and must not be SKIPPED either. + + Skipping would REDUCE measured exposure, loosening rails 4/5/6: that is fail-OPEN, which is + why this deliberately departs from the feasibility doc's "skip-or-flag" wording. Counting it + can only over-state exposure, which is the closed direction. + """ + _seed_filled_order( + repo, + product_id=FUTURES_PRODUCT_ID, + side=Side.BUY, + qty=Decimal("1"), + price=Decimal("400"), + created_at=NOW_TS - 86_400, + ) + _seed_filled_order( + repo, + product_id=EQUITY_PRODUCT_ID, + side=Side.BUY, + qty=Decimal("2"), + price=Decimal("50"), + created_at=NOW_TS - 86_400, + ) + + with caplog.at_level(logging.WARNING): + exposure = guards._open_exposure_by_asset(repo) + result = check(_intent(), repo, _config(allowlist=LIVE_ALLOWLIST), NOW_TS) + + # Counted under `_asset`'s key: the futures contract lands in its root's bucket (merging can + # only over-state ADA, the closed direction), the separator-less hash under itself. + assert exposure == {"ADA": Decimal("400"), EQUITY_PRODUCT_ID: Decimal("100")} + # The WARNING is how an operator finds out -- and it must NAME the row, or it cannot be + # acted on. `log_event` carries fields in the `keel_fields` extra, not in the message. + warned = { + r.keel_fields["product"] + for r in caplog.records + if r.getMessage() == "guards.exposure_row_unparseable" + } + assert warned == {FUTURES_PRODUCT_ID, EQUITY_PRODUCT_ID} + assert "guards.exposure_row_unparseable" in caplog.text + assert isinstance(result, GuardResult) # no raise + + +def test_a_malformed_history_row_still_counts_toward_the_exposure_cap(repo: Repository) -> None: + """The rail that matters: the $400 above is real money at risk, and rail 4 must see it.""" + _seed_filled_order( + repo, + product_id=FUTURES_PRODUCT_ID, + side=Side.BUY, + qty=Decimal("1"), + price=Decimal("960"), + created_at=NOW_TS - 86_400, + ) + + result = check(_intent(notional=Decimal("50")), repo, _config(), NOW_TS) + + assert "total_exposure_cap" in _keys(result), ( + "an unparseable row was dropped from exposure -- that is fail-OPEN" + ) + + # -- offline mode (paper trading only) ----------------------------------------- diff --git a/tests/test_init_and_seed.py b/tests/test_init_and_seed.py index 58c5a0f5..dd7d072f 100644 --- a/tests/test_init_and_seed.py +++ b/tests/test_init_and_seed.py @@ -104,6 +104,75 @@ def test_seed_status_live_bypasses_the_gate_and_warns(tmp_path): assert live[0]["kind"] == "dca" +# -- rules seed --products: rejected at the KEYBOARD, not at the rails --------- +# +# Feasibility study R2. Rails 18/19 stop an inadmissible product where the agent trades it; that +# is the right place for a safety rail and the wrong place for a typo. Seeding one wrote a row +# that looked seeded, that the agent then polled every cycle and vetoed forever. + + +def test_seed_refuses_a_futures_contract_and_names_it(tmp_path, valid_config_path): + repo = _repo(tmp_path) + result = CliRunner().invoke( + cli, ["--db", str(tmp_path / "t.db"), "--config", str(valid_config_path), + "rules", "seed", "--kinds", "dca", + "--products", "XLM-28AUG26-CDE", "--status", "live"] + ) + + assert result.exit_code != 0 + assert "XLM-28AUG26-CDE" in result.output + assert repo.get_rules() == [], "nothing may be seeded when the list is rejected" + + +def test_seed_refuses_a_derivative_shaped_id_that_settles_in_usd(tmp_path, valid_config_path): + """The R2 residual at the keyboard: rail 18 alone would pass `BTC-PERP-USD`.""" + result = CliRunner().invoke( + cli, ["--db", str(tmp_path / "t.db"), "--config", str(valid_config_path), + "rules", "seed", "--kinds", "dca", "--products", "BTC-PERP-USD"] + ) + assert result.exit_code != 0 + assert "BTC-PERP-USD" in result.output + + +def test_seed_refuses_an_unsettleable_but_well_formed_pair(tmp_path, valid_config_path): + """`BTC-EUR` is a real spot pair; it fails on settlement membership, not on shape.""" + result = CliRunner().invoke( + cli, ["--db", str(tmp_path / "t.db"), "--config", str(valid_config_path), + "rules", "seed", "--kinds", "dca", "--products", "BTC-EUR"] + ) + assert result.exit_code != 0 + assert "BTC-EUR" in result.output + assert "settles in EUR" in result.output + + +def test_seed_refuses_a_lowercase_id_with_a_hint_rather_than_fixing_it( + tmp_path, valid_config_path +): + repo = _repo(tmp_path) + result = CliRunner().invoke( + cli, ["--db", str(tmp_path / "t.db"), "--config", str(valid_config_path), + "rules", "seed", "--kinds", "dca", "--products", "btc-USD"] + ) + + assert result.exit_code != 0 + assert "did you mean BTC-USD" in result.output + assert repo.get_rules() == [], "never silently uppercased into a real seeded rule" + + +def test_seed_with_no_products_still_seeds_the_allowlist(tmp_path, valid_config_path): + """The default path must keep working: it now loads config unconditionally (it needs the + settlement set to validate against), where before it loaded config only on this branch.""" + repo = _repo(tmp_path) + result = CliRunner().invoke( + cli, ["--db", str(tmp_path / "t.db"), "--config", str(valid_config_path), + "rules", "seed", "--kinds", "dca"] + ) + + assert result.exit_code == 0, result.output + seeded = {r["params"]["product_id"] for r in repo.get_rules()} + assert seeded == {"BTC-USD", "ETH-USD", "PAXG-USD"} + + def test_seed_rejects_an_unknown_status(tmp_path): result = CliRunner().invoke( cli, ["--db", str(tmp_path / "t.db"), "rules", "seed", "--status", "bogus"] From 5efb66463a13cdf76a3629f147ffb0141574fd48 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Wed, 5 Aug 2026 18:48:17 -0400 Subject: [PATCH 2/2] fix(guards,screen,config): close the six review findings on rail 19 (R2) Every claim below was re-verified by execution before being changed. 1. `_open_exposure_by_asset` is now SIDE-DEPENDENT, and the absolute claim it carried was false. That figure is NET -- BUY adds, SELL subtracts -- so counting an unparseable SELL REDUCES the bucket rails 4/5/6 cap, i.e. it loosens them. Measured: ADA-USD BUY $900 -> {'ADA': 900}; + an ADA-28AUG26-CDE SELL $800 (same `_asset` bucket) -> {'ADA': 100}, and past the BUY total the trailing `if amt > 0` filter deletes the bucket outright. A futures SELL is exactly the row shape the study found passing every rail. Malformed BUY: counted (over-states = closed). Malformed SELL: skipped. Both WARN, now carrying `action=counted|skipped`. The old tests seeded BUY rows only, so the SELL half was unproven; two tests cover it now. 2. `assets screen` ADMITted the very instrument rail 19 exists to veto. Proven: with BTC attested and 2000 bars, `--products BTC-PERP-USD` printed ADMIT. The screen's only id-derived criterion was `quotable_in_settlement_currency`, which reads the LAST segment -- and BTC-PERP-USD's is USD. `screen_asset` gains a `spot_instrument` criterion applying `parse_spot_product_id` (rail 19's grammar, imported not restated) to a `product_id` MarketFacts now carries. It flows through `_screen_product`, so `assets propose` and `holdings --screen` inherit it. The `--products` exemption is PRESERVED -- the screen reports rather than refuses -- but what it reports is now true. 3. A non-uppercase allowlist entry was silently untradeable: `allowlist: [btc]` derived `btc-USD`, which fails rails 1 and 19 on every cycle. `_parse_allowlist` now shape-checks each entry against `is_spot_base_code`, the base-leg half of the same grammar, split out of `_SPOT_PRODUCT_ID_RE` so the two cannot disagree. REJECTED with the uppercase form in the message, not folded: a settlement code is COMPARED against uppercased output (so folding is free), an allowlist entry is CONCATENATED into a venue identifier. Blast radius nil -- every shipped config already lists uppercase tickers. 4. `rules seed` loads config unconditionally now, so four tests passed only because pytest runs from the repo root. All four pass `--config`; from a config-less cwd the branch now fails exactly the 7 tests `main` does. (The review named two; `tests/test_cli.py` had two more.) 5. `validate_product_ids` is split so callers can weigh the two failure kinds: `check_product_ids` returns typed `ProductIdProblem`s (SHAPE / SETTLEMENT). SHAPE stays fatal everywhere -- always a typo. SETTLEMENT stays fatal on `rules seed`, which writes a row the agent polls, and WARNS on `fetch` / `simulate`, which place no orders. Making it fatal there broke the screening workflow: `assets screen --products BTC-EUR` is exempt so the pair CAN be asked about, but the answer is dominated by "0 daily bars", and there was no way to fetch that history without first widening `settlement_currencies`. 6. Nits, each a claim that was wrong rather than merely thin: - Rail 19's comment (and the module docstring) overstated. `BTC-PERP` -- Coinbase International's real perpetual format -- PASSES rail 19's grammar and is stopped by rail 18 alone, so for two-segment ids spot-only remains a property of `settlement_currencies`. Stated honestly, pinned by a test. - The feasibility doc described the `assets screen` exemption BACKWARDS. - `monitor` was listed as a `--products` caller in two docstrings; it has no such option. (The module docstring never claimed it -- that third instance did not exist.) - ASSET_CLASSES cited the venue's SPOT/FUTURE/EQUITY while defining spot/futures/equity; now stated as keel-side spellings, which is what makes the near-miss test's rejection of "future" coherent rather than contradictory. - `test_quote_currencies_is_non_empty` kept, with the real reason: rail 18's default MIRRORS the adapter declaration (neither derived from the other), and an empty declaration makes that agreement vacuous. 1888 pass, ruff clean. Co-Authored-By: Claude Opus 5 (1M context) --- ...-08-05-coinbase-asset-class-feasibility.md | 77 +++++-- keel/cli.py | 40 +++- keel/commands/_products.py | 188 +++++++++++++----- keel/commands/rules.py | 6 +- keel/compliance/screen.py | 37 ++++ keel/execution/guards.py | 72 +++++-- .../keel_broker_api/capabilities.py | 18 +- .../keel_broker_api/conformance/suite.py | 14 +- packages/keel-core/keel_core/config.py | 38 ++++ packages/keel-core/keel_core/products.py | 29 ++- tests/broker_api/test_capabilities.py | 13 +- tests/commands/test_products.py | 59 +++++- tests/compliance/test_assets_cli.py | 53 +++++ tests/compliance/test_screen.py | 69 ++++++- tests/core/test_products.py | 52 ++++- tests/data/test_fetch_cli.py | 75 +++++++ tests/execution/test_guards.py | 114 ++++++++++- tests/test_cli.py | 12 +- tests/test_config.py | 63 ++++++ tests/test_init_and_seed.py | 19 +- tests/test_proposer.py | 6 +- 21 files changed, 933 insertions(+), 121 deletions(-) diff --git a/docs/experiments/2026-08-05-coinbase-asset-class-feasibility.md b/docs/experiments/2026-08-05-coinbase-asset-class-feasibility.md index 5c4eabec..790999e5 100644 --- a/docs/experiments/2026-08-05-coinbase-asset-class-feasibility.md +++ b/docs/experiments/2026-08-05-coinbase-asset-class-feasibility.md @@ -707,6 +707,17 @@ crypto). Gold and silver are named prohibited in the KB. Do not start here. leg's 2–10 bound is not a free choice — it is config's own `_CURRENCY_CODE_RE`, so a well-formed spot id that no `settlement_currencies` set could ever name cannot exist. A property test pins that agreement. + - ⚠️ **What rail 19 does NOT close, stated plainly.** It makes spot-only structural for ids of + three or more segments. It does not for **two-segment** ones: `BTC-PERP` — Coinbase + International's actual perpetual format, not a hypothetical — **passes** this grammar, + because `PERP` is a legal quote leg by shape and the grammar cannot know which four-letter + tokens are currencies without a currency table it deliberately does not carry. What stops + `BTC-PERP` is **rail 18**, on `PERP` not being in `settlement_currencies`. So for a + two-segment derivative id, spot-only remains a property of the settlement-currency *list*, + exactly as it was before this rail, and widening that list to a token a venue also uses as + an instrument suffix would reopen it. Pinned by + `test_rail19_does_NOT_close_the_two_segment_derivative_case_rail_18_does`. Closing it + properly needs the venue instrument model (A1/A6) priced above. - **Every mode, both sides, DCA included**, deliberately not in `LIVE_STATE_RAILS`, for rail 18's reason: it needs no broker and no account state, so paper cannot skip it. - **No config field**, unlike rail 18's `settlement_currencies`. Spot-only is this agent's @@ -721,9 +732,33 @@ crypto). Gold and silver are named prohibited in the KB. Do not start here. where it used to write a row the agent would poll and rails 18/19 veto forever. A lowercase id is **rejected with a "did you mean BTC-USD?" hint, never silently uppercased**: a product id is a venue identifier, not free text, and guessing at one is how a typo becomes a - position. `rules seed` loads config unconditionally now (it needs the settlement set); - `--products` on `assets screen` is refused one layer earlier than the screen's own - settlement criterion, which is unchanged and still unit-tested. + position. `rules seed` loads config unconditionally now (it needs the settlement set). + The same grammar's base-leg half (`is_spot_base_code`) is applied to `allowlist` entries at + config load, for `load_config`'s existing `quote_currency`-vs-`settlement_currencies` + reason: `allowlist: [btc]` derives `btc-USD`, which passes nothing and would have surfaced + as a veto on every cycle rather than as an error naming the file. + - **Two callers weigh the two failure kinds differently, and `validate_product_ids` reports + which kind failed so they can.** A SHAPE failure is always a typo and is fatal everywhere. + A SETTLEMENT mismatch is a real product this deployment does not settle in: fatal on `rules + seed`, which WRITES a row the agent polls, and a WARNING on `fetch`/`simulate`, which place + no orders. Making it fatal there broke the screening workflow outright — `assets screen + --products BTC-EUR` is exempt from validation precisely so the pair CAN be asked about, but + the screen's answer is dominated by "0 daily bars < 1460 required", and there was no way to + fetch that history without first widening `settlement_currencies`. The config change had to + be made before the evidence for making it could be gathered. + - **`assets screen` is EXEMPT from `--products` validation** (`validate=False`) — the one + caller that is. Screening is the command that ANSWERS "may keel trade this, and why not", + so a usage error would make the one tool whose job is to explain an inadmissible asset the + one tool that cannot be asked about one. **That exemption obliged the screen to grow the + shape criterion it lacked.** `screen_asset`'s only id-derived criterion was `settlement`, + and settlement reads the LAST segment: `quote_currency_of("BTC-PERP-USD")` is `"USD"`, so + with BTC attested and history cached, `keel assets screen --products BTC-PERP-USD` printed + `ADMIT` — the command whose stated job is answering "may keel trade this" saying YES about + the one product this work exists to refuse. `screen.py` now carries a `spot_instrument` + criterion beside `settlement`, applying `parse_spot_product_id` (rail 19's grammar, + imported rather than restated) to a `product_id` that `MarketFacts` now carries. It flows + through the `_screen_product` chokepoint, so `assets propose` and `assets holdings + --screen` inherit it. Exempting the option is honest only while the answer is right. - **`BrokerCapabilities.asset_classes` hardened, not wired.** `ASSET_CLASSES = {"spot", "futures", "equity"}` with an `__post_init__` rejection of anything else, mirroring the `ORDER_KINDS` check beside it, and two conformance assertions (`asset_classes` non-empty and @@ -748,16 +783,32 @@ crypto). Gold and silver are named prohibited in the KB. Do not start here. - **No config field for asset classes.** See above. - **`OrderIntent` still carries no instrument class**, for the same reason. - **One deliberate correction to this document's own wording.** The bullet above says of the - history walk: *"on history, skip-or-flag the row and keep going."* **Skipping is wrong, and - the shipped behaviour logs at WARNING (`guards.exposure_row_unparseable`) and STILL COUNTS the - row.** `_open_exposure_by_asset` feeds rails 4/5/6, which are **caps**: dropping a row - *reduces* measured exposure and therefore *loosens* all three. That is fail-**open** — a - malformed audit row would buy the agent headroom it has not got. Counting it can only - over-state exposure, which is the closed direction, and an over-stated cap refuses an order a - human can then look at. Tested both ways round - (`test_open_exposure_walk_survives_a_malformed_history_row`, and a companion asserting the row - still trips the exposure cap). + **One deliberate correction to this document's own wording — and then a correction to the + correction.** The bullet above says of the history walk: *"on history, skip-or-flag the row + and keep going."* That is unconditional, and so was the first amendment to it, which said the + row is always counted. **Neither absolute survives a SELL, and the shipped rule is + SIDE-DEPENDENT.** `_open_exposure_by_asset` is a NET figure — BUY adds, SELL subtracts — + feeding rails 4/5/6, which are **caps**. So: + + - a malformed **BUY** is **COUNTED**: it makes the measured figure larger, i.e. the cap + tighter, and an over-stated cap refuses an order a human can then look at; + - a malformed **SELL** is **SKIPPED**: counting it would *subtract* from the bucket its root + is capped by, which *loosens* all three rails. Measured by execution: `ADA-USD` BUY \$900 + gives `{'ADA': 900}`; adding an `ADA-28AUG26-CDE` SELL of \$800 — the same bucket, since + `_asset` reduces both to `ADA` — gives `{'ADA': 100}`, and past the BUY total the trailing + `if amt > 0` filter deletes the bucket outright. A large enough unreadable SELL does not + shrink a cap, it removes it. **And a futures SELL is exactly the row shape this study found + passing every shipped rail**, so this is the case, not a corner of it. + + Both choices are the same choice — never let a row nobody can read make this figure smaller — + and it is the row's SIGN, not the fact of it, that decides which action achieves that. Both + are logged at WARNING (`guards.exposure_row_unparseable`, carrying `action=counted|skipped`), + because seeing a bad row and having it release a cap are different events to whoever is + reading. Tested on both halves: `test_open_exposure_walk_survives_a_malformed_history_row` + plus its exposure-cap companion for the BUY side, + `test_a_malformed_SELL_history_row_is_SKIPPED_not_counted` and + `test_a_malformed_SELL_cannot_zero_out_a_bucket_and_release_the_concentration_cap` for the + SELL side, which the original tests seeded no rows for and so never exercised. **`_asset` was made total, not strict** — the second correction, and the smaller one. The bullet asks for a *violation* on a non-`BASE-QUOTE` id; the violation is rail 19's, on the diff --git a/keel/cli.py b/keel/cli.py index 413bc822..6816c0fe 100644 --- a/keel/cli.py +++ b/keel/cli.py @@ -514,6 +514,9 @@ def _market_facts(repo: Repository, product: str, quote: str) -> screen_mod.Mark # `-USD`, so it always fell through to "do we have bars", which the history criterion # already covers. One of four admission criteria was doing nothing. quotable_in_settlement_currency=quote_currency_of(product) == quote.upper(), + # Carried, not reduced: `screen_asset` applies rail 19's grammar to it, so the screen's + # shape verdict and the rail's cannot disagree, and the verdict can name the id. + product_id=product, ) @@ -775,9 +778,17 @@ def assets_screen(ctx: click.Context, products: str | None) -> None: config = _load_cfg(ctx) repo = _open_repo(ctx) # Deliberately UNVALIDATED, unlike every other `--products` caller: screening is the command - # that answers "may keel trade this", and `screen_asset` rejects a cross-settled or malformed - # id with a reason. Validating here would turn that answer into a usage error. - product_list = parse_products_option(products, config, validate=False) + # that answers "may keel trade this, and why not", and `screen_asset` has BOTH id-derived + # criteria of its own -- `settlement` (the quote leg vs `config.quote_currency`) and + # `spot_instrument` (the whole id vs rail 19's spot grammar) -- so it REJECTs a cross-settled + # or derivative-shaped id with the same reason a usage error would have carried, plus the + # history/liquidity/attestation verdicts a usage error would have suppressed. Validating + # here would replace that reasoned answer with a refusal to answer. + # + # That exemption is only honest because those criteria are real. Settlement alone was not + # enough: `quote_currency_of("BTC-PERP-USD")` is `"USD"`, so before the `spot_instrument` + # criterion this command ADMITted the one product shape rail 19 exists to veto. + product_list, _ = parse_products_option(products, config, validate=False) admitted = 0 for product in product_list: @@ -1258,16 +1269,31 @@ def pnl(ctx: click.Context, asset: str | None, raw_marks: tuple[str, ...]) -> No def _parse_products_option(products: str | None, config: Config) -> list[str]: - """`--products` for `fetch`/`screen`/`monitor`/`simulate`, refused here if keel cannot trade it. + """`--products` for `fetch`/`simulate`: a malformed id is refused, a cross-settled one warns. The parse itself lives in `commands._products` so `rules seed` uses the same one. This - wrapper exists only to turn its `ValueError` into a `click.BadParameter`, i.e. a usage error - naming the offending ids rather than a traceback (feasibility study R2). + wrapper turns its `ValueError` into a `click.BadParameter` -- a usage error naming the + offending ids rather than a traceback -- and prints the non-fatal reasons it hands back + (feasibility study R2). + + `settlement_is_fatal=False` is the difference from `rules seed`, and it is about what each + command WRITES. Seeding a rule for a product rail 18 vetoes puts a row in the table that the + agent polls forever; fetching its candles puts market data in the cache, which is exactly + what an operator needs before `assets screen` can tell them anything about the asset. See + `parse_products_option`. `monitor` is deliberately absent from this list: it has no + `--products` option and polls `_default_sim_products` directly. """ try: - return parse_products_option(products, config) + product_list, warnings = parse_products_option( + products, config, settlement_is_fatal=False + ) except ValueError as exc: raise click.BadParameter(str(exc), param_hint="--products") from exc + for warning in warnings: + # Loud, and on stderr-adjacent footing with the other ⚠️ notices: this run is legitimate, + # but no ORDER for such a product ever will be under the current config. + click.echo(f"⚠️ {warning}\n Fetching/simulating it is fine; trading it is not.") + return product_list def _sim_asset(product_id: str) -> str: diff --git a/keel/commands/_products.py b/keel/commands/_products.py index 59343aea..4e5e33c6 100644 --- a/keel/commands/_products.py +++ b/keel/commands/_products.py @@ -8,14 +8,29 @@ The same module owns the check applied to an id the operator TYPES (`--products`), for the same reason: a derivation and a validation that disagree about what a product id is would let the CLI refuse an id keel itself constructs, or accept one it cannot trade. + +Nothing here imports `click`. The checks raise `ValueError` or return data; turning either into a +usage error is the calling command's job, so this module stays testable without a CLI runner and +usable from anywhere. """ from __future__ import annotations +from dataclasses import dataclass + from keel_core.products import parse_spot_product_id, quote_currency_of from keel.config import Config +#: A `--products` id that is not `BASE-QUOTE` at all. **Always a typo**, for every caller: no +#: config edit rescues it, and no command has a legitimate use for one. +SHAPE = "shape" + +#: A well-formed spot pair whose quote leg is not in this deployment's `settlement_currencies`. +#: A real product, and a real answer -- `BTC-EUR` is listed on Coinbase. Rail 18 vetoes an ORDER +#: for it; a command that places none is entitled to weigh it as a warning. +SETTLEMENT = "settlement" + def _history_product(asset: str, quote: str) -> str: """The product id for an asset, in the deployment's settlement currency. @@ -38,8 +53,26 @@ def _default_sim_products(config: Config) -> list[str]: return [_history_product(asset, config.quote_currency) for asset in config.allowlist] -def validate_product_ids(ids: list[str], settlement_currencies: frozenset[str]) -> list[str]: - """Return `ids` unchanged, or raise `ValueError` naming every id keel could not trade. +@dataclass(frozen=True) +class ProductIdProblem: + """One reason one typed `--products` id is unusable, tagged with WHICH question it failed. + + The tag is the point. Both questions are worth asking wherever an operator types an id, but + they do not carry the same consequence -- see `SHAPE` and `SETTLEMENT` above -- and a caller + that could only string-match the message would have to grep prose to weigh them. `reason` is + a complete, self-contained sentence naming `product_id`, so a caller reporting a subset of + the problems still reports each of them in full. + """ + + product_id: str + kind: str + reason: str + + +def check_product_ids( + ids: list[str], settlement_currencies: frozenset[str] +) -> list[ProductIdProblem]: + """Every reason keel could not trade one of `ids`, in order. Raises nothing; decides nothing. The two questions here are the two the hard rails ask, deliberately and in the same order: @@ -54,17 +87,22 @@ def validate_product_ids(ids: list[str], settlement_currencies: frozenset[str]) exactly as they are: this is an ergonomics check standing in front of them, never a replacement for them, and it runs on the operator's list only. Nothing reads it at order time. - ⚠️ **A lowercase id is REJECTED, with a hint -- never silently uppercased.** `quote_currency_of` - case-folds because it is identifying the currency of an id that already exists; accepting - `btc-USD` here would mean the id the operator typed is not the id keel goes on to trade, and - a product id is a venue identifier, not free text. Guessing at one is how a typo becomes a - position. The hint costs a line and leaves the operator holding the fix. + **Shape is asked first and stops there**, per id, and that ordering is load-bearing rather + than incidental: `quote_currency_of("XLM-28AUG26-CDE")` is `"CDE"`, which would otherwise be + reported as "settles in CDE, not in your settlement_currencies" -- an accurate sentence that + invites the operator to widen `settlement_currencies` until a futures contract is admitted. + A malformed id has exactly one true diagnosis. - Reports EVERY bad id in one message. An operator fixing a list one error per invocation - learns it slowly and abandons it fast. Raises `ValueError` and nothing else, so callers can - wrap it in `click.BadParameter` and get a usage error rather than a traceback. + ⚠️ **A lowercase id is REPORTED, with a hint -- never silently uppercased.** + `quote_currency_of` case-folds because it is identifying the currency of an id that already + exists; accepting `btc-USD` here would mean the id the operator typed is not the id keel goes + on to trade, and a product id is a venue identifier, not free text. Guessing at one is how a + typo becomes a position. The hint costs a line and leaves the operator holding the fix. + + EVERY bad id is reported, not just the first. An operator fixing a list one error per + invocation learns it slowly and abandons it fast. """ - reasons: list[str] = [] + problems: list[ProductIdProblem] = [] for product_id in ids: if parse_spot_product_id(product_id) is None: # The hint fires only when case is the ONLY thing wrong, so it can never suggest an @@ -72,57 +110,113 @@ def validate_product_ids(ids: list[str], settlement_currencies: frozenset[str]) hint = "" if isinstance(product_id, str) and parse_spot_product_id(product_id.upper()): hint = f" -- did you mean {product_id.upper()}?" - reasons.append( - f"{product_id!r} is not a spot product id (expected BASE-QUOTE, uppercase, " - f"exactly one hyphen; keel is spot-only, so futures BASE-DDMMMYY-CDE and equity " - f"hashes are refused){hint}" + problems.append( + ProductIdProblem( + product_id=str(product_id), + kind=SHAPE, + reason=( + f"{product_id!r} is not a spot product id (expected BASE-QUOTE, " + f"uppercase, exactly one hyphen; keel is spot-only, so futures " + f"BASE-DDMMMYY-CDE and equity hashes are refused){hint}" + ), + ) ) continue settlement = quote_currency_of(product_id) if settlement not in settlement_currencies: - reasons.append( - f"{product_id!r} settles in {settlement}, which is not one of this deployment's " - f"settlement_currencies {sorted(settlement_currencies)} -- rail 18 would veto " - f"every order for it" + problems.append( + ProductIdProblem( + product_id=product_id, + kind=SETTLEMENT, + reason=( + f"{product_id!r} settles in {settlement}, which is not one of this " + f"deployment's settlement_currencies " + f"{sorted(settlement_currencies)} -- rail 18 would veto every order " + f"for it" + ), + ) ) - if reasons: - raise ValueError( - "unusable product id(s):\n" + "\n".join(f" - {reason}" for reason in reasons) - ) + return problems + + +def _unusable_message(problems: list[ProductIdProblem]) -> str: + return "unusable product id(s):\n" + "\n".join(f" - {p.reason}" for p in problems) + + +def validate_product_ids(ids: list[str], settlement_currencies: frozenset[str]) -> list[str]: + """Return `ids` unchanged, or raise `ValueError` naming every id keel could not trade. + + BOTH failure kinds are fatal here. This is the check for a command that WRITES something the + agent will act on -- `rules seed` -- where a rule the rails veto forever is not a lesser + problem than a typo, only a quieter one. `parse_products_option(settlement_is_fatal=False)` + is the softer variant, and it exists for commands that write no such thing. + + Raises `ValueError` and nothing else, so callers can wrap it in `click.BadParameter` and get + a usage error rather than a traceback. + """ + problems = check_product_ids(ids, settlement_currencies) + if problems: + raise ValueError(_unusable_message(problems)) return ids def parse_products_option( - products: str | None, config: Config, *, validate: bool = True -) -> list[str]: - """A `--products` option value as a validated product list; the allowlist when it is absent. - - The one parse of that option, shared by `fetch`/`monitor`/`simulate` (via - `cli._parse_products_option`) and `rules seed`, which used to split it inline and so could - not have been given this check without growing a second copy of the derivation. + products: str | None, + config: Config, + *, + validate: bool = True, + settlement_is_fatal: bool = True, +) -> tuple[list[str], list[str]]: + """A `--products` option value as `(product_ids, warnings)`; the allowlist when it is absent. + + The one parse of that option, shared by `fetch`/`simulate` (via `cli._parse_products_option`) + and `rules seed`, which used to split it inline and so could not have been given this check + without growing a second copy of the derivation. (`monitor` takes no `--products` at all -- + it polls `_default_sim_products` directly.) + + `warnings` are non-fatal problem reasons the caller must SHOW; returning them rather than + printing them is what keeps this module free of `click`. It is empty unless + `settlement_is_fatal=False`. + + **`settlement_is_fatal=False` is for `fetch` and `simulate`.** A `SHAPE` failure stays fatal + there -- it is always a typo -- but a `SETTLEMENT` mismatch becomes a warning, because + treating it as a usage error broke the screening workflow outright: `assets screen + --products BTC-EUR` is exempt from validation precisely so an operator CAN ask about a + cross-settled pair, but the screen's answer is dominated by "0 daily bars < 1460 required", + and there was then no way to fetch that history without first widening + `settlement_currencies`. The config change had to be made before the evidence for making it + could be gathered. Neither command places an order, so neither needs rail 18 standing in + front of it -- which is this branch's own stated reason for not validating the derived + default, applied consistently. `validate=False` is for `assets screen` ALONE, and is not a convenience. Screening is the - diagnostic that ANSWERS "may keel trade this, and why not" -- `screen_asset` has a settlement - criterion of its own and reports `REJECT` with a reason. Refusing the id at the option would - replace that reasoned verdict with a usage error, i.e. the one command whose entire job is to - explain an inadmissible asset would become the one command that cannot be asked about one. - Screening writes nothing and orders nothing; rails 18/19 stop the id if it ever reaches an - order by another route. - - Raises `ValueError` listing every unusable id. Callers are CLI commands and wrap it in + diagnostic that ANSWERS "may keel trade this, and why not", and `screen_asset` has both a + settlement AND a spot-shape criterion of its own (`keel/compliance/screen.py`), so it + reports `REJECT` with the same reason a usage error would have carried -- plus the history, + liquidity and attestation verdicts a usage error would have suppressed. Refusing the id at + the option would make the one command whose entire job is to explain an inadmissible asset + the one command that cannot be asked about one. Screening writes nothing and orders nothing; + rails 18/19 stop the id if it ever reaches an order by another route. + + Raises `ValueError` listing every fatal id. Callers are CLI commands and wrap it in `click.BadParameter`, so the operator gets a usage error rather than a traceback. ⚠️ Validation applies to what the operator TYPED, not to the allowlist-derived default. The - default is `_history_product`'s output over `config.allowlist`, and its shape is config's - question, checked once at load (`load_config` already refuses a `quote_currency` outside - `settlement_currencies` for exactly this reason). Validating it here would mean `keel fetch` - -- which places no orders and needs no rail -- started refusing configs it has always - accepted, for a defect the trading path already reports. Rails 18/19 remain the backstop for - an id that reaches an order by any route, typed or derived. + default is `_history_product`'s output over `config.allowlist`, and both of its legs are + config's question, answered at load: `_parse_allowlist` shape-checks each asset against the + base-leg half of the spot grammar, and `load_config` refuses a `quote_currency` outside + `settlement_currencies`. So a derived id that fails either question here is a config bug that + `load_config` should have caught, and the fix belongs there -- where the message can name the + file -- not in a per-command re-check. Rails 18/19 remain the backstop for an id that reaches + an order by any route, typed or derived. """ if not products: - return _default_sim_products(config) + return _default_sim_products(config), [] ids = [p.strip() for p in products.split(",") if p.strip()] if not validate: - return ids - return validate_product_ids(ids, config.settlement_currencies) + return ids, [] + problems = check_product_ids(ids, config.settlement_currencies) + fatal = [p for p in problems if settlement_is_fatal or p.kind != SETTLEMENT] + if fatal: + raise ValueError(_unusable_message(fatal)) + return ids, [p.reason for p in problems] diff --git a/keel/commands/rules.py b/keel/commands/rules.py index 209e0282..879aaaf3 100644 --- a/keel/commands/rules.py +++ b/keel/commands/rules.py @@ -295,7 +295,11 @@ def rules_seed( # keyboard, with the reason, instead of in a log line nobody is reading. config = _load_cfg(ctx) try: - product_list = parse_products_option(products, config) + # `settlement_is_fatal` stays at its default here, unlike `fetch`/`simulate`: this + # command WRITES a row the agent then polls every cycle, so a rule the rails veto + # forever is not a lesser problem than a typo, only a quieter one. Nothing is warned + # about and admitted; hence no warnings to print. + product_list, _ = parse_products_option(products, config) except ValueError as exc: raise click.BadParameter(str(exc), param_hint="--products") from exc diff --git a/keel/compliance/screen.py b/keel/compliance/screen.py index eb8762ea..ec4d5388 100644 --- a/keel/compliance/screen.py +++ b/keel/compliance/screen.py @@ -26,6 +26,8 @@ from dataclasses import dataclass, field from decimal import Decimal +from keel_core.products import parse_spot_product_id + #: §28.4's haram business lines, plus the crypto-specific readings it names. HARAM_SECTORS = frozenset( { @@ -88,6 +90,16 @@ class MarketFacts: daily_bars: int median_daily_volume: Decimal quotable_in_settlement_currency: bool + #: The venue id the other facts were gathered for. Carried rather than reduced to a bool so + #: `screen_asset` can apply `parse_spot_product_id` -- rail 19's own grammar, one copy -- and + #: so its verdict can NAME the id. `asset` is that id's base leg and cannot answer the shape + #: question: `BTC-PERP-USD` and `BTC-USD` have the same `asset`. + #: + #: Deliberately has NO default. A default would have to be some id, and any id that parses + #: is a fail-OPEN default for a criterion whose whole job is refusing one that does not -- + #: so a construction site that forgets it must fail loudly at the call, not quietly at the + #: verdict. + product_id: str @dataclass(frozen=True) @@ -169,6 +181,31 @@ def screen_asset( "settlement: not quotable in the configured settlement currency -- a cross would " "add a second exchange leg, and §65.7 requires each leg be priced and settled" ) + # The SHAPE criterion, and rail 19's question asked one gate earlier (feasibility study R2). + # Settlement used to be this screen's ONLY id-derived criterion, and settlement reads the + # LAST segment: `quote_currency_of("BTC-PERP-USD")` is `"USD"`, so a derivative-shaped id + # with a legitimate final segment passed it. `assets screen` -- the command that ANSWERS + # "may keel trade this, and why not", and the one `--products` caller deliberately exempt + # from option validation so that it can report rather than refuse -- therefore said ADMIT + # about the one product shape rail 19 exists to veto. The exemption is only honest if the + # answer is right. + # + # NO `ScreenPolicy` knob, unlike `require_settlement_quote` beside it, and for rail 19's + # reason: settlement currencies are an operator preference with a real escape hatch + # (`config.settlement_currencies`), whereas spot-only is this agent's charter. A knob whose + # only safe value is its default is a liability. + # + # `parse_spot_product_id` is rail 19's own grammar, imported rather than restated, so the + # screen and the rail cannot drift into disagreeing about what a spot id is -- an id this + # gate admits and that rail then vetoes forever is the worst answer either could give. + if parse_spot_product_id(facts.product_id) is None: + failures.append( + f"spot_instrument: {facts.product_id!r} is not a well-formed spot product id " + "(BASE-QUOTE, uppercase, exactly one hyphen). keel is spot-only, so futures " + "(BASE-DDMMMYY-CDE), equities (an opaque 64-char hash) and any other instrument " + "shape are refused regardless of what they settle in -- rail 19 would veto every " + "order for it" + ) # -- attested shariah classification --------------------------------------- if attestation is None: diff --git a/keel/execution/guards.py b/keel/execution/guards.py index 10b6fd3f..e91bf9d2 100644 --- a/keel/execution/guards.py +++ b/keel/execution/guards.py @@ -87,10 +87,11 @@ not redundant. `quote_currency_of("BTC-PERP-USD")` is `"USD"` -- a configured settlement currency -- and `_asset` reduces it to the allowlisted `"BTC"`, so a derivative-shaped id whose final segment is legitimate passes rails 1 AND 18 and is stopped only here. Rail 18 catches the classes -Coinbase lists TODAY on their settlement legs; rail 19 makes spot-only structural rather than a -property of which suffixes the venue currently happens to use. Every mode, both sides, DCA -included, and no config field to widen -- spot-only is this agent's charter, not an operator -preference. +Coinbase lists TODAY on their settlement legs; rail 19 makes spot-only structural for ids of THREE +OR MORE segments. It does not for two-segment ones -- `BTC-PERP` passes this grammar and is +stopped by rail 18 alone -- so there spot-only remains a property of `settlement_currencies`. See +the rail's own comment for that residual in full. Every mode, both sides, DCA included, and no +config field to widen -- spot-only is this agent's charter, not an operator preference. """ from __future__ import annotations @@ -227,36 +228,59 @@ def _order_notional(order: dict[str, Any]) -> Decimal: def _open_exposure_by_asset(repo: Repository) -> dict[str, Decimal]: """Net at-risk notional per asset from filled live orders (BUY adds, SELL reduces). - ⚠️ A row whose `product_id` is not a well-formed spot id is LOGGED AND STILL COUNTED, under - whatever key `_asset` gives it -- never skipped. A deliberate correction to the feasibility - study's own "skip-or-flag the row and keep going" wording (R2), and the direction is the - whole argument: this figure feeds rails 4/5/6, which are CAPS, so dropping a row REDUCES - measured exposure and LOOSENS every one of them. That is fail-OPEN -- a malformed audit row - would buy the agent headroom it has not got. Counting it can only over-state exposure, which - is the closed direction, and an over-stated cap refuses an order a human can then look at. - The WARNING is how the operator finds out; it is not a substitute for counting the money. + ⚠️ **An unparseable `product_id` is handled by SIDE, and always logged at WARNING.** A + malformed **BUY** is COUNTED, under whatever key `_asset` gives it; a malformed **SELL** is + SKIPPED. Both choices are the same choice -- never let a row nobody can read make this + figure SMALLER -- and it is the sign of the row, not the fact of the row, that decides which + action achieves that. + + The arithmetic is why. This is a NET figure feeding rails 4/5/6, which are CAPS: + + - **A counted BUY adds.** A larger measured exposure is a TIGHTER cap. Over-stating refuses + an order a human can then look at: the closed direction. + - **A counted SELL subtracts**, so counting one is the fail-OPEN move -- it buys the agent + headroom it has not got. `ADA-USD` BUY $900 measures `{'ADA': 900}`; add an + `ADA-28AUG26-CDE` SELL $800 (`_asset` -> the same `ADA` bucket) and counting it measures + $100. Past the BUY total the trailing `if amt > 0` filter drops the bucket entirely, so a + large enough unreadable SELL does not merely shrink a cap, it deletes it. Skipping is what + keeps the bucket at the figure the rows we CAN read support. + + This supersedes the feasibility study's "skip-or-flag the row and keep going" (R2), which + was unconditional, and the first correction of it, which was unconditionally the other way. + Neither absolute is right, because neither survives a SELL. + + A row with a side that is neither takes the SELL branch -- it contributes nothing either + way, and reporting it as skipped is the honest description of that. Such a row should be impossible going forward -- rail 19 vetoes the intent before it can be written, and the live `orders` table held zero rows when rail 19 shipped -- but "impossible" - is what the study said about a futures SELL passing every rail. + is what the study said about a futures SELL passing every rail, and a futures SELL is + exactly the shape this branch exists for. """ exposure: dict[str, Decimal] = {} for order in repo.get_orders(mode="live", status="filled"): product_id = order["product_id"] + side = order["side"] if parse_spot_product_id(product_id) is None: + # `action` is in the log line because "we saw a bad row" and "we let it release a + # cap" are different events to the operator reading this at 3am. + counted = side == Side.BUY.value log_event( logger, logging.WARNING, "guards.exposure_row_unparseable", product=str(product_id), order_id=order.get("id"), - side=order.get("side"), + side=side, + action="counted" if counted else "skipped", ) + if not counted: + continue asset = _asset(product_id) amount = _order_notional(order) - if order["side"] == Side.BUY.value: + if side == Side.BUY.value: exposure[asset] = exposure.get(asset, Decimal("0")) + amount - elif order["side"] == Side.SELL.value: + elif side == Side.SELL.value: exposure[asset] = exposure.get(asset, Decimal("0")) - amount return {asset: amt for asset, amt in exposure.items() if amt > 0} @@ -696,9 +720,19 @@ def check( # defences. `quote_currency_of("BTC-PERP-USD")` is `"USD"` -- configured -- so rail 18 # admits it, and `_asset` reduces it to the allowlisted `"BTC"`, so rail 1 admits it too. # Only the shape stops it. Coinbase lists no such product today; rail 18 catches the - # classes that DO exist (`CDE` futures, equity hashes) on their settlement legs. This - # rail is what makes spot-only structural rather than a property of which suffixes the - # venue currently happens to use. + # classes that DO exist (`CDE` futures, equity hashes) on their settlement legs. + # + # ⚠️ **THE RESIDUAL THIS DOES NOT CLOSE, stated plainly.** This rail makes spot-only + # structural for ids of three or more segments; it does not for TWO-segment ones. + # `BTC-PERP` -- Coinbase International's actual perpetual-futures format, not a + # hypothetical -- PASSES this grammar: `PERP` is a legal quote leg by shape, since the + # grammar cannot know which four-letter tokens are currencies without carrying a + # currency table it deliberately does not carry. What stops `BTC-PERP` is rail 18, on + # `PERP` not being in `settlement_currencies`. So for a two-segment derivative id, + # spot-only remains a property of the settlement-currency LIST, exactly as it was before + # this rail. Widening that list to a token a venue also uses as an instrument suffix + # would reopen it. Closing this properly needs a venue instrument model (A1/A6), which + # is priced in the feasibility study and is not what this rail is. # # BOTH SIDES, EVERY MODE, DCA INCLUDED — deliberately not in `LIVE_STATE_RAILS`, for # rail 18's reason: it needs no broker and no account state, so paper cannot skip it, and diff --git a/packages/keel-broker-api/keel_broker_api/capabilities.py b/packages/keel-broker-api/keel_broker_api/capabilities.py index ef5f880c..f8964cbe 100644 --- a/packages/keel-broker-api/keel_broker_api/capabilities.py +++ b/packages/keel-broker-api/keel_broker_api/capabilities.py @@ -6,12 +6,18 @@ from keel_broker_api.orders import ORDER_KINDS -#: The instrument classes an adapter may declare -- the three the 2026-08-05 Coinbase -#: asset-class study enumerated at the venue (`SPOT`, `FUTURE`, `EQUITY`; -#: `docs/experiments/2026-08-05-coinbase-asset-class-feasibility.md`). A closed vocabulary for -#: the same reason `ORDER_KINDS` is one: a declaration checked against nothing is a comment with -#: a type annotation, and the near-misses (`SPOT`, `future`, `perp`) are exactly the values that -#: would sit in a set gating nothing. +#: The instrument classes an adapter may declare: one KEEL-side name per class the 2026-08-05 +#: Coinbase asset-class study found at the venue +#: (`docs/experiments/2026-08-05-coinbase-asset-class-feasibility.md`). +#: +#: ⚠️ These are **keel's spellings, not the venue's.** Coinbase's `product_type` field reads +#: `SPOT` / `FUTURE` / `EQUITY`; this vocabulary is lowercase, and plural for futures. That is +#: deliberate -- an adapter declares what it can do in the port's words, so a second venue with +#: its own casing has one obvious answer rather than a choice -- and it is why the venue's own +#: spellings are REFUSED here rather than accepted as synonyms. `SPOT` and `future` are near +#: misses that a `frozenset` would otherwise carry silently into a set gating nothing, so +#: `__post_init__` rejects them by name, exactly as `ORDER_KINDS` does. An adapter author who +#: pastes the venue's value gets an error at construction naming what they pasted. ASSET_CLASSES: frozenset[str] = frozenset({"spot", "futures", "equity"}) diff --git a/packages/keel-broker-api/keel_broker_api/conformance/suite.py b/packages/keel-broker-api/keel_broker_api/conformance/suite.py index bb3a88f4..e4cce3fc 100644 --- a/packages/keel-broker-api/keel_broker_api/conformance/suite.py +++ b/packages/keel-broker-api/keel_broker_api/conformance/suite.py @@ -98,9 +98,17 @@ def test_asset_classes_is_non_empty_and_drawn_from_the_known_vocabulary(self) -> assert caps.asset_classes <= ASSET_CLASSES def test_quote_currencies_is_non_empty(self) -> None: - """Rail 18's default settlement set is derived FROM this declaration. An adapter that - declared none would be saying it settles in nothing, which cannot be true of a venue - that accepts orders.""" + """Kept alongside the `asset_classes` check above because rail 18 is the other half of + the same question, and this is the declaration it is checked against. + + `config.DEFAULT_SETTLEMENT_CURRENCIES` is `{"USD", "USDC"}` because that is what + `keel_broker_coinbase`'s `_CAPABILITIES.quote_currencies` says the venue settles in -- + neither is derived from the other (see the comment on that constant), and an agreement + between two independent statements is only meaningful while both actually state + something. An adapter declaring none would be saying it settles in nothing, which cannot + be true of a venue that accepts orders, and would make that agreement vacuous rather + than false -- the failure mode nothing else here would catch. + """ assert self.broker().capabilities().quote_currencies # --- capabilities cannot lie about orders --------------------------------------------- diff --git a/packages/keel-core/keel_core/config.py b/packages/keel-core/keel_core/config.py index bb408a55..85aae07b 100644 --- a/packages/keel-core/keel_core/config.py +++ b/packages/keel-core/keel_core/config.py @@ -17,6 +17,7 @@ import yaml from dotenv import dotenv_values +from keel_core.products import is_spot_base_code from keel_core.types import Granularity @@ -339,12 +340,49 @@ def _parse_auto_trade_mode(raw: dict[str, Any]) -> str: def _parse_allowlist(raw: dict[str, Any]) -> list[str]: + """`allowlist:` -- the BASE legs every product id keel constructs is built from. + + Entries are shape-checked against `is_spot_base_code`, the base-leg half of the spot grammar + rail 19 gates on, for the reason `_parse_settlement_currencies` shape-checks currency codes + and the reason `quote_currency` is cross-checked against `settlement_currencies`: an entry + this function admits and the rails then veto is a **silent, unfixable rejection**. An + allowlist asset only ever reaches the venue through `_history_product`'s + `f"{asset}-{quote_currency}"`, so `allowlist: [btc]` derives `btc-USD` -- which rail 19 + vetoes on every cycle and rail 1 never even reaches. The operator's first signal would be a + veto on an asset they believed they had just enabled. + + ⚠️ **REJECTED, never uppercased** -- deliberately the opposite of `settlement_currencies` + beside it, and the difference is what the value is FOR. A settlement code is *compared* + against `quote_currency_of`'s already-uppercased output, so folding it makes the two sides + agree by construction and changes nothing an operator can observe. An allowlist entry is + *concatenated* into a venue identifier. Folding it would mean the asset keel goes on to + trade is not the asset the file names, which is the same judgement + `commands._products.validate_product_ids` makes about a typed `--products` id: a venue + identifier is not free text, and guessing at one is how a typo becomes a position. The + message carries the uppercase form, so the fix costs the operator one keystroke and stays + theirs. (Blast radius nil: every shipped config -- live, paper-forward, sandbox, local -- + already lists uppercase tickers.) + + Reports the FIRST bad entry rather than all of them, matching the other `_parse_*` helpers + here; the whole-list report belongs to `validate_product_ids`, where an operator is typing a + list at a prompt rather than editing a file they can re-read. + """ allowlist = raw.get("allowlist") if not allowlist or not isinstance(allowlist, list): raise ConfigError("allowlist: missing or empty; must be a non-empty list of asset codes") for entry in allowlist: if not isinstance(entry, str) or not entry: raise ConfigError(f"allowlist: invalid entry {entry!r}; must be non-empty strings") + if not is_spot_base_code(entry): + # The hint fires only when case is the ONLY thing wrong, so it can never propose an + # entry that is itself inadmissible -- `btc-usd` gets the refusal, not advice. + hint = f" -- did you mean {entry.upper()}?" if is_spot_base_code(entry.upper()) else "" + raise ConfigError( + f"allowlist: invalid entry {entry!r}; an asset code is 1-16 UPPERCASE " + f"alphanumeric characters (BTC, PAXG, 1INCH) -- it is the base leg of the " + f"product id keel builds as '-{{quote_currency}}', so anything else " + f"derives an id rail 19 would veto on every cycle{hint}" + ) return list(allowlist) diff --git a/packages/keel-core/keel_core/products.py b/packages/keel-core/keel_core/products.py index 245f3df8..e5c03ea4 100644 --- a/packages/keel-core/keel_core/products.py +++ b/packages/keel-core/keel_core/products.py @@ -37,7 +37,13 @@ # `_CURRENCY_CODE_RE`, so a settlement currency that regex admits and this grammar rejects (or # the reverse) cannot exist. A well-formed spot id no `settlement_currencies` set could ever # name would be vetoed by rail 18 forever with nothing saying why. -_SPOT_PRODUCT_ID_RE = re.compile(r"[A-Z0-9]{1,16}-[A-Z0-9]{2,10}") +# +# The base leg is split out rather than inlined because `config._parse_allowlist` checks against +# it: `allowlist` holds BASE legs, which `_history_product` concatenates into ids this grammar +# then judges. Two copies of the base grammar could disagree, and the config side losing that +# disagreement means an allowlist entry that loads cleanly and is vetoed on every cycle. +_SPOT_BASE_CODE_RE = re.compile(r"[A-Z0-9]{1,16}") +_SPOT_PRODUCT_ID_RE = re.compile(_SPOT_BASE_CODE_RE.pattern + r"-[A-Z0-9]{2,10}") def quote_currency_of(product_id: str | None) -> str | None: @@ -56,6 +62,27 @@ def quote_currency_of(product_id: str | None) -> str | None: return quote.strip().upper() +def is_spot_base_code(code: object) -> bool: + """Whether `code` is a well-formed BASE leg -- the half of the spot grammar before the hyphen. + + `"BTC"` -> `True`; `"btc"`, `"BTC-USD"`, `"BT C"`, `""` and non-strings -> `False`. + + The question `config._parse_allowlist` asks. An allowlist entry is not a product id: it is + the base leg `_history_product` concatenates a settlement currency onto, so the whole-id + grammar would reject every legitimate entry. Asking the base-leg half of the SAME grammar is + what makes `asset in allowlist` imply `parse_spot_product_id(f"{asset}-{quote}")` for any + quote the currency regex admits -- i.e. what stops config from admitting an asset rail 19 + will veto forever. + + **No normalisation, and no case-folding**, for `parse_spot_product_id`'s reason: this decides + whether an id keel is about to CONSTRUCT will be well formed, and folding the input would + mean the asset keel trades is not the asset the config file names. + + **Total by contract.** Never raises, on any input -- hence `object`. + """ + return isinstance(code, str) and _SPOT_BASE_CODE_RE.fullmatch(code) is not None + + def parse_spot_product_id(product_id: object) -> tuple[str, str] | None: """`(base, quote)` if `product_id` is a well-formed SPOT id, else `None`. diff --git a/tests/broker_api/test_capabilities.py b/tests/broker_api/test_capabilities.py index 6d68e52e..5f1326ed 100644 --- a/tests/broker_api/test_capabilities.py +++ b/tests/broker_api/test_capabilities.py @@ -27,8 +27,9 @@ def _caps(**overrides: object) -> BrokerCapabilities: def test_the_vocabulary_is_the_three_classes_the_venue_study_enumerated() -> None: - """`SPOT`, `FUTURE` and `EQUITY` are what Coinbase lists. A set that drifts from the - vocabulary adapters declare against is how a typo becomes a silently permissive gate.""" + """One keel-side name per class Coinbase lists, in the PORT's spelling rather than the + venue's (`SPOT`/`FUTURE`/`EQUITY`). A set that drifts from the vocabulary adapters declare + against is how a typo becomes a silently permissive gate.""" assert ASSET_CLASSES == frozenset({"spot", "futures", "equity"}) @@ -39,7 +40,13 @@ def test_a_declaration_of_spot_is_accepted() -> None: @pytest.mark.parametrize("bogus", ["margin_spot", "SPOT", "perp", "", "future"]) def test_an_unknown_asset_class_is_refused_at_construction(bogus: str) -> None: """Mirrors the `supported_orders` check: an adapter cannot invent a class the engine has no - vocabulary for. `SPOT` and `future` are the near-misses that would otherwise pass silently.""" + vocabulary for. + + `SPOT` and `future` are the near-misses that matter most, and they are refused *because* + they are Coinbase's own spellings of `product_type`: an adapter author pasting the venue's + value is the likeliest way a set that gates nothing gets written, so it has to fail at + construction naming what was pasted rather than sit there looking declared. + """ with pytest.raises(ValueError) as excinfo: _caps(asset_classes=frozenset({bogus})) assert bogus in str(excinfo.value) diff --git a/tests/commands/test_products.py b/tests/commands/test_products.py index 46c50b67..7f562476 100644 --- a/tests/commands/test_products.py +++ b/tests/commands/test_products.py @@ -1,4 +1,4 @@ -"""`keel.commands._products.validate_product_ids` -- rejecting a bad id at the KEYBOARD. +"""`keel.commands._products` -- rejecting (or flagging) a bad `--products` id at the KEYBOARD. Rails 18 and 19 stop an inadmissible product where the agent trades it, which is the right place for a safety rail and the wrong place for a typo. `keel rules seed --products XLM-28AUG26-CDE @@ -12,7 +12,12 @@ import pytest -from keel.commands._products import validate_product_ids +from keel.commands._products import ( + SETTLEMENT, + SHAPE, + check_product_ids, + validate_product_ids, +) _SETTLEMENT = frozenset({"USD", "USDC"}) @@ -97,3 +102,53 @@ def test_it_never_raises_anything_but_ValueError(): for weird in ([None], [42], [""], [" "], [b"BTC-USD"]): with pytest.raises(ValueError): validate_product_ids(weird, _SETTLEMENT) + + +# -- the two failure KINDS are distinguishable, so callers can weigh them differently ---------- +# +# Feasibility study R2, corrected. Both questions are worth asking wherever an operator types an +# id, but they do not carry the same consequence, and a caller that can only string-match the +# message cannot act on the difference: +# +# SHAPE -- `BASE-QUOTE` or not. Always a typo. There is no config edit that rescues it, +# and no command for which it is a legitimate request. +# SETTLEMENT -- a real spot pair whose quote leg this deployment does not settle in. Rail 18 +# vetoes an ORDER for it; `keel fetch` places none, and needs the history before +# `assets screen` can say anything about the asset at all. + + +def test_the_two_failure_kinds_are_reported_separately(): + problems = check_product_ids(["BTC-USD", "XLM-28AUG26-CDE", "BTC-EUR"], _SETTLEMENT) + assert [(p.product_id, p.kind) for p in problems] == [ + ("XLM-28AUG26-CDE", SHAPE), + ("BTC-EUR", SETTLEMENT), + ] + + +def test_a_clean_list_has_no_problems(): + assert check_product_ids(["BTC-USD", "BTC-USDC"], _SETTLEMENT) == [] + + +def test_shape_is_asked_FIRST_so_a_malformed_id_is_never_reported_as_a_settlement_problem(): + """`quote_currency_of("XLM-28AUG26-CDE")` is `"CDE"`, a perfectly resolvable-looking leg. + Reporting that as "settles in CDE, widen settlement_currencies" would invite a config edit + that admits a futures contract -- so the shape question has to come first and stop there.""" + problems = check_product_ids(["XLM-28AUG26-CDE"], _SETTLEMENT) + assert [p.kind for p in problems] == [SHAPE] + + +def test_validate_product_ids_still_raises_on_BOTH_kinds(): + """`rules seed` keeps both fatal: it writes a row the agent will poll every cycle, and a rule + the rails veto forever is not a lesser problem than a typo -- it is a quieter one.""" + for bad in ("XLM-28AUG26-CDE", "BTC-EUR"): + with pytest.raises(ValueError): + validate_product_ids([bad], _SETTLEMENT) + + +def test_every_problem_carries_its_own_reason_string(): + """The message an operator reads is per-id, not per-run, so a caller that reports only some + of the problems still reports each one in full.""" + problems = check_product_ids(["XLM-28AUG26-CDE", "BTC-EUR"], _SETTLEMENT) + assert all(p.product_id in p.reason for p in problems) + assert "not a spot product id" in problems[0].reason + assert "settles in EUR" in problems[1].reason diff --git a/tests/compliance/test_assets_cli.py b/tests/compliance/test_assets_cli.py index ff940993..2a8fbe44 100644 --- a/tests/compliance/test_assets_cli.py +++ b/tests/compliance/test_assets_cli.py @@ -723,6 +723,7 @@ def test_the_derived_failure_tags_actually_match_screen_asset_output(): daily_bars=0, median_daily_volume=Decimal(0), quotable_in_settlement_currency=False, + product_id="SOL-EUR", ) tags = {f.split(":")[0] for f in screen_mod.screen_asset(facts, None).failures} @@ -843,6 +844,58 @@ def test_screen_REPORTS_on_a_futures_id_rather_than_refusing_the_option( assert "settlement" in result.output +def test_screen_REJECTS_the_derivative_shaped_id_rail_19_exists_to_refuse( + tmp_path, valid_config_path +): + """The residual, asked of the screen instead of the rails (feasibility study R2). + + `ADA-28AUG26-CDE` above fails on SETTLEMENT (`CDE` is not a configured currency), so it + never exercised the shape question at all. `BTC-PERP-USD` is the id that does: its quote leg + IS `USD`, so the settlement criterion admits it, and with attested BTC and cached history + every other criterion admitted it too. The one command whose job is answering "may keel + trade this" said ADMIT about the one product this rail exists to refuse. + + The exemption from `--products` validation is preserved -- the screen still REPORTS rather + than refusing -- but what it reports is now the truth. + """ + db_path = tmp_path / "t.db" + repo = _repo_at(db_path) + _seed_history(repo, "BTC-PERP-USD") + runner = CliRunner() + assert _attest(runner, db_path, valid_config_path, "BTC").exit_code == 0 + + result = runner.invoke( + cli, + ["--db", str(db_path), "--config", str(valid_config_path), + "assets", "screen", "--products", "BTC-PERP-USD"], + ) + + assert result.exit_code == 0, "screening must report a verdict, not a usage error" + assert "REJECT" in result.output + assert "spot_instrument" in result.output + assert "BTC-PERP-USD" in result.output, "the verdict must name the id it is about" + + +def test_screen_still_ADMITS_a_well_formed_spot_pair(tmp_path, valid_config_path): + """The new criterion must not cost the screen its ordinary answer: the same seeded, + attested BTC on a real spot id is still ADMITted.""" + db_path = tmp_path / "t.db" + repo = _repo_at(db_path) + _seed_history(repo, "BTC-USD") + runner = CliRunner() + assert _attest(runner, db_path, valid_config_path, "BTC").exit_code == 0 + + result = runner.invoke( + cli, + ["--db", str(db_path), "--config", str(valid_config_path), + "assets", "screen", "--products", "BTC-USD"], + ) + + assert result.exit_code == 0, result.output + assert "ADMIT" in result.output + assert "spot_instrument" not in result.output + + # -- assets propose ----------------------------------------------------------------------------- diff --git a/tests/compliance/test_screen.py b/tests/compliance/test_screen.py index 7b81215b..f4acfa62 100644 --- a/tests/compliance/test_screen.py +++ b/tests/compliance/test_screen.py @@ -14,12 +14,13 @@ ) -def _facts(asset="BTC", bars=2000, volume="50000000", quotable=True) -> MarketFacts: +def _facts(asset="BTC", bars=2000, volume="50000000", quotable=True, product=None) -> MarketFacts: return MarketFacts( asset=asset, daily_bars=bars, median_daily_volume=Decimal(volume), quotable_in_settlement_currency=quotable, + product_id=product if product is not None else f"{asset}-USD", ) @@ -138,13 +139,75 @@ def test_an_asset_not_quotable_in_the_settlement_currency_is_rejected(): assert any("settlement" in f for f in result.failures) +def test_an_instrument_that_is_not_a_spot_pair_is_rejected(): + """The criterion `assets screen` was missing (feasibility study R2). + + Settlement was the screen's ONLY id-derived criterion, and a derivative-shaped id whose last + segment is a legitimate settlement currency passes it -- `quote_currency_of("BTC-PERP-USD")` + is `"USD"`. So the command whose whole job is answering "may keel trade this" said ADMIT + about the one product shape rail 19 was built to refuse. + """ + result = screen_asset(_facts(product="BTC-PERP-USD"), _attestation()) + assert result.admitted is False + assert any(f.startswith("spot_instrument") for f in result.failures) + assert any("BTC-PERP-USD" in f for f in result.failures), "the verdict must name the id" + + +@pytest.mark.parametrize( + "product", + [ + "BTC-PERP-USD", # the R2 residual: derivative-shaped, USD-settled + "ADA-28AUG26-CDE", # futures + "ac568fb9e6c5a67da94f065a49fb7b0c59b7b258cfdf0a3b1560849071c3b05e", # equity hash + "btc-usd", # lowercase: not the id the venue lists + "BTCUSD", + "", + ], +) +def test_the_shape_criterion_uses_rail_19s_grammar_not_a_second_copy(product): + """One grammar, so the screen and the rail cannot disagree about what a spot id is: an id + the screen ADMITs but rail 19 vetoes is the worst possible answer to "may keel trade this".""" + result = screen_asset(_facts(product=product), _attestation()) + assert any(f.startswith("spot_instrument") for f in result.failures), product + + +def test_the_spot_instrument_verdict_survives_a_missing_attestation(): + """It is a market fact, computed before the attestation early-return, so an unattested + derivative reports BOTH reasons rather than only the shariah one.""" + result = screen_asset(_facts(product="BTC-PERP-USD"), None) + assert any(f.startswith("spot_instrument") for f in result.failures) + assert any(f.startswith("attestation") for f in result.failures) + + +def test_the_spot_instrument_criterion_can_never_be_waived(): + """`WAIVABLE_CRITERIA` is `{"history"}`; spot-only is this agent's charter, not a threshold. + A stray exception row naming it must be dropped by the up-front filter like any other.""" + result = screen_asset( + _facts(product="BTC-PERP-USD"), + _attestation(), + waived={"spot_instrument": "we really want it"}, + ) + assert result.admitted is False + assert any(f.startswith("spot_instrument") for f in result.failures) + + +def test_the_spot_instrument_failure_is_assessable_with_zero_cached_bars(): + """Like `settlement`, it reads the product id and never touches candles -- so it must NOT be + in `DATA_DERIVED_FAILURES`, or `assets holdings` would suppress a real verdict.""" + from keel.compliance.screen import DATA_DERIVED_FAILURES + + assert "spot_instrument" not in DATA_DERIVED_FAILURES + result = screen_asset(_facts(bars=0, volume="0", product="BTC-PERP-USD"), _attestation()) + assert any(f.startswith("spot_instrument") for f in result.failures) + + def test_every_failure_is_reported_not_just_the_first(): """A screening report that stops at the first problem wastes a round trip.""" result = screen_asset( - _facts(bars=10, volume="1", quotable=False), + _facts(bars=10, volume="1", quotable=False, product="BTC-PERP-EUR"), _attestation(sector="gambling", backing="dayn", yield_=True), ) - assert len(result.failures) >= 6 + assert len(result.failures) >= 7 def test_policy_thresholds_are_configurable(): diff --git a/tests/core/test_products.py b/tests/core/test_products.py index a623973d..462d2a13 100644 --- a/tests/core/test_products.py +++ b/tests/core/test_products.py @@ -13,7 +13,7 @@ import pytest from keel_core.config import _CURRENCY_CODE_RE -from keel_core.products import parse_spot_product_id, quote_currency_of +from keel_core.products import is_spot_base_code, parse_spot_product_id, quote_currency_of def test_the_quote_leg_is_the_part_after_the_last_dash(): @@ -130,3 +130,53 @@ def test_the_two_parsers_agree_on_the_quote_leg_of_a_well_formed_spot_id(): if expected is None: continue assert quote_currency_of(product_id) == expected[1], product_id + + +# -- is_spot_base_code (the base-leg half, which `config.allowlist` is checked against) --------- + + +@pytest.mark.parametrize("code", ["BTC", "ETH", "PAXG", "ADA", "XLM", "SOL", "LTC", "LINK", + "1INCH", "A", "ABCDEFGHIJKLMNOP"]) +def test_a_real_ticker_is_a_valid_base_code(code): + """Every asset the shipped configs list, plus the digit-leading and 16-char edges.""" + assert is_spot_base_code(code) is True + + +@pytest.mark.parametrize( + "bad", + [ + "btc", # lowercase: a typo, not a ticker -- and never folded for us + "Btc", + "BTC-USD", # a product id pasted where an asset belongs: derives `BTC-USD-USD` + "BT C", + "BTC/USD", + "BTC.", + "", + " BTC", + "BTC ", + "ABCDEFGHIJKLMNOPQ", # 17 chars: past the ceiling + ], +) +def test_a_malformed_base_code_is_refused(bad): + assert is_spot_base_code(bad) is False + + +@pytest.mark.parametrize("weird", [None, 42, 3.5, b"BTC", ["BTC"], {"BTC": 1}, object()]) +def test_is_spot_base_code_is_TOTAL_and_never_raises(weird): + assert is_spot_base_code(weird) is False + + +@pytest.mark.parametrize("code", ["BTC", "ETH", "PAXG", "1INCH", "A", "ABCDEFGHIJKLMNOP"]) +@pytest.mark.parametrize("quote", ["USD", "USDC", "EUR", "USDT"]) +def test_an_admitted_base_code_always_builds_an_id_the_spot_grammar_admits(code, quote): + """THE property `config._parse_allowlist` rests on, and the reason the base grammar is shared + rather than restated. + + `_history_product` is the only path from an allowlist entry to a venue id, and it is pure + concatenation. So "config admitted this asset" must imply "rail 19 admits the id it derives", + for every settlement currency `_CURRENCY_CODE_RE` allows -- otherwise config can hand the + operator an asset the rails veto on every cycle, which is precisely the silent unfixable + rejection the load-time checks exist to prevent. + """ + assert _CURRENCY_CODE_RE.fullmatch(quote), "test premise: a configurable settlement code" + assert parse_spot_product_id(f"{code}-{quote}") == (code, quote) diff --git a/tests/data/test_fetch_cli.py b/tests/data/test_fetch_cli.py index 448fdb5f..70dfd733 100644 --- a/tests/data/test_fetch_cli.py +++ b/tests/data/test_fetch_cli.py @@ -306,3 +306,78 @@ def _fake_repair(client, repo_arg, product, granularity, **kwargs): assert result.exit_code == 0, result.output assert len(calls) == 6 # 3 products x 2 granularities assert "repairing interior gaps" in result.output + + +# -- --products validation: a SHAPE error is fatal, a SETTLEMENT mismatch is not --------------- +# +# Feasibility study R2, corrected. Validating `--products` where the operator types it is right +# for `rules seed`, which WRITES a row the agent then polls. `fetch` places no orders and needs +# no rail -- and making a settlement mismatch fatal here broke the screening workflow outright: +# `assets screen --products BTC-EUR` is exempt from validation so an operator CAN ask about a +# cross-settled pair, but the screen's answer is dominated by "0 daily bars < 1460 required", +# and there was no way to fetch that history without first widening `settlement_currencies`. +# You had to make the config change before you could evaluate whether to make it. + + +def test_fetch_refuses_a_malformed_product_id_because_that_is_always_a_typo( + tmp_path, valid_config_path, monkeypatch +): + _no_network(monkeypatch) + db_path = tmp_path / "t.db" + _repo_at(db_path) + + result = CliRunner().invoke( + cli, + ["--db", str(db_path), "--config", str(valid_config_path), + "fetch", "--check", "--products", "XLM-28AUG26-CDE"], + ) + + assert result.exit_code != 0 + assert "XLM-28AUG26-CDE" in result.output + assert "not a spot product id" in result.output + + +def test_fetch_WARNS_on_a_cross_settled_pair_and_proceeds( + tmp_path, valid_config_path, monkeypatch +): + """The history has to be fetchable before the screen can say anything about the asset. + + `BTC-EUR` is a real Coinbase spot pair. Rail 18 vetoes an ORDER for it under the shipped + settlement set -- which is why the warning is loud -- but `fetch` writes candles, and + refusing to cache them makes `settlement_currencies` a decision the operator has to take + before they can gather the evidence for it. + """ + _no_network(monkeypatch) + db_path = tmp_path / "t.db" + _repo_at(db_path) + + result = CliRunner().invoke( + cli, + ["--db", str(db_path), "--config", str(valid_config_path), + "fetch", "--check", "--products", "BTC-EUR"], + ) + + assert "BTC-EUR" in result.output + assert "settles in EUR" in result.output + assert "MISSING" in result.output, "it must actually go on to assess the product" + # `--check` exits non-zero on a missing series, which is the assessment, not the refusal. + assert "Invalid value for --products" not in result.output + + +def test_fetch_reports_a_shape_error_even_when_a_settlement_warning_rides_along( + tmp_path, valid_config_path, monkeypatch +): + """One fatal id in the list still stops the run, and the warning-worthy one is not what + decides that -- otherwise the two kinds would have to be typed in separate invocations.""" + _no_network(monkeypatch) + db_path = tmp_path / "t.db" + _repo_at(db_path) + + result = CliRunner().invoke( + cli, + ["--db", str(db_path), "--config", str(valid_config_path), + "fetch", "--check", "--products", "BTC-EUR,XLM-28AUG26-CDE"], + ) + + assert result.exit_code != 0 + assert "XLM-28AUG26-CDE" in result.output diff --git a/tests/execution/test_guards.py b/tests/execution/test_guards.py index 36711e0e..9a54c8e9 100644 --- a/tests/execution/test_guards.py +++ b/tests/execution/test_guards.py @@ -14,6 +14,7 @@ from typing import Any import pytest +from keel_core.products import parse_spot_product_id from keel_core.subscription import SubscriptionStatus from keel.config import ( @@ -1240,6 +1241,28 @@ def test_rail19_passes_a_well_formed_spot_pair_rail_18_rejects(repo: Repository) assert "settlement_currency" in _keys(result) +def test_rail19_does_NOT_close_the_two_segment_derivative_case_rail_18_does( + repo: Repository, +) -> None: + """The residual this rail leaves open, pinned so the comment above cannot quietly rot. + + `BTC-PERP` is Coinbase International's real perpetual-futures format, and it PASSES rail + 19's grammar: `PERP` is a legal quote leg by shape, and the grammar cannot know which + four-letter tokens are currencies without a currency table it deliberately does not carry. + Rail 18 is what stops it. So for a two-segment derivative id, spot-only is still a property + of `settlement_currencies` -- and an operator who widened that list to a token their venue + also uses as an instrument suffix would reopen the hole. Rail 19 makes spot-only structural + for THREE-or-more-segment ids; that is the honest claim. + """ + assert parse_spot_product_id("BTC-PERP") == ("BTC", "PERP"), "the grammar admits it" + + result = check(_intent(product_id="BTC-PERP"), repo, _config(allowlist=LIVE_ALLOWLIST), NOW_TS) + + assert "spot_instrument" not in _keys(result), "rail 19 passes it -- that is the residual" + assert "settlement_currency" in _keys(result), "rail 18 is the only thing stopping it" + assert result.ok is False + + def test_rail19_violation_names_the_product_and_says_what_shape_is_required( repo: Repository, ) -> None: @@ -1299,11 +1322,12 @@ def test__asset_still_returns_exactly_what_it_returned_before(repo: Repository) def test_open_exposure_walk_survives_a_malformed_history_row( repo: Repository, caplog: pytest.LogCaptureFixture ) -> None: - """One unparseable audit row must not crash the cycle -- and must not be SKIPPED either. + """One unparseable audit row must not crash the cycle -- and a malformed BUY is COUNTED. - Skipping would REDUCE measured exposure, loosening rails 4/5/6: that is fail-OPEN, which is - why this deliberately departs from the feasibility doc's "skip-or-flag" wording. Counting it - can only over-state exposure, which is the closed direction. + The direction is what makes this safe, and it is SIDE-DEPENDENT because + `_open_exposure_by_asset` is a net figure: BUY adds, SELL subtracts. A malformed BUY counted + can only over-state exposure against caps 4/5/6, which is the closed direction. (A malformed + SELL is the opposite and is skipped -- see the two tests below.) """ _seed_filled_order( repo, @@ -1359,6 +1383,88 @@ def test_a_malformed_history_row_still_counts_toward_the_exposure_cap(repo: Repo ) +def test_a_malformed_SELL_history_row_is_SKIPPED_not_counted( + repo: Repository, caplog: pytest.LogCaptureFixture +) -> None: + """The other half, and the one the shipped rule originally got backwards. + + `_open_exposure_by_asset` is NET: SELL *subtracts*. So counting an unparseable SELL under + `_asset`'s key REDUCES the bucket its root is capped by, i.e. it LOOSENS rails 4/5/6 -- the + fail-OPEN direction the counting rule was chosen to avoid. A futures SELL is precisely the + row shape the feasibility study found passing every shipped rail, so this is not a + hypothetical. + + Refusing to let an unreadable row release a cap is the closed answer; the WARNING is still + how the operator finds out, and it says which way the row went. + """ + _seed_filled_order( + repo, + product_id="ADA-USD", + side=Side.BUY, + qty=Decimal("1"), + price=Decimal("900"), + created_at=NOW_TS - 86_400, + ) + _seed_filled_order( + repo, + product_id=FUTURES_PRODUCT_ID, # `_asset` -> "ADA": the same bucket + side=Side.SELL, + qty=Decimal("1"), + price=Decimal("800"), + created_at=NOW_TS - 86_400, + ) + + with caplog.at_level(logging.WARNING): + exposure = guards._open_exposure_by_asset(repo) + + assert exposure == {"ADA": Decimal("900")}, ( + "the unparseable SELL relieved ADA's measured exposure -- that is fail-OPEN" + ) + skipped = [ + r + for r in caplog.records + if r.getMessage() == "guards.exposure_row_unparseable" + and r.keel_fields["product"] == FUTURES_PRODUCT_ID + ] + assert skipped, "a skipped row must still be reported, or nobody can act on it" + assert skipped[0].keel_fields["action"] == "skipped" + + +def test_a_malformed_SELL_cannot_zero_out_a_bucket_and_release_the_concentration_cap( + repo: Repository, +) -> None: + """The rail that matters, stated as money. The trailing `if amt > 0` filter means a large + enough malformed SELL does not merely shrink a bucket, it deletes it -- and rail 5's + per-asset concentration cap then admits an order the honest figure refuses.""" + _seed_filled_order( + repo, + product_id="ADA-USD", + side=Side.BUY, + qty=Decimal("1"), + price=Decimal("900"), + created_at=NOW_TS - 86_400, + ) + _seed_filled_order( + repo, + product_id=FUTURES_PRODUCT_ID, + side=Side.SELL, + qty=Decimal("1"), + price=Decimal("5000"), + created_at=NOW_TS - 86_400, + ) + + result = check( + _intent(product_id="ADA-USD", notional=Decimal("50")), + repo, + _config(allowlist=LIVE_ALLOWLIST, max_exposure_usd=Decimal("1000")), + NOW_TS, + ) + + assert "per_asset_concentration_cap" in _keys(result), ( + "an unparseable SELL emptied ADA's bucket and bought the agent headroom it has not got" + ) + + # -- offline mode (paper trading only) ----------------------------------------- diff --git a/tests/test_cli.py b/tests/test_cli.py index 5afc9b37..8fd8ed54 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -519,7 +519,11 @@ def test_rules_seed_force_reseeds_even_when_present(tmp_path, valid_config_path) assert len(repo.get_rules()) == 2 * 3 * len(RULE_REGISTRY) -def test_rules_seed_respects_products_and_kinds_options(tmp_path): +def test_rules_seed_respects_products_and_kinds_options(tmp_path, valid_config_path): + # `--config` is passed even though this is not a config test: `rules seed` loads config + # unconditionally (it needs `settlement_currencies` to validate `--products`), so without it + # the default `config.yaml` resolves against the CURRENT WORKING DIRECTORY and this passes + # only because pytest happens to run from the repo root. See tests/test_init_and_seed.py. db_path = tmp_path / "test.db" repo = _repo_at(db_path) runner = CliRunner() @@ -528,6 +532,7 @@ def test_rules_seed_respects_products_and_kinds_options(tmp_path): cli, [ "--db", str(db_path), + "--config", str(valid_config_path), "rules", "seed", "--products", "BTC-USD", "--kinds", "dca", @@ -575,7 +580,7 @@ def test_rules_seed_rows_round_trip_through_build_rule(tmp_path, valid_config_pa assert rule.product_id == row["params"]["product_id"] -def test_rules_seed_needs_no_passphrase(tmp_path): +def test_rules_seed_needs_no_passphrase(tmp_path, valid_config_path): db_path = tmp_path / "test.db" runner = CliRunner() @@ -583,7 +588,8 @@ def test_rules_seed_needs_no_passphrase(tmp_path): cli, [ "--db", str(db_path), - "rules", "seed", + "--config", str(valid_config_path), + "rules", "seed", "--products", "BTC-USD", "--kinds", "dca", ], diff --git a/tests/test_config.py b/tests/test_config.py index 1cb6a9d0..f612c1c0 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -36,6 +36,69 @@ def test_load_config_missing_allowlist_raises_configerror_mentioning_allowlist(w load_config(path) +# -- allowlist entries are shape-checked, for the same reason settlement codes are ------------- +# +# Feasibility study R2 residual. `allowlist` is not a list of product ids -- it is the list of +# BASE legs `_history_product` builds them from (`f"{asset}-{quote_currency}"`). Rail 19 then +# applies the spot grammar to the whole id, so an entry that is not a well-formed base leg +# produces a product this deployment can never trade, on every cycle, with nothing at load time +# saying why. That is exactly the silent unfixable rejection the `quote_currency`-vs- +# `settlement_currencies` cross-check below exists to prevent. + + +def test_a_lowercase_allowlist_entry_is_refused_at_load_not_silently_untradeable(write_config): + """`allowlist: [btc]` derives `btc-USD`, which rail 19 vetoes and rail 1 never even reaches. + + The operator's first signal would otherwise be a veto on every cycle, for an asset they + believed they had just enabled. + """ + text = VALID_CONFIG_YAML.replace(" - BTC\n", " - btc\n", 1) + path = write_config(text) + + with pytest.raises(ConfigError, match="allowlist") as excinfo: + load_config(path) + assert "btc" in str(excinfo.value) + assert "BTC" in str(excinfo.value), "the message must carry the fix, not just the refusal" + + +def test_an_allowlist_entry_is_never_silently_uppercased(write_config): + """The other half of the same decision, asserted so a future 'helpful' `.upper()` fails here. + + `settlement_currencies` IS case-folded, because rail 18 compares it against + `quote_currency_of`'s already-uppercased output -- folding makes the two sides agree by + construction. An allowlist entry is not compared, it is CONCATENATED into a venue + identifier, so folding it would mean the asset keel trades is not the asset the file names. + """ + text = VALID_CONFIG_YAML.replace(" - BTC\n", " - btc\n", 1) + with pytest.raises(ConfigError): + load_config(write_config(text)) + + +@pytest.mark.parametrize("bad", ["BTC-USD", "BT C", "BTC/USD", "1INCHTOOLONGATICKERBYFAR", "*"]) +def test_an_allowlist_entry_that_is_not_a_base_leg_is_refused(write_config, bad): + """A product id, a space, punctuation, an over-long ticker -- each builds an id no + `settlement_currencies` set could rescue. `BTC-USD` is the likeliest of them: the allowlist + holds assets, and pasting a product id in derives `BTC-USD-USD`.""" + text = VALID_CONFIG_YAML.replace(" - BTC\n", f" - '{bad}'\n", 1) + + with pytest.raises(ConfigError, match="allowlist"): + load_config(write_config(text)) + + +def test_a_digit_leading_ticker_is_still_accepted(write_config): + """`1INCH-USD` is a real Coinbase spot pair, so the base-leg grammar is `[A-Z0-9]`, not + `[A-Z]`. Rejecting it would be a new, invented restriction rather than a shape check.""" + text = VALID_CONFIG_YAML.replace(" - BTC\n", " - 1INCH\n", 1) + assert "1INCH" in load_config(write_config(text)).allowlist + + +def test_every_shipped_config_allowlist_still_loads(write_config): + """Blast radius, stated: the live/paper/sandbox allowlists are uppercase tickers already.""" + for entry in ("BTC", "ETH", "PAXG", "SOL", "XLM", "LTC", "ADA", "LINK"): + text = VALID_CONFIG_YAML.replace(" - BTC\n", f" - {entry}\n", 1) + assert entry in load_config(write_config(text)).allowlist + + def test_load_config_negative_cap_raises_configerror(write_config): text = VALID_CONFIG_YAML.replace( "max_per_order_usd: 100", "max_per_order_usd: -100" diff --git a/tests/test_init_and_seed.py b/tests/test_init_and_seed.py index dd7d072f..09ff869d 100644 --- a/tests/test_init_and_seed.py +++ b/tests/test_init_and_seed.py @@ -78,23 +78,30 @@ def test_init_writes_config_and_seeds_candidates(tmp_path): # -- rules seed --status ------------------------------------------------------- +# +# ⚠️ Both of these pass `--config` even though neither is about config. `rules seed` loads config +# UNCONDITIONALLY now -- it needs `settlement_currencies` to validate `--products` -- so without +# it they resolve the default `config.yaml` relative to the CURRENT WORKING DIRECTORY and pass +# only because pytest happens to run from the repo root, where one exists. That is a test suite +# whose result depends on where it is invoked from, which is a worse defect than the assertion +# it hides. -def test_seed_defaults_to_candidate(tmp_path): +def test_seed_defaults_to_candidate(tmp_path, valid_config_path): repo = _repo(tmp_path) CliRunner().invoke( - cli, ["--db", str(tmp_path / "t.db"), "rules", "seed", - "--kinds", "dca", "--products", "BTC-USD"] + cli, ["--db", str(tmp_path / "t.db"), "--config", str(valid_config_path), + "rules", "seed", "--kinds", "dca", "--products", "BTC-USD"] ) rules = repo.get_rules() assert rules and all(r["status"] == "candidate" for r in rules) -def test_seed_status_live_bypasses_the_gate_and_warns(tmp_path): +def test_seed_status_live_bypasses_the_gate_and_warns(tmp_path, valid_config_path): repo = _repo(tmp_path) result = CliRunner().invoke( - cli, ["--db", str(tmp_path / "t.db"), "rules", "seed", - "--kinds", "dca", "--products", "BTC-USD", "--status", "live"] + cli, ["--db", str(tmp_path / "t.db"), "--config", str(valid_config_path), + "rules", "seed", "--kinds", "dca", "--products", "BTC-USD", "--status", "live"] ) assert result.exit_code == 0, result.output assert "LIVE status" in result.output diff --git a/tests/test_proposer.py b/tests/test_proposer.py index 2c912c5d..73d87ee4 100644 --- a/tests/test_proposer.py +++ b/tests/test_proposer.py @@ -112,6 +112,7 @@ def screen_fn(repo, product, quote): daily_bars=bars, median_daily_volume=Decimal("2000000"), quotable_in_settlement_currency=True, + product_id=product, ) result = screen_mod.ScreenResult( asset=product.split("-")[0], @@ -154,7 +155,7 @@ def test_shariah_hypothesis_is_never_passed_to_the_gate(): def screen_fn(repo, product, quote): captured.append((repo, product, quote)) return ( - screen_mod.MarketFacts("SOL", 0, Decimal(0), True), + screen_mod.MarketFacts("SOL", 0, Decimal(0), True, "SOL-USD"), screen_mod.ScreenResult("SOL", admitted=False, failures=["attestation: MISSING."]), ) @@ -178,7 +179,7 @@ def _report(admitted, bars, attested=False, hypothesis=None): ) def screen_fn(repo, product, quote): - facts = screen_mod.MarketFacts("SOL", bars, Decimal("0"), True) + facts = screen_mod.MarketFacts("SOL", bars, Decimal("0"), True, "SOL-USD") failures = ( [] if admitted @@ -274,6 +275,7 @@ def test_data_derived_failures_tags_actually_match_screen_asset_output(): daily_bars=0, median_daily_volume=Decimal(0), quotable_in_settlement_currency=False, + product_id="SOL-EUR", ) tags = {f.split(":")[0] for f in screen_mod.screen_asset(facts, None).failures} missing = DATA_DERIVED_FAILURES - tags