perps: margin-account list fields use bare list[...] instead of NullableList (null-tolerance inconsistency); roe sub-finding is a false positive #407

Context

Surfaced by the multi-LLM review of PR #403 (perps/margin API, EPIC #387). Two perps response-model strictness sub-findings were raised; on verification only one holds. This issue tracks the real one (margin-account list fields not using NullableList) and records why the roe sub-finding is a false positive so it isn't re-filed.

Problem

Sub-finding (1) — MarginPosition.return_on_equity (roe): FALSE POSITIVE, no change needed.
The finding claimed the spec marks roe "required-but-nullable", so default=None wrongly makes the key optional. That is not what the spec says. In specs/perps_openapi.yaml the MarginPosition.required list (lines 2147-2153) is exactly market_ticker, position, entry_price, unrealized_pnl, margin_used, fees — roe is not listed; it is only nullable: true (lines 2177-2183). So roe is genuinely optional in the spec, and the current model is correct:

kalshi/perps/models/portfolio.py:55-58

return_on_equity: float | None = Field(
    default=None,
    validation_alias=AliasChoices("roe", "return_on_equity"),
)

Making roe required (dropping default=None) would be spec-incorrect: it would reject valid responses that legitimately omit roe. It would also NOT be required by the contract test — TestPerps*Drift.test_required_drift (tests/test_contracts.py:780-795, a hard-fail since #172) only iterates the spec's required set (line 501), and roe isn't in it, so it is never checked. No action on (1).

Sub-finding (2) — bare list[...] on margin-account array fields: REAL (low severity).
kalshi/perps/models/margin_account.py uses bare list[...] for two required array fields:

• GetMarginBalanceResponse.subaccount_balances: list[MarginSubaccountBalance] — line 44
• GetMarginRiskResponse.positions: list[MarginRiskPosition] — line 71

Every sibling perps response array uses NullableList[...] (coerces a server-returned JSON null to []): kalshi/perps/models/portfolio.py:69,96,122 (positions/fills/trades), kalshi/perps/models/markets.py:93-94 (bids/asks), kalshi/perps/models/order_groups.py:43, and kalshi/perps/klear/models/margin.py:108,173-175,199,319. The bare-list fields here are the lone exception. If the demo/prod API ever returns null for subaccount_balances or positions (the documented motivation for NullableList, see kalshi/types.py:108-124), parsing raises ValidationError and the whole balance() / risk() call fails, whereas the adjacent endpoints degrade gracefully to [].

Existing tests (tests/perps/test_margin_account.py) only exercise empty []/{}, never a server null, so the gap is uncovered.

Scope correction vs. the original finding: the finding also named the dict[...] maps (maker_fee_rates/taker_fee_rates at lines 92-93, notional_value_risk_limits_by_market_ticker at line 80) as NullableList candidates. NullableList is list-only — there is no NullableDict in the codebase, and the established convention is to leave map fields as bare dict[...] (e.g. kalshi/models/search.py:37, kalshi/perps/models/exchange.py:44). So the dict maps are out of scope for a NullableList swap; only the two genuine list[...] fields should change.

Proposed fix

Minimal, list-fields-only change in kalshi/perps/models/margin_account.py:

1. Import NullableList from kalshi.types (alongside the existing DollarDecimal, FixedPointCount).
2. subaccount_balances: list[MarginSubaccountBalance] → NullableList[MarginSubaccountBalance] (line 44).
3. positions: list[MarginRiskPosition] → NullableList[MarginRiskPosition] (line 71).

Leave the three dict[...] map fields as-is (no NullableDict convention exists; do not invent one in this issue).

Acceptance criteria

• subaccount_balances and positions in kalshi/perps/models/margin_account.py use NullableList[...]; the three dict[...] fields are unchanged.
• mypy kalshi/ passes (NullableList[T] resolves to list[T], so downstream len()/indexing typing is unchanged).
• Regression test in tests/perps/test_margin_account.py: balance() with {"subaccount_balances": null, "settled_funds": "0.0000"} parses and yields resp.subaccount_balances == [].
• Regression test: risk() with positions set to JSON null parses and yields resp.positions == [].
• Existing empty-array/empty-map and happy-path tests still pass (no behavior change for []/populated arrays).
• No roe change is made (sub-finding 1 is a non-issue).

Notes

• Severity is low: purely defensive null-tolerance to match sibling perps responses; only bites if the API returns null for these specific arrays (not yet observed). Deferred from PR perps: full Perps (margin) API — REST + WebSocket + SCM/Klear (#387) #403 because the headline review items shipped in f8cc960 / da953b0 / f2ef973 and this is cosmetic robustness, not a correctness bug.
• This is a non-breaking change: callers already receive a list; null previously raised, now yields [].
• The roe sub-finding rested on a misread of the spec's required block — documented above so it isn't re-filed. Related: perps EPIC perps: [EPIC] Perps (margin) API — full SDK implementation tracker #387.
• Caveat: needs verification that the demo/prod API can actually return null here; absent that, this is consistency hardening rather than a fix for an observed failure.