fix(ws): seq_tracker wiring (#27) + close-out F-O-05 + single parse path (#28)

[TexasCoding]

Summary

Three follow-ups left over from Wave 3 / 4. Two commits, one PR.

Commit 1 — wire SequenceTracker into MessageDispatcher (closes task #27)

PR #137 (dispatcher correctness) added an optional seq_tracker=None kwarg on MessageDispatcher.__init__ so server-initiated unsubscribe (#81) can clear stale seq state alongside the sid mapping. The wiring at the construction site in KalshiWebSocket was never landed — the dispatcher always saw seq_tracker=None and could only clear half the state.

Also includes the recv-loop side of #86 (next item): _process_frame now passes pre_validated to dispatch for orderbook channels.

Commit 2 — close #84 F-O-05 + #86 (single parse path)

F-O-05 (closes #84 fully): dispatcher's validation-failure log used exc_info=True, which surfaces Pydantic's ValidationError.__str__ — that echoes the full input including trade prices, counts, and user identifiers straight into log sinks. Dropped exc_info; surface type + exception class only. The underlying exception stays on __cause__ for local debug.

#86 (single parse path):

• dispatch(raw: str) → dispatch(data: dict, *, pre_validated: BaseModel | None = None). Recv loop owns the json.loads (it already had one); the dispatcher's redundant second parse is gone.
• pre_validated lets the recv loop hand off an orderbook message it already validated for the local OrderbookManager, so the dispatcher routes the same instance to the subscription queue without re-running Pydantic. Eliminates the double-validate the issue called out (~3 µs/msg).
• Dropped now-unused import json from dispatch.py.

Test fallout

• 21 tests in test_dispatch.py bulk-updated from dispatcher.dispatch(raw) → dispatcher.dispatch(json.loads(raw)).
• Dropped test_dispatch_invalid_json — that path now lives in the recv loop's exception ladder (already covered by test_malformed_frame_logged_with_traceback_and_continues).
• 2 tests in test_recv_loop_hardening.py monkey-patched dispatch with the old (raw: str) signature; updated.
• +1 regression: test_dispatch_pre_validated_skips_revalidation asserts the dispatcher routes the same instance when pre_validated matches the expected model_cls (proves the double-validate is gone).

Closes #27
Closes #84
Closes #86

Test plan

• Suite: 1705 passed, 48 skipped
• uv run ruff check . clean
• uv run mypy kalshi/ clean (strict)

[claude]

Code Review

Overall: Clean, focused PR. Three well-scoped fixes that each do exactly one thing. Conventions followed throughout. One factual inaccuracy in a comment worth fixing before merge.

---

kalshi/ws/dispatch.py

Security fix (F-O-05) — Removing exc_info=True is the right call. ValidationError.__str__ on a failed Pydantic parse can include the full input envelope, and that payload can carry prices, counts, and user-identifiable fields. Logging only type(exc).__name__ is the right trade-off between debuggability and data hygiene. ✅

One inaccuracy in the inline comment:

except Exception as exc:
    # ...the underlying exception is preserved on __cause__ for debug.
    logger.warning("Failed to parse %s message: %s", msg_type, type(exc).__name__)
    return

exc is caught, its type name is logged, and then return discards it. Nothing is chained or preserved — there is no __cause__ here. To set __cause__ you'd need raise SomeWrapper(...) from exc, which isn't happening. A future developer searching for where to find the original exception will be misled.

Suggested fix — either drop the claim or make it true:

# Option A: just drop the false claim
# Drop exc_info: Pydantic's ValidationError.__str__ echoes full input
# (price, count, user fields) into log sinks. Surface type only.
logger.warning("Failed to parse %s message: %s", msg_type, type(exc).__name__)
return

# Option B: actually preserve it (adds a tiny wrapper exception, useful for debug builds)
raise RuntimeError(f"Failed to parse {msg_type} message") from exc
# (would need the outer try/except restructured; probably not worth it)

Option A is the minimal, correct fix.

dispatch signature change — data: dict[str, Any], *, pre_validated: BaseModel | None = None is clean. Using * to make pre_validated keyword-only prevents accidental positional misuse. The isinstance(pre_validated, model_cls) guard before using the pre-validated value is the right defensive approach — if the wrong type arrives it silently falls back to model_validate. ✅

Removed import json — correct cleanup since the dispatcher no longer owns JSON parsing. ✅

---

kalshi/ws/client.py

seq_tracker wiring — straightforward one-liner that completes the work from PR #137. Without this, server-sent unsubscribes were clearing only half the state (sid mapping but not seq watermarks). ✅

pre_validated threading — the pattern of validating once for the local OrderbookManager and handing the same instance to the dispatcher is clean. The path where self._orderbook_mgr is None leaves pre_validated = None, which causes the dispatcher to validate normally — correct fallback behavior. ✅

---

Tests

test_dispatch_pre_validated_skips_revalidation — the identity check (delivered is pre_validated) is the right way to prove the double-validate is gone; a value equality check wouldn't catch it. ✅

Dropped test_dispatch_invalid_json — appropriate. That path now lives in the recv loop, and test_malformed_frame_logged_with_traceback_and_continues covers it there. ✅

Bulk dispatcher.dispatch(json.loads(raw)) updates — mechanical but necessary; the mechanical nature is fine. ✅

Minor: test_dispatch_pre_validated_skips_revalidation subscribes to "orderbook_delta" but dispatches "orderbook_snapshot". This follows the same pattern as the existing test_dispatch_orderbook_snapshot, so it's clearly the established convention for this fake manager — not a new issue.

---

Summary

seq_tracker wiring	✅ Correct
F-O-05 exc_info removal	✅ Correct, one misleading comment to fix
Single parse path	✅ Clean
Test coverage	✅ Regression test + bulk updates
mypy / ruff	✅ Per PR description

One change requested: fix or remove the __cause__ claim in the inline comment in dispatch.py. Everything else looks good.

[TexasCoding]

Addressed:

• ✅ False __cause__ claim in inline comment — dropped. You're right; the except returns without re-raising, so no chain is set. Kept the security rationale (Pydantic input echo).

Skipped (justified):

• The orderbook_delta subscription / orderbook_snapshot dispatch mismatch in the regression test is the established FakeSubManager convention (mirrors test_dispatch_orderbook_snapshot). Not a new issue per bot.

[claude]

Code Review — PR #142

tl;dr: Three well-scoped fixes with good test coverage. Two CLAUDE.md style violations to address; no correctness issues.

---

Overview

This PR lands three distinct follow-ups cleanly:

1. feat: order amendments, decrease, and queue positions (v0.5.0) #27 — completes the SequenceTracker → MessageDispatcher wiring that PR fix(ws): dispatcher correctness — fan-out, server-unsub cleanup, error envelope surface #137 set up but never connected at the construction site
2. Log info leakage: dispatch exc_info=True dumps trade data; KalshiError str() includes URLs #84 (F-O-05) — plugs the exc_info=True information-leak in validation-failure logging
3. WS recv loop & dispatcher parse JSON twice + validate orderbook messages twice #86 — eliminates the double JSON parse / double Pydantic validate on orderbook frames by moving parse ownership to the recv loop and threading pre_validated through

---

Code Quality & Style

CLAUDE.md violations — two instances:

1. Multi-paragraph docstring on dispatch() (dispatch.py)

"Never write multi-paragraph docstrings or multi-line comment blocks — one short line max."

The new docstring is four lines across two paragraphs. The contract is better communicated by the type signature (data: dict[str, Any], pre_validated: BaseModel | None = None) plus a single-line summary. Suggest collapsing to:

async def dispatch(self, data: dict[str, Any], *, pre_validated: BaseModel | None = None) -> None:
    """Route a pre-parsed frame to its subscriber."""

The pre_validated semantics belong in the PR description, not the docstring, since the caller (recv loop) is internal and the contract is enforced by isinstance.

2. Multi-line comment block in _process_frame (client.py)

The 3-line comment added over the orderbook validation block (# Check for orderbook messages — validate ONCE for the local...) is a multi-line comment block. The old single-line comment (# Check for orderbook messages) was sufficient; the performance rationale lives in the PR description where it belongs.

---

Correctness

seq_tracker wiring (commit 1): Straightforward one-liner; the kwarg was already accepted by MessageDispatcher.__init__ and stored as self._seq_tracker. No issues.

isinstance guard in dispatch: The if pre_validated is not None and isinstance(pre_validated, model_cls) check is the right defense. If the recv loop somehow passes a mismatched type (internal bug), the code silently falls back to re-validation rather than routing the wrong type to the queue. Correct behavior; silent fallback is fine for an internal interface.

exc_info removal (F-O-05): Correct fix. exc_info=True on logger.warning embeds the full ValidationError.__str__, which Pydantic formats with field values — exactly what the issue flagged. Surfacing only type(exc).__name__ alongside msg_type retains enough signal for ops (distinguish ValidationError from TypeError, etc.) without echoing the payload. The underlying exception is still available via __cause__ for local debugging.

---

Security

• F-O-05 fix is the right call. Trade prices, counts, and user identifiers should never flow into log sinks unconditionally. If deeper debug is needed, structured logging with an explicit allowlist is the correct mechanism, not exc_info=True.

---

Performance

• Eliminating the redundant json.loads + model_validate on every orderbook frame is a legitimate win for high-frequency channels. The pre_validated pass-through is clean. The ~3µs/msg savings cited in the summary are directionally correct for Pydantic v2.

---

Test Coverage

Regression test (test_dispatch_pre_validated_skips_revalidation): Well-designed — uses assert delivered is pre_validated (identity check, not equality) to prove no re-validation occurred. The sub = mgr.add(2, "orderbook_delta") + "type": "orderbook_snapshot" mismatch in channel names is consistent with the pre-existing test_dispatch_orderbook_snapshot setup (FakeSubManager routes by sid, not channel name), so no bug there.

Dropped test (test_dispatch_invalid_json): Justified — JSON decode errors are now the recv loop's responsibility and are already covered by test_malformed_frame_logged_with_traceback_and_continues. The PR description calls this out explicitly.

One minor nit: The new regression test imports OrderbookSnapshotMessage via a local import inside the test method:

from kalshi.ws.models.orderbook_delta import OrderbookSnapshotMessage

OrderbookSnapshotMessage is already imported at the top of dispatch.py from the same path, so the path is correct. But the local import inside the test method is unusual — move it to the top-level imports for consistency with the rest of the file.

---

Summary

Area	Status
Correctness	✅ No issues
Security (F-O-05)	✅ Fixed correctly
Performance	✅ Improvement is sound
Test coverage	✅ Regression added, drop justified
CLAUDE.md style	⚠️ 2 violations (multi-line docstring + comment block)

Two style fixes needed before merge; everything else looks good.