From bf4612d32b678d3b6416971bfa8606239214f8df Mon Sep 17 00:00:00 2001 From: worker Date: Thu, 21 May 2026 06:27:20 -0500 Subject: [PATCH 1/2] fix(orders): require count + action on create() kwarg path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pre-#242 the kwarg-form `client.orders.create(ticker=..., side='yes')` silently defaulted missing `count` to 1 and missing `action` to 'buy'. For a real-money SDK that converted a missing-arg bug into a real 1-contract BUY fill — money risk. Drop the silent defaults on both sync and async kwarg paths and raise `TypeError` before any HTTP request when either is None. Mirror the spec at the model level by removing the `Decimal('1')` default on `CreateOrderRequest.count` (spec lists `count` as required); the `action` field was already required since #172. The `request=CreateOrderRequest(...)` overload path is unaffected: the model now declares both required so a fully-populated request still dispatches normally. BREAKING CHANGE: callers relying on `orders.create(ticker, side)` defaulting to 1-contract buy must now pass `count=` and `action=` explicitly (or build a `CreateOrderRequest`). This is technically breaking but money-risk-driven. Closes #242 --- kalshi/models/orders.py | 10 +-- kalshi/resources/orders.py | 26 ++++++-- tests/test_async_orders.py | 103 +++++++++++++++++++++++++++---- tests/test_models.py | 19 +++++- tests/test_orders.py | 100 ++++++++++++++++++++++++++---- tests/test_request_overload.py | 10 +-- tests/ws/test_count_migration.py | 12 ++-- 7 files changed, 236 insertions(+), 44 deletions(-) diff --git a/kalshi/models/orders.py b/kalshi/models/orders.py index cf45b26..83bfcad 100644 --- a/kalshi/models/orders.py +++ b/kalshi/models/orders.py @@ -160,9 +160,11 @@ class CreateOrderRequest(BaseModel): it. Callers passing ``type="market"`` (or similar) now get a ``ValidationError`` at construction time. - ``ticker``, ``side``, and ``action`` are all required by the spec. - Pre-v2.3.0 the SDK defaulted ``action`` to ``"buy"`` as a convenience; - that default has been removed to match the spec required-set (#172). + ``ticker``, ``side``, ``action``, and ``count`` are all required by the + spec. Pre-v2.3.0 the SDK defaulted ``action`` to ``"buy"`` (#172); the + follow-up (#242) also removed the silent ``count=1`` default — both are + money-risk drivers, since a missing arg would otherwise translate into + a real 1-contract BUY rather than a clear error. See ``kalshi.resources.orders.OrdersResource.create`` for the user-facing method that builds this model internally. @@ -171,7 +173,7 @@ class CreateOrderRequest(BaseModel): ticker: str side: str action: str - count: FixedPointCount = Field(default=Decimal("1"), serialization_alias="count_fp") + count: FixedPointCount = Field(serialization_alias="count_fp") yes_price: DollarDecimal | None = Field( default=None, serialization_alias="yes_price_dollars", diff --git a/kalshi/resources/orders.py b/kalshi/resources/orders.py index fa10b11..c0f1509 100644 --- a/kalshi/resources/orders.py +++ b/kalshi/resources/orders.py @@ -95,15 +95,19 @@ def _build_create_order_body( exchange_index=exchange_index, ) if request is None: - if ticker is None or side is None: + if ticker is None or side is None or count is None or action is None: raise TypeError( - "create() requires `ticker` and `side` (or pass `request=...`)" + "create() requires `ticker`, `side`, `count`, and `action` " + "(or pass `request=...`). Pre-#242 the SDK silently defaulted " + "missing `count` to 1 contract and missing `action` to \"buy\" — " + "that has been removed: a missing arg would otherwise translate " + "into a real 1-contract BUY on the wire." ) request = CreateOrderRequest( ticker=ticker, side=side, - action=action if action is not None else "buy", - count=to_decimal(count if count is not None else 1), + action=action, + count=to_decimal(count), yes_price=to_decimal(yes_price) if yes_price is not None else None, no_price=to_decimal(no_price) if no_price is not None else None, client_order_id=client_order_id, @@ -372,6 +376,13 @@ def create( the OpenAPI spec. Callers passing ``type="limit"`` now get a ``TypeError``. + #242 (v2.5): on the kwarg path, ``count`` and ``action`` are now + REQUIRED — passing neither raises ``TypeError`` before any HTTP + request. Previously the SDK silently defaulted to ``count=1`` and + ``action="buy"``, which converted a missing-arg bug into a real + 1-contract BUY fill. The ``request=CreateOrderRequest(...)`` + overload is unaffected (the model itself now declares them required). + v1.1 (#56): pass a pre-built ``request=CreateOrderRequest(...)`` instead of individual kwargs. Mutually exclusive with the kwarg form. """ @@ -834,6 +845,13 @@ async def create( the OpenAPI spec. Callers passing ``type="limit"`` now get a ``TypeError``. + #242 (v2.5): on the kwarg path, ``count`` and ``action`` are now + REQUIRED — passing neither raises ``TypeError`` before any HTTP + request. Previously the SDK silently defaulted to ``count=1`` and + ``action="buy"``, which converted a missing-arg bug into a real + 1-contract BUY fill. The ``request=CreateOrderRequest(...)`` + overload is unaffected (the model itself now declares them required). + v1.1 (#56): pass a pre-built ``request=CreateOrderRequest(...)`` instead of individual kwargs. Mutually exclusive with the kwarg form. """ diff --git a/tests/test_async_orders.py b/tests/test_async_orders.py index 57dae8a..c1bd9e6 100644 --- a/tests/test_async_orders.py +++ b/tests/test_async_orders.py @@ -81,7 +81,7 @@ async def test_no_phantom_type_in_wire( return_value=httpx.Response(200, json={"order": _MINIMAL_ORDER}) ) - await client.orders.create(ticker="MKT", side="yes", yes_price=0.5) + await client.orders.create(ticker="MKT", side="yes", action="buy", count=1, yes_price=0.5) body = json.loads(route.calls[0].request.content) assert "type" not in body @@ -97,7 +97,7 @@ async def test_count_fp_not_count_in_wire( return_value=httpx.Response(200, json={"order": _MINIMAL_ORDER}) ) - await client.orders.create(ticker="MKT", side="yes", yes_price=0.5, count=3) + await client.orders.create(ticker="MKT", side="yes", action="buy", yes_price=0.5, count=3) body = json.loads(route.calls[0].request.content) assert "count_fp" in body @@ -118,6 +118,8 @@ async def test_time_in_force_reaches_wire( await client.orders.create( ticker="MKT", side="yes", + action="buy", + count=1, yes_price=0.5, time_in_force="fill_or_kill", ) @@ -139,6 +141,8 @@ async def test_post_only_reduce_only_reach_wire( await client.orders.create( ticker="MKT", side="yes", + action="buy", + count=1, yes_price=0.5, post_only=True, reduce_only=False, @@ -163,6 +167,8 @@ async def test_buy_max_cost_int_cents_wire( await client.orders.create( ticker="MKT", side="yes", + action="buy", + count=1, yes_price=0.5, buy_max_cost=500, ) @@ -185,6 +191,8 @@ async def test_subaccount_order_group_cancel_on_pause_stp_wire( await client.orders.create( ticker="MKT", side="yes", + action="buy", + count=1, yes_price=0.5, subaccount=2, order_group_id="grp-x", @@ -208,6 +216,79 @@ async def test_type_kwarg_removed(self, client: AsyncKalshiClient) -> None: type="market", # type: ignore[call-arg] ) + @respx.mock + @pytest.mark.asyncio + async def test_missing_count_and_action_raises_before_http( + self, + client: AsyncKalshiClient, + respx_mock: respx.MockRouter, + ) -> None: + """#242: async kwarg-form create() with missing ``count`` / ``action`` + must raise ``TypeError`` BEFORE any HTTP request is dispatched.""" + route = respx_mock.post( + "https://demo-api.kalshi.co/trade-api/v2/portfolio/orders" + ).mock(return_value=httpx.Response(200, json={"order": _MINIMAL_ORDER})) + + with pytest.raises(TypeError, match=r"count.*action"): + await client.orders.create(ticker="X", side="yes") + with pytest.raises(TypeError, match=r"count.*action"): + await client.orders.create(ticker="X", side="yes", action="buy") + with pytest.raises(TypeError, match=r"count.*action"): + await client.orders.create(ticker="X", side="yes", count=10) + + assert route.call_count == 0 + + @respx.mock + @pytest.mark.asyncio + async def test_explicit_count_action_kwargs_still_work( + self, + client: AsyncKalshiClient, + respx_mock: respx.MockRouter, + ) -> None: + """#242: regression — explicit ``count`` + ``action`` kwargs still build + and dispatch normally.""" + route = respx_mock.post( + "https://demo-api.kalshi.co/trade-api/v2/portfolio/orders" + ).mock(return_value=httpx.Response(200, json={"order": _MINIMAL_ORDER})) + + await client.orders.create( + ticker="X", side="yes", count=10, action="buy", yes_price="0.5", + ) + + assert route.call_count == 1 + body = json.loads(route.calls[0].request.content) + assert body["count_fp"] == "10" + assert body["action"] == "buy" + assert body["yes_price_dollars"] == "0.5" + + @respx.mock + @pytest.mark.asyncio + async def test_request_overload_unaffected_by_242( + self, + client: AsyncKalshiClient, + respx_mock: respx.MockRouter, + ) -> None: + """#242: ``request=CreateOrderRequest(...)`` path unaffected by the + kwarg-overload guard.""" + route = respx_mock.post( + "https://demo-api.kalshi.co/trade-api/v2/portfolio/orders" + ).mock(return_value=httpx.Response(200, json={"order": _MINIMAL_ORDER})) + + await client.orders.create( + request=CreateOrderRequest( + ticker="X", + side="yes", + count=Decimal("10"), + action="buy", + yes_price=Decimal("0.5"), + ) + ) + + assert route.call_count == 1 + body = json.loads(route.calls[0].request.content) + assert body["count_fp"] == "10" + assert body["action"] == "buy" + class TestAsyncOrdersCreate: @respx.mock @@ -228,7 +309,7 @@ async def test_create_limit_order(self, orders: AsyncOrdersResource) -> None: }, ) ) - order = await orders.create(ticker="TEST-MKT", side="yes", count=10, yes_price=0.65) + order = await orders.create(ticker="TEST-MKT", side="yes", action="buy", count=10, yes_price=0.65) assert order.order_id == "ord-123" assert order.yes_price == Decimal("0.6500") assert order.count == 10 @@ -244,7 +325,7 @@ async def test_create_order_no_price(self, orders: AsyncOrdersResource) -> None: }, ) ) - order = await orders.create(ticker="TEST-MKT", side="yes") + order = await orders.create(ticker="TEST-MKT", side="yes", action="buy", count=1) assert order.order_id == "ord-456" @respx.mock @@ -256,7 +337,7 @@ async def test_decimal_price_conversion(self, orders: AsyncOrdersResource) -> No json={"order": order_dict(order_id="ord-789", ticker="T")}, ) ) - await orders.create(ticker="T", side="yes", yes_price=0.65) + await orders.create(ticker="T", side="yes", action="buy", count=1, yes_price=0.65) body = json.loads(route.calls[0].request.content) assert body["yes_price_dollars"] == "0.65" @@ -269,7 +350,7 @@ async def test_validation_error(self, orders: AsyncOrdersResource) -> None: return_value=httpx.Response(400, json={"message": "invalid ticker"}) ) with pytest.raises(KalshiValidationError): - await orders.create(ticker="INVALID", side="yes") + await orders.create(ticker="INVALID", side="yes", action="buy", count=1) class TestAsyncOrdersGet: @@ -484,8 +565,8 @@ async def test_batch_create(self, orders: AsyncOrdersResource) -> None: ) ) reqs = [ - CreateOrderRequest(ticker="A", side="yes", action="buy"), - CreateOrderRequest(ticker="B", side="no", action="buy"), + CreateOrderRequest(ticker="A", side="yes", action="buy", count=1), + CreateOrderRequest(ticker="B", side="no", action="buy", count=1), ] from kalshi.models.orders import BatchCreateOrdersResponse @@ -527,7 +608,7 @@ async def test_batch_create_partial_failure_does_not_crash( ) ) result = await orders.batch_create( - [CreateOrderRequest(ticker="A", side="yes", action="buy")] + [CreateOrderRequest(ticker="A", side="yes", action="buy", count=1)] ) assert result.orders[0].order is not None assert result.orders[1].order is None @@ -1187,8 +1268,8 @@ async def test_wraps_orders_key(self, orders: AsyncOrdersResource) -> None: await orders.batch_create( [ - CreateOrderRequest(ticker="A", side="yes", action="buy"), - CreateOrderRequest(ticker="B", side="no", action="buy"), + CreateOrderRequest(ticker="A", side="yes", action="buy", count=1), + CreateOrderRequest(ticker="B", side="no", action="buy", count=1), ] ) diff --git a/tests/test_models.py b/tests/test_models.py index 17f18a3..c887c4d 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -221,6 +221,7 @@ def test_create_order_serializes_with_dollars_alias(self) -> None: ticker="T", side="yes", action="buy", + count=1, yes_price=Decimal("0.65"), ) data = req.model_dump(mode="json", exclude_none=True, by_alias=True) @@ -741,6 +742,7 @@ def test_accepts_time_in_force(self) -> None: ticker="MKT", side="yes", action="buy", + count=1, time_in_force="fill_or_kill", ) body = req.model_dump(exclude_none=True, by_alias=True) @@ -753,6 +755,7 @@ def test_accepts_post_only_and_reduce_only(self) -> None: ticker="MKT", side="yes", action="buy", + count=1, post_only=True, reduce_only=False, ) @@ -767,6 +770,7 @@ def test_accepts_self_trade_prevention_and_order_group(self) -> None: ticker="MKT", side="yes", action="buy", + count=1, self_trade_prevention_type="maker", order_group_id="grp-123", ) @@ -781,6 +785,7 @@ def test_accepts_cancel_on_pause_and_subaccount(self) -> None: ticker="MKT", side="yes", action="buy", + count=1, cancel_order_on_pause=True, subaccount=5, ) @@ -796,6 +801,7 @@ def test_buy_max_cost_is_int_cents(self) -> None: ticker="MKT", side="yes", action="buy", + count=1, buy_max_cost=500, ) body = req.model_dump(exclude_none=True, by_alias=True) @@ -819,6 +825,7 @@ def test_buy_max_cost_rejects_fractional_value(self) -> None: ticker="MKT", side="yes", action="buy", + count=1, buy_max_cost="5.5", # type: ignore[arg-type] ) @@ -837,6 +844,7 @@ def test_buy_max_cost_rejects_decimal(self) -> None: ticker="MKT", side="yes", action="buy", + count=1, buy_max_cost=Decimal("500"), # type: ignore[arg-type] ) with pytest.raises(ValidationError): @@ -844,6 +852,7 @@ def test_buy_max_cost_rejects_decimal(self) -> None: ticker="MKT", side="yes", action="buy", + count=1, buy_max_cost=Decimal("5.00"), # type: ignore[arg-type] ) @@ -858,6 +867,7 @@ def test_buy_max_cost_rejects_float(self) -> None: ticker="MKT", side="yes", action="buy", + count=1, buy_max_cost=5.0, # type: ignore[arg-type] ) @@ -869,6 +879,7 @@ def test_buy_max_cost_accepts_int_string(self) -> None: ticker="MKT", side="yes", action="buy", + count=1, buy_max_cost="500", # type: ignore[arg-type] ) body = req.model_dump(exclude_none=True, by_alias=True) @@ -877,7 +888,7 @@ def test_buy_max_cost_accepts_int_string(self) -> None: def test_omits_none_fields_from_wire(self) -> None: from kalshi.models.orders import CreateOrderRequest - req = CreateOrderRequest(ticker="MKT", side="yes", action="buy") + req = CreateOrderRequest(ticker="MKT", side="yes", action="buy", count=1) body = req.model_dump(exclude_none=True, by_alias=True) # Core fields present assert body["ticker"] == "MKT" @@ -898,6 +909,7 @@ def test_phantom_type_field_removed(self) -> None: ticker="MKT", side="yes", action="buy", + count=1, type="limit", # type: ignore[call-arg] ) @@ -911,6 +923,7 @@ def test_forbid_extra_rejects_unknown_kwarg(self) -> None: ticker="MKT", side="yes", action="buy", + count=1, bogus_field="x", # type: ignore[call-arg] ) @@ -1092,8 +1105,8 @@ def test_wraps_order_list(self) -> None: ) orders = [ - CreateOrderRequest(ticker="MKT-A", side="yes", action="buy"), - CreateOrderRequest(ticker="MKT-B", side="no", action="sell"), + CreateOrderRequest(ticker="MKT-A", side="yes", action="buy", count=1), + CreateOrderRequest(ticker="MKT-B", side="no", action="sell", count=1), ] req = BatchCreateOrdersRequest(orders=orders) body = req.model_dump(exclude_none=True, by_alias=True) diff --git a/tests/test_orders.py b/tests/test_orders.py index 1b92f01..860ac4c 100644 --- a/tests/test_orders.py +++ b/tests/test_orders.py @@ -80,7 +80,7 @@ def test_no_phantom_type_in_wire( return_value=httpx.Response(200, json={"order": _MINIMAL_ORDER}) ) - client.orders.create(ticker="MKT", side="yes", yes_price=0.5) + client.orders.create(ticker="MKT", side="yes", action="buy", count=1, yes_price=0.5) body = json.loads(route.calls[0].request.content) assert "type" not in body @@ -96,7 +96,7 @@ def test_count_fp_not_count_in_wire( return_value=httpx.Response(200, json={"order": _MINIMAL_ORDER}) ) - client.orders.create(ticker="MKT", side="yes", yes_price=0.5, count=3) + client.orders.create(ticker="MKT", side="yes", action="buy", yes_price=0.5, count=3) body = json.loads(route.calls[0].request.content) assert "count_fp" in body @@ -117,6 +117,8 @@ def test_time_in_force_reaches_wire( client.orders.create( ticker="MKT", side="yes", + action="buy", + count=1, yes_price=0.5, time_in_force="fill_or_kill", ) @@ -138,6 +140,8 @@ def test_post_only_reduce_only_reach_wire( client.orders.create( ticker="MKT", side="yes", + action="buy", + count=1, yes_price=0.5, post_only=True, reduce_only=False, @@ -159,7 +163,7 @@ def test_buy_max_cost_int_cents_wire( return_value=httpx.Response(200, json={"order": _MINIMAL_ORDER}) ) - client.orders.create(ticker="MKT", side="yes", yes_price=0.5, buy_max_cost=500) + client.orders.create(ticker="MKT", side="yes", action="buy", count=1, yes_price=0.5, buy_max_cost=500) body = json.loads(route.calls[0].request.content) assert body["buy_max_cost"] == 500 @@ -179,6 +183,8 @@ def test_subaccount_order_group_cancel_on_pause_stp_wire( client.orders.create( ticker="MKT", side="yes", + action="buy", + count=1, yes_price=0.5, subaccount=2, order_group_id="grp-x", @@ -201,6 +207,74 @@ def test_type_kwarg_removed(self, client: KalshiClient) -> None: type="market", # type: ignore[call-arg] ) + def test_missing_count_and_action_raises_before_http( + self, + client: KalshiClient, + respx_mock: respx.MockRouter, + ) -> None: + """#242: kwarg-form create() with missing ``count`` / ``action`` must raise + ``TypeError`` BEFORE any HTTP request is dispatched. Pre-#242 the SDK + silently defaulted to count=1, action="buy" — converting a missing-arg + bug into a real 1-contract BUY fill (money risk).""" + route = respx_mock.post( + "https://demo-api.kalshi.co/trade-api/v2/portfolio/orders" + ).mock(return_value=httpx.Response(200, json={"order": _MINIMAL_ORDER})) + + with pytest.raises(TypeError, match=r"count.*action"): + client.orders.create(ticker="X", side="yes") + with pytest.raises(TypeError, match=r"count.*action"): + client.orders.create(ticker="X", side="yes", action="buy") + with pytest.raises(TypeError, match=r"count.*action"): + client.orders.create(ticker="X", side="yes", count=10) + + assert route.call_count == 0 + + def test_explicit_count_action_kwargs_still_work( + self, + client: KalshiClient, + respx_mock: respx.MockRouter, + ) -> None: + """#242: regression — explicit ``count`` + ``action`` kwargs still build + and dispatch normally.""" + route = respx_mock.post( + "https://demo-api.kalshi.co/trade-api/v2/portfolio/orders" + ).mock(return_value=httpx.Response(200, json={"order": _MINIMAL_ORDER})) + + client.orders.create(ticker="X", side="yes", count=10, action="buy", yes_price="0.5") + + assert route.call_count == 1 + body = json.loads(route.calls[0].request.content) + assert body["count_fp"] == "10" + assert body["action"] == "buy" + assert body["yes_price_dollars"] == "0.5" + + def test_request_overload_unaffected_by_242( + self, + client: KalshiClient, + respx_mock: respx.MockRouter, + ) -> None: + """#242: the ``request=CreateOrderRequest(...)`` path is unaffected by + the kwarg-overload guard — the model itself now declares count/action + required, so a fully-populated request still dispatches.""" + route = respx_mock.post( + "https://demo-api.kalshi.co/trade-api/v2/portfolio/orders" + ).mock(return_value=httpx.Response(200, json={"order": _MINIMAL_ORDER})) + + client.orders.create( + request=CreateOrderRequest( + ticker="X", + side="yes", + count=Decimal("10"), + action="buy", + yes_price=Decimal("0.5"), + ) + ) + + assert route.call_count == 1 + body = json.loads(route.calls[0].request.content) + assert body["count_fp"] == "10" + assert body["action"] == "buy" + class TestOrdersCreate: @respx.mock @@ -220,7 +294,7 @@ def test_create_limit_order(self, orders: OrdersResource) -> None: }, ) ) - order = orders.create(ticker="TEST-MKT", side="yes", count=10, yes_price=0.65) + order = orders.create(ticker="TEST-MKT", side="yes", action="buy", count=10, yes_price=0.65) assert order.order_id == "ord-123" assert order.yes_price == Decimal("0.6500") assert order.count == 10 @@ -235,7 +309,7 @@ def test_create_order_no_price(self, orders: OrdersResource) -> None: }, ) ) - order = orders.create(ticker="TEST-MKT", side="yes") + order = orders.create(ticker="TEST-MKT", side="yes", action="buy", count=1) assert order.order_id == "ord-456" @respx.mock @@ -245,7 +319,7 @@ def test_decimal_price_conversion(self, orders: OrdersResource) -> None: 200, json={"order": order_dict(order_id="ord-789", ticker="T")} ) ) - orders.create(ticker="T", side="yes", yes_price=0.65) + orders.create(ticker="T", side="yes", action="buy", count=1, yes_price=0.65) import json @@ -260,7 +334,7 @@ def test_validation_error(self, orders: OrdersResource) -> None: return_value=httpx.Response(400, json={"message": "invalid ticker"}) ) with pytest.raises(KalshiValidationError): - orders.create(ticker="INVALID", side="yes") + orders.create(ticker="INVALID", side="yes", action="buy", count=1) class TestOrdersGet: @@ -435,8 +509,8 @@ def test_batch_create(self, orders: OrdersResource) -> None: ) ) reqs = [ - CreateOrderRequest(ticker="A", side="yes", action="buy"), - CreateOrderRequest(ticker="B", side="no", action="buy"), + CreateOrderRequest(ticker="A", side="yes", action="buy", count=1), + CreateOrderRequest(ticker="B", side="no", action="buy", count=1), ] from kalshi.models.orders import BatchCreateOrdersResponse @@ -476,7 +550,7 @@ def test_batch_create_partial_failure_does_not_crash( ) ) result = orders.batch_create( - [CreateOrderRequest(ticker="A", side="yes", action="buy")] + [CreateOrderRequest(ticker="A", side="yes", action="buy", count=1)] ) assert len(result.orders) == 2 assert result.orders[0].order is not None @@ -1206,8 +1280,8 @@ def test_wraps_orders_key(self, orders: OrdersResource) -> None: orders.batch_create( [ - CreateOrderRequest(ticker="A", side="yes", action="buy"), - CreateOrderRequest(ticker="B", side="no", action="buy"), + CreateOrderRequest(ticker="A", side="yes", action="buy", count=1), + CreateOrderRequest(ticker="B", side="no", action="buy", count=1), ] ) @@ -1238,7 +1312,7 @@ def test_batch_create_uses_bytes_path(self, orders: OrdersResource) -> None: orders._transport, "request", wraps=orders._transport.request, ) as spy: orders.batch_create( - [CreateOrderRequest(ticker="A", side="yes", action="buy")] + [CreateOrderRequest(ticker="A", side="yes", action="buy", count=1)] ) spy.assert_called_once() args, kwargs = spy.call_args diff --git a/tests/test_request_overload.py b/tests/test_request_overload.py index 9a8961c..cd326b6 100644 --- a/tests/test_request_overload.py +++ b/tests/test_request_overload.py @@ -179,7 +179,7 @@ def test_passing_request_and_kwarg_raises( with pytest.raises(TypeError, match=r"Pass either `request=\.\.\.` or"): orders.create( - request=CreateOrderRequest(ticker="MKT", side="yes", action="buy"), + request=CreateOrderRequest(ticker="MKT", side="yes", action="buy", count=1), ticker="OTHER", ) @@ -197,8 +197,8 @@ def test_request_model_produces_same_body_as_kwargs( ) inner = [ - CreateOrderRequest(ticker="MKT-A", side="yes", action="buy"), - CreateOrderRequest(ticker="MKT-B", side="no", action="buy"), + CreateOrderRequest(ticker="MKT-A", side="yes", action="buy", count=1), + CreateOrderRequest(ticker="MKT-B", side="no", action="buy", count=1), ] orders.batch_create(inner) kwarg_body = json.loads(route.calls[0].request.content) @@ -217,7 +217,7 @@ def test_passing_request_and_kwarg_raises( return_value=httpx.Response(200, json={"orders": []}) ) - inner = [CreateOrderRequest(ticker="MKT-A", side="yes", action="buy")] + inner = [CreateOrderRequest(ticker="MKT-A", side="yes", action="buy", count=1)] with pytest.raises(TypeError, match=r"Pass either `request=\.\.\.` or"): orders.batch_create( inner, @@ -267,7 +267,7 @@ def test_multi_required_kwargs_missing( orders: OrdersResource, ) -> None: # create() requires `ticker` and `side`. Passing only one raises. - with pytest.raises(TypeError, match=r"create\(\) requires `ticker` and `side`"): + with pytest.raises(TypeError, match=r"create\(\) requires `ticker`, `side`, `count`, and `action`"): orders.create(ticker="MKT") def test_list_required_kwarg_missing( diff --git a/tests/ws/test_count_migration.py b/tests/ws/test_count_migration.py index ff2a1f0..b0f8138 100644 --- a/tests/ws/test_count_migration.py +++ b/tests/ws/test_count_migration.py @@ -39,10 +39,14 @@ def test_create_order_count_is_decimal(self) -> None: req = CreateOrderRequest(ticker="ECON-GDP", side="yes", count=Decimal("10"), action="buy") assert isinstance(req.count, Decimal) - def test_create_order_count_default(self) -> None: - req = CreateOrderRequest(ticker="ECON-GDP", side="yes", action="buy") - assert req.count == Decimal("1") - assert isinstance(req.count, Decimal) + def test_create_order_count_no_default(self) -> None: + # #242: `count` no longer defaults to Decimal("1") — it is required. + # A missing-arg bug would otherwise silently become a 1-contract BUY. + import pytest + from pydantic import ValidationError + + with pytest.raises(ValidationError): + CreateOrderRequest(ticker="ECON-GDP", side="yes", action="buy") def test_create_order_count_serializes(self) -> None: req = CreateOrderRequest(ticker="ECON-GDP", side="yes", count=Decimal("10"), action="buy") From 7b6dacdd55dfc851d1d0288bd726dff98b920ab1 Mon Sep 17 00:00:00 2001 From: worker Date: Thu, 21 May 2026 06:30:46 -0500 Subject: [PATCH 2/2] fix(orders): reject bool in CreateOrderRequest.buy_max_cost validator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `buy_max_cost` is integer cents, but bool is an `int` subclass. `buy_max_cost=True` was slipping through as the integer 1 (= a 1 cent cap) — the exact failure mode #225 closed for `DollarDecimal` and `FixedPointCount`. A caller who accidentally passed a flag (e.g. `risk_check_passed`, `dry_run`) where cents were expected ended up placing a real order capped at $0.01 with no error. Prepend an `isinstance(v, bool)` rejection before the Decimal/float branches, mirroring the wording/style of the existing rejections. Audited the file: `buy_max_cost` is the only field with a Decimal/float guard, so no other validators need the same patch. Closes #243 --- kalshi/models/orders.py | 16 ++++++++++--- tests/test_async_orders.py | 4 +++- tests/test_models.py | 43 ++++++++++++++++++++++++++++++++++ tests/test_orders.py | 4 +++- tests/test_request_overload.py | 5 +++- 5 files changed, 66 insertions(+), 6 deletions(-) diff --git a/kalshi/models/orders.py b/kalshi/models/orders.py index 83bfcad..8b7e3c1 100644 --- a/kalshi/models/orders.py +++ b/kalshi/models/orders.py @@ -198,16 +198,26 @@ class CreateOrderRequest(BaseModel): @field_validator("buy_max_cost", mode="before") @classmethod - def _reject_decimal_and_float_buy_max_cost(cls, v: object) -> object: - """Reject Decimal and float inputs on buy_max_cost. + def _reject_decimal_float_and_bool_buy_max_cost(cls, v: object) -> object: + """Reject Decimal, float, and bool inputs on buy_max_cost. Spec says integer cents. Accepting Decimal would silently coerce callers who pass Decimal('5.00') (expecting $5.00 under the old DollarDecimal semantics) into 5 cents — data corruption with no - error. Reject at the boundary. + error. Bool is an ``int`` subclass, so ``True`` would otherwise + slip through as 1 (= 1 cent cap), the same class of bug #225 + closed for ``DollarDecimal`` / ``FixedPointCount``. Reject at + the boundary (#243). int and int-shaped strings are fine (Pydantic coerces normally). """ + if isinstance(v, bool): + raise ValueError( + "buy_max_cost must be int (cents), not bool — " + "bool is an int subclass, so True would otherwise slip " + "through as 1 (= 1 cent cap). Pass cents directly " + "(e.g., 500 for $5.00)." + ) if isinstance(v, Decimal): raise ValueError( "buy_max_cost must be int (cents), not Decimal. " diff --git a/tests/test_async_orders.py b/tests/test_async_orders.py index c1bd9e6..533cc3a 100644 --- a/tests/test_async_orders.py +++ b/tests/test_async_orders.py @@ -309,7 +309,9 @@ async def test_create_limit_order(self, orders: AsyncOrdersResource) -> None: }, ) ) - order = await orders.create(ticker="TEST-MKT", side="yes", action="buy", count=10, yes_price=0.65) + order = await orders.create( + ticker="TEST-MKT", side="yes", action="buy", count=10, yes_price=0.65 + ) assert order.order_id == "ord-123" assert order.yes_price == Decimal("0.6500") assert order.count == 10 diff --git a/tests/test_models.py b/tests/test_models.py index c887c4d..d406ae5 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -871,6 +871,49 @@ def test_buy_max_cost_rejects_float(self) -> None: buy_max_cost=5.0, # type: ignore[arg-type] ) + def test_buy_max_cost_rejects_bool(self) -> None: + """#243: bool inputs must raise — bool is an ``int`` subclass, so + ``True`` would otherwise slip through as ``1`` (= 1 cent cap), a + silent money-risk failure matching the #225 class of bug for + ``DollarDecimal`` / ``FixedPointCount``. A caller who passes a flag + (``risk_check_passed``, ``dry_run``) where cents were expected must + get a clear error, not a $0.01-capped order.""" + from pydantic import ValidationError + + from kalshi.models.orders import CreateOrderRequest + + with pytest.raises(ValidationError, match=r"bool"): + CreateOrderRequest( + ticker="MKT", + side="yes", + action="buy", + count=1, + buy_max_cost=True, # type: ignore[arg-type] + ) + with pytest.raises(ValidationError, match=r"bool"): + CreateOrderRequest( + ticker="MKT", + side="yes", + action="buy", + count=1, + buy_max_cost=False, # type: ignore[arg-type] + ) + + def test_buy_max_cost_accepts_plain_int(self) -> None: + """#243: regression — plain int (cents) still works.""" + from kalshi.models.orders import CreateOrderRequest + + req = CreateOrderRequest( + ticker="MKT", + side="yes", + action="buy", + count=1, + buy_max_cost=500, + ) + assert req.buy_max_cost == 500 + assert isinstance(req.buy_max_cost, int) + assert not isinstance(req.buy_max_cost, bool) + def test_buy_max_cost_accepts_int_string(self) -> None: """Int-shaped strings are coerced normally (e.g., loading from env/config).""" from kalshi.models.orders import CreateOrderRequest diff --git a/tests/test_orders.py b/tests/test_orders.py index 860ac4c..6e5c565 100644 --- a/tests/test_orders.py +++ b/tests/test_orders.py @@ -163,7 +163,9 @@ def test_buy_max_cost_int_cents_wire( return_value=httpx.Response(200, json={"order": _MINIMAL_ORDER}) ) - client.orders.create(ticker="MKT", side="yes", action="buy", count=1, yes_price=0.5, buy_max_cost=500) + client.orders.create( + ticker="MKT", side="yes", action="buy", count=1, yes_price=0.5, buy_max_cost=500 + ) body = json.loads(route.calls[0].request.content) assert body["buy_max_cost"] == 500 diff --git a/tests/test_request_overload.py b/tests/test_request_overload.py index cd326b6..88d6c2d 100644 --- a/tests/test_request_overload.py +++ b/tests/test_request_overload.py @@ -267,7 +267,10 @@ def test_multi_required_kwargs_missing( orders: OrdersResource, ) -> None: # create() requires `ticker` and `side`. Passing only one raises. - with pytest.raises(TypeError, match=r"create\(\) requires `ticker`, `side`, `count`, and `action`"): + with pytest.raises( + TypeError, + match=r"create\(\) requires `ticker`, `side`, `count`, and `action`", + ): orders.create(ticker="MKT") def test_list_required_kwarg_missing(