[TexasCoding]

Fixes #237 — nightly integration suite was red after v2.4.0 because two test files asserted pre-audit behavior:

Changes

• test_malformed_params_returns_validation_error + test_validation_error_details_attribute switched from limit=-1 (now blocked by Resources: validate "limit" ranges client-side per spec bounds #214's client-side validation) to a malformed cursor that the SDK forwards verbatim and the server still 400s.
• test_batch_create_cancel (sync + async) updated for Orders: V1 batch_create / batch_cancel must return typed per-leg responses (partial-success defect) #194's typed BatchCreateOrdersResponse: iterates response.orders[i].order, treats None .order as a failed leg, reduces to successful order_ids for teardown.

Closes #237

[claude]

Code Review — PR #238

Overview: Fixes the nightly integration suite after v2.4.0 by updating two test files to match current SDK behavior: replacing limit=-1 (now a client-side ValueError per #214) with a malformed cursor that still produces a server-side 400, and updating the batch order assertions to work with the typed BatchCreateOrdersResponse introduced in #194.

---

What's Good

• Correct diagnosis. The root cause (client-side limit validation added in Resources: validate "limit" ranges client-side per spec bounds #214 making limit=-1 raise ValueError instead of KalshiValidationError) is accurately identified, and the fix is minimal and targeted.
• Partial-success handling is now right. The old test asserted isinstance(orders, list) against a return type that no longer exists. The new code properly iterates response.orders, filters None legs, and asserts the typed Order shape — this correctly exercises the model that Orders: V1 batch_create / batch_cancel must return typed per-leg responses (partial-success defect) #194 introduced.
• Sync/async symmetry. Both test_orders.py blocks are updated identically, which is the right pattern for this codebase.
• Surgical changes. No unrelated edits, no formatting noise, no stray refactors. Every touched line traces directly to the stated goal.

---

Issues / Suggestions

1. assert successful failure message may be misleading (minor)

assert successful, f"All legs failed: {response.orders}"

response.orders will print the full list of BatchCreateOrdersResponseEntry objects. If the model doesn't have a readable __repr__, this message will be hard to triage. Consider surfacing the error fields explicitly:

errors = [e.error for e in response.orders if e.order is None]
assert successful, f"All legs failed. Errors: {errors}"

This is a minor readability concern, not a correctness issue.

2. The malformed-cursor test depends on server behavior (medium)

sync_client.markets.list(cursor="!!!not-a-valid-cursor!!!")

The original limit=-1 test was clean: it asserted a deterministic, locally-enforced contract. The replacement relies on the live demo server choosing to 400 this input, which is technically server-defined behavior that could change (e.g., server starts ignoring unknown cursors, or starts returning an empty page instead of an error). This is an acceptable pragmatic trade-off for an integration test, but worth noting. A comment explaining why this specific value is expected to 400 (rather than just return an empty page) would help future maintainers — the current docstring explains the "why not limit=-1" half but not the "why a malformed cursor reliably 400s" half.

3. test_validation_error_details_attribute still uses the same cursor (minor)

The second test at line 102 only checks that exc.details exists — it doesn't assert the 400 status code. The trigger mechanism is the same as test 1, so if the server behavior changes, both tests fail together. This is fine as-is; just noting the coupling.

4. No unit-test counterpart for the ValueError path (out of scope, noted)

There's no non-integration test asserting that markets.list(limit=-1) raises ValueError. This PR isn't the right place for it — it's a fix for a green CI, not a feature — but it would be worth a separate issue/PR to add a unit test for the client-side bound validation introduced in #214, so the behavior is locked in without requiring a live API call.

---

Summary

The changes are correct, minimal, and well-explained. The only actionable suggestion before merge is improving the assert successful failure message to surface error details (item 1). Everything else is low-priority polish or out-of-scope follow-up work.

Verdict: Approve with minor suggestion on the failure message.