Skip to content

docs: add v2.1→v2.3 migration sections; fix list_all cap claim; align WS channel docs#227

Merged
TexasCoding merged 1 commit into
mainfrom
audit/w0c-docs-sweep
May 21, 2026
Merged

docs: add v2.1→v2.3 migration sections; fix list_all cap claim; align WS channel docs#227
TexasCoding merged 1 commit into
mainfrom
audit/w0c-docs-sweep

Conversation

@TexasCoding

Copy link
Copy Markdown
Owner

Wave 0-C of the 2026-05-21 SDK audit umbrella. Docs-only sweep — no code changes.

Changes

migration.md

README + docs/websockets.md

  • Aligned on '11 typed subscribe_* channels + 2 escape-hatch' (control_frames, root — confirmed present in asyncapi.yaml).
  • README channel list uses SDK method-name suffixes (subscribe_market_lifecycle, subscribe_multivariate_lifecycle, subscribe_order_group) instead of wire-channel names.
  • docs/websockets.md table verified already correct — no edits needed.

Verified zero repo-wide matches for the typo'd method names (subscribe_market_lifecycle_v2, etc.) and no remaining 1000-page safety cap claim in migration.md.

Closes #200, Closes #218
Refs #224

… WS channel docs

- migration.md: new v2.1→v2.2 and v2.2→v2.3 sections; correct list_all() page-cap contradiction (unbounded by default since v2.0).

- README + docs/websockets.md: agree on 11 typed subscribe_* methods (+ generic escape hatch where applicable). Use SDK method suffixes (subscribe_market_lifecycle, subscribe_multivariate_lifecycle, subscribe_order_group) instead of wire-channel names.

Closes #200, Closes #218

Refs #224
@claude

claude Bot commented May 21, 2026

Copy link
Copy Markdown

Code Review — PR #227: docs: add v2.1→v2.3 migration sections; fix list_all cap claim; align WS channel docs

Docs-only PR. No runtime risk. Factual accuracy verified against the live codebase.


Overview

This PR adds two new migration guide sections to docs/migration.md (v2.1→v2.2 and v2.2→v2.3), corrects a false pagination claim, and aligns the README WebSocket channel list with actual SDK method names. All changes are documentation only — no code is modified.


Factual Accuracy (verified against source)

Claim Status
All 11 subscribe_* method names in README ✅ Match ws/client.py exactly
#the-11-channels anchor in docs/websockets.md ✅ Exists at line 32
pagination.md link target ✅ File exists
run_forever() raises KalshiSubscriptionError on no subscription ws/client.py:616
stop_event cooperative shutdown ws/client.py:580
Frame stash / _drain_resubscribe_stash ws/client.py:342–376
ThreadPoolExecutor in KalshiAuth auth.py:231
max_pages=0 raises ValueError resources/_base.py:55–57
Old "1000-page safety cap" was wrong ✅ No such cap in any _list_all path

Everything checks out.


Issues

1. Minor — KalshiAuth.close() obligation not flagged as a second soft-break

The v2.2→v2.3 intro says:

"soft-breaking on the WebSocket surface in one place: run_forever()"

But the ThreadPoolExecutor addition is also a soft-break for standalone KalshiAuth users: the executor leaks until the process exits if close() is never called. The section describes the fix correctly later on, but the framing at the top undercounts the breaking surfaces. Consider: "soft-breaking in two places: run_forever() and KalshiAuth.close() for standalone users".

2. Minor — max_pages wording is narrower than the validation

Pass max_pages=N for an explicit cap; passing max_pages=0 raises ValueError.

_validate_max_pages rejects any max_pages <= 0, not only 0. Calling with -1 raises the same error. Suggest: "passing max_pages=0 or any non-positive value raises ValueError".

3. Nit — issue cross-references are assertion-only

The v2.2→v2.3 section uses inline issue refs (#175, #185, #176, etc.) without prose context. The refs are helpful, but a reader who isn't tracking the issue list gets no signal about what they contain. Low priority, but worth a one-line parenthetical per feature if this doc is intended as the primary onboarding path.


What's working well

  • The list_all() fix is the right call. The old "1000-page safety cap" wording was actively misleading — no such cap exists in the code. The new wording correctly describes the unbounded-by-default / max_pages=N opt-in contract and links to pagination.md as the canonical reference.
  • README channel names. Switching from wire-channel names (market_lifecycle_v2, order_group_updates) to SDK method names (subscribe_market_lifecycle, subscribe_order_group) is strictly more useful for copy-paste. All 11 method names were verified against ws/client.py.
  • Recovery pattern in v2.1→v2.2. The try/except ValidationError example is appropriately scoped — it shows wrapping the call site, not the entire program.

Verdict

LGTM with the minor items above. None of the issues are blockers. The factual accuracy is solid and the corrections (pagination cap, channel names) are overdue. Issue 1 (soft-break count) is the only one worth addressing before merge if you want the migration guide to be self-contained for someone upgrading cold.

@TexasCoding
TexasCoding merged commit 47017e4 into main May 21, 2026
5 checks passed
@TexasCoding
TexasCoding deleted the audit/w0c-docs-sweep branch May 21, 2026 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant