[TexasCoding]

7 issues across security, HTTP transport, WebSocket reliability, models/types, REST resources, performance, and docs — identified by an independent 7-agent review of v2.5.0 and executed across 3 sequential waves (W0 docs, W1 money/correctness, W2 polish) in disjoint git worktrees. 7 PRs merged ahead of this release commit.

Wave summary

Wave	Issues / PRs	Outcome
W0 docs	#299 (#303), #300 (#304)	docs/resources/orders.md stale comment dropped; docs/configuration.md reference table completed
W1 money/correctness	#295 (#305), #296 (#306), #297 (#307), #298 (#308)	StrictInt across all Request models; orderbook snapshot identity adoption restored; WS re-entry guard + partial-failure state reset; case-insensitive header merge + KALSHI-ACCESS-* rejection + Content-Type pin
W2 polish	#301 (#309)	Sync/AsyncTransport.close() explicitly idempotent

Breaking changes

• int request fields reject bool (bug: int-typed request fields silently coerce bool (subaccount routing, transfers, counts) #295) — every money-routing / counting integer of every Request model now uses kalshi.StrictInt. bool is an int subclass and used to silently coerce True→1 / False→0, misrouting subaccount/from_subaccount/amount_cents/count/etc. Now raises ValidationError.
• KALSHI-ACCESS-* rejected in extra_headers (bug(transport): case-mismatched extra_headers keys leak alongside auth/body Content-Type on the wire #298) — rejected at both KalshiConfig(extra_headers=) construction and per-request extra_headers= kwargs. The previous case-sensitive merge let a caller-supplied lowercase kalshi-access-key co-exist with the SDK-signed uppercase one on the wire.

Migration guide: docs/migration.md v2.5 → v2.6 section covers both fences and the raw-subscribe_orderbook_delta aliasing note.

Verification

• uv run mypy kalshi/ clean (76 files)
• uv run ruff check . clean
• uv run pytest tests/ — W1 / W2 commits brought the count to 2780+ passing
• gitnexus index refreshed post-W1: 8,272 nodes / 16,951 edges / 300 flows

Closes

Closes #295. Closes #296. Closes #297. Closes #298. Closes #299. Closes #300. Closes #301.

---

Release tag: not pushed yet. After this PR merges, the v2.6.0 tag is what triggers the .github/workflows/release.yml workflow which builds the wheel, runs twine check, and publishes to PyPI via OIDC. Tag intentionally deferred so the maintainer reviews + approves the actual PyPI publish.

[claude]

Code Review — PR #310: release 2.6.0

This is a release-commit PR: the diff itself is CHANGELOG, ROADMAP, migration docs, and version bumps. The real substance is in the 7 already-merged component PRs (#303–#309). I reviewed the actual implementation of each wave.

---

Overview

Clean, well-structured release. Two breaking changes with genuine safety value, one performance restoration, two correctness fixes, and two polish items. All waves executed in disjoint worktrees; the approach is sound.

---

Wave 1 — StrictInt across Request models (#305)

Implementation is correct. _reject_bool_int uses isinstance(value, bool) before the int check, which is the right guard since bool is an int subclass in Python. StrictInt is properly exported from kalshi/__init__.py and __all__.

Minor inconsistency (low severity): _reject_bool_int raises ValueError, while _coerce_decimal raises TypeError for the same bool input. Both surface to callers as pydantic.ValidationError, so there's no user-visible difference — but it's worth aligning to one convention if this pattern is extended further.

Coverage looks correct for request models. I checked subaccount_number specifically: it's int (not StrictInt) on the Order and Trade response models (correct — response models don't need the guard), and StrictInt on UpdateSubaccountNettingRequest (correct). The CHANGELOG's claim of full request-model coverage appears accurate.

---

Wave 1 — Orderbook snapshot identity adoption (#306)

_apply_snapshot_inplace correctly drops the dict(...) wrappers and assigns by identity. ✓

apply_snapshot defensive copy is correct but subtle — it's worth calling out for future readers:

1. _apply_snapshot_inplace sets state.yes = msg.msg.yes (identity).
2. apply_snapshot then immediately does state.yes = dict(msg.msg.yes), replacing the adopted reference with a copy.

This means state.yes ends up as a copy, which is the intended safety contract. The ordering is important: if the copy were placed inside _apply_snapshot_inplace on a conditional, the hot-path guarantee would be fragile. Placing it after-the-fact in the public wrapper is the right architectural split.

Aliasing hazard note in migration docs is correct and clearly written. Raw subscribe_orderbook_delta consumers who retain msg.msg.yes after a snapshot will see future delta mutations because the recv loop never calls apply_snapshot. This is a real footgun — consider whether subscribe_orderbook_delta's docstring should carry a one-liner warning in addition to the migration doc entry.

---

Wave 1 — WebSocket re-entry guard (#307)

_start() guard (if self._connection is not None or self._running) catches both the mid-connect and fully-running states. ✓

Partial-connect failure cleanup in the except BaseException block correctly closes and nulls self._connection without touching self._sub_mgr etc. (those are only assigned after connect() succeeds, so they remain None on a connect failure). ✓

_stop() manager teardown clears all refs after await self._connection.close(), which is the right order — teardown logic still has manager refs during shutdown. ✓

One edge to verify: _pause_granted and _resume_signal events are .clear()'d in _stop(). If a reused instance has callers awaiting one of these before the prior session fully exits, they'll get stuck. The comment "Reset state AFTER awaiting close" covers this ordering, but it may be worth a test that covers the pause-then-reconnect path (not just subscribe-then-reconnect).

---

Wave 1 — Transport security (#308)

_ci_merge implementation is correct. Verified the case-dedup logic handles canonical-case conflicts across layers and same-layer duplicates as expected. ✓

_assert_no_auth_headers correctly uses .lower().startswith(AUTH_HEADER_PREFIX) where AUTH_HEADER_PREFIX = "kalshi-access-". Extracting the prefix into _constants.py to avoid the circular-import hazard with config.py is a good structural call. ✓

Construction-time validation in KalshiConfig.__post_init__ closes the bypass where extra_headers at construction time could escape the per-request guard. ✓

Explicit Content-Type: application/json pin in _base.py for all JSON helpers. Checked — all _post, _put, _delete_with_body, and async mirrors pin the header explicitly. ✓

---

Wave 2 — Idempotent close() (#309)

Correct and minimal. _closed flag matches the established KalshiAuth / KalshiClient pattern. The docstring's honest scoping note ("sync is sequential-safe; async is race-free under cooperative scheduling") is appropriate. ✓

---

Documentation (PR #303, #304)

docs/migration.md v2.5 → v2.6 section is thorough. The code examples for both breaking changes are concrete and accurate. The aliasing note for raw subscribers is present and correct.

Minor nit in migration.md: The subscribe_orderbook_delta aliasing note shows:

async for msg in stream:
    if isinstance(msg, OrderbookSnapshotMessage):
        held = msg.msg.yes        # ← LIVE dict, mutated by next delta

The comment clarifies the hazard, but the example of stream is ambiguous — subscribe_orderbook_delta returns a typed iterator. Tightening the example to call out that stream is specifically from subscribe_orderbook_delta (not client.orderbook(ticker)) would prevent confusion.

---

CHANGELOG / ROADMAP / version bumps

The CHANGELOG entry matches the actual changes accurately. The wave-summary table is a helpful audit trail. Version bump from 2.5.0 → 2.6.0 is consistent across kalshi/__init__.py and pyproject.toml. ✓

---

Summary

Area	Verdict
Code correctness	✅ All implementations verified correct
Project conventions	✅ StrictInt pattern, extra="forbid" on Request models, _closed flag
Security (auth forge surface)	✅ Closed at both construction and per-request call sites
Performance (orderbook)	✅ Identity adoption restored on hot path
Test coverage	✅ Each PR includes targeted regression/pinning tests
Breaking change docs	✅ Migration guide is complete and accurate

Recommend approval. One actionable suggestion before the v2.6.0 tag:

• Consider adding a one-liner warning in the subscribe_orderbook_delta docstring about the snapshot aliasing hazard (perf(ws): _apply_snapshot_inplace dict() copy nullifies #263 identity-adoption speedup #296), since the migration guide alone won't be seen by users who don't upgrade. Low severity, but the mutation-through-reference footgun is real.

[TexasCoding]

Bot recommends approval. The one actionable item — "consider adding a one-liner warning in subscribe_orderbook_delta docstring about the snapshot aliasing hazard" — was already shipped in #306 round-4 (commit 6fb6115, also in main as ab96247): the public subscribe_orderbook_delta docstring already carries an explicit aliasing note that points readers at client.orderbook(ticker) for immutable views, and the parallel note lives on OrderbookSnapshotPayload. Release-PR bot didn't reach the merged docstring in its scan.

Merging. The v2.6.0 tag is intentionally NOT pushed here — maintainer can tag via git tag v2.6.0 && git push origin v2.6.0 to trigger the OIDC PyPI publish through release.yml (gated by the pypi GitHub environment).