fix(broker-alpaca): NaN-safe money coercion, host-map-only construction, quoted path segments - #383
Closed
eaitbrahim wants to merge 3 commits into
Closed
fix(broker-alpaca): NaN-safe money coercion, host-map-only construction, quoted path segments#383eaitbrahim wants to merge 3 commits into
eaitbrahim wants to merge 3 commits into
Conversation
…r, conformance green Phase A of the keel-broker-alpaca PRD (#369): `packages/keel-broker-alpaca`, an original implementation of the `keel-broker-api` port against Alpaca's publicly documented Trading + Market Data REST APIs (no `alpaca-py`, no third-party adapter code — raw REST over an injected `requests` transport, the keel-broker-robinhood convention). Zero changes under `keel/`. FR-by-FR coverage: - FR-1 Package: workspace-pinned pyproject (`keel-core==0.9.3`, `keel-broker-api==0.9.3`), `py.typed`, registered under `keel.brokers` as `alpaca`, wired into the dev group (optional venue, like robinhood), mypy strict from birth, Dependabot + the pip-audit export exclusion. - FR-2 Venue identity: `venue = "alpaca"`, USD quotes, `asset_classes = {"equity"}` (the port's keel-side vocabulary). - FR-3 Orders: all four port kinds — `market_ioc_quote` maps to Alpaca's notional market order (`notional` + `type: market` + `time_in_force: day`, as the docs require), `market_ioc_base` to fractional `qty` market, `limit_gtc`/`stop_limit_gtc` to `gtc` limit/stop-limit. Fractional sizes render positionally (`format(d, "f")`), never scientific notation. `extended_hours: false` pinned on every body. - FR-4 Preview: Alpaca has no preview endpoint, so the preview is SYNTHESIZED — the keel-broker-robinhood precedent for venues without one (Robinhood's `synthesizes_preview=True` / `supports_native_preview=False` declaration, its "no endpoint that validates the order is a quote" reasoning). The book read is `GET /v2/stocks/{sym}/quotes/latest`; `best_bid`/`best_ask` ride in `Preview.detail` for the #332 warning and the #350 spread gate, and pricing uses only the crossed side (ask for buys, bid for sells). `ap`/`bp` = 0 is the documented "no active side" and lands in `Preview.errors`, never as a price. - FR-5 Market data: `15Min`/`1Hour`/`1Day` → FIFTEEN_MINUTE/ONE_HOUR/ONE_DAY, all other granularities refused with `ValueError` (the port's sanctioned refusal). Pagination follows `next_page_token` (bounded, like the sibling transports). The data tier is a DECLARED capability: constructor-validated `iex|sip`, sent on every data request — never the venue's silent `sip` default. - FR-6 Balances/positions: USD row with `available = min(buying_power, cash)` — on a cash account (`multiplier == 1`) the gap between the two is exactly the unsettled T+1 proceeds, surfaced honestly; long positions map `qty_available` → available, `qty` → total; short rows are skipped (long-only by construction). - FR-7 Fees: commission $0; sells modelled with the regulatory pass-throughs in `fees.py` as provenance-commented constants — SEC Section 31 $22.90/$1M (Alpaca's own regulatory-fees page; SEC advisory 2026-2 moves it to $20.60/$1M as of 2026-04-04 — recorded as a re-measurement point), FINRA TAF $0.000166/share capped $8.30 (FINRA Schedule A §4(b)(7)). The model feeds `Preview.est_fee` on sells; buys are honestly zero. - FR-8 Conformance: the shared suite (`keel_broker_api.conformance.suite.BrokerConformanceTests`) runs against the adapter via `tests/conformance/test_alpaca_conformance.py`, wired the same way the coinbase/robinhood/fake suites are (fixture-backed `FakeTransport`). Green. - FR-11 Rate limits/hosts: 429 retried honoring `Retry-After` when sent, exponential backoff otherwise, bounded attempt budget, then a typed `AlpacaAPIError`. Paper/live hosts come from an endpoint enum (`TRADING_HOSTS`) — there is no URL-shaped parameter for the trading host, so a paper configuration cannot reach the live host; tested structurally. Declared capability gaps (also in the package README): - Bracket/OCO and stop-market are NOT declared: the port's `OrderSpec` has no bracket concept and no stop-market kind, and this adapter does not invent venue-side order kinds the engine cannot ask for. `MarketOnOpen`/`MarketOnClose` likewise (available at the venue, unused, unexpressible in the port). - `supports_fee_summary` is false: the Trading API publishes no fee tiers, no fees-paid total, no volume window — a fabricated `FeeSummary` would read as coverage (the #197 lesson). - FR-10 corporate actions: bars are requested split-adjusted (`transport.BAR_ADJUSTMENT = "split"` so a cached series can state its policy); announcement consumption and dividend-purification recording are Phase B. - FR-9 session awareness is wired ahead of the rails: `is_market_open()` reads the venue's `/v2/clock` (no local calendar); extended hours are off by posture. Also fixes a pre-existing red on main introduced by #380: the README trademark sentence "not affiliated with, endorsed by, or sponsored by" trips `test_no_document_claims_a_review_has_occurred`'s naive "endorsed by" substring scan (verified failing on a clean 768585d tree). Reworded to "no affiliation with / no endorsement from / no sponsorship from" — the disclaimer is unchanged in strength, the trust-scanner stays strict. Fixes #369
…on, quoted path segments
Adversarial-review findings on the new keel-broker-alpaca package:
1. _decimal_or_none now rejects non-finite Decimals explicitly: JSON NaN/Infinity
tokens arrive via parse_constant (not parse_float) and Decimal(str(...)) parses
them without raising, so the except never fired -- NaN crashed preview_order
(bid > 0) and get_balances (min()), Infinity compared as a real price, and NaN
silently poisoned Candle OHLCV. Non-finite now lands in the same None path as
any unparseable leaf, keeping the preview's errors invariant true.
2. Removed the dead trading_host=/data_host= constructor escapes (zero callers):
TRADING_HOSTS is now the only map from an environment choice to a host, making
the README's no-paper-to-live-path claim literally true of the signature.
3. Percent-encode interpolated path segments (quote(..., safe='')) at every site:
/v2/orders/{id} on GET/DELETE and /v2/stocks/{symbol}/bars|quotes/latest, so a
'/' or '?' inside an id or symbol can never reshape the request.
4. to_unix_seconds refuses offset-less timestamps (ValueError) instead of letting
.timestamp() silently assume the host's local zone.
5. Reworded the SEC Section 31 provenance: advisory 2026-2 (.60/M) is already
in force (2026-04-04), Alpaca's page is the stale side, and the encoded .90
deliberately tracks the venue's own figure (~/bin/zsh.02/k, conservative) with the
delta as the re-measurement trigger.
All tests written red-first; gates: 3110 passed/1 skipped, ruff clean, mypy clean.
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #382 (merged pre-review): the adversarial review's five findings, applied TDD red-first.
_decimal_or_noneparsed'nan'/'Infinity'successfully (JSONNaNarrives viaparse_constant), crashingpreview_orderatbid > 0andquote_size / crossed,get_balancesatmin(buying_power, cash), and silently poisoning Candle OHLCV — contradicting the preview docstring's populate-errorsinvariant. Nowis_finite()-guarded; three red-first tests cover quote sides, balances, and bar leaves.trading_host=/data_host=overrides (zero callers anywhere) are REMOVED;TRADING_HOSTSis the only endpoint→host map and the README's "no configuration path from a paper credential to the live host, by construction" is now literally true (pinned by test).safe="") at all four sites;A/B-USDand a crafted order id can no longer reshape the request (asserted on the outgoing URL).ValueErrorper the module's refusal convention.Gates:
uv run pytest -q→ 3110 passed, 1 skipped; ruff clean; mypy clean (249 files).Refs #369 (Phase A hardening).