test(contracts): map remaining 42 REST sub-models + promote completeness gate (#171)

[TexasCoding]

Closes #171.

Summary

Maps the remaining 42 SDK response/request models into CONTRACT_MAP, taking it from 49 → 91 entries. Promotes test_contract_map_completeness from warnings.warn to pytest.fail so the next unmapped model fails CI loudly.

This closes the same regression class that hid Balance.balance_dollars for five review rounds in v2.1.0 — an unmapped model is invisible to drift checks, so additive spec changes slide through silently. The WS side has been hard-fail since #157; this brings the REST side to parity.

Acceptance (per #171)

• len(CONTRACT_MAP) increases from 49 to 91. ✅ (exactly the issue's "~91" target)
• pytest tests/test_contracts.py -W error::Warning -k test_contract_map_completeness passes. ✅
• The test body's warnings.warn(...) is replaced with pytest.fail(...). ✅
• test_additive_drift regression-free. ✅ (one finding addressed by adding 4 spec fields to PriceDistribution)

What changed

Infrastructure

• _get_schema_fields / _get_required_fields gain a dotted-path syntax for inline schemas the spec doesn't name at the top level. BatchCancelOrdersV2Request.orders.items resolves to the per-entry inline object inside the batch wrapper. Used for 3 V2 batch wrapper entries (Batch{Cancel,Create}OrdersV2{Request,Response} per-entry shapes).
• _resolve_schema helper centralizes schema lookup; returns {} for array-typed spec schemas so field-by-field drift checks skip them cleanly. Used for the OrderbookLevel ↔ PriceLevelDollarsCountFp mapping.

42 new CONTRACT_MAP entries

Module	Count	Notes
markets	4	BidAskDistribution, Candlestick, OrderbookLevel, PriceDistribution
orders	19	All *V2* request/response + 3 inline batch entries
events	2	MarketMetadata, SettlementSource
exchange	4	Schedule, DailySchedule, WeeklySchedule, MaintenanceWindow
portfolio	4	Deposit, Withdrawal, IndexedBalance, PositionsResponse
series	3	EventCandlesticks, PercentilePoint, ForecastPercentilesPoint
multivariate	6	AssociatedEvent, LookupPoint, Create/LookupTickers request/response pairs

5 entries use renamed spec names (Candlestick → MarketCandlestick, PositionsResponse → GetPositionsResponse, etc.); 3 use the new dotted-path syntax for inline schemas; 1 (OrderbookLevel) maps to an array-typed spec schema.

Drift surfaced by mapping (tightened per #172 policy)

Mapping these models for the first time made test_required_drift and test_additive_drift see them — and they immediately caught real drift. Tightened to required (no default):

• BidAskDistribution: open, high, low, close
• Candlestick: yes_bid, yes_ask, price, volume, open_interest, end_period_ts
• MarketMetadata: image_url, color_code
• Schedule: standard_hours, maintenance_windows
• WeeklySchedule: all 7 day-of-week fields
• PositionsResponse: market_positions, event_positions
• EventCandlesticks: market_tickers, market_candlesticks, adjusted_end_ts
• ForecastPercentilesPoint: percentile_points
• AssociatedEvent: is_yes_only, active_quoters
• LookupPoint: selected_markets, last_queried_ts

Added (additive drift in PriceDistribution):

• mean, previous, min, max (_dollars wire form, all optional per spec — candlestick window may contain no trades)

Test fixtures

3 new builders in tests/_model_fixtures.py:

• candlestick_dict — spec-complete Candlestick wire dict
• bid_ask_distribution_dict — OHLC sub-dict for Candlestick.yes_bid / .yes_ask
• price_distribution_dict — Candlestick.price (all fields optional)

6 tests migrated to use them (test_historical.py, test_markets.py, test_models.py, test_series.py, test_series_models.py).

Verification

• uv run pytest tests/ --ignore=tests/integration → 2107 passed, 0 warnings.
• uv run pytest tests/test_contracts.py -W error::Warning → 459 passed, gate is live.
• ruff check clean on every file touched by this PR.

Soft-breaking footprint

Same shape as #172: callers who construct Candlestick/BidAskDistribution/MarketMetadata/Schedule/WeeklySchedule/PositionsResponse/EventCandlesticks/ForecastPercentilesPoint/AssociatedEvent/LookupPoint directly with partial values will now hit ValidationError. Common-case real construction is via model_validate(server_json), which is unaffected. Test fixtures should use the new _model_fixtures builders.

Out of scope

• The OrderbookLevel ↔ positional-tuple semantic gap (SDK uses object, spec uses 2-tuple) is documented in the ContractEntry notes but not "fixed" — the SDK's object shape is the more useful ergonomic. Worth a separate discussion if we want to harmonize, but the drift gate now correctly skips the field-by-field check on this one.

[claude]

Code Review — PR #181: Contract map completeness + hard-fail gate

Overall: Solid infrastructure PR with clear motivation. The hard-fail promotion is the right call, and the dotted-path syntax for inline schemas is a clean solution. A few nits and one structural inconsistency worth addressing before merge.

---

What this does well

• Goal is well-scoped. 49 → 91 entries, gate promoted to pytest.fail, WS/REST parity achieved. The description of the regression class this closes (Balance.balance_dollars hiding for five rounds) makes the value concrete.
• _resolve_schema helper. Extracting it out of _get_schema_fields and centralizing the dotted-path walk is the right refactor. The $ref resolution on each iteration correctly handles the case where a property is itself a ref.
• Fixture builders are a genuine improvement. Replacing inline dicts with candlestick_dict() makes the tests resilient to future required-field additions — no more hunting 6 test files to update raw dicts.
• PriceDistribution additions are correctly optional. Flagging mean/previous/min/max as spec additions with the reason (no-trade window) is one of the few cases where a comment genuinely earns its place.

---

Issues

1. _get_required_fields doesn't consistently use _resolve_schema (minor bug risk)

_get_schema_fields now always goes through _resolve_schema and will pytest.fail if a schema is missing. But _get_required_fields special-cases dotted paths:

# tests/test_contracts.py
if "." in schema_name:
    schema = _resolve_schema(spec, schema_name)
else:
    schema = schemas.get(schema_name, {})   # ← silently returns {} if missing

For non-dotted names, a missing or misspelled schema silently returns an empty required set instead of failing. Since _get_schema_fields is called first in test_required_drift, you'd likely catch the error there — but the inconsistency is fragile. Simplest fix: replace the branching with a single _resolve_schema call, the same way _get_schema_fields was simplified.

def _get_required_fields(spec, schema_name):
    schema = _resolve_schema(spec, schema_name)
    ...

2. Tautological assertion in test_models.py

After yes_bid was tightened to required (non-optional), this assertion is now always true:

# tests/test_models.py ~line 766
assert c.yes_bid is not None

Pydantic won't populate yes_bid with None when the field is BidAskDistribution (no | None). The assertion can be removed or replaced with an attribute check like assert c.yes_bid.open == Decimal("0.4000").

3. Import ordering in test_models.py

candlestick_dict is inserted after create_order_group_response_dict in the import block, but alphabetically it should come before it. Minor, but the rest of the imports in this file appear to be alpha-sorted.

# current
from tests._model_fixtures import (
    create_order_group_response_dict,
    candlestick_dict,    # ← should be first
    ...

---

Observations (not blocking)

• price_distribution_dict is a no-op wrapper. It returns {} plus overrides, which is just dict(**overrides). That's correct (all PriceDistribution fields are optional), and keeping it consistent with the other builders is a reasonable choice — just note it doesn't add a meaningful baseline.

• OrderbookLevel semantic gap is documented but not resolved. The note in ContractEntry is clear and the decision to skip field-by-field comparison is sound. Good call flagging it for future discussion rather than shoehorning a fix here.

• test_schema_coverage now delegates failure to _resolve_schema. The behavior is correct but the test body reads as a bare loop with no assertion. A single-line comment would help: "resolution failure is reported by _resolve_schema via pytest.fail". Optional.

---

Summary

The hard-fail promotion and fixture builder migration are both CI-safety wins. The _get_required_fields inconsistency is the only substantive concern — it's a latent silent-pass case that could mask future schema errors. The rest are nits. Worth a quick fixup commit before merge.

[TexasCoding]

Addressed in d858d90:

CI failure (lint I001) — fixed. candlestick_dict was inserted in the wrong alphabetical slot in both tests/test_models.py and tests/test_series_models.py imports. Local ruff happened to be lenient on those; CI's run flagged them. ruff check --fix cleaned it up.

1. _get_required_fields inconsistency — fixed. Now goes through _resolve_schema unconditionally (was branching on "." in schema_name, with the plain-name path falling through to a silent schemas.get(schema_name, {})). Missing schemas now fail loudly, matching _get_schema_fields.

2. Tautological is not None assertions — dropped. c.yes_bid is not None and c.price is not None in test_candlestick_nested_structure both became always-true once Candlestick.yes_bid / .price were tightened to required non-nullable. The downstream c.yes_bid.open == Decimal(...) chains still exercise the parsed structure.

3. Import order in test_models.py — covered by the CI ruff fix above.

Non-blocking observations not actioned:

• price_distribution_dict is an effective no-op — agreed. Kept for symmetry with the other builders and to give consumers a single import surface; if more PriceDistribution fields ever become required, the builder is the only file to update.
• OrderbookLevel semantic gap — already documented in the ContractEntry notes. Worth a separate discussion if we want to harmonize.
• test_schema_coverage could use a comment — there's already a one-line comment on the loop body: # _resolve_schema fails the test internally if the path doesn't resolve.

Verification: full unit suite still 2107 passed, 0 warnings. ruff check clean on every touched file.

[claude]

Code Review — PR #181: Contract-map completeness + hard-fail gate

Overall this is a clean, well-scoped infrastructure PR. The motivation is solid: unmapped models are invisible to drift checks, and the Balance.balance_dollars regression in v2.1.0 is exactly the kind of silent failure this gate prevents. The hard-fail promotion is the right call now that the map is complete.

---

What's Good

• _resolve_schema extraction is a clean refactor. Centralising schema resolution (with dotted-path support) removes the duplicated lookup logic between _get_schema_fields and _get_required_fields, and the docstring clearly documents the path syntax.
• Hard-fail promotion is well-justified and the comment in the old warn-only block is properly cleaned up rather than left as archaeology.
• Fixture builders (candlestick_dict, bid_ask_distribution_dict) follow the existing _model_fixtures pattern and correctly provide all now-required fields as defaults.
• test_schema_coverage migration from assert entry.spec_schema in schemas to calling _resolve_schema is correct — dotted paths weren't resolvable via a plain dict key lookup.
• CHANGELOG is detailed and accurate; the "soft-breaking footprint" call-out is the right communication for downstream users.

---

Issues

Minor bug: stale is not None assertion in async historical test

test_historical.py (~line 715 in the PR branch):

assert candles[0].yes_bid is not None

yes_bid is now BidAskDistribution (required, not Optional). This assertion is vacuously true — it can never fire. test_models.py correctly removed the equivalent assertions (assert c.yes_bid is not None, assert c.price is not None), but this one was missed. It should be replaced with a value assertion, e.g.:

assert candles[0].yes_bid.open == Decimal("0.40")

price_distribution_dict adds no functional value

def price_distribution_dict(**overrides: Any) -> dict[str, Any]:
    """Spec-shaped PriceDistribution dict (Candlestick.price). All optional."""
    base: dict[str, Any] = {}
    base.update(overrides)
    return base

This is effectively dict(**overrides) — the function body is an identity operation over an empty base. It exists purely for naming/intent signalling, which is fine, but callers currently pass it as "price": price_distribution_dict() which produces "price": {}. A short comment inside the function explaining that PriceDistribution fields are all optional (so a valid wire value is {}) would make this less surprising to future readers — currently the empty body looks like an oversight.

_resolve_schema: if not node could false-fail on a legitimate empty object schema

if not node:
    pytest.fail(f"Schema path '{schema_name}' failed at segment {segment!r}")

If any mid-path segment legitimately resolves to {} (an empty object schema — unusual but allowed in OpenAPI), this fires a false fail. In practice the current dotted paths (*.orders.items) won't hit this, but it's a latent fragility. if node is None or if node == {} with separate messaging would be safer. Low priority since the current path set is small.

CHANGELOG double blank line (cosmetic)

+### Contract-map completeness (#171)
+...
+
+
+### Required-but-optional drift closure (#172)

Two blank lines between the new and existing sections — rest of the file uses single blank lines between ### entries.

---

Verification questions (not blockers)

• LookupPoint.last_queried_ts changes from datetime | None = None to required datetime. If the live API ever omits this field (e.g., when no lookups have been made), this will raise ValidationError at runtime. Was this verified against the API's actual behavior, or only from the spec? The spec being required doesn't always match the API returning the field.
• AssociatedEvent.active_quoters and PositionsResponse.market_positions/event_positions — same question. If these can be omitted by the server rather than sent as [], the tightening will silently break callers. Given the PR passed 2107 tests with no integration test failures noted, this seems fine, but worth a note.

---

Summary

Area	Status
Infrastructure (_resolve_schema, dotted-path)	✅ Clean
Hard-fail promotion	✅ Correct
42 new CONTRACT_MAP entries	✅ Well-mapped
Model field tightening	✅ Spec-aligned
Fixture builders	✅ Good pattern
Stale is not None assertion	⚠️ Should fix
price_distribution_dict body	⚠️ Needs a comment
CHANGELOG whitespace	nit

The stale assertion is the only change worth making before merge. Everything else is either a nit or a verification question.

[TexasCoding]

Round 2 addressed in 1736b7c:

Stale is not None in test_historical.py (and 5 others) — fixed. Caught the one the bot flagged (line 715, async historical) plus 6 more I'd missed: 2 in test_async_markets.py, 1 more in test_historical.py, and 3 in test_markets.py. All are tautologies after the Candlestick / BidAskDistribution / PriceDistribution tightening; downstream attribute assertions still exercise the structure.

_resolve_schema false-fail on empty schema — fixed. Was using .get(segment, {}) followed by if not node, which would false-fail on any spec schema that legitimately resolves to {} (rare but allowed in OpenAPI). Rewrote to explicit if segment not in properties / if "items" not in node checks with per-mode error messages. Distinguishes "key missing from parent" from "key present, schema is empty object".

price_distribution_dict body — added a 4-line docstring section explaining the intentionally empty base (all fields optional per spec because the OHLC window may contain no trades) so the empty body doesn't read as an oversight.

CHANGELOG double blank line — fixed.

Verification questions (LookupPoint.last_queried_ts, AssociatedEvent.active_quoters, PositionsResponse.market_positions/event_positions): same posture as #172 — tightened by default per the spec's required: true, with the EXCLUSIONS escape hatch (kind="server_omits_despite_required") ready if the nightly integration job surfaces a real-world omission. These three were not on this PR's explicit watch list (which called out Trade taker direction fields, MultivariateEventCollection open/close dates, and WS *_ts_ms), but they fit the same pattern; I'll add them to the post-merge watch list.

Verification: full unit suite still 2107 passed, 0 warnings. ruff check . clean.

[claude]

Code Review — PR #181: Contract-map completeness (#171)

Overall: This is solid, well-scoped infrastructure work. The approach is correct, the execution is clean, and the PR description matches the diff faithfully. A few things worth noting below.

---

What's done well

• Surgical changes. Every line traces directly to the goal: map the remaining 42 models, tighten required fields, harden the gate. No adjacent cleanup noise.
• _resolve_schema refactor is clean. The dotted-path syntax (Parent.field.items) is a pragmatic way to address inline schemas without polluting the OpenAPI spec. The $ref resolution at both intermediate and final nodes is correct (including the edge case where the root node is itself a $ref — rest is empty, so the trailing if "$ref" in node: handles it).
• Removing is not None assertions in tests after tightening fields to required is the right cleanup — those guards were masking the actual contract.
• test_schema_coverage updated to call _resolve_schema instead of direct dict lookup — keeps the coverage test honest about dotted-path entries.
• CHANGELOG entry is detailed and accurate. The soft-breaking-change callout is good hygiene.

---

Issues / things to verify

1. WeeklySchedule day-of-week fields are now required — is the API consistent?

# exchange.py
monday: NullableList[DailySchedule]   # was = []
tuesday: NullableList[DailySchedule]  # was = []
...

NullableList[T] is list[T] | None, so None is still valid. But if the API ever omits a day key entirely (vs. sending null or []), validation now raises. Worth confirming the exchange status endpoint always returns all 7 keys — either a real API call or a grep through any recorded fixtures.

2. AssociatedEvent.is_yes_only: bool — previously bool = False, now required

# multivariate.py
is_yes_only: bool   # was: bool = False

A bool default of False is qualitatively different from an empty list default: it was arguably load-bearing as a "absent = false" semantic. If the API reliably includes this field, tightening is correct; if not, callers who rely on absent-equals-false will get a ValidationError. The spec says required — trusting that — but worth a callout for callers who construct AssociatedEvent directly in tests or scripts.

3. _get_required_fields has no explicit early-return for non-object schemas

_get_schema_fields documents "returns {} for array-typed schemas" (presumably via a type-check branch I can see was added). _get_required_fields does NOT have an equivalent guard — it calls _resolve_schema, then does schema.get("required", []). For an array schema that has no required key, this returns set() which is correct, but it's silent. If the logic ever diverges, test_required_drift could pass vacuously on array-typed schemas. A comment mirroring _get_schema_fields's note would make this intentional.

4. price_distribution_dict returns {}

def price_distribution_dict(**overrides: Any) -> dict[str, Any]:
    base: dict[str, Any] = {}
    base.update(overrides)
    return base

Semantically correct (all fields are optional), but future devs calling price_distribution_dict() and expecting something testable will be surprised. The docstring explains it well — this is fine as-is, just noting it.

5. One test still builds a partial candlestick without yes_ask

In test_historical.py async test (~line 720):

candlestick_dict(
    end_period_ts=1700000000,
    yes_bid={...},
    volume_fp="500.00",
)

yes_ask is not overridden, so it comes from the builder default (bid_ask_distribution_dict()). That's correct — candlestick_dict() provides all required fields by default. Just confirming the intent is that the test doesn't care about yes_ask values here, which is fine.

---

Minor nit

# test_contracts.py, updated docstring
# "Hard-fails via ``pytest.fail``. See the docstring of
#  tests/_contract_support.py for the broader rationale; as of #172/#171
#  every contract gate in this file is now hard-fail."

The #172/#171 ordering is reverse-chronological (172 shipped before 181). Non-blocking — just a heads-up if order matters to you.

---

No concerns with

• The 42 new ContractEntry additions — all look correct against the PR's spec mapping table.
• Dotted-path implementation in _resolve_schema — handles $ref at intermediate and terminal nodes, fails fast with clear messages.
• Removing warnings import — clean since warnings.warn is gone.
• test_series_models.py — candlestick_dict(end_period_ts=2000) is the correct fix for the previously under-specified fixture.

---

Verdict: Approve with the above notes. Issue 2 (bool tightening) and issue 1 (WeeklySchedule) are the only ones worth a quick sanity check before merge; the rest are informational.