test(recorder): cover non-JSON body branch (HTML 502, plain text, binary)

[TexasCoding]

Summary

Wave 2 #100: pin RecordingTransport's non-JSON body branch — previously only JSON responses had test coverage, so a regression in the latin-1 fallback / text branch of build_response would go silently undetected.

4 new tests in tests/test_mock_transport.py (+127 lines):

• test_record_non_json_html_body_persists_as_text — 502 with text/html body; asserts body_kind == "text", byte-for-byte round-trip, content-type preserved
• test_replay_non_json_html_body_surfaces_server_error — replays from disk only; verifies KalshiServerError(status_code=502) with the HTML payload visible in the message
• test_record_non_json_plain_text_body_persists_as_text — 503 with text/plain; charset=utf-8
• test_record_non_json_binary_body_persists_as_text — application/octet-stream body containing every byte 0x00–0xFF (invalid UTF-8) to prove the latin-1 round-trip is lossless

Implementation notes:

• max_retries=0 on the test config keeps 502/503 from triggering the SDK retry loop and piling up duplicate recorded pairs
• Tests exit the KalshiClient context before reading fixture files from disk so they hit the post-perf: O(1) qsize, AWS Full Jitter, buffered recordings #111 buffered/flush-on-close shape correctly

Recorder bugs found: none. Behavior is correct; latin-1 round-trip is lossless and build_response reconstructs non-JSON bodies properly.

Closes #100

Test plan

• Suite: 1564 passed, 48 skipped (+4)
• uv run ruff check . clean
• uv run mypy kalshi/ clean (strict)

[claude]

Code Review — PR #132: Non-JSON body branch coverage

Overall: Solid testing PR that fills a real coverage gap. The max_retries=0 rationale is well-reasoned, the latin-1 bytes(range(256)) round-trip test is particularly thorough, and the tests correctly exit the client context before reading fixtures (post-#111 flush-on-close correctness). A few issues worth addressing before merge:

---

Issues

1. Multi-line docstrings/comments violate CLAUDE.md style

CLAUDE.md is explicit: "Never write multi-paragraph docstrings or multi-line comment blocks — one short line max."

Three of the four new test functions have two-line docstrings, and the section header is a verbose multi-line block comment:

# ---------------------------------------------------------------------------
# Non-JSON body coverage (#100 — F-Q-08)
#
# Real-world recordings will eventually capture an HTML 502 ...
# ---------------------------------------------------------------------------

The existing tests in this file use single-line docstrings consistently (e.g. """Two requests differing only in headers/body produce the same fingerprint."""). The multi-line docstrings should be condensed to one line each, and the block comment can either be dropped or trimmed to a single # Non-JSON body coverage — previously untested text/binary path in RecordingTransport.

---

2. test_record_non_json_plain_text_body_persists_as_text — no replay verification

This test records a plain-text 503 and asserts the fixture shape, but never replays it. The plain-text replay path (build_response's text branch) is therefore untested in isolation. Either add a replay assertion at the end (mirroring what test_record_non_json_binary_body_persists_as_text does), or document why replay is intentionally skipped.

---

3. Binary replay asserts only that an error is raised, not that bytes survived

test_record_non_json_binary_body_persists_as_text ends with:

replay = ReplayTransport(fixtures_dir)
with KalshiClient(transport=replay, max_retries=0) as client, pytest.raises(KalshiServerError):
    client.exchange.status()

This checks liveness but not correctness — if build_response corrupted the binary body during replay, the test would still pass. Since KalshiServerError embeds the body, you could add:

    pytest.raises(KalshiServerError) as exc_info:
...
assert body.decode("latin-1") in str(exc_info.value)  # or assert exc_info.value.status_code == 502

For truly non-printable binary content the string check is awkward, but at minimum exc_info.value.status_code == 502 + exc_info.value.status_code being present is a stronger assertion than just "raises."

---

4. Minor: assert "502 Bad Gateway" in str(exc_info.value) is format-coupled

In test_replay_non_json_html_body_surfaces_server_error, the assertion depends on the HTML content appearing in str(KalshiServerError). This is reasonable since the PR description says "HTML payload is the error 'message'", but it would be worth a brief comment explaining that assumption, or using a less brittle check like asserting exc_info.value.status_code == 502 separately and checking _HTML_502.decode("latin-1") against the error body attribute if one exists.

---

Nits

• The _HTML_502 module-level constant is used in two tests — fine. But b"Service Unavailable\nRetry in a bit." and bytes(range(256)) are defined inline in their respective tests. Consistency isn't required here since CLAUDE.md discourages premature abstraction, but worth noting.
• No async variants (AsyncRecordingTransport / AsyncReplayTransport) are added. The sync/async transports share the serialization logic, so this is probably intentional — just confirming that was a conscious call, not an oversight.

---

Summary

The core coverage is valuable and the implementation is correct per the PR description. The blocking items are:

1. Multi-line docstrings/comments (CLAUDE.md style rule)
2. Plain-text replay path unverified (coverage gap)

Item 3 (binary replay assertion strength) is a nice-to-have. Everything else is minor.