Skip to content

fix(brokers): correct rh_best_bid_ask to what the venue actually sends - #414

Merged
eaitbrahim merged 1 commit into
mainfrom
fix/rh-best-bid-ask-fixture
Aug 19, 2026
Merged

fix(brokers): correct rh_best_bid_ask to what the venue actually sends#414
eaitbrahim merged 1 commit into
mainfrom
fix/rh-best-bid-ask-fixture

Conversation

@eaitbrahim

Copy link
Copy Markdown
Contributor

Closes the fixture half of #413.

The wrong claim

tests/fixtures/rh_best_bid_ask.json carried:

{"next": null, "previous": null,
 "results": [{"symbol": "BTC-USD", "timestamp": "...", "bid": "65380.00", "ask": "65480.00"}]}

A tidy 15 bps spread BTC-USD does not produce, and next/previous this endpoint does not send.

What the venue actually returns

Raw body, verbatim:

{"results":[{"symbol":"BTC-USD","timestamp":"2026-08-19T19:20:41.579217945Z","bid":"68329.2","ask":"68324.17"}]}

Three samples ~2s apart, five pairs, 2026-08-19:

symbol ask − bid ①
BTC-USD −0.86 bps −0.28 bps −1.38 bps
ETH-USD +0.10 bps −0.29 bps −1.29 bps
DOGE-USD −0.11 bps −0.11 bps −0.59 bps
XLM-USD +3.21 bps +3.63 bps +5.09 bps
ADA-USD +2.41 bps +2.41 bps +2.49 bps

Not a label swap — that would invert every pair equally. The legs are sampled independently and then stamped with one timestamp the row does not earn: where the true spread is thinner than the sampling jitter (BTC, ETH, DOGE, all well under 1 bp) they land out of order; where it is wider (XLM, ADA) the ordering survives.

Why the probe never caught it

scripts/robinhood_smoke.py compares shapes, never values. It matched this fixture on every run. This is #217 F4's failure inverted — that fixture invented keys the venue never sends, this one invented an ordering it does not produce, which is harder to spot because a tidy spread is what a reader expects to see.

The fix

  • rh_best_bid_ask.json — an observed crossed BTC-USD row, no next/previous.
  • rh_best_bid_ask_uncrossed.jsonnew, an observed XLM-USD row.

Both, because either alone would be a fresh false claim: a lone crossed row asserts "always crossed" as confidently as the old one asserted "never". That is #230's lesson — a fixture can be wrong by being unrepresentative, not only by being invented.

Two tests asserted bid < ask, one per side of the port, and are inverted. The adapter-side test now pins that the endpoint admits both orderings, and that the crossing is jitter-sized (< 5 bps) rather than a semantic inversion — so a future row crossing by percent reads as the different, worse finding it would be.

transport.get_best_bid_ask gains the warning, including that translate.to_price_side's BUY→ask / SELL→bid mapping inverts on a crossed row (both directions optimistic — the one outcome that mapping exists to prevent). Its field list also claimed rows carry symbol, bid and ask while omitting timestamp, which #217 F8 had already established; corrected.

The README's "the five READ fixtures match observed responses" gains the #413 caveat next to the #230 one.

Scope

No behaviour change — nothing calls this endpoint today. Deciding the contract (normalise vs. declare it unusable for pricing) stays on #413, so a first consumer cannot pick silently.

Full suite: 3556 passed, 3 skipped. ruff check and mypy clean.

🤖 Generated with Claude Code

`rh_best_bid_ask.json` carried `bid` 65380.00 / `ask` 65480.00 -- a tidy
15 bps spread that BTC-USD does not produce -- plus `"next": null,
"previous": null`, which this endpoint does not send at all.

Measured live on 2026-08-19, three samples ~2s apart across five pairs:
BTC-USD and DOGE-USD returned `bid` ABOVE `ask` every time, ETH-USD on two
of three, XLM-USD and ADA-USD on none. Every crossing was under 1.4 bps,
and the pairs that never crossed are exactly the ones whose real spread
(2-5 bps) exceeds that jitter. So the two legs are sampled independently
and then stamped with one `timestamp` the row does not earn -- not a label
swap, which would invert every pair equally.

This is #217 F4's failure inverted. That fixture invented KEYS the venue
never sends; this one invented an ORDERING it does not produce, which is
harder to catch because a tidy spread is what a reader expects to see. The
read-only probe never caught it because `compare_shapes` compares shapes,
never values -- it matched this fixture on every run.

Both fixtures are now observed rows: `rh_best_bid_ask.json` a crossed
BTC-USD one, `rh_best_bid_ask_uncrossed.json` an XLM-USD one. Either alone
would be a fresh false claim -- a lone crossed row asserts "always crossed"
as confidently as the old one asserted "never" -- which is #230's lesson
about unrepresentative fixtures, not just invented ones.

Two tests asserted `bid < ask` and are inverted accordingly, one per side
of the port. The adapter-side test now pins that the endpoint admits BOTH
orderings, and that the crossing is jitter-sized rather than a semantic
inversion, so a future row crossing by percent reads as the different and
worse finding it would be.

`transport.get_best_bid_ask` gains the warning, including that
`translate.to_price_side`'s BUY->ask / SELL->bid mapping inverts on a
crossed row: both directions come out optimistic, the one outcome that
mapping exists to prevent. Its field list also said rows carry `symbol`,
`bid` and `ask` and omitted `timestamp`, which #217 F8 had already added.

Nothing calls this endpoint today -- #413 owns deciding the contract
before anything does. No behaviour changes here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@eaitbrahim
eaitbrahim merged commit 1d05ea4 into main Aug 19, 2026
5 checks passed
@eaitbrahim
eaitbrahim deleted the fix/rh-best-bid-ask-fixture branch August 19, 2026 19:41
eaitbrahim added a commit that referenced this pull request Aug 20, 2026
Minor, not patch. Three things since v0.9.3 change what an implementor or a
deployment can rely on:

* A NEW DISTRIBUTION. `keel-broker-alpaca` (#382, #384) plus the
  paper-equities profile that selects it (#386), so a deployment can now be
  US equities via the broker port rather than crypto only.
* THE PORT CONTRACT MOVED TWICE. `market_clock`/`market_schedule` made
  venues session-aware (#385), and `place_order` gained `idempotency_key`
  (#419). Both carry defaults so no CALLER breaks, but a third-party adapter
  that does not accept them is no longer a `Broker` -- the conformance suite
  now says so. That is exactly the kind of change a patch bump must not
  hide.
* THE OPERATOR CONSOLE. The TUI became keel's console across #399-#408, and
  `keel update` (#415/#417) makes a deployment self-updating.

Every pinned sibling moves with it. The four production distributions are
required `==` at this exact version (`RELEASING.md`, "Release assets"), so a
bump that missed one would install a mixed set -- the `keel-trader 0.5.7`
against `keel-core 0.5.5` failure `keel versions` exists to catch, and which
`~/keel` actually ran across two releases.

Also in this window, on the Robinhood adapter: the best_bid_ask fixture
corrected against the live venue (#414), a credential guard that catches the
error it only claimed to (#416), pre-flight sizing reported on the preview
(#418), transport backoff (#420), and the fenced one-order probe (#421).

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant