Skip to content

fix(tooling): summarise the whole list in the probe, and restore the field it hid - #232

Merged
eaitbrahim merged 1 commit into
mainfrom
fix/probe-list-shapes
Aug 11, 2026
Merged

fix(tooling): summarise the whole list in the probe, and restore the field it hid#232
eaitbrahim merged 1 commit into
mainfrom
fix/probe-list-shapes

Conversation

@eaitbrahim

Copy link
Copy Markdown
Contributor

Closes #230.

D1 — shape_of only ever inspected results[0]

It 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), 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 on 71% of pairs, including every asset keel trades.

shape_of now merges every element: the union of the keys, a key carried by only some of them marked min_order_amount (63/89), and a key the venue types inconsistently rendered Decimal|str (77 str, 12 Decimal) instead of silently taking the first. One summary row and a count come back however long the list is — an 89-pair response never prints 89 shapes.

How a partially present key compares against a fixture

A fixture is a single representative object and cannot express "63/89", so the convention is that the fixture carries the union of what a row can hold, and the count is reported as information rather than as a mismatch. rh_trading_pairs.json's one row is BTC-USD, and BTC-USD is sent the minimum, so:

Mechanically: every note lives in a trailing (...) suffix on the shape token, and compare_shapes compares tokens with that suffix stripped. It rides on the key, so a partially present key whose value is an object is annotated the same way as one whose value is a leaf, and it survives --json verbatim. Treating partial presence as a difference instead would fail the trading_pairs probe on every run against a venue behaving exactly as measured — the same cry-wolf failure #217 F5 taught this script to avoid.

A mixed type is deliberately not bare-equal to either of its halves, so it still reports TYPE DIFFERS with both tallies in the message. At a venue already caught quoting the same kind of value two ways in one object (#217 F6), that is a finding, not a formatting detail.

The report, on the measured 63/26 split

=== trading_pairs ===
  shape matches rh_trading_pairs.json
  note: results[].min_order_amount  present on 63/89 elements

and in --json:

"results": [
  {
    "asset_code": "str",
    "asset_increment": "str",
    "is_api_tradable": "bool",
    "max_order_size": "str",
    "min_order_amount (63/89)": "str",
    "quote_code": "str",
    "quote_increment": "str",
    "status": "str",
    "symbol": "str"
  },
  "... 89 items"
]

That exact run — 89 rows, BILL-USD first, 63 carrying the key — is asserted end to end in test_the_real_63_of_89_split_matches_the_fixture_and_is_reported, including exit code 0 and the note line.

D2 — the regression

min_order_amount is restored to tests/fixtures/rh_trading_pairs.json with BTC-USD's real 0.1. The row's max_order_size is corrected to the measured 20.0000000000000000 at the same time (it read 10.00000000); the increments were already correct.

The false claim is corrected in packages/keel-broker-robinhood/README.md (§ "No published minimum order size" → "The minimum order size is published, but only on some pairs"), in RobinhoodTransport.get_trading_pairs' docstring, and in the two suite tests that asserted the field's absence. Dated documents under docs/experiments/ are left alone. #198's prerequisite list still needs its comment corrected — min_order_amount is a lower-bound source for every asset keel trades, read as optional per pair.

D3 — regression test

test_a_key_only_a_LATER_element_carries_is_still_reported. Against the old shape_of:

>       assert len(diffs) == 1, f"a key only the second element carries went unreported: {diffs}"
E       AssertionError: a key only the second element carries went unreported: []
E       assert 0 == 1
E        +  where 0 = len([])

tests/scripts/test_robinhood_smoke.py:178: AssertionError
1 failed, 31 deselected in 0.05s

Also

The module docstring's shape-validation claim is now true and scoped: a run corroborates only what the account's own data exercises, and no report line distinguishes "the venue has no such field" from "this account produced no row carrying it". The orders probe on an account with no crypto history still proves path, signature and envelope only, never field names.

The read-only guarantee is untouched — _ReadOnly still installs the GET-only guard onto _request, and test_running_every_probe_issues_only_gets (including the probes-actually-succeeded assertion) passes unchanged.

Gates

$ uv run ruff check keel tests packages scripts
All checks passed!

$ uv run mypy
Success: no issues found in 219 source files

$ uv run pytest -q
2455 passed, 1 skipped in 36.99s
SKIPPED [1] packages/keel-broker-api/keel_broker_api/conformance/suite.py:254: adapter serves no granularity the suite could exercise

2444 → 2455 is the 11 tests added here; the one skip is the expected conformance candles probe.

Not run live — this branch has no credential. The proof is in the tests; a live re-run against the venue is yours.

References #218 (removed the field) and #198 (carries the false claim).

🤖 Generated with Claude Code

…field it hid

`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 eaitbrahim added bug Something isn't working tooling Dev/release tooling (Docs, CI & tooling) labels Aug 11, 2026
@eaitbrahim
eaitbrahim merged commit 13bbf99 into main Aug 11, 2026
1 check passed
@eaitbrahim
eaitbrahim deleted the fix/probe-list-shapes branch August 11, 2026 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working tooling Dev/release tooling (Docs, CI & tooling)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The probe only inspects results[0], and it cost us a real field — min_order_amount was removed on a false negative

1 participant