feat(executor): the quote balance comes from the port's shape, and only available counts

keel/data/cb_client.py

@@ -26,7 +26,7 @@
 from decimal import Decimal
 from typing import Any, Protocol
 
-from keel_broker_api.results import CancelOutcome
+from keel_broker_api.results import Balance, CancelOutcome
 from keel_core.telemetry import log_exception, log_venue_failure
 
 from keel.types import Candle, Granularity, Side
@@ -233,6 +233,42 @@ def get_accounts(self) -> list[dict]:
             )
         return accounts
 
+    def get_balances(self) -> list[Balance]:
+        """The same read as `get_accounts`, in the PORT's shape (#524).
+
+        This client predates `keel-broker-api` and its `get_accounts` returns venue-shaped dicts
+        (`available_balance`, plus `uuid`/`default`/`active` nobody reads). The port's answer is
+        `list[Balance]` -- `currency`, `available`, `total` -- and `executor._fetch_available_quote`
+        had to probe for BOTH shapes, dict key or attribute, because it did not know which kind of
+        broker it held.
+
+        Teaching this client the port's shape removes that fork without flipping anything: the
+        executor now asks one question, and the answer is the same type whether it is talking to
+        this pre-port client or to a real adapter. When `_build_broker` finally resolves through
+        `load_broker`, this path needs no further change.
+
+        `total` is `available + hold`, matching `keel_broker_coinbase.adapter.get_balances`
+        exactly -- Coinbase exposes no single "total" field, and the two implementations must not
+        disagree about what the word means while both exist.
+        """
+        try:
+            response = self._transport.get_accounts()
+        except Exception:
+            log_venue_failure(logger, "cb_client.accounts_fetch_failed")
+            raise
+        balances: list[Balance] = []
+        for raw in _field(response, "accounts", []) or []:
+            available = Decimal(_field(_field(raw, "available_balance") or {}, "value", "0"))
+            hold = Decimal(_field(_field(raw, "hold") or {}, "value", "0"))
+            balances.append(
+                Balance(
+                    currency=str(_field(raw, "currency", "")),
+                    available=available,
+                    total=available + hold,
+                )
+            )
+        return balances
+
     def preview_order(self, product_id: str, side: Side, order_configuration: dict) -> dict:
         """Preview an order (no funds moved) -- returns `Decimal` money fields + any errors."""
         response = self._transport.preview_order(

keel/execution/executor.py

@@ -63,7 +63,7 @@
 `trade_outcomes` row so rail 16 does not count a scaled-out winner as a loss).
 
 **USDC-funding balance (rail 13, Issue #59).** For a BUY `_build_intent` fetches the live
-available balance of the PRODUCT's quote leg from `broker.get_accounts()` and hands
+available balance of the PRODUCT's quote leg from `broker.get_balances()` and hands
 it to `guards.check` via `OrderIntent.available_quote` -- guards itself has no broker access, by
 design. This happens *before* `guards.check` runs (the balance is an input to the rail, not
 something guarded itself), so it is the one broker call this module makes ahead of the guard
@@ -411,7 +411,7 @@ def _coerce_increment(raw: object) -> Decimal | None:
 
 
 def _fetch_available_quote(broker: Any, quote_currency: str | None) -> Decimal | None:
-    """Live available balance of `quote_currency` from `broker.get_accounts()`.
+    """Live available balance of `quote_currency`, from the port's `get_balances()`.
 
     `quote_currency` is the **product's own settlement leg** (`BTC-USD` -> `USD`), not
     `config.quote_currency`: the currency an order spends is a property of the product. Checking
@@ -432,7 +432,7 @@ def _fetch_available_quote(broker: Any, quote_currency: str | None) -> Decimal |
         # that is expected and already handled (rail 13 is skipped offline).
         return None
     try:
-        accounts = broker.get_accounts()
+        balances = broker.get_balances()
     except Exception:
         # `log_venue_failure`, not `log_exception`: an unreachable venue outside a trade cycle
         # is a dashboard balance refresh on a sleeping laptop, and this line is the SECOND
@@ -444,22 +444,14 @@ def _fetch_available_quote(broker: Any, quote_currency: str | None) -> Decimal |
         log_venue_failure(logger, "executor.quote_fetch_failed", quote_currency=quote_currency)
         return None
 
-    for account in accounts or []:
-        currency = (
-            account.get("currency")
-            if isinstance(account, dict)
-            else getattr(account, "currency", None)
-        )
-        if (currency or "").upper() != quote_currency.upper():
-            continue
-        balance = (
-            account.get("available_balance")
-            if isinstance(account, dict)
-            else getattr(account, "available_balance", None)
-        )
-        if balance is None:
-            return None
-        return balance if isinstance(balance, Decimal) else Decimal(str(balance))
+    # ONE shape, since #524. This probed for two -- a dict key or an attribute, `currency` or
+    # `.currency`, `available_balance` or `.available_balance` -- because it did not know whether
+    # it held the pre-port `CoinbaseClient` or a port adapter. `CoinbaseClient.get_balances` now
+    # answers in the port's `Balance` type as well, so there is one question and one answer, and
+    # the fork that existed only to bridge them is gone.
+    for balance in balances or []:
+        if balance.currency.upper() == quote_currency.upper():
+            return balance.available
 
     return None
 

tests/data/test_cb_client.py

@@ -14,7 +14,7 @@
 from typing import Any
 
 import pytest
-from keel_broker_api.results import CancelOutcome
+from keel_broker_api.results import Balance, CancelOutcome
 from keel_core import telemetry
 
 from keel.data.cb_client import CoinbaseClient
@@ -205,6 +205,63 @@ def test_get_spot_returns_decimal() -> None:
     assert transport.calls["get_product"] == {"product_id": "BTC-USD"}
 
 
+# --- get_balances (the port's shape, #524) ------------------------------------------------
+
+
+def test_get_balances_answers_the_ports_type() -> None:
+    """`Balance`, not this client's account dicts.
+
+    The point of the method: `executor._fetch_available_quote` used to probe for a dict key OR an
+    attribute because it did not know whether it held this pre-port client or a real adapter.
+    Answering in the port's own type removes the fork -- one question, one shape, whichever kind
+    of broker is on the other end.
+    """
+    client = CoinbaseClient(FakeTransport(accounts=_load_fixture("cb_accounts.json")))
+
+    balances = client.get_balances()
+
+    assert all(isinstance(b, Balance) for b in balances)
+    btc = next(b for b in balances if b.currency == "BTC")
+    assert btc.available == Decimal("0.53219871")
+    usd = next(b for b in balances if b.currency == "USD")
+    assert usd.available == Decimal("1042.55")
+
+
+def test_get_balances_totals_available_plus_hold_like_the_adapter_does() -> None:
+    """Coinbase exposes no single "total" field, so both implementations compute it -- and they
+    must not disagree about what the word means while both exist.
+    `keel_broker_coinbase.adapter.get_balances` sums `available_balance` and `hold`; so does this.
+    """
+    transport = FakeTransport(
+        accounts={
+            "accounts": [
+                {
+                    "currency": "USD",
+                    "available_balance": {"value": "100.25"},
+                    "hold": {"value": "9.75"},
+                }
+            ]
+        }
+    )
+
+    balance = CoinbaseClient(transport).get_balances()[0]
+
+    assert balance.available == Decimal("100.25")
+    assert balance.total == Decimal("110.00")
+
+
+def test_get_balances_reraises_an_unreachable_venue() -> None:
+    """Rail 13 fails closed on the exception itself, so this must not swallow it -- the same
+    contract `get_accounts` keeps."""
+
+    class _Down:
+        def get_accounts(self, **_: object) -> object:
+            raise ConnectionError("venue unreachable")
+
+    with pytest.raises(ConnectionError):
+        CoinbaseClient(_Down()).get_balances()  # type: ignore[arg-type]
+
+
 # --- get_accounts -------------------------------------------------------------------------
 
 

tests/execution/test_executor.py

@@ -18,6 +18,7 @@
 from typing import Any
 
 import pytest
+from keel_broker_api.results import Balance
 from keel_core.subscription import SubscriptionStatus
 
 from keel.config import (
@@ -95,20 +96,26 @@ def __init__(
         # Ordered log of exchange interactions -- lets a test assert SEQUENCE, not just that a
         # call happened. `_roll_stop` must cancel before it places.
         self.events: list[str] = []
-        self.get_accounts_calls = 0
+        self.get_balances_calls = 0
 
-    def get_accounts(self) -> list[dict[str, Any]]:
-        self.get_accounts_calls += 1
+    def get_balances(self) -> list[Balance]:
+        """The port's shape since #524.
+
+        The "no balance known" case is an EMPTY LIST rather than a row carrying `None`. That is
+        not a shortcut around `Balance` requiring a `Decimal`: in the port's model a currency
+        with no account simply is not in the list, and `_fetch_available_quote` returns `None`
+        for it either way -- by falling off the loop instead of by testing a null field.
+        """
+        self.get_balances_calls += 1
         if self._balances is not None:
-            return [{"currency": c, "available_balance": b} for c, b in self._balances.items()]
-        if self._usdc_balance is None:
             return [
-                {"currency": "USD", "available_balance": None},
-                {"currency": "USDC", "available_balance": None},
+                Balance(currency=c, available=b, total=b) for c, b in self._balances.items()
             ]
+        if self._usdc_balance is None:
+            return []
         return [
-            {"currency": "USD", "available_balance": self._usdc_balance},
-            {"currency": "USDC", "available_balance": self._usdc_balance},
+            Balance(currency="USD", available=self._usdc_balance, total=self._usdc_balance),
+            Balance(currency="USDC", available=self._usdc_balance, total=self._usdc_balance),
         ]
 
     def preview_order(self, product_id: str, side: Any, order_configuration: dict) -> dict:
@@ -487,8 +494,8 @@ class _BrokerAccountsError:
     or placing.
     """
 
-    def get_accounts(self) -> list[dict[str, Any]]:
-        raise ConnectionError("simulated broker outage fetching accounts")
+    def get_balances(self) -> list[Balance]:
+        raise ConnectionError("simulated broker outage fetching balances")
 
     def preview_order(self, *args: Any, **kwargs: Any) -> Any:
         raise AssertionError("preview_order must not be called when the balance fetch failed")
@@ -504,7 +511,7 @@ def test_execute_fetches_the_available_quote_balance_for_a_buy_and_places(repo):
     result = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS)
 
     assert result.placed is True
-    assert broker.get_accounts_calls == 1
+    assert broker.get_balances_calls == 1
 
 
 def test_broker_balance_fetch_error_vetoes_the_buy_before_preview_or_place(repo):
@@ -551,7 +558,7 @@ def test_exit_signal_never_fetches_a_balance(repo):
     result = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS)
 
     assert result.placed is True
-    assert broker.get_accounts_calls == 0
+    assert broker.get_balances_calls == 0
 
 
 # -- autonomous mode: compliant -> placed without a prompt
@@ -2211,12 +2218,12 @@ def test_no_account_at_all_for_the_required_currency_fails_closed(repo):
 
 
 class _UnreachableBroker:
-    """A broker whose `get_accounts` raises as an offline HTTP stack does."""
+    """A broker whose `get_balances` raises as an offline HTTP stack does."""
 
     def __init__(self, exc: BaseException) -> None:
         self._exc = exc
 
-    def get_accounts(self) -> list[dict]:
+    def get_balances(self) -> list[Balance]:
         raise self._exc
 
 
@@ -2233,6 +2240,52 @@ def _quote_failure_payload(caplog, exc: BaseException) -> tuple[Decimal | None,
     return result, json.loads(formatter.format(records[0]))
 
 
+def test_the_quote_read_takes_available_and_never_total() -> None:
+    """**Rail 13 is about SPENDABLE funds, and `Balance` carries two numbers that differ.**
+
+    `available` is what the venue will let an order draw on; `total` includes funds on hold --
+    settling proceeds, collateral behind a resting order. Reading `total` would let rail 13 pass
+    an order the account cannot actually fund, which is the precise failure the rail exists to
+    prevent, and it would do it silently because both numbers are plausible balances.
+
+    Pinned with a balance whose two figures DIFFER. Every other fake in this file sets them
+    equal, so before this test existed, swapping `.available` for `.total` in
+    `_fetch_available_quote` passed the entire suite -- verified by making that change.
+    """
+    from keel.execution.executor import _fetch_available_quote
+
+    class _OnHold:
+        def get_balances(self) -> list[Balance]:
+            # 900 of the 1000 is on hold: spendable is 100.
+            return [Balance(currency="USD", available=Decimal("100"), total=Decimal("1000"))]
+
+    assert _fetch_available_quote(_OnHold(), "USD") == Decimal("100")
+
+
+def test_the_quote_read_matches_the_currency_case_insensitively() -> None:
+    """Venues disagree about casing and the product's quote leg is derived from a product id.
+    A `usd` row must satisfy a `USD` question -- the same comparison `gather_holdings` makes."""
+    from keel.execution.executor import _fetch_available_quote
+
+    class _Lowercase:
+        def get_balances(self) -> list[Balance]:
+            return [Balance(currency="usd", available=Decimal("42"), total=Decimal("42"))]
+
+    assert _fetch_available_quote(_Lowercase(), "USD") == Decimal("42")
+
+
+def test_the_quote_read_answers_none_when_the_currency_has_no_account() -> None:
+    """`None` means UNKNOWN and rail 13 fails closed on it. In the port's model a currency with
+    no account simply is not in the list -- there is no row carrying a null balance to inspect."""
+    from keel.execution.executor import _fetch_available_quote
+
+    class _NoUsd:
+        def get_balances(self) -> list[Balance]:
+            return [Balance(currency="EUR", available=Decimal("500"), total=Decimal("500"))]
+
+    assert _fetch_available_quote(_NoUsd(), "USD") is None
+
+
 def test_quote_fetch_logs_an_unreachable_venue_as_a_warning(caplog) -> None:
     exc = type("ConnectionError", (Exception,), {})("api.coinbase.com unreachable")