diff --git a/packages/keel-broker-robinhood/README.md b/packages/keel-broker-robinhood/README.md index 6cf3a22b..a59f2fc6 100644 --- a/packages/keel-broker-robinhood/README.md +++ b/packages/keel-broker-robinhood/README.md @@ -154,7 +154,7 @@ quoted strings, and `accounts` does **both in the same object**: | | fields | | --- | --- | | unquoted numbers | `estimated_price.{ask,bid,quantity,fee_ratio,est_fee,est_total_cost}`, `accounts.fee_tier_status.*`, `holdings.{total_quantity,quantity_available_for_trading}` | -| quoted strings | `accounts.buying_power`, `trading_pairs.{asset_increment,quote_increment,max_order_size}`, `best_bid_ask.{bid,ask}` | +| quoted strings | `accounts.buying_power`, `trading_pairs.{asset_increment,quote_increment,max_order_size,min_order_amount}`, `best_bid_ask.{bid,ask}` | There is therefore no venue-wide rule to code against and no field that may be assumed to be one form or the other. Two things together make every read safe, and **both** are required: @@ -164,18 +164,24 @@ a round-trip no-op for a `Decimal`. Do not "simplify" either into `Decimal(value an `isinstance` branch — there is nothing stable to branch on. The fixtures mirror the venue field for field, mixed quoting included, so the suite exercises both paths. -### No published minimum order size +### The minimum order size is published, but only on some pairs -`GET /api/v2/crypto/trading/trading_pairs/` publishes `asset_increment`, `quote_increment` and -`max_order_size`, and **no minimum of any kind** -- neither `min_order_amount` nor -`min_order_size`. This was confirmed live across four cursor pages in #217; the fixture had -invented `min_order_amount`, and `transport.get_trading_pairs`' docstring named it as an input. +`GET /api/v2/crypto/trading/trading_pairs/` publishes `asset_increment`, `quote_increment`, +`max_order_size` -- and `min_order_amount` on **63 of the 89 pairs**, including BTC-USD (`0.1`) +and ETH-USD. The other 26 pairs omit the key entirely. `min_order_size` does not exist on any pair. -The consequence is for the pre-flight sizing check proposed in #198: increment rounding and an -upper bound can be validated locally against this endpoint, and a **lower** bound cannot be -validated at all, because the venue never states one. An undersized order is discoverable only as -a rejection at placement. Anything designing that check must not assume a minimum is available -here. +⚠️ **This section previously said the venue publishes no minimum of any kind, and that was +false.** #217 F3 reached it from a probe run, and #218 deleted `min_order_amount` from +`tests/fixtures/rh_trading_pairs.json` on the strength of it. The probe's `shape_of` reduced a +list to its FIRST element, and `results[0]` is BILL-USD -- one of the 26 pairs that genuinely lack +the field. A run across all four cursor pages was still a run that read one row. #230 fixed the +probe to merge every element and restored the field. + +The consequence for the pre-flight sizing check proposed in #198: increment rounding, an upper +bound **and** a lower bound can all be validated locally against this endpoint for the assets keel +trades. The lower bound must be read as optional per pair -- absent means "the venue states none +for this pair", and an undersized order there is still discoverable only as a rejection at +placement. ### No sandbox @@ -189,7 +195,14 @@ runs it live. GET-only probe that compares each endpoint's live shape against the committed fixture. After the first run of it (#217), the five READ fixtures -- `rh_accounts.json`, `rh_holdings.json`, `rh_trading_pairs.json`, `rh_best_bid_ask.json`, `rh_estimated_price.json` -- match observed -responses. **The three order fixtures (`rh_order_open.json`, `rh_order_filled.json`, +responses. + +⚠️ **Read that sentence with #230 in mind.** Until then the probe summarised a list by its FIRST +element, so "matches observed responses" meant "matches `results[0]`" -- which is how a field on +63 of 89 trading pairs was declared non-existent and deleted. The probe now merges every element +of a list and marks a partially present key `key (63/89)`, so the claim means what it says; but a +probe run still only corroborates what the account's own data exercises, and a fixture is only as +corroborated as its most recent run. **The three order fixtures (`rh_order_open.json`, `rh_order_filled.json`, `rh_order_canceled.json`) remain unverified against the venue**, because observing an order object requires placing a real order, which that script refuses by construction. Their field names are still read from the documentation alone, and `place_order` / `get_order` / diff --git a/packages/keel-broker-robinhood/keel_broker_robinhood/transport.py b/packages/keel-broker-robinhood/keel_broker_robinhood/transport.py index 5f0d9913..8391a185 100644 --- a/packages/keel-broker-robinhood/keel_broker_robinhood/transport.py +++ b/packages/keel-broker-robinhood/keel_broker_robinhood/transport.py @@ -414,14 +414,18 @@ def get_trading_pairs(self, symbol: str | None = None) -> Any: `quote_increment`, and `max_order_size` are what would let this package round a size to the venue's tick LOCALLY instead of discovering the violation as a rejection. - ⚠️ **A minimum order size is NOT among them: this endpoint publishes none.** The rows - carry `symbol`, `asset_code`, `quote_code`, `asset_increment`, `quote_increment`, - `max_order_size`, `status` and `is_api_tradable`, and that is all -- there is no - `min_order_amount` and no `min_order_size` (#217 F3, observed live across four cursor - pages). This docstring named `min_order_amount` until that run, and the fixture invented - it, which between them gave the pre-flight minimum-size check proposed in #198 a source - that does not exist. Increment rounding and an upper bound can be checked locally against - this endpoint; a lower bound cannot be checked at all without a different source. + ⚠️ **The rows are not all the same shape.** Every row carries `symbol`, `asset_code`, + `quote_code`, `asset_increment`, `quote_increment`, `max_order_size`, `status` and + `is_api_tradable`. `min_order_amount` is carried by 63 of the 89 pairs -- BTC-USD (`0.1`) + and ETH-USD among them -- and absent from the other 26, so anything reading it must treat + it as optional per pair rather than assume the endpoint is uniform. `min_order_size` does + not exist at all. + + #217 F3 recorded that no minimum of any kind existed, and #218 removed `min_order_amount` + from the fixture on that basis. Both were wrong: the probe that produced F3 inspected + `results[0]` only, and `results[0]` is BILL-USD, one of the 26 (#230). The pre-flight + minimum-size check proposed in #198 therefore DOES have a lower-bound source for every + asset keel trades -- with the caveat that it is per pair and may be missing. That work is deliberately not done here, and the reason is the same principle that shapes `cancel_order` and `_account`: a pre-flight check that runs before every diff --git a/scripts/robinhood_smoke.py b/scripts/robinhood_smoke.py index 9ffeef45..f12cc4d6 100644 --- a/scripts/robinhood_smoke.py +++ b/scripts/robinhood_smoke.py @@ -17,6 +17,23 @@ NOT a conformance suite and NOT part of the shipped wheel -- it is an operator tool, run by hand when a credential exists, and its only output is a shape report. +## What "the shape matched" is worth, exactly + +A collection is validated across ALL of its elements, not sampled. `shape_of` unions the keys of +every element of a list and marks a key carried by only some of them `key (63/89)`, so a field +that 71% of `trading_pairs` rows carry can no longer hide behind a `results[0]` that lacks it. +That is not a hypothetical: it is #230. Until it was fixed this script reported 5/5 and then 6/6 +matched while blind to `min_order_amount`, a field BTC-USD and ETH-USD both carry, and #218 +deleted that field from the fixture believing the report. + +⚠️ **A run corroborates only what the account's own data exercises.** The probe cannot validate +field names it never receives, and no report line distinguishes "the venue has no such field" +from "this account produced no row carrying it". The `orders` probe is the standing example: on +an account with no crypto order history `results` comes back empty, so that probe proves the +path, the signature and the pagination envelope and **nothing whatsoever about field names**. The +same caveat applies in miniature to any endpoint whose rows vary -- a key absent from all 89 rows +this account can see is absent from THIS observation, which is weaker than absent from the API. + The first run of it (#217) settled all three: ten requests, zero 401s, every endpoint path correct, `fee_tier_status` corroborated key for key -- and four fixture shapes wrong, one of them a live defect that left every market preview unpriced. It also produced five false positives of @@ -68,6 +85,7 @@ import argparse import json import sys +from collections import Counter from decimal import Decimal from pathlib import Path from typing import Any @@ -157,26 +175,137 @@ def _request(self, method: str, path: str, **kwargs: Any) -> Any: return self._inner(method, path, **kwargs) +def _annotate(text: str, note: str) -> str: + """Attach a human note to a shape token without changing what the token IS. + + Everything a shape carries beyond the bare structure -- `(63/89)` on a partially present key, + the per-type tally on a key the venue quotes inconsistently -- lives in a trailing ` (...)` + suffix, and `_bare` below strips it. That split is the single place where "how many elements + carried this" is decided to be INFORMATION rather than a difference: `compare_shapes` compares + bare tokens, so a fixture that carries the key matches a venue that sends it on 63 of 89 rows, + while the count still reaches the operator's terminal and the `--json` output verbatim. + + The suffix is stored in the token rather than in a parallel structure so it survives + `json.dumps` and so a partially present key whose value is an OBJECT is annotated the same way + as one whose value is a leaf -- the note rides on the key, which every value type has. + """ + return f"{text} ({note})" + + +def _bare(token: Any) -> Any: + """A shape token with its ` (...)` note removed, which is the form comparisons use.""" + if isinstance(token, str): + return token.split(" (", 1)[0] + return token + + +def _kind(item: Any) -> str: + """One word for what an element IS, for the mixed-type tally: a type name, or the container.""" + if isinstance(item, dict): + return "object" + if isinstance(item, list): + return "array" + return shape_of(item) + + +def _merged(items: list[Any]) -> Any: + """Summarise what EVERY element of a list looks like, as one element-shaped value. + + This is the fix for #230 D1. The previous implementation reduced a list to + `[shape_of(value[0]), "... N items"]`, so every probe validated one element and reported a + match for the whole collection. `trading_pairs` returns 89 pairs in two distinct key-sets -- + 63 carry `min_order_amount`, 26 do not, and `results[0]` is one of the 26 -- so the probe + reported 6/6 matched while blind to a field present on every asset keel trades. A field + carried by SOME elements is an ordinary API shape; the probe has to model it, not collapse it. + + Three merges, by what the elements are: + + * **objects** -> the UNION of their keys. A key present on only some of them is annotated + `key (63/89)`, which makes partial presence visible without dropping the key from the shape + or claiming it is always there. The key is still in the union, so a fixture that omits it + is reported `NEW AT VENUE` -- which is exactly the miss #218 shipped. + * **lists** -> the elements are flattened and merged as one population. A nested list is + summarised across the whole parent collection rather than per parent, because the question + is still "what can a row look like". + * **anything else** -> the distinct type names, joined with `|` and tallied, e.g. + `Decimal|str (77 str, 12 Decimal)`. Taking the first element's type instead would hide a + real venue inconsistency at a venue that has already been caught quoting the same kind of + value two ways in one object (#217 F6), so a mixed type is deliberately NOT equal to either + of its halves and reports as a `TYPE DIFFERS`. + + Only ONE shape comes back however long the list is: an 89-pair response prints one merged row + and a count, never 89 rows. + """ + total = len(items) + + if all(isinstance(item, dict) for item in items): + merged: dict[str, Any] = {} + for key in sorted({key for item in items for key in item}): + present = [item[key] for item in items if key in item] + label = key if len(present) == total else _annotate(key, f"{len(present)}/{total}") + merged[label] = _merged(present) + return merged + + if all(isinstance(item, list) for item in items): + flattened = [element for item in items for element in item] + if not flattened: + return [""] + return [_merged(flattened), f"... {len(flattened)} items"] + + tallies = Counter(_kind(item) for item in items) + if len(tallies) == 1: + return next(iter(tallies)) + counts = ", ".join(f"{count} {kind}" for kind, count in tallies.most_common()) + return _annotate("|".join(sorted(tallies)), counts) + + def shape_of(value: Any) -> Any: """Reduce a decoded JSON value to its structure, discarding every leaf. - A list collapses to a single-element summary rather than one entry per item: the question is - what an element looks like, and a 90-pair `trading_pairs` response would otherwise bury the - answer in 90 identical copies. An empty list is reported as such, since "the venue returned - nothing" is itself a finding -- it is how an unfunded account presents, and it is what would - make a shape comparison vacuously pass. + A list collapses to ONE summary of all of its elements plus a count, rather than one entry per + item: the question is what an element can look like, and a 89-pair `trading_pairs` response + would otherwise bury the answer in 89 near-identical copies. That summary is a union, not a + sample -- see `_merged` for why the difference cost this repository a real field. An empty list + is reported as such, since "the venue returned nothing" is itself a finding: it is how an + unfunded account presents, and it is what would make a shape comparison vacuously pass. """ if isinstance(value, dict): return {key: shape_of(val) for key, val in sorted(value.items())} if isinstance(value, list): if not value: return [""] - return [shape_of(value[0]), f"... {len(value)} items"] + return [_merged(value), f"... {len(value)} items"] if value is None: return "null" return type(value).__name__ +def annotations_in(shape: Any, path: str = "") -> list[str]: + """Every note `shape_of` attached, as report lines -- partial presence and mixed types. + + These are NOT differences, and printing them among the differences would be the cry-wolf + failure `_PAGINATION_ENVELOPE_KEYS` exists to avoid, one layer up. They are what the operator + needs to read a clean run honestly: "the shape matched" plus "and `min_order_amount` was on 63 + of the 89 rows" is a true statement about the venue, where either half alone is not. + """ + where = path or "" + if isinstance(shape, str): + if shape == _bare(shape): + return [] + return [f" note: {where} mixed types across elements: {shape}"] + if isinstance(shape, dict): + notes: list[str] = [] + for key, val in shape.items(): + child = f"{path}.{_bare(key)}" if path else _bare(key) + if key != _bare(key): + notes.append(f" note: {child} present on {key.split(' (', 1)[1][:-1]} elements") + notes.extend(annotations_in(val, child)) + return notes + if isinstance(shape, list) and shape: + return annotations_in(shape[0], f"{path}[]") + return [] + + def fixture_shape(path: Path) -> Any: """The shape of a committed fixture, as a PROBE could ever observe it. @@ -208,23 +337,39 @@ def compare_shapes(live: Any, fixture: Any, path: str = "") -> list[str]: venue does not send is the dangerous one -- that is a field the adapter may already be reading -- but a key the venue sends and the fixture omits is how a capability gets missed, and `fees_usd` (issue #197) is exactly that shape of miss. + + Both sides are compared through `_bare`, which drops the ` (...)` notes `shape_of` attaches. + That is the decision #230 turns on, and it cuts two ways deliberately: + + * A key the venue sends on only SOME elements is compared as an ordinary key. A fixture is one + representative object and cannot say "63 of 89", so the fixture is expected to carry the + UNION of the keys the venue can send -- `rh_trading_pairs.json`'s single row is BTC-USD, and + BTC-USD is sent `min_order_amount`. A fixture that carries it matches cleanly; a fixture + that omits it is reported `NEW AT VENUE`, which is precisely the #218 regression this + restores the ability to catch. The `63/89` itself reaches the operator through + `annotations_in`, as information rather than as a difference. + * A key the venue types INCONSISTENTLY across elements is not bare-equal to either of its + types, so `Decimal|str` against a fixture's `str` still reports `TYPE DIFFERS` -- with both + tallies in the message, because at this venue that is a finding and not a formatting detail. """ diffs: list[str] = [] if isinstance(fixture, dict) and isinstance(live, dict): - for key in sorted(set(fixture) | set(live)): + live_by_key = {_bare(key): val for key, val in live.items()} + fixture_by_key = {_bare(key): val for key, val in fixture.items()} + for key in sorted(set(fixture_by_key) | set(live_by_key)): where = f"{path}.{key}" if path else key - if key not in live: - diffs.append(f" MISSING AT VENUE {where} (fixture has {fixture[key]!r})") - elif key not in fixture: - diffs.append(f" NEW AT VENUE {where} (venue sends {live[key]!r})") + if key not in live_by_key: + diffs.append(f" MISSING AT VENUE {where} (fixture has {fixture_by_key[key]!r})") + elif key not in fixture_by_key: + diffs.append(f" NEW AT VENUE {where} (venue sends {live_by_key[key]!r})") else: - diffs.extend(compare_shapes(live[key], fixture[key], where)) + diffs.extend(compare_shapes(live_by_key[key], fixture_by_key[key], where)) return diffs if isinstance(fixture, list) and isinstance(live, list): if fixture and live and fixture[0] != "" and live[0] != "": diffs.extend(compare_shapes(live[0], fixture[0], f"{path}[]")) return diffs - if live != fixture: + if _bare(live) != _bare(fixture): diffs.append(f" TYPE DIFFERS {path or ''} fixture={fixture!r} venue={live!r}") return diffs @@ -313,6 +458,11 @@ def report(results: dict[str, Any], as_json: bool) -> int: for line in diffs: print(line) failures += 1 + # Printed on a clean probe too, and after the differences rather than among them: a key on + # 63 of 89 rows is a true fact about the venue, not a fault, and a match is only honestly + # readable next to it. + for note in annotations_in(result["shape"]): + print(note) print( f"\n{len(PROBES) - failures}/{len(PROBES)} probes matched their fixture." diff --git a/tests/broker_robinhood/test_adapter.py b/tests/broker_robinhood/test_adapter.py index 24420bc8..40f4b58d 100644 --- a/tests/broker_robinhood/test_adapter.py +++ b/tests/broker_robinhood/test_adapter.py @@ -1274,10 +1274,14 @@ def test_place_order_returns_a_domain_type() -> None: # These assert on `tests/fixtures/rh_*.json` rather than on adapter behaviour, which is unusual # and deliberate. Robinhood ships no sandbox, so a fixture is the ONLY statement this repository # makes about what the venue sends -- and #217 found three of them stating things it does not: -# an `estimated_price.price` that made every market preview unpriced, a `trading_pairs` -# minimum-order field that does not exist, and a `best_bid_ask` row that was invented outright. -# A wrong fixture is not a test-data nit here; it is a false claim about a live-money venue that -# the rest of the suite then confirms. +# an `estimated_price.price` that made every market preview unpriced and a `best_bid_ask` row that +# was invented outright. A wrong fixture is not a test-data nit here; it is a false claim about a +# live-money venue that the rest of the suite then confirms. +# +# It cuts the other way too, and #230 is the proof: #217 F3 read `results[0]` alone, concluded the +# venue publishes no minimum order size, and #218 deleted a REAL field from `rh_trading_pairs.json` +# on the strength of it. A fixture can be wrong by omission, and a probe that samples one row will +# not tell you. # --------------------------------------------------------------------------------------------- #: Money and size fields the venue sends as UNQUOTED JSON numbers, keyed by fixture. Paths are @@ -1292,7 +1296,12 @@ def test_place_order_returns_a_domain_type() -> None: #: minute -- see `test_this_venue_is_not_internally_consistent_about_quoting`. _QUOTED_FIELDS: dict[str, tuple[str, ...]] = { "rh_accounts.json": ("buying_power",), - "rh_trading_pairs.json": ("asset_increment", "quote_increment", "max_order_size"), + "rh_trading_pairs.json": ( + "asset_increment", + "quote_increment", + "max_order_size", + "min_order_amount", + ), "rh_best_bid_ask.json": ("bid", "ask"), } @@ -1366,17 +1375,26 @@ def test_the_account_fee_tier_status_is_numeric_throughout() -> None: assert all(isinstance(value, Decimal) for value in tier.values()) -def test_trading_pairs_publishes_no_minimum_order_field() -> None: - """#217 F3: the venue sends neither `min_order_amount` nor `min_order_size`. +def test_trading_pairs_publishes_a_minimum_order_amount_for_the_assets_keel_trades() -> None: + """⚠️ #230 D2, reversing #217 F3 -- which was wrong, and this file asserted it for two PRs. + + `min_order_amount` **exists**, and BTC-USD carries it (`0.1`). #217 F3 concluded otherwise, and + #218 deleted the field from this fixture, because the probe that "confirmed it live across + four cursor pages" only ever inspected `results[0]` -- which is BILL-USD, one of the 26 pairs + of 89 that genuinely lack the field. The other 63, BTC-USD and ETH-USD among them, carry it. + + So the venue publishes a minimum for every asset keel trades, the pre-flight sizing check + proposed in #198 does have a lower-bound source, and a fixture missing it is a fixture missing + a field the venue sends on the only row it claims to represent. `min_order_size` really is + absent -- that half of F3 held up. - The fixture invented `min_order_amount`, and `transport.get_trading_pairs`' docstring named it - as an input for the pre-flight sizing check proposed in #198. There is no source for a minimum - on this endpoint, so that half of the check has no basis -- `asset_increment`/`quote_increment` - rounding and `max_order_size` remain, a minimum does not. Keeping the invented key would let - that follow-up be written against a field that will simply be absent at runtime. + The fixture's single row is BTC-USD deliberately: `scripts/robinhood_smoke.py` compares a + merged shape of all 89 live rows against this one object, so the object has to carry the union + of what a row can hold or the probe reports a difference it should not. """ pair = load_fixture("rh_trading_pairs.json")["results"][0] - assert "min_order_amount" not in pair + assert pair["symbol"] == "BTC-USD" + assert pair["min_order_amount"] == "0.1" assert "min_order_size" not in pair assert set(pair) == { "symbol", @@ -1385,6 +1403,7 @@ def test_trading_pairs_publishes_no_minimum_order_field() -> None: "asset_increment", "quote_increment", "max_order_size", + "min_order_amount", "status", "is_api_tradable", } diff --git a/tests/broker_robinhood/test_transport.py b/tests/broker_robinhood/test_transport.py index 5acb74a1..5e3a1e05 100644 --- a/tests/broker_robinhood/test_transport.py +++ b/tests/broker_robinhood/test_transport.py @@ -552,10 +552,11 @@ def test_trading_pairs_and_best_bid_ask_surface_their_documented_fields(http: An `max_order_size` (a bound a rejected order would violate), and the bid/ask legs a spread check would compare. - There is deliberately **no minimum-order assertion**. The fixture used to carry - `min_order_amount` and the venue sends no such field, nor `min_order_size` (#217 F3) -- so a - pre-flight minimum check has no source on this endpoint, and asserting an invented bound here - would keep pointing #198 at one. + `min_order_amount` is asserted here again after #230. #217 F3 read it off `results[0]` -- + BILL-USD, one of the 26 pairs of 89 that lack the field -- and concluded the venue publishes no + minimum at all; #218 then removed it from the fixture. The other 63 pairs carry it, BTC-USD + (`0.1`) and ETH-USD among them, so the pre-flight sizing check proposed in #198 has a real + lower-bound source for every asset keel trades. `min_order_size` is still absent. The fixture's raw bytes are replayed rather than a re-serialized decode of them, so the values reach the assertions through the same `json.loads(..., parse_float=Decimal)` the live path @@ -577,7 +578,8 @@ def test_trading_pairs_and_best_bid_ask_surface_their_documented_fields(http: An assert isinstance(pair["asset_increment"], str), "this endpoint quotes its numbers -- #217 F6" assert Decimal(pair["asset_increment"]) == Decimal("0.00000001") assert Decimal(pair["max_order_size"]) > 0 - assert "min_order_amount" not in pair + assert isinstance(pair["min_order_amount"], str), "quoted, like the rest of this endpoint" + assert Decimal(pair["min_order_amount"]) == Decimal("0.1") assert "min_order_size" not in pair http(_FakeResponse(text=_fixture_text("rh_best_bid_ask.json"))) diff --git a/tests/fixtures/rh_trading_pairs.json b/tests/fixtures/rh_trading_pairs.json index f780ecb3..654b6b01 100644 --- a/tests/fixtures/rh_trading_pairs.json +++ b/tests/fixtures/rh_trading_pairs.json @@ -8,7 +8,8 @@ "quote_code": "USD", "asset_increment": "0.00000001", "quote_increment": "0.01", - "max_order_size": "10.00000000", + "max_order_size": "20.0000000000000000", + "min_order_amount": "0.1", "status": "tradable", "is_api_tradable": true } diff --git a/tests/scripts/test_robinhood_smoke.py b/tests/scripts/test_robinhood_smoke.py index 187d89c0..2563d9fc 100644 --- a/tests/scripts/test_robinhood_smoke.py +++ b/tests/scripts/test_robinhood_smoke.py @@ -18,9 +18,11 @@ PROBES, ReadOnlyViolation, _ReadOnly, + annotations_in, compare_shapes, fixture_shape, load_credentials, + report, run_probes, shape_of, ) @@ -127,6 +129,62 @@ def test_an_empty_list_is_reported_rather_than_silently_matching() -> None: assert shape_of([]) == [""] +def test_a_list_is_summarised_by_the_union_of_its_elements_not_by_the_first() -> None: + """#230 D1: the key the second element carries has to appear in the summary.""" + shape = shape_of([{"symbol": "BILL-USD"}, {"symbol": "BTC-USD", "min_order_amount": "0.1"}]) + assert shape == [{"symbol": "str", "min_order_amount (1/2)": "str"}, "... 2 items"] + + +def test_a_partially_present_key_is_marked_with_its_count() -> None: + """The live 63/26 split, rendered the way an operator reads it. + + The count is what makes partial presence *visible* rather than either silently dropped (which + is what the old first-element summary did) or asserted as universal (which would be a + different lie: 26 pairs really do lack this key). + """ + pairs = [{"symbol": f"P{i}-USD"} for i in range(26)] + pairs += [{"symbol": f"Q{i}-USD", "min_order_amount": "0.1"} for i in range(63)] + + element = shape_of(pairs)[0] + + assert element["min_order_amount (63/89)"] == "str" + assert "min_order_amount" not in element, "the bare key would claim every pair carries it" + + +def test_an_89_element_list_still_prints_one_shape_and_a_count() -> None: + """Summarising the whole list must not mean rendering the whole list.""" + shape = shape_of([{"symbol": f"P{i}-USD"} for i in range(89)]) + assert shape == [{"symbol": "str"}, "... 89 items"] + + +def test_elements_that_type_the_same_key_differently_are_not_silently_collapsed() -> None: + """⚠️ This venue quotes the same kind of value two ways in one object (#217 F6). + + Taking the first element's type would hide exactly the ambiguity #197 turns on. The merged + token names both types and tallies them, and -- because it is not equal to either half -- it + reports as a `TYPE DIFFERS` against a fixture that can only state one. + """ + shape = shape_of([{"fee_charged": "0.01"}, {"fee_charged": Decimal("0.01")}]) + + assert shape[0]["fee_charged"].startswith("Decimal|str") + assert "1 str" in shape[0]["fee_charged"] and "1 Decimal" in shape[0]["fee_charged"] + + diffs = compare_shapes(shape, shape_of([{"fee_charged": "0.01"}])) + assert len(diffs) == 1 + assert "TYPE DIFFERS" in diffs[0] + + +def test_nested_objects_inside_list_elements_are_merged_too() -> None: + """A key two levels down is as invisible to a first-element sample as a top-level one.""" + element = shape_of( + [ + {"tier": {"fee_ratio": "0.006"}}, + {"tier": {"fee_ratio": "0.006", "next_fee_tier_ratio": "0.004"}}, + ] + )[0] + assert element["tier"] == {"fee_ratio": "str", "next_fee_tier_ratio (1/2)": "str"} + + # --- shape comparison ---------------------------------------------------------------------- @@ -157,6 +215,61 @@ def test_identical_shapes_produce_no_differences() -> None: assert compare_shapes(shape, shape) == [] +def test_a_key_only_a_LATER_element_carries_is_still_reported() -> None: + """⚠️ #230 D1, the defect this whole change exists for, as one executable claim. + + `shape_of` used to reduce 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 and ETH-USD + among them) and 26 do not -- and `results[0]` is BILL-USD, one of the 26. The probe therefore + reported 5/5 and then 6/6 matched while blind to a field present on 71% of pairs, including + every asset keel trades, and #218 deleted that field from the fixture on the strength of it. + + A field carried by SOME elements is an ordinary API shape, not an anomaly. The probe must + model it, which starts with seeing it at all. + """ + live = shape_of([{"symbol": "BTC-USD"}, {"symbol": "ETH-USD", "min_order_amount": "0.1"}]) + fixture = shape_of([{"symbol": "BTC-USD"}]) + + diffs = compare_shapes(live, fixture) + + assert len(diffs) == 1, f"a key only the second element carries went unreported: {diffs}" + assert "NEW AT VENUE" in diffs[0] + assert "min_order_amount" in diffs[0] + + +def test_a_partially_present_key_matches_a_fixture_that_carries_it() -> None: + """The other half of the #230 D1 decision, and the reason it is not simply "flag everything". + + A fixture is ONE representative object, so it cannot express "63 of 89". The convention chosen + here is that the fixture carries the UNION of what a row can hold -- `rh_trading_pairs.json`'s + row is BTC-USD, and BTC-USD is sent `min_order_amount` -- and the count reaches the operator as + a note rather than as a difference. Treating partial presence as a mismatch instead would make + every run of the `trading_pairs` probe fail against a venue behaving exactly as documented, + which is the cry-wolf failure #217 F5 already taught this script to avoid. + """ + live = shape_of([{"symbol": "BILL-USD"}, {"symbol": "BTC-USD", "min_order_amount": "0.1"}]) + fixture = shape_of([{"symbol": "BTC-USD", "min_order_amount": "0.1"}]) + + assert compare_shapes(live, fixture) == [] + + +def test_the_presence_count_is_reported_as_a_note_not_as_a_difference() -> None: + """A clean match is only honest read next to "and 26 of the 89 rows lacked that key".""" + live = shape_of({"results": [{"symbol": "BILL-USD"}, {"symbol": "BTC-USD", "min_order": "1"}]}) + + notes = annotations_in(live) + + assert notes == [" note: results[].min_order present on 1/2 elements"] + + +def test_a_mixed_type_is_noted_as_well_as_reported() -> None: + notes = annotations_in(shape_of({"results": [{"fee": "0.01"}, {"fee": Decimal("0.01")}]})) + assert len(notes) == 1 + assert "results[].fee" in notes[0] + assert "mixed types" in notes[0] + + def test_differences_are_found_inside_list_elements() -> None: live = shape_of([{"symbol": "BTC-USD"}]) fixture = shape_of([{"symbol": "BTC-USD", "min_order_amount": "1.00"}]) @@ -276,6 +389,81 @@ def test_the_pagination_envelope_is_stripped_only_from_a_paginated_shape() -> No assert "next" not in fixture_shape(_FIXTURES / "rh_accounts.json") +def _trading_pair(symbol: str, *, minimum: bool) -> dict[str, Any]: + """One row shaped like the venue's, with or without the key 26 of the 89 pairs omit.""" + row: dict[str, Any] = { + "symbol": symbol, + "asset_code": symbol.split("-")[0], + "quote_code": "USD", + "asset_increment": "0.00000001", + "quote_increment": "0.01", + "max_order_size": "20.0000000000000000", + "status": "tradable", + "is_api_tradable": True, + } + if minimum: + row["min_order_amount"] = "0.1" + return row + + +def _live_shaped_results(fixture_name: str) -> Any: + """A fixture replayed as the post-pagination payload a probe actually hands to `shape_of`.""" + payload = json.loads((_FIXTURES / fixture_name).read_text(), parse_float=Decimal) + return {"results": payload["results"]} + + +def test_the_real_63_of_89_split_matches_the_fixture_and_is_reported( + capsys: pytest.CaptureFixture[str], +) -> None: + """⚠️ #230 end to end, against the split measured live: 63 pairs with, 26 without. + + This is the run that has to come out right, and both halves of "right" are asserted: + + * **exit 0 and no differences.** The restored `min_order_amount` in `rh_trading_pairs.json` + (BTC-USD's real `0.1`) is what earns that -- delete it again and this probe reports + `NEW AT VENUE`, which is how #218 would have been caught. + * **the 63/89 printed.** A bare "shape matches" over a collection whose rows differ is the + overstatement that started all of this. The count is the difference between "the probe + checked this" and "the probe checked `results[0]`". + + `results[0]` is deliberately BILL-USD, the pair the live venue returns first and one of the 26 + that lack the key -- the exact ordering that made the old summary wrong. + """ + pairs = [_trading_pair("BILL-USD", minimum=False)] + pairs += [_trading_pair(f"P{i}-USD", minimum=False) for i in range(25)] + pairs += [_trading_pair(sym, minimum=True) for sym in ("BTC-USD", "ETH-USD")] + pairs += [_trading_pair(f"Q{i}-USD", minimum=True) for i in range(61)] + assert len(pairs) == 89 + assert sum("min_order_amount" in pair for pair in pairs) == 63 + + results = { + name: {"ok": True, "shape": shape_of(_live_shaped_results(fixture_name))} + for name, fixture_name in PROBES + } + results["trading_pairs"] = {"ok": True, "shape": shape_of({"results": pairs})} + + exit_code = report(results, as_json=False) + + out = capsys.readouterr().out + assert exit_code == 0, f"a venue behaving exactly as measured must not fail the probe:\n{out}" + assert "shape matches rh_trading_pairs.json" in out + assert "note: results[].min_order_amount present on 63/89 elements" in out + assert "all 6 probes matched their fixtures." in out + # One summary row, not 89: the report has to stay readable at the venue's real page count. + assert out.count("min_order_amount") == 1 + + +def test_the_fixture_carries_the_field_218_removed() -> None: + """The regression itself, pinned where the probe would meet it (#230 D2). + + Without this, the live 63/89 rows would report `NEW AT VENUE results[].min_order_amount` on + every run -- a real difference against a fixture that dropped a field the venue sends. + """ + pair = json.loads((_FIXTURES / "rh_trading_pairs.json").read_text())["results"][0] + assert pair["symbol"] == "BTC-USD", "the row has to be a pair that CARRIES the minimum" + assert pair["min_order_amount"] == "0.1" + + def test_a_failing_probe_does_not_abort_the_others() -> None: class _Exploding(_StubTransport): def get_best_bid_ask(self, symbol: str) -> Any: