feat: add Tastytrade broker (equities, options, multileg spreads) - #1010
Open
3452sdfgsdf wants to merge 9 commits into
Open
feat: add Tastytrade broker (equities, options, multileg spreads)#10103452sdfgsdf wants to merge 9 commits into
3452sdfgsdf wants to merge 9 commits into
Conversation
added 6 commits
May 4, 2026 20:53
Adds an initial Tastytrade broker built on the unofficial `tastytrade` Python SDK (12.4+). The SDK is async-only, so the adapter owns a daemon-thread asyncio bridge and dispatches all SDK calls via run_coroutine_threadsafe. Working in this commit: - OAuth auth (provider_secret + refresh_token, sandbox/prod) - Account, balance, and position queries - Order cancellation Stubbed (logged warnings, follow-ups): - Order submission, modification, and parsing - Multileg orders - Streaming (polling/AlertStreamer) - Market data (chains/quotes/historical via DXLink) Includes offline smoke tests (3 passing) plus a live sandbox smoke test gated on TASTYTRADE_* env credentials.
Replaces the order-related stubs from the initial scaffold with real implementations against the tastytrade SDK. Order submission: - _submit_order: equity + single-leg equity options (limit/market/stop/ stop_limit). Builds a tastytrade.order.Leg and NewOrder and dispatches via account.place_order through the asyncio bridge. Advanced (OCO/OTO/ bracket) orders still log a clear warning and return None — those need NewComplexOrder and will land in a follow-up. - _submit_orders: multileg path (credit/debit/even/limit/market). Builds one NewOrder containing all legs, validates a shared underlying, sends a positive price (sign comes from leg actions), and returns a parent Order with child orders attached. Parent order_type is normalized to Lumibot's allowed values (LIMIT/MARKET) since credit/debit/even are wire-level pricing modes, not Lumibot OrderType members. Mapping helpers: - _to_occ_symbol: Lumibot Asset -> 21-char OCC option symbol with 6-char left-padded root (e.g. 'AAPL 260717C00230000'). - _occ_to_asset: inverse parser for read-back. - _lumi_side_to_tt_action / _lumi_order_type_to_tt / _lumi_tif_to_tt: enum bridges between Lumibot and tastytrade.order. Parsing + read-back: - _parse_broker_order: PlacedOrder -> Lumibot Order, with parent+child expansion for multileg orders. Maps the full Tastytrade OrderStatus enum to Lumibot's status taxonomy. - _pull_broker_order: account.get_order(identifier). - _pull_broker_all_orders: account.get_live_orders for the working set. Tests (12 offline pass + live sandbox still gated on creds): - OCC formatting + round-trip - Side mapping (equity vs option) - Equity limit submission (verifies Leg + NewOrder shape) - Option limit submission (verifies OCC symbol on the leg) - Multileg credit spread (verifies absolute price + leg action mix) - Multileg mixed-underlying rejection - Single-leg + multileg PlacedOrder parsing Modify, advanced orders, streaming, and market data remain stubbed — follow-ups.
Wires the broker into Lumibot's strategy executor by adding the
PollingStream-based event loop and replacing the modify-order stub
with a real implementation.
Polling stream:
- _get_stream_object returns PollingStream(polling_interval) with
polling_interval exposed on the constructor (default 5.0s, matching
Tradier).
- _register_stream_events binds POLL/NEW/FILLED/CANCELED/ERROR to
_process_trade_event so the executor sees order lifecycle events.
- _run_stream calls _stream_established then runs the polling loop.
- do_polling pulls live orders, parses each, and dispatches transitions:
submitted/open -> NEW_ORDER
fill -> FILLED_ORDER (with weighted avg fill price + qty
from the PlacedOrder's leg fills)
canceled -> CANCELED_ORDER
error -> ERROR_ORDER (reject_reason if available)
Tracked-but-no-longer-at-broker orders are dispatched as cancelled.
Partial fills are intentionally not dispatched in polling mode (same
rationale as Tradier: polling will routinely miss them).
Order modification:
- _modify_order rebuilds the leg + NewOrder with the new limit/stop and
calls account.replace_order. Tastytrade replace returns a new id, so
we update the local Order's identifier and price fields.
Tests (16 offline pass + live sandbox still gated):
- _get_stream_object returns a PollingStream with the right interval
- _modify_order calls replace_order with the updated price and absorbs
the broker's new id back onto the local order
- _avg_fill_from_legs computes a size-weighted average across leg fills
- _avg_fill_from_legs returns None when nothing has filled yet
Native AlertStreamer + DXLinkStreamer integration and TastytradeData
market-data methods remain TODO.
Replaces the warning stubs in TastytradeData with REST-based
implementations against the tastytrade SDK's market_data and
instruments modules.
- get_last_price: fetch a market-data snapshot via
market_data.get_market_data and return mid -> last -> mark, falling
back to (bid+ask)/2 if those are unset.
- get_quote: same fetch, populated into a Lumibot Quote with
bid/ask/mid_price/price/sizes/volume/timestamp and the raw payload
preserved on raw_data['tt_market_data'].
- get_chains: pivot instruments.get_option_chain's
dict[date, list[Option]] response into Lumibot's nested
{"Multiplier": 100, "Chains": {"CALL": {date: [strikes]}, "PUT": ...}}
shape, with strikes sorted per expiration.
Both calls go through the broker's asyncio bridge when one is wired in
(broker passes its _AsyncBridge.run as the runner kwarg). Standalone
construction falls back to asyncio.run.
OCC option-symbol formatting is reused from the broker
(Tastytrade._to_occ_symbol). Index assets are also accepted (mapped to
InstrumentType.INDEX).
Tests (20 offline pass + live sandbox still gated):
- get_last_price prefers mid over last/mark
- get_last_price falls back to (bid+ask)/2 when mid/last/mark all None
- get_quote populates bid/ask/mid/price/sizes/volume/timestamp
- get_chains pivots TT chain to Lumibot's nested shape with sorted
strikes and lowercase-tolerant option_type matching
get_historical_prices remains a stub — needs the DXLink streamer or a
separate historical-bar source.
Three issues surfaced when running the broker against a live Tastytrade production account (read-only paths only — balances, positions, market data, option chains). 1. _strategy_name collision: I had defined _strategy_name as a static helper, but Broker base class already uses self._strategy_name as a string attribute (set in Broker.__init__). The override broke _pull_positions with 'str object is not callable'. Drop the helper and use the existing self._strategy_name_from_input(strategy) classmethod from the base class. 2. Equity-option positions were skipped: the scaffold _tt_position_to_asset only handled instrument_type == 'equity' and warned on options. Now that the broker has _occ_to_asset, parse equity options into proper Lumibot Asset(OPTION, expiration, strike, right) instances. Verified against a real account holding -2 TQQQ 2026-05-08 $63.50 calls — parses correctly with negative qty for the short position. 3. Live smoke test was being skipped by the apitest gating in the repo's conftest.py (which requires Polygon/Theta creds to run any apitest- marked test). The Tastytrade live test only needs Tastytrade creds, so drop the apitest marker. The skipif on TASTYTRADE_* env vars still gates it correctly. Validated live (read-only) against production: - OAuth token refresh - Account.get + Account.get_balances - Account.get_positions (equity + equity options) - market_data.get_market_data (single SPY last price = real) - instruments.get_option_chain (SPY chain, multiple expirations) Tastytrade's market-data REST is aggressively rate-limited (429 on two back-to-back calls). For active polling, callers should debounce or wait for native DXLinkStreamer integration.
Two real-API failures uncovered when running place/replace/cancel
end-to-end against a live Tastytrade production account.
1. cant_buy_for_credit error on a BUY:
Tastytrade encodes credit/debit in the SIGN of NewOrder.price
(negative = debit, positive = credit) and the SDK serializer strips
abs() before sending while emitting price-effect separately. Sending
a positive price for a BUY made Tastytrade interpret it as
"buy for a credit" and reject the order.
Fix: add _is_debit_action + _sign_single_leg_price helpers and apply
the right sign in _submit_order / _modify_order. For multileg, sign
the price by order_type_norm: 'credit' -> positive, 'debit' ->
negative, 'even' -> 0, 'limit' -> infer from leg net direction
(raise on mixed-action limit since the sign is ambiguous).
2. order_legs.action: is invalid on an equity BUY:
Tastytrade's standard order endpoint rejects plain ``Buy`` / ``Sell``
on equity legs even though those values exist in the OrderAction
enum (they're for special cases like notional market orders).
Equity legs need the explicit open/close form.
Fix: map Lumibot equity sides to Tastytrade open/close actions:
BUY -> BUY_TO_OPEN
SELL -> SELL_TO_CLOSE
SELL_SHORT -> SELL_TO_OPEN
BUY_TO_COVER -> BUY_TO_CLOSE
Plus pass-through for explicit *_TO_* sides.
Validated live (read+write) against a production IRA, market closed,
limit price far from market so it cannot fill:
STEP 1 place BUY 1 SPY @ $1.00 LIMIT DAY -> id 463096620, Received
STEP 2 replace $1.00 -> $1.50 -> id 463096622, Received
STEP 3 cancel -> id 463096622, Cancelled
The new id on replace is Tastytrade's normal behavior (cancel-and-new);
_modify_order absorbs the new id back onto the Lumibot Order.
Tests updated to match the new signed-price + open/close-action wire
shape. 20 offline pass.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a Tastytrade broker adapter and TastytradeData data source that wrap the asynchronous tastytrade SDK via a background asyncio bridge, implement order/position mapping and polling-based reconciliation, export the classes from package namespaces, add the SDK dependency, and include unit + optional live smoke tests. ChangesTastytrade Broker and Data Source Integration
Sequence DiagramsequenceDiagram
actor Client
participant Broker as Tastytrade Broker
participant Bridge as _AsyncBridge
participant SDK as Tastytrade SDK
participant DataSource as TastytradeData
participant Stream as PollingStream
Client->>Broker: submit_order(order)
Broker->>Broker: convert order (OCC, sides, types, TIF)
Broker->>Bridge: run(SDK.place_order(...))
Bridge->>SDK: execute coroutine
SDK-->>Bridge: order response
Bridge-->>Broker: return result
Broker->>Stream: dispatch NEW event
loop Polling Cycle
Stream->>Broker: do_polling()
Broker->>Bridge: run(SDK.get_live_orders)
Bridge->>SDK: execute coroutine
SDK-->>Bridge: live orders
Bridge-->>Broker: parsed orders
Broker->>Broker: reconcile statuses, compute fills
Broker->>Stream: dispatch FILLED/CANCELED/ERROR
end
Client->>DataSource: get_quote(asset)
DataSource->>Bridge: run(SDK.snapshot)
Bridge->>SDK: execute coroutine
SDK-->>Bridge: market data
Bridge-->>DataSource: return data
DataSource-->>Client: Quote
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Pylint (4.0.5)tests/conftest.py************* Module pylintrc ... [truncated 15483 characters] ... "module": "tests.conftest", tests/test_tastytrade_broker_smoke_apitest.py************* Module pylintrc ... [truncated 57444 characters] ... est_live_sandbox_balances_and_positions", Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lumibot/brokers/tastytrade.py`:
- Around line 103-107: The run method's call to future.result(timeout=...) can
raise (e.g., TimeoutError) but currently leaves the coroutine running on the
background loop; update TastytradeBroker.run (the run method) to cancel the
pending asyncio.Future when result() raises or times out: wrap
future.result(...) in try/except, and on any Exception (especially TimeoutError)
call future.cancel() (and optionally await/handle cancellation on the background
loop), then re-raise the original exception so the caller sees the failure while
ensuring the background task is cancelled.
- Around line 157-158: The current assignment if is_test is None and "SANDBOX"
in config: is_test = bool(config.get("SANDBOX")) wrongly treats string values
like "false" or "0" as True; change the logic in tastytrade.py where is_test is
set to explicitly parse string config values: when config.get("SANDBOX") is a
str, normalize to lower() and map accepted true tokens ("true","1","yes","on")
to True and false tokens ("false","0","no","off") to False, otherwise for
non-str values use the existing bool() conversion; update the is_test assignment
and related branch to use this parsed result.
- Around line 719-752: _detect multileg orders early in _modify_order and reject
them instead of rebuilding a single leg: before calling self._build_leg(order)
check the parent order for multiple legs (e.g., inspect order.legs or an
equivalent attribute/collection on the Order object) and if len(legs) > 1 log an
error or raise ValueError and return (do not proceed to build or replace);
update the error message to mention multileg replacements are not supported and
reference _modify_order and _build_leg so the failure is immediate and explicit
for spread orders.
- Around line 840-857: The equity parse path in _leg_to_lumi_side currently only
maps plain "buy"/"sell", so explicit Tastytrade equity actions like "buy to
open", "sell to open", "buy to close", and "sell to close" fall through to the
default BUY; update the non-option mapping in _leg_to_lumi_side to include those
explicit action strings and map them to the correct Order.OrderSide (e.g., any
"buy ..." -> Order.OrderSide.BUY, any "sell ..." -> Order.OrderSide.SELL) using
the same constants already used in the function so explicit equity read-backs
are parsed correctly.
- Around line 965-969: The bug is a race where orders that are reported
filled/rejected by get_live_orders() are dispatched via
_safe_stream_dispatch(FILLED_ORDER/ERROR_ORDER) but stored.status isn’t updated
synchronously, so on the next poll the missing-order check (which uses
order.is_active()) misclassifies them as canceled; to fix, when you detect a
filled or error order in _pull_broker_all_orders/_process_live_orders (where you
call _safe_stream_dispatch with FILLED_ORDER or ERROR_ORDER), immediately update
the stored order state (stored.status = OrderStatus.FILLED or OrderStatus.ERROR
or mark stored as inactive) before returning/continuing so order.is_active()
reflects the new state and the missing-order branch won’t treat it as canceled;
reference _pull_broker_all_orders, get_live_orders, _safe_stream_dispatch,
FILLED_ORDER, ERROR_ORDER, stored.status, and order.is_active when applying the
change.
In `@tests/test_tastytrade_broker_smoke_apitest.py`:
- Around line 653-665: Update the test decorator to mark it as an API smoke test
(add `@pytest.mark.apitest` above the existing `@pytest.mark.skipif`) and
instantiate the broker in sandbox mode by passing is_test=True to Tastytrade
(modify the broker creation in test_live_sandbox_balances_and_positions from
Tastytrade(connect_stream=False) to Tastytrade(connect_stream=False,
is_test=True)) so the test is explicitly opt-in and guaranteed to hit the
sandbox; keep the existing skipif credential guard intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e9aa9857-17fe-497d-985a-d1193bab592d
📒 Files selected for processing (6)
lumibot/brokers/__init__.pylumibot/brokers/tastytrade.pylumibot/data_sources/__init__.pylumibot/data_sources/tastytrade_data.pyrequirements.txttests/test_tastytrade_broker_smoke_apitest.py
Five small correctness fixes from code review.
1. _AsyncBridge.run: cancel pending future on exception.
future.result(timeout=...) leaving the underlying coroutine running on
the background loop is a leak — most visibly on TimeoutError. Wrap in
try/except, call future.cancel() on any exception, and re-raise so the
caller still sees the failure.
2. SANDBOX parsing: bool('false') == True.
Config path was bool(config.get('SANDBOX')), which lights up cert env
for any non-empty string including 'false'. Add a small _parse_truthy
helper used by both the config and env-var paths; treat the standard
true/false token set explicitly.
3. _modify_order rejects multileg explicitly.
_build_leg only constructs a single leg from the parent Order, so
calling replace_order on a spread would silently submit a one-legged
replacement. Detect order_class == MULTILEG (or len(child_orders) > 1)
up front, log a clear error, and return None. Cancel-and-resubmit is
the workaround until multileg replace lands.
4. _leg_to_lumi_side parses explicit equity actions.
Tastytrade requires the open/close form on equity legs (BUY_TO_OPEN /
SELL_TO_CLOSE / etc.) and that's what comes back when reading orders.
The previous equity dict only mapped plain 'buy'/'sell' so reads
silently fell through to OrderSide.BUY. Map all four explicit forms
to the matching Lumibot OrderSide.
5. do_polling updates stored.status synchronously on terminal events.
_safe_stream_dispatch enqueues an event the stream worker handles
asynchronously — until that event is processed, stored.status is
stale. The next poll cycle then sees the order missing from
get_live_orders, calls is_active() which still returns True, and
re-dispatches CANCELED on top of an already-FILLED/ERROR'd order.
Set stored.status = FILLED / CANCELED / ERROR right after dispatch so
is_active() reflects the new state on the next pass.
Tests: 4 new offline tests cover findings 1-4 (24 offline pass). Finding
5 has no dedicated unit test — the change is three inline status
assignments inside do_polling and a meaningful test would require
mocking out the strategy executor's order tracking machinery, which is
disproportionate. Verified by inspection.
Skipped: one CodeRabbit suggestion to add @pytest.mark.apitest and
is_test=True to the live smoke test. The apitest marker would re-trigger
Lumibot's conftest auto-skip on Polygon/Theta credentials (see commit
e4dace73 for why we removed it). is_test=True would force cert env even
when the user provides production creds, breaking the prod read-only
flow used during validation. The existing TASTYTRADE_SANDBOX env var
already controls cert-vs-prod selection.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
tests/test_tastytrade_broker_smoke_apitest.py (1)
774-796:⚠️ Potential issue | 🟠 Major | ⚡ Quick winLive smoke test still lacks
@pytest.mark.apitestand sandbox enforcement.The test runs whenever the three credential env vars are set — even with production credentials — because there is no
TASTYTRADE_SANDBOXguard and nois_test=Truepassed to the broker constructor. This was flagged in a prior review and remains unaddressed.Specific problems:
- Missing
@pytest.mark.apitestdecorator (coding-guideline requirement).Tastytrade(connect_stream=False)can connect to the live production API when run with production credentials.- The
skipifcondition should additionally requireTASTYTRADE_SANDBOXto be set to a truthy value.🛠️ Proposed fix
+@pytest.mark.apitest `@pytest.mark.skipif`( not all(os.environ.get(k) for k in ( "TASTYTRADE_CLIENT_SECRET", "TASTYTRADE_REFRESH_TOKEN", "TASTYTRADE_ACCOUNT_NUMBER", - )), - reason="Tastytrade credentials not configured.", + )) or os.environ.get("TASTYTRADE_SANDBOX", "").strip().lower() not in ("1", "true", "yes", "y"), + reason="Tastytrade sandbox credentials not configured.", ) def test_live_sandbox_balances_and_positions(): """Hit the sandbox API for balances + positions; expects no exceptions.""" from lumibot.brokers.tastytrade import Tastytrade - broker = Tastytrade(connect_stream=False) + broker = Tastytrade(connect_stream=False, is_test=True)As per coding guidelines, "Prefer existing pytest markers (
apitest,acceptance_backtest, etc.) over creating new environment variables to skip tests."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_tastytrade_broker_smoke_apitest.py` around lines 774 - 796, Add the missing apitest marker, ensure the test only runs against the sandbox by requiring TASTYTRADE_SANDBOX to be truthy in the pytest.mark.skipif condition, and instantiate the broker in test_live_sandbox_balances_and_positions with is_test=True (e.g., Tastytrade(connect_stream=False, is_test=True)) so the Tastytrade class uses sandbox endpoints; specifically update the skipif to check os.environ.get("TASTYTRADE_SANDBOX") along with the three credential vars and add `@pytest.mark.apitest` above the test function.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_tastytrade_broker_smoke_apitest.py`:
- Line 399: The list comprehension uses an ambiguous single-letter variable name
`l` in "actions = [l.action for l in new_order.legs]" which triggers Ruff E741;
change the loop variable to a clear name such as `leg` (e.g., "actions =
[leg.action for leg in new_order.legs]") so the variable is unambiguous and Ruff
will not flag it, updating any other occurrences of `l` in that expression if
present.
- Around line 93-113: Replace the non-thread-safe asyncio.Event with a
threading.Event for cross-thread signalling: change the `cancelled =
asyncio.Event()` declaration to use `threading.Event()` and ensure
`bridge._loop.call_soon_threadsafe(cancelled.set)` remains the setter from the
loop thread while the test's `cancelled.is_set()` check runs on the main thread;
also remove the dead `deadline = asyncio.get_event_loop().time() + 1.0 if False
else None` line (it's unused). Update imports as needed and keep `_slow()` and
the `with pytest.raises(concurrent.futures.TimeoutError): bridge.run(_slow(),
timeout=0.05)` logic intact.
---
Duplicate comments:
In `@tests/test_tastytrade_broker_smoke_apitest.py`:
- Around line 774-796: Add the missing apitest marker, ensure the test only runs
against the sandbox by requiring TASTYTRADE_SANDBOX to be truthy in the
pytest.mark.skipif condition, and instantiate the broker in
test_live_sandbox_balances_and_positions with is_test=True (e.g.,
Tastytrade(connect_stream=False, is_test=True)) so the Tastytrade class uses
sandbox endpoints; specifically update the skipif to check
os.environ.get("TASTYTRADE_SANDBOX") along with the three credential vars and
add `@pytest.mark.apitest` above the test function.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 05c37abd-c54b-4d95-8cce-5c9fb3409162
📒 Files selected for processing (2)
lumibot/brokers/tastytrade.pytests/test_tastytrade_broker_smoke_apitest.py
🚧 Files skipped from review as they are similar to previous changes (1)
- lumibot/brokers/tastytrade.py
added 2 commits
May 4, 2026 21:33
Address two follow-up review comments on the test file:
1. test_async_bridge_cancels_future_on_timeout used asyncio.Event to
signal from the background asyncio thread back to the main test
thread. asyncio.Event is bound to a single event loop and isn't
thread-safe; the right primitive for cross-thread signalling is
threading.Event. Swap it in. Also drop a stray dead expression
(`deadline = ... if False else None`) that was a holdover from an
earlier draft.
2. test_submit_orders_multileg_credit_spread had `for l in
new_order.legs` which trips Ruff E741 ("ambiguous variable name").
Rename to `for leg in ...`.
24 offline tests still pass.
Resolves the deferred review comment on test_live_sandbox_balances_and_ positions: add the explicit @pytest.mark.apitest decoration AND force is_test=True per CodeRabbit's recommendation, while avoiding the collateral damage we documented earlier (apitest alone would skip the test on missing Polygon/ThetaData creds). conftest.py: - Register a new tastytrade marker class, mirroring the existing ibkr marker for tests that hit a non-Polygon/Theta external API. - Extend the requires_ibkr branch in pytest_runtest_setup to also short-circuit Polygon/Theta credential checks when the tastytrade marker is present. test_live_sandbox_balances_and_positions: - Add @pytest.mark.apitest + @pytest.mark.tastytrade so it's discoverable as an API test without inheriting the polygon/thetadata gating. - Construct broker with is_test=True so the test always lands on Tastytrade's certification (sandbox) env regardless of how the user set TASTYTRADE_SANDBOX — prevents accidental prod-account hits in CI. 24 offline tests still pass; live test still gated by skipif on TASTYTRADE_* env credentials.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_tastytrade_broker_smoke_apitest.py`:
- Around line 5-7: The docstring says there is an `apitest`-marked test but the
bottom test only has `@pytest.mark.skipif`; either add the missing marker or
update the docstring. Fix by adding the appropriate pytest marker (e.g.,
annotate the bottom test with `@pytest.mark.apitest` or the agreed-upon
`@pytest.mark.tastytrade_api` and ensure a matching conftest registration) OR
change the module docstring to state the test is gated by `@pytest.mark.skipif`
instead of claiming an `apitest` marker; locate the bottom test (the one
currently decorated with `@pytest.mark.skipif`) and the module docstring to make
the consistent change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e37f06fe-e0c8-435d-8cc2-cc1423bb5f45
📒 Files selected for processing (1)
tests/test_tastytrade_broker_smoke_apitest.py
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.
Summary
Adds a Tastytrade broker built on the unofficial
tastytradePython SDK (12.4+). The SDK is async-only, so this adapter owns a daemon-thread asyncio bridge and dispatches every SDK call throughasyncio.run_coroutine_threadsafe. Tastytrade was the only one of Lumibot's commonly-requested broker integrations that didn't ship; this fills that gap.What works (live-validated against a production Tastytrade IRA)
provider_secret+refresh_token, sandbox + prod)Account.get+Account.get_balancesAccount.get_positions(equity + equity options with OCC parsing)market_data.get_market_data(last price, quote)instruments.get_option_chainreturned in Lumibot's nested{Multiplier, Chains: {CALL, PUT}}shapeBUY_TO_OPEN/SELL_TO_CLOSEper Tastytrade convention)PlacedOrder→ LumibotOrder, with multileg parent+child expansion)account.get_order,account.get_live_orders)Stubbed (follow-ups)
NewComplexOrder)_modify_order— current implementation only rebuilds a single leg, so spread modification is broken (cancel + resubmit works)AlertStreamer+DXLinkStreamer); polling is used todayTastytradeDataWire-format gotchas discovered during live validation
NewOrder.pricesign encodes credit/debit (negative = debit, positive = credit). The SDK serializer stripsabs()and emitsprice-effecton the wire. Sending a positive price for a BUY producescant_buy_for_credit.Buy/Sellfrom theOrderActionenum even though those values exist — the standard order endpoint requires the explicit open/close form for stocks (BUY_TO_OPEN,SELL_TO_CLOSE, etc.). PlainBuyreturnsorder_legs.action: is invalid.replace_orderreturns a new order id (cancel-and-new pattern), not an in-place edit._modify_orderabsorbs the new id back onto the local Order.get_market_datacalls trip a 429. Active polling should debounce or wait forDXLinkStreamerintegration.Tests
20 offline unit tests covering:
Leg,NewOrder, signed price)PlacedOrderparsing (single leg + multileg parent/child)_modify_ordercallsreplace_orderTastytradeData.get_last_price/get_quote/get_chainsPlus a live smoke test (
test_live_sandbox_balances_and_positions) gated onTASTYTRADE_*env credentials, which exercises balances + positions against a real account. Maintainers with sandbox creds will see it run automatically.Configuration
The broker reads credentials in this order: explicit kwargs >
configdict > environment.TASTYTRADE_CLIENT_SECRETTASTYTRADE_REFRESH_TOKENTASTYTRADE_ACCOUNT_NUMBERTASTYTRADE_SANDBOXtrue/1/yesto use Tastytrade's certification (sandbox) environmentTest plan
Summary by CodeRabbit
New Features
Limitations
Tests
Chores