From 2b31f0cd047b27fd60e820510bc58613b706fc3e Mon Sep 17 00:00:00 2001 From: Jeff West Date: Fri, 22 May 2026 08:25:33 -0500 Subject: [PATCH] fix(types): NaN/Inf guard; None-first nested col; request price bounds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit to_decimal accepted NaN/Infinity despite docstring promising safety; now delegates to _coerce_decimal so the public helper and the DollarDecimal / FixedPointCount validators share the same is_finite() guard (#325). Page._columns peeked only col[0] to decide whether to dump nested BaseModel cells. An Optional[NestedModel] column whose first row is None (common across Market/Event/Series/Settlement nullable struct columns) skipped the dump pass and leaked raw BaseModel instances into pandas/polars, breaking the polars Struct dtype the docstring promises. Now probes the whole column for the first non-None BaseModel (#328). Request-side DollarDecimal price fields had no sign or tick bounds — Decimal('-0.65') or Decimal('0.12345') constructed cleanly and round-tripped to a server 400. Introduce an OrderPrice annotated alias (BeforeValidator coerce + AfterValidator non-negative + $0.0001 tick) applied to CreateOrderRequest / AmendOrderRequest yes_price/no_price and CreateOrderV2Request / AmendOrderV2Request price. Response-side fields (PnL/fee/settlement averages) keep bare DollarDecimal since negatives are legitimate there (#343). Closes #325, #328, #343 --- kalshi/models/common.py | 8 +- kalshi/models/orders.py | 14 ++-- kalshi/types.py | 63 ++++++++++++--- tests/test_models.py | 167 +++++++++++++++++++++++++++++++++++++++- tests/test_types.py | 29 +++++++ 5 files changed, 263 insertions(+), 18 deletions(-) diff --git a/kalshi/models/common.py b/kalshi/models/common.py index 097c68e..1e84505 100644 --- a/kalshi/models/common.py +++ b/kalshi/models/common.py @@ -82,7 +82,13 @@ def _columns(self) -> dict[str, list[object]]: columns: dict[str, list[object]] = {} for field in fields: col: list[object] = [getattr(item, field) for item in self.items] - if col and isinstance(col[0], BaseModel): + # Probe the whole column for the first non-None ``BaseModel`` rather + # than peeking only ``col[0]``: an ``Optional[NestedModel]`` field + # whose first row is ``None`` (common across Market / Event / Series + # nullable struct columns) would otherwise skip the dump pass and + # leak raw ``BaseModel`` instances into pandas / polars, breaking + # the polars Struct inference the docstring promises (#328). + if next((v for v in col if isinstance(v, BaseModel)), None) is not None: col = [v.model_dump(mode="python") if isinstance(v, BaseModel) else v for v in col] columns[field] = col return columns diff --git a/kalshi/models/orders.py b/kalshi/models/orders.py index a78595b..3f549ef 100644 --- a/kalshi/models/orders.py +++ b/kalshi/models/orders.py @@ -7,7 +7,7 @@ from pydantic import AliasChoices, AwareDatetime, BaseModel, Field, field_validator, model_validator -from kalshi.types import DollarDecimal, FixedPointCount, StrictInt +from kalshi.types import DollarDecimal, FixedPointCount, OrderPrice, StrictInt # Literal aliases for fixed-enum kwargs on order resource methods and the # matching V1 / V2 request models. @@ -175,11 +175,11 @@ class CreateOrderRequest(BaseModel): side: SideLiteral action: ActionLiteral count: FixedPointCount = Field(serialization_alias="count_fp") - yes_price: DollarDecimal | None = Field( + yes_price: OrderPrice | None = Field( default=None, serialization_alias="yes_price_dollars", ) - no_price: DollarDecimal | None = Field( + no_price: OrderPrice | None = Field( default=None, serialization_alias="no_price_dollars", ) @@ -253,11 +253,11 @@ class AmendOrderRequest(BaseModel): ticker: str side: SideLiteral action: ActionLiteral - yes_price: DollarDecimal | None = Field( + yes_price: OrderPrice | None = Field( default=None, serialization_alias="yes_price_dollars", ) - no_price: DollarDecimal | None = Field( + no_price: OrderPrice | None = Field( default=None, serialization_alias="no_price_dollars", ) @@ -473,7 +473,7 @@ class CreateOrderV2Request(BaseModel): client_order_id: str side: BookSideLiteral count: FixedPointCount - price: DollarDecimal + price: OrderPrice time_in_force: TimeInForceLiteral self_trade_prevention_type: SelfTradePreventionTypeLiteral expiration_time: StrictInt | None = None @@ -557,7 +557,7 @@ class AmendOrderV2Request(BaseModel): ticker: str side: BookSideLiteral - price: DollarDecimal + price: OrderPrice count: FixedPointCount client_order_id: str | None = None updated_client_order_id: str | None = None diff --git a/kalshi/types.py b/kalshi/types.py index a30e309..808fd7e 100644 --- a/kalshi/types.py +++ b/kalshi/types.py @@ -5,7 +5,7 @@ from decimal import Decimal from typing import Annotated, Any, TypeVar -from pydantic import BeforeValidator, PlainSerializer +from pydantic import AfterValidator, BeforeValidator, PlainSerializer T = TypeVar("T") @@ -96,15 +96,13 @@ def to_decimal(value: int | float | str | Decimal) -> Decimal: Always goes through str() to avoid float representation errors. e.g., to_decimal(0.65) returns Decimal("0.65"), not Decimal(0.65). + + Rejects ``bool`` inputs (#225 pattern) and non-finite values + (``NaN``/``Infinity``); delegates to :func:`_coerce_decimal` so the + public helper and the :data:`DollarDecimal` / :data:`FixedPointCount` + validators share a single guard (#325). """ - if isinstance(value, bool): - raise TypeError( - "Cannot convert bool to Decimal — bool is an int subclass, " - "so this is almost always a typo (did you mean count=1?)." - ) - if isinstance(value, Decimal): - return value - return Decimal(str(value)) + return _coerce_decimal(value) def _none_to_empty_list(value: Any) -> Any: @@ -153,3 +151,50 @@ def _reject_bool_int(value: object) -> object: StrictInt = Annotated[int, BeforeValidator(_reject_bool_int)] """``int`` that rejects ``bool`` — use on every Request-model integer field. See #295.""" + + +_REQUEST_PRICE_TICK = Decimal("0.0001") + + +def _ensure_request_price(value: Decimal) -> Decimal: + """Validate a request-side dollar price (#343). + + ``DollarDecimal`` is shared with response-side fields (PnL, fees, + settlements) where negatives and arbitrary precision are legitimate. + Request-side price fields, however, are bounded by Kalshi's order + contract: prices must be non-negative and aligned to the $0.0001 + tick. Catching both at construction (rather than as a server-side + 400) matches the boundary-validation pattern that ``StrictInt`` / + ``Literal`` / ``_reject_bool_int`` apply elsewhere. + """ + if value < 0: + raise ValueError( + f"Order price must be non-negative (got {value}); request-side " + "DollarDecimal fields reject negatives. Negative prices are " + "legitimate only on response-side PnL/fee/settlement fields." + ) + if value != value.quantize(_REQUEST_PRICE_TICK): + raise ValueError( + f"Order price {value} is finer than the $0.0001 tick; round " + "to four decimal places (e.g. Decimal('0.5600')) before " + "constructing the request." + ) + return value + + +OrderPrice = Annotated[ + Decimal, + BeforeValidator(_coerce_decimal), + AfterValidator(_ensure_request_price), + PlainSerializer(_decimal_to_str, return_type=str, when_used="json"), +] +"""Request-side dollar price with sign and $0.0001 tick guards (#343). + +Same wire shape as :data:`DollarDecimal` (BeforeValidator coerces to +``Decimal`` without float drift, PlainSerializer emits a fixed-point +string on ``mode="json"``), but adds an :class:`AfterValidator` that +rejects negatives and sub-tick precision. Use on +``CreateOrderRequest`` / ``AmendOrderRequest`` / V2 equivalents so a +caller passing ``Decimal('-0.65')`` or ``Decimal('0.12345')`` fails at +construction instead of round-tripping to a server 400. +""" diff --git a/tests/test_models.py b/tests/test_models.py index 49d9d41..2904444 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -7,7 +7,7 @@ from typing import ClassVar import pytest -from pydantic import ValidationError +from pydantic import BaseModel, ValidationError from kalshi.models.communications import RFQ, Quote from kalshi.models.events import Event, EventMetadata @@ -1820,3 +1820,168 @@ def test_issue_326_v1_subaccount_ge_zero( # Zero (primary subaccount / first shard) remains valid. instance = model_cls(**{**other_kwargs, field_name: 0}) assert getattr(instance, field_name) == 0 + + +class TestIssue328PageColumnsNoneFirstNested: + """#328: ``Page._columns`` peeked only ``col[0]`` to decide the dump branch. + + An ``Optional[NestedModel]`` field whose first row is ``None`` (common + across nullable struct columns on Market / Event / Series / Settlement) + skipped the per-cell ``model_dump`` pass — the second row's populated + nested model leaked out as a raw ``BaseModel`` instance, breaking the + polars ``Struct`` dtype the docstring promises (#264). + """ + + def test_issue_328_page_columns_handles_none_first_nested(self) -> None: + from kalshi.models.common import Page + + class _Inner(BaseModel): + label: str + + class _Row(BaseModel): + ticker: str + inner: _Inner | None = None + + page: Page[_Row] = Page( + items=[ + _Row(ticker="A", inner=None), + _Row(ticker="B", inner=_Inner(label="x")), + ], + cursor=None, + ) + + cols = page._columns() + assert cols["ticker"] == ["A", "B"] + # Pre-fix: cols["inner"] == [None, _Inner(label="x")] (raw BaseModel leaks) + # Post-fix: the populated row dumps to a dict; the None row stays None. + assert cols["inner"][0] is None + assert cols["inner"][1] == {"label": "x"} + + def test_issue_328_page_to_polars_yields_struct_for_none_first_nested(self) -> None: + pl = pytest.importorskip("polars") + + from kalshi.models.common import Page + + class _Inner(BaseModel): + label: str + + class _Row(BaseModel): + ticker: str + inner: _Inner | None = None + + page: Page[_Row] = Page( + items=[ + _Row(ticker="A", inner=None), + _Row(ticker="B", inner=_Inner(label="x")), + ], + cursor=None, + ) + + df = page.to_polars() + assert isinstance(df["inner"].dtype, pl.Struct) + + +class TestIssue343DollarDecimalRequestBounds: + """#343: request-side ``DollarDecimal`` fields lacked sign and tick bounds. + + The shared :data:`DollarDecimal` alias is intentionally permissive for + response-side PnL/fee/settlement fields where negatives and arbitrary + precision are legitimate. Request-side price fields, however, are bounded + by Kalshi's order contract: non-negative and aligned to the $0.0001 tick. + The :data:`OrderPrice` alias applied on CreateOrderRequest / AmendOrderRequest + / CreateOrderV2Request / AmendOrderV2Request enforces both at construction. + """ + + def test_issue_343_dollar_decimal_request_rejects_negative_and_invalid_tick(self) -> None: + from kalshi.models.orders import ( + AmendOrderRequest, + AmendOrderV2Request, + CreateOrderRequest, + CreateOrderV2Request, + ) + + # V1 CreateOrderRequest: yes_price / no_price negative -> ValidationError + with pytest.raises(ValidationError, match="non-negative"): + CreateOrderRequest( + ticker="T", side="yes", action="buy", + count=Decimal("1"), yes_price=Decimal("-0.65"), + ) + with pytest.raises(ValidationError, match="non-negative"): + CreateOrderRequest( + ticker="T", side="no", action="buy", + count=Decimal("1"), no_price=Decimal("-0.01"), + ) + + # Sub-tick precision -> ValidationError + with pytest.raises(ValidationError, match=r"\$0\.0001 tick"): + CreateOrderRequest( + ticker="T", side="yes", action="buy", + count=Decimal("1"), yes_price=Decimal("0.12345"), + ) + + # V1 AmendOrderRequest mirrors V1 Create. + with pytest.raises(ValidationError, match="non-negative"): + AmendOrderRequest( + ticker="T", side="yes", action="buy", + yes_price=Decimal("-0.5"), + ) + with pytest.raises(ValidationError, match=r"\$0\.0001 tick"): + AmendOrderRequest( + ticker="T", side="no", action="sell", + no_price=Decimal("0.99999"), + ) + + # V2 CreateOrderV2Request. + with pytest.raises(ValidationError, match="non-negative"): + CreateOrderV2Request( + ticker="T", client_order_id="c1", side="bid", + count=Decimal("1"), price=Decimal("-0.5"), + time_in_force="good_till_canceled", + self_trade_prevention_type="taker_at_cross", + ) + with pytest.raises(ValidationError, match=r"\$0\.0001 tick"): + CreateOrderV2Request( + ticker="T", client_order_id="c1", side="bid", + count=Decimal("1"), price=Decimal("0.55555"), + time_in_force="good_till_canceled", + self_trade_prevention_type="taker_at_cross", + ) + + # V2 AmendOrderV2Request. + with pytest.raises(ValidationError, match="non-negative"): + AmendOrderV2Request( + ticker="T", side="ask", + price=Decimal("-0.01"), count=Decimal("1"), + ) + with pytest.raises(ValidationError, match=r"\$0\.0001 tick"): + AmendOrderV2Request( + ticker="T", side="ask", + price=Decimal("0.12345"), count=Decimal("1"), + ) + + def test_issue_343_accepts_zero_and_tick_aligned_prices(self) -> None: + from kalshi.models.orders import CreateOrderRequest + + # Zero is non-negative; round-tick four-decimal value is aligned. + req = CreateOrderRequest( + ticker="T", side="yes", action="buy", + count=Decimal("1"), yes_price=Decimal("0"), + ) + assert req.yes_price == Decimal("0") + + req = CreateOrderRequest( + ticker="T", side="yes", action="buy", + count=Decimal("1"), yes_price=Decimal("0.5600"), + ) + assert req.yes_price == Decimal("0.5600") + + def test_issue_343_response_dollar_decimal_unchanged(self) -> None: + # Response-side averages (V2) keep bare DollarDecimal — negatives must + # remain legal for PnL/fee/settlement parity. Pin the contract. + from kalshi.models.orders import CreateOrderV2Response + + resp = CreateOrderV2Response.model_validate({ + "order_id": "o", "fill_count": "1", "remaining_count": "0", + "ts_ms": 0, "average_fill_price": "-0.0001", + }) + assert resp.average_fill_price == Decimal("-0.0001") diff --git a/tests/test_types.py b/tests/test_types.py index b1672ce..e81337d 100644 --- a/tests/test_types.py +++ b/tests/test_types.py @@ -129,3 +129,32 @@ def test_create_order_request_rejects_nan_yes_price(self) -> None: count=Decimal("1"), yes_price=Decimal("NaN"), ) + + +class TestIssue325ToDecimalRejectsNanInf: + """#325: ``to_decimal`` accepted NaN/Infinity while ``_coerce_decimal`` rejected them. + + The public helper is documented as the safe constructor; users following the + pattern of pre-coercing with ``to_decimal`` and then handing the result to + code paths that bypass pydantic re-validation (direct Decimal arithmetic, + hand-built dict + ``model_dump_json``) would otherwise ship the literal + string ``"NaN"`` to a real-money endpoint. + """ + + def test_issue_325_to_decimal_rejects_nan_inf(self) -> None: + with pytest.raises(ValueError, match="finite"): + to_decimal(Decimal("NaN")) + with pytest.raises(ValueError, match="finite"): + to_decimal(Decimal("Infinity")) + with pytest.raises(ValueError, match="finite"): + to_decimal(Decimal("-Infinity")) + with pytest.raises(ValueError, match="finite"): + to_decimal("NaN") + with pytest.raises(ValueError, match="finite"): + to_decimal(float("nan")) + with pytest.raises(ValueError, match="finite"): + to_decimal(float("inf")) + + def test_to_decimal_preserves_finite_decimal_identity(self) -> None: + d = Decimal("0.65") + assert to_decimal(d) is d