test(infra): wire response-side drift through EXCLUSIONS map (PR0)

[TexasCoding]

Closes #158. PR0 of the response-side spec-drift hardening stack (#157). Subsequent PRs (#PR1–#PR4) backfill the real model gaps; #PR5 flips warn→fail.

What this changes

tests/_contract_support.py

• Rewrite module docstring + EXCLUSIONS comment header to document all four consumers: TestRequestParamDrift, TestRequestBodyDrift, TestSpecDrift (additive + required), TestWsSpecDrift (additive + required). Note that a single (fqn, field) key may serve more than one check—e.g. CreateOrderRequest is both a request body and a response schema in the spec.
• Add 4 new response-side entries:
  • Market.response_price_units (spec_deprecated)
  • Order.action (spec_deprecated)
  • Orderbook.orderbook_fp (wire_normalization)
  • TickerPayload.time (spec_deprecated)

The other 3 entries listed in the issue (CreateRFQRequest.target_cost_centi_cents, CreateRFQRequest.contracts_fp, CreateOrderRequest.sell_position_floor) already existed as body-side entries with more detailed reasons; they are now reused by response-side checks via the new wiring. Adding duplicate keys would have overwritten the existing reasons.

tests/test_contracts.py

• _classify_drift now consults EXCLUSIONS.get((entry.sdk_model, field)) alongside entry.ignored_fields in both the additive and required loops. One change here covers test_additive_drift, test_required_drift, and test_ws_additive_drift (all three funnel through this helper).
• test_ws_required_drift inline loop gets the same EXCLUSIONS lookup.
• test_exclusion_map_is_current case 1 now walks all three source maps (BODY_MODEL_MAP, CONTRACT_MAP, WS_CONTRACT_MAP) and flags a key stale only when the field is absent from every applicable spec source. The else branch's error message now also mentions kalshi.ws.models.* as a valid prefix.

No production code (kalshi/**) touched.

Verification

$ uv run pytest tests/test_contracts.py
372 passed, 57 warnings in 65.65s

$ uv run pytest tests/test_contracts.py -W error::UserWarning
57 failed, 315 passed   # baseline on main was 60 failed

$ uv run mypy kalshi/
Success: no issues found in 76 source files

$ uv run mypy tests/_contract_support.py tests/test_contracts.py
Found 11 errors in 1 file (checked 2 source files)
# all 11 pre-existing on main, verified via `git stash`

$ uv run ruff check .
All checks passed!

Smoke-tested test_exclusion_map_is_current by temporarily flipping Market.response_price_units and (separately) TickerPayload.time to bogus field names. Both branches surface the entry with "absent from every spec source (...)", then revert cleanly.

Direct grep against the -W error output confirms each of the 7 (model, field) keys no longer surfaces in any warning bullet:

• Market.response_price_units — gone from test_additive_drift[Market]
• Order.action — gone from test_additive_drift[Order] (Fill.action, CreateOrderRequest.action, FillPayload.action remain on the required side; those are separate keys, intentionally not in scope)
• CreateRFQRequest.target_cost_centi_cents — gone
• Orderbook.orderbook_fp — gone
• CreateRFQRequest.contracts_fp — gone (RfqDeletedPayload.contracts_fp remains on a different model)
• CreateOrderRequest.sell_position_floor — gone
• TickerPayload.time — gone (price_dollars, ts_ms remain)

Note on the failure count in acceptance criterion 2

The issue claimed "exactly 53 failures" under -W error::UserWarning. The actual count is 57. The discrepancy is because the issue's math assumes 1 allowlist entry → 1 test fail→pass transition. In reality, only 3 tests have that property (test_additive_drift[Orderbook], [CreateRFQRequest], [CreateOrderRequest] — the three where the allowlisted field was the sole drift bullet). Market / Order / TickerPayload additive tests lose one bullet each but still warn because they retain many other unfiltered drift entries (fee_waiver_expiration_time, outcome_side, book_side, price_dollars, ts_ms, etc.).

The substantive intent of the criterion — "Confirms the EXCLUSIONS map is being consulted by the response-side checks" — holds: the per-entry grep above shows all 7 keys are filtered while every other drift bullet is preserved.

Out of scope (per the issue)

• Backfilling SDK model gaps (#PR1–#PR4).
• Flipping warnings.warn(...) → pytest.fail(...) (#PR5).
• Required-but-Optional drift policy (~204 entries, separate decision).

Part of the response-side spec-drift hardening stack tracked in #157.

[claude]

Code Review — PR #165: Wire response-side drift through EXCLUSIONS map

Overall: This is a clean, focused PR that does exactly what it says. No production code is touched, the change surface is narrow, and the 4 new EXCLUSION entries are all well-reasoned. A few minor issues worth addressing before merge.

---

What the PR does well

• Minimal and correct. The core change — adding two if (entry.sdk_model, field) in EXCLUSIONS: continue guards to _classify_drift and one to test_ws_required_drift — is the smallest possible diff to achieve the goal.
• test_exclusion_map_is_current rewrite is an improvement. The old version only checked BODY_MODEL_MAP for model-keyed entries. The new version checks all three sources (body, REST response, WS payload), which correctly handles shared models like CreateOrderRequest that appear in multiple maps. The "absent from every spec source" error message is clearer than the old "not in BODY_MODEL_MAP".
• New EXCLUSION entries are well-documented. All four new entries include a concrete reason and a typed kind. The wire_normalization rationale for Orderbook.orderbook_fp is especially useful since it explains a non-obvious reshaping that happens at the resource layer.
• _classify_drift extension automatically covers test_ws_additive_drift. No separate fix needed — the funnel-through-helper design pays off here.

---

Issues

Minor: _load_asyncapi_spec() called inside the EXCLUSIONS loop

In test_exclusion_map_is_current (line ~1401), _load_asyncapi_spec() is called inside the body of the for w_entry in WS_CONTRACT_MAP branch:

for w_entry in WS_CONTRACT_MAP:
    if w_entry.sdk_model == fqn:
        ws_spec = _load_asyncapi_spec()   # ← disk read + YAML parse per WS entry
        spec_sources.append(...)
        break

With one WS exclusion today (TickerPayload.time) this is fine, but as the WS exclusion list grows it will parse the AsyncAPI YAML once per WS-model key in EXCLUSIONS. Consider hoisting ws_spec = _load_asyncapi_spec() to just before the outer for (fqn, name), excl in EXCLUSIONS.items() loop (similar to how spec = _load_spec() is already hoisted). Guard with the same if not ASYNCAPI_FILE.exists(): pytest.skip(...) pattern used elsewhere if the file may be absent.

Minor: Missing continue after "absent from every spec source" stale append

In the updated test_exclusion_map_is_current:

if not any(name in props for _label, props in spec_sources):
    source_list = ...
    stale.append(...)
    # no `continue` here

model_cls = _get_model_class_from_fqn(fqn)  # always runs
if name in _model_aliases(model_cls):
    stale.append(...)    # ← second stale entry for the same key

If a field is absent from every spec source, the code still falls through to check _model_aliases. An entry that is stale-because-field-missing could then also produce a second stale entry saying "SDK does emit it." This existed in the old code too (not a regression), but the new multi-source path makes the double-report more likely to surface in practice. Adding continue after the first stale.append(...) would prevent the confusing double message.

Nit: continue skips CONTRACT_MAP/WS_CONTRACT_MAP when body_schema is None

if spec_ref is not None:
    ...
    if body_schema is None:
        stale.append(...)
        continue      # ← skips Sources 2 and 3

If a model is in BODY_MODEL_MAP but its body schema can't be resolved via METHOD_ENDPOINT_MAP, the continue prevents checking whether the field is valid via CONTRACT_MAP or WS_CONTRACT_MAP. For a shared model this would be a false-positive stale entry. In practice this path shouldn't trigger for any current entry (the BODY_MODEL_MAP models all have resolvable schemas), but it's a latent footgun. Low priority, but worth a note in a follow-up.

---

Test coverage

The 7-entry grep verification in the PR description is thorough and convincing. The smoke test of test_exclusion_map_is_current by flipping field names to bogus values is exactly the right way to validate the stale-detection path.

One gap: there's no explicit test that a WS-model EXCLUSION entry (like TickerPayload.time) causes test_exclusion_map_is_current to consult WS_CONTRACT_MAP rather than silently pass with an empty spec_sources. The PR description says the smoke test was run manually — consider adding a comment in test_exclusion_map_is_current noting that the ws_spec branch is exercised by TickerPayload.time, so future readers know there's live coverage of that path.

---

Summary

Item	Severity	Blocking?
_load_asyncapi_spec() inside loop	Minor perf	No
Missing continue after "absent from every spec source"	Minor correctness	No
continue skips other spec sources on body_schema=None	Latent footgun	No
No inline comment pointing to live WS branch coverage	Nit	No

The PR is correct for the cases it targets, and the EXCLUSIONS wiring is exactly the right approach for this layer of the stack. The issues are pre-existing in character or low-probability edge cases. Recommend merging after addressing the _load_asyncapi_spec() hoisting (or confirming you're OK with the current behavior as a known TODO).

[TexasCoding]

Thanks for the review. Addressed in 36b9ab8.

1. _load_asyncapi_spec() inside the loop — fixed

Lazy-cached via a sentinel initialized before the outer loop:

spec = _load_spec()
# Lazy-loaded on first WS-keyed exclusion (avoids parsing the AsyncAPI
# YAML when no WS entry needs it; reused across all WS entries when one
# does — currently exercised live by `TickerPayload.time`).
ws_spec: dict[str, Any] | None = None
stale: list[str] = []

for (fqn, name), excl in EXCLUSIONS.items():
    ...
    for w_entry in WS_CONTRACT_MAP:
        if w_entry.sdk_model == fqn:
            if ws_spec is None:
                ws_spec = _load_asyncapi_spec()
            spec_sources.append(...)
            break

Chose lazy-with-sentinel over unconditional hoisting so the AsyncAPI YAML parse is skipped entirely when no WS-keyed exclusion exists. Cost is 1x per test run when at least one WS entry needs it, regardless of how many WS entries the list grows to.

3. continue after body_schema is None — fixed

Replaced with if/else so the stale diagnostic still fires (BODY_MODEL_MAP/METHOD_ENDPOINT_MAP inconsistency IS a real bug worth surfacing) but the loop falls through to CONTRACT_MAP and WS_CONTRACT_MAP:

if body_schema is None:
    # METHOD_ENDPOINT_MAP / BODY_MODEL_MAP inconsistency is a
    # real bug; surface it, but still try CONTRACT_MAP and
    # WS_CONTRACT_MAP so a shared model isn't falsely flagged
    # as unknown when one of its other sources would resolve.
    stale.append(...)
else:
    spec_sources.append(...)

If body resolution fails AND the field is missing from other sources, both stale messages now fire — which is correct: the infra is broken AND the entry is bad. If body resolution fails but CONTRACT_MAP covers the field, only the infra diagnostic fires (no false-positive on the entry itself).

Live WS coverage comment — added

The ws_spec initialization comment explicitly names TickerPayload.time as the live exercise of the WS branch, so future readers know there is real coverage even without a dedicated regression test.

Not addressed: dual-stale-message after "absent from every spec source"

You correctly note this is pre-existing (predates this PR). I think the two messages carry independent diagnostic value:

• "spec does NOT have it" — the spec moved; the exclusion's spec coordinate is stale.
• "SDK does emit it" — the SDK now implements it; the exclusion's SDK assertion is stale.

A truly orphaned entry that fails both would be doubly stale, and reporting both tells the maintainer exactly which side moved (or both did) so they can fix root-cause rather than guess. Adding continue between them would lose that signal. Leaving as-is.

Re-verification

uv run pytest tests/test_contracts.py            → 372 passed, 57 warnings
uv run ruff check .                              → All checks passed!
uv run mypy kalshi/                              → Success: no issues

Smoke-retested all three test_exclusion_map_is_current branches:

• REST stale (Market.bogus_rest_xyz) → FAILED as expected
• WS stale (TickerPayload.bogus_ws_xyz) → FAILED as expected (exercises lazy-load)
• baseline (no patch) → PASSED

+14 -5 on tests/test_contracts.py.

[claude]

Follow-up review — updated diff after 36b9ab8

All three issues from the prior review have been addressed. Reviewing the final diff state:

---

Fixes confirmed

_load_asyncapi_spec() lazy-with-sentinel (issue 1) — The ws_spec: dict[str, Any] | None = None sentinel before the outer loop, with if ws_spec is None: ws_spec = _load_asyncapi_spec() inside the WS branch, is the right call. Avoids parsing AsyncAPI YAML entirely when no WS exclusion exists; costs exactly one parse when any WS entry is present. Clean.

if/else after body_schema is None (issue 3) — The else: spec_sources.append(...) guard is correct. A BODY_MODEL_MAP/METHOD_ENDPOINT_MAP inconsistency now fires its own diagnostic but still lets CONTRACT_MAP and WS_CONTRACT_MAP resolve the field. A truly bad entry gets both stale messages; a body-infra bug on a model that's also a response schema doesn't produce a false-positive on the entry itself.

Dual stale messages (issue 2, intentionally not addressed) — The author's rationale is sound: "spec coordinate stale" and "SDK now emits it" are independent facts; surfacing both tells a maintainer which side moved. Leaving as-is is the right call.

---

Remaining observations (non-blocking)

• kind values are free-form strings. spec_deprecated, wire_normalization, paginator_handled are all used as plain str. No enum enforces a closed vocabulary, so a typo (spec_deprecatd) would silently pass. Not a problem today with ~20 entries, but worth noting as the exclusion list grows toward the post-v2.0 backfill work (#PR1–#PR4).

• _classify_drift comment ordering. The new docstring says skips are honored "1. entry.ignored_fields, 2. EXCLUSIONS" — but in the additive loop both checks appear together (continue after each), while in the required loop the order also matches. Accurate and readable; no action needed.

• Test symmetry. test_ws_additive_drift gets the EXCLUSIONS guard via _classify_drift (funnel-through-helper), while test_ws_required_drift gets it as an inline guard. This asymmetry is pre-existing (required WS drift wasn't routing through _classify_drift before this PR either) and not introduced here. Worth flagging as a future cleanup (Harden response-side spec drift: promote additive warnings to hard-fail #157 stack), but not a blocker.

---

Verdict

Ready to merge. The core change is minimal and correct, all review issues are addressed, and the test_exclusion_map_is_current rewrite is a net improvement in coverage and error messages. The 4 new EXCLUSION entries are well-reasoned and smoke-tested.