[TexasCoding]

Closes #270

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

[claude]

Code Review — PR #292

Wave 3 of the post-v2.4 audit. Solid batch of correctness improvements. Three minor issues worth addressing before merge, plus one lower-priority note.

---

Overview

Five independent items land together:

1. WS payload datetime → AwareDatetime (matches REST models standardised in docs+polish: env-vars NameError, stale docs, limits default, WS perf, cancel idempotency, events default; populate_by_name on WS; AwareDatetime on REST #234)
2. CreateOrderRequest enum fields narrowed from bare str to Literals
3. UnixSecondsTimestamp documentation alias for Unix-seconds int fields
4. _coerce_decimal NaN/Infinity guard — prevents shipping "NaN" to money endpoints
5. Contract-test type classifier updated to recognise AwareDatetime/NaiveDatetime

All items have targeted tests. The changes are surgical and match existing conventions.

---

Issues

1. to_decimal() public helper bypasses the NaN guard — medium severity

_coerce_decimal now rejects non-finite Decimals, but to_decimal() in types.py short-circuits on isinstance(value, Decimal): return value (line 105-106) and never calls _coerce_decimal. A caller who does:

from kalshi.types import to_decimal
from decimal import Decimal
to_decimal(Decimal("NaN"))  # returns Decimal("NaN") silently

…can then pass the result as yes_price= to CreateOrderRequest — but only if they use to_decimal() directly instead of constructing the model with a raw Decimal("NaN"). The Pydantic BeforeValidator on DollarDecimal will still catch it at model construction time, so the protection is not entirely absent, but the public helper is inconsistent with the intent of this PR. Consider adding the same guard to to_decimal():

result = Decimal(str(value)) if not isinstance(value, Decimal) else value
if not result.is_finite():
    raise ValueError("Decimal must be finite (got NaN/Infinity)")
return result

2. WS tests only cover rejection; no happy-path acceptance test — low severity

TestWsPayloadsRejectNaiveDatetime verifies three rejection cases, but there is no test confirming that a valid timezone-aware string ("2026-01-01T00:00:00Z") actually parses into an AwareDatetime. This is useful as a regression guard — if someone accidentally swaps AwareDatetime back to NaiveDatetime, the rejection tests still pass (NaiveDatetime rejects +00:00 offsets too), but a parse test would fail. One test per class is enough:

def test_user_orders_payload_accepts_aware_created_time(self) -> None:
    payload = user_orders_payload_dict(created_time="2026-01-01T00:00:00Z")
    m = UserOrdersPayload.model_validate(payload)
    assert m.created_time.tzinfo is not None

3. ForwardRef string matching in test_contracts.py is fragile — low severity

The new set literal at line 604-608 matches __forward_arg__ by string value:

if isinstance(base, typing.ForwardRef) and base.__forward_arg__ in (
    "datetime", "datetime | None",
    "AwareDatetime", "AwareDatetime | None",
    "NaiveDatetime", "NaiveDatetime | None",
):

If Pydantic changes how it stringifies complex forward refs (e.g., Optional[AwareDatetime] vs AwareDatetime | None depending on Python version), this silently stops matching and fields would be classified as "unknown", causing drift false-positives. A short comment explaining why these exact strings appear (Pydantic's lazy annotation resolution before first model_validate) would make this more maintainable, and a fallback that logs a warning on "unknown" classification might catch future drift.

---

Minor notes (no action required)

• UnixSecondsTimestamp: The alias is Annotated[int, "string metadata"] — there is no runtime validation added (the string is ignored by Pydantic). This is intentional per the docstring ("same wire shape as plain int") and is consistent with how NullableList/DollarDecimal document intent via Annotated. Fine as-is.
• Verbose class docstrings on Trade and MarketPosition: The added docstrings are on the long side for inline class docs (explaining the required-but-nullable design decision). The content is non-obvious and worth keeping somewhere, but a commit message or inline field comment might be a lighter touch. Not blocking.
• Bool check reordering in _coerce_decimal: Moving isinstance(value, bool) before isinstance(value, Decimal) is harmless (bool is never a Decimal instance), but the new order is logically cleaner. Good cleanup.

---

Summary

Items 1 and 2 are worth addressing before merge. Item 3 is a maintenance note. Everything else looks correct and well-tested.