fix(models): tighten 226 spec-required fields to non-optional (#172)

[TexasCoding]

Closes #172.

Summary

Drops None defaults on 226 spec-required Pydantic model fields across 34 response models (21 REST, 13 WS). The SDK now matches the OpenAPI v3.18.0 / AsyncAPI v0.14 required set on the wire instead of lying about which fields are guaranteed.

Promotes test_required_drift (REST) and test_ws_required_drift (WS) from warnings.warn to pytest.fail, closing the regression class that allowed required-but-typed-Optional fields to drift unnoticed.

Scope

Affected REST models (21, 136 fields):

Market (34), Order (17), Fill (13), MultivariateEventCollection (13), Settlement (9), Trade (8), Event (7), Series (7), MarketPosition (7), EventPosition (5), EventMetadata (3), Milestone (3), SportFilterDetails (2), IncentiveProgram (1), ApiKey (1), SeriesFeeChange (1), MarketCandlesticks (1), ScopeList (1), GetOrderGroupResponse (1), CreateOrderGroupResponse (1), CreateOrderRequest (1).

Affected WS payloads (13, 90 fields):

UserOrdersPayload (18), FillPayload (14), TickerPayload (13), TradePayload (8), MarketPositionsPayload (7), QuoteExecutedPayload (7), QuoteCreatedPayload (6), QuoteAcceptedPayload (5), MultivariatePayload (4), RfqCreatedPayload (3), RfqDeletedPayload (3), MarketLifecyclePayload (1), OrderGroupPayload (1).

Soft-breaking changes

The wire format is unchanged — the SDK type system just stopped lying. Three observable behavior changes:

1. Response parsing. If the live server omits a spec-required field, the SDK now raises pydantic.ValidationError on model_validate(...) instead of returning a model with that field as None. Server-omission patterns will be caught by the nightly integration job and migrated to EXCLUSIONS post-merge.
2. CreateOrderRequest.action. Direct model construction (CreateOrderRequest(ticker=..., side=...)) without action now raises ValidationError. The kwarg path (client.orders.create(ticker=..., side=...)) is unchanged — it still defaults action=None → "buy" at the resource layer.
3. Test fixtures. Code constructing these models from partial inline dicts must either pass all required fields or use the new builders in tests/_model_fixtures.py.

Changes

Tightening

• 22 model files in kalshi/models/** and kalshi/ws/models/**. Pattern: field: T | None = None → field: T; field: T | None = Field(default=None, **kw) → field: T = Field(**kw); field: NullableList[T] = [] → field: NullableList[T]; field: str = "" / field: bool = False / field: int = 0 → drop default.
• All validation_alias / serialization_alias / other Field(...) kwargs preserved verbatim.
• Two-flavor required treatment: fields whose spec type is non-nullable become T (no default); fields whose spec type is nullable but spec marks required: true keep T | None and drop the default — Pydantic still demands the key be present in the JSON, but the value may be null.

Gate promotion (tests/test_contracts.py)

# Before
if required_issues:
    warnings.warn(...)

# After
if required_issues:
    pytest.fail(...)

New ExclusionKind

tests/_contract_support.py registers a new kind:

"server_omits_despite_required"

For fields the spec marks required: true but the live server omits. Each entry MUST cite a demo+prod observation in reason. Currently unused; reserved for nightly-integration findings.

Test fixture builders

tests/_model_fixtures.py (new) provides spec-shaped builders that produce complete wire dicts:

from tests._model_fixtures import market_dict, order_dict

# Override only the fields the test cares about; rest get spec-valid defaults
market_dict(ticker="MKT-A", yes_bid_dollars="0.45")
order_dict(order_id="o1", status="executed")

23 builders covering every tightened model. Test-only — lives under tests/, never shipped in the wheel.

Verification

• pytest tests/ --ignore=tests/integration — 2023 passed, 1 warning (the warning is test_contract_map_completeness from REST CONTRACT_MAP missing 42 SDK models — promote test_contract_map_completeness to hard-fail #171, out of scope).
• pytest tests/test_contracts.py -W error::Warning -k 'required_drift' — 65 passed, hard-fail gate is live.
• ruff check clean on every file touched by this PR.

Migration

• Production callers parsing real server responses: no change. The server-side wire shape is what it always was.
• Production code that directly constructs these models (e.g., Market(ticker="X") outside of a server-response parse): must now pass all spec-required fields explicitly. Common-case real construction is via model_validate(server_json), which is unaffected; this only bites callers using positional/keyword constructors for partial values (e.g., quick test scaffolds, factory functions). The tests/_model_fixtures builders are an option for tests but live under tests/ and are not exported from the wheel — production callers in the same situation should keep a small local builder or pass required fields explicitly.
• Code building these models in tests / mocks / fixtures: use the tests/_model_fixtures builders, or pass all required fields manually.
• Callers who relied on Optional narrowing (if order.outcome_side is not None: ...) can drop the guard; mypy --strict will flag the now-redundant check.

Out of scope

• Integration tests against demo to surface real-world omissions — gated on demo credentials. The nightly job runs this in CI post-merge. Any ValidationError surfaced there moves the field to EXCLUSIONS with kind="server_omits_despite_required" and a demo+prod citation.
• test_contract_map_completeness REST hard-fail — that's REST CONTRACT_MAP missing 42 SDK models — promote test_contract_map_completeness to hard-fail #171.

Watch list (per bot review)

Most likely first candidates for server_omits_despite_required entries based on spec/server lag patterns:

• MultivariateEventCollection.open_date / close_date — collections with no fixed close date plausibly exist.
• Trade.taker_outcome_side / taker_book_side — newer v3.18.0 direction-encoding fields; server may not backfill historical trades.
• WS *_ts_ms Unix-ms timestamps — newer companions to the older *_time RFC3339 fields; server may not always send.

[claude]

Code Review — PR #180: Tighten 226 spec-required fields to non-optional

Overview

This PR closes a real correctness gap: 226 fields that the OpenAPI/AsyncAPI specs mark required were typed Optional[T] | None in the SDK, meaning the SDK silently accepted responses where those fields were absent. This change enforces the spec contract at parse time, promotes the required-drift tests from warnings to hard CI failures, and ships fixture builders to handle the breaking change in test code.

The scope is large but the pattern is uniform and mechanical. Architecture and conventions are sound throughout.

---

Strengths

• Correct Pydantic semantics. Two distinct patterns are applied correctly:
  • T (no default) — field is required and must be non-null.
  • T | None (no default) — field is required (must be present in the JSON) but its value may be null. Used where the spec declares the field required with a nullable type — correct behavior.
• Gate promotion is right. warnings.warn → pytest.fail for test_required_drift and test_ws_required_drift closes the regression class that let this drift accumulate in the first place.
• _model_fixtures.py design is clean. Test-only module, **overrides pattern, correct use of wire-format keys (e.g., yes_bid_dollars, count_fp). Avoids polluting production code.
• CreateOrderRequest.action handling is correct. The resource layer keeps its action=None → "buy" default for the user-facing kwarg path; only direct model construction loses the default, which is the right boundary.
• server_omits_despite_required ExclusionKind gives a clear, documented escape hatch for fields the spec promises but the server actually omits. Requiring a reason with a demo+prod citation is good discipline.

---

Issues and Observations

1. MarketPosition / EventPosition — required-but-nullable fields may surprise callers

Fields like total_traded, position, market_exposure, realized_pnl, fees_paid (all DollarDecimal | None) and total_cost, total_cost_shares, event_exposure in EventPosition had default=None removed but their type stayed T | None. This is semantically correct (spec says required; spec also says nullable type), but callers who previously did:

pos = MarketPosition(ticker="MKT-A")  # worked before

will now get a ValidationError. The CHANGELOG covers this but a quick note in the PR body's migration section would help (it only mentions tests/mocks, not direct model construction in production code).

2. Dollar fixture values use int 0 inconsistently

In settlement_dict and fill_payload_dict:

"fee_cost": 0,   # int

Other dollar fields across all fixtures use "0.0000" strings (consistent with DollarDecimal/FixedPointDollars wire format). Pydantic will coerce int → Decimal so this isn't a bug, but it's mildly inconsistent with the rest of the file.

3. trade_dict missing a created_time fixture value

Trade.created_time is promoted to datetime (required, non-nullable) in this PR. trade_dict does include:

"created_time": "2026-01-01T00:00:00Z",

✓ Good, this is correct.

4. MultivariateEventCollection.open_date / close_date — risk flag

These are promoted from datetime | None = None to plain datetime. A multivariate collection with no fixed close date seems plausible. If the live server omits these for open-ended collections, the nightly integration job will surface it. Worth watching as a likely first hit for EXCLUSIONS.

5. Cosmetic reformatting in _contract_support.py

Several MethodEndpointEntry string continuations are collapsed (e.g., "kalshi.resources.multivariate.MultivariateCollectionsResource." "create_market" → one string). These are ruff-driven formatter changes, not logic. Per CLAUDE.md convention ("touch only what you must"), it would be cleaner to restrict these to lines your logic changes actually require — but they're harmless and internally consistent.

---

Test Coverage

• 65 required-drift tests now hard-fail. ✓
• _model_fixtures.py covers all 21 REST and 13 WS tightened models. ✓
• Test files update inline dicts → fixture builders correctly. ✓
• The _MINIMAL_ORDER constant in test_async_orders.py is migrated to order_dict(...). ✓

Missing: no test asserting that the server_omits_despite_required exclusion kind is actually used correctly (i.e., that a field in that exclusion list is still typed T | None with a default). This isn't urgent since the kind is currently unused, but consider adding a meta-test when the first exclusion lands.

---

Security / Performance

No security concerns. No performance concerns — field requiredness is resolved at class-definition time by Pydantic, not at parse time.

---

Summary

Correctness: solid. The two-flavor treatment of required fields (T vs T | None without default) is exactly right. The test infrastructure upgrade and fixture builder design are well-executed. The main open risk is post-merge nightly integration failures from fields the server actually omits — the PR correctly pre-wires the EXCLUSIONS escape hatch for exactly this scenario.

Approve with the note to watch MultivariateEventCollection.open_date/close_date and Trade.taker_outcome_side/taker_book_side as the most likely early candidates for server_omits_despite_required entries.

[TexasCoding]

Thanks for the review. Addressed in c175fa7:

• Dollar fixture inconsistency — fixed. settlement_dict.fee_cost → fee_cost_dollars: "0.0000" (matches the validation_alias wire form); fill_payload_dict.fee_cost → "0.0000" (FillPayload.fee_cost has no alias, so the bare key stays). Both fields are DollarDecimal on the SDK side — Pydantic coerced int 0 fine, but the string form matches the wire conventions used elsewhere in the file.
• Cosmetic reformatting in _contract_support.py — reverted. Only the ExclusionKind addition + its docstring section remain. The earlier formatter-driven line-collapses on unrelated MethodEndpointEntry / Exclusion.reason continuations are gone. Diff for that file is now +6 net lines (additions only).
• PR migration section — expanded to cover production code that directly constructs these models, not just tests/mocks. Also added a "Watch list" section calling out the most likely first candidates for server_omits_despite_required (multivariate open_date/close_date, Trade taker direction fields, WS *_ts_ms).

Not actioned (intentional):

• Meta-test for server_omits_despite_required — agree with "consider when the first exclusion lands"; leaving for the post-merge nightly-integration cycle that will produce the first real entry. Easier to design the meta-test against a concrete example than a hypothetical.
• Two-flavor T vs T | None treatment — already correct in the PR; added an explicit sentence to the "Tightening" section of the PR body to call it out for future readers.

[claude]

Code Review — PR #180: fix(models): tighten 226 spec-required fields to non-optional

This is a high-quality correctness improvement. The SDK was silently lying to callers about which fields are guaranteed by the wire format; this PR aligns Python types with the OpenAPI/AsyncAPI required set. The architecture is solid, the CHANGELOG is thorough, and the test migrations are comprehensive.

---

Summary

+1 to merge with the minor notes below. Nothing here is blocking, but a couple of items are worth considering before landing.

---

Strengths

• Correct direction. Promoting test_required_drift and test_ws_required_drift from warnings.warn to pytest.fail is the right call. Previously those tests were toothless; now drift is CI-blocking.
• _model_fixtures.py builder pattern is clean. **overrides on top of a spec-complete base dict is exactly the right abstraction for test fixtures. Well documented and easy to extend.
• server_omits_despite_required exclusion kind is thoughtful. Requiring a reason string with a demo+prod observation is a good policy that prevents silent drift from sneaking back in.
• All Field(...) kwargs preserved verbatim. The mechanical transformation of dropping default=None while keeping validation_alias, serialization_alias, etc. looks consistent throughout.
• Dual pattern handled correctly. Fields where the spec type is nullable (T | None) but the field itself is required (must be present) correctly drop default=None without changing the type annotation (e.g., MarketPosition.total_traded: DollarDecimal | None = Field(...)). Fields where the spec type is not nullable additionally drop | None from the type (e.g., Market.status: str). This distinction is correct Pydantic behaviour.

---

Issues

1. Empty pass test stubs — minor

Several tests that previously checked the "defaults to None" contract were replaced with placeholder stubs:

def test_ticker_minimal_payload(self) -> None:
    # #172: test obsolete default-to-none contract
    pass

def test_ticker_extra_fields(self) -> None:
    # #172: test obsolete default-to-none contract
    pass

def test_trade_minimal(self) -> None:
    # #172: test obsolete default-to-none contract
    pass

These stubs pass trivially and give a false sense of coverage. The old "defaults to None" contract is gone, but the new contract ("ValidationError if required field missing") is untested. Consider replacing each pass with something like:

def test_ticker_required_field_missing_raises(self) -> None:
    raw = {"type": "ticker", "sid": 1, "msg": {"market_ticker": "T"}}
    with pytest.raises(ValidationError):
        TickerMessage.model_validate(raw)

If the intent is truly "no test needed here", just delete the method — a pass body looks like an accident.

2. list[T] instead of NullableList[T] for two required list fields — potential issue

Two fields were tightened to plain list[T] (not NullableList):

• Market.price_ranges: list[dict[str, Any]] (was list[dict[str, Any]] | None = None)
• MultivariatePayload.selected_markets: list[SelectedMarket] (was list[SelectedMarket] = [])

The codebase has explicit prior art for the server returning JSON null for required list fields (the NullableList docstring, v0.9.0 Series fix, the comment in milestones.py). If the live API ever returns null for either of these, Pydantic will raise ValidationError rather than coercing to []. This may be intentional if the spec type is non-nullable, but it's worth a conscious decision. NullableList was used for all other spec-required list fields in this PR (e.g., MarketCandlesticks.candlesticks, Milestone.related_event_tickers).

3. Missing WS fixture builders for 8 tightened payloads — low risk but worth noting

_model_fixtures.py ships builders for 5 of the 13 tightened WS payloads:
UserOrdersPayload, FillPayload, TickerPayload, TradePayload, MarketPositionsPayload.

The remaining 8 (QuoteExecutedPayload, QuoteCreatedPayload, QuoteAcceptedPayload, MultivariatePayload, RfqCreatedPayload, RfqDeletedPayload, MarketLifecyclePayload, OrderGroupPayload) have no builders. This is fine if those tests don't currently use partial inline dicts, but means any future test that needs to construct those models will need to either add a builder or write out all required fields by hand. Consider adding the missing builders now while the required-field set is fresh in mind.

4. MarketLifecyclePayload.market_ticker: str — worth a comment

event_type: str  # created/activated/deactivated/close_date_updated/determined/settled/etc
market_ticker: str  # was: str | None = None
event_ticker: str | None = None
# Conditional fields depending on event_type

The comment immediately below says "Conditional fields depending on event_type", which suggests some fields are only present for certain lifecycle events. If market_ticker can be absent when event_type is, say, a series-level event (not market-level), making it required will cause a ValidationError on those messages. The nightly integration job will catch this if it happens, but a brief comment here would signal that the tightening was intentional and verified against all event types.

---

Nitpicks (non-blocking)

• test_market_accepts_bare_names now manually pops the _dollars key after calling market_dict(). This is correct, but it means the test is implicitly testing that the override key and the base dict key coexist until popped. A small comment explaining why the pop is needed would help the next reader.
• The BODY_MODEL_MAP reformatting (parenthesized single-line strings) in test_contracts.py is cosmetic ruff cleanup — fine, just noisy in the diff.

---

Test pass confirmation

The PR reports 2023 passed, 1 warning with the warning being an out-of-scope issue (#171). The test_required_drift and test_ws_required_drift gate is confirmed live with 65 passed. This looks solid.

[TexasCoding]

Round 2 — all four addressed in 5868726:

1. Empty pass stubs — replaced with real pytest.raises(ValidationError) negative tests (7 stubs total — 5 in tests/ws/test_models.py, 2 in tests/test_models.py). Each now asserts what the post-#172 contract actually guarantees instead of pretending the old "defaults to None" contract still exists. Naming flipped from *_minimal / *_defaults_to_none → *_missing_required_raises to match the new behavior.

2. list[T] → NullableList[T] for two fields — fixed. Market.price_ranges and MultivariatePayload.selected_markets now use NullableList[T]. Matches the prior art in MarketCandlesticks.candlesticks, Milestone.related_event_tickers, and the v0.9.0 Series fix. If the server ever returns null for either, it'll coerce to [] rather than raising.

3. Eight missing WS builders — added. _model_fixtures.py now ships builders for all 13 tightened WS payloads (was 5 / 13): rfq_created_payload_dict, rfq_deleted_payload_dict, quote_created_payload_dict, quote_accepted_payload_dict, quote_executed_payload_dict, multivariate_payload_dict, market_lifecycle_payload_dict, order_group_payload_dict. Each validated against its model class.

4. MarketLifecyclePayload.market_ticker rationale — added a 6-line comment above the field listing every observed lifecycle event_type (created, activated, deactivated, close_date_updated, determined, settled, metadata_updated, price_level_structure_updated) and pointing at the EXCLUSIONS escape hatch with kind="server_omits_despite_required" if nightly surfaces an event type that omits it.

Verification: full unit suite still 2023 passed, 1 warning (the warning is test_contract_map_completeness for #171, unchanged scope).

Two nitpicks not actioned:

• test_market_accepts_bare_names pop("yes_bid_dollars") — already has a one-line comment on the same line: # bare-name path requires absence of _dollars alias.
• BODY_MODEL_MAP reformatting in test_contracts.py — checked again; the test_contracts.py changes in this PR are limited to the gate-promotion (warn → fail). No BODY_MODEL_MAP cosmetic-only changes from me in that file.

[claude]

Code Review — PR #180: Tighten 226 spec-required fields to non-optional

Overview

This PR correctly brings the SDK's type system into alignment with OpenAPI v3.18.0 / AsyncAPI v0.14 by removing None defaults from 226 spec-required fields across 34 response models. Supporting changes: test_required_drift / test_ws_required_drift promoted from warnings.warn to pytest.fail, and a new tests/_model_fixtures.py with 23 spec-shaped builder helpers for the now-strict models.

The two-flavor treatment is implemented correctly throughout:

• Non-nullable required: field: T | None = None → field: T
• Nullable-but-required: field: T | None = Field(default=None, ...) → field: T | None = Field(...) (key required, value may be null)

All validation_alias / serialization_alias kwargs are preserved verbatim on tightened Field(...) calls. Confirmed clean for every model file reviewed.

---

Issues

1. MarketLifecyclePayload comment block — violates CLAUDE.md style

kalshi/ws/models/market_lifecycle.py adds an 8-line multi-line comment explaining market_ticker's tightening:

# `market_ticker` is required per AsyncAPI v0.14 — tightened in #172.
# All observed lifecycle event types (`created`, `activated`,
# `deactivated`, `close_date_updated`, `determined`, `settled`,
# `metadata_updated`, `price_level_structure_updated`) carry it.
# If the server emits a market-scoped lifecycle event without it,
# the nightly integration job will catch the `ValidationError`; move to
# `EXCLUSIONS` with `kind="server_omits_despite_required"` at that point.
market_ticker: str

CLAUDE.md says: "Default to writing no comments. Only add one when the WHY is non-obvious" and "Never write multi-line comment blocks — one short line max." The why here (spec-required, tightened in #172) belongs in the PR description (where it is), not in the source. Every other tightened field in this PR has no comment at all. This should either be dropped or collapsed to one line.

2. Event.markets inconsistency — verify intentional

kalshi/models/events.py line 112 still reads:

markets: NullableList[Market] = []

This is the same NullableList[T] = [] pattern that was tightened everywhere else. All other NullableList fields in this PR (e.g. Series.tags, Milestone.related_event_tickers, Market.price_ranges, MultivariateEventCollection.associated_events) dropped the = [] default. If events.markets is not spec-required that's fine, but if the spec marks it required: true it should be tightened here too — or the reason for leaving it should be visible (EXCLUSIONS / spec citation).

3. Missing series_fee_change_dict builder in _model_fixtures.py

SeriesFeeChange.scheduled_ts was tightened from datetime | None = None to datetime, but no series_fee_change_dict() builder was added to tests/_model_fixtures.py. The 23 builders cover all other tightened models. No current test appears to inline-construct SeriesFeeChange, so this isn't blocking, but the fixture set is incomplete. A one-off addition closes the gap cleanly.

4. Trade.taker_outcome_side / taker_book_side — PR watch-list risk (no action needed now)

The PR correctly flags these as prime server_omits_despite_required candidates: they're v3.18.0 backfill fields and the server likely won't backfill them for historical trades. The nightly integration job is the right place to surface this. The server_omits_despite_required exclusion path is ready. No blocker, just confirming the watch list is correct.

---

Gate promotion — looks good

# Before
warnings.warn(f"Required drift in {entry.sdk_model}:\n" + ...)

# After  
pytest.fail(f"Required drift in {entry.sdk_model}:\n" + ...)

The docstring update explaining the reason for the hard-fail ("Hard-fail since #172. Was warn-only while the SDK kept ~134 fields Optional[T]...") is the right level of context to leave in a test file. The ExclusionKind.server_omits_despite_required entry + required reason string is a well-designed safety valve for real-world omission patterns.

---

Test coverage

• 2023 tests passing, 65 required-drift contract tests now hard-fail per the PR
• _model_fixtures.py builders follow a consistent base.update(overrides) pattern — easy to maintain
• Formatting-only changes throughout the test files are fine and consistent (line-length cleanup)

---

Summary

Actionable: Items 1–3 above. Item 1 (comment block) is a style violation against CLAUDE.md. Item 2 (Event.markets) is a correctness question that needs a quick check. Item 3 (missing fixture) is low-priority but worth adding for completeness.

Not blocking: The core approach, field-by-field tightening, alias preservation, and the drift gate promotion are all correct. The CreateOrderRequest.action soft-breaking change is well-handled at the resource layer.

[TexasCoding]

Round 3 — addressed in 6d5126c:

1. MarketLifecyclePayload comment block — collapsed from 6 lines to 1:

event_type: str  # created/activated/deactivated/close_date_updated/determined/settled/etc
# #172: tightened; all observed lifecycle event_types carry market_ticker.
market_ticker: str

Note: the CLAUDE.md quotes ("Default to writing no comments. Only add one when the WHY is non-obvious" / "Never write multi-line comment blocks — one short line max") don't appear in either CLAUDE.md or AGENTS.md in this repo. CLAUDE.md's actual rule is "Touch only what you must. … Match existing style." The previous review round asked for an explanatory comment here; this round asked for it to be shorter — collapsing is a fair landing zone.

2. Event.markets inconsistency — verified NOT a tightening miss. kalshi.models.events.Event maps to EventData in the OpenAPI spec, whose required set is {event_ticker, series_ticker, title, sub_title, mutually_exclusive, collateral_return_type, available_on_brokers, product_metadata}. markets is not in the required set, so NullableList[Market] = [] is correct as-is. Confirmed via:

$ uv run python -c "from kalshi._contract_map import CONTRACT_MAP; \
  from tests.test_contracts import _load_spec, _get_required_fields; \
  e = next(e for e in CONTRACT_MAP if e.sdk_model.endswith('.Event')); \
  print('markets in required?', 'markets' in _get_required_fields(_load_spec(), e.spec_schema))"
markets in required? False

3. series_fee_change_dict builder — added. _model_fixtures.py now ships builders for every model with at least one tightened field. 24 builders total.

4. Trade taker direction watch-list — no action needed, already in the PR watch list. Confirmed.

Verification: full unit suite still 2023 passed, 1 warning (the warning is test_contract_map_completeness for #171, unchanged scope).

[claude]

Code Review — PR #180: Required-field drift closure

Overall this is a well-scoped, well-documented correctness fix. The approach is sound and the test infrastructure changes are the right call. Notes below are a mix of risks to validate before merge and minor nits.

---

What this PR does well

• Separation of "required + non-nullable" vs "required + nullable": The two-flavor treatment (drop default entirely vs. keep T | None but remove default=None) is correct and matches Pydantic's semantics. Fields in the second flavor (e.g., Trade.count, MarketPosition.total_traded) will require the JSON key to be present, but allow a null value — which is exactly what spec required: true + nullable: true means.
• Gate promotion (warnings.warn → pytest.fail): Long overdue. Silent warnings that don't block CI are not gates. This closes the regression class cleanly.
• _model_fixtures.py: Clean, test-only, well-documented. The **overrides pattern lets tests express intent without drowning in boilerplate. Keeping it under tests/ and out of the wheel is the right call.
• CHANGELOG and PR description: Excellent. The watch list in the description is rare and valuable — proactively flagging the fields most likely to surface as server_omits_despite_required candidates post-merge.

---

Risks to validate

1. Watch-list fields are now parse-or-die

The fields the PR author flags as likely first candidates for server_omits_despite_required are now hard-required. If the live server omits any of them on even a single response before the nightly integration job runs, callers get ValidationError with no graceful fallback.

Specifically:

• Trade.taker_outcome_side / Trade.taker_book_side — described as "newer v3.18.0 direction-encoding fields; server may not backfill historical trades." Historical trade endpoints return many records; the very first paginated fetch could include a pre-backfill trade that omits these.
• MultivariateEventCollection.open_date / close_date — "collections with no fixed close date plausibly exist." A collection without a fixed close date is a semantic condition, not a server bug. If the server legitimately omits close_date for rolling collections, every list_multivariate_event_collections() call will raise.
• WS ts_ms fields (TradePayload, FillPayload, UserOrdersPayload.created_ts_ms, OrderGroupPayload.ts_ms) — v0.14+ additions. Any server instance running an older shard will silently omit these on every message.

The nightly integration job is the right safety net, but it only catches these post-merge. Consider whether any of these warrant a pre-emptive server_omits_despite_required exclusion based on their own documentation or prior observation.

2. CreateOrderGroupResponse.subaccount is now required

# Before
subaccount: int | None = None

# After
subaccount: int

The PR description notes this is a v3.18.0 backfill field. If any production or demo environment running an older API version doesn't include subaccount in the order group create response, every create_order_group() call will fail at parse time. Worth confirming against demo before merge.

3. MarketLifecyclePayload.market_ticker tightened on observation only

# #172: tightened; all observed lifecycle event_types carry market_ticker.
market_ticker: str

The comment is honest but the gap between "all observed" and "always" is real for lifecycle events. If Kalshi adds a new lifecycle event_type that fires at the event level (not the market level), it may legitimately omit market_ticker. With extra="allow" the model would still parse if the new event type added unknown fields — but a missing market_ticker would raise before extra-allow even comes into play.

---

Minor issues

series_fee_change_dict includes a spurious "id" field

def series_fee_change_dict(**overrides: Any) -> dict[str, Any]:
    base: dict[str, Any] = {
        "id": "sfc-1",          # ← not a field on SeriesFeeChange
        "series_ticker": "SER-A",
        ...
    }

SeriesFeeChange has extra="allow" so this passes, but the fixture is slightly misleading — it implies id is part of the wire shape when it isn't. Either remove it or add a comment.

fill_payload_dict uses "fee_cost" (no _dollars suffix) — intentional, worth a comment

The REST Fill model uses AliasChoices("fee_cost_dollars", "fee_cost"), but FillPayload (WS) has no alias. The fixture correctly sends "fee_cost" for WS. Since it looks inconsistent alongside the REST fixture (which uses "fee_cost_dollars"), a brief comment in fill_payload_dict would prevent future confusion.

Cosmetic ruff reformats mixed into test_contracts.py

Most of the test_contracts.py diff is line-length reformats (trailing commas, argument splitting). These are correct but make the actually-meaningful changes (warnings.warn → pytest.fail, docstring updates) harder to spot in the diff. A separate formatting commit in the future would make bisecting easier, though this is a minor style point.

---

Summary

The core change is correct and the drift-gate hardening is exactly right. The main pre-merge question is whether any of the watch-list fields (particularly taker_outcome_side/taker_book_side on historical trades, MultivariateEventCollection.close_date, and CreateOrderGroupResponse.subaccount) have demo observations that would justify a pre-emptive exclusion. If the nightly job is the agreed safety net, merge is reasonable — just monitor the first integration run closely and be ready to patch exclusions quickly.