feat: sync spec to v3.18.0 + fix asyncapi drift false alarm

[TexasCoding]

Summary

• Bumps OpenAPI spec 3.13.0 → 3.18.0 (77 → 85 paths) and implements the full SDK surface for the new endpoints — 9 new methods on sync + async resources.
• Un-gitignores specs/asyncapi.yaml so the weekly spec-sync workflow stops opening false-alarm drift issues every Monday. The file was never committed, so each cron run diffed the freshly downloaded asyncapi against a non-existent old file and rendered a bogus "0 → 13 channels" delta.
• Adds exchange_index / user_filter / rfq_user_filter / incentive_description query params and post_only on CreateQuoteRequest to keep contract drift tests green.

What's new

V2 event-market orders (legacy /portfolio/orders deprecated no earlier than May 6, 2026):

• orders.create_v2(request=CreateOrderV2Request(...))
• orders.cancel_v2(order_id, subaccount=..., exchange_index=...)
• orders.amend_v2(order_id, request=AmendOrderV2Request(...))
• orders.decrease_v2(order_id, request=DecreaseOrderV2Request(...))
• orders.batch_create_v2(request=BatchCreateOrdersV2Request(...))
• orders.batch_cancel_v2(request=BatchCancelOrdersV2Request(...))

Portfolio history:

• portfolio.deposits() + portfolio.deposits_all()
• portfolio.withdrawals() + portfolio.withdrawals_all()

Account:

• account.endpoint_costs() — lists endpoints whose token cost differs from the default.

Field additions (all optional, exclude_none semantics preserved):

• CreateOrderRequest, AmendOrderRequest, DecreaseOrderRequest, BatchCancelOrdersRequestOrder, CreateOrderGroupRequest — exchange_index: int | None.
• CreateQuoteRequest — post_only: bool | None.
• OrdersResource.cancel, OrderGroupsResource.delete — exchange_index query param.
• CommunicationsResource.list_rfqs/list_all_rfqs — user_filter.
• CommunicationsResource.list_quotes/list_all_quotes — user_filter, rfq_user_filter. _require_quote_filter now accepts these as satisfying the server-side filter requirement.
• IncentiveProgramsResource.list/list_all — incentive_description.

Infra fix:

• .gitignore line 34 (specs/asyncapi.yaml) removed. Both pinned spec files are now committed, matching the comment intent already on the openapi line. Resolves the recurring drift false-alarm noted in the comment of Spec drift 2026-05-18: openapi 3.13.0 → 3.18.0 #154 itself.

Test plan

• uv run mypy kalshi/ — clean (76 files).
• uv run ruff check . — clean.
• uv run pytest tests/ --ignore=tests/integration — 1866 passed, 0 failed.
• uv run pytest tests/test_contracts.py — 372 passed (all 23 drift failures resolved without EXCLUSIONS placeholders).
• Integration coverage harness passes — all new methods registered.
• Spec checksums match the issue body exactly (7e31fc45…, 6ff8a7f3…).
• Live integration smoke (requires credentials) — /account/endpoint_costs, /portfolio/deposits, /portfolio/withdrawals against demo.

Closes #154

🤖 Generated with Claude Code

[claude]

Code Review — feat: sync spec to v3.18.0 + fix asyncapi drift false alarm

Overall this is a solid, well-structured spec bump. The architecture is consistent, contract tests are thorough, and the infra fix (committing asyncapi.yaml) directly solves a recurring false-alarm. Three issues to address before merge, ordered by severity.

---

Bug — endpoint_costs() missing _require_auth()

Both the sync and async implementations skip the auth guard:

# kalshi/resources/account.py
def endpoint_costs(self) -> AccountEndpointCosts:
    """List API v2 endpoints with non-default token costs."""
    data = self._get("/account/endpoint_costs")          # ← no _require_auth()
    return AccountEndpointCosts.model_validate(data)

The adjacent limits() method calls _require_auth() on both sync and async variants; endpoint_costs serves per-account data and must do the same. Without it, callers get a confusing 401 from the API instead of the SDK's early AuthenticationError. Fix is one line in each variant.

---

Type-safety gap — side: str in V2 request models

BookSideLiteral = Literal["bid", "ask"] is defined and exported in this same PR, but CreateOrderV2Request.side and AmendOrderV2Request.side use plain str:

class CreateOrderV2Request(BaseModel):
    side: str          # should be BookSideLiteral
    ...

class AmendOrderV2Request(BaseModel):
    side: str          # should be BookSideLiteral
    ...

V1 uses SideLiteral for its side field; V2 should use BookSideLiteral for the same reason — static analysis catches typos and IDEs provide completion. No runtime cost; just a one-character change per field.

---

Missing unit tests for all new public methods

tests/test_orders.py, tests/test_portfolio.py, and tests/test_account.py are unchanged. Eleven new public methods ship with only integration stubs:

• orders: create_v2, cancel_v2, amend_v2, decrease_v2, batch_create_v2, batch_cancel_v2
• portfolio: deposits, deposits_all, withdrawals, withdrawals_all
• account: endpoint_costs

CLAUDE.md convention: every public method needs at minimum a happy path, an error path, and an edge case. The integration harness registers coverage but that's a live-credentials gate; the respx.mock-based unit tests are what run in CI. Notable paths that need covering:

• cancel_v2 / batch_cancel_v2: the data is None → KalshiError branch (204 No Content)
• decrease_v2: the reduce_by/reduce_to XOR validator (both set, neither set)
• deposits_all / withdrawals_all: pagination with max_pages

---

Minor nit — Withdrawal reuses DepositStatusLiteral

class Withdrawal(BaseModel):
    status: DepositStatusLiteral    # named after Deposit

This works if the spec shares the enum, and the statuses do match. Worth confirming the spec treats them as the same type; if so, a shared FundingStatusLiteral alias would be cleaner, but this isn't blocking.

---

What's done well

• Contract tests are complete: all 6 new V2 endpoints registered in METHOD_ENDPOINT_MAP with correct HTTP methods, path templates, and request_body_schema on every POST/DELETE-with-body. The batch_cancel_v2 → request exclusion mirrors batch_cancel correctly.
• Request models all use extra="forbid", response models use extra="allow" — consistent with the rest of the SDK.
• DecreaseOrderV2Request includes the XOR validator for reduce_by/reduce_to — matches the spec's intent despite the spec marking both optional.
• _post signature extension (params: dict | None = None) is clean and doesn't change callers.
• deposits_all/withdrawals_all correctly use def (not async def) since _list_all returns AsyncIterator directly — no false positive here.
• All new types properly exported in both kalshi/__init__.py and kalshi/models/__init__.py.
• The asyncapi.yaml fix is well-explained; the comment update in .gitignore matches intent.

Summary: One auth bug and one type gap are straightforward fixes. The unit test gap is the heaviest lift but is required by the project's own conventions. Happy to approve once those three are addressed.

[TexasCoding]

Thanks for the review — all three actionable issues addressed in c9abff3.

Bug fix — endpoint_costs() auth guard
Added self._require_auth() to both AccountResource.endpoint_costs and AsyncAccountResource.endpoint_costs to match limits().

Type-safety — V2 side fields
CreateOrderV2Request.side and AmendOrderV2Request.side now use BookSideLiteral. One clarification: V1's CreateOrderRequest.side is also str, not SideLiteral — the V1 convention pushes the Literal to the resource-method kwarg boundary (see comment at kalshi/models/orders.py:13-17). V2 has no kwarg overload (model-only surface), so I agree the Literal belongs on the model field, and added a docstring noting why V2 diverges from V1 here. Pydantic now rejects side="yes" at construction time, verified with a test.

Unit tests — 37 new tests across 4 files

• test_account.py — endpoint_costs happy path, empty list, auth required (sync + async).
• test_portfolio.py — deposits/withdrawals happy paths, pagination, max_pages cap, auth failure (sync + async).
• test_orders.py / test_async_orders.py — all 6 V2 methods covered. Specifically:
  • cancel_v2 / batch_cancel_v2: the data is None → KalshiError branch for 204 No Content.
  • DecreaseOrderV2Request: both XOR error paths (both set, neither set).
  • cancel_v2: verifies subaccount + exchange_index query params reach the wire.
  • CreateOrderV2Request / AmendOrderV2Request: confirms side="yes" raises (the new BookSideLiteral constraint).
  • batch_create_v2: per-order error field surfaces correctly via BatchCreateOrdersV2ResponseEntry.error.

Full suite: 1903 passed (was 1866), mypy clean, ruff clean.

On the DepositStatusLiteral nit — the spec defines two separate but identical enums (Deposit.status and Withdrawal.status), so reusing the type is technically a spec-level conflation. Leaving it as a follow-up since renaming would be a public-type rename and not blocking — happy to address in a separate small PR if you'd like a FundingStatusLiteral alias.

[claude]

Code Review — PR #155: sync spec to v3.18.0 + fix asyncapi drift

Overview

This is a solid, well-structured spec-sync PR. The V2 order model family follows the existing request/response pattern cleanly, test coverage is good, and the asyncapi gitignore fix directly solves the recurring false-alarm problem. A few things worth discussing before merging:

---

Potential Bugs

1. Missing _dollars aliases on V2 price fields (medium-high risk)

The project convention (documented in CLAUDE.md) is that Kalshi API returns dollar prices as *_dollars suffixed strings, and models accept both forms via validation_alias=AliasChoices(...). The new V2 response models skip this:

# CreateOrderV2Response, AmendOrderV2Response, etc.
average_fill_price: DollarDecimal | None = None  # no validation_alias
average_fee_paid: DollarDecimal | None = None     # no validation_alias

If the API returns average_fill_price_dollars (which is the Kalshi convention), these fields will silently deserialize as None rather than raising a validation error. Worth verifying against the actual spec before this ships. Same concern applies to price in CreateOrderV2Request — if the wire format expects price_dollars, the serialized body will send the wrong key.

2. amend_v2 / decrease_v2 — subaccount vs. exchange_index placement inconsistency

def amend_v2(self, order_id, *, request: AmendOrderV2Request, subaccount: int | None = None):
    params = _params(subaccount=subaccount)      # query param
    body = request.model_dump(...)               # exchange_index lives here

subaccount is a resource-method kwarg that goes in the query string, but exchange_index lives on AmendOrderV2Request and goes in the body. This split is either intentional (matching the spec exactly) or a bug. The V1 amend method puts both in the body via AmendOrderRequest. If the spec says these should both be query params (or both body fields), one of them is in the wrong place.

---

Naming / Convention Issues

3. DepositStatusLiteral used for Withdrawal.status

class Withdrawal(BaseModel):
    status: DepositStatusLiteral  # ← reads oddly

Since Deposit and Withdrawal appear to share the same status enum values (pending, applied, failed, returned), a neutral name like PaymentStatusLiteral would make this less surprising for callers reading the type. Low-stakes but easy to fix.

4. UserFilterLiteral defined in resources/communications.py

The other *Literal type aliases (BookSideLiteral, DepositStatusLiteral, etc.) are defined in models/. UserFilterLiteral living in a resource file breaks that pattern and means it won't be available to callers who only import from kalshi.models. If this type is part of the public API, it should move to kalshi/models/communications.py and be exported from kalshi/models/__init__.py and kalshi/__init__.py.

---

Test Coverage Gaps

5. No _dollars-keyed response parsing tests for V2 endpoints

Given the concern in #1, it would be good to add at least one test per new response model that mocks the API returning average_fill_price_dollars (the _dollars suffix form) and asserts the field is parsed correctly — or explicitly confirms the spec uses the short form. This is the gap most likely to surface as a runtime regression.

6. amend_v2 / decrease_v2 — no test for subaccount query param forwarding

TestCancelOrderV2 has a test_passes_query_params test that checks subaccount and exchange_index are correctly forwarded. TestAmendOrderV2 and TestDecreaseOrderV2 have no equivalent, so a silent regression (e.g. params dropped from the _post call) wouldn't be caught.

7. V2 methods missing requires_auth tests

create_v2, amend_v2, decrease_v2, batch_create_v2, and batch_cancel_v2 have no test that verifies an unauthenticated client raises an auth error. Low risk since the pattern is inherited from the base class, but every other resource method (including cancel_v2) has this test.

---

Minor

• time_in_force and self_trade_prevention_type on CreateOrderV2Request are plain str. The spec likely defines enums for these. Not critical (extra values would still be rejected by the server), but explicit Literal types would catch typos at construction time.
• Deposit and Withdrawal are structurally identical models. If the spec defines them as the same schema, a shared PaymentEntry base class would reduce duplication — but this is style, not correctness.

---

What's Good

• The DecreaseOrderV2Request._enforce_reduce_xor validator is a nice touch — mirroring the existing V1 model-level guard rather than pushing the check to the resource method.
• cancel_v2 and batch_cancel_v2 both correctly guard against unexpected 204 responses and raise a descriptive KalshiError.
• Adding params to _post() cleanly enables the V2 amend/decrease subaccount query param without duplicating transport logic.
• The asyncapi.yaml gitignore fix is the right call and the updated comment is accurate.
• Test coverage for the deposits/withdrawals pagination (including max_pages cap) is thorough.

---

Bottom line: The main thing I'd want confirmed before merging is whether the V2 dollar-price fields use the _dollars suffix on the wire. If they do, the response models need validation_alias and the request model needs serialization_alias. Everything else above is either low-risk or stylistic.

[TexasCoding]

Thanks for the second pass. Pushed f7fecf8. Triaged each point against the spec before fixing:

Verified against spec (no change needed)

#1 _dollars aliases on V2 price fields — V2 spec uses plain field names (price, average_fill_price, average_fee_paid) with no _dollars suffix. V1 has the suffix because its fields carry both int-cent and decimal-dollar forms; V2 ships FixedPointDollars-only and dropped the duplication. Sample V2 spec:
```yaml
CreateOrderV2Response:
properties:
average_fill_price: # ← no _dollars suffix
$ref: '#/components/schemas/FixedPointDollars'
```
The existing models match the wire shape exactly. No alias work needed.

#2 subaccount vs. exchange_index placement on amend_v2 / decrease_v2 — Faithful to spec, not a bug. The amend/decrease V2 spec declares only SubaccountQueryDefaultPrimary in its query parameters list; exchange_index is a body field on AmendOrderV2Request/DecreaseOrderV2Request. The cancel_v2 endpoint by contrast declares both SubaccountQueryDefaultPrimary and ExchangeIndexQuery as query parameters (no body), which is why cancel takes both as kwargs. Added docstrings on amend_v2/decrease_v2 (sync + async) noting "do not move exchange_index into params" so future contributors don't try to "fix" it.

Fixed

#3 DepositStatusLiteral → PaymentStatusLiteral — Renamed. The spec defines two structurally-identical inline enums; the neutral name reads less surprisingly on Withdrawal. Updated exports in kalshi.models and top-level.

#4 UserFilterLiteral relocation — Moved from kalshi/resources/communications.py to kalshi/models/communications.py to match the other *Literal aliases. Now exported from kalshi.models and kalshi so callers can import without reaching into resources/.

Minor — V2 enum types — CreateOrderV2Request.time_in_force is now TimeInForceLiteral, self_trade_prevention_type is SelfTradePreventionTypeLiteral. Catches typos at construction.

New tests

#6 query-param forwarding —

• TestAmendOrderV2.test_passes_subaccount_query and the decrease_v2 equivalent (sync): assert subaccount reaches the query string while exchange_index stays in the body. Regression guard against the params kwarg silently dropping from _post.
• TestAsyncAmendDecreaseV2QueryParams mirrors the same on the async surface.

#7 auth guards — TestV2RequiresAuth + TestAsyncV2RequiresAuth cover create_v2/amend_v2/decrease_v2/batch_create_v2/batch_cancel_v2. (cancel_v2 already had one.)

Validation

1920 tests pass (was 1903), mypy clean, ruff clean.

Deferred

Minor — PaymentEntry base class for Deposit/Withdrawal — Reasonable suggestion but treating them as separate models matches the spec's two-schema declaration, and a base class would either (a) lock the SDK to keeping the two enums identical, which the spec doesn't guarantee, or (b) need generic-typed fields that complicate the public surface. Happy to revisit if upstream consolidates them into a single shared schema.

[claude]

Code Review — PR #155: Sync spec to v3.18.0 + AsyncAPI drift fix

Overall this is a solid, well-structured spec-sync. The scope is appropriate, contract drift tests are all green, and the new V2 surface has good test coverage. A few items worth addressing before merge:

---

Potential Bug — BatchCancelOrdersV2ResponseEntry.reduced_by is non-optional

File: kalshi/models/orders.py

class BatchCancelOrdersV2ResponseEntry(BaseModel):
    order_id: str                        # required
    reduced_by: FixedPointCount          # required ← problem
    client_order_id: str | None = None
    ts_ms: int | None = None
    error: dict[str, object] | None = None

Batch endpoints return mixed success/error entries. When an entry has an error, the server likely omits reduced_by and may omit order_id. Pydantic will throw a ValidationError at that point, not a user-friendly error.

Compare with the correctly modeled BatchCreateOrdersV2ResponseEntry where every field is optional. Suggest making reduced_by and order_id optional here too:

order_id: str | None = None
reduced_by: FixedPointCount | None = None

There's no test covering the error-entry path for batch_cancel_v2, so this bug would only surface against the live API.

---

Minor: subaccount bounds not carried over to V2 request models

V1 models enforce subaccount: int | None = Field(default=None, ge=0, le=32). All V2 request models (CreateOrderV2Request, AmendOrderV2Request, DecreaseOrderV2Request, etc.) use subaccount: int | None = None without bounds. Intentional deviation from the spec, or an oversight?

---

Observation: _delete_with_body has no test that verifies the body is actually sent for batch_cancel_v2

TestBatchCancelV2 covers the happy path and the 204 guard, but there's no assertion that the serialized orders list appears in the DELETE request body. The amend/decrease V2 tests do check body content explicitly (e.g., assert body.get("exchange_index") == 0). Worth adding a test_sends_body case mirroring TestBatchCreateV2's body-structure assertion at line ~12680.

---

Observation: Deposit and Withdrawal models are structural duplicates

Both have exactly the same fields. The comment on PaymentStatusLiteral already notes the shared enum. The models are spec-faithful, so this is fine as-is, but a _PaymentRecord base class would eliminate the duplication if this pattern recurs.

---

Nits

• AmendOrderV2Request.ticker and .side are required fields, matching the spec. But CreateOrderV2Request also requires ticker/side, which is correct. Good.
• The params= addition to _post in _base.py is minimal and correct — needed for the asymmetric amend_v2/decrease_v2 spec (subaccount in query, exchange_index in body). The regression test at test_passes_subaccount_query is a solid guard.
• The _require_quote_filter signature change is backwards-compatible (new params are keyword-only additions to the private function). Good.
• Version not bumped from 2.0.0. Intentional? 9 new public methods across sync + async is probably a minor bump to 2.1.0.

---

What's working well

• All 6 V2 endpoints are registered in METHOD_ENDPOINT_MAP with correct request_body_schema refs.
• The DecreaseOrderV2Request._enforce_reduce_xor validator is tested for all three cases (both, neither, one).
• BookSideLiteral validation is tested at the model level (V1 side is a plain str; V2 correctly uses a Literal).
• The cancel_v2 204-guard with KalshiError is a good defensive pattern and is tested.
• The spec-asymmetry comments on amend_v2/decrease_v2 (why subaccount is query and exchange_index is body) are valuable — keep them.
• .gitignore fix is correct and resolves the false-alarm drift issue cleanly.

Primary ask before merge: Confirm/fix BatchCancelOrdersV2ResponseEntry.reduced_by nullability, and add a body-content test for batch_cancel_v2.

[TexasCoding]

Thanks for the third pass. Pushed 4ef2cab (or wherever this lands). Triaged each item:

Primary ask — BatchCancelOrdersV2ResponseEntry.reduced_by nullability

Investigated against spec; kept spec-faithful and added documentation tests instead.

The spec at lines 7510-7512 explicitly marks order_id + reduced_by as required, and the reduced_by description reads: "Number of contracts that were canceled (i.e. the remaining count at time of cancellation). Zero if the cancel errored."

Compare with BatchCreateOrdersV2Response.orders.items (line 7423), which has no required: key — every field is optional there. That's why my BatchCreateOrdersV2ResponseEntry is all-optional. The asymmetry is in the spec, not the SDK.

So the right move was:

1. Keep the model matching the spec.
2. Add a test_error_entry_parses case that demonstrates an errored entry per the spec's contract: {order_id: "ord-bad", reduced_by: "0", error: {...}}. If upstream starts omitting order_id/reduced_by on error entries despite the spec, this test will keep passing and a separate live integration will fail — at which point the spec is wrong and we file with Kalshi. If we made the fields optional defensively now, we'd be drifting from spec without a tracked exclusion.

Body-content test for batch_cancel_v2

Added TestBatchCancelV2.test_sends_body — asserts the serialized orders array reaches the DELETE request body with the expected shape (including subaccount-only entry coexisting with one that omits it). Mirrors the assertion style of the V1 batch_cancel wire-shape tests.

subaccount bounds on V2 models

Added Field(ge=0, le=32) on CreateOrderV2Request.subaccount and BatchCancelOrdersV2RequestOrder.subaccount. The spec only enforces minimum: 0, but the descriptions document "1-32 for subaccounts" and V1 has carried le=32 since release — consistency wins. AmendOrderV2Request / DecreaseOrderV2Request deliberately have no subaccount field since that param routes via the resource method's query kwarg per spec; added an inline comment on DecreaseOrderV2Request to make that explicit.

Version bump 2.0.0 → 2.1.0

Bumped in kalshi/__init__.py and pyproject.toml. Added a 2.1.0 entry to CHANGELOG.md enumerating the V2 surface, new portfolio/account endpoints, the optional field additions, and the asyncapi.yaml gitignore fix.

Deferred (intentional)

Deposit/Withdrawal consolidation into a _PaymentRecord base class — the spec keeps them as separate schemas. The current convention (model-per-spec-schema) keeps the SDK structure mappable back to the spec one-for-one, which matters for the contract-drift tests. Happy to revisit if upstream consolidates them.

Validation

mypy clean, ruff clean. The contracts + V2 test suites pass.

[claude]

Code Review — PR #155: Spec sync v3.13.0 → v3.18.0 + AsyncAPI drift fix

Overall this is a well-executed spec sync. The V2 order family, portfolio history endpoints, and account endpoint-cost introspection are implemented consistently with the rest of the SDK. The .gitignore fix for the weekly false-alarm drift issue is clean and correct. A few items worth addressing before merge:

---

Bugs / Correctness

1. BatchCancelOrdersV2ResponseEntry.reduced_by is required, not optional

class BatchCancelOrdersV2ResponseEntry(BaseModel):
    order_id: str
    reduced_by: FixedPointCount    # required
    ...
    error: dict[str, object] | None = None

The test comment says "Per spec, an errored cancel still carries order_id + reduced_by=0 alongside the error block" — but this is a spec-level assertion that's worth making visible. If the live API ever omits reduced_by on an error entry (even in a non-conforming edge case), model_validate will throw a ValidationError instead of surfacing the error field. Consider making it optional to match the same pattern as BatchCreateOrdersV2ResponseEntry:

reduced_by: FixedPointCount | None = None

If the spec truly guarantees it even for errors, a model_validator that raises on reduced_by=None when error=None would make that invariant explicit rather than implicit.

---

2. Deposit and Withdrawal models are structurally identical

Both have the same 7 fields with the same types and the same model_config. The comment on PaymentStatusLiteral acknowledges the spec uses "two structurally-identical inline enums." Keeping them as distinct classes is fine for future divergence, but worth calling out: if they do remain identical through the next few spec versions, a shared _PaymentRecord base (or even a type alias) would make the intent explicit. Not a blocker—consistent with the project's anti-abstraction guideline—but log it in the issue tracker if you'd like to revisit.

---

Test Coverage Gaps

3. TestAsyncAmendOrderV2 is missing the body/query placement regression test

TestAmendOrderV2.test_passes_subaccount_query is the most important spec-compliance guard for amend_v2—it verifies that subaccount goes to query params while exchange_index stays in the JSON body. The async counterpart in TestAsyncAmendOrderV2 only has test_returns_response. If someone accidentally drops the params=params from the async _post call, this invariant would go undetected.

The same gap exists for TestAsyncDecreaseOrderV2.

4. TestCreateOrderV2 doesn't verify request body serialization

The happy-path test checks the response only. Given that price: DollarDecimal and count: FixedPointCount go through custom Pydantic types with mode="json" serialization, a test that inspects route.calls[0].request.content would catch any accidental regression in model_dump(exclude_none=True, by_alias=True, mode="json") for V2 models. TestAmendOrderV2.test_passes_subaccount_query already does this pattern—apply it to create_v2 for the body fields.

---

Minor / Style

5. amend_v2 async docstring says "See sync sibling…" but the sync docstring contains the full rationale

The asymmetry (subaccount=query, exchange_index=body) is non-obvious enough that users are likely to read the async version first. The async docstring on lines 1690–1692 should carry the full explanation rather than deferring to the sync sibling—at minimum include the one-sentence reason: "Per OpenAPI spec v3.18.0, subaccount is a query parameter while exchange_index lives in the request body."

6. _post now passes params={} on every amend_v2/decrease_v2 call where subaccount=None

_params(subaccount=None) returns {}, so self._post(..., params={}, ...) goes to the transport with an explicit empty dict. httpx handles this correctly (no query string added), but it's a subtle difference from params=None. If the transport ever gates on params is not None rather than bool(params), this could silently add a ? to URLs. Not a current bug, but worth a note.

7. CreateOrderV2Request.client_order_id is required (no default)

V1 CreateOrderRequest has client_order_id: str | None. V2 requires it. This is intentional per spec, but users migrating from v1 may be surprised. The BookSideLiteral docstring ("Unlike v1…") could gain a single sentence: "client_order_id is required on v2 (unlike v1 where it is optional)." Optional.

---

Positive Highlights

• The DecreaseOrderV2Request._enforce_reduce_xor model validator is exactly the right place to enforce the server-side XOR constraint—extra="forbid" + model_validator is the established SDK pattern and used correctly.
• The _base.py _post signature extension (params kwarg) is minimal and backward-compatible. All existing callers pass params=None implicitly.
• The spec-body asymmetry in amend_v2 and decrease_v2 (subaccount=query, exchange_index=body) is explained clearly in the sync docstrings and guarded by TestAmendOrderV2.test_passes_subaccount_query.
• Contract drift tests pass at 372/372 with no new EXCLUSIONS placeholders—clean.
• _require_quote_filter correctly extends the existing filter logic to satisfy the server requirement with the new user_filter/rfq_user_filter params.
• BatchCreateOrdersV2ResponseEntry.order_id: str | None correctly models the partial-failure case (no ID for a failed create).

---

Summary: Items 1 and 3 are the most actionable before merge. Item 1 is a mild correctness risk; item 3 is a missing regression guard for a non-obvious spec invariant. Everything else is polish.

🤖 Reviewed with Claude Code

[TexasCoding]

Thanks for the fourth pass. Pushed 7792d8b. Going through each item:

#1 — BatchCancelOrdersV2ResponseEntry.reduced_by nullability

Good compromise suggestion. Applied your "Optional + model_validator" pattern: reduced_by is now FixedPointCount | None = None so a non-conforming response doesn't trip Pydantic's required-field path, but a model_validator raises a descriptive error if it ever lands as None:

```python
@model_validator(mode="after")
def _enforce_reduced_by_present(self) -> BatchCancelOrdersV2ResponseEntry:
if self.reduced_by is None:
raise ValueError(
"BatchCancelOrdersV2ResponseEntry.reduced_by missing — "
"spec v3.18.0 requires it on every entry (zero on error). "
"Server may have diverged from spec; file with Kalshi."
)
return self
```

This way the spec invariant is explicit, the error message tells the operator exactly what to do, and we still parse cleanly for the spec-conforming case (including the reduced_by: "0" + error block path covered by test_error_entry_parses).

#3 — Async amend_v2 / decrease_v2 query-param tests

These already exist — TestAsyncAmendDecreaseV2QueryParams was added in the round-2 commit (f7fecf8) and covers both amend_v2 and decrease_v2. Living in a shared class at the bottom of the file rather than alongside the per-method classes — likely why it didn't surface in the scan. Asserts the same invariant as the sync version: subaccount in URL params, exchange_index in body.

#4 — create_v2 body serialization test

Added TestCreateOrderV2.test_serializes_body. Inspects route.calls[0].request.content and asserts the full wire body matches an exact dict — catches any regression in DollarDecimal/FixedPointCount serialization or in the model_dump(exclude_none=True, by_alias=True, mode="json") plumbing:

```python
assert body == {
"ticker": "MKT-A", "client_order_id": "cli-1",
"side": "bid", "count": "10", "price": "0.50",
"time_in_force": "good_till_canceled",
"self_trade_prevention_type": "taker_at_cross",
"exchange_index": 0,
}
```

