You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Surfaced by the multi-LLM review of PR #403 (perps/margin API, branch feat/perps-api). This is a deferred follow-up to the Klear margin work; the headline review findings were already fixed in f8cc960 / da953b0 / f2ef973, this one was not.
Problem
_validate_date_range in kalshi/perps/klear/resources/margin.py:51-69 validates start_date / end_date with datetime.date.fromisoformat() (lines 59-60) but then forwards the original caller-supplied strings to the wire unchanged via _params(start_date=start_date, end_date=end_date) (sync margin_reports, line 88; async margin_reports, line 252).
On Python 3.11+ datetime.date.fromisoformat() was widened to accept the full ISO 8601 grammar, not just canonical YYYY-MM-DD. The project requires >=3.12 (pyproject.toml:7) and runs on 3.12, where I confirmed:
Both pass SDK validation, so margin_reports(start_date="20260605", end_date=...) (or a W-week-date) sails through _validate_date_range and is sent to GET /margin/reportsverbatim as start_date=20260605. The endpoint's spec is documented as YYYY-MM-DD (module docstring, lines 6-7), so a strict server rejects the request with an opaque 400 — defeating the entire stated purpose of the boundary guard ("surfaces a clear error instead of an opaque server 400", lines 55-57). Worse, a lenient server could silently interpret 2026-W23-5 differently than the user intended.
The existing regression test test_rejects_malformed_or_inverted_dates_client_side (tests/perps/klear/test_margin.py:192-206) only covers "nope" and an inverted range; it does not exercise the non-canonical-but-parseable forms, so this gap is untested.
Proposed fix
In _validate_date_range, after parsing, assert the input round-trips to its canonical form and reject otherwise. Minimal change at kalshi/perps/klear/resources/margin.py:58-65:
try:
start=datetime.date.fromisoformat(start_date)
end=datetime.date.fromisoformat(end_date)
exceptValueErrorasexc:
raiseValueError(
f"start_date / end_date must be YYYY-MM-DD strings (got "f"start_date={start_date!r}, end_date={end_date!r}): {exc}"
) fromexcforlabel, raw, parsedin (("start_date", start_date, start), ("end_date", end_date, end)):
ifparsed.isoformat() !=raw:
raiseValueError(
f"{label} must be canonical YYYY-MM-DD (got {raw!r}; did you mean {parsed.isoformat()!r}?)"
)
date.isoformat() always emits canonical YYYY-MM-DD, so "20260605" and "2026-W23-5" are rejected while "2026-06-05" passes. This keeps the existing ValueError/YYYY-MM-DD message contract the current test matches on.
Acceptance criteria
_validate_date_range rejects non-canonical ISO forms ("20260605", "2026-W23-5") with a ValueError, before any HTTP call.
Canonical YYYY-MM-DD input still passes and is forwarded unchanged.
Both sync and async margin_reports are covered (they share _validate_date_range, so one guard fix covers both).
Add regression test(s) in tests/perps/klear/test_margin.py (extend test_rejects_malformed_or_inverted_dates_client_side or add a sibling) asserting:
start_date="20260605" raises and route.called is False.
A W-week-date ("2026-W23-5") raises and route.called is False.
Existing "2026-05-01" / "2026-06-01" happy path still sends start_date=2026-05-01 verbatim.
Notes
Severity is low: it only bites callers passing non-canonical ISO strings, and the failure mode is a server 400 rather than a correctness/money bug. Deferred because the PR-403 headline fixes (ws/klear/rest) took priority and this is a boundary-hardening polish item, not a regression.
Alternative considered: accept a datetime.date and always serialize via .isoformat(). That is cleaner long-term but changes the public signature (currently str-typed start_date/end_date) and would need the same treatment across the perps surface — out of scope for a minimal fix here; the round-trip assertion is the surgical option.
Context
Surfaced by the multi-LLM review of PR #403 (perps/margin API, branch
feat/perps-api). This is a deferred follow-up to the Klear margin work; the headline review findings were already fixed in f8cc960 / da953b0 / f2ef973, this one was not.Problem
_validate_date_rangeinkalshi/perps/klear/resources/margin.py:51-69validatesstart_date/end_datewithdatetime.date.fromisoformat()(lines 59-60) but then forwards the original caller-supplied strings to the wire unchanged via_params(start_date=start_date, end_date=end_date)(syncmargin_reports, line 88; asyncmargin_reports, line 252).On Python 3.11+
datetime.date.fromisoformat()was widened to accept the full ISO 8601 grammar, not just canonicalYYYY-MM-DD. The project requires>=3.12(pyproject.toml:7) and runs on 3.12, where I confirmed:Both pass SDK validation, so
margin_reports(start_date="20260605", end_date=...)(or aW-week-date) sails through_validate_date_rangeand is sent toGET /margin/reportsverbatim asstart_date=20260605. The endpoint's spec is documented asYYYY-MM-DD(module docstring, lines 6-7), so a strict server rejects the request with an opaque 400 — defeating the entire stated purpose of the boundary guard ("surfaces a clear error instead of an opaque server 400", lines 55-57). Worse, a lenient server could silently interpret2026-W23-5differently than the user intended.The existing regression test
test_rejects_malformed_or_inverted_dates_client_side(tests/perps/klear/test_margin.py:192-206) only covers"nope"and an inverted range; it does not exercise the non-canonical-but-parseable forms, so this gap is untested.Proposed fix
In
_validate_date_range, after parsing, assert the input round-trips to its canonical form and reject otherwise. Minimal change atkalshi/perps/klear/resources/margin.py:58-65:date.isoformat()always emits canonicalYYYY-MM-DD, so"20260605"and"2026-W23-5"are rejected while"2026-06-05"passes. This keeps the existingValueError/YYYY-MM-DDmessage contract the current test matches on.Acceptance criteria
_validate_date_rangerejects non-canonical ISO forms ("20260605","2026-W23-5") with aValueError, before any HTTP call.YYYY-MM-DDinput still passes and is forwarded unchanged.margin_reportsare covered (they share_validate_date_range, so one guard fix covers both).tests/perps/klear/test_margin.py(extendtest_rejects_malformed_or_inverted_dates_client_sideor add a sibling) asserting:start_date="20260605"raises androute.called is False.W-week-date ("2026-W23-5") raises androute.called is False."2026-05-01"/"2026-06-01"happy path still sendsstart_date=2026-05-01verbatim.Notes
datetime.dateand always serialize via.isoformat(). That is cleaner long-term but changes the public signature (currentlystr-typedstart_date/end_date) and would need the same treatment across the perps surface — out of scope for a minimal fix here; the round-trip assertion is the surgical option.