[TexasCoding]

Wave 1-D of the audit umbrella. One issue, mechanical retype sweep across WS payload models.

Changes

• WS: count/size/timestamp payload fields typed as raw str (should be Decimal/datetime) #198 WS payload count/size/volume fields now FixedPointCount instead of str (matches REST models; eliminates silent str+int TypeErrors in consumer code).
  • TickerPayload / TradePayload / FillPayload / UserOrdersPayload / MarketPositionsPayload count/size/volume fields.
  • OrderbookSnapshotPayload.yes/.no → list[tuple[DollarDecimal, FixedPointCount]] (matches OrderbookDeltaPayload).
  • RfqCreatedPayload.contracts + QuoteAcceptedPayload.contracts_accepted → FixedPointCount (matches RfqDeletedPayload.contracts).
  • UserOrdersPayload + Communications RFC3339 timestamps → datetime.
• Simplified OrderbookManager.apply_snapshot — dropped redundant Decimal() wrappers now that payloads return Decimal.

Tests

16 new tests including a parametrized REST↔WS field type symmetry test; 85 targeted tests pass; ruff + mypy --strict clean.

Coordination note

This branch must merge before W1-C (audit/w1c-ws-correctness) because W1-C will rebase onto it (overlap on orderbook.py agreed via IRC during execution).

Closes #198
Refs #224

[claude]

Code Review

Overall: Clean, focused mechanical retype sweep. The changes are well-scoped, internally consistent, and the simplification in orderbook.py is a direct and correct payoff of the type upgrade. Minor concerns noted below.

---

What this PR does well

• Consistency: All _fp-aliased count/size/volume fields across ticker, trade, fill, user_orders, market_positions, orderbook_delta, and communications are uniformly upgraded to FixedPointCount. No half-measures.
• apply_snapshot simplification (orderbook.py:79-80): Dropping the Decimal(p)/Decimal(q) wrappers is exactly right — Pydantic now handles coercion at parse time, so calling dict() on already-typed tuples is clean and removes redundant conversion.
• Test suite quality: The parametrized test_rest_ws_field_type_symmetry test is particularly good — it pins REST↔WS annotation equality directly as a CI guard rather than relying on code review memory. The isinstance() checks in TestWsPayloadDecimalCoercion verify the runtime type, not just value equality.
• No scope creep: dollar_volume/dollar_open_interest in TickerPayload (still str) are correctly left alone — outside _fp scope.

---

Issues / concerns

1. Removed TODO in RfqCreatedPayload without confirmed live capture (medium risk)

The original TODO(v0.15.1) block explicitly noted that the user_orders live capture showed demo emitting created_ts_ms as an integer instead of the spec's ISO string, and flagged the same risk for the Communications channel:

"Capture a live frame on demo when the channel is active and confirm or adjust."

This PR removes that TODO and commits to created_ts: datetime, but the PR description doesn't mention a live capture was done. created_ts and created_ts_ms are different field names, so this is lower risk than it might appear — a server sending created_ts_ms wouldn't silently break created_ts parsing. But if the server emits created_ts as an integer millisecond value (mirroring the user_orders pattern), Pydantic's datetime validator will reject it as a ValidationError.

Suggestion: Either add a comment confirming the live capture was done, or add a BeforeValidator that handles the int → datetime coercion for millisecond timestamps (similar to how user_orders handles *_ts_ms). At minimum, document the assumption.

2. RfqDeletedPayload.deleted_ts not in TestWsPayloadDatetimeCoercion

test_communications_timestamps_parse_as_datetime tests RfqCreatedPayload, QuoteCreatedPayload, and QuoteExecutedPayload, but skips RfqDeletedPayload.deleted_ts. The field was changed in this PR and should have a regression guard.

3. FixedPointCount and DollarDecimal are runtime-identical

Both are Annotated[Decimal, BeforeValidator(_coerce_decimal), PlainSerializer(_decimal_to_str)]. The parametrized symmetry test asserts rest_field == ws_field which will pass for any (DollarDecimal, DollarDecimal) or (FixedPointCount, FixedPointCount) pair, but would also pass for a (DollarDecimal, FixedPointCount) pair since they're the same type alias under the hood. This is not a bug in the PR — it's a pre-existing condition — but worth noting so future reviewers don't over-trust the symmetry test as a type-semantic guard.

---

Nits

• In communications.py, RfqCreatedPayload's docstring still references extra="allow" implicitly (via the model config inherited), but the updated docstring no longer mentions the caveat. Could be clearer about the extra="allow" rationale now that the capture TODO is gone.
• The # _fp format trailing comments that were removed — good cleanup.

---

Summary

Approve with the suggestion to address (1) before merge or immediately after — the removed uncertainty acknowledgment about live frame capture is the only non-trivial risk in this PR. Everything else is mechanical and correct.

[claude]

Code Review — PR #230: WS payload type sweep (FixedPointCount + datetime)

Verdict: Approve with two pre-merge notes.

---

What's good

• Complete sweep: Every _fp-aliased count/size/volume field across all seven WS model files is uniformly upgraded to FixedPointCount. No half-measures or missed fields.
• apply_snapshot simplification (orderbook.py:79–80): Replacing {Decimal(p): Decimal(q) for p, q in ...} with dict(msg.msg.yes) is the correct payoff — coercion now happens at parse time, not at every snapshot application.
• Contract test update (test_contracts.py:579–582): Adding datetime to _sdk_type_kind and loosening the date-time rule to accept "datetime" alongside "str" is exactly right. Spec says string + format=date-time; a typed datetime is strictly more correct.
• test_rest_ws_field_type_symmetry: Parametrized annotation equality test is a solid long-term guard. Six pairs covering FixedPointCount and DollarDecimal symmetry between REST and WS models.
• isinstance() checks in TestWsPayloadDecimalCoercion: Verifying runtime type, not just value equality, is the right approach for a regression guard.

---

Issues

1. Removed TODO without confirming live capture (pre-merge)

The deleted block in RfqCreatedPayload documented a specific open question:

"Capture a live frame on demo when the channel is active and confirm or adjust. ... extra="allow" provides a soft landing."

The PR description doesn't mention a live capture was done. The risk: if the demo server sends created_ts as a millisecond integer (mirroring the user_orders channel's created_ts_ms observation), Pydantic v2 will coerce it via Unix-seconds interpretation, yielding a datetime in year ~55000 rather than a ValidationError. The extra="allow" safeguard is still present, but the field is now a silent corruption risk rather than a loud parse failure.

Suggested fix: If live capture confirmed ISO string output, add a comment: # Confirmed via live demo capture: server emits ISO string. If not confirmed, restore a shorter version of the caveat noting the assumption.

2. RfqDeletedPayload.deleted_ts type assertion missing

test_rfq_deleted_rfq_context (line 850) parses a payload with deleted_ts but asserts only on event_ticker, contracts, and target_cost. The changed field (deleted_ts: str → datetime) has no isinstance assertion, making it the only timestamp change in this PR without a type regression guard.

Suggested addition to TestWsPayloadDatetimeCoercion:

rfq_deleted = RfqDeletedPayload.model_validate({
    "id": "rfq-1", "creator_id": "u1", "market_ticker": "T",
    "deleted_ts": "2026-04-19T18:43:37Z",
})
assert isinstance(rfq_deleted.deleted_ts, datetime)

---

Nits

• FixedPointCount / DollarDecimal are runtime-identical (Annotated[Decimal, BeforeValidator(_coerce_decimal), PlainSerializer(_decimal_to_str)]). The symmetry test's rest_field == ws_field assertion would silently pass even for a (DollarDecimal, FixedPointCount) cross-pair. Not introduced here — pre-existing — but worth noting so future reviewers don't over-trust the symmetry test as a semantic-type guard.
• datetime serialization: WS payloads are receive-only, so no custom PlainSerializer is needed. If any of these models are ever serialized back to wire format, the default Pydantic isoformat() output may not match the original wire string exactly (e.g., offset representation). Fine for now; worth noting if serialization is ever added.
• # _fp format comment removal: Correct cleanup throughout.