polish: Wave 5 backlog — transport perf, config knobs, docs, CI hardening

[TexasCoding]

Summary

Wave 5 #106 — 5 small polish items from the umbrella backlog. Items picked are low-risk, single-purpose, and stayed clear of Wave 3/4 (kalshi/ws/) and sibling #90 (kalshi/models/) territory.

Commit	Item	Files
9c8b7e9	F-R-04 perf — cache urlparse(base_url).path once per transport instead of re-parsing on every request	kalshi/_base_client.py
ec68e9d	F-R-15 feat — opt-in KalshiConfig.http2: bool = False and KalshiConfig.limits: httpx.Limits | None = None. Non-breaking; defaults preserve existing behavior	kalshi/config.py, kalshi/_base_client.py, tests/test_config.py (+4 tests)
40606c6	F-N-10 docs — AsyncKalshiClient.ws lifecycle docstring	kalshi/async_client.py
ea4eb0e	F-O-11 ci — shred PEM on integration-nightly workflow exit	.github/workflows/integration-nightly.yml
a972cbd	F-O-12 ci — upload sigstore attestations on PyPI release	.github/workflows/release.yml
(followup)	CHANGELOG — Added entry for http2/limits config knobs

Items deferred (with rationale)

• F-N-13 (empty cli dirs): already deleted; no-op.
• F-N-17 (mutable defaults in models): kalshi/models/* is sibling-branch territory (Type-annotation drift: size/count fields use DollarDecimal instead of FixedPointCount #90).
• F-N-09 (duplicate _to_decimal_*): explicitly covered by Type-annotation drift: size/count fields use DollarDecimal instead of FixedPointCount #90.
• F-N-16 (yield from vs return self._list_all): already standardized by Wave 2 Test coverage: _list_all max_pages cap + expose on public *_all() methods #98; no occurrences remain.
• F-P-15 / F-R-02 (MessageQueue maxlen): kalshi/ws/ Wave 3/4 territory.
• F-R-10 (RSA sign via executor): behavior change, deserves its own issue.
• F-P-16, F-R-13: kalshi/ws/ territory.
• F-R-05/11/12: speculative; issue itself flags as "not actionable."

⚠️ Wave 2 #102 CWD-slip pattern recurred

The agent's first 5 commits landed on the parent repo's local main by mistake (Bash CWD reset between calls — same trap as Wave 2 #102). The agent cherry-picked all 5 to the proper branch in its worktree, then surgically rewound refs/heads/main back to origin/main. Verified clean before push.

Test plan

• Suite: 1706 passed (+4 new config tests), 48 skipped
• uv run ruff check . clean
• uv run mypy kalshi/ clean (strict)
• Release workflow change (a972cbd) won't actually run until next tag
• Integration-nightly workflow change (ea4eb0e) effective on next nightly cron

Refs #106

[claude]

Code Review — Wave 5 Polish (#141)

Five isolated, low-risk items. Overall the changes are clean and well-scoped. A few things worth addressing before merge:

---

F-R-04 — urlparse caching ✅

Correct and minimal. Caching urlparse(config.base_url).path once per transport init is the right move on a hot request path. The comment ("Cached once: base_url is immutable on a frozen KalshiConfig") is the right level of explanation.

---

F-R-15 — http2 / limits config knobs

Bug: limits forwarding isn't actually tested

test_custom_limits_forwarded_to_sync_client asserts transport._config.limits is limits — that proves the dataclass stored the value, but not that it was forwarded to the httpx.Client. The real forwarding is in client_kwargs["limits"] = config.limits, but no test exercises the httpx side. The simplest verification:

assert transport._client._limits.max_connections == 10

httpx.Client stores limits on ._limits; it's a private attr but stable enough for a test.

Missing coverage: AsyncTransport

Both transports got identical code changes (_base_path cache + client_kwargs dict), but only SyncTransport is tested. Should add symmetric tests for AsyncTransport:

async def test_http2_flag_forwarded_to_async_client(self, test_auth):
    config = KalshiConfig(http2=False)
    transport = AsyncTransport(test_auth, config)
    try:
        assert transport._config.http2 is False
    finally:
        await transport.aclose()

async def test_custom_limits_forwarded_to_async_client(self, test_auth):
    limits = httpx.Limits(max_connections=10, max_keepalive_connections=5)
    config = KalshiConfig(limits=limits)
    transport = AsyncTransport(test_auth, config)
    try:
        assert transport._client._limits.max_connections == 10
    finally:
        await transport.aclose()

Minor: httpx.Limits is mutable inside a frozen dataclass

frozen=True prevents config.limits = new_limits but does nothing to stop config.limits.max_connections = 99 after construction. This is a general Python limitation, not a bug introduced here, but it's a subtle footgun for a config object users might assume is fully immutable. Calling it out for documentation consideration, not a blocker.

---

F-N-10 — AsyncKalshiClient.ws docstring ✅

The .. note:: block is exactly what users need. The "capture once" example with @ws.on(...) clearly demonstrates the hazard. The placement of the note before Usage:: is the right order (warn before example). No changes needed.

---

F-O-11 — Shred PEM on workflow exit

Ordering looks correct. if: always() && steps.guard.outputs.has_secrets == 'true' ensures the shred runs even if pytest fails, but only if the PEM was ever written. ✅

Minor: shred on journaled filesystems

shred is not guaranteed to overwrite data on ext4 with journaling (GHA ubuntu-latest default). The journal can retain block pointers or copies of data. That said, GHA runners are ephemeral and the VM is destroyed post-job, so this is belt-and-suspenders regardless. Consider shred -u --zero if you want the strongest guarantee — --zero adds a final zero-overwrite pass after the random passes, which is a common defense against journal-based recovery. Not a blocker.

---

F-O-12 — Sigstore attestations on PyPI release

Supply chain note (pre-existing, not introduced here): pypa/gh-action-pypi-publish@release/v1 is a floating tag. The attestations: true addition is the right direction for supply chain hygiene, but the action itself isn't SHA-pinned. If this action is ever compromised and a new release/v1 is pushed, the signing would happen against a malicious action. Consider pinning to a specific SHA (e.g., pypa/gh-action-pypi-publish@a95462a...). Not introduced by this PR, but worth flagging while touching the workflow.

The attestations: write permission is correctly scoped to publish-pypi only. ✅

---

Summary

Item	Status	Blocker?
urlparse cache	✅ Clean	—
http2/limits config	⚠️ Missing AsyncTransport tests; limits forwarding not verified	Yes (test coverage)
ws docstring	✅ Clean	—
Shred PEM	✅ Correct; shred --zero optional improvement	No
Sigstore attestations	✅ Correct; pre-existing SHA-pin gap	No

The test gaps on AsyncTransport and the limits→httpx forwarding assertion are the ones I'd want addressed before merge. Everything else is solid.

[claude]

Code Review — Wave 5 Polish (#141)

PR is merged; feedback below is retrospective. Items marked Future are worth filing as follow-up issues if they matter to the milestone.

---

Overview

Five tightly-scoped changes: a micro-perf fix, two new config knobs, a lifecycle docstring, and two CI hardening steps. The scope discipline is excellent — the deferred-items table especially. No complaints there.

---

What's solid

• urlparse cache (_base_client.py): Correct. KalshiConfig is frozen=True, so _base_path genuinely can't drift. The comment accurately reflects the invariant.
• Non-breaking defaults: http2=False and limits=None preserve existing behavior exactly — zero risk to existing consumers.
• CI shred step: if: always() + file-existence guard + shred -u || rm -f fallback is the right pattern. Step ordering (shred before "Report failure") is correct.
• Sigstore attestation: Straightforward addition; supply-chain posture improvement.
• WS lifecycle docstring: The "each access returns a new instance" gotcha is exactly the kind of thing that burns users silently — glad it's documented.

---

Issues

1. Tests verify config round-trip, not httpx forwarding

The two new forwarding tests check transport._config.http2 and transport._config.limits, not the underlying httpx.Client. You could delete the client_kwargs["http2"] = config.http2 line and both tests would still pass.

The comment in the test acknowledges the limitation ("httpx exposes http2 via the private _h2_pool"). A stronger approach:

# Inspect the constructed client via its public interface
transport = SyncTransport(test_auth, KalshiConfig(http2=False))
# httpx.Client.is_closed is public; _transport._pool._http2 is semi-private but stable
assert transport._client._transport._pool._http2 is False

Or use unittest.mock.patch("httpx.Client") to intercept constructor kwargs:

with patch("httpx.Client") as mock_client:
    SyncTransport(test_auth, KalshiConfig(http2=True))
    _, kwargs = mock_client.call_args
    assert kwargs["http2"] is True

Either approach would catch a regression where the kwarg is accidentally dropped.

Future: worth a follow-up to strengthen these two tests.

2. No AsyncTransport tests

The four new tests cover only SyncTransport. AsyncTransport.__init__ has an identical client_kwargs block — a parallel regression there would go undetected.

Future: add test_http2_flag_forwarded_to_async_client and test_custom_limits_forwarded_to_async_client.

3. httpx.Limits on a frozen=True dataclass may break hashability

frozen=True auto-generates __hash__ using all fields. If httpx.Limits defines __eq__ without __hash__ (Python then sets __hash__ = None), then hash(KalshiConfig(limits=some_limits)) will raise TypeError.

Quick check to verify:

import httpx
print(httpx.Limits.__hash__)  # None → problem; method → fine

If it's None, the fix is either:

# in KalshiConfig
limits: httpx.Limits | None = field(default=None, hash=False)

or switch to an intermediate frozen dataclass or compare=False.

This won't surface unless someone hashes a KalshiConfig (e.g., uses it as a dict key or set element), but it's a silent footgun on a frozen=True type.

Recommend: verify and file an issue if confirmed.

---

Minor observations

• The limits field is the only mutable value inside a frozen=True config. The field reference can't be reassigned, but someone holding a reference to the httpx.Limits object and mutating it after config construction would silently affect the transport pool. Documenting this in the docstring ("treat as immutable after construction") would help.
• http2=True isn't tested — reasonable given it requires the h2 extra, but a note in the test class docstring explaining why would save future readers from wondering.

---

Summary

Solid wave — correct, minimal, well-scoped. The main gap is test depth: the forwarding tests pass even if the forwarding is broken. Items 1 and 2 are straightforward follow-ups; item 3 deserves a quick verification before it bites a user in a hash context.