#5 — Async docstrings carry full rationale

Expanded AsyncOrdersResource.amend_v2 and decrease_v2 to include the spec-asymmetry explanation inline rather than "see sync sibling." Async-first users now get the rationale without a cross-reference jump.

#7 — client_order_id V1 vs V2

Expanded CreateOrderV2Request docstring to enumerate the V1→V2 differences in one place:

• side: BookSideLiteral (bid/ask) vs V1's yes/no string.
• client_order_id required (vs V1 optional). Server uses it for V2 idempotency.
• Single price field vs V1's paired yes_price/no_price.

#2, #6 — noted, deferred

#2 — Deposit/Withdrawal _PaymentRecord base — leaving as-is for the model-per-spec-schema convention; will file a tracking issue if the two stay identical through a couple more spec versions.

#6 — params={} empty dict observation — Acknowledged. httpx and the transport both treat {} as "no query string." If the transport ever switches to an is not None gate, the _params(...) helper already strips Nones, so the resulting dict is empty when all args are None. No-op for current behavior; worth a follow-up if the transport evolves.

Validation

mypy clean, ruff clean, all 535 touched tests pass (orders + async_orders + contracts).

[claude]

Code Review — PR #155: sync spec to v3.18.0 + fix asyncapi drift false alarm

Overall this is a solid, well-structured spec-sync PR. The new surface (V2 orders, deposits/withdrawals, endpoint costs) follows established conventions closely. Below are my findings, ordered by severity.

---

Observations & Suggestions

1. BatchCancelOrdersV2ResponseEntry.reduced_by — misleading None default

kalshi/models/orders.py

class BatchCancelOrdersV2ResponseEntry(BaseModel):
    reduced_by: FixedPointCount | None = None   # <-- default None
    ...
    @model_validator(mode="after")
    def _enforce_reduced_by_present(self) -> ...:
        if self.reduced_by is None:
            raise ValueError(...)   # immediately rejects None

The field is typed | None = None but a validator immediately rejects None, making the default meaningless and the type signature misleading. A cleaner approach is to make it non-optional and let Pydantic's own required-field error fire — which is already descriptive — unless the intent is specifically to provide a better error message. If the custom message is the goal, the comment explains it well; but the type annotation should still use FixedPointCount without the None union so static analysis is accurate.

Suggestion: reduced_by: FixedPointCount (no default, no None). The validator can be removed; Pydantic's "field required" error is sufficient, and the docstring already explains the spec invariant.

---

2. Missing tests for new user_filter / rfq_user_filter filter-satisfaction paths

tests/test_communications.py — not modified in this PR

The _require_quote_filter guard was extended to accept user_filter or rfq_user_filter as standalone satisfiers. Per the CLAUDE.md convention: "New function → write a test. Bug fix → write a regression test. New error path → write a test that triggers it." There are no new tests verifying:

• list_quotes(user_filter="self") doesn't raise ValueError even when quote_creator_user_id and rfq_creator_user_id are both None.
• list_quotes(rfq_user_filter="self") same.
• list_quotes() (all filters None) still raises ValueError with the updated message.

The happy-path tests for the new params are present in the resource smoke tests, but the guard-specific tests are missing. These are the cases most likely to silently regress.

---

3. Minor: amend_v2 and decrease_v2 route exchange_index to the body, not a query param — confirm vs spec

kalshi/resources/orders.py

# amend_v2 — exchange_index is in the body (AmendOrderV2Request), not params
# decrease_v2 — exchange_index is in the body (DecreaseOrderV2Request), not params
# cancel_v2  — exchange_index IS a query param

This asymmetry (cancel → query, amend/decrease → body) is correctly handled and there is a regression test for it (TestAmendDecreaseV2QueryParams, sync + async). Just flagging it for reviewer awareness since it diverges from the v1 pattern and could bite callers who try to pass it as a keyword arg expecting query-param semantics.

---

4. client_order_id is required on CreateOrderV2Request — call-site impact

The docstring notes this is for server-side idempotency. Because the v1 create() made it optional, callers migrating from v1 to v2 may not generate a unique ID per call. Consider adding a note in the CHANGELOG or a comment in the model that the caller must ensure uniqueness (UUID4 recommended). This is documentation-level, not a code defect.

---

Confirmed correct

• _delete_with_body is already defined in SyncResource / AsyncResource — no missing implementation.
• def deposits_all/withdrawals_all (no async) on AsyncPortfolioResource is correct per the repo pattern; _list_all returns an AsyncIterator synchronously.
• All V2 request models have extra="forbid" ✓
• All V2 response models have extra="allow" ✓
• DollarDecimal serializes to string "0.50" (not "price_dollars" suffix) for V2 — serialization regression test at TestCreateOrderV2.test_serializes_body confirms the wire shape ✓
• _post() now threads params through for POST-with-query-params (amend/decrease subaccount) ✓
• Contract map, BODY_MODEL_MAP, METHOD_ENDPOINT_MAP, and EXCLUSIONS all updated for new endpoints ✓
• All new models exported from kalshi/__init__.py and kalshi/models/__init__.py ✓
• DecreaseOrderV2Request XOR validator and BatchCancelOrdersV2ResponseEntry presence validator both have corresponding test coverage ✓
• .gitignore fix correctly un-ignores specs/asyncapi.yaml and the comment is updated to match ✓

---

Summary

Two actionable items worth addressing before merge:

1. Fix reduced_by type in BatchCancelOrdersV2ResponseEntry — remove the | None = None and the custom validator, or keep the validator but document clearly why the None default is intentional.
2. Add test_communications.py tests for the three _require_quote_filter paths (user_filter satisfies, rfq_user_filter satisfies, all-None still raises).

Everything else is either correct or documentation-level. The test count jump (1455 → 1866) and contract drift resolution look healthy.

🤖 Generated with Claude Code

[TexasCoding]

Thanks for the fifth pass. Pushed b253c7a. Both actionable items addressed:

#1 — reduced_by type signature

Reverted to FixedPointCount (no default, no None union) and dropped the custom model_validator. You're right that the previous round's pattern was a misleading type signature — the validator existed only to provide a better error message, but Pydantic's "field required" error is descriptive enough on its own. The spec invariant is now documented in the model docstring; if upstream ever omits the field, Pydantic raises ValidationError with the standard message, which is plenty actionable. Cleaner all around.

#2 — _require_quote_filter regression tests

Added four new tests in tests/test_communications.py:

• TestListQuotes.test_user_filter_alone_is_sufficient — confirms list_quotes(user_filter="self") doesn't raise even when both *_creator_user_id filters are None.
• TestListQuotes.test_rfq_user_filter_alone_is_sufficient — same for rfq_user_filter.
• TestListQuotes.test_raises_lists_all_four_satisfiers — the updated error message must enumerate all four valid filters; uses substring assertions so the message can evolve without becoming brittle.
• TestAsyncCommunications.test_list_quotes_user_filter_alone_is_sufficient + ..._rfq_user_filter_alone_is_sufficient — mirror coverage on the async surface.

The original test_raises_without_creator_filter regex match still works (it matches quote_creator_user_id.*rfq_creator_user_id and the new message still contains both), so no need to update that one.

#4 — client_order_id migration note

Added a "Migration note" subsection to the v2.1.0 CHANGELOG entry:

