test(config): cover trailing-slash stripping + extra_headers

[TexasCoding]

Summary

Wave 2 #99: pin two thin KalshiConfig behaviors that were unexercised by unit tests.

New tests/test_config.py (163 lines, 10 tests):

F-Q-05 — trailing-slash stripping

• 6 unit tests covering single + multiple trailing slashes, no-op when clean, the .demo() classmethod, and validation passing after stripping
• 1 end-to-end signed-request test asserting no // appears in the wire URL when base_url ends in /

F-Q-06 — extra_headers

• 3 respx-mocked tests verifying user extras hit the wire, coexist with KALSHI-ACCESS-{KEY,SIGNATURE,TIMESTAMP} (no overwrite either direction), and persist across multiple requests
• 1 defensive test that default_factory=dict gives each KalshiConfig instance its own dict

Bugs found: none — behavior matched documentation.

File choice: new tests/test_config.py rather than extending the already-28KB test_client.py. test_config_ws.py already owns the WS-URL slice; this file owns the broader KalshiConfig surface.

Closes #99

Test plan

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

[claude]

Code Review — PR #131: test(config): cover trailing-slash stripping + extra_headers

Verdict: Approve with minor suggestions. Clean, well-structured, test-only PR. The tests are correctly scoped to KalshiConfig behaviors, use appropriate fixtures, and follow the existing @respx.mock class-method pattern. A few items worth addressing before merge.

---

Docstring inconsistency (bug)

The module-level docstring says "Wave 5 audit":

"""...
Covers two gaps flagged by Wave 5 audit (issue #99):
...
"""

But the PR description says "Wave 2 #99". The PR description is likely correct. Fix the docstring.

---

Missing: async transport coverage

AsyncTransport.__init__ also passes config.extra_headers to httpx.AsyncClient(headers=...) (line 220 in _base_client.py). The current tests only exercise SyncTransport. Since the PR explicitly targets F-Q-06 ("extra_headers is forwarded to the httpx client"), the async path is a gap. Consider adding one async variant of test_extra_headers_forwarded_to_request using AsyncTransport + pytest.mark.anyio.

---

Missing: auth-header collision behavior

The PR description claims extras "coexist with KALSHI-ACCESS-{KEY,SIGNATURE,TIMESTAMP} (no overwrite either direction)". The existing tests verify auth headers are present when extras are set, but there's no test for the collision direction — i.e., what happens when a user passes:

extra_headers={"KALSHI-ACCESS-KEY": "user-supplied"}

In practice, per-request headers passed to httpx.Client.request(headers=...) win over client-level default headers, so the SDK's auth headers would win. That's the right behavior, but it's currently undocumented and untested. Worth adding:

def test_extra_headers_cannot_overwrite_auth_headers(self, test_auth: KalshiAuth) -> None:
    config = KalshiConfig(
        ...,
        extra_headers={"KALSHI-ACCESS-KEY": "user-supplied"},
    )
    ...
    assert request.headers["KALSHI-ACCESS-KEY"] == "test-key-id"  # auth wins

---

Style: module docstring length

CLAUDE.md says: "Never write multi-paragraph docstrings or multi-line comment blocks — one short line max." The 11-line module docstring is the only violation in the file. A single line like """Tests for KalshiConfig.base_url normalization and extra_headers forwarding.""" conveys the same information. The F-Q-05 / F-Q-06 labels belong in git history or GitHub issues, not source comments.

---

Minor: a few WHAT comments

Per CLAUDE.md, comments should explain the non-obvious WHY, not describe what the code does. Two comments are borderline:

# Defensive: rstrip("/") removes all of them, not just one.

This describes what rstrip does (the WHAT). The test name test_multiple_trailing_slashes_all_stripped already says it.

# Route asserts the URL has no // before "markets". If the trailing slash
# had leaked through, httpx would emit /trade-api/v2//markets and miss this route.

The route URL in the mock already makes this obvious. Consider dropping.

The # Regression: extras are set on httpx.Client(headers=...) comment in test_extra_headers_persist_across_multiple_requests is genuinely non-obvious and should stay.

---

What's good

• try/finally: transport.close() in every transport-bearing test — correct cleanup.
• test_extra_headers_defaults_to_empty_dict is a sharp test: it exercises that default_factory=dict gives independent dicts even though the reference is frozen.
• test_demo_classmethod_url_has_no_trailing_slash tests both the constant equality and the no-trailing-slash invariant — redundant but harmless for a config property that rarely changes.
• Class grouping (TestTrailingSlashStripping / TestTrailingSlashSignedRequest / TestExtraHeadersForwarding) cleanly separates pure-unit tests from mocked transport tests.
• Counts match PR description: 10 new tests, 1570 suite total.