fix(brokers): read robinhood's real estimated_price shape, correct every fixture - #218
Merged
Conversation
…ery fixture First live run against a real Robinhood credential (#216) proved the response shapes wrong in four places and the probe script wrong in a fifth (#217). F1 (blocker). `_estimated_price` read `price`, which this venue does not send; the unit price is in the column named after the side asked for (`ask`/`bid`). Every market preview against the live venue came back `est_quote_size = 0.000` with `errors` populated. There is deliberately no fallback to the other side's column -- pricing a sell off an ask overstates the proceeds of an exit -- so a row missing the requested side stays unpriced. `preview_order` now also reads the venue's own `est_fee` instead of deriving one from the account tier, and reconciles `est_total_cost` against `price x quantity` and `est_fee` per response rather than assuming either reading. All three self-consistent readings recover the same fee-exclusive notional, which is what `Preview.est_quote_size` is defined to carry; a total matching none of them is priced as sent AND reported through `Preview.errors`. `synthetic=True` and `supports_native_preview=False` are unchanged and now argued for explicitly: `/estimated_price/` prices a quantity, it does not validate an order. F2. Every money value arrives unquoted, so every fixture is now an unquoted JSON number and both fixture loaders decode with `parse_float=Decimal`, the way the transport does. This makes #194's parser change exercised rather than merely present. F3. `trading_pairs` publishes no `min_order_amount` and no `min_order_size`. Removed from the fixture; the transport docstring and README now record that a pre-flight minimum-size check (#198) has no source on this endpoint. F4. `best_bid_ask` rewritten to the observed `symbol`/`bid`/`ask`; the five invented keys are gone. F5. The probe compared a post-pagination aggregate against a raw single page, so it reported `next`/`previous` MISSING AT VENUE on all five probes and buried the real findings. `fixture_shape` normalizes the fixture to what a probe can observe -- envelope stripped, numbers parsed as `Decimal` -- and a test runs the real comparison for every probe. The three order fixtures remain unverified against the venue: observing an order object requires placing a real one. Closes #217 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Verified F5 offline by replaying each fixture as the venue's single page through the exact comparison Before this PR the same simulation printed |
…ote timestamp Second live probe run against the branch (#217 F6-F8). F6. The venue is NOT uniformly unquoted -- it mixes, and `accounts` mixes within one object: `buying_power` is a quoted string beside an unquoted `fee_tier_status.fee_ratio`. `trading_pairs` and `best_bid_ask` quote all their money; `estimated_price` and `holdings` quote none. Reverted the six over-converted fields to strings so every fixture matches the venue field for field, and recorded the mixed quoting where it can be acted on: the transport's parser comment, `_decimal_or_none`, the README, and an executable test. The `parse_float=Decimal` + `Decimal(str(x))` pairing is what makes both forms land on the same exact number and neither half is optional. F7. `est_total_cost` is sent on the ask side only; a bid row carries `bid`, `quantity`, `fee_ratio` and `est_fee` and no total. The code already priced this correctly by falling through the reconciliation -- now it is intentional: a new `rh_estimated_price_bid.json` (a verbatim live bid row) and a test pinning that a sell prices from `bid x quantity` with the venue's `est_fee`, empty `errors`, and no `/accounts/` round trip. An error on every sell preview is an error nobody reads, so this must stay a clean answer rather than a degraded one. F8. `best_bid_ask` rows carry `timestamp`; added. Also: `rh_estimated_price.json` is now the verbatim live ask row, which settles the reconciliation empirically -- `64975.78 * 0.001 + 0.61726991 == 65.59304991`, i.e. fee-INCLUSIVE, exactly the reading that would have double-counted the fee. The relation is still derived per response, since that is one symbol, one side, one moment, and the bid side omits the field entirely. Preview tests size their specs from the fixture's echoed quantity rather than a literal, so the two cannot drift apart. Since the three order fixtures remain unverified on a venue proven to mix quoting, `get_order` is now tested against both the quoted and unquoted form of the same order and required to produce identical `Decimal`s. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
F6-F8 pushed as 9ec3312. Re-ran the offline simulation -- each fixture replayed as the venue's single page through the exact comparison Fixture field types after F6, as |
This was referenced Aug 9, 2026
eaitbrahim
added a commit
that referenced
this pull request
Aug 9, 2026
#219) `scripts/robinhood_smoke.py` (#216) has required `ROBINHOOD_API_KEY` and `ROBINHOOD_PRIVATE_KEY` since it landed, and #218 exercised it against the live venue twice, but neither PR added the names to `.env.example` -- so the only record of what to put in `.env` was the script's own failure message, which you see after you already went looking. Both are commented as probe-only on purpose. `load_secrets` does not read them, so a reader who fills them in and expects the adapter to come alive in a run has misread the state of this package; the README says the same thing at length and this is the one-line version at the point of use. The seed encoding is called out because a PEM is the likelier guess and fails with a length error rather than anything that names the cause. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 11, 2026
eaitbrahim
added a commit
that referenced
this pull request
Aug 11, 2026
…field it hid (#232) `shape_of` reduced a list to `[shape_of(value[0]), "... N items"]`, so every probe validated ONE element and reported a match for the whole collection. Live, `trading_pairs` returns 89 pairs in two distinct key-sets: 63 carry `min_order_amount` -- BTC-USD (`0.1`) and ETH-USD among them -- and 26 do not, and `results[0]` is BILL-USD, one of the 26. The probe reported 5/5 and then 6/6 matched while blind to a field present on 71% of pairs, including every asset keel trades. #218 then deleted that field from `rh_trading_pairs.json` believing the report, and #198's prerequisite list, the README and `get_trading_pairs`' docstring all record "the venue exposes no minimum-order field" on the same basis. `shape_of` now merges every element of a list: the union of the keys, with a key carried by only some elements marked `min_order_amount (63/89)`, and a key the venue types inconsistently across elements rendered `Decimal|str (77 str, 12 Decimal)` rather than silently taking the first. One summary row and a count come back however long the list is. A partially present key is compared as an ordinary key. A fixture is one representative object and cannot say "63 of 89", so the convention is that it carries the UNION of what a row can hold -- `rh_trading_pairs.json`'s row is BTC-USD, which is sent the minimum -- and the count reaches the operator through `annotations_in` as a note printed after the verdict, not as a difference. That makes the #218 regression a reported `NEW AT VENUE` and a correct fixture a clean match, without failing every run against a venue behaving exactly as measured. A mixed type is not bare-equal to either of its halves, so it still reports `TYPE DIFFERS`. Also restores `min_order_amount` to the fixture with BTC-USD's real `0.1` (and its real `max_order_size`, `20.0000000000000000`), corrects the README section and the transport docstring, and scopes the module docstring's shape-validation claim: a run corroborates only what the account's data exercises, and the `orders` probe on an account with no history still proves path, signature and envelope only. The read-only guarantee is untouched. Closes #230 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
eaitbrahim
added a commit
that referenced
this pull request
Aug 11, 2026
…the CTS scoring fix (#241) A minor bump, not a patch, for three reasons that each require operator action or change behaviour the deployment is currently relying on. SCHEMA. `SCHEMA_VERSION` goes 9 -> 10 (#223). Both deployed databases are at 9 and must be migrated before this build can use them. BEHAVIOUR REQUIRING OPERATOR ACTION. #223 adds a second attested claim -- what CONTRACT a venue listing is, not only what the underlying asset is. It fails closed with no backfill, deliberately, so after this lands `keel assets screen` REJECTS every product with `instrument_wrapper: UNATTESTED` until `keel assets attest-instrument` is run once per product. Live trading is unaffected: rail 1 gates buys on `config.allowlist`, not on the screen. LIVE SCORING CHANGED. #227 fixed `is_round_number`, which returned True for every 2dp-quoted price and so handed BTC/ETH/PAXG a free CTS point on every bar. Scores on those three assets are genuinely lower under this build than under 0.5.7. Also ships: the Robinhood crypto adapter behind the broker port (#216/#218/#222/#229, not wired to the live path), the TUI activity feed (#235/#237), the CTS factor collinearity study (#224), `Preview.synthetic` at the confirm gate (#221), rail 9 seeing a bracket's own stop (#212), and CI gating merges on the `test` check (#234/#238). Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Findings from the first live run against a real Robinhood credential (#216), so these are
statements about the venue rather than readings of the documentation. Ten requests, zero 401s:
signing, headers, the timestamp window, the base URL, pagination and every endpoint path are
confirmed correct and none of them are touched here.
Closes #217. Follows #216. Removes an assumed basis for part of #198.
F1 (blocker) --
_estimated_priceread a field the venue does not sendadapter.pyread_field(rows[0], "price", "0"). The observed row is:There is no
price. Every market preview against the live venue came backest_quote_size = 0.000witherrorspopulated -- confirm mode was unusable on this venue.(
to_price_side: buy ->ask, sell ->bid). There is deliberately no fallback to the otherside's column: pricing a sell off an
askoverstates the proceeds of an exit, the exactoptimistic direction
to_price_sideexists to prevent. A row not carrying the requested side istreated as unpriced.
preview_ordernow reads the venue's ownest_feerather than multiplying the notional bythe account's
fee_tier_status.fee_ratio.detail["fee_basis"]says which was used, andGET /accounts/is no longer fetched at all on a healthy market preview -- one request insteadof two, on a path the executor uses while unwinding.
est_total_costis reconciled, not assumed. See below.value still yields
None-> populatedPreview.errors, never a silent zero.synthetic=True/supports_native_preview=Falseare unchanged, and the docstrings now arguefor it rather than stating it:
/estimated_price/prices a quantity. It does not validatethe order, check buying power, check the account's size bounds, or reserve anything -- an order
it prices happily can be rejected the instant it is placed. That gap is what
Preview.syntheticexists to carry, and reading more of the venue's numbers does not close it.
Confirmed live on the branch.
SELL 0.001 BTC -> quote=64.95451 fee=0.617067845,BUY 0.001 BTC -> quote=64.947570 fee=0.617001915, bothsynthetic=True,errors=(), two GETsand no
/accounts/call. Side mapping verified:side=askanswers with anaskcolumn,side=bidwith abidcolumn.On
est_total_cost: what interpretation was chosen, and why it is not a guessI could not run live, so rather than pick a reading and hope, the relation is determined from
the venue's own numbers on every response. The row states
price,quantity,est_feeandest_total_cost-- one equation, one unknown -- and exactly one of three readings fits anyself-consistent response:
est_total_costtotal == notionaltotalest_total_cost_less_est_feetotal == notional + feetotal - feeest_total_cost_plus_est_feetotal == notional - feetotal + feeThe third is not padding: a buy's "total cost" plausibly adds the fee while a sell's plausibly
nets it out of the proceeds, and only the
askside was observed live.Pushing back on one instruction in the issue: "use
est_total_costforest_quote_size"cannot be right unconditionally.
Previewcarriesest_quote_sizeandest_feeas separatefields, and the limit path fills
est_quote_sizewithbase_size * limit_price-- afee-exclusive notional. Assigning a fee-inclusive total straight into it would double-count
the fee at the confirm gate, once inside the quote size and once in
est_fee. So the venue'snumber is what is used, adjusted by the venue's own fee according to whichever relation its
numbers satisfy.
detail["cost_basis"]reports which one.A total fitting none of the three is priced from
est_total_costexactly as sent andreported through
Preview.errors. That middle course is deliberate: refusing to price woulddegrade an exit preview over a number that is probably right, and pricing it silently would put a
cost in front of a human with an unverified relationship to the order.
The committed fixture encodes the fee-inclusive reading
(
est_total_cost == ask * quantity + est_fee, exact to the last digit), and a test asserts thatself-consistency, so a future live run that disagrees falsifies it loudly. That is my prior, not
the code's assumption: a total that merely restated
ask * quantitywould be redundant with twofields already on the row, which makes the fee-inclusive reading the one worth sending. The
adapter does not depend on my being right.
Two further checks the four numbers make possible:
quantitythan was requested, itsest_feeandest_total_costare answers about a different order. They are not scaled -- that would be the"estimate that moves between the quote and the fill" this package refuses everywhere else -- so
only the unit price is used and
errorssays why.F2 -- fixtures quoted money the venue sends unquoted
(Superseded in part by F6 below: the venue quotes some money and not other money. The fixtures
now mirror it field for field.)
Every money and size value in all eight
tests/fixtures/rh_*.jsonis now an unquoted JSON number,and both fixture loaders decode with
parse_float=Decimal, exactly asRobinhoodTransport._requestdoes. This is what makes #194's parser change exercised ratherthan merely present -- with the values quoted,
Decimal(str(v))operated on astrthat wasalready exact and the parser setting was never on the path.
tests/broker_robinhood/test_transport.pygrew a_fixture_texthelper for tests that need thevenue's exact digits:
_FakeResponse(payload=...)re-serializes withjson.dumps(which cannotencode a
Decimal, and which routes a decoded float throughrepr), so those tests now replaythe fixture's raw bytes through the transport's own decoder instead.
Every adapter read was checked against a
Decimal-valued_field.Decimal(str(x))handles both,and the
... or "0"idioms are redundant-but-correct for a falsyDecimal("0"). Nothing assumedstr.F3 --
trading_pairspublishes no minimum order sizeRemoved the invented
min_order_amountfrom the fixture. Neither it normin_order_sizeexists;transport.get_trading_pairs' docstring named the former as an input for the pre-flight sizingcheck proposed in #198, so both the docstring and the README now record that a lower bound
cannot be validated locally at all on this venue -- increment rounding and
max_order_sizecan,an undersized order is discoverable only as a rejection.
F4 -- the
best_bid_askfixture was inventedRewritten to the observed
symbol/bid/ask. The five keys it carried (price,buy_spread,sell_spread,ask_inclusive_of_buy_spread,bid_inclusive_of_sell_spread) do notexist. The adapter still does not call
get_best_bid_ask-- F1 gave no reason to, since/estimated_price/is size-aware and this endpoint is not.F5 -- the probe script's own false positive
_paginatereturns{"results": [...]}by design, and all five probes route through it, so thescript was comparing a post-pagination aggregate against a raw single-page fixture and reporting
next/previousMISSING AT VENUEon every probe -- ten lines of noise the four real findingshad to be read past.
fixture_shape()now normalizes the fixture to what a probe can actually observe: the paginationenvelope is stripped (only from payloads that have
results, so an order object is untouched),and numbers are decoded with
parse_float=Decimal. The second half matters as much as thefirst after F2 -- without it the script would report
TYPE DIFFERS fixture='float' venue='Decimal'on every money field of every probe, the same cry-wolf failure one layer down.Regression tests run the real comparison for each of the five probes against its own fixture and
require zero differences; a verified clean report is in the comment below.
Still unverified against the venue
rh_order_open.json,rh_order_filled.json,rh_order_canceled.json. Their shapes areunchanged here beyond the F2 quoting, and their field names still come from the documentation
alone -- observing an order object requires placing a real order, which
robinhood_smoke.pyrefuses by construction.
place_order,get_orderandcancel_orderall depend on them. Notedin the README under "No sandbox".
Gates
Exactly one skip, unchanged: the conformance candles probe, which Robinhood refuses at every
granularity because this API has no candles endpoint.
Nothing under
.github/workflows/, noconfig*.yaml, no.db, no.env. The adapter is stillnot wired into the live engine path. No credential value was printed or committed.
Follow-up: F6-F8 from the second live probe run
F6 -- the venue's quoting is MIXED, not uniformly unquoted
Reverted the six over-converted fields to quoted strings. Every fixture now matches the venue
field for field:
estimated_price.{ask,bid,quantity,fee_ratio,est_fee,est_total_cost},accounts.fee_tier_status.*,holdings.{total_quantity,quantity_available_for_trading}accounts.buying_power,trading_pairs.{asset_increment,quote_increment,max_order_size},best_bid_ask.{bid,ask}accountssendsbuying_powerquoted beside an unquotedfee_tier_status.fee_ratio, inthe same object, in the same response. So there is no venue-wide rule to code against and no field
that may be assumed to be one form or the other -- not even two sitting side by side.
That is recorded in four durable places rather than a commit message: the
parse_float=Decimalcomment in
transport._request(which previously over-claimed "every money field arrivesunquoted" -- corrected),
adapter._decimal_or_none's docstring, a new README section, andtest_this_venue_is_not_internally_consistent_about_quoting, which asserts the two contradictorytypes out of one object so the claim cannot rot. The safe read is the pairing this package already
has --
parse_float=Decimalin the transport so an unquoted number never touches a binaryfloat, plusDecimal(str(value))in the adapter, exact for astrand a round-trip no-op for aDecimal. Neither half is optional, and anisinstancebranch would have nothing stable tobranch on.
test_transport.pynow assertsisinstance(pair["asset_increment"], str)andisinstance(priced["ask"], Decimal)in the same test, so the contrast is visible in one place.Money comparisons go through
Decimal(...)rather than comparing the raw values, because thesearrive quoted and
"9" > "10"lexically -- an ordering bug that would only surface once a pricecrossed a digit boundary.
F7 --
est_total_costis absent on the bid sideConfirmed: the ask row carries the total, the bid row carries
bid,quantity,fee_ratioandest_feeand no total at all. As you say, the existing code already priced this correctly, byfalling through the reconciliation. It is now intentional and covered:
tests/fixtures/rh_estimated_price_bid.json, a verbatim live bid row.test_preview_order_prices_a_sell_without_the_est_total_cost_the_venue_omitspins that a sellprices from
bid * quantitywith the venue's ownest_fee,errors == (),cost_basis == "price_x_quantity",fee_basis == "venue_est_fee", and no/accounts/roundtrip.
_estimated_price, at thetotal is Nonebranch of_reconcile_total, and inthe README, each stating the same thing: a missing total is the normal bid-side answer, not
a degraded one. An exit preview that reported a problem on every call is an exit preview nobody
would read, so this must stay a clean answer.
The bid fixture is deliberately not added to
PROBES-- the probe queriesside=ask, and thetwo sides do not share a shape, so covering both needs a side-aware probe rather than a sixth
endpoint entry. That is noted at
PROBESas a follow-up. The probe count stays at five.F8 --
best_bid_askwas missingtimestampAdded. Rows are now
{symbol, timestamp, bid, ask}, withbid/askquoted per F6.Also in this round
rh_estimated_price.jsonis the verbatim live ask row (0.001 BTC), so the fee-inclusiverelation is asserted against ground truth. Preview tests size their specs from a
_QUOTED_SIZEconstant tied to the fixture's echoed
quantity, because the adapter refuses the venue's totalswhen the echoed size differs -- a literal in the test would silently start exercising the
mismatch path the day the fixture is re-captured at another size.
get_orderis tested against the quoted and the unquoted form of the same order and required toreturn identical
Decimals. That covers the half a fixture guess would otherwise leave untested,and it fails if anyone simplifies
Decimal(str(v))toDecimal(v).Gates (re-run)
Still exactly one skip.
One thing to check when you re-run the probe
The F7 dump in your message renders the
estimated_pricevalues as quoted strings(
'ask':'64975.78'), while the F6 table lists that same endpoint as unquoted. I went with F6 --it agrees with the original briefing and with the probe reporting zero
TYPE DIFFERSforestimated_priceagainst a branch whose fixture had them unquoted. If the next run reports typedifferences on that endpoint, that inference is what was wrong.