feat(fix): order-entry message flow (closes #424)#431
Conversation
Order-entry (OE) FIX messages over the shared codec/session engine, for both the prediction and margin products. Outbound (typed enum vocabulary, required fields enforced): - NewOrderSingle (35=D), OrderCancelRequest (35=F), OrderCancelReplaceRequest (35=G), OrderMassCancelRequest (35=q) - Side / OrdType / TimeInForce / ExecInst (POST_ONLY) / SelfTradePreventionType round-trip to the wire; NoPartyIDs group via the #423 components; Price rides the FIX PRICE field (integer cents on prediction by default, fixed-point dollars on margin / UseDollars=Y); quantities are fractional decimals. Inbound (fields optional; char/int code fields raw for robustness): - ExecutionReport (35=8) with NoMiscFees / NoPartyIDs / NoCollateralAmountChanges groups, position/fee/collateral detail on ExecType=Trade - OrderCancelReject (35=9), OrderMassCancelReport (35=r), BusinessMessageReject (35=j) - decode_app_message(raw) dispatches an inbound RawMessage (from FixSession.on_message) to its typed model. Also folds in the PR #430 review follow-ups: FixGroupMeta/_GroupSpec now type entry_model as the public FixGroup (not the private base), and group decoding logs (debug) on a non-integer or short NumInGroup instead of silently dropping. 13 new tests (round-trips incl. groups, enum/price wire values, dispatch, inbound robustness, and a session send/receive integration test). ruff + mypy --strict clean; 117 FIX tests pass. Closes #424. Part of #402. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Code Review — feat(fix): order-entry message flow (#431)OverviewClean, well-scoped PR that adds the full order-entry FIX flow. The outbound/inbound design split (typed enums + required fields for outbound; raw chars/ints + optional fields for inbound) is the right approach for a production FIX client where server messages must parse leniently. The 13 tests cover all message types and the key behavioural paths. Issues & Risks1.
Suggested fix: add 2. price: DollarDecimal | None = fixfield(Tag.PRICE, FixType.PRICE, default=None)Since the exchange only supports limit orders, a cancel-replace without a price will be rejected server-side. Making 3.
Test Coverage Gaps
Minor Observations
SummaryThe design is solid and the implementation follows existing SDK conventions well. The three actionable items above (TransactTime, required price on replace, missing top-level export) should be resolved before merge; the test gaps are lower priority but worth a follow-up issue. |
- export APP_MESSAGE_MODELS from kalshi.fix top-level __all__ (was only exported
from kalshi.fix.messages)
- comment the Kalshi compound "int;int" exec_id format on ExecutionReport
- tests: malformed group decode (short-count + non-integer NumInGroup debug
paths), decode_app_message dispatch for OrderCancelReject /
OrderMassCancelReport / BusinessMessageReject, a quantity-only
OrderCancelReplaceRequest, and the NewOrderSingle empty-parties default
Two review items were intentionally NOT changed — they apply generic FIX 5.0SP2
requiredness, not the Kalshi dictionary v1.03:
- TransactTime(60) is absent from the Kalshi NewOrderSingle/Cancel/Replace
messages (and the order-entry doc example omits tag 60); adding it would send
a tag those messages do not define.
- Price is required='N' in OrderCancelReplaceRequest (G); a quantity-only amend
is valid ("required if changing price"), so price stays optional.
ruff + mypy --strict clean; 122 FIX tests pass.
Part of #424 / #402.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks — addressed in 26637ca. Actionable items1. TransactTime (60) on outbound D/F/G — not applicable to the Kalshi dialect (no change). The review applies generic FIX 5.0SP2, but the Kalshi FIX dictionary v1.03 does not include Adding tag 60 would send a field those messages don't define (risking a 2. Required 3. Test gaps — added
Minor
Appreciate the thorough pass — the type-narrowing confirmation ( |
Code ReviewSummary: This PR adds the order-entry FIX message flow: four outbound request models, four inbound report models, a typed dispatch function, and 13 tests. The architecture is clean, the outbound/inbound design split is well-motivated, and the follow-up improvements to Issues1. In alloc_account: int | None = fixfield(Tag.ALLOC_ACCOUNT, FixType.INT, default=None)FIX standard tag 79 ( This same field appears on 2. FIX 5.0 SP2 requires 3. def decode_app_message(raw: RawMessage) -> FixMessage | None:
model = APP_MESSAGE_MODELS.get(raw.msg_type or "")
return model.from_raw(raw) if model is not None else NoneIf the registered model's 4. The dict is exported in Minor Points5. The FIX spec includes 6. Field ordering in self_trade_prevention_type: SelfTradePreventionType | None = ...
parties: Annotated[list[Party], FixGroupMeta(Tag.NO_PARTY_IDS, Party)] = groupfield()
secondary_cl_ord_id: str | None = ...The framework serializes in declaration order. Having What's Good
VerdictItems 1 and 3 are the most actionable before merge (potential runtime bug and undocumented exception contract). Item 2 needs a comment or a small addition. Items 4–6 are lower priority but worth a pass. 🤖 Generated with Claude Code |
…stry - decode_app_message now catches ValidationError / ValueError / ArithmeticError and returns None (logged) so a malformed inbound message is never raised into the consumer's on_message; regression test added - APP_MESSAGE_MODELS wrapped in MappingProxyType (read-only dispatch registry) - comment that AllocAccount (79) is INT per Kalshi dictionary v1.03 (subaccount number 0-32), not the FIX-standard STRING - note the intentional TransactTime (60) omission on NewOrderSingle (absent from the Kalshi 35=D message) Verified against dictionary v1.03, left unchanged: - AllocAccount (79) is INT in the Kalshi dict. - OrderMassCancelReport (35=r) has no TotalAffectedOrders (533); tag 533 is not in the Kalshi dictionary at all. - NewOrderSingle field/group order matches the dictionary's 35=D layout. ruff + mypy --strict clean; 123 FIX tests pass. Part of #424 / #402. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks — addressed in 247bf2b. Actionable3. 4. 1. 2. Minor5. 6. Thanks for the careful second pass — the immutable-registry and exception-contract points were good catches. |
Code Review — feat(fix): order-entry message flow (#431)OverviewAdds the order-entry FIX message flow: four outbound request types ( Overall this is a clean, well-reasoned implementation. The outbound-typed / inbound-lenient split is the right call for a FIX integration, and the test suite covers the important paths thoroughly. Strengths
IssuesMedium —
|
…eview) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks for the approval. Follow-ups handled:
(15c1a4a) |
Code Review — PR #431: feat(fix): order-entry message flowOverall: Solid implementation that follows the established FIX message patterns. The outbound/inbound design split (strict typed enums on send, optional raw codes on receive) is the right call for FIX protocol robustness. The Issues1.
model = APP_MESSAGE_MODELS.get(raw.msg_type or "")
if model is None:
return None # <- expected path
...
except (ValidationError, ValueError, ArithmeticError):
logger.warning(...)
return None # <- unexpected path, same return valueFor 2. The inline comment says # More precise:
except (ValidationError, ValueError, decimal.DecimalException):3.
Minor observations4.
5.
6. The "NR only" note in the docstring is accurate, but there's no enforcement. A caller using a multivariate session could accidentally send a mass-cancel and get a confusing exchange rejection. This is a FIX-layer constraint and not easily enforced here — just flagging it as something to revisit if the session layer gains request validation. What's good
Verdict: Items 1–3 are worth addressing before merge; 4–6 are nice-to-haves. The core implementation is correct and well-tested. 🤖 Generated with Claude Code |
Summary
The order-entry (OE) FIX message flow for both products, over the shared codec/session engine. Builds on the foundation (#422) and repeating-group support (#423).
Messages
Outbound (typed enum vocabulary; required fields enforced so a caller error fails at construction):
NewOrderSingle(35=D),OrderCancelRequest(35=F),OrderCancelReplaceRequest(35=G),OrderMassCancelRequest(35=q)Side/OrdType/TimeInForce/ExecInst(POST_ONLY) /SelfTradePreventionTyperound-trip to the wire;NoPartyIDsvia the [FIX] Repeating-group support in the typed message framework #423Partycomponent.Inbound (fields optional; char/int code fields kept raw
str/intso a slightly off-spec report parses cleanly rather than rejecting — compare againstkalshi.fix.enums):ExecutionReport(35=8) withNoMiscFees/NoPartyIDs/NoCollateralAmountChangesgroups and position/fee/collateral detail onExecType=TradeOrderCancelReject(35=9),OrderMassCancelReport(35=r),BusinessMessageReject(35=j)decode_app_message(raw)dispatches an inboundRawMessage(delivered toFixSession.on_message) to its typed model.Pricing & quantities
Price(44) rides the FIXPRICEfield; units follow the session'sUseDollars— integer cents (1–99) on prediction by default, fixed-point dollars on margin /UseDollars=Y. Amounts useDollarDecimal, quantitiesFixedPointCount(no float drift). Sending an order is the generic, already-typedawait session.send(NewOrderSingle(...))— no OE coupling added to the transport layer.Also (PR #430 review follow-ups)
FixGroupMeta/_GroupSpecnow typeentry_modelas the publicFixGrouprather than the private base.NumInGroupinstead of silently dropping the group.Tests (
tests/fix/test_order_entry.py, 13 new)Round-trips for all eight messages (incl. groups), enum + price on-wire values, cents-vs-dollars price encoding, the
decode_app_messagedispatch (andNonefor admin), inbound robustness on a sparse report, and a session send-order/receive-ExecutionReport integration test against the mock acceptor.Verification
uv run ruff check .— cleanuv run mypy kalshi/(strict) — clean (150 files)uv run pytest tests/fix/— 117 passedCloses #424. Part of #402.
🤖 Generated with Claude Code