[TexasCoding]

Wave 2-C of audit umbrella. 7 P3 docs items + 2 P2 model items.

Changes

• Polish bundle: docs cleanup (env-vars NameError, stale AUDIT doc, kalshi_python_async ref, limits default, cancel idempotency docs, WS perf section, events.get default) #222 P3 docs cleanup: env-vars NameError fix (uses KalshiConfig.demo(timeout=...)); deleted stale docs/AUDIT-resource-params.md; dropped stale kalshi_python_async 'referenced in project README' claim; docs/configuration.md limits default cell corrected to None; new docs/websockets.md Performance section (queue sizing, orjson example, recv-loop threading); cancel/delete idempotency notes on orders / communications / api-keys / order-groups docs; events.get(with_nested_markets: bool | None = None) (was bool = False).
• Polish bundle: WS small items (Full Jitter reconnect, side Literal, populate_by_name, AwareDatetime, sentinel idempotence) #221 P2.3 populate_by_name=True added to every WS payload model_config (except orderbook_delta.py — W2-A's scope).
• Polish bundle: WS small items (Full Jitter reconnect, side Literal, populate_by_name, AwareDatetime, sentinel idempotence) #221 P2.4 datetime → pydantic.AwareDatetime on REST response-model timestamp fields across 12 modules; new datetime-semantics note in docs/concepts.md.

Closes #222
Refs #221 (W2-A lands items 1/2/5)
Refs #224

[claude]

Code Review — PR #234 (Wave 2-C: docs polish + AwareDatetime + populate_by_name)

Overall this is a clean, well-scoped PR. The three code changes (events default, populate_by_name, AwareDatetime) are all correct. A few things worth calling out before merge.

---

Issues

1. Missing async test coverage for events.get default change

Two new tests were added to TestEventsGet (sync) but neither has an async counterpart in TestAsyncEventsGet:

• test_get_with_default_does_not_send_with_nested_markets_param
• test_get_with_explicit_false_still_serializes

Both the sync and async get() methods were changed (resources/events.py lines 155 and 258). The existing TestAsyncEventsGet class covers the with_nested_markets=True case — missing the None/False variants means the async path isn't regression-covered.

2. AwareDatetime is a subtle semi-breaking change — missing CHANGELOG entry

Any caller who constructs a REST response model directly with a naive datetime (e.g., in tests, fixtures, or application code that builds an Order(created_time=datetime.now()...)) will now get a ValidationError at runtime. The API wire path is unaffected (Kalshi always returns RFC3339+Z), but direct construction is a real use case.

This isn't necessarily wrong to ship, but it should be in CHANGELOG.md under a "Breaking" or "Changed" note since it's a validation tightening across 12 public models.

3. WeeklySchedule.start_time / end_time — verify API wire format

exchange.py's WeeklySchedule has start_time: AwareDatetime and end_time: AwareDatetime. The field names suggest these might encode time-of-day (e.g., trading hours) rather than full timestamps. If the Kalshi API ever returns a bare time string like "09:30:00" or a date-only string for these fields, Pydantic will raise ValidationError. Worth a quick spot-check against the live /exchange/schedule response or the OpenAPI spec.

---

Minor nits

from pydantic import ValidationError inside test methods (tests/test_models.py lines 506, 537 in the new block)

This matches the existing pattern in the file (same import style at lines 813, 690), so it's internally consistent — just noting it's unusual. Not a blocker.

Test fixture parametrization coverage for AwareDatetime

TestAwareDatetimeRejectsNaive only parametrizes over Order, Market, and Settlement. The change touches 12 modules. The three models chosen do demonstrate the principle, but communications, exchange, and historical models (which have the most timestamp-heavy payloads) aren't exercised. A light pass over the other fixture builders could catch a broken fixture_builder call before they surface in CI.

---

What's good

• Docs: The env-vars dataclasses.replace fix is correct and the rewrite to KalshiConfig.demo(timeout=..., max_retries=5) is idiomatic. The idempotency callout blocks across four resource docs follow a consistent pattern and are accurate.
• events.get default: Changing bool = False to bool | None = None so the param is omitted by default is the right call — sending ?with_nested_markets=false on every call was noise. The two existing sync tests (None omits, False serializes) are well-targeted.
• populate_by_name=True: Correct fix for all WS payload models that use AliasChoices. Excluding orderbook_delta.py per W2-A scope is the right boundary.
• WS Performance section: Actionable and technically accurate. The queue-sizing rule, overflow strategy table, and recv-loop threading notes are all correct. The orjson.dumps → bytes note is accurate.
• Deleting AUDIT-resource-params.md: Right call to remove working-session artifacts from the published docs tree.

---

Summary

Two things to address before merge:

1. Add async variants of the two new events.get tests to TestAsyncEventsGet.
2. Add a CHANGELOG entry noting the AwareDatetime validation tightening as a breaking change for direct model construction (not for API parsing).

