Skip to content

feat!: spec sync 3.23.0 → 3.24.0 (v7.0.0) — close spec-drift #467/#470#472

Merged
TexasCoding merged 4 commits into
mainfrom
reconcile/spec-3.24.0
Jul 10, 2026
Merged

feat!: spec sync 3.23.0 → 3.24.0 (v7.0.0) — close spec-drift #467/#470#472
TexasCoding merged 4 commits into
mainfrom
reconcile/spec-3.24.0

Conversation

@TexasCoding

@TexasCoding TexasCoding commented Jul 10, 2026

Copy link
Copy Markdown
Owner

Summary

Reconciles the SDK to upstream OpenAPI/AsyncAPI 3.24.0 — the real, deterministic drift behind the recurring #467 (nightly) and #470 (weekly) issues. Vendored specs are re-pinned to 3.24.0 and the hand-written models/resources updated to match; the full contract-drift suite goes green. Closes #467, #470.

⚠️ Stacked on #471 (harden/spec-drift-guardrails). This branch builds on the CI/test hardening PR and uses its new symbols (_UNIMPLEMENTED_ENDPOINTS, the operation-level coverage test). Merge #471 first; GitHub will then reduce this PR's diff to just the reconcile commits.

Changes

Breaking

  • Position-transfer price is fixed-point dollars, not integer cents. price_cents (int, 0–100) → price (Decimal, fixed-point dollars) on ApplySubaccountPositionTransferRequest, SubaccountTransfer, and subaccounts.transfer_position() (sync + async). Upstream renamed the wire field price_centsprice (FixedPointDollars).
    • Migration: transfer_position(..., price_cents=50)transfer_position(..., price=Decimal("0.50")).
    • The old client-side le=100c cap is gone — OrderPrice guards non-negativity + the $0.0001 tick and leaves the upper bound to the server (matches CreateOrderRequest).

