perps: full Perps (margin) API — REST + WebSocket + SCM/Klear (#387)

[TexasCoding]

Perps (margin) API — full SDK implementation

Implements the entire Kalshi Perps (margin) milestone (#387): the perpetual-futures REST + WebSocket surfaces and the Self-Clearing-Member "Klear" settlement API, as standalone clients alongside the existing prediction-API surface. Additive — no changes to KalshiClient/AsyncKalshiClient behavior.

What's included

• PerpsClient / AsyncPerpsClient — separate host (external-api.kalshi.com) + KALSHI_PERPS_* credentials, reusing the RSA-PSS signer + HTTP transport unchanged. 8 resource families (exchange, markets, orders, order_groups, portfolio, margin, funding, transfers), 34 REST operations. Margin order side is bid/ask; prices DollarDecimal, counts FixedPointCount.
• PerpsWebSocket — 6 typed channels (ticker carries funding_rate + next_funding_time_ms, orderbook_delta, trade, fill, user_orders, order_group_updates); epoch-ms *_ms timestamps; zero edits to the shared prediction-API WS (forked orderbook, subclassed sequence tracker/connection).
• KlearClient / AsyncKlearClient — a third auth surface (cookie-session + MFA login), 9 SCM settlement endpoints (reports, obligations, settlement/guaranty-fund balances, withdrawal). Money fields are integer centicents.
• Per-spec contract-drift harness (TestPerps*Drift / TestPerpsScm*Drift), 3 vendored specs + weekly spec-sync coverage, docs/perps.md + README section, 3 runnable examples/, CHANGELOG + version bump 3.1.0 → 3.2.0, and gated demo integration tests.

Verification

• ruff check . ✓ · mypy kalshi/ (strict) ✓ (132 files) · mkdocs build --strict ✓
• Full hermetic pytest tests/: 3585 passed, 316 skipped, 0 failed (~660 new tests; perps integration tests auto-skip without demo credentials).
• Security: the SCM/Klear layer passed an adversarial security review (credential leakage, cookie confinement, no-retry on login/withdraw, session guard on every endpoint, MFA non-auto-loop, host/path/SSRF — all clean). The one finding (the real-money withdraw_settlement_balance accepting negative/zero amounts) was fixed with a positive-amount boundary guard + regression test.

Notable

• Spec catches: the SCM spec ships a 9th endpoint (guaranty_fund_balance) the issue text missed; corrected several issue-vs-spec discrepancies (order_groups.delete has no exchange_index; orders.list_all needed limit; PriceDistributionHistorical is required-but-nullable).
• PerpsConfig/KlearConfig subclass KalshiConfig (Liskov) so the transport is reused unchanged. Perps models are namespaced under kalshi.perps (not flooded into kalshi.__all__) since many names twin the prediction-API models.

Commit map

Per-issue commits where cleanly separable; WS (#397/#398) and SCM (#399/#400) landed one commit each (their code is intertwined in dispatch.py/client.py).

Closes #388
Closes #389
Closes #390
Closes #391
Closes #392
Closes #393
Closes #394
Closes #395
Closes #396
Closes #397
Closes #398
Closes #399
Closes #400
Closes #401
Completes the #387 epic.

🤖 Generated with Claude Code

[claude]

Code Review — Perps (margin) API full implementation (#387 epic)

This is a large but well-structured PR. The security posture is strong, the conventions are consistently applied, and the test coverage is comprehensive. I have a handful of substantive findings and a few minor ones.

---

Overview

Adds PerpsClient/AsyncPerpsClient (34 REST operations, separate host), PerpsWebSocket (6 typed channels), and KlearClient/AsyncKlearClient (9 SCM settlement endpoints, cookie-session auth). Architecture is additive — no changes to the existing prediction-API surface. Broadly well done.

---

Findings

1. MarginResource (Klear) is misplaced — namespace inconsistency

File: kalshi/perps/resources/margin.py

MarginResource and AsyncMarginResource inherit from KlearSyncResource/KlearAsyncResource (Klear-specific base classes), but live in kalshi/perps/resources/ rather than kalshi/perps/klear/resources/. The auth resource is correctly placed at kalshi/perps/klear/resources/auth.py. The Klear client imports it across the namespace boundary:

# klear/client.py
from kalshi.perps.resources.margin import MarginResource  # ← should be klear/resources/margin.py

This is confusing for maintainers and risks callers importing the Klear resource thinking it's a perps REST resource. The fix is to move the file to kalshi/perps/klear/resources/margin.py and update the single import in klear/client.py (and klear/async_client.py).

---

2. PerpsConfig.__post_init__ skips super().__post_init__() — long-term maintenance risk

File: kalshi/perps/config.py:71

Skipping super().__post_init__() is correctly motivated (the parent rejects non-prediction-API hosts), but it means any future security check added to KalshiConfig.__post_init__ — new header validation, stricter protocol enforcement, etc. — will silently not apply to perps. This has already happened once (the KALSHI-ACCESS-* header rejection and extra_headers freeze are re-implemented manually here rather than inherited).

Suggest adding a comment naming the exact checks that were manually ported from KalshiConfig.__post_init__, so a future maintainer knows what needs staying in sync. Something like:

# Manually mirrors KalshiConfig.__post_init__ checks not inherited here:
# - trailing-slash strip on base_url / ws_base_url
# - KALSHI-ACCESS-* header rejection in extra_headers
# - extra_headers freeze to MappingProxyType
# - h2 spec check
# When KalshiConfig.__post_init__ gains new guards, mirror them here.

---

3. _apply_delta_inplace silently routes unknown side values to ask

File: kalshi/perps/ws/orderbook.py:180

levels = state.bid if str(side) == "bid" else state.ask

If side is neither "bid" nor "ask" (e.g., a new wire value, a parse error, a future spec change), this silently applies the delta to the ask side. A bad delta on the wrong side corrupts the local orderbook state with no log warning and no exception. The "bid" branch is correct; consider making the else-branch explicit:

if str(side) == "bid":
    levels = state.bid
elif str(side) == "ask":
    levels = state.ask
else:
    logger.warning("Unknown perps orderbook side %r for %s; dropping delta", side, ticker)
    return False

---

4. Bid levels sorted ascending — may mismatch REST convention

File: kalshi/perps/ws/orderbook.py:71-73

bid_levels = [
    MarginOrderbookLevel.model_construct(price=price, quantity=qty)
    for price, qty in sorted(self.bid.items())   # ascending
]

Standard orderbook convention is bids descending (best bid first), asks ascending (best ask first). The _PerpsBookState.to_orderbook sorts both sides ascending, so the WS-produced MarginOrderbook will have bids in a different order than whatever the REST GET /margin/markets/{ticker}/orderbook returns (if that endpoint sorts bids descending). Worth verifying against the spec and the REST resource's implementation.

---

5. Fatal-error teardown in _recv_loop duplicates _stop() cleanup

File: kalshi/perps/ws/client.py:308-323

The KalshiBackpressureError / KalshiSubscriptionError path in _recv_loop manually nils out _connection, _sub_mgr, _seq_tracker, _orderbook_mgr, _dispatcher and resets the pause events. This is the same set of assignments as _stop(). If a new manager is added (e.g., a funding rate tracker), both places need updating — and the duplicate already diverged once (the inline path skips _recv_task = None).

Consider extracting the state-clear logic into a private _clear_session_state() helper and calling it from both _stop() and the fatal-error path.

---

6. margin_reports accepts arbitrary date strings — no format guard

File: kalshi/perps/resources/margin.py:53-67

def margin_reports(self, *, start_date: str, end_date: str, ...) -> GetMarginReportsResponse:

The spec requires YYYY-MM-DD. A malformed string (e.g., "2025/01/01", epoch seconds as string) will produce a server 400 with no SDK-level error context. At minimum, validate end_date >= start_date here — the withdrawal amount already has a positive-value guard; this endpoint deserves the same defensive treatment. A regex or datetime.date.fromisoformat(start_date) construction would surface the error at the SDK boundary.

---

Minor / Observations

• KlearClient.from_env has no transport param while PerpsClient.from_env does. Minor inconsistency; blocks test injection without going through the constructor.

• MarginReport.url is noted as sensitive in the docstring ("treat it as sensitive, keep it out of logs") but there's no __repr__ override to enforce this. If someone logs a MarginReport object, the presigned URL will appear. Consistent with the credential-redaction care taken elsewhere, a redacting repr would be expected.

• LogInResponse has no __str__ alias (unlike LogInRequest which sets __str__ = __repr__). Pydantic's default model.__str__() calls __repr__ for Pydantic v2 models, but explicitly aliasing it (as LogInRequest does) would make the intent unambiguous.

• _recv_loop teardown leaves _recv_task non-None in the fatal-error path (unlike _stop() which sets it to None). If _stop() is called after a fatal teardown, asyncio.wait_for(self._recv_task, ...) in _stop() will be called on a done task, which is benign — but it's inconsistent.

---

Security ✅

The security surface was the most important thing to verify and it holds up well:

• Credential redaction in LogInRequest.__repr__, LogInResponse.__repr__, KlearAuth.__repr__ — all correct.
• Tests explicitly verify no credential or cookie leakage in logs.
• POST/DELETE never retried (duplicate order / cancel risk).
• POST /log_in not retried even on 429.
• Positive-amount boundary guard on withdraw_settlement_balance with a regression test.
• PerpsConfig blocks HTTPS-to-remote plaintext and KALSHI-ACCESS-* header injection.
• Split REST/WS environment guard (prod REST + demo WS) — correct.
• KlearAuth session state cleared on close().

---

Test Coverage ✅

~660 new tests at 3585 passing / 0 failed is solid. Contract-drift harness for perps and SCM is the right defensive layer. The test_no_credentials_in_logs test in test_auth.py is exactly the kind of test that should be here.

---

Summary: Four substantive findings (items 1–4 above) worth fixing before merge — particularly the MarginResource namespace misplacement (1) and the silent bad-side routing (3). Items 5–6 are lower priority. Security, conventions, and test coverage are all in good shape.

🤖 Generated with Claude Code

[claude]

Code Review — PR #403: Perps (margin) API full implementation

Overview

This is a large, well-structured additive PR. It delivers three independent client surfaces — PerpsClient/AsyncPerpsClient (REST), PerpsWebSocket (WS), and KlearClient/AsyncKlearClient (cookie-session SCM) — without modifying any existing prediction-API code. The security story (session cookie handling, credential redaction, withdrawal amount guard, MFA non-auto-retry) is solid and the contract-drift harness extension is exactly right.

---

What's done well

• Security model is tight: KlearAuth.repr() redacts the token, LogInRequest.repr() redacts email/password/code/MFA secrets, presigned MarginReport.url is redacted, _require_session() guard on every SCM endpoint, no RSA-PSS for Klear, session cookie properly reset before re-login.
• Withdrawal boundary guard (_require_positive_withdrawal + WithdrawalAmount) catches the single real-money write at construction rather than server-side.
• MFA handled correctly: single-call, no auto-retry, session not marked active until non-MFA-challenge success; re-login clears the prior session cookie before the attempt (preventing cross-account cookie replay).
• WS cooperative pause/resume (the _pause_pending/_pause_granted/_resume_ack triple-handshake) prevents the concurrent-recv ConcurrencyError that websockets raises on overlapping .recv() calls — good edge case coverage.
• Spec-sync workflow security (issue Security: spec-sync workflow is a supply-chain risk (writable + untrusted upstream) #92 / finding F-O-01): permissions: contents: read + issues: write only; no push, no PR, no code-write; third-party actions pinned to full SHA; dedup fingerprint prevents re-opening issues on re-run.
• Contract-drift harness (TestPerps*Drift) parametrically tested against vendored specs — the right way to catch query/path/body param drift.
• Test depth is excellent: credential-leak-in-logs tests, repr() redaction tests, no-retry POST tests, MFA non-auto-loop assertion.

---

Issues

Medium — Fix before merge

1. Stale module docstring in ws/dispatch.py (misleads future maintainers)

The docstring at the top of kalshi/perps/ws/dispatch.py says:

MESSAGE_MODELS is an empty stub here — the per-channel data payload models … land in the perps: WS channels issue (#398), which will register them in this map. This foundation issue only needs control-frame routing …

But the actual code (lines 55–63) fully populates MESSAGE_MODELS with all seven types. The docstring is from an earlier draft and should be updated to reflect what shipped.

2. Raw dict in _handle_orphan_subscribed bypasses the Pydantic model

kalshi/perps/ws/dispatch.py lines 210–215:

cmd = {
    "id": msg_id,
    "cmd": "unsubscribe",
    "params": {"sids": [sid]},
}
await self._sub_mgr._connection.send(cmd)

Every other command path (subscribe, unsubscribe, update, list) goes through the extra="forbid" Pydantic model (UnsubscribeCommand/UnsubscribeParams). This one path skips that, meaning a key typo here would silently drop on the wire. Per CLAUDE.md: "Don't build inline dict bodies in resource methods." The orphan-unsubscribe should use UnsubscribeCommand:

cmd = UnsubscribeCommand(
    id=msg_id, params=UnsubscribeParams(sids=[sid])
)
await self._sub_mgr._connection.send(
    cmd.model_dump(exclude_none=True, by_alias=True, mode="json")
)

UnsubscribeCommand and UnsubscribeParams are already imported in channels.py and available from kalshi.perps.ws.models.control.

3. Unretrieved task exception → asyncio "Task exception was never retrieved" warning

In kalshi/perps/ws/client.py _stop() (lines 189–195):

if self._recv_task and not self._recv_task.done():
    with contextlib.suppress(asyncio.CancelledError, Exception):
        await asyncio.wait_for(self._recv_task, timeout=2.0)
    if not self._recv_task.done():
        self._recv_task.cancel()
        with contextlib.suppress(asyncio.CancelledError, Exception):
            await self._recv_task

When _recv_loop exits with an unhandled exception (e.g., KalshiConnectionError from a permanent close code, or the raise in the catchall except Exception: handler), the task is already done by the time _stop() runs. The not self._recv_task.done() guard is False, so the entire block is skipped. Python's asyncio logs "Task exception was never retrieved" when the task is garbage collected, because no one ever called .result() or awaited the failed task.

The fix is to retrieve the exception for an already-done task:

if self._recv_task and not self._recv_task.done():
    with contextlib.suppress(asyncio.CancelledError, Exception):
        await asyncio.wait_for(self._recv_task, timeout=2.0)
    if not self._recv_task.done():
        self._recv_task.cancel()
        with contextlib.suppress(asyncio.CancelledError, Exception):
            await self._recv_task
elif self._recv_task is not None:
    # Task already done; retrieve stored exception to prevent asyncio warning.
    with contextlib.suppress(Exception):
        self._recv_task.exception()

The same pattern applies to the prediction-API KalshiWebSocket._stop() and should be fixed in tandem if it has the same structure.

---

Minor — Nice-to-fix

4. Two more stale #398 comments in ws/client.py

• Line ~74: "perps: WebSocket channels & payload models — orderbook_delta, ticker(+funding), trade, fill, user_orders, order_group_updates #398's concrete payload models plug into the dispatcher; this set lets the recv loop gate orderbook-apply + stale-sid checks without importing those models yet." — the models are imported and in use from line 47 onwards.
• Line ~338: "Parse into typed model (empty until perps: WebSocket channels & payload models — orderbook_delta, ticker(+funding), trade, fill, user_orders, order_group_updates #398 populates MESSAGE_MODELS)." — MESSAGE_MODELS is fully populated.

5. update_subscription / update_subscription_single_sid use bare list[str]

ws/client.py lines 750 and 781: market_tickers: list[str] | None. CLAUDE.md notes that the list builtin is shadowed by .list_subscriptions() on this very class. channels.py already uses builtins.list[str] for the same parameter; the client facade should match.

---

Questions / spec clarity

• MarginTickerPayload.last_trade_size (line 67–70 of ws/models/ticker.py) has no default. If the wire omits it (e.g., for a market that hasn't traded yet), validation will fail. Is it truly required in the perps AsyncAPI spec, or should it have | None = None?
• PerpsConfig._validate_perps_url validates against _PERPS_KNOWN_HOSTS (the union of REST + WS hosts) for both base_url and ws_base_url. This means external-api.kalshi.com (REST host) would also pass for ws_base_url before the split-env check catches it. Not a security issue (the split-env guard fires), but the field-specific sets could give a cleaner error message if ever needed.

---

Verdict

The PR is close to mergeable. The security model and test depth are production quality. The three issues above are all in the WS layer: (1) stale docstring, (2) raw dict bypassing the Pydantic guard for orphan-unsubscribe, and (3) unretrieved task exception. Items 1 and 2 are one-liners; item 3 requires a small elif branch. None of these block correctness for the REST or SCM surfaces — they're WS-only and all fixable without architectural changes.

🤖 Generated with Claude Code

[claude]

Code Review — PR #403: Perps (margin) API — REST + WebSocket + SCM/Klear

This is a large additive PR (~23k lines across 116 files). I focused on correctness, security, and maintainability. The implementation is overall well-engineered, but there are a few items worth addressing.

---

What's Done Well

Security posture is strong:

• LogInRequest.__repr__ redacts credentials; hide_input_in_errors=True keeps passwords out of Pydantic ValidationError text — a subtle but important detail.
• KlearAuth.__repr__ never leaks the bearer token.
• WithdrawSettlementBalanceRequest enforces a positive amount via AfterValidator(_require_positive_withdrawal) — good defense against sign-flip bugs on the only real-money write in this surface.
• Session cookie is cleared before re-login (self._transport.clear_cookie("session")), preventing stale account crossover when log_in is called a second time.
• POST/DELETE never retried (mirrors the prediction-API contract; cancel + withdraw can't be idempotently replayed).
• Plaintext HTTP rejected for non-loopback hosts in both PerpsConfig and KlearConfig.
• KALSHI-ACCESS-* rejected in extra_headers at config construction time.

Architecture:

• Fully additive — KalshiClient/AsyncKalshiClient behavior is untouched.
• PerpsConfig/KlearConfig subclass KalshiConfig (Liskov) so the unmodified SyncTransport/AsyncTransport accept them.
• Perps models live under kalshi.perps rather than flooding kalshi.__all__, avoiding twin-name collisions.
• WS is a clean fork — zero edits to the prediction-API WS; perps subclasses SequenceTracker/ConnectionManager rather than copying the state machine.
• _build_create_body shared builder keeps sync/async resources in lockstep.
• Contract drift harness extended for perps REST + AsyncAPI WS commands/responses.
• test_review_fixes.py shows prior review findings were addressed — good practice.

---

Issues

1. __post_init__ mirror pattern — maintenance debt (Medium)

Both PerpsConfig.__post_init__ and KlearConfig.__post_init__ deliberately skip super().__post_init__() and manually replicate the shared guards from KalshiConfig.__post_init__ (trailing-slash strip, scheme check, plaintext-to-remote guard, KALSHI-ACCESS-* rejection, extra_headers freeze, http2/h2 check). Each file carries:

# NB: deliberately does NOT call super().__post_init__() — …
# When KalshiConfig.__post_init__ gains a new guard, mirror it here too.

That comment is the only thing standing between a future KalshiConfig guard addition and a silent regression in the perps/klear configs. The fix is to refactor the shared guards into a protected static method on KalshiConfig:

# In KalshiConfig
@staticmethod
def _validate_common(config: KalshiConfig) -> None:
    # … shared guards (slash strip, scheme, plaintext-to-remote, headers, http2) …

Then PerpsConfig.__post_init__ and KlearConfig.__post_init__ call KalshiConfig._validate_common(self) instead of duplicating the logic. This doesn't change any externally observable behavior but eliminates the maintenance risk.

2. _LOCAL_HOSTS triplication (Low)

_LOCAL_HOSTS = frozenset({"localhost", "127.0.0.1", "::1"}) appears identically in:

• kalshi/config.py
• kalshi/perps/config.py
• kalshi/perps/klear/config.py

This should live once in kalshi/_constants.py and be imported by all three. Triplication means a future addition (e.g., "[::1]") has to land in three places.

3. id shadows the Python builtin (Low)

def settlement_balance_withdrawal(self, *, id: str, ...) -> ...:

id shadows Python's built-in id(). Consider withdrawal_id as the external parameter name — it's more descriptive and avoids the shadow. The wire parameter (query param id=…) can be mapped via _params(id=withdrawal_id) to preserve the spec's field name.

4. KlearConfig inherits ws_base_url (Cosmetic)

KlearClient has no WebSocket surface, but KlearConfig inherits ws_base_url: str = "wss://..." from KalshiConfig. It is never validated or used, which is harmless, but API consumers who inspect KlearConfig fields might be confused. At minimum, the docstring could explicitly note that ws_base_url is inherited but unused for Klear.

---

Minor Notes

• classify_ok distinguishes list_subscriptions vs update_ack by whether msg is an array. This is well-documented and tested. It is a spec assumption — if the spec moves to a proper discriminator field later, this needs updating. Acceptable as-is.
• No __all__ in kalshi/perps/__init__.py. Star-import behavior is undefined. Low priority since the recommended import style is explicit.
• The weekly spec-sync CI in .github/workflows/spec-sync.yml is a good addition; confirms vendored specs don't silently drift.

---

Test Coverage

3585 tests, ~660 new. The hermetic perps suite covers:

• Happy paths, error paths, edge cases per resource
• Contract drift (query + path + body params against vendored spec)
• WS parity (AsyncAPI command/response type/cmd Literals)
• Security regressions (positive amount, session guard, credential isolation)
• Integration tests auto-skipped without demo credentials

Coverage looks solid. No gaps identified.

---

Summary

Severity	Finding
Medium	__post_init__ guard duplication — future KalshiConfig guard additions won't propagate to PerpsConfig/KlearConfig
Low	_LOCAL_HOSTS defined in three files; belongs in _constants.py
Low	id parameter name shadows Python builtin in settlement_balance_withdrawal
Cosmetic	KlearConfig inherits unused ws_base_url without documentation callout

The medium-severity item (#1) is the one I'd most want resolved before merge since it's a silent security-regression risk. The rest are polish. Overall the PR is well-thought-out and the security-critical paths (credential handling, real-money writes, session isolation) are correctly implemented.

🤖 Generated with Claude Code