Skip to content

feat(fix): market-settlement / post-trade reports (closes #429)#438

Merged
TexasCoding merged 3 commits into
mainfrom
feat/429-fix-settlement
Jun 6, 2026
Merged

feat(fix): market-settlement / post-trade reports (closes #429)#438
TexasCoding merged 3 commits into
mainfrom
feat/429-fix-settlement

Conversation

@TexasCoding

Copy link
Copy Markdown
Owner

Summary

FIX Market Settlement / Post-Tradeprediction only, and the final sub-issue of the #402 epic. A read-only MarketSettlementReport (35=UMS) stream on the KalshiPT session (settlement on by default) and on KalshiRT when opted in. Builds on the foundation (#422) and the nested repeating-group support (#423).

Messages (messages/settlement.py)

MarketSettlementReport (35=UMS), inbound — fields optional, codes raw (the inbound convention). It reuses the existing MarketSettlementParty group entry (built in #423), which nests NoCollateralAmountChanges and NoMiscFees per party. Money fields: settlement_price (cents, e.g. 100.00), collateral/fee amounts → DollarDecimal; LongQty/ShortQtyFixedPointCount (no float). New enums: MarketResult (yes/no/scalar), CollateralAmountType (BALANCE/PAYOUT).

Multi-fragment reassembly (settlement.py)

Large settlement batches paginate across several UMS fragments. The MarketSettlementReportID differs per page, so SettlementReassembler correlates fragments by Symbol, accumulates each symbol's party entries, and emits one combined MarketSettlementReport on the terminal fragment (LastFragment=Y or absent; =N means more pages). Per-symbol isolation handles interleaved batches; a standalone report passes through unchanged.

reasm = SettlementReassembler()
async with client.post_trade(on_message=handle) as session:
    ...
# handle(raw): complete = reasm.add(decode_app_message(raw))  # None until the batch is whole

ReceiveSettlementReports (tag 20127)

FixConfig.receive_settlement_reports is bool | None, passed straight to the Logon: None omits the tag (gateway default — on for KalshiPT, off for KalshiRT), True opts KalshiRT in (20127=Y), False opts KalshiPT out (20127=N). The Logon field, all settlement tags, and client.post_trade() already existed from the foundation.

Pre-PR adversarial review

A multi-agent review (4 lenses → per-finding skeptical verification, 8 confirmed) ran against this branch. It verified the money path and nested-group decode are correct (field/type faithfulness, two-party nested decode with no cross-contamination, negative collateral + sub-cent fees round-trip exact). Folded in:

  • The reassembler keys only on Symbol (the report id differs per page) and the protocol has no batch-id, so its contiguous-per-symbol-batch assumption + limitations are now documented on the class: no TotNum reconciliation; two interleaved same-symbol batches would coalesce (a well-formed stream sends a symbol's fragments contiguously, so it doesn't occur — sequential same-symbol batches reassemble independently); orphaned buffers persist until clear(); duplicate terminals re-emit (consumers handle idempotently).
  • clear() now warns when discarding non-empty buffers (observability for dropped partial settlement data).
  • Filed [FIX] Dictionary↔model drift test for the FIX message set (analogous to REST TestRequestBodyDrift) #437 for a FIX dictionary↔model drift test (infra follow-up, analogous to the REST drift suite — spans all FIX flows, out of scope here).

Tests (tests/fix/test_settlement.py, 17 new)

Golden UMS wire; nested-group round-trip including two parties each with their own collateral + fee groups (negative collateral, sub-cent fee); a decode of the docs example with LastFragment trailing the party group (proves the positional nested decode); dispatch; multi-fragment reassembly (buffer / emit / per-symbol / standalone / no-symbol / sequential same-symbol / clear-warns); a KalshiPT settlement-stream integration test against the mock acceptor; and the ReceiveSettlementReports logon flag across opt-in / opt-out / default.

Verification

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

Closes #429. Part of #402.

🤖 Generated with Claude Code

FIX market settlement — prediction only. A read-only MarketSettlementReport
(35=UMS) stream on KalshiPT (default on) and on KalshiRT (opt in via
ReceiveSettlementReports). This is the last sub-issue of the #402 FIX epic.

- messages/settlement.py: MarketSettlementReport (UMS), inbound (fields optional,
  codes raw). Reuses the MarketSettlementParty group entry (from #423), which
  nests NoCollateralAmountChanges + NoMiscFees per party. market_result is a raw
  str (compare against the new MarketResult enum); settlement_price (cents) and
  collateral/fee amounts are DollarDecimal; positions FixedPointCount.
- enums.py: MarketResult (yes/no/scalar), CollateralAmountType (BALANCE/PAYOUT).
- settlement.py: SettlementReassembler — paginated batches span multiple UMS
  fragments whose MarketSettlementReportID differs per page, so it correlates by
  Symbol and accumulates parties until the terminal fragment (LastFragment Y/
  absent), emitting one combined report.
- config: receive_settlement_reports (bool | None) threaded into the Logon — None
  omits tag 20127 (gateway default: on for PT, off for RT), True opts RT in
  (20127=Y), False opts PT out (20127=N). The Logon field + all tags already
  existed from the foundation; client.post_trade() is the session factory.
- dispatch registers UMS; exports from kalshi.fix / kalshi.fix.messages.

Hardening from a pre-PR adversarial review (8 confirmed findings; the money path
and nested decode were verified correct). The reassembler keys only on Symbol
(the report id differs per page) with no batch-id in the protocol, so its
contiguous-per-symbol-batch assumption + limitations (no TotNum reconciliation;
interleaved same-symbol batches would coalesce — not produced by a well-formed
stream; orphaned buffers persist until clear(); duplicate terminals re-emit) are
now documented on the class, and clear() warns when it discards non-empty
buffers. Filed #437 for a FIX dictionary<->model drift test (infra follow-up).

17 new tests: golden UMS wire, nested-group round-trip (incl. two parties each
with collateral + fee groups, negative collateral, sub-cent fee), a decode of the
docs example with LastFragment trailing the party group, dispatch, multi-fragment
reassembly (buffer/emit/per-symbol/standalone/no-symbol/sequential-same-symbol/
clear-warns), a KalshiPT settlement-stream integration test, and the
ReceiveSettlementReports logon flag across opt-in / opt-out / default.

ruff + mypy --strict clean; 248 FIX tests pass.

Closes #429. Part of #402.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jun 6, 2026

Copy link
Copy Markdown

Code Review: feat(fix): market-settlement / post-trade reports (closes #429)

Overall: Well-structured, spec-faithful implementation with good test coverage. The multi-agent adversarial review process clearly paid off — the money types are correct, nested groups are clean, and the documented limitations are honest. A few minor points worth discussing before merge.


✅ Strengths

  • Message model is minimal and correct. All fields are optional (inbound convention), money amounts use DollarDecimal/FixedPointCount (no float drift), and the nested MarketSettlementParty group is reused cleanly from [FIX] Repeating-group support in the typed message framework #423.
  • SettlementReassembler limitations are well-documented. The class docstring is honest about TotNum reconciliation not happening, interleaved same-symbol batch coalescence, and orphaned buffers on disconnect. The pending_symbols() / clear() API gives callers the hooks they need.
  • clear() warning is good observability. Silent data drops at reconnect would be hard to debug.
  • Test coverage is strong. The two-party cross-contamination test (test_settlement_report_two_parties_with_nested_groups_roundtrip), the doc-example trailing LastFragment decode, and the sequential same-symbol batch test are exactly the edge cases that bite in practice.
  • receive_settlement_reports three-way pass-through is correct. The comment in session.py explains why this field is handled differently from listener_session / skip_pending_exec_reports (False has semantic meaning: explicitly opt PT out), which avoids a future "why not True if X else None here?" confusion.

⚠️ Minor Issues

1. MarketResult enum member names are more verbose than necessary

class MarketResult(StrEnum):
    YES_OUTCOME = "yes"
    NO_OUTCOME = "no"
    SCALAR_VALUE = "scalar"

The convention in this file is concise identifiers that mirror the wire value or a tight descriptor (BUY_YES, SELL_NO, NEW, CANCELED). Adding _OUTCOME / _VALUE suffixes makes MarketResult.YES_OUTCOME read awkwardly when the class name already supplies context. Simpler alternatives:

YES = "yes"
NO = "no"
SCALAR = "scalar"

YES and NO are valid Python identifiers (not keywords). This is purely a naming consistency nib — not a correctness issue — but it would be a breaking change to rename post-ship if adopted.

2. Orphaned-fragment warning drops parties silently

if report.symbol is None:
    logger.warning("settlement fragment without Symbol; cannot reassemble")
    return None

The parties inside this orphaned fragment are discarded with no indication of how many were lost. Consider adding the party count:

logger.warning(
    "settlement fragment without Symbol; dropping %d parties", len(report.parties)
)

Minor, but makes the log actionable.

3. tot_num_market_settlement_reports is receive-only but never used

The field exists in the model (correct — it arrives on the wire) but the reassembler ignores it by design. The class docstring says "does not reconcile against TotNumMarketSettlementReports", and #437 tracks the follow-up drift test. No action needed here — just noting this is the right call for scope.

4. Terminal fragment's market_settlement_report_id is arbitrary

When model_copy assembles the final report, it inherits the last fragment's MarketSettlementReportID (each page has a different one). Consumers keying off that ID for deduplication would get inconsistent results across fragments. The class docstring documents this ("A re-delivered terminal fragment re-emits its report, so consumers should handle emitted reports idempotently"), but it might be worth explicitly noting that the ID in the assembled report is the last fragment's ID, not a canonical batch ID.


🟢 No Issues

  • Dispatch registration (APP_MESSAGE_MODELS) is correct — MsgType.MARKET_SETTLEMENT_REPORT.valueMarketSettlementReport.
  • __init__.py / messages/__init__.py exports are complete and sorted (passes ruff RUF022).
  • config.py factory methods (prediction / margin) both thread receive_settlement_reports through symmetrically.
  • groupfield() usage in MarketSettlementReport.parties matches the established pattern in market_data messages.
  • model_copy(update={"parties": parties}) is the correct Pydantic v2 API for immutable model updates.

Summary

Items 1 and 2 are worth a quick fix before merge (naming is hard to change post-ship; the logging gap is cheap). Items 3 and 4 are known/acceptable and are already documented. No blocking issues.

- enums: rename MarketResult members YES_OUTCOME/NO_OUTCOME/SCALAR_VALUE ->
  YES/NO/SCALAR to match the file's concise convention (BUY_YES, NEW, BID); the
  class name already supplies the "outcome" context. Pre-ship, so no break.
- settlement: the symbol-less fragment warning now reports the dropped party
  count (actionable log); document that an assembled report carries the terminal
  fragment's MarketSettlementReportID (no canonical batch id — dedup off Symbol +
  clearing date).

No action (already correct / documented per the review): tot_num is receive-only
by design (see #437); APP_MESSAGE_MODELS + exports verified complete.

ruff + mypy --strict clean; 248 FIX tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jun 6, 2026

Copy link
Copy Markdown

Code Review — feat(fix): market-settlement / post-trade reports

Overall this is a clean, well-scoped addition. The reassembler design is solid, the protocol edge-cases are clearly documented, and the test suite is thorough. A few things worth addressing before merge:


Medium — settlement_price type vs. documented semantics

kalshi/fix/messages/settlement.py, line 60:

settlement_price: DollarDecimal | None = fixfield(
    Tag.SETTLEMENT_PRICE, FixType.PRICE, default=None
)

The class docstring says "settlement_price is in cents (e.g. Decimal('100.00'))", but the field uses DollarDecimal. In prediction markets, settlement at 100.00 means 100 cents = $1.00, not $100.00. The type name DollarDecimal implies dollar amounts, making the "cents" description actively misleading. Please either:

  • Fix the docstring to say "dollars (e.g. Decimal('1.00'))" if the wire value is already in dollars, or
  • Clarify in the docstring and a comment why the wire value 100.00 is cents and how DollarDecimal maps to it.

Medium — # type: ignore[arg-type] in integration test

tests/fix/test_settlement.py, line 313:

results = [reasm.add(decode_app_message(r)) for r in received]  # type: ignore[arg-type]

decode_app_message returns FixMessage | None, but reasm.add() takes MarketSettlementReport. Suppressing with type: ignore hides a real type gap. The test should assert the type first:

decoded_msgs = [decode_app_message(r) for r in received]
assert all(isinstance(m, MarketSettlementReport) for m in decoded_msgs)
results = [reasm.add(m) for m in decoded_msgs]  # type: ignore[arg-type]  ← now at least a cast is all that's needed

Better yet, keep the assert — that way a misrouted dispatch actually fails the test rather than hiding downstream.


Low — test_reassembler_non_final_without_symbol_dropped doesn't verify the warning

tests/fix/test_settlement.py, line ~252:

test_reassembler_clear_warns_on_pending correctly uses caplog to assert the warning was emitted. test_reassembler_non_final_without_symbol_dropped tests the drop behavior but not the warning. Parallel structure would be:

def test_reassembler_non_final_without_symbol_dropped(caplog: pytest.LogCaptureFixture) -> None:
    r = SettlementReassembler()
    with caplog.at_level(logging.WARNING, logger="kalshi.fix"):
        assert r.add(_frag(None, ["a"], False)) is None
    assert r.pending_symbols() == set()
    assert any("without Symbol" in rec.message for rec in caplog.records)

Low — MarketResult.SCALAR has no test coverage

MarketResult has three members (YES, NO, SCALAR) but every test fixture uses "yes". A single parametrized case or a dedicated assertion in the golden-wire test for "scalar" would close this gap.


Nit — reassembler merge when a terminal fragment hits a pending buffer

The scenario where last_fragment=None (field absent) arrives for a symbol that already has pending fragments is treated identically to last_fragment=True. This is correct per the protocol ("absent = Y"), but it's the subtlest path through add() and isn't explicitly exercised. Worth one line in the existing test_reassembler_standalone parametrize, e.g., a third case that first buffers a non-final fragment for the same symbol, then delivers a last=None terminal — confirming the merge fires.


Already correct / no action needed

  • market_result: str | None vs MarketResult enum: the inbound-raw-string convention is documented and consistent with the rest of the FIX message layer. StrEnum equality makes the test assertions work correctly ("yes" == MarketResult.YES).
  • tot_num_market_settlement_reports modeled but not reconciled: by design, noted in the class docstring; [FIX] Dictionary↔model drift test for the FIX message set (analogous to REST TestRequestBodyDrift) #437 tracks the drift test.
  • receive_settlement_reports passthrough design (vs. the True if x else None pattern on the other flags): intentional — None here means "gateway default", not "False not sent". The session.py comment explains this clearly.
  • Export completeness (__all__, dispatch map, __init__.py): verified clean.

- settlement.py: clarify settlement_price — the value is in cents (2dp; 100.00 =
  full YES payout, 30.60 = scalar), DollarDecimal is just the no-float coercion
  type (same cents-on-prediction convention as the order-entry price), not dollars.
- tests: drop the type: ignore in the PT-stream test by narrowing each decoded
  message via isinstance (a misrouted dispatch now fails the test); assert the
  "without Symbol" warning in the no-symbol drop test; parametrize the
  multi-fragment terminal over True/None (exercises absent-terminal merge); add
  MarketResult YES/NO/SCALAR wire + round-trip coverage (SCALAR was untested).

No action (already correct per the review): market_result raw-str inbound
convention; tot_num receive-only (#437); receive_settlement_reports passthrough.

ruff + mypy --strict clean; 252 FIX tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jun 6, 2026

Copy link
Copy Markdown

Code Review — feat(fix): market-settlement / post-trade reports

Overall: High-quality, well-tested implementation. The reassembler design is honest about its limitations, the tri-state logon flag is handled correctly, and the 17 tests cover the critical edge cases. A few items worth addressing before merge.


Issues

1. DollarDecimal for a cents-denominated field (naming mismatch)

The PR docstring notes settlement_price holds cents (100.00 = 100c, not $100), but the type is DollarDecimal. Any consumer reading the field type alone will assume dollars and apply incorrect calculations. The docstring is the only safeguard and is easy to miss.

Consider either renaming the field to settlement_price_cents to signal the unit at the call site, or adding a settlement_price_dollars property. At minimum, add a comment in the test that makes the unit explicit. This is the only field in the PR with a semantic unit mismatch from its type name.


2. GH issue numbers in module docstrings violate CLAUDE.md conventions

Both kalshi/fix/messages/settlement.py and kalshi/fix/settlement.py open with (GH #429) in the module docstring. CLAUDE.md: "Don't reference the current task, fix, or callers ... those belong in the PR description and rot as the codebase evolves." The issue number belongs in the commit/PR, not the module.


3. Tri-state comment placement (nit)

In settlement.py, the comment "Terminal fragment (True or None)." appears after the is False branch rather than before it. Since the is False strict-check is non-obvious (why not not report.last_fragment?), a brief note before the guard would help future editors understand the three-state design without hunting for context.


4. market_result comparison against MarketResult enum in tests (nit)

test_settlement_report_market_result_values asserts rt.market_result == result where rt.market_result: str | None and result: MarketResult. This works via StrEnum string equality, but mypy strict flags the comparison. Prefer assert rt.market_result == wire (matches the declared type) or assert MarketResult(rt.market_result) == result (also validates the raw string is a valid enum member).


Strengths

  • Reassembler documents its limits clearly. Symbol-keyed with no TotNum reconciliation, sequential-same-symbol assumption, orphaned-buffer persistence until clear(), idempotent re-emit — all stated. The clear() warning for non-empty buffers is good observability.
  • Tri-state logon flag handled correctly. Direct pass-through for receive_settlement_reports (vs. True if X else None for the other flags) lets False survive to the wire to opt PT out. The comment explains why.
  • Test coverage is thorough. Golden wire, two-party nested-group round-trip (negative collateral, sub-cent fee), trailing-LastFragment doc-example decode, dispatch routing, 6 reassembler cases (sequential same-symbol, drop-without-symbol warning, clear-when-empty silence), mock-acceptor integration, all three logon flag states.
  • model_copy for assembly is correct. Immutable update pattern, no aliasing surprises.
  • is False check is correct. Strict identity check properly distinguishes False (non-final), True (explicit terminal), and None (absent = terminal).

Verdict: Approve with items 1 and 2 addressed. Items 3 and 4 are nits. The reassembler correctness and test quality are solid.

@TexasCoding
TexasCoding merged commit 8c818e3 into main Jun 6, 2026
5 checks passed
@TexasCoding
TexasCoding deleted the feat/429-fix-settlement branch June 6, 2026 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FIX] Market Settlement / Post-Trade (prediction)

1 participant