ws: data frames for active subs are dropped during normal subscribe/unsubscribe/update command windows

[TexasCoding]

Context

Surfaced by the multi-LLM review of PR #403 (review item "F2"). Both WebSocket layers (prediction kalshi/ws and perps kalshi/perps/ws) cooperatively park the recv loop while a control command is in flight, and _wait_for_response only stashes the non-matching frames it reads during that window when _stashing is True — which is set only by reconnect/gap-recovery paths, not by ordinary subscribe/unsubscribe/update_subscription.

Problem

When a control command runs, _do_subscribe (and the perps equivalent) pause the recv loop so _wait_for_response can own connection.recv():

• kalshi/ws/client.py:727-754 — async with self._subscribe_lock: await self._pause_recv_loop(); sub = await self._sub_mgr.subscribe(...).
• The perps client uses the identical mechanism: _subscribe_lock + cooperative recv-loop pause (kalshi/perps/ws/client.py:112 and surrounding pause/resume logic).

While the loop is parked, every frame the server sends is read by _wait_for_response. Any frame whose id does not match the command id is treated as "non-matching" and, because _stashing is False on the normal command path, falls into the discard branch:

• Prediction: kalshi/ws/channels.py:218-228 — matches id, else if self._stashing: _maybe_stash(...) else: logger.debug("Discarding non-matching frame during subscribe: type=%s", ...).
• Perps: kalshi/perps/ws/channels.py:168-187 — same structure, else: logger.debug("Discarding non-matching frame during command: type=%s", ...).

_stashing is set True only inside resubscribe_all (kalshi/ws/channels.py:418, kalshi/perps/ws/channels.py:430) and resubscribe_one (kalshi/ws/channels.py:502). So a delta/trade/ticker frame for an already-active subscription that happens to arrive interleaved with a subscribe/unsubscribe/update ack is silently dropped — it is never routed to the dispatcher or applied to the local orderbook.

Concrete failure: a user holds an active orderbook_delta subscription, then issues a second subscribe (or an update_subscription) on the same connection. During the brief command window, an orderbook delta for the existing sub lands and is discarded. The local orderbook is now missing that delta → a sequence gap / silent orderbook desync on the existing subscription. The window is small (one command round-trip, default 5s timeout) and only matters on a shared connection with active high-volume channels, which is why severity is low — but the corruption is silent (debug-level log only).

This is the documented, intentional default: tests/ws/test_channels.py:459 (test_stashing_flag_off_by_default) asserts _stashing is False with the rationale "Default is off so normal subscribe paths don't accumulate stale state." So this is a design tradeoff shared by both layers, not a perps regression, and was not changed by the recent fix commits (f8cc960/da953b0/f2ef973).

Proposed fix

This is a decision, not a clear bug. Two options:

1. Document and accept (cheapest). Keep current behavior; add a short doc note on the command-window drop so consumers know a desync is possible on a busy shared connection and that gap-recovery (resubscribe_one) will heal it. No code change.
2. Always-stash + drain on recv-loop resume (safe fix, applied to BOTH layers). Enable stashing for the duration of every command (not just resubscribe), and have the client drain take_stash() back through the dispatcher when it resumes the recv loop — exactly the mechanism already used after resubscribe_all. This closes the window without inventing new machinery. To avoid the "stale state" concern the test cites, the stash must be drained on every resume (success or failure), not left to accumulate.

Whichever is chosen, apply it symmetrically to kalshi/ws/channels.py + kalshi/ws/client.py and kalshi/perps/ws/channels.py + kalshi/perps/ws/client.py so the two WS implementations do not diverge. Do not make a perps-only change.

Acceptance criteria

• Decision recorded on this issue: option 1 (document) or option 2 (always-stash + drain).
• If option 2: _wait_for_response stashes non-matching frames carrying an int sid during normal subscribe/unsubscribe/update windows, in both kalshi/ws/channels.py and kalshi/perps/ws/channels.py.
• If option 2: the client drains take_stash() through the dispatcher when resuming the recv loop after a normal command (kalshi/ws/client.py _do_subscribe and the perps equivalent), with the stash drained on both the success and failure paths.
• Regression test (both layers): with an active subscription, feed a data frame for its sid interleaved before the command ack during a normal subscribe/unsubscribe/update; assert the frame reaches the dispatcher / orderbook (option 2) — and update/replace test_stashing_flag_off_by_default so the new invariant is the documented one.
• If option 1: a docstring/doc note on _wait_for_response (both layers) describing the command-window drop and that gap recovery heals it; no behavior change.

Notes

• Deferred from PR perps: full Perps (margin) API — REST + WebSocket + SCM/Klear (#387) #403 because it is a documented design tradeoff shared by both WS layers, not a perps-surface defect, and the PR's headline findings were already fixed in f8cc960/da953b0/f2ef973.
• Tradeoff: option 2 always pays the stash/drain bookkeeping on every command for a window that is usually empty, and reintroduces the "accumulate stale state" risk the current default was chosen to avoid — mitigated by draining on every resume. Option 1 leaves a rare silent desync that the existing resubscribe_one gap-recovery path will eventually correct once a bad sequence number is observed.
• Related: perps WS EPIC perps: [EPIC] Perps (margin) API — full SDK implementation tracker #387. The _stashing machinery and take_stash() drain already exist for the reconnect path (WS: data frames arriving during reconnect+resubscribe are dropped instead of stashed #176/WS: resubscribe_one stash is never drained — frames captured during gap recovery are dropped #254), so option 2 reuses proven code rather than adding a new mechanism.
• Needs verification on the exact interleaving probability in production; the window only matters on a single connection multiplexing an active high-volume channel with a concurrent command.

[TexasCoding]

Resolution: keep as by-design (with the rationale documented below).

After review, discarding a non-matching data frame during a normal command window is an intentional, principled tradeoff rather than a defect:

1. It is gap-detected, not silent. When a delta is dropped during a command, the next delta for that sid arrives with a sequence jump, which the PerpsSequenceTracker (and the prediction SequenceTracker) flags via the gap path. So the local book's brief inconsistency is surfaced with a recovery signal, not silently corrupted — materially less severe than "silent desync."

2. The stash/discard asymmetry is principled. Frames are stashed only during resubscribe_all (rebuilding a book from scratch after a reconnect — where a lost frame would corrupt the fresh book with no prior baseline to gap-detect against), and discarded during normal commands (book already established + gap-backstopped). The prediction WS documents this explicitly: "Default is off so normal subscribe paths don't accumulate stale state."

3. The behavior is shared with the shipped prediction WS, and the only "fix" (always-stash + drain on recv-loop resume) is the highest-risk change in the review set — core recv-loop concurrency in a released layer — for a window that is already gap-detected. The risk/benefit does not favor it.

If a future use case issues many (un)subscribes while order books stream live and the gap-recovery cost becomes material, we can revisit the stash-on-resume design across both WS layers together (to avoid divergence). For now, closing as by-design.

A clarifying code comment will be added at the discard site in kalshi/perps/ws/channels.py (_wait_for_response) alongside the WS-bug cluster (#411) so the intent is visible in-code, not just here.