Skip to content

Commit e5b08b4

Browse files
eaitbrahimclaude
andauthored
feat(broker-api): a product-catalogue read, the first of #524's two blockers (#561)
#524 names two reasons the live path cannot move onto the port. This closes the first and touches no live behaviour: nothing calls `get_instrument` yet. ── THE GAP ──────────────────────────────────────────────────────────────────── `executor._base_increment_for` (#516) needs the finest `base_size` a venue will accept, and gets it today by calling `broker.list_products()` and picking through raw dicts for `product_id` and `base_increment`. That is the pre-port CoinbaseClient's shape. The port had no catalogue read at all, so there was nothing for the executor to move ONTO. ── ONE PRODUCT, NOT THE CATALOGUE ───────────────────────────────────────────── `get_instrument(product_id) -> Instrument | None`, and the shape is the caller's own argument: `list_products` returns ~900 rows on Coinbase and `_base_increment_for` caches exactly ONE per miss, because `Repository.set_state` commits per call and caching all of them would mean ~900 fsyncs inside the order-placement path. Coinbase's transport has carried `get_product` all along. `None` means "this venue does not list that product" and is an ANSWER, not a failure -- which is why this differs from `get_order`. An order id was handed to the caller by this venue; a product id comes from an operator's allowlist and may simply not be listed here. `Instrument` carries `product_id` and `base_increment` and nothing else. Quote granularity and minimum sizes are the same class of fact and would sit here naturally, but nothing reads them, and a field no caller reads is a field no test meaningfully checks. `__post_init__` refuses a non-positive increment. It is what a caller quantizes against, so a zero crossing the port is a division error or a silent zero size on the exit path. ── FIVE ADAPTERS, THREE DIFFERENT TRUTHS ────────────────────────────────────── * coinbase -- `get_product`, unwrapping either envelope; `None` for missing, unparseable, zero or negative. * robinhood -- `get_trading_pairs(symbol)`; the venue's `min_order_size` IS this fact, and the port keeps keel's name for it rather than the venue's. * fake -- a fixed increment, and `None` for a reserved `NOT-LISTED` id so the conformance suite actually exercises the absent case. * alpaca -- `NotImplementedError`. Alpaca HAS `/v2/assets/{symbol}`; this adapter's Transport protocol does not declare it. Deliberately not `None`: `None` would tell the executor a symbol is unlisted when the truth is that nobody wrote the read, which on the live path means silently skipping quantization for every equity. * kraken -- the stub's standard refusal. ── CONFORMANCE ──────────────────────────────────────────────────────────────── Two suite tests, run against all four registered adapters. No capability flag gates them, deliberately: a product catalogue is not an optional venue FEATURE, it is something every venue has and some adapters have not been taught to read. `NotImplementedError` says which of those it is. Mutation-checked: returning a raw venue dict fails the type assertion, and a zero increment fails before it can be returned at all. Gates: 4292 passed / 3 skipped, ruff clean, mypy clean across 347 files. Claude-Session: https://claude.ai/code/session_01T6yA5khYnJ2qzheArRToQ2 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 556d15b commit e5b08b4

9 files changed

Lines changed: 247 additions & 1 deletion

File tree

‎packages/keel-broker-alpaca/keel_broker_alpaca/adapter.py‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
Balance,
5454
CancelOutcome,
5555
FeeSummary,
56+
Instrument,
5657
MarketSchedule,
5758
OrderStatus,
5859
PlaceResult,
@@ -363,6 +364,24 @@ def _reject_unsupported(self, spec: OrderSpec) -> None:
363364
f"(supported: {', '.join(sorted(_CAPABILITIES.supported_orders))})"
364365
)
365366

367+
def get_instrument(self, product_id: str) -> Instrument | None:
368+
"""Not written yet, and that is a statement about this ADAPTER, not about Alpaca.
369+
370+
Alpaca has an assets endpoint (`/v2/assets/{symbol}`) carrying exactly this -- whether a
371+
symbol is fractionable and its minimum trade increment. This adapter's `Transport`
372+
protocol does not declare it, so there is nothing here to read it through, and adding one
373+
is a change to the transport contract rather than a line in this method.
374+
375+
`NotImplementedError` rather than `None`. `None` is the port's word for "this venue does
376+
not list that product", and answering it here would tell a caller a symbol is unlisted
377+
when the truth is that nobody has written the lookup -- which on the executor's path
378+
means silently skipping quantization for every equity.
379+
"""
380+
raise NotImplementedError(
381+
"keel-broker-alpaca has no product-catalogue read: Transport declares no assets "
382+
"endpoint. See #524."
383+
)
384+
366385
def preview_order(self, spec: OrderSpec) -> Preview:
367386
"""Synthesize a preview. Always `synthetic=True` -- Alpaca has no preview
368387
endpoint, so no number below is a quote the venue stands behind.

‎packages/keel-broker-api/keel_broker_api/conformance/suite.py‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ def broker(self) -> MyVenueAdapter:
4343
Balance,
4444
CancelOutcome,
4545
FeeSummary,
46+
Instrument,
4647
MarketSchedule,
4748
OrderStatus,
4849
PlaceResult,
@@ -328,6 +329,48 @@ def test_fee_summary_matches_its_declaration(self) -> None:
328329
assert isinstance(summary.volume_usd, Decimal)
329330
assert isinstance(summary.fees_usd, Decimal)
330331

332+
def test_get_instrument_answers_the_port_type_or_declares_it_is_unwritten(self) -> None:
333+
"""`Instrument | None`, or an explicit `NotImplementedError` -- never a venue dict.
334+
335+
There is no capability flag gating this one, unlike `get_fee_summary`, and that is
336+
deliberate: a product catalogue is not an optional venue FEATURE, it is something every
337+
venue has and some adapters have not been taught to read yet. `NotImplementedError` says
338+
which of those it is. `None` must not be used for it -- `None` is this method's word for
339+
"this venue does not list that product", and an adapter answering it for an unwritten
340+
lookup would tell `executor._base_increment_for` a symbol is unlisted when the truth is
341+
that nobody wrote the read, which on the live path means silently skipping quantization.
342+
343+
The value is checked rather than only its type: a zero or negative increment is what a
344+
caller divides and quantizes against, so it must never cross the port at all.
345+
"""
346+
broker = self.broker()
347+
try:
348+
instrument = broker.get_instrument("BTC-USD")
349+
except NotImplementedError:
350+
return
351+
352+
if instrument is None:
353+
return
354+
assert isinstance(instrument, Instrument), (
355+
f"get_instrument returned {type(instrument).__name__}, not the port's Instrument"
356+
)
357+
assert isinstance(instrument.base_increment, Decimal)
358+
assert instrument.base_increment > 0, "a non-positive increment must never cross the port"
359+
assert instrument.product_id == "BTC-USD", (
360+
"the Instrument must describe the product that was asked for"
361+
)
362+
363+
def test_get_instrument_may_answer_none_for_a_product_the_venue_does_not_list(self) -> None:
364+
"""An id that no venue lists. `None` or `NotImplementedError` are both correct; an
365+
exception of any other kind is not, because a product id reaching this method comes from
366+
an operator's allowlist and being absent is ordinary."""
367+
broker = self.broker()
368+
try:
369+
instrument = broker.get_instrument("NOT-LISTED")
370+
except NotImplementedError:
371+
return
372+
assert instrument is None or isinstance(instrument, Instrument)
373+
331374
# --- no broker-native type crosses the port --------------------------------------------
332375

333376
def test_get_balances_returns_only_domain_types(self) -> None:

‎packages/keel-broker-api/keel_broker_api/port.py‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
Balance,
1414
CancelOutcome,
1515
FeeSummary,
16+
Instrument,
1617
MarketSchedule,
1718
OrderStatus,
1819
PlaceResult,
@@ -128,6 +129,23 @@ def get_candles(
128129

129130
def get_balances(self) -> list[Balance]: ...
130131

132+
def get_instrument(self, product_id: str) -> Instrument | None:
133+
"""One product's venue-imposed granularity, or `None` if this venue does not list it.
134+
135+
**`None` is an answer, not a failure, and that is why this differs from `get_order`.** An
136+
order id is one the caller was handed by this venue, so its absence is a genuine
137+
inconsistency worth raising on. A product id arrives from an operator's config allowlist
138+
and may simply not be listed here -- a perfectly ordinary fact about a venue, and the one
139+
a multi-venue deployment most needs to be able to ask without catching an exception.
140+
141+
Raise for anything that is a real failure: an unreachable venue, a refused credential, a
142+
response that cannot be parsed. `executor._base_increment_for` treats every one of those
143+
the same way it treats `None` -- unknown, send the quantity unquantized, never refuse the
144+
exit -- but that is the CALLER's policy about its own path, not a licence for an adapter
145+
to swallow errors on its behalf.
146+
"""
147+
...
148+
131149
def preview_order(self, spec: OrderSpec) -> Preview: ...
132150

133151
def place_order(self, spec: OrderSpec, *, idempotency_key: str | None = None) -> PlaceResult:

‎packages/keel-broker-api/keel_broker_api/results.py‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,43 @@ class Balance:
134134
total: Decimal
135135

136136

137+
@dataclass(frozen=True)
138+
class Instrument:
139+
"""One tradeable product's venue-imposed granularity.
140+
141+
**Why this exists at all.** `executor._base_increment_for` (#516) needs the finest `base_size`
142+
a venue will accept, and reads it today by calling `broker.list_products()` and picking
143+
through raw dicts for `product_id` and `base_increment`. That is the pre-port
144+
`CoinbaseClient`'s shape, and it is one of the two gaps #524 names as the reason the live path
145+
cannot move onto the port: the port had no catalog read at all.
146+
147+
**One product, not the catalogue, and that is the caller's own argument.** `list_products`
148+
returns every product the venue lists -- about 900 on Coinbase -- and
149+
`_base_increment_for` caches exactly ONE of them per miss, because `Repository.set_state`
150+
commits per call and caching all of them would mean ~900 fsyncs inside the order-placement
151+
path. A port method shaped like the caller's need lets an adapter ask the venue for one
152+
product where the venue supports that, and filter locally where it does not.
153+
154+
**Only `base_increment`, for now.** Quote-side granularity and minimum sizes are the same
155+
class of fact and would sit here naturally, but nothing reads them yet, and a field no caller
156+
reads is a field no test meaningfully checks.
157+
"""
158+
159+
product_id: str
160+
#: The venue's finest acceptable `base_size` for this product. Always positive: an adapter
161+
#: that cannot obtain a usable value returns `None` from `get_instrument` rather than
162+
#: constructing an Instrument carrying zero, which a caller would quantize against and get
163+
#: a division error or a silent zero size.
164+
base_increment: Decimal
165+
166+
def __post_init__(self) -> None:
167+
if self.base_increment <= 0:
168+
raise ValueError(
169+
f"base_increment must be positive, got {self.base_increment} for "
170+
f"{self.product_id}"
171+
)
172+
173+
137174
@dataclass(frozen=True)
138175
class Preview:
139176
"""What the human approves at the confirm gate (`executor.py:311`).
@@ -204,6 +241,7 @@ def __post_init__(self) -> None:
204241

205242
__all__ = [
206243
"Balance",
244+
"Instrument",
207245
"CancelOutcome",
208246
"FeeSummary",
209247
"MarketSchedule",

‎packages/keel-broker-coinbase/keel_broker_coinbase/adapter.py‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
Balance,
2727
CancelOutcome,
2828
FeeSummary,
29+
Instrument,
2930
MarketSchedule,
3031
OrderStatus,
3132
PlaceResult,
@@ -138,6 +139,33 @@ def _reject_unsupported(self, spec: OrderSpec) -> None:
138139
if spec.kind not in _CAPABILITIES.supported_orders:
139140
raise UnsupportedOrder(f"coinbase does not support order kind {spec.kind!r}")
140141

142+
def get_instrument(self, product_id: str) -> Instrument | None:
143+
"""One product's `base_increment`, read from Coinbase's per-product endpoint.
144+
145+
`get_product` rather than `get_products`: the caller
146+
(`executor._base_increment_for`) needs ONE product and caches ONE, and asking the venue
147+
for the whole catalogue -- about 900 rows -- inside the order-placement path to use a
148+
single field of it is the wrong shape. The transport has carried `get_product` since
149+
before this method existed.
150+
151+
`None` for a product this venue does not list, or whose `base_increment` is missing,
152+
unparseable or non-positive. All four are the same fact to a caller -- no usable
153+
granularity -- and none of them is an error worth raising on: a product id comes from an
154+
operator's allowlist and may simply not be listed here.
155+
"""
156+
response = self._require_transport().get_product(product_id)
157+
raw = _field(response, "product", response)
158+
increment = _field(raw, "base_increment")
159+
if increment is None:
160+
return None
161+
try:
162+
value = Decimal(str(increment))
163+
except (ArithmeticError, TypeError, ValueError):
164+
return None
165+
if value <= 0:
166+
return None
167+
return Instrument(product_id=product_id, base_increment=value)
168+
141169
def preview_order(self, spec: OrderSpec) -> Preview:
142170
"""Preview via Coinbase's own endpoint -- hence `synthetic=False`."""
143171
self._reject_unsupported(spec)

‎packages/keel-broker-fake/keel_broker_fake/adapter.py‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
Balance,
3535
CancelOutcome,
3636
FeeSummary,
37+
Instrument,
3738
MarketSchedule,
3839
OrderStatus,
3940
PlaceResult,
@@ -146,6 +147,17 @@ def get_balances(self) -> list[Balance]:
146147
Balance(currency="BTC", available=Decimal("0.5"), total=Decimal("0.75")),
147148
]
148149

150+
def get_instrument(self, product_id: str) -> Instrument | None:
151+
"""A fixed satoshi-grained increment for anything, and `None` for one reserved id.
152+
153+
The reserved id is the point: `None` is a documented answer of this port method ("this
154+
venue does not list it"), and a fake that could only ever answer with an Instrument would
155+
let the conformance suite pass without any adapter ever exercising the absent case.
156+
"""
157+
if product_id == "NOT-LISTED":
158+
return None
159+
return Instrument(product_id=product_id, base_increment=Decimal("0.00000001"))
160+
149161
def preview_order(self, spec: OrderSpec) -> Preview:
150162
"""This venue has no preview endpoint and does not estimate one.
151163

‎packages/keel-broker-kraken/keel_broker_kraken/adapter.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
Balance,
3434
CancelOutcome,
3535
FeeSummary,
36+
Instrument,
3637
MarketSchedule,
3738
OrderStatus,
3839
PlaceResult,
@@ -95,6 +96,10 @@ def get_balances(self) -> list[Balance]:
9596
"""Not implemented: the stub reads no account and holds no credentials."""
9697
raise NotImplementedError(_STUB_MESSAGE)
9798

99+
def get_instrument(self, product_id: str) -> Instrument | None:
100+
"""Not implemented: the stub reads no catalogue."""
101+
raise NotImplementedError(_STUB_MESSAGE)
102+
98103
def preview_order(self, spec: OrderSpec) -> Preview:
99104
"""Not implemented: the stub previews nothing (`can_preview` is False in
100105
`capabilities()`, so callers are told before they try)."""

‎packages/keel-broker-robinhood/keel_broker_robinhood/adapter.py‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@
6767
Balance,
6868
CancelOutcome,
6969
FeeSummary,
70+
Instrument,
7071
MarketSchedule,
7172
OrderStatus,
7273
PlaceResult,
@@ -482,6 +483,31 @@ def _reject_unsupported(self, spec: OrderSpec) -> None:
482483
f"(supported: {', '.join(sorted(_CAPABILITIES.supported_orders))})"
483484
)
484485

486+
def get_instrument(self, product_id: str) -> Instrument | None:
487+
"""One pair's minimum order increment, from `trading_pairs/`.
488+
489+
Robinhood reports `min_order_size` for a pair, which is the same fact
490+
`base_increment` names on Coinbase: the finest base quantity the venue will accept. The
491+
port's field keeps keel's name for it, not the venue's, exactly as `BracketGTC` keeps
492+
`take_profit_price` rather than Coinbase's `limit_price`.
493+
494+
`None` for an unlisted symbol or an unusable value -- see the port's docstring for why
495+
that is an answer rather than an error.
496+
"""
497+
response = self._require_transport().get_trading_pairs(symbol=product_id)
498+
for raw in _results(response):
499+
if _field(raw, "symbol") != product_id:
500+
continue
501+
size = _field(raw, "min_order_size")
502+
if size is None:
503+
return None
504+
try:
505+
value = Decimal(str(size))
506+
except (ArithmeticError, TypeError, ValueError):
507+
return None
508+
return Instrument(product_id=product_id, base_increment=value) if value > 0 else None
509+
return None
510+
485511
def preview_order(self, spec: OrderSpec) -> Preview:
486512
"""Synthesise a preview. Always `synthetic=True` -- there is no preview endpoint here.
487513

