feat(broker-api): a product-catalogue read, the first of #524's two blockers

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

@@ -53,6 +53,7 @@
     Balance,
     CancelOutcome,
     FeeSummary,
+    Instrument,
     MarketSchedule,
     OrderStatus,
     PlaceResult,
@@ -363,6 +364,24 @@ def _reject_unsupported(self, spec: OrderSpec) -> None:
                 f"(supported: {', '.join(sorted(_CAPABILITIES.supported_orders))})"
             )
 
+    def get_instrument(self, product_id: str) -> Instrument | None:
+        """Not written yet, and that is a statement about this ADAPTER, not about Alpaca.
+
+        Alpaca has an assets endpoint (`/v2/assets/{symbol}`) carrying exactly this -- whether a
+        symbol is fractionable and its minimum trade increment. This adapter's `Transport`
+        protocol does not declare it, so there is nothing here to read it through, and adding one
+        is a change to the transport contract rather than a line in this method.
+
+        `NotImplementedError` rather than `None`. `None` is the port's word for "this venue does
+        not list that product", and answering it here would tell a caller a symbol is unlisted
+        when the truth is that nobody has written the lookup -- which on the executor's path
+        means silently skipping quantization for every equity.
+        """
+        raise NotImplementedError(
+            "keel-broker-alpaca has no product-catalogue read: Transport declares no assets "
+            "endpoint. See #524."
+        )
+
     def preview_order(self, spec: OrderSpec) -> Preview:
         """Synthesize a preview. Always `synthetic=True` -- Alpaca has no preview
         endpoint, so no number below is a quote the venue stands behind.

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

@@ -43,6 +43,7 @@ def broker(self) -> MyVenueAdapter:
     Balance,
     CancelOutcome,
     FeeSummary,
+    Instrument,
     MarketSchedule,
     OrderStatus,
     PlaceResult,
@@ -328,6 +329,48 @@ def test_fee_summary_matches_its_declaration(self) -> None:
         assert isinstance(summary.volume_usd, Decimal)
         assert isinstance(summary.fees_usd, Decimal)
 
+    def test_get_instrument_answers_the_port_type_or_declares_it_is_unwritten(self) -> None:
+        """`Instrument | None`, or an explicit `NotImplementedError` -- never a venue dict.
+
+        There is no capability flag gating this one, unlike `get_fee_summary`, and that is
+        deliberate: a product catalogue is not an optional venue FEATURE, it is something every
+        venue has and some adapters have not been taught to read yet. `NotImplementedError` says
+        which of those it is. `None` must not be used for it -- `None` is this method's word for
+        "this venue does not list that product", and an adapter answering it for an unwritten
+        lookup would tell `executor._base_increment_for` a symbol is unlisted when the truth is
+        that nobody wrote the read, which on the live path means silently skipping quantization.
+
+        The value is checked rather than only its type: a zero or negative increment is what a
+        caller divides and quantizes against, so it must never cross the port at all.
+        """
+        broker = self.broker()
+        try:
+            instrument = broker.get_instrument("BTC-USD")
+        except NotImplementedError:
+            return
+
+        if instrument is None:
+            return
+        assert isinstance(instrument, Instrument), (
+            f"get_instrument returned {type(instrument).__name__}, not the port's Instrument"
+        )
+        assert isinstance(instrument.base_increment, Decimal)
+        assert instrument.base_increment > 0, "a non-positive increment must never cross the port"
+        assert instrument.product_id == "BTC-USD", (
+            "the Instrument must describe the product that was asked for"
+        )
+
+    def test_get_instrument_may_answer_none_for_a_product_the_venue_does_not_list(self) -> None:
+        """An id that no venue lists. `None` or `NotImplementedError` are both correct; an
+        exception of any other kind is not, because a product id reaching this method comes from
+        an operator's allowlist and being absent is ordinary."""
+        broker = self.broker()
+        try:
+            instrument = broker.get_instrument("NOT-LISTED")
+        except NotImplementedError:
+            return
+        assert instrument is None or isinstance(instrument, Instrument)
+
     # --- no broker-native type crosses the port --------------------------------------------
 
     def test_get_balances_returns_only_domain_types(self) -> None:

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

