feat: spec sync 3.20.0 → 3.21.0 + nightly drift issue automation

[TexasCoding]

Summary

Spec sync from upstream OpenAPI v3.20.0 → v3.21.0 plus implementation of the four new 3.21.0 endpoints, and closes the nightly spec-drift CI gap.

Why

The nightly Spec Drift Detection workflow has been failing since upstream moved to 3.21.0, but no issue was being created — issue-filing lived only in the weekly Weekly Spec Sync (Mondays only). This PR fixes the drift and wires the nightly job to open a tracking issue on failure.

Drift fixes (all additive)

• events.list / list_all — tickers filter
• communications quotes — min_ts / max_ts
• perps MarginMarket — settlement_mark_price / liquidation_mark_price / reference_price (nested TickerPrice)
• perps MarginPosition — required subaccount
• WS ErrorPayload — market_tickers

New endpoints (sync & async, fully tested)

• client.communications.block_trade_proposals — list / list_all / create / accept (+ 5 models)
• account.volume_progress() (+ 3 models)

Infra

• spec-drift.yml: a report-failure job opens (and dedups on a stable marker) a spec-drift tracking issue on scheduled failure; least-privilege permissions, SHA-pinned actions
• TestWsSpecDrift fixture → @staticmethod (removes the 60-error cascade under -W error::UserWarning)
• new _post_void transport helper for 204 endpoints; minimal_kwargs test helper supports AwareDatetime

Release

• 4.0.0 → 4.1.0, re-vendored specs, CHANGELOG ## 4.1.0, doc banners → v3.21.0 / 104 operations

Verification

• uv run ruff check . ✓
• uv run mypy kalshi/ ✓ (157 files)
• full unit suite: 4308 passed, 3 skipped
• nightly-strict contract mirror (pytest tests/test_contracts.py -W error::UserWarning): 670 passed

🤖 Generated with Claude Code

[claude]

Code Review — PR #449: spec sync 3.20.0 → 3.21.0 + nightly drift issue automation

Overview

This is a clean, well-structured spec-sync PR. All additions are additive (new endpoints, new fields on existing models), the dual sync/async pattern is followed consistently, and the drift machinery change is a real improvement. A few things worth flagging before merge.

---

Positive notes

• _post_void is the right primitive for 204 endpoints. It mirrors _put/_delete semantics and plugs cleanly into both the sync and async transports.
• ProposeBlockTradeRequest uses StrictInt with ge=1 for price_centi_cents and centicount. Correct — these are plain integer wire fields, not FixedPointCount, and the validation boundary is good.
• AcceptBlockTradeProposalRequest serializes to {} when called with no args, which is the right behavior for a required: false request body — the body still forces Content-Type: application/json, matching the _post_json pattern.
• Contract map / drift tests are fully wired: METHOD_ENDPOINT_MAP, BODY_MODEL_MAP, _contract_map.py, and EXCLUSIONS are all updated. The new endpoints will fail CI immediately if the spec drifts again.
• @staticmethod on TestWsSpecDrift._load is the correct fix — a self-less class-scoped fixture will emit UserWarning under -W error::UserWarning because pytest doesn't inject self for class-scoped fixtures.
• Test coverage for new endpoints is solid: model parse/serialize, happy path, error paths, auth guards, and async equivalents all present.

---

Issues to resolve before merge

1. actions/checkout version comment is wrong (CI / security hygiene)

In .github/workflows/spec-drift.yml, the report-failure job has:

uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd  # v6.0.2

