refactor: fold the platform client into a shared facade base

[chandrasekharan-zipstack]

Stacked on #29. Review that one first; this PR's diff is only what came out of reviewing it.

Why

#29 added PlatformAPIClient as a second, independent client. The pooled transport, the close/reopen handling and the httpx-to-requests exception translation were copied out of APIDeploymentsClient, and the copy silently dropped the retry policy. Two copies of transport code drift, and only one of them was getting fixes.

Both clients now inherit a _HttpxFacade base that owns the transport, the retry-with-Retry-After policy and the exception translation. A subclass supplies its error class and its methods. ~184 duplicated lines go away, and the platform operations get the retry behaviour the README already promised them.

Defects fixed

• list_deployments sent workflow=None on every unfiltered call. The generated builder renders that one parameter with str() before it filters None out, so the literal string "None" went on the wire as a filter matching no workflow. Unset filters are now omitted rather than passed as None, which also holds if the generator special-cases another parameter later. (Caught by Greptile — it was mine, introduced when I widened the signature to | None = None.)
• The pool was built outside the lock. AuthenticatedClient.get_httpx_client() builds lazily and unsynchronised, so publishing the client to self before warming it let two racing threads build two pools. It is now warmed inside the lock. (The docstring claiming httpx made this safe was wrong.)
• A close during flight escaped untranslated. httpx answers a send on a closed client with a bare RuntimeError, which is not in the subtree _translate_transport_errors covers — so moving the resolve inside that region, as this PR first did, fixed nothing. It is now translated at the send, as a ConnectionError. (Also Greptile; also mine.)

Exceptions

APIDeploymentsClientException (which predates #29 — it is in the repo's first commit) never worked. Its __init__ nested three more defs that were never bound to the class, so the message was dropped, Exception.__init__ was never called, str(e) was empty and the error_message() the docstring advertised did not exist.

It is now an alias of a new UnstractError base, with APIDeploymentError and PlatformClientError beneath it. The alias points at the base, not a leaf, so except APIDeploymentsClientException keeps catching everything either client raises, including anything added later. .value and error_message() are deliberately not reimplemented — they never worked, so nothing can depend on them.

Also here

• The generated models are re-exported. WhoAmIResponse, APIDeploymentSummary and PaginatedAPIDeploymentSummaryList were generated and then unreachable. Callers who want typing can now WhoAmIResponse.from_dict(...); facade methods keep returning dict[str, Any]. No hand-written mirror of the spec, because regeneration would not update one.
• The platform client gets its own logger. Both shared the module logger, so logging_level on one re-levelled a live instance of the other, turning its debug output — which includes response bodies — on or off as a side effect.
• Bodies are validated. A 2xx that is unreadable, or JSON that is not an object, now raises an error naming the status, content-type and a bounded excerpt, instead of an AttributeError from .get() several frames later. The ERROR log for it is bounded to the same excerpt the exception carries.
• Inputs are checked at the boundary. An org_id that is empty or blank is refused before the request instead of producing a // or %20%20 path; a path component on base_url is warned about rather than silently thrown away by urljoin.
• SKILL.md step 5 gains the recipe for adding an operation: which class it belongs to, the method shape, and why it builds from _get_kwargs rather than sync_detailed.

Things reviewers should push back on if they disagree

• _get_kwargs is still used, deliberately. Routing through the public sync_detailed looks cleaner but reintroduces the bug feat: pick up the whoami and deployment-listing operations #29 fixed: the generated _parse_response calls PlatformKeyError.from_dict(response.json()) unguarded on a declared error status, so a gateway answering 401 with HTML raises JSONDecodeError before the facade sees the status. The private coupling is pinned by a test, so a generator bump fails in CI rather than at a customer.
• close() still lets the next call rebuild the pool. That is pinned by an existing test and stated in the docstring, so it is behaviour, not a leak.
• type(e).__name__ changes — APIDeploymentError where it was APIDeploymentsClientException. Nothing in the tree or on PyPI matches on that, and except is unaffected.

Testing

458 passing. New coverage: the _get_kwargs contract, an unfiltered listing sending no filters, close-during-send, header propagation over a real httpx.MockTransport, close-then-reuse, single-build-under-contention, retry on a retryable status, transport settings propagation, non-object JSON, unreadable 2xx bodies (and the bound on their log), empty and blank org_id, the base_url path warning, the package-root re-exports, alias catchability, message preservation, and logger isolation.

Every fix was mutation-tested — the fix reverted in client.py, the suite re-run, and the failing test confirmed. Nine mutations, nine caught. The logger-isolation regression initially passed unnoticed, which is why that test exists.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CnhmFYFRBrK376tZyDtwUM

[greptile-apps]

via Greptile

Confidence Score: 5/5

The PR appears safe to merge, with all previous findings resolved and no new actionable defects identified.

Summary

• Adds the public PlatformKeyClient surface and generated response-model exports.
• Introduces a common exception hierarchy while preserving the legacy catch-all alias.
• Fixes omitted-filter handling, concurrent pool initialization, close-during-send translation, bounded error logging, and organization-ID validation.
• Documents platform-key usage and the shared error contract.
• Expands compatibility and regression coverage for the refactored behavior.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    Caller[Caller] --> Deployment[APIDeploymentsClient]
    Caller --> Platform[PlatformKeyClient]
    Deployment --> Facade[_HttpxFacade]
    Platform --> Facade
    Facade --> Pool[Authenticated httpx pool]
    Facade --> Retry[Retry and Retry-After policy]
    Facade --> Translation[requests-compatible exception translation]
    Deployment --> DeploymentAPI[Deployment execution API]
    Platform --> PlatformAPI[Platform identity and listing API]

Loading

_{Reviews (3) · Last reviewed commit: "refactor: fold the platform client into ..."}