Skip to content

release: 3.1.0 — spec sync 3.19.0 → 3.20.0 (events.fee_changes, is_block_trade, WS event_fee_update)#386

Merged
TexasCoding merged 2 commits into
mainfrom
spec-sync/3.20.0
Jun 4, 2026
Merged

release: 3.1.0 — spec sync 3.19.0 → 3.20.0 (events.fee_changes, is_block_trade, WS event_fee_update)#386
TexasCoding merged 2 commits into
mainfrom
spec-sync/3.20.0

Conversation

@TexasCoding

Copy link
Copy Markdown
Owner

Summary

OpenAPI + AsyncAPI spec sync v3.19.0 → v3.20.0, Closes #385.

The audited change set is broader than the issue's headline: upstream re-published 3.20.0 with three extra additive changes after the drift issue was filed, so the OpenAPI checksum in #385 is stale (a37111… → live b72a2a…). The AsyncAPI checksum still matches (2f72d0…). All changes below were found via a structural diff of the live specs against the committed snapshots, not just the issue body.

New public surface

Change SDK
New GET /events/fee_changes (paginated) events.fee_changes() / events.fee_changes_all() (sync + async) + EventFeeChange model
New is_block_trade query param on /markets/trades + /historical/trades kwarg on markets.list_trades / list_all_trades / list_trades_all and historical.trades / trades_all
New required Trade.is_block_trade response field is_block_trade: bool (strict — missing key raises)
New WS event_fee_update message on the existing market_lifecycle_v2 channel EventFeeUpdateMessage / EventFeeUpdatePayload; subscribe_market_lifecycle() now yields MarketLifecycleMessage | EventFeeUpdateMessage

EventFeeChange / EventFeeUpdatePayload carry fee_type_override and fee_multiplier_override, both present-but-None when an override is cleared. Channel count stays 11event_fee_update is a second message type on an existing channel, not a new channel.

Spec relaxations that need no facade change (audited, intentional)

  • CreateOrderV2Request.client_order_id relaxed to optional upstream — SDK keeps it required by design for V2 idempotency (docstring clarified). Body-drift compares wire names only, so no test impact.
  • EventData.product_metadata relaxed to optional — retired the now-obsolete server_omits_despite_required exclusion (spec and SDK now agree; the field stays dict | None because the demo server still omits it, guarded by test_parses_when_server_omits_product_metadata).
  • New ApiKeyScope / FeeType enums — SDK keeps scopes / fee_type as str for forward-compat.

Housekeeping

  • 3.0.1 → 3.1.0 (pyproject.toml, kalshi/__init__.py).
  • CHANGELOG entry; README + docs/index.md banners → 99 operations, OpenAPI v3.20.0.
  • Regenerated kalshi/_generated/models.py (gitignored, local reference only).
  • Docs updated: events / markets / historical / websockets resource pages, plus CLAUDE.md API-reference line.

