[TexasCoding]

Closes #239

Part of #273 (post-v2.4 audit), Wave 1 money-risk batch.

[claude]

Code Review — PR #280: fix(config): reject demo+base_url split REST/WS environment

Summary: Fixes a real-money safety bug (#239) where demo=True combined with an explicit non-demo base_url silently routed REST calls to production while WebSocket signals came from the demo book. The fix adds defense-in-depth: a pre-config guard in both client constructors and a host-level cross-check in KalshiConfig.__post_init__. Overall this is a well-targeted, high-value change — approval pending two issues below.

---

Strengths

• Correct fix at the right level. The two-layer guard (client constructor + KalshiConfig.__post_init__) catches both the demo=True + explicit URL case and any direct KalshiConfig(base_url=DEMO, ws_base_url=PROD) construction, including the from_env path.
• Error messages are excellent. They name the exact conflict, what behavior it would have produced, and two concrete remediation paths. Users won't be left guessing.
• Test coverage is thorough. test_split_env.py covers the new contract systematically: both mismatch directions, localhost bypasses, from_env env-var path, sync/async symmetry.
• All affected test fixtures updated. The PR correctly adds ws_base_url=DEMO_WS_URL to every fixture that previously passed only base_url=DEMO_BASE_URL.

---

Issues

1. _PROD_HOST / _DEMO_HOST defined inside __post_init__ — minor

config.py:98-99

_PROD_HOST = "api.elections.kalshi.com"
_DEMO_HOST = "demo-api.kalshi.co"

These are re-created on every KalshiConfig() call and are invisible to client.py's pre-config guard. Both files independently embed the same hostname strings. Promote them to module-level constants (alongside PRODUCTION_BASE_URL etc.) so there's a single source of truth. Not a blocker but worth doing given the safety-critical nature of the strings.

2. Issue-number comment prefixes violate CLAUDE.md — minor

client.py:87, async_client.py:87, config.py:89

# #239: reject demo=True combined with an explicit non-demo

CLAUDE.md: "Don't reference the current task, fix, or callers… since those belong in the PR description and rot as the codebase evolves." The WHY (real-money risk) is worth keeping — drop the #239: prefix and keep the rest of the explanation.

3. Breaking change needs a CHANGELOG entry — required before merge

KalshiConfig(base_url=DEMO_BASE_URL) (without ws_base_url) now raises ValueError because ws_base_url defaults to PRODUCTION_WS_URL. This is intentional and correct, but it's a breaking change for any existing code that constructed a demo config that way. CHANGELOG.md must document it under the next release with a migration note (use KalshiConfig.demo() or pass ws_base_url=DEMO_WS_URL explicitly).

---

Minor Gaps (non-blocking)

• Async mirror of test_demo_true_with_demo_base_url_ok is missing. TestKalshiClientSplitEnv has a "redundant but consistent — must not raise" check for the sync client; TestAsyncKalshiClientSplitEnv lacks the equivalent. Low risk but breaks the sync/async symmetry pattern.
• demo REST + localhost WS combination is untested. test_localhost_base_demo_ws_ok covers the inverse; the other direction is an equivalent legit dev setup.
• The comment in config.py says "a caller who set demo=True alongside an explicit prod base_url" — but the config-level check is broader (direct construction, no demo flag involved). Slightly misleading framing.

---

Decision

Request changes on item 3 (CHANGELOG). Items 1–2 are straightforward follow-ups that can land in the same commit. The logic, tests, and error messages are solid — once the CHANGELOG is updated this is ready to merge.

[claude]

Code Review — PR #280: fix(config): reject demo+base_url split REST/WS environment

Verdict: Approve with minor notes. This is a well-targeted money-risk safety fix. The two-layer defense (client constructor guard + KalshiConfig.__post_init__ cross-check) is the right approach, error messages are actionable, and the test suite is thorough. A few things worth calling out below.

---

What this PR does

Closes a silent misconfiguration bug (#239): passing demo=True alongside an explicit non-demo base_url would produce a split REST/WS environment — live orders against a production book, but WS signals from a demo feed. The fix adds two independent validation layers:

1. Client-level guard (client.py / async_client.py): raises early when demo=True + base_url resolves to a non-demo host, before KalshiConfig is ever constructed.
2. Config-level guard (config.py): __post_init__ cross-checks base_url vs ws_base_url host pairs against known Kalshi environments (prod/demo), rejecting any mix regardless of how KalshiConfig was built.

---

Issues

Breaking change not yet in CHANGELOG

KalshiClient(base_url=DEMO_BASE_URL) (no demo=True, no explicit ws_base_url) previously worked silently — and was silently broken (demo REST + prod WS). Now it raises via the config-level guard. This is the correct fix, but it's a breaking API change for any user who hardcoded the demo URL. It should be documented in CHANGELOG.md under a Breaking Changes note, not just in the commit message/issue.

Test gap: demo=True + explicit ws_base_url mismatch has no test

KalshiClient(demo=True, ws_base_url=PRODUCTION_WS_URL)

The client-level guard only checks base_url, so this passes through unchecked there. The config-level guard does catch it (because base_url defaults to DEMO_BASE_URL and ws_base_url=PRODUCTION_WS_URL triggers the cross-check), but this path is untested. A single test in TestKalshiClientSplitEnv would pin it:

def test_demo_true_with_prod_ws_url_raises(self) -> None:
    with pytest.raises(ValueError, match="split REST/WS environment"):
        KalshiClient(demo=True, ws_base_url=PRODUCTION_WS_URL)

Async success case missing from TestAsyncKalshiClientSplitEnv

TestKalshiClientSplitEnv has test_demo_true_with_demo_base_url_ok (the redundant-but-valid path). TestAsyncKalshiClientSplitEnv has no equivalent. Minor, but symmetry is useful for regression prevention.

---

Minor observations (non-blocking)

Local variable vs module-level constants in config.py

The new check defines _prod_host / _demo_host as local variables inside __post_init__. The rest of the module uses module-level _KNOWN_HOSTS, _LOCAL_HOSTS for host validation. This is a small style inconsistency — not wrong, just worth noting if consistency matters.

base_url.rstrip("/") != DEMO_BASE_URL is correct

DEMO_BASE_URL has no trailing slash, so rstrip("/") correctly normalizes before comparison. Confirming intentional — ✓.

from_env env-var path is correctly covered

from_env passes demo and base_url as kwargs directly to __init__, so both from_env env-var tests (KALSHI_DEMO=true + KALSHI_API_BASE_URL=<prod>) correctly exercise the client-level guard. ✓

Duplication between client.py and async_client.py is intentional

Both share identical 14-line guard blocks. Per CLAUDE.md conventions, no abstraction is warranted for a two-callsite check — duplication is fine here.

---

Summary

The core fix is correct, the test coverage is solid for the primary failure modes, and the error messages provide clear remediation steps. The two gaps above (CHANGELOG entry and the demo=True + ws_base_url test) are worth addressing before merge, but neither is a correctness issue.