WeeklySchedule is worth a quick spec-check but is unlikely to be an issue in practice.

[claude]

Code Review — PR #234: Wave 2-C docs/polish + AwareDatetime + populate_by_name

Overview

Wave 2-C of the audit umbrella (#224). Three bundled categories:

1. P3 docs polish — NameError fix in env-vars example, stale audit doc deletion, limits default correction, WS performance guide, idempotency notes on four resource docs, concepts.md datetime semantics section.
2. P2.3 WS model config — populate_by_name=True added to all WS payload model_config dicts.
3. P2.4 datetime hardening — datetime → pydantic.AwareDatetime across all REST response model timestamp fields (12 modules).

Overall the PR is well-scoped and the intent is clear. A few issues below, two of which are worth addressing before merge.

---

Issues

🔴 Discrepancy: orderbook_delta.py is in the diff despite being explicitly excluded

The PR description states: populate_by_name=True added "except orderbook_delta.py — W2-A's scope." But the diff shows all four model_config dicts in kalshi/ws/models/orderbook_delta.py getting the flag. This is either:

• A no-op if W2-A already landed it (safe, but the PR description is wrong), or
• An unintentional addition if W2-A didn't (scope creep from W2-C's stated bounds).

Please verify and update the PR description accordingly.

🔴 Breaking change: AwareDatetime migration needs a CHANGELOG entry

The P2.4 section describes this as a model hardening item, but it is a breaking change for any user who constructs SDK models directly (e.g., in their own tests or application code). Before this PR:

Order(... created_time=datetime.now())   # worked

After:

Order(... created_time=datetime.now())   # ValidationError

This needs to appear in CHANGELOG.md with a migration note. The docs/concepts.md addition is good context but won't be seen during pip install --upgrade. Suggested placement: under a "Breaking changes" header in the current in-progress section, with the one-line fix (datetime.now(UTC) or datetime.now().replace(tzinfo=UTC)).

🟡 Behavior change: events.get() default also needs a CHANGELOG note

Changing with_nested_markets: bool = False → bool | None = None is behaviorally correct (let the server default apply rather than sending an explicit false), but it's observable: callers who previously got ?with_nested_markets=false on every request will now get no param. The tests correctly pin the new contract. A one-line entry in CHANGELOG.md under "Changes" is sufficient.

🟡 Missing async tests for the events.get() default change

The two new tests (test_get_with_default_does_not_send_with_nested_markets_param, test_get_with_explicit_false_still_serializes) only cover the sync EventsResource.get() path. AsyncEventsResource.get() has the same signature change and should have symmetric coverage.

🟡 WeeklySchedule.start_time / end_time as AwareDatetime — verify wire format

WeeklySchedule in exchange.py uses start_time and end_time, which are semantically time-of-day fields. AwareDatetime requires a full ISO-8601 datetime with timezone. If the Kalshi API returns a bare time string ("08:00:00Z") without a date component, Pydantic will reject it. Worth checking the live /exchange/schedule response before shipping — or at least adding a contract test fixture that reproduces what the server actually returns.

🟡 Test coverage gap for AwareDatetime — fixture builders in other test files

TestAwareDatetimeRejectsNaive validates the constraint against 3 models. More importantly: check whether any existing fixture builders in tests/_model_fixtures.py pass naive datetimes into the affected models. If they do, those tests will now silently pass a naive value and fail model construction. A quick grep -r "datetime(" tests/ for calls without tzinfo= or UTC would confirm whether the full suite is clean.

---

Positives

• NameError fix is clean: replacing the two-step KalshiConfig.demo() / dataclasses.replace(...) pattern with direct constructor kwargs removes the dataclasses import and makes the example copy-paste correct.
• WS Performance section is excellent — concrete message-rate table, the maxsize >= 2 × peak_burst_per_second rule of thumb, the DROP_OLDEST vs ERROR strategy rationale, and the on_error foot-gun warning are all immediately actionable. This will prevent real incidents.
• Idempotency notes on orders/communications/api-keys/order-groups are exactly the kind of "gotcha" documentation that prevents production support tickets.
• _SDK_NAME_PAYLOADS parametrize pattern for the WS populate_by_name tests is clean and easy to extend when new payload models are added.
• with_nested_markets test pair — testing both "default omits param" and "explicit False still serializes" pins the behavioral contract precisely.
• AwareDatetime tests — testing the positive case (RFC3339+Z parses and round-trips to UTC) alongside the negative case (naive raises ValidationError) is the right structure.

---

Summary

Two items to fix before merge: the orderbook_delta.py scope discrepancy and a CHANGELOG entry for the AwareDatetime breaking change. The async test gap and WeeklySchedule wire-format verification are the next priority. Everything else is polish.