@@ -13,6 +13,7 @@
     Balance,
     CancelOutcome,
     FeeSummary,
+    Instrument,
     MarketSchedule,
     OrderStatus,
     PlaceResult,
@@ -128,6 +129,23 @@ def get_candles(
 
     def get_balances(self) -> list[Balance]: ...
 
+    def get_instrument(self, product_id: str) -> Instrument | None:
+        """One product's venue-imposed granularity, or `None` if this venue does not list it.
+
+        **`None` is an answer, not a failure, and that is why this differs from `get_order`.** An
+        order id is one the caller was handed by this venue, so its absence is a genuine
+        inconsistency worth raising on. A product id arrives from an operator's config allowlist
+        and may simply not be listed here -- a perfectly ordinary fact about a venue, and the one
+        a multi-venue deployment most needs to be able to ask without catching an exception.
+
+        Raise for anything that is a real failure: an unreachable venue, a refused credential, a
+        response that cannot be parsed. `executor._base_increment_for` treats every one of those
+        the same way it treats `None` -- unknown, send the quantity unquantized, never refuse the
+        exit -- but that is the CALLER's policy about its own path, not a licence for an adapter
+        to swallow errors on its behalf.
+        """
+        ...
+
     def preview_order(self, spec: OrderSpec) -> Preview: ...
 
     def place_order(self, spec: OrderSpec, *, idempotency_key: str | None = None) -> PlaceResult:

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

@@ -134,6 +134,43 @@ class Balance:
     total: Decimal
 
 
+@dataclass(frozen=True)
+class Instrument:
+    """One tradeable product's venue-imposed granularity.
+
+    **Why this exists at all.** `executor._base_increment_for` (#516) needs the finest `base_size`
+    a venue will accept, and reads 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, and it is one of the two gaps #524 names as the reason the live path
+    cannot move onto the port: the port had no catalog read at all.
+
+    **One product, not the catalogue, and that is the caller's own argument.** `list_products`
+    returns every product the venue lists -- about 900 on Coinbase -- and
+    `_base_increment_for` caches exactly ONE of them per miss, because `Repository.set_state`
+    commits per call and caching all of them would mean ~900 fsyncs inside the order-placement
+    path. A port method shaped like the caller's need lets an adapter ask the venue for one
+    product where the venue supports that, and filter locally where it does not.
+
+    **Only `base_increment`, for now.** Quote-side granularity and minimum sizes are the same
+    class of fact and would sit here naturally, but nothing reads them yet, and a field no caller
+    reads is a field no test meaningfully checks.
+    """
+
+    product_id: str
+    #: The venue's finest acceptable `base_size` for this product. Always positive: an adapter
+    #: that cannot obtain a usable value returns `None` from `get_instrument` rather than
+    #: constructing an Instrument carrying zero, which a caller would quantize against and get
+    #: a division error or a silent zero size.
+    base_increment: Decimal
+
+    def __post_init__(self) -> None:
+        if self.base_increment <= 0:
+            raise ValueError(
+                f"base_increment must be positive, got {self.base_increment} for "
+                f"{self.product_id}"
+            )
+
+
 @dataclass(frozen=True)
 class Preview:
     """What the human approves at the confirm gate (`executor.py:311`).
@@ -204,6 +241,7 @@ def __post_init__(self) -> None:
 
 __all__ = [
     "Balance",
+    "Instrument",
     "CancelOutcome",
     "FeeSummary",
     "MarketSchedule",

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

@@ -26,6 +26,7 @@
     Balance,
     CancelOutcome,
     FeeSummary,
+    Instrument,
     MarketSchedule,
     OrderStatus,
     PlaceResult,
@@ -138,6 +139,33 @@ def _reject_unsupported(self, spec: OrderSpec) -> None:
         if spec.kind not in _CAPABILITIES.supported_orders:
             raise UnsupportedOrder(f"coinbase does not support order kind {spec.kind!r}")
 
+    def get_instrument(self, product_id: str) -> Instrument | None:
+        """One product's `base_increment`, read from Coinbase's per-product endpoint.
+
+        `get_product` rather than `get_products`: the caller
+        (`executor._base_increment_for`) needs ONE product and caches ONE, and asking the venue
+        for the whole catalogue -- about 900 rows -- inside the order-placement path to use a
+        single field of it is the wrong shape. The transport has carried `get_product` since
+        before this method existed.
+
+        `None` for a product this venue does not list, or whose `base_increment` is missing,
+        unparseable or non-positive. All four are the same fact to a caller -- no usable
+        granularity -- and none of them is an error worth raising on: a product id comes from an
+        operator's allowlist and may simply not be listed here.
+        """
+        response = self._require_transport().get_product(product_id)
+        raw = _field(response, "product", response)
+        increment = _field(raw, "base_increment")
+        if increment is None:
+            return None
+        try:
+            value = Decimal(str(increment))
+        except (ArithmeticError, TypeError, ValueError):
+            return None
+        if value <= 0:
+            return None
+        return Instrument(product_id=product_id, base_increment=value)
+
     def preview_order(self, spec: OrderSpec) -> Preview:
         """Preview via Coinbase's own endpoint -- hence `synthetic=False`."""
         self._reject_unsupported(spec)

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

@@ -34,6 +34,7 @@
     Balance,
     CancelOutcome,
     FeeSummary,
+    Instrument,
     MarketSchedule,
     OrderStatus,
     PlaceResult,
@@ -146,6 +147,17 @@ def get_balances(self) -> list[Balance]:
             Balance(currency="BTC", available=Decimal("0.5"), total=Decimal("0.75")),
         ]
 