actions/checkout is at v4.x — there is no v6.2.0. The SHA pinning itself is safe (it won't float), but the comment is wrong and will confuse future maintainers trying to audit or update it. Verify the SHA maps to the correct actions/checkout release and fix the comment.

2. Settlement.market_result enum silently removes void — worth a CHANGELOG note

The updated spec changes:

# before
enum: ['yes', 'no', 'scalar', 'void']
# after
enum: ['yes', 'no', 'scalar']

The PR body calls all changes "additive," but this removes a value from a response-side enum. The Settlement model itself uses model_config = {"extra": "allow"} so existing runtime code won't break (the field is a plain str), but type-checked code or pattern-matching on "void" will silently stop being covered by the enum. This should be documented in the CHANGELOG under "Changed" (even if the enum is informational rather than enforced).

3. TickerPrice.populate_by_name is dead config

class TickerPrice(BaseModel):
    price: DollarDecimal
    ts_ms: int
    model_config = {"extra": "allow", "populate_by_name": True}

populate_by_name: True has no effect without any aliases. Remove it to match the style of other simple response models in this codebase (e.g., BlockTradeProposal also has it unnecessarily, but at least there populate_by_name is set because it has extra = "allow" and was likely copy-pasted from a model with aliases).

4. BlockTradeProposal.populate_by_name is also unused

Same observation — BlockTradeProposal has no Field(alias=...) or validation_alias. populate_by_name: True is a no-op here. Remove to reduce config noise.

---

Minor observations (non-blocking)

• Trailing space in spec — /account/limits summary becomes "Get Account API Limits " (trailing space). Worth cleaning up to keep the spec tidy.

• status: str on BlockTradeProposalsResource.list/list_all — the spec doesn't define an enum for status, so str is correct, but a note in the docstring about observed values (e.g., "open", "expired", "accepted") would help callers.

• No missing-required-fields test for ProposeBlockTradeRequest — there are tests for zero-price and extra-key, but no test that omitting a required field (e.g., market_ticker) raises ValidationError. Not strictly required given the existing pattern, but would complete the model test suite.

• Spec YAML formatting changes — the bulk of the diff is cosmetic (>- → single-line, > → |). These are semantically equivalent but make the diff very hard to audit for actual functional changes. For future spec syncs, consider doing the reformatting in a separate commit.

---

Summary

The implementation is solid and follows all project conventions. The two items worth resolving before merge are the actions/checkout version comment (a security/auditability concern) and a CHANGELOG note about the Settlement.market_result enum change. The populate_by_name cleanup is optional polish.

🤖 Generated with Claude Code

[claude]

Code Review — PR #449: spec sync 3.20.0 → 3.21.0 + nightly drift issue automation

Overview

This PR syncs the SDK against OpenAPI v3.21.0 and AsyncAPI, adds four new endpoints (block_trade_proposals.list/list_all/create/accept, account.volume_progress), fixes five existing drift points, and closes the nightly spec-drift CI gap by having failures open (and dedup) a GitHub tracking issue. All changes are additive. Full suite at 4308 tests confirms clean state.

---

Strengths

• Test coverage is thorough. Every new endpoint gets a happy path, error path, auth guard, and async mirror. Model tests cover edge cases (extra="forbid" enforcement, zero-value rejections, missing required fields). This matches the project standard well.
• _post_void pattern is clean. Correctly mirrors _put/_delete void semantics — fires the request, discards the body, lets error statuses raise via the transport. The async counterpart is structurally identical.
• CI security. SHA-pinned actions on report-failure (which holds issues:write and runs against upstream content) while keeping the read-only drift-check on tag aliases is the correct threat model. The marker-based dedup avoids duplicate issues and appends dated comments for recurring failures.
• @staticmethod fix on TestWsSpecDrift._load. The root cause of the 60-error cascade is correctly identified — pytest class-scoped fixtures must not accept self when they don't use it under -W error::UserWarning.
• AwareDatetime support in _request_model_fixtures.py. Necessary for the new BlockTradeProposal model; avoids a future class of _OVERRIDES entries.

---

Issues / Concerns

1. expiration_ts: Any in resource method signatures (minor type hygiene)

The underlying ProposeBlockTradeRequest.expiration_ts is typed AwareDatetime, so Pydantic will coerce/validate at construction time, but the resource method signature offers IDEs and callers no type guidance. Strings work because Pydantic coerces them, but the overloads could at least use AwareDatetime | str to match how similar fields are handled elsewhere. Low runtime risk, but a missed DX opportunity.

2. MarginPosition.subaccount added as a required field

Adding a required field to a response model is a compatibility break for any downstream user constructing MarginPosition directly (e.g. in their own test mocks). The project-owned fixtures are correctly updated. This is spec-accurate and the right call, but the CHANGELOG entry files it under "Added" — it should appear under "Breaking" or at least a "Note" sub-section so consumers upgrading from 4.0.0 know to update their mocks.

3. Block-trade models absent from _contract_map.py — intentional?

The three new account-volume models are registered with generated counterparts:

ContractEntry(sdk_model="kalshi.models.account.AccountVolumeProgress", ...)

But none of the five new communications models appear. If there are no generated OpenAPI counterparts for the block-trade models, omitting them is correct — but a short comment in the map (or a line in the PR description) would make the absence clearly intentional rather than an oversight.

4. Spec YAML trailing space (cosmetic, vendored)

/account/limits summary reads "Get Account API Limits " with a trailing space. No CI impact, just a minor spec nit.

5. Shell MARKER hardening (low risk)

MARKER is a literal constant so there is no injection risk today. Worth adding readonly MARKER=... to make the invariant explicit — if MARKER ever gains a single quote the jq contains() expression silently breaks.

---

Minor Nits

• _post_void always sends Content-Type: application/json even when json=None. For accept this is correct (body always serializes to at least {}), but as a general helper it could mislead future callers who pass no body. Matching _put/_delete (which don't force the header independently) would be safer for long-term reuse.
• TestAsyncBlockTradeProposals has no @pytest.mark.asyncio on its methods. Presumably asyncio_mode = "auto" is set in pyproject.toml, which is fine, but worth confirming for anyone reading the tests in isolation.
• Comment # Communications endpoints in the spec appears above /multivariate_event_collections/{collection_ticker} — looks like a copy/paste artifact from the reformat.

---

Summary

This is a well-executed spec sync. New endpoints follow established patterns exactly, test coverage is solid, and the CI automation is properly secured. The three substantive items (type annotation on expiration_ts, CHANGELOG classification of the subaccount field change, and the intentionality of missing contract map entries) are the only things worth a second look before merging.

🤖 Generated with Claude Code

[claude]

Code Review

Overall this is a high-quality, well-structured spec-sync PR. The implementation is clean, conventions are followed throughout, and test coverage is solid. A few issues worth addressing before merge.

---

Potential Breaking Change — Semver Concern

MarginPosition.subaccount added as a required field (kalshi/perps/models/portfolio.py)

Adding a required field to a response model is a backward-incompatible change for any downstream code that constructs MarginPosition directly (test mocks, fixture data, serialization round-trips). The CHANGELOG calls this out, which is good, but the PR bumps 4.0.0 → 4.1.0 (minor). Under strict semver, this warrants a major bump (5.0.0) or the field should default to 0 / None with a deprecation note.

Given the field is spec-required and Kalshi will always include it in API responses, the real-world blast radius is limited to test code — but it's still a breaking change by contract. Recommend either bumping to 5.0.0 or declaring subaccount: int = 0 to absorb the compat concern.

---

Missing BlockTradeProposalStatusLiteral

BlockTradeProposalsResource.list(status: str | None) uses bare str for the status filter, which is inconsistent with the rest of the codebase (e.g., QuoteStatusLiteral, RfqStatusLiteral). The spec almost certainly enumerates the valid values (open, expired, executed, cancelled, etc.).

# consistent with existing pattern
BlockTradeProposalStatusLiteral = Literal["open", "expired", "accepted", "executed"]

This is a minor ergonomics issue since str still works, but it degrades IDE autocompletion and static analysis for callers.

---

_post_void sends Content-Type: application/json when body could be empty

# kalshi/resources/_base.py
self._transport.request(
    "POST", path, params=params, json=json,
    headers={"Content-Type": "application/json"},  # ← always set
    extra_headers=extra_headers,
)

When json=None, the transport still advertises Content-Type: application/json with no body. In practice this is never triggered — accept() always passes {} — but it's technically wrong. Existing _post_json avoids this by always having a body. Worth either adding an assertion (assert json is not None) or conditionally setting the header.

---

GitHub Actions SHA pin — verify against actual tag

- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd  # v6.0.2

SHA pins are the right call for a job with issues:write permissions. Recommend independently verifying this SHA resolves to the v6.0.2 tag on the actions/checkout repository before merging, since a wrong SHA + comment would silently use the wrong commit.

---

Test Coverage — Gaps

TestAsyncBlockTradeProposals missing @pytest.mark.asyncio — these pass because asyncio_mode = "auto" is presumably set project-wide, but the decorators are present on other async test classes (e.g., TestAsyncAccountVolumeProgress). Worth standardizing for clarity.

No test for create() with optional subaccount/subtrader fields for the seller side — the sync test test_accepts_request_model covers buyer_subaccount, but seller_subtrader_id / seller_subaccount go untested on the wire. Minor gap, not blocking.

No test that _post_void sends the 204 response without trying to parse JSON — the accept() tests verify the request body is sent correctly, but don't explicitly confirm a None return rather than a JSON parse attempt. Adding assert result is None covers the semantic intent.

---

Minor Observations (non-blocking)

• _join_tickers helper for events.py — clean reuse; the import from _base is correct.
• @staticmethod fix on TestWsSpecDrift._load — correct fix; the UserWarning cascade under -W error was a real CI hazard.
• AwareDatetime support in _request_model_fixtures.py — necessary and correctly added for the ProposeBlockTradeRequest contract tests.
• report-failure job dedup logic — the stable HTML marker approach (<!-- spec-drift-bot: nightly-spec-drift-tracker -->) is clean and avoids duplicate issues without content-fingerprinting drift.
• ProposeBlockTradeRequest mutual exclusivity note — correctly documented in the docstring as a known spec limitation rather than SDK enforcement.

---

Summary

Severity	Issue
🔴 Consider	MarginPosition.subaccount required field is a breaking change; evaluate major bump
🟡 Recommend	Add BlockTradeProposalStatusLiteral type alias for consistency
🟡 Recommend	Verify actions/checkout SHA independently before merge
🟢 Minor	_post_void Content-Type defensiveness
🟢 Minor	Async test decorator inconsistency

The core logic, model design, contract coverage, and CI improvements are all solid. Address the semver question and the status literal before merging.