diff --git a/packages/keel-broker-alpaca/README.md b/packages/keel-broker-alpaca/README.md index 14cc63ae..e53bf9b2 100644 --- a/packages/keel-broker-alpaca/README.md +++ b/packages/keel-broker-alpaca/README.md @@ -9,8 +9,10 @@ from any third-party Alpaca adapter. "Alpaca" appears here solely to identify wh package talks to. US equities, cash account, long-only, regular session. Paper and live are separate -hosts selected by an explicit `endpoint` choice — there is no configuration path from a -paper credential to `https://api.alpaca.markets`, by construction. +hosts selected by an explicit `endpoint` choice — `transport.TRADING_HOSTS` is the only +construction path to a trading host (no constructor parameter accepts a host URL), so +there is no configuration path from a paper credential to +`https://api.alpaca.markets`, by construction. ## What works @@ -30,11 +32,12 @@ paper credential to `https://api.alpaca.markets`, by construction. Commission is $0. Sells carry regulatory pass-throughs, modelled in `fees.py` with the rates as provenance-commented constants: -- **SEC Section 31**: $22.90 per $1,000,000 of sale proceeds — Alpaca's own - regulatory-fees page ($27.80 previously; the SEC adjusts the rate periodically, and - its advisory 2026-2 moves it to $20.60 per $1M as of 2026-04-04 — a documented - re-measurement point, encoded as the venue's published figure until Alpaca's page - moves). +- **SEC Section 31**: $22.90 per $1,000,000 of sale proceeds — the figure Alpaca's own + regulatory-fees page still publishes. The SEC's advisory 2026-2 rate ($20.60 per $1M) + has been in force since 2026-04-04, so the venue's page is the stale side; the model + deliberately tracks what the venue itself charges, which over-states the statutory + rate by ~$0.02 per $10k (conservative for a sell preview's proceeds), and that delta + is the re-measurement trigger for when Alpaca's page updates. - **FINRA TAF**: $0.000166 per share, capped at $8.30 per trade — the cap is on Alpaca's page; the per-share rate is FINRA Schedule A §4(b)(7), in force since 2021-01-01. diff --git a/packages/keel-broker-alpaca/keel_broker_alpaca/adapter.py b/packages/keel-broker-alpaca/keel_broker_alpaca/adapter.py index 3583cbda..b23393a1 100644 --- a/packages/keel-broker-alpaca/keel_broker_alpaca/adapter.py +++ b/packages/keel-broker-alpaca/keel_broker_alpaca/adapter.py @@ -482,13 +482,25 @@ def _decimal_or_none(value: Any) -> Decimal | None: transport already parses unquoted numbers as `Decimal`, and `Decimal(str(value))` here lands both shapes on the same exact number. `None` rather than zero: an absent number and a zero number must never be the same value at a preview gate. + + Non-finite values are `None` too, and the check is explicit because the `except` + below never fires for them: JSON's `NaN`/`Infinity` tokens arrive via + `parse_constant` (not `parse_float`) as `float("nan")`/`float("inf")`, and + `Decimal(str(...))` parses BOTH without raising -- `Decimal("NaN")` then crashes any + ordering comparison (`bid > 0`, `min(buying_power, cash)`) and `Decimal("Infinity")` + compares as a real price. A non-finite number is not a money value; refusing it here + is what keeps the preview's "every path that could not price the order populates + `errors`" invariant true for these rows too. """ if value is None or isinstance(value, bool): return None try: - return Decimal(str(value)) + parsed = Decimal(str(value)) except (InvalidOperation, ValueError, TypeError): return None + if not parsed.is_finite(): + return None + return parsed def _terminal_unknown(order_id: str) -> OrderStatus: diff --git a/packages/keel-broker-alpaca/keel_broker_alpaca/fees.py b/packages/keel-broker-alpaca/keel_broker_alpaca/fees.py index 6c9bf6b6..de745099 100644 --- a/packages/keel-broker-alpaca/keel_broker_alpaca/fees.py +++ b/packages/keel-broker-alpaca/keel_broker_alpaca/fees.py @@ -9,12 +9,15 @@ Provenance (all read 2026-08-17): -* **SEC Section 31 fee** -- charged on SELLS, per $1,000,000 of principal. Alpaca's own - regulatory-fees page (https://alpaca.markets/support/regulatory-fees) states the - current rate as $22.90 per $1M ($27.80 previously). The SEC adjusts this rate - periodically by fee-rate advisory (advisory 2026-2 moves it to $20.60 per $1M as of - 2026-04-04); the venue's published figure is the one encoded, and the drift is a - documented re-measurement point, not a silent correction. +* **SEC Section 31 fee** -- charged on SELLS, per $1,000,000 of principal. The SEC's + advisory 2026-2 rate ($20.60 per $1M) took effect 2026-04-04 and IS the rate in force; + Alpaca's own regulatory-fees page (https://alpaca.markets/support/regulatory-fees) + still publishes $22.90 ($27.80 previously) -- the venue's page is the stale side of + the two. The encoded $22.90 deliberately tracks what the venue itself charges, which + over-states the statutory rate by ~$0.02 per $10k -- the conservative direction for a + sell preview's proceeds -- and that delta is the documented re-measurement trigger: + when Alpaca's page moves to the advisory figure, the constant and this provenance + move with it. * **FINRA Trading Activity Fee (TAF)** -- charged on SELLS, per share, capped per trade. The cap ($8.30 for equities) is on Alpaca's page above; the per-share rate ($0.000166) is FINRA's, Schedule A to the FINRA By-Laws §4(b)(7) (SR-FINRA-2020-032, in force since diff --git a/packages/keel-broker-alpaca/keel_broker_alpaca/translate.py b/packages/keel-broker-alpaca/keel_broker_alpaca/translate.py index ba87de0e..37de2c89 100644 --- a/packages/keel-broker-alpaca/keel_broker_alpaca/translate.py +++ b/packages/keel-broker-alpaca/keel_broker_alpaca/translate.py @@ -224,8 +224,21 @@ def to_unix_seconds(value: str) -> int: Venue timestamps can carry fractional seconds and (per the schema) explicit offsets; fractional seconds truncate because `Candle.ts` is whole seconds and a bar's open time is second-aligned anyway. + + An offset-less timestamp is REFUSED, never read as local time: without an offset + `fromisoformat` yields a naive datetime whose `.timestamp()` silently assumes the + host's zone, so the same bar would timestamp differently per machine. `ValueError` + is this module's refusal signal (`to_timeframe`'s), and refusing is the fail-closed + direction -- the venue's contract sends `Z` or an explicit offset, so anything else + is garbage, not a zone to guess. """ - return int(datetime.fromisoformat(value.replace("Z", "+00:00")).timestamp()) + parsed = datetime.fromisoformat(value.replace("Z", "+00:00")) + if parsed.tzinfo is None: + raise ValueError( + f"alpaca timestamp {value!r} carries no UTC offset; refusing to read a naive " + "datetime as local time" + ) + return int(parsed.timestamp()) __all__ = [ diff --git a/packages/keel-broker-alpaca/keel_broker_alpaca/transport.py b/packages/keel-broker-alpaca/keel_broker_alpaca/transport.py index f728d92c..c494bca9 100644 --- a/packages/keel-broker-alpaca/keel_broker_alpaca/transport.py +++ b/packages/keel-broker-alpaca/keel_broker_alpaca/transport.py @@ -133,11 +133,11 @@ def get_latest_quote(self, symbol: str, feed: str) -> Any: ... class AlpacaTransport: """The live, network-backed `Transport`: header-authed JSON over HTTPS. - The trading host is derived from `endpoint` and cannot be overridden -- a paper - configuration must be structurally unable to reach the live venue. The `trading_host` - and `data_host` constructor parameters exist for tests pointing at a local recorder; - they are explicit escapes, not a configuration surface, and nothing in this workspace - passes them in production code. + The trading host is derived from `endpoint` and NOTHING else: the constructor accepts + no host URL of any kind (there were `trading_host`/`data_host` keyword escapes here + once -- zero callers, dead surface, removed), so `TRADING_HOSTS` is the only map from + an environment choice to a host and a paper credential cannot be pointed at the live + venue by any configuration path. """ def __init__( @@ -150,8 +150,6 @@ def __init__( timeout: float = 10.0, max_attempts: int = 3, sleep: Callable[[float], None] = time.sleep, - trading_host: str | None = None, - data_host: str | None = None, ) -> None: if endpoint not in TRADING_HOSTS: raise ValueError( @@ -171,8 +169,8 @@ def __init__( self._timeout = timeout self._max_attempts = max_attempts self._sleep = sleep - self._trading_host = trading_host if trading_host is not None else TRADING_HOSTS[endpoint] - self._data_host = data_host if data_host is not None else DATA_HOST + self._trading_host = TRADING_HOSTS[endpoint] + self._data_host = DATA_HOST @property def endpoint(self) -> str: @@ -273,6 +271,9 @@ def _request_json( URL per request, so a recorded call is exactly what the venue received. Like the Robinhood transport, `quote_via=quote` (not `quote_plus`) so a `+` in a value is never decoded server-side as a space, and `safe=""` so nothing rides unencoded. + The same discipline covers PATH segments: callers percent-encode every + interpolated id/symbol with `quote(..., safe="")`, so a `/` or `?` inside one can + never reshape the request into a different resource. `parse_float=Decimal`, never `response.json()`: this venue mixes quoted and unquoted money fields (see the module docstring), and the parser is the only @@ -313,7 +314,7 @@ def get_order(self, order_id: str) -> Any: rather than launder a network blip into "this order does not exist". """ try: - return self._request_json("GET", f"/v2/orders/{order_id}") + return self._request_json("GET", f"/v2/orders/{quote(order_id, safe='')}") except AlpacaAPIError as exc: if exc.status_code == 404: return None @@ -328,7 +329,8 @@ def cancel_order(self, order_id: str) -> Any: filled) are returned as statuses rather than raised because both are ordinary answers the adapter maps to `False`; every other failure raises. """ - response = self._send("DELETE", f"{self._trading_host}/v2/orders/{order_id}") + path = f"/v2/orders/{quote(order_id, safe='')}" + response = self._send("DELETE", f"{self._trading_host}{path}") status = int(getattr(response, "status_code", 0)) if status < 400 or status in (404, 422): return status @@ -355,7 +357,10 @@ def get_bars( if page_token is not None: params["page_token"] = page_token return self._request_json( - "GET", f"/v2/stocks/{symbol}/bars", host=self._data_host, params=params + "GET", + f"/v2/stocks/{quote(symbol, safe='')}/bars", + host=self._data_host, + params=params, ) def get_latest_quote(self, symbol: str, feed: str) -> Any: @@ -365,7 +370,10 @@ def get_latest_quote(self, symbol: str, feed: str) -> Any: signal the adapter treats as "no book on that side", not a price of zero. """ return self._request_json( - "GET", f"/v2/stocks/{symbol}/quotes/latest", host=self._data_host, params={"feed": feed} + "GET", + f"/v2/stocks/{quote(symbol, safe='')}/quotes/latest", + host=self._data_host, + params={"feed": feed}, ) diff --git a/tests/broker_alpaca/test_adapter.py b/tests/broker_alpaca/test_adapter.py index 644badcf..3fe2a2e6 100644 --- a/tests/broker_alpaca/test_adapter.py +++ b/tests/broker_alpaca/test_adapter.py @@ -260,6 +260,18 @@ def test_an_unknown_endpoint_is_refused_at_construction(self) -> None: with pytest.raises(ValueError, match="endpoint"): AlpacaAdapter(endpoint=bad) + def test_no_constructor_parameter_accepts_a_host_url(self) -> None: + """The `trading_host`/`data_host` keyword escapes are GONE, so the documented + endpoint-to-host map is the only construction path to a trading host: no + parameter accepts a host URL at all, which is what makes the README's "no + configuration path from a paper credential to the live host, by construction" + literally true (FR-11). `TypeError` is Python's own "no such keyword" answer -- + there is nothing to validate because there is nothing to pass.""" + with pytest.raises(TypeError): + AlpacaTransport("key-id", "secret", trading_host=LIVE_TRADING_HOST) # type: ignore[call-arg] + with pytest.raises(TypeError): + AlpacaTransport("key-id", "secret", data_host="https://example.test") # type: ignore[call-arg] + def test_the_data_tier_is_a_declared_choice_not_an_assumption(self) -> None: """IEX (free) vs SIP is a declared capability (FR-5): the adapter names its feed on every market-data request instead of letting the venue default it, because the @@ -327,6 +339,22 @@ def test_a_short_position_row_is_not_reported_as_a_holding(self) -> None: balances = AlpacaAdapter(transport).get_balances() assert [b.currency for b in balances] == ["USD"] + def test_a_nonfinite_buying_power_is_handled_not_a_crash(self) -> None: + """A NaN `buying_power` arrives as `float("nan")` (the `parse_constant` path, not + `parse_float`), and `min()` over a NaN `Decimal` raises. The existing + balances convention for a money field that cannot be parsed is the same as an + absent one -- read as zero via the `or Decimal("0")` every balance field carries + -- so a garbage spendable figure never reaches a `Balance` row and nothing + raises.""" + account = load_fixture("alpaca_account.json") + account["buying_power"] = float("nan") + + balances = AlpacaAdapter(FakeTransport(account=account)).get_balances() + + usd = {b.currency: b for b in balances}["USD"] + assert usd.available == Decimal("0"), "an unparseable buying power reads as zero" + assert usd.total == Decimal("102086.50"), "the parseable cash figure is untouched" + # --------------------------------------------------------------------------------------------- # Candles (FR-5, FR-10's adjusted/raw policy) @@ -367,6 +395,24 @@ def test_every_unsupported_granularity_is_refused(self) -> None: with pytest.raises(ValueError, match="timeframe"): adapter.get_candles(_PRODUCT, granularity, 0, 86_400) + def test_a_nonfinite_bar_value_is_never_stored_in_a_candle(self) -> None: + """A NaN high arrives as `float("nan")`, and `Decimal("NaN")` is TRUTHY -- so + without an explicit finiteness check the `or Decimal("0")` fallback never fires + and the NaN rides into `Candle.high` silently, poisoning every indicator that + touches the series. The module's existing convention for an unparseable bar leaf + is the same as an absent one: read as zero, never as the venue's garbage.""" + bars = load_fixture("alpaca_bars_page1.json") + bars["bars"][0]["h"] = float("nan") + bars["next_page_token"] = None + + candles = AlpacaAdapter(FakeTransport(bars_pages=[bars])).get_candles( + _PRODUCT, Granularity.ONE_DAY, 1_700_000_000, 1_700_086_400 + ) + + assert len(candles) == 2, "both fixture bars survive; only the NaN leaf changes" + assert candles[0].high == Decimal("0"), "the unparseable-leaf-reads-as-zero rule" + assert candles[0].close == Decimal("131.9"), "parseable leaves are untouched" + # --------------------------------------------------------------------------------------------- # Preview: synthesized from the book (FR-4, FR-7) @@ -447,6 +493,26 @@ def test_a_quote_with_no_active_ask_leaves_the_buy_unpriced_and_says_so(self) -> assert preview.errors, "an unpriced leg must appear in errors" assert any("ask" in e for e in preview.errors) + def test_a_nonfinite_quote_side_is_unpriced_and_reported_never_a_crash(self) -> None: + """JSON `NaN`/`Infinity` tokens ride `parse_constant`, not `parse_float`, so they + reach the adapter as `float("nan")`/`float("inf")` -- and `Decimal(str(...))` + parses BOTH without raising, which means the `except` in `_decimal_or_none` never + fires. `Decimal("NaN")` then crashes the `bid > 0` comparison and `Decimal( + "Infinity")` compares `> 0` as a real price; a non-finite side must land in the + same "no active side" path as a zero one -- `errors` says so, nothing raises -- + for the preview docstring's "every path that could not price the order populates + `errors`" invariant to hold.""" + quote = json.loads('{"quote": {"bp": NaN, "ap": Infinity}}', parse_float=Decimal) + preview = AlpacaAdapter(FakeTransport(quote=quote)).preview_order( + MarketIOCByQuote(product_id=_PRODUCT, side=Side.BUY, quote_size=Decimal("100")) + ) + + assert preview.est_base_size == Decimal("0") + assert preview.errors, "a non-finite quote side must appear in errors" + assert any("ask" in e for e in preview.errors) + assert preview.detail["best_bid"] == "none" + assert preview.detail["best_ask"] == "none" + def test_a_non_usd_product_is_refused_before_any_request_is_made(self) -> None: transport = _full_transport() with pytest.raises(UnsupportedOrder, match="USD"): diff --git a/tests/broker_alpaca/test_translate.py b/tests/broker_alpaca/test_translate.py index 04a4c44e..6cd84ead 100644 --- a/tests/broker_alpaca/test_translate.py +++ b/tests/broker_alpaca/test_translate.py @@ -208,3 +208,13 @@ def test_rfc3339_and_epoch_round_trip() -> None: assert to_unix_seconds("2026-08-14T14:30:00.999Z") == ts # ...and an explicit offset must parse too, not only a trailing Z. assert to_unix_seconds("2026-08-14T10:30:00-04:00") == ts + + +def test_an_offset_less_timestamp_is_refused_never_read_as_local_time() -> None: + """`fromisoformat` without an offset yields a NAIVE datetime, and `.timestamp()` + silently assumes the host's local zone -- the same bar would timestamp differently + per machine. The fail-closed rule (the venue contract sends `Z` or an explicit + offset, so anything else is garbage): refuse with `ValueError`, this module's + refusal signal (`to_timeframe`'s), rather than guess the zone.""" + with pytest.raises(ValueError, match="offset"): + to_unix_seconds("2026-08-14T14:30:00") diff --git a/tests/broker_alpaca/test_transport.py b/tests/broker_alpaca/test_transport.py index 5ea7115c..37a73206 100644 --- a/tests/broker_alpaca/test_transport.py +++ b/tests/broker_alpaca/test_transport.py @@ -243,6 +243,42 @@ def test_get_positions_and_get_clock_use_their_documented_paths(http: Any) -> No assert recorder.calls[1]["url"] == f"{PAPER_TRADING_HOST}/v2/clock" +# --------------------------------------------------------------------------------------------- +# Path-segment encoding: a symbol or order id is always ONE segment +# --------------------------------------------------------------------------------------------- + + +def test_a_symbol_containing_a_path_separator_stays_one_segment(http: Any) -> None: + """A malformed product id (`A/B-USD`) survives `to_symbol` as `A/B`, and an unquoted + `/` would reshape `/v2/stocks/A/B/bars` into a DIFFERENT resource. Path segments are + percent-encoded with `safe=""` exactly like the query string, so the symbol rides as + `A%2FB` -- one segment, whatever it contains.""" + recorder = http([_FakeResponse(payload={"bars": []}), _FakeResponse(payload={"quote": {}})]) + transport = _transport() + + transport.get_bars("A/B", "1Day", "2026-08-01T00:00:00Z", "2026-08-14T00:00:00Z", "iex") + transport.get_latest_quote("A/B", "iex") + + assert urlsplit(recorder.calls[0]["url"]).path == "/v2/stocks/A%2FB/bars" + assert urlsplit(recorder.calls[1]["url"]).path == "/v2/stocks/A%2FB/quotes/latest" + + +def test_an_order_id_with_query_or_traversal_characters_stays_one_segment(http: Any) -> None: + """A `?` in an order id would open a query string and `../` would walk out of + `/v2/orders/`; both are data in an opaque id, so both ride percent-encoded (`%3F`, + `%2F`) and the request is never reshaped -- there is no query part at all.""" + recorder = http([_FakeResponse(payload={"id": "o1"}), _FakeResponse(204)]) + transport = _transport() + + transport.get_order("o1?x=../evil") + transport.cancel_order("o1?x=../evil") + + expected = f"{PAPER_TRADING_HOST}/v2/orders/o1%3Fx%3D..%2Fevil" + assert recorder.calls[0]["url"] == expected + assert recorder.calls[1]["url"] == expected + assert all(urlsplit(call["url"]).query == "" for call in recorder.calls) + + # --------------------------------------------------------------------------------------------- # Response handling: money as Decimal, the 404 sentinel, cancel statuses # ---------------------------------------------------------------------------------------------