Skip to content

feat: finalize additive-drift hardening — backfill comms/order_groups + WS payloads + flip warn\u2192fail (#161, #162, #163)#168

Merged
TexasCoding merged 3 commits into
mainfrom
pr3-5/finalize-additive-drift-hardening
May 19, 2026
Merged

feat: finalize additive-drift hardening — backfill comms/order_groups + WS payloads + flip warn\u2192fail (#161, #162, #163)#168
TexasCoding merged 3 commits into
mainfrom
pr3-5/finalize-additive-drift-hardening

Conversation

@TexasCoding

Copy link
Copy Markdown
Owner

Closes #161, #162, #163. Final three PRs of the response-side spec-drift hardening stack (#157), bundled into one branch per user request (implement in one branch if possible).

Three logical commits preserved for review:

Why one PR: the three issues form a single logical unit. PR5 has a hard dependency on PR3 + PR4 ("otherwise this PR turns 53 warnings into 53 failures"). With all three bundled in one branch in the correct order, the flip is safe. Total diff ~535 lines across 19 files — still tractable for a single review.


PR3 (#161) — communications + order_groups backfill

8 new optional fields:

  • RFQ (+1): creator_subaccount
  • Quote (+3): creator_subaccount, rfq_creator_subaccount, post_only
  • OrderGroup (+1): exchange_index
  • GetOrderGroupResponse (+1): exchange_index
  • CreateOrderGroupResponse (+2): subaccount, exchange_index

All typed int | None (bool | None for post_only) matching spec types exactly. 3 new test classes / 8 tests in tests/test_models.py.

PR4 (#162) — WebSocket payload backfill (33 fields, 11 payloads, 8 files)

Three themes:

*_ts_ms Unix-ms timestamps — spec promotes ms-integer as the primary timestamp form. Existing ts fields stay for back-compat. No auto-conversion — latency-sensitive callers get the int directly.

Payload New fields
TickerPayload price, ts_ms
FillPayload outcome_side, book_side, ts_ms
OrderbookDeltaPayload ts_ms
TradePayload taker_outcome_side, taker_book_side, ts_ms
UserOrdersPayload outcome_side, book_side, self_trade_prevention_type, created_ts_ms, last_updated_ts_ms, expiration_ts_ms
MarketLifecyclePayload additional_metadata, floor_strike, price_level_structure, yes_sub_title
OrderGroupPayload ts_ms
RfqCreatedPayload mve_collection_ticker, mve_selected_legs
RfqDeletedPayload event_ticker, contracts, target_cost
QuoteCreatedPayload event_ticker, yes_contracts_offered, no_contracts_offered, rfq_target_cost
QuoteAcceptedPayload event_ticker, yes_contracts_offered, no_contracts_offered, rfq_target_cost

Direction encodingoutcome_side / book_side / taker_*_side reuse SideLiteral / BookSideLiteral / SelfTradePreventionTypeLiteral from kalshi.models.orders. Mirrors REST backfill from #159.

MVE + RFQ context echoes on Communications payloads — server now echoes the parent-RFQ context on quote events so subscribers don't need a lookup roundtrip.

One test class TestWsV0140Backfill in tests/ws/test_models.py with 11 tests — one per payload covering at least one new field. Added QuoteCreatedPayload to module imports.

PR5 (#163) — promote additive drift to hard-fail

Flipped warn → pytest.fail (3 sites)

  • TestSpecDrift.test_additive_drift — REST response model additive drift
  • TestWsSpecDrift.test_ws_additive_drift — WS payload additive drift
  • TestWsSpecDrift.test_ws_contract_map_completeness — WS payload coverage

Kept warn-only

  • test_required_drift / test_ws_required_drift — issue explicitly defers required-but-optional policy as a separate ≈204-entry decision.
  • test_contract_map_completeness (REST) — deviation from issue text. 42 pre-existing unmapped REST classes (sub-models like Candlestick / OrderbookLevel, V2 request/response family, internal containers like PositionsResponse / IndexedBalance / Deposit / Withdrawal). The issue assumed this check would pass after ErrorPayload registration; only the WS check was at zero. Promoting REST completeness here would require mapping all 42 — out of scope. Filed as inline comment for follow-up.

Registered ErrorPayload

kalshi.ws.models.base.ErrorPayload → spec errorResponsePayload in WS_CONTRACT_MAP. Clears the WS completeness warning so the flip is safe.

Envelope extra="allow" policy

Applied model_config = {"extra": "allow"} to 14 WS classes lacking it (envelopes + helpers). Payloads were already covered by #143; this completes the WS surface.

New test_ws_models_use_extra_allow in tests/test_model_extra_policy.py iterates every BaseModel in kalshi.ws.models.* and pins the policy.

CHANGELOG

Unreleased section summarizing the full #157 stack (#159#163).


Verification

$ uv run pytest tests/ --ignore=tests/integration -q
2023 passed, 35 warnings in 129.91s

Up from 1965 baseline (+58 tests). All 35 warnings are test_required_drift / test_ws_required_drift (kept warn-only per issue). Zero additive-drift warnings remaining.

$ uv run mypy kalshi/                            -> Success: no issues found in 76 source files
$ uv run ruff check .                            -> All checks passed!

Smoke tests (warn→fail confirmed via field deletion)

Test 1 — delete Market.exchange_index locally, run test_additive_drift[Market]:

FAILED tests/test_contracts.py::TestSpecDrift::test_additive_drift[Market]
E   Failed: Additive drift in kalshi.models.markets.Market:
E     - Spec field 'exchange_index' has no SDK mapping

Exit 1 (was UserWarning). ✅ Reverted.

Test 2 — delete ErrorPayload from WS_CONTRACT_MAP locally, run test_ws_contract_map_completeness:

FAILED tests/test_contracts.py::TestWsSpecDrift::test_ws_contract_map_completeness
E   Failed: WS payload models without contract map entries:
E     - kalshi.ws.models.base.ErrorPayload

Exit 1 (was UserWarning). ✅ Reverted.


Stack closure

This PR closes the response-side spec-drift hardening stack tracked in #157. After merge:

Future work (out of scope, file a follow-up issue if desired):

  • Map the 42 unmapped REST sub-models / V2 family / internal containers and promote test_contract_map_completeness to hard-fail.
  • Required-but-Optional drift policy decision (≈204 entries on test_required_drift).

#161)

PR3 of the response-side spec-drift hardening stack. 8 new optional
fields across 5 mid-tier response models. Eliminates 8 additive-drift
warning bullets and clears 5 test_additive_drift cases.

Fields added
------------

`RFQ` (+1): creator_subaccount
`Quote` (+3): creator_subaccount, rfq_creator_subaccount, post_only
`OrderGroup` (+1): exchange_index
`GetOrderGroupResponse` (+1): exchange_index
`CreateOrderGroupResponse` (+2): subaccount, exchange_index

Design decisions
----------------

- All new fields are `int | None = None` (or `bool | None` for post_only),
  matching the spec types exactly.
- post_only mirrors CreateQuoteRequest.post_only (already on request side
  from v2.1.0) — server echoes when the caller is the quote creator.
- subaccount/exchange_index on CreateOrderGroupResponse echoes the routing
  context the server resolved.
- All new fields appended at end of each model with `# v3.18.0 backfill
  (#161)` marker. Matches PR1/PR2 pattern.

Tests
-----

3 new test classes in tests/test_models.py (8 tests). Hoisted RFQ/Quote/
OrderGroup/* to module-level imports. Annotated _MINIMAL dict on
TestQuoteV3180Fields as `ClassVar[dict[str, str]]` per ruff RUF012.

Closes #161.
PR4 of the response-side spec-drift hardening stack. 33 new optional
fields across 11 WebSocket payload models. Clears all 11
test_ws_additive_drift cases.

Key themes
----------

1. **`*_ts_ms` Unix-ms timestamps** — spec promotes ms integer as the
   primary form (TickerPayload.ts_ms, OrderbookDeltaPayload.ts_ms,
   TradePayload.ts_ms, FillPayload.ts_ms, OrderGroupPayload.ts_ms,
   UserOrdersPayload.{created,last_updated,expiration}_ts_ms). Existing
   `ts: int|str|datetime` fields stay for back-compat. NO auto-conversion
   — surface both so latency-sensitive callers get the int form directly.

2. **Direction encoding** — outcome_side / book_side /
   taker_outcome_side / taker_book_side on FillPayload, TradePayload,
   UserOrdersPayload. Mirrors the REST-side backfill from PR1 (#159).
   Reuses SideLiteral / BookSideLiteral from kalshi.models.orders.

3. **MVE + RFQ context echoes** on Communications payloads —
   mve_collection_ticker, mve_selected_legs, contracts_fp,
   target_cost_dollars, yes_contracts_offered_fp,
   no_contracts_offered_fp, rfq_target_cost_dollars across
   RfqCreatedPayload, RfqDeletedPayload, QuoteCreatedPayload,
   QuoteAcceptedPayload.

Field counts per file
---------------------

ticker.py            +2: price, ts_ms
fill.py              +3: outcome_side, book_side, ts_ms
orderbook_delta.py   +1: ts_ms
trade.py             +3: taker_outcome_side, taker_book_side, ts_ms
user_orders.py       +6: outcome_side, book_side, self_trade_prevention_type,
                         created_ts_ms, last_updated_ts_ms, expiration_ts_ms
market_lifecycle.py  +4: additional_metadata, floor_strike,
                         price_level_structure, yes_sub_title
order_group.py       +1: ts_ms
communications.py   +13: across 4 payload classes (RFQ + Quote echoes)

Design decisions
----------------

- `ts_ms` is plain `int | None`, NOT datetime — spec gives ms since
  epoch as an integer. Do NOT auto-convert.
- `MarketLifecyclePayload.floor_strike` typed as `Decimal | None` (spec:
  `type: number, format: double`) — matches the REST Market.floor_strike
  precedent rather than the issue's `str | None` text (which contradicted
  the spec).
- `additional_metadata` typed as `dict[str, Any] | None` — spec says
  object with subproperties.
- `mve_selected_legs` typed as `list[dict[str, object]] | None` — matches
  CommunicationsMessage.msg's existing `dict[str, object]` pattern,
  avoids the `typing.Any` import.
- WS communications additions use SDK short names with `validation_alias`
  to match the existing per-payload pattern (e.g. `contracts` aliased to
  `contracts_fp`/`contracts`) — keeps sibling consistency.

Tests
-----

One new test class `TestWsV0140Backfill` in tests/ws/test_models.py with
11 tests — one per payload covering at least one new field. Defaults-to-
None paths are already exercised by the existing per-payload `*_minimal`
tests. Added `QuoteCreatedPayload` to the test_models imports.

Closes #162.
…ad; WS envelopes extra=allow (#163)

PR5 of the response-side spec-drift hardening stack. With backfills
from #159#162 landed, additive-drift warnings are at zero. Flip
`warnings.warn(...)` → `pytest.fail(...)` so future spec syncs that
introduce unmodelled fields fail CI instead of slipping through five
rounds of review (the v2.1.0 `Balance.balance_dollars` failure mode).

Also closes two ROADMAP carry-overs:

1. Register `kalshi.ws.models.base.ErrorPayload` in `WS_CONTRACT_MAP`
   (spec name: `errorResponsePayload`). Clears the WS completeness
   warning so it can be promoted.

2. Apply `model_config = {"extra": "allow"}` to every WS envelope and
   helper class (14 classes across 9 files). PR #143 covered payloads;
   this completes the WS surface. Test
   `test_ws_models_use_extra_allow` (new) iterates every BaseModel in
   `kalshi.ws.models.*` and pins the policy.

Changes
-------

tests/test_contracts.py:
  - Flip 3 warning sites to `pytest.fail`:
    - TestSpecDrift.test_additive_drift
    - TestWsSpecDrift.test_ws_additive_drift
    - TestWsSpecDrift.test_ws_contract_map_completeness
  - REST `test_contract_map_completeness` stays warn-only (the issue
    expected it to pass, but 42 pre-existing sub-models / V2 family
    classes / internal containers remain unmapped — out of scope for
    this PR; tracked for a follow-up).
  - Required-drift and required-ws-drift stay as warnings (issue
    explicitly defers as separate ≈204-entry policy decision).
  - Update module docstring to reflect new FAIL semantics.

kalshi/_contract_map.py:
  - Add ContractEntry for ErrorPayload → errorResponsePayload.
  - Drop `ErrorPayload` from the intentionally-excluded comment.

kalshi/ws/models/*.py:
  - base.py: add extra=allow on SubscriptionInfo, ErrorPayload,
    SubscribedMessage, UnsubscribedMessage, ErrorMessage.
  - 8 per-channel files: add extra=allow on every *Message envelope
    plus SelectedMarket helper in multivariate.py.

tests/test_model_extra_policy.py:
  - Add `test_ws_models_use_extra_allow` parameterized across every
    BaseModel discovered in `kalshi.ws.models.*` modules. Pins the
    policy so future model additions that forget the config fail CI.

CHANGELOG.md:
  - New Unreleased section summarizing the full drift hardening stack
    (#159#163).

Verification
------------

uv run pytest tests/ --ignore=tests/integration -q
  -> 2023 passed, 35 warnings (all required-drift, kept warn-only).
     Up from 1965 baseline (+58 new tests: PR3 +8, PR4 +11, PR5 +38
     extra-policy entries + +1 net for hoisted imports/refactors).
uv run mypy kalshi/                            -> Success: no issues
uv run ruff check .                            -> All checks passed!

Smoke-tested both flips:
  - Delete Market.exchange_index locally
    -> test_additive_drift[Market] now FAILS with pytest.fail
       (was a UserWarning). Exit 1, message: "Additive drift in
       kalshi.models.markets.Market: Spec field 'exchange_index' has
       no SDK mapping".
  - Delete ErrorPayload from WS_CONTRACT_MAP locally
    -> test_ws_contract_map_completeness FAILS with pytest.fail.
       Exit 1, message: "WS payload models without contract map
       entries: kalshi.ws.models.base.ErrorPayload".
Both reverted after smoke test.

Closes #163.
@claude

claude Bot commented May 19, 2026

Copy link
Copy Markdown

Code Review — PR #168: finalize additive-drift hardening

Overview

This bundles three logical sub-PRs (#161/#162/#163) that together form a safe unit: the first two backfill the missing spec fields, and the third flips additive drift from warn → hard-fail. Bundling is the right call — flipping first would have broken CI on the 53 gaps, and splitting the flip into a fourth PR would have left a CI window where warnings silently accumulate again.


PR3 (#161) — communications + order_groups backfill ✅

Clean field additions. int | None for subaccount/exchange_index, bool | None for post_only — all match spec types. AliasChoices not needed here since these field names don't have _fp/_dollars variants, so bare field declarations are correct.

One minor issue — mve_selected_legs typing in communications.py:

# current
mve_selected_legs: list[dict[str, object]] | None = None

# suggested
mve_selected_legs: list[dict[str, Any]] | None = None

dict[str, object] makes value access typed as object, so code like legs[0]["event_ticker"].upper() would fail mypy. dict[str, Any] is the idiomatic choice for untyped JSON blobs — it's what market_lifecycle.py already uses for additional_metadata. communications.py just needs from typing import Any added.


PR4 (#162) — WebSocket payload backfill ✅

The design choices here are good:

  • No auto-conversion between ts (seconds/RFC3339) and ts_ms (Unix ms). Correct — latency-sensitive callers need the raw int; auto-conversion would silently corrupt timestamps for anyone still using ts.
  • SideLiteral / BookSideLiteral / SelfTradePreventionTypeLiteral reused from kalshi.models.orders instead of raw str. This is an improvement over the existing taker_side: str | None pattern — Pydantic will now reject "invalid" at parse time.
  • AliasChoices("contracts_fp", "contracts") / ("target_cost_dollars", "target_cost") patterns correctly applied.

Minor test inconsistency:

RfqDeletedPayload is imported inline inside test_rfq_deleted_rfq_context while all other payload types in the same file are imported at the module level. Should be moved to the top-level imports block alongside RfqCreatedPayload.

# tests/ws/test_models.py — move this out of the test body
from kalshi.ws.models.communications import RfqDeletedPayload

PR5 (#163) — promote warn → fail ✅

The flip itself is minimal and correct. A few things worth calling out:

test_ws_models_use_extra_allow design: The dynamic discovery approach (pkgutil.iter_modules + inspect.getmembers) runs at module import time (not inside a test), which is the right place — it means pytest parametrizes before collection starts and any future model added without extra="allow" fails CI immediately rather than at a runtime assertion. This is the correct pattern.

REST contract map completeness kept as warn-only: The inline comment and PR description both explain the 42 pre-existing unmapped entries clearly. The deviation from the original issue text is documented with rationale. This is fine — the important surface (WS completeness, which was at zero) is now hard-fail.

ErrorPayload registration: The notes field explaining the naming mismatch (errorResponsePayloadErrorPayload) is helpful context for future maintainers.


Test coverage

+58 tests reaching 2023 total. Coverage looks solid:

One gap: TestWsV0140Backfill doesn't have a mve_selected_legs=None test for RfqCreatedPayload, but the PR notes this is covered by existing minimal tests. Acceptable, but worth a comment pointing to which test covers it.


Security / Performance

No security concerns. The _ws_model_classes() dynamic discovery is a one-time O(module count) operation at import time — negligible for CI.


Summary

Category Status
Correctness ✅ All fields match spec types; no auto-conversion; literals validated at parse
Conventions ✅ AliasChoices, DollarDecimal, extra="allow", model_config placement all correct
Test coverage ✅ 58 new tests; policy test auto-catches future regressions
Blocking issues None
Minor issues dict[str, object]dict[str, Any] in communications.py; inline import in test

Verdict: Approved with minor suggestions. The dict[str, object] type and the inline import are worth fixing but neither is blocking. The bundling strategy and the warn→fail flip sequencing are both correct.

@TexasCoding TexasCoding merged commit 8f98e63 into main May 19, 2026
5 checks passed
@TexasCoding TexasCoding deleted the pr3-5/finalize-additive-drift-hardening branch May 19, 2026 23:36
@TexasCoding TexasCoding mentioned this pull request May 20, 2026
TexasCoding added a commit that referenced this pull request May 20, 2026
Cuts v2.2.0 — the response-side spec drift hardening stack (#157).

Version bumps:
- pyproject.toml: 2.1.0 → 2.2.0
- kalshi/__init__.py: 2.1.0 → 2.2.0

Docs:
- CHANGELOG.md: finalize ## Unreleased → ## 2.2.0 — 2026-05-19 with a
  release-summary headline. The Added/Changed/Fixed entries written
  incrementally across #166#169 stay as-is.
- ROADMAP.md: add v2.2.0 to Shipped with a coverage summary; drop the
  two "Next milestone" items now done by this release (response-side
  drift detection; WS envelope extra=allow); add two new follow-ups
  (map remaining REST sub-models into CONTRACT_MAP; required-but-
  optional drift policy decision).
- CLAUDE.md: active milestone → post-v2.2.

Release scope (65 new optional fields + warn→fail flip + 1 bugfix):
- REST response models: Market +11, Order +8, Fill +4, Event +3,
  EventMetadata +2, Settlement +1, Trade +2, IncentiveProgram +1,
  RFQ +1, Quote +3, OrderGroup +1, GetOrderGroupResponse +1,
  CreateOrderGroupResponse +2 (#166, #167, #168).
- WS payload models: 33 fields across 11 payloads — Unix-ms timestamps
  (`*_ts_ms`), outcome_side/book_side direction encoding, MVE linkage,
  RFQ/Quote context echoes (#168).
- Test infrastructure: additive drift now hard-fails CI; unmapped WS
  models hard-fail; required-but-optional stays warn-only (~204
  entries, separate policy decision). EXCLUSIONS allowlist wired
  through response-side checks (#165). ErrorPayload registered in
  WS_CONTRACT_MAP; WS envelopes/helpers all use extra=allow (#168).
- Bugfix: dropped le=32 cap on 6 subaccount request fields; demo
  allocates ephemeral subaccount numbers above 32 (observed 41 in
  nightly, 44 locally). Spec defines no upper bound (#169).

No API breaks. All changes additive or relaxing. Bump is semver-minor.

Verification:
- uv run pytest tests/ --ignore=tests/integration -q
    -> 2023 passed, 35 warnings (all required-drift, kept warn-only)
- uv run pytest tests/integration/test_subaccounts.py::TestSubaccountsSync::test_transfer_between_subaccounts
    -> 1 passed against live demo (#164 unblocked end-to-end)
- uv run mypy kalshi/      -> Success: no issues found in 76 source files
- uv run ruff check .      -> All checks passed!
- kalshi.__version__ check -> 2.2.0

After merge, push tag v2.2.0 to trigger the release workflow
(.github/workflows/release.yml): build sdist+wheel → PyPI trusted-
publish → GitHub release with CHANGELOG section as notes.
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.

PR3: backfill v3.18.0 fields on communications / order_groups

1 participant