[TexasCoding]

Closes #269

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

[claude]

Code Review — PR #290

Summary: Wave 3 of the post-v2.4 audit. Adds positions_all() pagination helpers to PortfolioResource and FcmResource (sync + async), applies @deprecated to spec-marked multivariate endpoints, adds lookback_seconds enum validation, and extends orders.list/list_all event_ticker to accept list[str] per MultipleEventTickerQuery. Also includes a substantial formatting-only cleanup pass across the test suite.

---

✅ What's done well

• Pattern fidelity. positions_all() on both resources follows the existing settlements_all / orders_all template exactly — _require_auth() + _validate_max_pages() before _list_all(), cursor excluded via PAGINATOR_SKIP_LIST, and a clear docstring explaining why event_positions are intentionally dropped.
• @deprecated placement. Applying the decorator to the implementation function (not the @overload stubs) is correct per PEP 702 — type checkers propagate the warning through overload resolution.
• filterwarnings in pyproject.toml. Suppressing the noisy DeprecationWarning in test output while still letting it surface in user code is the right trade-off, and the comment explains the rationale.
• _join_tickers max_items. Clean keyword-only addition that enforces the spec cap at the right layer. Skipping the cap for pre-joined strings is well-documented.
• Contract support. positions_all entries are correctly wired into METHOD_ENDPOINT_MAP, cursor exclusions, and PAGINATOR_SKIP_LIST for both sync and async variants.

---

🐛 Bugs / correctness issues

None found. The async positions_all() methods are correctly def (not async def) — consistent with the established pattern where _list_all returns an AsyncIterator directly.

---

🔍 Issues / suggestions

1. Missing async deprecation-warning tests (gap)

test_multivariate.py tests all three deprecation warnings on the sync class but has no equivalent tests for AsyncMultivariateCollectionsResource. The @deprecated decorator is present on the async implementations, but it's untested. A brief async test class (or parametrized approach) would close the gap symmetrically.

class TestAsyncMultivariateDeprecationWarnings:
    @pytest.mark.asyncio
    async def test_lookup_history_emits_deprecation_warning(self, async_mv): ...
    @pytest.mark.asyncio
    async def test_lookup_tickers_emits_deprecation_warning(self, async_mv): ...
    @pytest.mark.asyncio
    async def test_create_market_emits_deprecation_warning(self, async_mv): ...

2. test_positions_all_forwards_filters in fcm doesn't verify limit

TestPositionsAll.test_positions_all_forwards_filters passes limit=50 but the assertion block doesn't check that url.params["limit"] == "50". Compare with TestPortfolioPositionsAll.test_positions_all_forwards_filters_and_omits_cursor, which does assert params["limit"] == "100". Minor inconsistency worth fixing for parity.

3. test_list_all_rejects_event_ticker_list_over_spec_max — comment says "eager" but doesn't explicitly verify it

The test comment says "Eager validation — must fire at call time, not on iteration", but the test just calls orders.list_all(...) without asserting the ValueError fires before any HTTP is dispatched. Adding a respx.mock route and asserting route.call_count == 0 after the raise would make the eagerness claim machine-verified rather than just documented.

4. typing-extensions lower bound

>=4.5 is correct — deprecated was introduced in 4.5.0. Worth a quick check that the project's minimum Python version aligns (4.5 requires Python ≥ 3.8, which should be fine).

---

📋 Nit / style

• The formatting-only churn across test files (collapsing parenthesized strings, trailing commas) is functionally harmless but makes the diff harder to review. Worth splitting into a separate polish commit or PR to keep functional changes readable — though this may already be intentional given the PR title.
• _validate_lookback_seconds returns the validated int value, which is a clean pattern. Consistent with how _validate_limit works elsewhere.

---

🔒 Security

No concerns. No new endpoints, no user-input paths, no serialization changes outside the established patterns.

---

Overall

The functional additions are correct, well-tested (sync side), and follow conventions closely. The two coverage gaps (async deprecation tests, fcm limit assertion) are minor but worth addressing before merge. Everything else is ✅.