+    def get_instrument(self, product_id: str) -> Instrument | None:
+        """A fixed satoshi-grained increment for anything, and `None` for one reserved id.
+
+        The reserved id is the point: `None` is a documented answer of this port method ("this
+        venue does not list it"), and a fake that could only ever answer with an Instrument would
+        let the conformance suite pass without any adapter ever exercising the absent case.
+        """
+        if product_id == "NOT-LISTED":
+            return None
+        return Instrument(product_id=product_id, base_increment=Decimal("0.00000001"))
+
     def preview_order(self, spec: OrderSpec) -> Preview:
         """This venue has no preview endpoint and does not estimate one.
 

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

@@ -33,6 +33,7 @@
     Balance,
     CancelOutcome,
     FeeSummary,
+    Instrument,
     MarketSchedule,
     OrderStatus,
     PlaceResult,
@@ -95,6 +96,10 @@ def get_balances(self) -> list[Balance]:
         """Not implemented: the stub reads no account and holds no credentials."""
         raise NotImplementedError(_STUB_MESSAGE)
 
+    def get_instrument(self, product_id: str) -> Instrument | None:
+        """Not implemented: the stub reads no catalogue."""
+        raise NotImplementedError(_STUB_MESSAGE)
+
     def preview_order(self, spec: OrderSpec) -> Preview:
         """Not implemented: the stub previews nothing (`can_preview` is False in
         `capabilities()`, so callers are told before they try)."""

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

@@ -67,6 +67,7 @@
     Balance,
     CancelOutcome,
     FeeSummary,
+    Instrument,
     MarketSchedule,
     OrderStatus,
     PlaceResult,
@@ -482,6 +483,31 @@ def _reject_unsupported(self, spec: OrderSpec) -> None:
                 f"(supported: {', '.join(sorted(_CAPABILITIES.supported_orders))})"
             )
 
+    def get_instrument(self, product_id: str) -> Instrument | None:
+        """One pair's minimum order increment, from `trading_pairs/`.
+
+        Robinhood reports `min_order_size` for a pair, which is the same fact
+        `base_increment` names on Coinbase: the finest base quantity the venue will accept. The
+        port's field keeps keel's name for it, not the venue's, exactly as `BracketGTC` keeps
+        `take_profit_price` rather than Coinbase's `limit_price`.
+
+        `None` for an unlisted symbol or an unusable value -- see the port's docstring for why
+        that is an answer rather than an error.
+        """
+        response = self._require_transport().get_trading_pairs(symbol=product_id)
+        for raw in _results(response):
+            if _field(raw, "symbol") != product_id:
+                continue
+            size = _field(raw, "min_order_size")
+            if size is None:
+                return None
+            try:
+                value = Decimal(str(size))
+            except (ArithmeticError, TypeError, ValueError):
+                return None
+            return Instrument(product_id=product_id, base_increment=value) if value > 0 else None
+        return None
+
     def preview_order(self, spec: OrderSpec) -> Preview:
         """Synthesise a preview. Always `synthetic=True` -- there is no preview endpoint here.
 

