feat(fix): RFQ / quoting message family (closes #428)

[TexasCoding]

Summary

FIX RFQ / Quoting — the full request-for-quote message family, prediction only. RFQ spans two roles on different sessions: RFQ creators (KalshiRT) solicit and accept quotes; market makers (KalshiRFQ) answer them. QuoteRequest (R) and Quote (S) are bidirectional — sent outbound via helpers and also decoded inbound as notifications. Builds on the foundation (#422), repeating groups (#423), and order entry (#424).

Messages (messages/rfq.py) — 13

35=	Class	Dir
R	QuoteRequest	both	.create(); carries the MVE legs + parties groups
S	Quote	both	.submit() (maker)
Z	QuoteCancel	out	maker
U7	QuoteConfirm	out	maker
UA	AcceptQuote	out	creator
UE	RFQCancel	out	.for_req_id() / .for_rfq_id()
b	QuoteRequestAck	in
AI	QuoteStatusReport	in	quote lifecycle
AG	QuoteRequestReject	in
U8	QuoteConfirmStatus	in
U9	QuoteCancelStatus	in
UB	RFQCancelStatus	in
UC	AcceptQuoteStatus	in

Outbound-only messages type their required fields as required; inbound + bidirectional messages keep fields optional with raw int/str codes (the order-entry inbound convention — a slightly off-spec server message parses rather than rejecting), and the helper constructors enforce the per-role required fields on the outbound path. Reuses the existing Party and MultivariateSelectedLeg group entries.

Name-collision resolution

The dictionary names the status field and its carrying message identically for four cases (e.g. tag 21010 and message 35=U8 are both QuoteConfirmStatus). The message classes keep the spec name; the value enums are suffixed Result — QuoteConfirmResult / QuoteCancelResult / RFQCancelResult / AcceptQuoteResult — so msg.quote_confirm_status == QuoteConfirmResult.REJECTED works without shadowing. New enums also: QuoteRequestType, QuoteStatus, QuoteRequestRejectReason.

Pricing

BidPx/OfferPx ride the FIX Price field as DollarDecimal: a maker submits integer cents (132=75), a creator notification is fixed-point dollars (132=0.7500) — DollarDecimal parses either with no float drift and no SDK-side conversion (units follow the session, like order entry). All RFQ tags already existed from the foundation; client.rfq() is the session factory.

Pre-PR adversarial review hardening

A multi-agent review (4 lenses → per-finding skeptical verification, 9 findings → 8 confirmed) ran before this PR; all confirmed findings are folded in:

• Real export bug fixed: AcceptQuote / AcceptQuoteStatus were in kalshi.fix.__all__ but never imported into the namespace — from kalshi.fix import AcceptQuote and import * raised. Added the import + a regression test asserting every __all__ name is bound (mypy can't catch this).
• Helpers now enforce the per-role required fields they claimed to: Quote.submit requires at least one of bid_px/offer_px; QuoteRequest.create requires a symbol and at least one of order_qty/cash_order_qty.
• AcceptQuote.side inversion documented — for AcceptQuote, Side.BUY_YES accepts the maker's NO quote and Side.SELL_NO the YES quote (the field reuses the shared FIX Side; the member names read opposite to the accept semantics).
• Corrected the four *Result enum doc cross-references after the collision rename.

Tests (tests/fix/test_rfq.py, 41 new)

Golden exact-wire fixtures pinned to the docs examples (incl. the MVE-legs QuoteRequest); round-trips for all 13 messages (with parties group); the four status → *Result enum comparisons; helper-guard negatives (submit no-price, create no-size); a public-API export-contract check; dispatch over every inbound-decodable type; and both the market-maker (R → S → PENDING/ACCEPTED → U7 → U8) and rfq-creator (R → b → S → UA → UC → UE → UB) quote-lifecycle integration tests against the mock acceptor.

Verification

• uv run ruff check . — clean
• uv run mypy kalshi/ (strict) — clean (156 files)
• uv run pytest tests/fix/ — 223 passed

Closes #428. Part of #402.

🤖 Generated with Claude Code

[claude]

Code Review — feat(fix): RFQ / quoting message family

Overview

Adds the full RFQ/quoting FIX message family (13 messages, 6 enums, dispatch wiring) in a single focused file (messages/rfq.py). The design follows existing conventions cleanly, the *Result suffix strategy for enum/message name collisions is the right call, and the pre-PR adversarial pass is evident in the quality. 41 tests with golden wire fixtures + two end-to-end lifecycle tests is solid coverage for a protocol feature this size.

---

Issues

Medium — Spelling inconsistency between *Cancel enums

In enums.py:

• RFQCancelResult.CANCELED (American spelling)
• QuoteCancelResult.CANCELLED (British spelling)
• QuoteStatus.CANCELLED (British spelling)

These are the names callers write in comparison expressions (status == RFQCancelResult.CANCELED). Having the two cancel-result enums spell the concept differently is real usability friction and a breaking-change fix later. The rest of the codebase uses CANCELLED, so RFQCancelResult.CANCELLED is the consistent choice.

---

Medium — RFQCancel direct construction silently produces an empty-body cancel

The two factory classmethods enforce exactly one identifier, but RFQCancel() with no args is valid Python and serializes to a message body with no identifiers — the exchange rejects it with no SDK-side feedback. Compare to Quote.submit and QuoteRequest.create, which guard their required invariants. A model_validator(mode="after") that raises if both quote_req_id and rfq_id are None would close this gap without touching inbound-parse robustness (inbound messages do not use this constructor path).

---

Minor — Quote.submit does not expose bid_size / offer_size / order_qty

The model has these three fields but submit() accepts none of them. If that is intentional (the exchange derives size from the RFQ), the docstring should say so — e.g. "exchange derives size from the RFQ; set via direct construction if needed" — to prevent the next reader from filing a bug.

---

Minor — type: ignore in lifecycle test

assert acceptor.first("R").get(Tag.QUOTE_REQ_ID) == REQ  # type: ignore[union-attr]

Assigning to a local first lets mypy verify the None check:

r = acceptor.first("R")
assert r is not None
assert r.get(Tag.QUOTE_REQ_ID) == REQ

---

What is working well

• Module-level docstring clearly maps the bidirectional vs. inbound vs. outbound split — exactly the right place for this.
• *Result suffix strategy is the right resolution to the spec name collision and is consistently applied across all four affected enums.
• Export regression test (test_fix_public_api_exports_are_importable) is a genuinely valuable addition — mypy misses this class of bug.
• AcceptQuote.side inversion is prominently documented in both the class docstring and the PR description. Right approach for a spec-mandated foot-gun.
• DollarDecimal for prices correctly handles both cents (prediction, maker) and fixed-point-dollars (UseDollars, creator) without SDK-side conversion.
• Golden wire fixture pattern with pinned expected strings makes regressions immediately obvious.
• Both lifecycle tests cover the full message sequence end-to-end, not just individual messages.

---

Summary

Three actionable items before merge: fix the CANCELED/CANCELLED spelling inconsistency in RFQCancelResult, add a model validator to RFQCancel to reject the all-None case, and clarify the submit docstring on size fields. The type: ignore is cosmetic. Everything else is solid.

[claude]

Code Review — feat(fix): RFQ / quoting message family (#436)

Overview

Adds the full 13-message RFQ family to the FIX layer. The architecture is clean: bidirectional messages (R/S) are all-optional for inbound robustness and have enforced constructors for outbound; outbound-only messages use required typed fields; inbound-only messages are all-optional with raw int/char codes. The *Result suffix convention to resolve the naming collision with same-named status messages is the right call and is well-documented.

---

Issues

Spelling inconsistency: CANCELED vs CANCELLED

RFQCancelResult uses CANCELED (one L); QuoteCancelResult uses CANCELLED (two L's). Both are in the same file and same PR:

# enums.py
class QuoteCancelResult(IntEnum):
    CANCELLED = 0   # two Ls

class RFQCancelResult(IntEnum):
    CANCELED = 0    # one L

The lifecycle test catches this in a subtle way — RFQCancelResult.CANCELED vs QuoteCancelResult.CANCELLED. Users consuming these enums will likely typo one or the other. Recommend aligning both to CANCELLED to match the QuoteStatus.CANCELLED = 17 spelling already in the file.

RFQCancel helpers silently drop parties

The for_req_id / for_rfq_id helpers don't expose the parties group field. If a caller needs to include party information in an RFQCancel, they must know to construct the class directly — nothing in the API surface guides them there. Either drop parties from RFQCancel (if the spec doesn't require it for cancel) or add an optional parties kwarg to both helpers. The same pattern issue applies to AcceptQuote, which has no factory method at all despite having required fields (quote_id, side) — direct construction works fine here but is inconsistent with the rest of the outbound family.

---

Missing Tests

• QuoteRequestType enum comparison — test_golden_quote_request_ack uses the raw int 1 but there's no assertion like msg.quote_request_type == QuoteRequestType.MANUAL. The *Result enum comparisons are covered in test_status_field_enum_comparisons; QuoteRequestType and QuoteStatus are not (the lifecycle test covers QuoteStatus.ACCEPTED implicitly, but a focused unit test would be cleaner).

• QuoteStatusReport REJECTED / CANCELLED status — The lifecycle test exercises PENDING (10) and ACCEPTED (0) but never REJECTED (5) or CANCELLED (17). Golden tests for those two branches would catch any tag-number drift for those values.

• AcceptQuote.side inversion — The inversion semantics (BUY_YES accepts maker's NO quote) are documented in the class docstring, which is the right place, but there's no test that asserts the semantic inversion. A comment-only guarantee is fragile. Consider a test like:

  def test_accept_quote_side_inversion_documented() -> None:
      # BUY_YES (1) accepts the maker's NO quote — the field is inverted per spec.
      msg = AcceptQuote(quote_id=QID, side=Side.BUY_YES)
      assert msg.side == Side.BUY_YES  # wire value 1
      # The docstring is the spec reference; this test pins the wire value.

---

Minor / Nits

• QuoteStatusReport.side is typed str | None (raw char). All other side fields in the codebase use Side (the enum). The reason for staying raw here is explained implicitly ("AcceptedSide present only when ACCEPTED"), but making the type Side | None or at minimum noting the raw-char choice in the docstring would reduce surprise.

• test_golden_rfq_cancel_and_status tests three distinct messages in one function. Each assertion is tight so it's not a real problem, but it breaks the single-concern test pattern followed everywhere else in the test suite.

• The _require_identifier comment says "outbound-only message (never decoded inbound), so this guard is safe" — this is accurate but slightly backwards. The guard is safe precisely because the validator runs on construction, not parsing. If RFQCancel were ever added to the dispatch table the validator would reject any inbound message missing both identifiers. Worth stating the constraint from the positive direction: "validator enforces outbound contract; do not add to APP_MESSAGE_MODELS / dispatch without revisiting."

---

What's Done Well

• Export-contract regression test (test_fix_public_api_exports_are_importable) directly addresses the class of bug fixed here and is the right permanent guard.
• Golden wire fixtures match the spec examples format and are a reliable contract for the tag ordering.
• The bidirectional R/S design (all-optional model + enforcing classmethod) is the cleanest way to handle messages that appear on both paths without duplicating classes.
• DollarDecimal handling for cents vs fixed-point dollars across sessions is correctly delegated to the type — no SDK-side conversion that could drift.
• The pre-PR adversarial review note in the PR body is valuable provenance.

---

Summary

One real issue to address before merge: the CANCELED/CANCELLED spelling inconsistency. The missing tests for QuoteStatus REJECTED/CANCELLED branches and the RFQCancel parties exposure gap are worth tracking (or fixing if trivial). Everything else is minor.

[claude]

Code Review — feat(fix): RFQ / quoting message family

Overview

Solid, well-scoped PR that adds the full RFQ message family to the FIX layer. The bidirectional / outbound-only / inbound-only split is clearly communicated, the *Result enum-suffix naming resolves the dictionary name collisions cleanly, and folding in the pre-PR adversarial review findings before opening (export bug, helper guards, enum doc fixes) means the PR arrives in good shape.

---

Bugs / Correctness

1. RFQCancelStatus is missing rfq_id (rfq.py:594–597)

RFQCancel can be sent with either quote_req_id or rfq_id. If the creator cancels by rfq_id, the exchange response presumably echoes the identifier it was given. RFQCancelStatus only carries quote_req_id, so a cancel-by-rfq_id response would silently drop the server field. Needs a rfq_id: str | None field to match both paths.

# current
class RFQCancelStatus(FixMessage):
    quote_req_id: str | None = fixfield(Tag.QUOTE_REQ_ID, ...)
    rfq_cancel_status: int | None = fixfield(Tag.RFQ_CANCEL_STATUS, ...)
    text: str | None = fixfield(Tag.TEXT, ...)

# suggested
    rfq_id: str | None = fixfield(Tag.RFQ_ID, FixType.STRING, default=None)

2. APP_MESSAGE_MODELS not visibly updated

dispatch.py correctly routes the 9 decodable RFQ types, but the diff shows no update to APP_MESSAGE_MODELS (wherever it is defined). If that registry drives contract/drift tests, the new types would silently fall outside the test suite. Worth confirming that QuoteRequest, Quote, QuoteRequestAck, QuoteStatusReport, QuoteRequestReject, QuoteConfirmStatus, QuoteCancelStatus, RFQCancelStatus, and AcceptQuoteStatus are all represented there.

---

Design / Conventions

3. _require_identifier validator on RFQCancel is a fragile outbound-only guard

The model-level validator works correctly now and the comment is good, but it creates a hidden landmine: if RFQCancel is ever added to dispatch (e.g. for a future maker-side cancel notification), every inbound message lacking both identifiers will fail at parse time. A class-level note is the only protection.

A more robust alternative is to drop the model validator and enforce the constraint only in the for_req_id / for_rfq_id helpers, keeping the model parseable both directions:

# helpers raise; model stays permissive
@classmethod
def for_req_id(cls, quote_req_id: str) -> RFQCancel:
    if not quote_req_id:
        raise ValueError("quote_req_id must be non-empty")
    return cls(quote_req_id=quote_req_id)

If keeping the model validator (reasonable for now), adding a TYPE_CHECKING-only Literal["outbound-only"] marker or a class attribute _OUTBOUND_ONLY = True would give future developers a machine-checkable signal.

4. QuoteStatusReport.side typing is inconsistent with documented inbound convention

All other inbound status codes use int | None (raw, compare against enum). side is str | None (raw char). Neither is wrong — CHAR vs INT are different FIX types — but the discrepancy warrants a short comment in the field definition noting it's a FIX CHAR, not an INT, to explain why it diverges from quote_status: int | None.

---

Tests

5. Variable name shadow in test_market_maker_quote_lifecycle (test_rfq.py:952–964)

The name s is used first for the raw acceptor message (s = acceptor.first("S")), then re-used as the iteration variable in a generator expression (all(isinstance(s, QuoteStatusReport) for s in statuses)). Python scoping makes this harmless, but it's confusing to read. Rename one — raw_s for the acceptor message is an easy fix.

6. No test for cancel-by-rfq_id response round-trip

test_golden_rfq_cancel_and_status only tests the quote_req_id path. Since RFQCancel.for_rfq_id is a public helper, there should be a golden fixture for the status response on the rfq_id path too (which would also surface the missing field noted in #1).

def test_golden_rfq_cancel_by_rfq_id_status() -> None:
    assert _wire(RFQCancelStatus(rfq_id=RFQ, rfq_cancel_status=0)) == f"21023={RFQ}|21013=0"

7. No negative test for QuoteRequest.create with MVE fields

The helper hard-codes no_related_sym=1 and requires symbol. A user building an MVE RFQ via the helper (passing symbol plus multivariate_collection_ticker) would silently produce a malformed message. The docstring says to use direct construction for MVE, but a guard (or at least a negative test showing that the helper raises on MVE-specific args) would make this contract explicit.

---

Minor

• Quote.submit docstring says "zero = no quote for that side" but the method raises if both are None — fine semantically, but clarifying that None (absent) and Decimal(0) (zero bid) are different might prevent user confusion.
• test_status_field_enum_comparisons exercises 4 of the 6 *Result enums. QuoteConfirmResult.REJECTED and AcceptQuoteResult.ACCEPTED are both covered, but AcceptQuoteResult.REJECTED and QuoteConfirmResult.ACCEPTED are not separately asserted — low priority, just completeness.
• The lifecycle tests are thorough and read well. The until fixture-based polling pattern is clean.

---

Summary

The PR is well-structured and near-complete. The primary actionable items are:

1. Must fix: RFQCancelStatus missing rfq_id field
2. Should verify: APP_MESSAGE_MODELS coverage for new decodable types
3. Nice to have: test for cancel-by-rfq_id status golden wire

The export-contract regression test (test_fix_public_api_exports_are_importable) is a welcome addition that should stay. The AcceptQuote.side inversion documentation is handled well. Overall, good work.

[claude]

Code Review — feat(fix): RFQ / quoting message family (#436)

Overall: This is a solid, well-scoped PR. The pre-PR adversarial-review pass caught the real export bug and the missing helper guards, and the test suite (golden-wire fixtures + lifecycle integration tests) gives strong coverage. No blockers. A few things worth addressing or at least acknowledging:

---

Design / Correctness

QuoteStatusReport.side is a raw str | None, not Side | None

The inbound-raw-types convention is intentional and documented, but AcceptQuote.side (outbound) uses the typed Side enum — so the two side fields on the same conceptual flow have different types. The docstring says "raw char (FIX CHAR — not an int like the status codes), present only when ACCEPTED", but there's no test showing the correct comparison pattern (accepted_report.side == "1" vs accepted_report.side == Side.BUY_YES.value). The lifecycle test does pin the raw-string comparison (assert accepted_report.side == "1"), which at least makes the expectation visible.

Suggestion: Add one line to QuoteStatusReport's docstring making this explicit: "Compare against ``Side.BUY_YES.value`` (``\"1\"``), not the ``Side`` enum directly." Low effort, prevents a common gotcha.

no_related_sym=1 is hardcoded in QuoteRequest.create but the MVE direct-construction path doesn't enforce it

The spec presumably always requires NoRelatedSym=1 for any RFQ. Via create() it's always set; via direct construction for MVE it's the caller's responsibility and the model won't reject no_related_sym=None. The PR body calls this out ("MVE/parlay RFQs... construct QuoteRequest directly"), so this is a known tradeoff, but it's worth a model-level validator or at least documenting on the field.

Sparse QuoteStatus values (0, 5, 10, 17)

The test_quote_status_report_status_values parametrize pins all four values — good. But if the exchange can also send other 297 values (e.g. for expired or error states not in the spec snapshot), the model will store an unmapped int that the calling code can't compare against the enum. This is intentional per the inbound-raw-types convention, but worth noting in the enum docstring: "values observed in the Kalshi FIX spec; unknown values parse as raw int."

---

RFQCancel._require_identifier model validator

The validator is clean and the outbound-only guard test is a nice defensive touch. One edge case: RFQCancel() with both quote_req_id and rfq_id set is allowed — does the spec say which takes priority? If not, fine, but if there's a spec preference (client ID over server ID or vice versa), a note in the docstring would help callers.

---

Tests

Strengths: Golden-wire fixtures, all-13 round-trips, helper-guard negatives, public-API export contract check, and both lifecycle tests are all exactly the right things to test. The APP_MESSAGE_MODELS tripwire for RFQ_CANCEL is clever.

Missing negative: No test for direct QuoteRequest() construction with neither symbol nor multivariate_collection_ticker — the model accepts it (both optional), but the exchange would reject it. Consider: QuoteRequest(quote_req_id=REQ, no_related_sym=1, order_qty=Decimal("100")) — is this a valid intermediate state or a bug at construction time?

Fragile count assertions in lifecycle tests: await until(lambda: len(received) == 3) is fine for a controlled mock, but if the mock sends a Heartbeat or other session message mid-test the count check would fail spuriously. This is probably fine given how MockAcceptor works, but worth a mental note.

test_market_maker_quote_lifecycle uses RFQ as the quote_req_id in the inbound notification (notif = QuoteRequest.create(RFQ, symbol=SYM, ...)). RFQ = "server-rfq-456" — a server-assigned ID being used as quote_req_id (a client-assigned field). This is just test data, but the naming is slightly confusing. Consider create(REQ, ...) for the maker notification path.

---

Style / Conventions

• All new enums follow the existing IntEnum/StrEnum pattern. ✓
• Alphabetical ordering in __all__ and import blocks is maintained. ✓
• No stray comments or unrelated reformatting. ✓
• DollarDecimal for BidPx/OfferPx. ✓
• mypy strict + ruff clean per PR checklist. ✓

---

Summary

Finding	Severity	Blocking?
QuoteStatusReport.side comparison pattern undocumented	Low	No
MVE direct-construction skips no_related_sym enforcement	Low	No
Sparse QuoteStatus enum not documented as exhaustive/non-exhaustive	Low	No
RFQCancel dual-ID priority not documented	Trivial	No
Missing negative test for bare QuoteRequest()	Low	No
Lifecycle test uses RFQ where REQ is clearer	Trivial	No

Nothing here blocks the merge — the implementation is correct, the critical guards are in place, and the test suite is comprehensive. The suggestions above are mostly documentation/clarity improvements.

🤖 Generated with Claude Code