fix(ws): orderbook overhaul — fresh snapshots + O(1) apply_delta

[TexasCoding]

Summary

Wave 3 — two paired orderbook fixes. Same files, two logical commits, single PR.

#85 — Stop mutating Pydantic models in place

OrderbookManager previously built its emitted Orderbook by mutating the Pydantic model in place. Consumers holding a reference saw leaked mutations on the next delta. Separated mutable internal state (new private _BookState dataclass) from the emitted model: each get() / apply_*() returns a fresh Orderbook snapshot. Consumers can hold references safely.

#87 — apply_delta from O(n) → O(1)

Internal level storage switched from list[OrderbookLevel] to dict[Decimal, Decimal]. Update/insert/delete is dict-time. The sorted-level-list contract on the emitted Orderbook is materialized lazily in to_orderbook() (the unavoidable O(n log n) work moves from per-delta to per-snapshot-emit).

Perf characterization

Before: O(n) lookup + O(n log n) on new-level insert. Reported ~1.91 µs mid-book / ~3.06 µs end-of-book at n=99.
After: O(1) on update; O(n log n) only on snapshot emit. Per-delta wire-format access is bounded as the book grows (verified n=10 vs n=500 via counting-dict probe).

Public API

No change. OrderbookManager.{apply_snapshot,apply_delta,get,remove,clear} signatures unchanged. Orderbook / OrderbookLevel models unchanged. Observable behavior change (intentional per #85): consecutive get() calls return distinct instances.

Wave 3 boundaries

No touches to kalshi/ws/dispatch.py or kalshi/ws/client.py / channels.py (sibling branches own those).

Closes #85
Closes #87

Test plan

• +2 regression tests in tests/ws/test_orderbook.py (14 → 16). Pin the snapshot-immutability invariant (OrderbookManager mutates Pydantic models in place; consumers see leaked mutations #85) + counter-based perf check (Orderbook apply_delta is O(n) per delta; use price-indexed dict #87).
• Full suite: 1622 passed, 48 skipped
• uv run ruff check . clean
• uv run mypy kalshi/ clean (strict)

[claude]

Code Review — PR #136

Overview

Two well-scoped fixes to OrderbookManager with a clean architectural separation: a new private _BookState dataclass handles mutable internal state, while the public Orderbook model becomes a pure snapshot. The O(1) delta fix and the immutability fix are logically orthogonal but share the same files — bundling them makes sense given the single-PR scope.

---

What Works Well

• Correct design: Separating mutable internal state (_BookState) from emitted snapshots (Orderbook) is the right pattern. Consumers can now safely hold references across updates.
• Edge-case preservation: The new apply_delta logic correctly preserves all edge cases from the old implementation:
  • Negative delta on a non-existent level: silently dropped (elif delta > 0)
  • Delta that drives qty ≤ 0: level removed (del levels[price])
  • New level from positive delta: inserted
• Sort order consistency: sorted(self.yes.items()) matches the old levels.sort(key=lambda lv: lv.price) — both ascending by price. The PR comment explicitly calls this out, which is good.
• Snapshot immutability test: test_returned_book_is_snapshot_not_live_view covers both identity invariant and value invariant for all three methods (apply_snapshot, apply_delta, get). Thorough.
• Counting-dict test: Avoids wall-clock timing in favor of a structural invariant — correct choice for CI stability.

---

Issues and Suggestions

1. Counting dict test misses items() — undocumented intentional gap

In test_apply_delta_is_constant_time_in_book_depth, CountingDict overrides get, __getitem__, __setitem__, __delitem__, and __contains__, but not items(). Since to_orderbook() calls sorted(self.yes.items()) — iterating all n entries — those O(n) accesses are invisible to the counter. The test is measuring delta-application accesses only, not the full apply_delta() call cost.

This is arguably correct (the test comment says "apply_delta must touch at most a constant number of levels per delta"), but it's subtle — a future reader could reasonably believe the test covers the full call. Worth a one-line comment:

# items() is not overridden: to_orderbook()'s O(n log n) sort is counted separately;
# this probe isolates only the delta-application dict accesses.

2. CountingDict.get return type loses specificity

def get(self, key: object, default: object = None) -> object:  # type: ignore[override]

The # type: ignore[override] is necessary because dict[Decimal, Decimal].get has a more specific signature, but the -> object return type means any downstream use of the return value will require a cast. For test-only code this is fine, but -> Decimal | None would be more accurate and eliminate the need for the ignore:

def get(self, key: object, default: Decimal | None = None) -> Decimal | None:  # type: ignore[override]

Minor — test-only code.

3. Private symbol imported in tests without comment

from kalshi.ws.orderbook import _BookState

Importing a private symbol is intentional here (the test is pinning an internal invariant), but it's the only place in the test suite where this pattern appears. A short comment would signal to future maintainers that this is deliberate, not accidental:

# Import private _BookState to inject a counting dict and verify O(1) access pattern.
from kalshi.ws.orderbook import _BookState

4. Zero-delta edge case — not a bug, but worth noting

If the wire sends delta=0 for an existing level, the current code updates the level in-place with the same quantity (levels[price] = new_qty). This is a no-op semantically and matches old behavior, but it's technically an unnecessary write to the dict. Not a correctness issue; just worth tracking if Kalshi ever sends zero-deltas in practice.

---

Correctness Verification

The core logic change in apply_delta:

# Before
existing_idx = -1
for i, level in enumerate(levels):   # O(n) scan
    if level.price == price:
        existing_idx = i
        break

# After
existing_qty = levels.get(price)      # O(1)

This is correct. The functional equivalence holds for all cases (update, delete, insert, ignored-negative).

The to_orderbook() sort is unavoidable (consumers expect a sorted list) and correctly deferred to snapshot emit rather than per-delta.

---

Test Coverage Assessment

Case	Covered?
Snapshot immutability (identity)	✅ new test
Snapshot immutability (value)	✅ new test
get() immutability	✅ new test
Delta O(1) access count	✅ new test
Existing tests (snapshot, delta, remove, etc.)	✅ unchanged

Full suite: 1622 passed, 48 skipped — no regressions. Good.

---

Summary

This is a clean, well-reasoned fix. The architectural split (_BookState vs Orderbook) is right-sized and doesn't over-engineer. The two regression tests are solid. The four points above are minor — items 1 and 3 are the most worth addressing before merge for long-term maintainability. Items 2 and 4 are optional polish.

Verdict: Approve with minor suggestions. The correctness and performance improvements are sound; the test suite adequately pins the new invariants.