tests/broker_coinbase/test_adapter.py

@@ -46,13 +46,15 @@ def __init__(
         placed: dict[str, Any] | None = None,
         summary: dict[str, Any] | None = None,
         order: dict[str, Any] | None = None,
+        product: dict[str, Any] | None = None,
     ) -> None:
         self._candles = candles
         self._accounts = accounts
         self._preview = preview
         self._placed = placed
         self._summary = summary
         self._order = order
+        self._product = product
         self.calls: dict[str, dict[str, Any]] = {}
         # Ids this transport has actually issued via `create_order`, so `cancel_orders` can tell
         # a genuine order apart from one the suite's unknown-id test made up -- the same
@@ -71,7 +73,8 @@ def get_candles(
         return self._candles
 
     def get_product(self, product_id: str, **kwargs: Any) -> Any:
-        return {}
+        self.calls["get_product"] = {"product_id": product_id}
+        return {} if self._product is None else self._product
 
     def get_accounts(self, **kwargs: Any) -> Any:
         self.calls["get_accounts"] = {}
@@ -402,3 +405,57 @@ def cancel_orders(self, order_ids: list[str], **kwargs: Any) -> Any:
     outcome = adapter.cancel_order("abc")
     assert outcome is CancelOutcome.UNKNOWN
     assert not outcome.settled
+
+
+# -- the product catalogue (#524) ------------------------------------------------------------
+
+
+def test_get_instrument_reads_the_base_increment_from_the_per_product_endpoint() -> None:
+    """`get_product`, not `get_products`.
+
+    The caller (`executor._base_increment_for`) needs ONE product and caches ONE; fetching the
+    whole ~900-row catalogue inside the order-placement path to use a single field of it is the
+    wrong shape, and Coinbase exposes the per-product read this uses instead.
+    """
+    transport = FakeTransport(product={"product_id": "BTC-USD", "base_increment": "0.00000001"})
+    adapter = CoinbaseAdapter(transport)
+
+    instrument = adapter.get_instrument("BTC-USD")
+
+    assert instrument is not None
+    assert instrument.product_id == "BTC-USD"
+    assert instrument.base_increment == Decimal("0.00000001")
+    assert transport.calls["get_product"] == {"product_id": "BTC-USD"}
+
+
+def test_get_instrument_unwraps_a_product_envelope() -> None:
+    """Coinbase returns the product both bare and wrapped in `{"product": {...}}` depending on
+    the endpoint and the client version; both must read the same."""
+    adapter = CoinbaseAdapter(
+        FakeTransport(product={"product": {"product_id": "ETH-USD", "base_increment": "0.0001"}})
+    )
+    instrument = adapter.get_instrument("ETH-USD")
+    assert instrument is not None and instrument.base_increment == Decimal("0.0001")
+
+
+@pytest.mark.parametrize(
+    "payload",
+    [
+        {},
+        {"product_id": "BTC-USD"},
+        {"product_id": "BTC-USD", "base_increment": None},
+        {"product_id": "BTC-USD", "base_increment": "not-a-number"},
+        {"product_id": "BTC-USD", "base_increment": "0"},
+        {"product_id": "BTC-USD", "base_increment": "-1"},
+    ],
+)
+def test_get_instrument_answers_none_for_anything_unusable(payload: dict[str, Any]) -> None:
+    """Missing, unparseable, zero and negative are one fact to a caller: no usable granularity.
+
+    Zero and negative matter most. `executor._order_configuration` quantizes a SELL against this
+    value, so a zero crossing the port is a division error or a silent zero size on the exit
+    path -- which is why `Instrument.__post_init__` refuses it too, and why this returns `None`
+    rather than constructing one.
+    """
+    adapter = CoinbaseAdapter(FakeTransport(product=payload))
+    assert adapter.get_instrument("BTC-USD") is None