Skip to content

perps: klear margin-report date validation accepts non-canonical ISO forms and forwards them to the wire #409

Description

@TexasCoding

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_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:

>>> datetime.date.fromisoformat("20260605")    # basic (no-dash) form
datetime.date(2026, 6, 5)
>>> datetime.date.fromisoformat("2026-W23-5")  # ISO week date
datetime.date(2026, 6, 5)

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/reports verbatim 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)
except ValueError as exc:
    raise ValueError(
        f"start_date / end_date must be YYYY-MM-DD strings (got "
        f"start_date={start_date!r}, end_date={end_date!r}): {exc}"
    ) from exc
for label, raw, parsed in (("start_date", start_date, start), ("end_date", end_date, end)):
    if parsed.isoformat() != raw:
        raise ValueError(
            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.
  • Part of the perps EPIC perps: [EPIC] Perps (margin) API — full SDK implementation tracker #387.

Metadata

Metadata

Assignees

No one assigned

    Labels

    perpsPerps / margin (perpetual futures) APIpolishCode-quality and DX improvements; non-functional

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions