feat(broker-api): a bracket is one order kind, because two legs race (#502 stage 1) - #521
Merged
Conversation
…502 stage 1) Stage 1 of the #502 design: the port vocabulary, and nothing else. This ships with NO live caller, deliberately -- the executor still builds its raw trigger_bracket_gtc dict against the pre-port CoinbaseClient, and moving it is stage 2's job. Grep confirms zero references to BracketGTC under keel/. BracketGTC joins the OrderSpec sum type. It is an EXIT bracket that closes a held position, not an entry-plus-exits parent order: keel enters with market IOC and protects afterwards, so encoding an entry here would represent a shape no keel path produces. __post_init__ refuses non-positive numerics and a stop that does not sit on the losing side of the target -- equal legs are refused as firmly as inverted ones, because an equal-leg 'bracket' is a stop and a target racing at the same price, which is not a shape a rule can mean. No stop_direction field. It is derivable from side exactly as StopLimitGTC derives it, and a field would let a caller build a SELL bracket that triggers upward -- representable nonsense, which is what the sum type exists to prevent. The port's names are keel's (take_profit_price), not Coinbase's (limit_price), so a second venue's translation does not start from Coinbase's vocabulary and so it cannot be confused with LimitGTC.limit_price. The Coinbase translation is byte-identical to what executor._bracket_order_configuration emits today -- three keys, no stop_direction -- and a test pins that parity so the two cannot drift while both exist. The test imports both; production code does not. DEVIATION FROM THE PLAN, recorded because it recurs: 'other adapters refuse structurally, add no code' was only half true. Alpaca and Robinhood match OrderSpec exhaustively with assert_never, so widening the union broke both under mypy and each needed an explicit refusal case. That is the better outcome -- it mirrors the defence-in-depth Robinhood's translator already applies -- but every future OrderSpec kind will hit the same wall. Two venue facts worth recording while they were established: Alpaca DOES support order_class=bracket, but equities-only, and this adapter declares asset_classes={'equity'} -- so its absence means 'not written yet', not 'impossible'. Robinhood and Kraken are genuine venue limitations (Kraken's close[...] is an OTO, not an OCO). Gates: pytest 4550 passed / 3 skipped; ruff check keel tests packages clean; mypy clean across 353 source files. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NyeggYtojNXCTHeD3JHxb6
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
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.
Stage 1 of the #502 design. Port vocabulary only — zero live-path change, and deliberately no live caller.
The executor still builds its raw
trigger_bracket_gtcdict against the pre-portCoinbaseClient; moving it is stage 2's job. Grep confirms zero references toBracketGTCunderkeel/.What it adds
BracketGTCjoins theOrderSpecsum type. An exit bracket that closes a held position — not an entry-plus-exits parent order, because keel enters with market IOC and protects afterwards, so encoding an entry would represent a shape no keel path produces.__post_init__refuses non-positive numerics and a stop that doesn't sit on the losing side of the target. Equal legs are refused as firmly as inverted ones — an equal-leg "bracket" is a stop and a target racing at the same price, which is not a shape a rule can mean.No
stop_directionfield. Derivable fromsideexactly asStopLimitGTCderives it; a field would let a caller build a SELL bracket that triggers upward — representable nonsense, which is what the sum type exists to prevent.Names are keel's, not Coinbase's.
take_profit_price, notlimit_price— so a second venue's translation doesn't start from Coinbase's vocabulary, and so it can't be confused withLimitGTC.limit_price.The Coinbase translation is byte-identical to what
executor._bracket_order_configurationemits today, pinned by a parity test so the two can't drift while both exist. The test imports both; production code doesn't.Deviation from the plan, worth knowing because it recurs
The design assumed "other adapters refuse structurally, add no code." Half true. Alpaca and Robinhood match
OrderSpecexhaustively withassert_never, so widening the union broke both under mypy — each needed an explicit refusal case.That's the better outcome (it mirrors the defence-in-depth Robinhood's translator already applies), but every future
OrderSpeckind will hit the same wall, including stage 2's.Two venue facts established while here
order_class=bracket— equities-only, and this adapter declaresasset_classes={"equity"}. So its absence means "not written yet", not "impossible".close[...]is an OTO, not an OCO.Latent gap found, not fixed here
Kraken has no
_reject_unsupported— every method raisesNotImplementedErrorrather thanUnsupportedOrder, so it would failtest_every_undeclared_order_kind_is_refusedif it had a conformance subclass. It doesn't have one, so nothing fails today. Flagging rather than widening this PR's scope.Verification
pytest -q— 4,550 passed, 3 skippedruff check keel tests packages— cleanmypy— clean, 353 source filesAll three re-run independently after the implementing agent reported, not taken on trust.
Not in this PR
Stages 2–4 of the design: the hardened roll protocol, the
run_oncestop-management step, and the adapter flip. Two prerequisites surfaced by the same design pass are filed separately as #519 (crash between cancel and replace leaves a position naked and silent) and #520 (positions.initial_stopnot persisted, so the live break-even roll is uncomputable).