Test plan

  • ruff check . clean · mypy kalshi/ strict clean (78 files)
  • ✅ Full unit suite: 3011 passed. New tests: events fee_changes (sync+async happy/filter/limit-bound/paginate), is_block_trade serialization (true/false/omitted), Trade.is_block_trade parse + missing-raises, WS event_fee_update dispatch (override set + cleared).
  • ✅ Contract param/body drift + WS drift (EventFeeUpdatePayload exercised by additive/required/coverage/field-type checks).
  • ✅ Integration coverage harness green (new methods registered) + GET /events/fee_changes validated live against the demo API.
  • ℹ️ Two unrelated, pre-existing local-only failures are not from this PR and pass in CI: 8 historical integration tests (demo server returning HTTP 500 on /historical/*) and 4 from_env tests (local .env leak; verified green with .env absent, i.e. the CI environment).

🤖 Generated with Claude Code

… WS event_fee_update

Syncs OpenAPI + AsyncAPI snapshots to v3.20.0 (#385). Upstream re-published
3.20.0 with extra additive changes after the drift issue was filed, so the
OpenAPI checksum in #385 is stale; the AsyncAPI checksum still matches.

New surface
- events.fee_changes() / fee_changes_all() (sync + async) for the new paginated
  GET /events/fee_changes; EventFeeChange model. Both override fields are
  present-but-None when an override is cleared.
- is_block_trade query param on markets.list_trades / list_all_trades (+ legacy
  list_trades_all) and historical.trades / trades_all.
- Trade.is_block_trade — new spec-required, non-nullable bool response field.
- WebSocket event_fee_update message on the existing market_lifecycle_v2 channel
  (EventFeeUpdateMessage / EventFeeUpdatePayload). subscribe_market_lifecycle now
  yields MarketLifecycleMessage | EventFeeUpdateMessage; channel count stays 11.

Spec relaxations that need no facade change (audited)
- CreateOrderV2Request.client_order_id relaxed to optional upstream; SDK keeps it
  required by design for V2 idempotency (docstring clarified).
- EventData.product_metadata relaxed to optional; retired the now-obsolete
  server_omits_despite_required exclusion (spec and SDK now agree).
- New ApiKeyScope / FeeType enums; SDK keeps str for forward-compat.

Housekeeping
- Version 3.0.1 → 3.1.0; CHANGELOG, README + docs banners (99 operations,
  OpenAPI v3.20.0); regenerated kalshi/_generated/models.py (gitignored).
- Tests: unit (events fee_changes sync+async, is_block_trade serialization,
  Trade field parse/missing, WS dispatch set+cleared), contract drift +
  WS drift, and live integration coverage for the new methods.

Closes #385

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

claude Bot commented Jun 4, 2026

Copy link
Copy Markdown

Code Review — PR #386: release 3.1.0 (spec sync v3.19.0 → v3.20.0)

Overview

Clean, well-scoped spec sync. Four changes from upstream (new GET /events/fee_changes, is_block_trade on trade endpoints, Trade.is_block_trade required field, and WS event_fee_update on the market_lifecycle_v2 channel) are all correctly implemented and fully tested.


Code Quality & Conventions ✅

All new code follows project conventions:

  • EventFeeChange uses extra="allow" (forward-compatible, correct).
  • Trade.is_block_trade: bool has no default — a missing key raises ValidationError, which is exactly right for a spec-required non-nullable field (matches the existing pattern on taker_outcome_side).
  • _bool_param(is_block_trade) is used for serialization, correctly preserving False as an explicit query param rather than dropping it. The test test_is_block_trade_filter_serializes verifies this edge case.
  • _event_fee_changes_params follows the existing helper pattern in resources/events.py exactly.
  • Deprecation: is_block_trade is threaded into the deprecated list_trades_all as well — correct for consistency.
  • server_omits_despite_required exclusion for Event.product_metadata is properly retired with an explanatory comment rather than a silent delete.

Potential Issues

1. Behavioral breaking change on subscribe_market_lifecycle (worth flagging to users)

The return type change from AsyncIterator[MarketLifecycleMessage] to AsyncIterator[MarketLifecycleMessage | EventFeeUpdateMessage] is a runtime breaking change for existing subscribers that access .msg.market_ticker or similar fields without first discriminating on .type:

# Code written against 3.0.x — will raise AttributeError at runtime now
# if an event_fee_update message arrives on the channel:
async for msg in client.ws.subscribe_market_lifecycle():
    print(msg.msg.market_ticker)  # EventFeeUpdatePayload has no market_ticker

This is correctly documented in the CHANGELOG and PR body, and the 3.1.0 minor bump is defensible since event_fee_update is an additive upstream change. Just worth considering whether a prominent migration note (e.g., in docs/websockets.md under the new callout) would help, since "minor" version usually doesn't signal "must update all consumers of this method."

2. EventFeeUpdateMessage / EventFeeUpdatePayload not in top-level kalshi namespace

EventFeeChange is exported from kalshi/__init__.py, but EventFeeUpdateMessage and EventFeeUpdatePayload are only in kalshi.ws.models. If other WS models (MarketLifecycleMessage, FillMessage, etc.) are in the top-level namespace, these should be too — users calling subscribe_market_lifecycle() will need to import EventFeeUpdateMessage to do isinstance checks, and the natural discovery path is from kalshi import EventFeeUpdateMessage.

3. Minor: seq: int | None = None on EventFeeUpdateMessage is undocumented

The field is added as an SDK extension (not in the AsyncAPI schema), which is fine. But the docstring says "NO required seq" without explaining why the field is present at all. A one-liner like # seq not emitted by the server; accepted if present for forward-compat would clarify intent for future editors.


Test Coverage ✅

Thorough:

  • TestEventsFeeChanges: page parsing, cleared-override (None fields), filter params, limit validation, pagination via side_effect.
  • test_is_block_trade_filter_serializes: True/False/omitted all verified.
  • test_is_block_trade_missing_raises: regression guard for the required-but-no-default pattern.
  • WS dispatch: test_dispatch_event_fee_update and test_dispatch_event_fee_update_cleared cover both set and cleared states.
  • Integration tests registered in the coverage harness; fee_changes validated against live demo.

One small observation: test_fee_changes_all in the integration suite breaks out after 2 items (if count >= 2: break). This is fine given live-API uncertainty, but it means the pagination path isn't exercised in integration. The unit test_fee_changes_all_paginates with respx.mock side_effect does cover it.


Security ✅

No security concerns. No new auth paths, no user input passed unsanitized. limit is validated via existing _validate_limit.


Summary

The spec sync itself is solid and the implementation is clean. The two actionable items before merging:

  1. Verify whether EventFeeUpdateMessage / EventFeeUpdatePayload should be in the top-level kalshi namespace for consistency with other WS models. If yes, add them.
  2. Consider adding a short migration note in docs/websockets.md (or the CHANGELOG) about discriminating on .type in subscribe_market_lifecycle() consumers — the existing callout is good but users scanning the changelog entry alone may miss it.

Item 1 is a correctness/ergonomics issue; item 2 is optional polish. Everything else looks good to merge.

- EventFeeUpdateMessage.seq: inline comment explaining it's accepted-if-present
  for forward-compat/parity (server doesn't emit it). (review item 3)
- docs/websockets.md: promote the event_fee_update callout to a migration
  warning with a discriminate-on-.type example (naive .msg.market_ticker
  access raises AttributeError). (review item 1)
- CHANGELOG: add the same behavioral note for changelog-only readers.

Not changed: EventFeeUpdateMessage/Payload stay in kalshi.ws.models (review
item 2). No WS message/payload models are exported top-level — REST models
(EventFeeChange) are top-level by convention, WS models live under
kalshi.ws.models. Promoting only these two would be inconsistent.

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

Copy link
Copy Markdown
Owner Author

Thanks for the review — addressed in acaffba.

Item 1 (migration note) — done. Promoted the docs/websockets.md callout from a note to a warning with a concrete discriminate-on-.type example (showing that naive .msg.market_ticker access raises AttributeError), and added the same behavioral note to the CHANGELOG bullet so changelog-only readers catch it.

Item 3 (seq rationale) — done. Added an inline comment on EventFeeUpdateMessage.seq explaining it's accepted-if-present for forward-compat / parity with the sibling MarketLifecycleMessage on the same channel, and that the server doesn't emit it.

Item 2 (top-level export) — verified, no change (the premise doesn't hold). No WS message/payload models are exported from the top-level kalshi namespace — grep -E "Message|Payload" kalshi/__init__.py is empty; MarketLifecycleMessage, FillMessage, TradeMessage, etc. all live only in kalshi.ws.models. The asymmetry you spotted is the actual convention: REST models (EventFeeChange, like Event / SeriesFeeChange) are top-level, WS models are namespaced under kalshi.ws.models. Promoting only EventFeeUpdateMessage / EventFeeUpdatePayload to the top level would make them the sole WS models there. isinstance checks use from kalshi.ws.models import EventFeeUpdateMessage, consistent with every other WS model. Happy to revisit if there's appetite for surfacing the whole WS model set top-level, but that'd be a separate, broader change.

The integration-pagination observation is fair — the live fee_changes_all test breaks after 2 items for runtime safety; the cursor-walk itself is covered by the respx.mock side_effect unit test test_fee_changes_all_paginates.

@claude

claude Bot commented Jun 4, 2026

Copy link
Copy Markdown

Code Review: PR 386 - release 3.1.0 spec sync. Two substantive items: (1) missing is_block_trade serialization test for historical.trades - the markets test covers it but _historical_trades_params is a separate path; (2) Trade.is_block_trade as required with no default is a breaking change for hand-rolled Trade fixtures, worth a Breaking note in CHANGELOG even at minor version. Minor: populate_by_name inconsistency between EventFeeUpdatePayload (WS, has it) and EventFeeChange (REST, missing it) - no-op since neither has aliases but inconsistent with WS model convention. What is good: dispatch routing is correct (MESSAGE_MODELS keyed by type, not channel), False param preservation via _bool_param tested, product_metadata exclusion cleanup is clean, contract map entries present for both new models, WS migration warning is actionable, seq forward-compat choice is well-reasoned, 3011 tests passing.

@TexasCoding
TexasCoding merged commit cd9ae6f into main Jun 4, 2026
5 checks passed
@TexasCoding
TexasCoding deleted the spec-sync/3.20.0 branch June 4, 2026 00:47
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.

1 participant