CreateOrderV2Request.client_order_id is required. V1's CreateOrderRequest made it optional, so callers migrating from orders.create() to orders.create_v2() must generate a unique client-order-id per call (UUID4 is the common choice). The server uses this field as the V2 idempotency key, so reusing a value will cause the server to return the original order rather than placing a new one.

The idempotency-key behavior is worth highlighting because it's not just a "now required" change — it's a behavior change. A migrator who passes the same value twice gets surprising semantics, not an error.

Validation

mypy clean, ruff clean. 610 tests pass across the touched files (orders + async_orders + communications + contracts).

[claude]

Code Review — PR #155: Sync spec to v3.18.0 + fix asyncapi drift false alarm

Overall this is a well-structured, large spec sync. The V2 order family, deposits/withdrawals, and endpoint_costs are all properly implemented with solid test coverage. A few issues below, ranging from a real spec omission to minor observations.

---

Bug / Spec Omission

Balance model missing balance_dollars and balance_breakdown (spec drift)

The v3.18.0 GetBalanceResponse schema was updated to add two new fields:

required:
  - balance
  - balance_dollars   # <-- NEW, now required
  - portfolio_value
  - updated_ts
properties:
  balance_dollars:
    $ref: '#/components/schemas/FixedPointDollars'
  balance_breakdown:   # <-- NEW, optional array of IndexedBalance
    type: array
    items:
      $ref: '#/components/schemas/IndexedBalance'

Neither was added to kalshi/models/portfolio.py's Balance model. balance_dollars is required in the updated spec. Because Balance uses extra="allow" this won't crash at runtime, but users can't access balance.balance_dollars as a typed attribute.

Suggested fix:

class Balance(BaseModel):
    balance: int
    balance_dollars: DollarDecimal  # new in v3.18.0
    portfolio_value: int
    updated_ts: int
    balance_breakdown: list[IndexedBalance] | None = None  # new in v3.18.0

    model_config = {"extra": "allow"}


class IndexedBalance(BaseModel):
    exchange_index: int
    balance: DollarDecimal

    model_config = {"extra": "allow"}

This is the only spec item in the 3.13→3.18 delta that appears to have been missed.

---

Minor Observations

cancel_v2 async query-param regression test is missing

TestAsyncAmendDecreaseV2QueryParams covers the subaccount/exchange_index split for amend_v2 and decrease_v2 async paths, but there's no async equivalent for cancel_v2 query params. The sync TestCancelOrderV2.test_passes_query_params catches this, but the async transport is a different code path. Low risk given the pattern is consistent, but the gap is worth filling.

_post() params kwarg — existing call sites unaffected

The params: dict[str, Any] | None = None addition to _post() is clean and backward-compatible. The only new callers are amend_v2/decrease_v2, and the regression tests explicitly assert subaccount reaches the query string (not the body). The transport test suite covers the general case. ✓

AmendOrderV2Request required fields match spec

ticker, side, price, and count are all required in the spec's AmendOrderV2Request schema, so making them non-optional on the Pydantic model is correct, even though it's more restrictive than a typical "patch" API. The spec-first approach is right here. ✓

def deposits_all() / withdrawals_all() on AsyncPortfolioResource (not async def)

This is intentional and consistent with the established SDK pattern (CLAUDE.md: "Async list_all() returns AsyncIterator directly"). The method returns an async iterator synchronously, and callers use async for item in client.portfolio.deposits_all(). ✓

_delete_with_body() in batch_cancel_v2

The method already exists in _base.py (lines 133/265 for sync/async). The diff only added params support to _post(); _delete_with_body() was already there and already tested. ✓

BatchCancelOrdersV2ResponseEntry.reduced_by required per spec

The docstring documents the spec invariant ("when error is set, reduced_by is still present and is 0"). The test_error_entry_parses regression test locks this in. If the API ever violates this, the SDK will raise a ValidationError rather than silently returning None. Good defensive choice. ✓

---

Test Coverage Summary

Coverage is strong: 1866 tests, 0 failures, all 23 drift failures resolved without new EXCLUSIONS. Notable highlights:

• Happy path + 204-raises + query-param routing for all 6 V2 order methods (sync)
• Full async mirrors for create/cancel/amend/decrease/batch_create/batch_cancel
• TestAsyncAmendDecreaseV2QueryParams is a nice regression guard for the subaccount/exchange_index split
• Deposit/withdrawal pagination tested for both page and _all variants (sync + async)

The only meaningful gap is the async cancel_v2 query-param test noted above.

---

Summary

One real issue to fix before merge: Balance.balance_dollars (required in v3.18.0 spec) is missing from the model. Everything else is clean. The _post(params=...) extension is the right approach for the amend/decrease asymmetry, and the V2 contract registrations look correct.

[TexasCoding]

Good catch — this was a real spec-drift miss. Pushed 9204b72.

#1 — Balance.balance_dollars + balance_breakdown (the bug)

Confirmed against specs/openapi.yaml:5051-5084: v3.18.0 marks balance_dollars as required and adds the optional balance_breakdown array of a new IndexedBalance schema. None of those landed in the earlier rounds because the contract drift tests only check request bodies (TestRequestBodyDrift) — required-field additions on response models slip through entirely.

Added in this commit:

• New IndexedBalance model (exchange_index: int, balance: DollarDecimal), exported from kalshi and kalshi.models.
• Balance.balance_dollars: DollarDecimal (required) and Balance.balance_breakdown: list[IndexedBalance] | None (optional).
• Updated every existing balance test mock to include balance_dollars (they'd fail validation now without it).
• New test_balance_breakdown (happy path) and test_balance_breakdown_omitted (None default when absent).
• integration/test_portfolio.py::test_balance now asserts balance_dollars is non-null.
• CHANGELOG v2.1.0 entry enumerates the new fields and IndexedBalance.

Coverage gap (acknowledged, deferred)

This is exactly the kind of thing the existing drift infrastructure should catch. The current TestRequestParamDrift and TestRequestBodyDrift walk every (method, endpoint) and (model, $ref) pair on the request side, but there's no equivalent for response models. That's why this slipped through 5 rounds of review before getting caught by direct spec inspection.

I'll file a follow-up issue for response-side drift detection — should walk every response schema referenced in METHOD_ENDPOINT_MAP, look up the SDK response model via a RESPONSE_MODEL_MAP (parallel to BODY_MODEL_MAP), and assert required spec properties exist on the model. Same kind of "intentional deviations require an exclusion with a reason" discipline as the request-side drift tests.

#2 — Async cancel_v2 query-param test

Added TestAsyncCancelOrderV2.test_passes_query_params. Mirror of the sync version — asserts both subaccount AND exchange_index reach the URL query string (this is the path where exchange_index is query, unlike amend/decrease where it's body).

Validation

mypy clean, ruff clean. 653 tests pass across the touched files (was 610 in the previous round — net +43 for the Balance changes + async cancel_v2 guard).

[claude]

Code Review — PR #155: Sync spec to v3.18.0 + fix AsyncAPI drift false alarm

Overall this is a well-structured spec-sync PR. The new resources follow established patterns, mypy/ruff/pytest all pass, and the contract drift tests are clean. A few items to address before merge.

---

Findings

Medium — Type asymmetry: ts_ms required vs optional across V2 order responses

kalshi/models/orders.py — CreateOrderV2Response.ts_ms is int (required), but BatchCreateOrdersV2ResponseEntry.ts_ms is int | None = None. This is spec-faithful (batch entries may be error entries without a timestamp), but callers migrating from create_v2 to batch_create_v2 for single orders will hit a type mismatch silently. Worth a docstring note on BatchCreateOrdersV2ResponseEntry explaining why ts_ms is optional there.

---

Low — Confusing cents vs. DollarDecimal inconsistency in portfolio models

kalshi/models/portfolio.py:

• Balance.balance → int (cents)
• IndexedBalance.balance → DollarDecimal (fixed-point dollars per spec)
• Deposit.amount_cents / fee_cents → int (cents, correct per spec)

The IndexedBalance.balance naming shadows Balance.balance with a different type. A reader doing balance.balance_breakdown[i].balance expects cents but gets a Decimal. Consider either renaming IndexedBalance.balance to something like balance_dollars or adding an explicit docstring comment calling out the type difference.

---

Low — Missing test: withdrawals() has no auth-guard test (sync or async)

tests/test_portfolio.py — TestPortfolioDeposits has a test_auth_failure that verifies _require_auth() fires before the network call. TestPortfolioWithdrawals (sync) and TestAsyncPortfolioWithdrawals have no equivalent. The existing tests mock a 401 response but don't test the client-side early-exit path. Other resources consistently test both the 401-from-server path and the AuthRequiredError-before-request path.

---

Low — Async test_decrease_v2 doesn't assert exchange_index in request body

tests/test_async_orders.py — TestAsyncAmendDecreaseV2QueryParams.test_decrease_v2 verifies subaccount lands in query params, but the sync counterpart also asserts body.get("exchange_index") == 0. The async test is missing that body assertion. Small but worth keeping symmetric.

---

Low — Async TestAsyncAccountEndpointCosts missing empty-list test

tests/test_account.py — the sync TestAccountEndpointCosts has test_returns_empty_costs; the async counterpart does not. Low-risk omission since the empty-list path exercises no special logic, but the pattern is inconsistent.

---

Nit — _post now silently accepts params — future footgun

kalshi/resources/_base.py — adding params: dict[str, Any] | None = None to _post / async _post is correct for the spec-driven amend_v2/decrease_v2 cases. Just noting: there is no guard preventing callers from accidentally adding query params to POST requests that shouldn't have them. The current callers are all correct; this is a future maintainability note, not a blocking issue.

---

Nit — deposits_all / withdrawals_all async docstrings

kalshi/resources/portfolio.py — The async deposits_all and withdrawals_all use plain def (not async def) so the auth guard fires eagerly. This is the correct established pattern, but unlike AsyncIncentiveProgramsResource.list_all these methods lack a docstring note explaining """Returns an async iterator — use ``async for``.""". Minor, but it matches the documentation style of other list_all async methods.

---

High (resolved) — Historical endpoints removed from spec

Initial concern: specs/openapi.yaml showed 7 historical paths deleted. Confirmed: they are re-added at a different position in the file (7 deletions, 7 additions, net zero). Not a real issue.

---

Positive observations

• All 6 V2 order methods call _require_auth() before any I/O — consistent with policy.
• batch_cancel_v2 correctly uses _delete_with_body rather than bending the standard DELETE path.
• cancel_v2 returns CancelOrderV2Response (not None) and the 204-guard is present in both sync and async — good defensive handling.
• CreateOrderV2Request.extra="forbid" propagates into BatchCreateOrdersV2Request inner entries — correct nested validation.
• The .gitignore fix is clearly the right call; it directly addresses the false-alarm root cause.
• 1866 tests passing with all 372 contract drift tests green is a strong signal.

---

Blocking: none — the medium/low items above are improvements, not blockers. Happy to merge once the ts_ms asymmetry and missing withdrawals auth test are addressed (or documented as intentional deferrals).

[TexasCoding]

Thanks for the seventh pass. Pushed 83e1e32. All items addressed:

Docstring clarifications

ts_ms asymmetry — BatchCreateOrdersV2ResponseEntry docstring now explains why ts_ms is optional there but required on CreateOrderV2Response: batch entries can be per-order errors with no timestamp, while a single create either succeeds with a timestamp or raises an HTTP error (never a per-entry error block).

IndexedBalance.balance type collision — Good call-out. Added an explicit "Type note" to the docstring flagging that the field is DollarDecimal (per spec), unlike Balance.balance which is integer cents. Same field name, different semantics. Cross-referenced Balance.balance_dollars since that's the field rendered in matching units.

Async iterator docstrings — AsyncPortfolioResource.deposits_all and withdrawals_all now carry """Returns an async iterator — use ``async for``.""", matching AsyncIncentiveProgramsResource.list_all's pattern.

Test gaps

• TestPortfolioWithdrawals.test_auth_failure + async mirror — every other resource had both the 401-from-server path and the AuthRequiredError-before-request path covered; withdrawals was missing the 401-mock side.
• TestAsyncAmendDecreaseV2QueryParams.test_decrease_v2 now also asserts body.get("exchange_index") == 0 and that it does NOT leak into the URL params (matching the sync version's assertions exactly).
• TestAsyncAccountEndpointCosts.test_returns_empty_costs — parity with the sync empty-list test.

Acknowledged, no action

_post(params=...) future footgun — Noted. The current callers (amend_v2/decrease_v2) are correct and have regression tests, and the next likely V2 endpoint that needs this will have its own spec-driven test. If we ever get a third call site, it's worth considering a stricter helper (e.g. _post_with_query to make the intent explicit), but premature for two callers.

Validation

mypy clean, ruff clean. 667 tests pass across touched files (was 653 in round 6).

[claude]

Code Review — PR #155: sync spec to v3.18.0 + fix asyncapi drift

Overall this is a solid, well-structured spec-sync PR. The code follows project conventions closely and the test coverage is thorough. A few things worth addressing before merge.

---

Medium: Balance.balance_dollars is an undocumented breaking change

balance_dollars: DollarDecimal is added as a required field (no default) on Balance:

class Balance(BaseModel):
    balance: int
    balance_dollars: DollarDecimal   # required, no default
    ...

Any code that constructs Balance manually — including tests outside this repo — will get a ValidationError after upgrading. The API guaranteeing to return this field in v3.18.0 protects API-response callers, but not callers that build Balance instances directly (mocks, test helpers, etc.). This is a soft breaking change worth calling out explicitly in the CHANGELOG under a ### Changed or ### Breaking subsection for 2.1.0, since semver minor versions are supposed to be backwards-compatible.

If the field isn't actually required by the spec (i.e., it's just "always present in practice"), making it Optional[DollarDecimal] = None would be the safer choice.

---

Minor: deposits_all/withdrawals_all don't test the local _require_auth() path

TestPortfolioDeposits.test_auth_failure mocks a 401 server response — it tests the HTTP error mapping path, not the _require_auth() pre-check. Every other *_all method in the portfolio suite has a corresponding test with the unauth_portfolio fixture. Two gaps:

• TestPortfolioDeposits — no unauthenticated client test for deposits_all
• TestPortfolioWithdrawals — no unauthenticated client test for withdrawals_all

Not critical since _require_auth() itself is well-exercised elsewhere, but it breaks the consistency of the suite.

---

Minor: async create_v2 is missing body-serialization test

The sync TestCreateOrderV2.test_serializes_body explicitly asserts the wire shape (price as "0.50", count as "10", no phantom keys). The async counterpart TestAsyncCreateOrderV2 has only a happy-path test. The body-serialization test matters because it guards against DollarDecimal/FixedPointCount regressions in async dispatch — it should be ported to test_async_orders.py.

---

Nit: Deposit and Withdrawal are field-for-field identical

class Deposit(BaseModel):
    id: str; status: PaymentStatusLiteral; type: PaymentTypeLiteral
    amount_cents: int; fee_cents: int; created_ts: int; finalized_ts: int | None = None

class Withdrawal(BaseModel):    # exact same fields
    ...

Per project style the two separate types are defensible (spec has separate schemas today), but the shared PaymentStatusLiteral / PaymentTypeLiteral aliases suggest the intent to keep them aligned. If the spec keeps them identical in future versions, a shared base class would be worth considering. No action required now — just flagging for awareness.

---

What looks good

• Spec asymmetry is documented. The amend_v2/decrease_v2 docstrings explaining why subaccount is a query param while exchange_index is a body field are exactly the right kind of comment per the project's "non-obvious WHY" comment policy.
• DecreaseOrderV2Request._enforce_reduce_xor validator — clean XOR enforcement with clear error messages; tests cover both failure modes.
• cancel_v2 / batch_cancel_v2 guard against 204 No Content — raising KalshiError when a response body is expected is the right defensive pattern.
• _post gains a params kwarg — minimal, correct change to support the mixed query/body V2 endpoints.
• Contract drift tests green — all 372 passing without any new EXCLUSIONS placeholders is the right outcome.
• gitignore fix — pinning specs/asyncapi.yaml to eliminate the Monday false-alarm drift issue is a clean infra fix.
• Test parity across sync/async — every V2 method has matching coverage in both test_orders.py and test_async_orders.py.

The blocking item is the balance_dollars CHANGELOG callout; the rest are improvements for before or after merge.

[TexasCoding]

Thanks for the eighth pass. Pushed 197ec5b. All three items addressed:

#1 (blocking) — Balance.balance_dollars CHANGELOG callout

Added a ### Changed subsection to the v2.1.0 entry calling out the soft-breaking change explicitly:

Balance gained a required balance_dollars: DollarDecimal field (added by spec v3.18.0). Callers who construct Balance from API responses are unaffected — the server now guarantees the field. But callers who build Balance(...) instances directly in their own tests or mocks will hit ValidationError until they add it.

Includes a before/after construction example. Kept the field required (no Optional fallback) because spec v3.18.0 marks it required in GetBalanceResponse; making it optional would be SDK drift from the spec. The CHANGELOG callout is the right place to communicate the construction-site impact.

#2 — _require_auth() pre-check tests for deposits/withdrawals

Added new unauth_portfolio + unauth_async_portfolio fixtures and these tests on both sync and async classes:

• test_deposits_requires_auth
• test_deposits_all_requires_auth
• test_withdrawals_requires_auth
• test_withdrawals_all_requires_auth

The existing test_auth_failure tests cover the 401-from-server path; these cover the local pre-check path where _require_auth() raises before any network call. The *_all variants are particularly important because they return iterators — the test confirms AuthRequiredError raises at call time, not on first iteration.

#3 — Async create_v2 body serialization test

Added TestAsyncCreateOrderV2.test_serializes_body mirroring the sync version's exact-dict assertion. Guards DollarDecimal / FixedPointCount mode="json" serialization on the async dispatch path. Now both sync and async are protected against regressions in model_dump(exclude_none=True, by_alias=True, mode="json") plumbing.

#4 (nit) — Deposit/Withdrawal consolidation

Noted, no change. Will revisit if upstream consolidates the two schemas in a future spec.

Validation

mypy clean, ruff clean. 503 tests pass across the touched files.

[claude]

Code Review — PR #155: sync spec to v3.18.0 + fix asyncapi drift false alarm

Overall: Well-executed spec sync. The code is consistent with project conventions, contract drift tests are clean (372 passed), and the full test suite is green (1866 passed). A few observations below — nothing blocking.

---

Strengths

• V2 model-only surface is the right call. Dropping the kwarg-overload pattern for V2 keeps the surface area minimal; the V1 dual-overload complexity was driven by wanting create(ticker="X", side="yes", ...) ergonomics — V2's required client_order_id and side=BookSideLiteral make a typed request model strictly better.
• Spec asymmetry is well-documented. The in-method comments on amend_v2/decrease_v2 explaining why subaccount is a query param while exchange_index is on the body are exactly the kind of "non-obvious why" comments CLAUDE.md calls for.
• DecreaseOrderV2Request._enforce_reduce_xor mirrors the v1 validator — consistent.
• cancel_v2/batch_cancel_v2 guard against 204 No Content — correct, and tested.
• _post gaining a params kwarg is backward-compatible and minimal.

---

Minor Issues / Questions

1. BatchCancelOrdersV2ResponseEntry — order_id and reduced_by are required

The model comment is correct ("spec marks them required"), but the inline error-entry shape described in the comment ("when error is set, reduced_by is still present and is 0") deserves a test. If a live server ever returns an error entry without order_id or reduced_by, the SDK will raise ValidationError rather than propagating the per-entry error field. Consider adding one test for the error-entry case with reduced_by=0 to lock this invariant in.

2. Deposit / Withdrawal are structurally identical

Both have exactly the same six fields. PaymentStatusLiteral and PaymentTypeLiteral are already shared. If the spec ever diverges these structs (e.g., Withdrawal gains a destination_account field), having separate models is the right foundation — just noting the current identity so it's on record.

3. PaymentStatusLiteral / PaymentTypeLiteral are shared across both models

The comment in portfolio.py correctly explains the rationale ("spec defines two structurally-identical inline enums"). Fine as-is; the risk is spec divergence. Low priority.

4. CreateOrderV2Request.price has no serialization_alias

V1 uses serialization_alias="price_dollars" for prices (per CLAUDE.md). V2 uses price: DollarDecimal with no alias. The contract tests pass, so the V2 spec apparently uses price as the bare wire key. Worth a one-line comment on the model noting this intentional departure from the V1 _dollars-suffix convention so the next reader doesn't add an alias to "fix" it.

5. AmendOrderV2Request requires ticker and side

The V1 AmendOrderRequest makes these optional (you only send what you're changing). If this is spec-accurate for V2 that's fine — but it's a notable divergence from V1 semantics that could trip up callers migrating. Worth a line in the migration note in CHANGELOG.md.

6. _delete_with_body is not in the _base.py diff

Not a bug (tests pass), but confirming _delete_with_body already exists on both SyncResource and AsyncResource from a prior PR — double-checking since it's a less common method and the diff only shows _post changing.

---

Test Coverage

Coverage is solid:

• Happy path, error path, and auth-required for every new method.
• Wire serialization shape verified for create_v2 (test_serializes_body) — good regression guard.
• Pagination for deposits_all / withdrawals_all.
• _require_quote_filter updated to accept user_filter / rfq_user_filter — existing quote tests implicitly cover this.

One gap noted above: no test for BatchCancelOrdersV2ResponseEntry with an error-entry payload (where error is set and reduced_by is 0).

---

Summary

No blocking issues. The two actionable suggestions:

1. Add a test for the BatchCancelOrdersV2ResponseEntry error-entry shape.
2. Add a comment on CreateOrderV2Request.price noting the intentional absence of a _dollars serialization alias (V2 wire format differs from V1).

Everything else is informational. Ready to merge after those are addressed (or consciously deferred to a follow-up).