[TexasCoding]

Closes #263

Part of #273 (post-v2.4 audit), Wave 2.

[claude]

Code Review — PR #282: collapse snapshot apply into a single dict walk

Overview

This PR eliminates a redundant O(n) per-level Python walk when applying WebSocket orderbook snapshots. Previously, Pydantic validated wire data into list[tuple[DollarDecimal, FixedPointCount]] and _apply_snapshot_inplace then called dict(...) to rebuild a price→count map — paying Decimal construction + tuple unpacking twice. The new BeforeValidator(_levels_to_dict) field type coerces directly into dict[Decimal, Decimal] during parse, and the manager's copy is a pure pointer-shallow-copy. The formatting-only changes across bench_orderbook_delta.py, orderbook.py, and tests look like a Ruff/Black pass and are fine.

---

Bug: misleading comment overstates the optimization

kalshi/ws/models/orderbook_delta.py lines 24–25 and OrderbookSnapshotPayload docstring:

"b) _apply_snapshot_inplace adopts the dict directly with no rebuild."
"…can adopt the map with no second iteration (#263)."

kalshi/ws/orderbook.py lines 128–129:

yes_levels = dict(msg.msg.yes)
no_levels = dict(msg.msg.no)

dict(existing_dict) is still an O(n) walk — it copies every key–value pair. The dict is not adopted directly; it is shallow-copied. The actual win here is real: a pointer-copy of already-coerced Decimals is much cheaper than the old tuple-unpack + Decimal construction. But the claim "no second iteration" is false, and _apply_snapshot_inplace was not updated to match the comment.

Two ways to resolve this:

Option A (fix the comment): Update the docstring to say "a shallow copy (pointer-only, no Decimal re-construction)" instead of "adopts the map directly with no rebuild."

Option B (complete the optimization): Remove the dict() copies in _apply_snapshot_inplace and adopt the parsed dict directly:

# _apply_snapshot_inplace lines 128-129
yes_levels = msg.msg.yes
no_levels = msg.msg.no

The recv-loop discards the snapshot message after _apply_snapshot_inplace, so aliasing is safe in that path. If you want to keep defensive behavior for direct callers of apply_snapshot, a note in the docstring is sufficient. Option B gives the full "no second walk" that the PR claims.

---

Style: multi-paragraph docstrings violate CLAUDE.md

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

The _levels_to_dict docstring is 14 lines and the comment block above PriceCountMap is 4 lines. The performance rationale is non-obvious enough to warrant some comment, but these should be collapsed. One line on the WHY, not a paragraph per sub-point.

---

Missing test coverage for _levels_to_dict

The updated tests in test_models.py verify the dict output shape, but _levels_to_dict itself has three code paths with no dedicated coverage:

Path	Covered?
None → {}	No
dict with all-Decimal keys+values (fast-path, same object returned)	No
dict with non-Decimal keys+values (slow-path, re-coerced)	No
iterable of 2-element pairs (wire shape, happy path)	Yes (via existing snapshot tests)

The fast-path is subtle: _levels_to_dict returns the same dict object when all keys/values are already Decimal. If _apply_snapshot_inplace ever adopts directly (Option B above), this identity matters — a test should verify it.

---

Minor: tighten the return type

def _levels_to_dict(value: Any) -> Any:

Can be -> dict[Decimal, Decimal] — BeforeValidator accepts any callable; the return type is not constrained to Any. Makes mypy more useful for callers.

---

Summary

Real optimization?	Yes — Decimal construction moves from two passes to one
Comment accuracy	Off — dict() copy is still O(n), not "adopted directly"
Style compliance	Minor violations (multi-paragraph docstrings)
Test gaps	_levels_to_dict fast-path, None input, dict-with-strings input
Correctness	No behavioral bugs; the shallow copy is safe

The core change is sound. The main ask before merging: fix the misleading "adopts directly / no second iteration" claim (either update the comment or update the code to match it), and add the three missing _levels_to_dict unit tests.