feat(guards): add rail 19, the spot-instrument rail (R2) - #167
Conversation
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
Independent review applied — six findings, all confirmedA fresh-context reviewer swept all 1,114,112 Unicode codepoints against the grammar (zero non-ASCII accepted in either leg), enumerated all 41 stored product ids across both live DBs and the pre-0.4.0 backup (all parse), and confirmed no exit can be trapped. Three findings were substantive: 1. Counting a malformed SELL row was fail-OPEN — my reasoning was wrong. 2. 3. A lowercase allowlist entry was silently untradeable. Plus: 1888 tests pass, ruff clean. From a non-repo cwd the failure count now matches |
Minor: rail 19 (#167) is a new un-overridable hard rail, adds a screen criterion, and adds a load-time failure for a non-uppercase allowlist entry. It also tightens --products at the CLI on every command that writes or trades. uv.lock is relocked in the SAME commit this time. Leaving it behind is what made the 0.4.0 release fail its self-identification check: CI's `uv sync` rewrote a tracked file, so the artifact reported (DIRTY). The live deployment is on 0.4.0, which has rail 18 but not rail 19; cutting this release is what gets it there. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 participant
Implements R2 from
docs/experiments/2026-08-05-coinbase-asset-class-feasibility.md, closing the residual rail 18 (#164) deliberately left open.The residual
Rail 18 checks the settlement leg. It does not check the instrument shape:
Verified against the real live config — a derivative-shaped id with a legitimate final segment clears both shipped defences. Rail 19 is the only thing that stops it, and that is asserted as a test, in the same "the hole, stated as a test" style as rail 18's.
BTC-PERP-USD, SELL and BUY, live and offlineA validator, not a classifier
A classifier (
id → {spot, futures, equity}) must enumerate the shapes keel refuses, so it fails open on a shape Coinbase has not invented yet. A validator answers only "is this a well-formed spot id" and fails closed on novelty — the right posture for a spot-only halal agent, and it avoids building the id→class classifier the study prices as A1 scope.Grammar:
[A-Z0-9]{1,16}-[A-Z0-9]{2,10}. All 936 live spot ids match; all 99 futures (ROOT-DDMMMYY-CDE) and all 1000 equity ids (64-hex) fail. The quote bound is tied to config's_CURRENCY_CODE_REso the two grammars cannot drift apart.quote_currency_ofis untouched — rails 13/18 depend on its loose parse to name"CDE"in their messages.Two deliberate corrections
The history walk counts, it does not skip. The study said "skip-or-flag" an unparseable row in
_open_exposure_by_asset. Skipping would reduce measured exposure and thereby loosen rails 4/5/6 — fail-open. It now logs a warning and still counts the row. Overcounting is the closed direction.assets screenis exempt from--productsvalidation. Every other caller (fetch,monitor,simulate,rules seed) now refuses an untradeable id at the keyboard. Screening must not: it is the command that answers "may keel trade this, and why not", andscreen_assetreportsREJECTwith the reason. 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.What was deliberately NOT built
No engine read of
capabilities().asset_classes. The live path buildsCoinbaseClient, which has nocapabilities(); paper passesbroker=None. Such a gate is dead code that reads as a defence — the same pattern already built and deleted once in #164. The field instead gets a vocabulary check and conformance assertions so it cannot rot, and the reconciliation is deferred to the broker-port migration as a load-time check.Also not built: the id→class classifier (A1 scope, fails open), and
capabilities()onCoinbaseClient(that is the migration itself, or a second source of truth).Verification
1804 tests pass (1698 → +106),
ruff check .clean.Separately, the deployment upgrade to 0.4.0 was verified end to end: a full agent cycle ran clean against a throwaway DB copy —
mode=confirm polled=225 products=['ADA-USD','BTC-USD','ETH-USD','PAXG-USD','XLM-USD'] stale=[] signals=0.🤖 Generated with Claude Code