Skip to content

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

Closed

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.

Activity

  1. added
    polishCode-quality and DX improvements; non-functional
    perpsPerps / margin (perpetual futures) API
    on Jun 5, 2026
  2. added a commit that references this issue on Jun 5, 2026
    4373a2c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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