‎tests/broker_coinbase/test_adapter.py‎

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,15 @@ def __init__(
4646
placed: dict[str, Any] | None = None,
4747
summary: dict[str, Any] | None = None,
4848
order: dict[str, Any] | None = None,
49+
product: dict[str, Any] | None = None,
4950
) -> None:
5051
self._candles = candles
5152
self._accounts = accounts
5253
self._preview = preview
5354
self._placed = placed
5455
self._summary = summary
5556
self._order = order
57+
self._product = product
5658
self.calls: dict[str, dict[str, Any]] = {}
5759
# Ids this transport has actually issued via `create_order`, so `cancel_orders` can tell
5860
# a genuine order apart from one the suite's unknown-id test made up -- the same
@@ -71,7 +73,8 @@ def get_candles(
7173
return self._candles
7274

7375
def get_product(self, product_id: str, **kwargs: Any) -> Any:
74-
return {}
76+
self.calls["get_product"] = {"product_id": product_id}
77+
return {} if self._product is None else self._product
7578

7679
def get_accounts(self, **kwargs: Any) -> Any:
7780
self.calls["get_accounts"] = {}
@@ -402,3 +405,57 @@ def cancel_orders(self, order_ids: list[str], **kwargs: Any) -> Any:
402405
outcome = adapter.cancel_order("abc")
403406
assert outcome is CancelOutcome.UNKNOWN
404407
assert not outcome.settled
408+
409+
410+
# -- the product catalogue (#524) ------------------------------------------------------------
411+
412+
413+
def test_get_instrument_reads_the_base_increment_from_the_per_product_endpoint() -> None:
414+
"""`get_product`, not `get_products`.
415+
416+
The caller (`executor._base_increment_for`) needs ONE product and caches ONE; fetching the
417+
whole ~900-row catalogue inside the order-placement path to use a single field of it is the
418+
wrong shape, and Coinbase exposes the per-product read this uses instead.
419+
"""
420+
transport = FakeTransport(product={"product_id": "BTC-USD", "base_increment": "0.00000001"})
421+
adapter = CoinbaseAdapter(transport)
422+
423+
instrument = adapter.get_instrument("BTC-USD")
424+
425+
assert instrument is not None
426+
assert instrument.product_id == "BTC-USD"
427+
assert instrument.base_increment == Decimal("0.00000001")
428+
assert transport.calls["get_product"] == {"product_id": "BTC-USD"}
429+
430+
431+
def test_get_instrument_unwraps_a_product_envelope() -> None:
432+
"""Coinbase returns the product both bare and wrapped in `{"product": {...}}` depending on
433+
the endpoint and the client version; both must read the same."""
434+
adapter = CoinbaseAdapter(
435+
FakeTransport(product={"product": {"product_id": "ETH-USD", "base_increment": "0.0001"}})
436+
)
437+
instrument = adapter.get_instrument("ETH-USD")
438+
assert instrument is not None and instrument.base_increment == Decimal("0.0001")
439+
440+
441+
@pytest.mark.parametrize(
442+
"payload",
443+
[
444+
{},
445+
{"product_id": "BTC-USD"},
446+
{"product_id": "BTC-USD", "base_increment": None},
447+
{"product_id": "BTC-USD", "base_increment": "not-a-number"},
448+
{"product_id": "BTC-USD", "base_increment": "0"},
449+
{"product_id": "BTC-USD", "base_increment": "-1"},
450+
],
451+
)
452+
def test_get_instrument_answers_none_for_anything_unusable(payload: dict[str, Any]) -> None:
453+
"""Missing, unparseable, zero and negative are one fact to a caller: no usable granularity.
454+
455+
Zero and negative matter most. `executor._order_configuration` quantizes a SELL against this
456+
value, so a zero crossing the port is a division error or a silent zero size on the exit
457+
path -- which is why `Instrument.__post_init__` refuses it too, and why this returns `None`
458+
rather than constructing one.
459+
"""
460+
adapter = CoinbaseAdapter(FakeTransport(product=payload))
461+
assert adapter.get_instrument("BTC-USD") is None

0 commit comments

Comments
 (0)