Deprecated (soft)

  • exchange.announcements() now emits DeprecationWarning. Upstream removed GET /exchange/announcements + the Announcement schema in 3.24.0 (404s live). Per prior decision, the method + model are retained pending confirmation the removal is permanent — upstream has transiently dropped endpoints as publishing glitches before (fix: spec drift — settlement_sources on Event + lifecycle strike fields (#451) #452). Contract bindings dropped; _SOFT_DEPRECATED_MODELS / soft_deprecated allowlists keep the completeness suites green without re-mapping a gone endpoint.

Added

  • communications.quotes.get_for_rfq(rfq_id, quote_id) (sync + async) — GET /communications/rfqs/{rfq_id}/quotes/{quote_id}, the RFQ-scoped get-a-quote (returns GetQuoteResponse).
  • klear.margin.active_obligations() (sync + async) — GET /margin/active_obligationsGetActiveMarginObligationsResponse (plural sibling of active_obligation()).
  • klear.margin.settlement_estimate_by_asset_class() (sync + async) — GET /margin/settlement_estimate_by_asset_classGetSettlementEstimateByAssetClassResponse + AssetClassSettlementEstimate (+ shared AssetClassLiteral).
  • SubaccountNettingConfig.exchange_index (int, required)
  • portfolio.balance() optional exchange_index query param (sync + async) — this drift was invisible to Spec drift 2026-07-06: openapi 3.23.0 → 3.23.0 #470's schema/path diff; only the param-drift test caught it.
  • ObligationEntry.asset_class (perps SCM / Klear; AssetClassLiteral = Literal["Crypto"], required)

Test plan

  • uv run pytest tests/4162 passed, 311 skipped
  • uv run pytest tests/test_contracts.py665 passed (all 13 drift failures cleared; new endpoints mapped)
  • uv run ruff check . — clean
  • uv run mypy kalshi/ — clean (157 files)
  • Vendored specs re-pinned to 3.24.0 via scripts/sync_spec.py; reference _generated/models.py regenerated + audited

Notes for reviewers

  • The price_cents → price change is a format change (cents→dollars), not just a rename — hence the major bump and migration note.
  • Announcement handling is soft-deprecate, not delete, by design (glitch precedent). Integration test_announcements is skip-marked; the method stays registered in the coverage harness because it still exists.
  • The 3 new endpoints were implemented in the second commit (originally deferred). The *_for_rfq quote family — now including get_for_rfq — remains outside the integration coverage harness, matching its existing siblings (accept_for_rfq/delete_for_rfq); the klear MarginResource is likewise outside the harness by design. No behavior change there.

Issue links

Closes #467
Closes #470

TexasCoding and others added 3 commits July 10, 2026 08:04
…, content titles

The recurring #467 (nightly) / #470 (weekly) issues are two detectors on one
real, deterministic upstream drift — but the reporting mechanics amplified the
pain. Fix the mechanics without weakening the read-only security model.

Nightly (spec-drift.yml):
- Comment only when the failing-test-ID set changes (a fingerprint), not every
  failing night — a deterministic upstream produced identical spam indefinitely.
- `spec-drift-ack` / `snoozed` label silences comments while a reconcile is in
  flight, without closing the tracker.
- New report-success job auto-closes the tracker on a green scheduled run
  (the workflow previously only ever opened/commented).

Weekly (spec-sync.yml):
- Title leads with a content signal ([content-changed] + path delta + short
  fingerprint) instead of a bare version — Kalshi changes content in place
  without bumping info.version, so "3.23.0 → 3.23.0" read as a false alarm.
- Supersede stale weekly snapshots so exactly one weekly drift issue stays open
  instead of one per upstream micro-revision. Scoped to spec-sync-bot so the
  nightly tracker is never touched.

Refs #467, #470

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Direction 1 — less brittle (stop benign churn redding CI against a fast-moving,
loosely-versioned upstream):
- Additive drift split: a spec-REQUIRED field the SDK lacks still hard-fails
  (act now); a spec-OPTIONAL field becomes a non-blocking
  AdditiveOptionalDriftWarning. It deliberately subclasses Warning (not
  UserWarning) so the nightly `-W error::UserWarning` gate cannot promote benign
  optional drift to a red build. +3 regression tests lock the contract.
- test_schema_coverage batches all unresolved schemas into one failure instead
  of failing on the first (one-at-a-time reruns on multi-removal drift).

Direction 2 — close false-negatives (catch real drift the suite missed):
- test_every_spec_endpoint_is_mapped asserts every spec OPERATION
  (core+perps+perps_scm) is in a METHOD_ENDPOINT_MAP, minus a reasoned
  _UNIMPLEMENTED_ENDPOINTS allowlist. Catches ADDED endpoints/operations the
  schema/path-count diff misses — verified against live 3.24.0, which had it
  caught the two new /margin/* endpoints AND a new GET on an existing
  /communications/rfqs/.../quotes path that the weekly path diff reported as
  "Added endpoints: (none)".
- kwarg_rename exclusions are now spec-anchored (test_exclusion_map_is_current):
  the spec-side param must still exist for the endpoint, so a rename can't go
  stale and silently mask an upstream param removal/rename.

Docstring updated to the actual post-#171/#172 behavior and to document why
response-side field REMOVAL is intentionally NOT gated: the vendored "Kalshi
Trade API Manual Endpoints" spec is a curated subset, so SDK response models are
a superset by design; gating removals would red CI on every intentional SDK
field. Request-BODY drift already gates removals (_check_model_drift), which is
where a rename's dropped old name is caught.

Refs #467, #470

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

Reconciles the vendored specs to upstream 3.24.0 and the hand-written SDK to
match. Kalshi changed some content in place without bumping info.version, so
this also absorbs edits made under the same string.

BREAKING:
- Position-transfer price is now fixed-point dollars (Decimal), not integer
  cents. `price_cents` → `price` (OrderPrice / DollarDecimal) on
  ApplySubaccountPositionTransferRequest, SubaccountTransfer, and
  transfer_position() (sync + async). Pass price=Decimal("0.50") for a 50¢ basis.
  The old le=100c cap is gone — OrderPrice guards non-negativity + the $0.0001
  tick and leaves the upper bound to the server (matches CreateOrderRequest).

Deprecated:
- exchange.announcements() (sync + async) now emits DeprecationWarning. Upstream
  removed GET /exchange/announcements + the Announcement schema in 3.24.0 (404s
  live). The method + model are RETAINED pending confirmation the removal is
  permanent (transient-drop precedent: #452). Contract bindings dropped;
  _SOFT_DEPRECATED_MODELS / soft_deprecated allowlists keep the completeness
  suites green without re-mapping a gone endpoint.

Added:
- SubaccountNettingConfig.exchange_index (int, required)
- portfolio.balance() optional exchange_index query param (sync + async)
- ObligationEntry.asset_class (perps SCM / Klear; Literal["Crypto"], required)

Deferred to follow-up feature work (allowlisted in _UNIMPLEMENTED_ENDPOINTS):
- GET /communications/rfqs/{rfq_id}/quotes/{quote_id}
- GET /margin/active_obligations
- GET /margin/settlement_estimate_by_asset_class

Verified: full suite 4142 passed / 311 skipped, ruff clean, mypy kalshi/ clean.

Closes #467
Closes #470

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

claude Bot commented Jul 10, 2026

Copy link
Copy Markdown

Review

I reviewed the full diff (note: this PR is stacked on #471, so the diff currently includes that branch's CI-hardening changes too — my comments below focus on the actual "spec sync 3.24.0" reconcile commit 6f0b7ff).

Summary

This is a well-executed, carefully-documented spec sync. The breaking price_centsprice (fixed-point dollars) rename is handled consistently across the request model, response model, sync/async resource methods, and overload signatures, and is backed by solid test coverage (happy path, serialization, validation boundaries, malformed input). The soft-deprecation approach for exchange.announcements() (retain + DeprecationWarning + allowlist instead of a hard delete) is a reasonable, low-risk way to handle an upstream endpoint that may have been transiently dropped, and it's well justified given the #452 precedent cited in the PR description. The new test_every_spec_endpoint_is_mapped contract test closing the "added endpoint" blind spot is a genuine improvement in drift detection.

Issues found

1. Stale docs — docs/resources/subaccounts.md still documents the removed price_cents API (not touched by this PR)

  • Line 12: quick-reference table still shows transfer_position(..., price_cents).
  • Line 57: prose still says price_cents (0–100) sets the cost basis....
  • Line 68: the code example still passes price_cents=55 — this will now raise TypeError (unexpected keyword argument) if a user copies it, since price_cents no longer exists on transfer_position().

This is the one real gap given the PR explicitly calls this a breaking change — worth fixing before merge (or as an immediate fast-follow) so the docs don't actively mislead users into a runtime error.

2. Docstring inconsistency — async transfer_position() still says "spec v3.23.0"
kalshi/resources/subaccounts.py:441 (async overload docstring) reads """Move an open position between subaccounts (spec v3.23.0).""", while the sync counterpart at line 248 was correctly updated to "spec v3.24.0". Minor/cosmetic, but worth a one-line fix since both were touched in the same commit.

Things that look correct (verified, not just trusted from the description)

  • OrderPrice (used for the new price field) only guards non-negativity + the $0.0001 tick, matching the stated intent of leaving the upper bound to the server (mirrors CreateOrderRequest).
  • The spec's ApplySubaccountPositionTransferRequest.price field is $ref: FixedPointDollars named exactly price (not price_dollars), so no serialization_alias is needed — consistent with the SDK's stated convention.
  • SubaccountTransfer.price is correctly optional (DollarDecimal | None) since the spec doesn't mark it required (position-transfer-only field on a discriminated response row).
  • _SOFT_DEPRECATED_MODELS / _UNIMPLEMENTED_ENDPOINTS allowlists are narrowly scoped and self-documenting (each entry carries a reason), consistent with CLAUDE.md's guidance against silent trackers.
  • asset_class: Literal["Crypto"] addition to ObligationEntry follows the existing Literal-alias pattern already used in that file.

Nit

  • CHANGELOG.md:45-48 has an inline code span (`GET ... ) that wraps across a line break mid-token. It'll still render fine under CommonMark (the newline collapses to a space), so this is cosmetic only.

No security, performance, or test-coverage concerns beyond the doc drift above — the new price field construction path is well-covered by both unit and integration tests, and the contract-drift suite changes (required-vs-optional additive drift split, test_every_spec_endpoint_is_mapped) are themselves tested (TestAdditiveDriftClassification).

Follow-up within the 3.23.0 → 3.24.0 reconcile: implement the endpoints that
were recognized-but-deferred in _UNIMPLEMENTED_ENDPOINTS.

- communications.quotes.get_for_rfq(rfq_id, quote_id) (sync + async) —
  GET /communications/rfqs/{rfq_id}/quotes/{quote_id} -> GetQuoteResponse
  (RFQ-scoped variant of the flat quotes.get).
- klear.margin.active_obligations() (sync + async) —
  GET /margin/active_obligations -> GetActiveMarginObligationsResponse
  (plural sibling of active_obligation()).
- klear.margin.settlement_estimate_by_asset_class() (sync + async) —
  GET /margin/settlement_estimate_by_asset_class ->
  GetSettlementEstimateByAssetClassResponse + AssetClassSettlementEstimate.
  ObligationEntry.asset_class refactored onto the shared AssetClassLiteral.

Wired into METHOD_ENDPOINT_MAP / PERPS_SCM_* maps + CONTRACT_MAP; removed from
_UNIMPLEMENTED_ENDPOINTS; +20 unit tests. Full suite 4162 passed / 311 skipped,
ruff + mypy kalshi/ clean.

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

claude Bot commented Jul 10, 2026

Copy link
Copy Markdown

Review

Reviewed the hand-written diff (skipped the vendored specs/*.yaml files as generated content). This is a solid, well-tested spec-drift reconciliation — the price-unit rename, soft-deprecation, and new endpoints all have matching contract-map entries, drift-test coverage, and unit/integration tests. A couple of small things worth fixing before merge:

Bug: docstring misattached after inserting AssetClassLiteral

kalshi/perps/klear/models/margin.py (~line 74-80):

WithdrawalStatusLiteral = Literal["pending", "processing", "processed", "failed"]

# Spec ``AssetClass`` enum (single value today). Named alias mirrors the other
# *Literal aliases; shared by ObligationEntry.asset_class and the settlement
# estimates keyed by asset class (spec sync 3.24.0).
AssetClassLiteral = Literal["Crypto"]
"""Spec withdrawal ``status`` — lifecycle of an async settlement-balance withdrawal."""

The attribute docstring ("""Spec withdrawal status...""") was originally the very next line after WithdrawalStatusLiteral, forming a PEP-257-style attribute docstring for it. Inserting AssetClassLiteral in between means that string literal is now attached to AssetClassLiteral instead — any doc tooling (Sphinx autodoc, IDE hovers) will show "lifecycle of an async settlement-balance withdrawal" as the docstring for the asset class literal, and WithdrawalStatusLiteral loses its docstring entirely. Move the withdrawal docstring back immediately under WithdrawalStatusLiteral, and (optionally) give AssetClassLiteral its own trailing docstring line to match the style of the others in this file.

Nit: stale version string in async docstring

kalshi/resources/subaccounts.py:441 — the async transfer_position() docstring still reads (spec v3.23.0), while the sync version right above it (line 248) was correctly updated to (spec v3.24.0) as part of this same rename. Cosmetic only, but worth a one-line fix for consistency.

What looks good

  • Price rename correctness: verified against specs/openapi.yamlApplySubaccountPositionTransferRequest.price is genuinely a top-level price field ($ref: FixedPointDollars), not price_dollars, so no serialization_alias is needed; the request model, response model (SubaccountTransfer.price: DollarDecimal), and the OrderPrice reuse for construction-time validation are all consistent with the rest of the codebase's conventions.
  • Soft-deprecation of exchange.announcements(): DeprecationWarning is emitted on both sync/async paths, contract-map/exclusion entries are removed with clear comments explaining why, _SOFT_DEPRECATED_MODELS/soft_deprecated allowlists keep completeness tests green without falsely re-mapping a removed endpoint, and the integration tests are appropriately skipped (with a reason) rather than deleted.
  • New endpoints (communications.quotes.get_for_rfq, klear.margin.active_obligations, klear.margin.settlement_estimate_by_asset_class) all reuse the existing _seg() path-segment sanitizer (no injection risk), have matching METHOD_ENDPOINT_MAP entries, and their response models were checked against specs/perps_scm_openapi.yaml — required/optional fields line up correctly (e.g. AssetClassSettlementEstimate.next_runtime required, breakdown maps optional).
  • New test_every_spec_endpoint_is_mapped test is a nice completeness improvement — it closes the exact gap (added-upstream-endpoint-ships-unmapped) that let two of this PR's new perps endpoints go undetected by the schema-only diff, and the one legitimately-unimplemented endpoint (GET /margin/large_trader_positions) is properly recorded with a reason.
  • Additive-drift severity split (AdditiveOptionalDriftWarning vs. hard failure on new required fields) is well-reasoned and has dedicated tests locking the "must not become a UserWarning" contract that makes it survive the nightly strict gate.
  • Test coverage is thorough: happy path, null-coercion, auth-required, and async variants for every new surface.

Nothing here blocks merge in my view — the two items above are small and easy to fix. Nice work tightening the drift-detection gaps (test_every_spec_endpoint_is_mapped) while doing the reconcile itself.

Note: I wasn't able to execute the test suite/mypy in this environment (sandboxed, no command-execution approval available), so this review is based on static reading of the diff and cross-checking against the vendored spec files — please confirm CI is green.

@TexasCoding TexasCoding merged commit 50d9a58 into main Jul 10, 2026
7 checks passed
@TexasCoding TexasCoding deleted the reconcile/spec-3.24.0 branch July 10, 2026 14:11
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.

Spec drift 2026-07-06: openapi 3.23.0 → 3.23.0 Nightly spec-drift: strict contract tests failing (since 2026-07-05)

1 participant