UN-4010 [FEAT] Support every API deployment request parameter via a generated transport

[chandrasekharan-zipstack]

What

APIDeploymentsClient issues its requests through a transport generated from the API's OpenAPI spec instead of hand-built requests calls, and gains the ten request parameters the deployment accepts that had no argument to travel through.

Why

Every parameter the deployment accepts had to be added to this client by hand, so it lagged the API: of the twelve execute accepts, the client could send two, and only through the constructor. Generating the transport from the spec the backend now commits (Zipstack/unstract#2237) makes the wire format follow the API rather than a hand-maintained copy of it.

How

• specs/docstudio-oss.json + tools/gen_sdk.sh regenerate src/unstract/api_deployments/sdk_docstudio/ with a pinned generator. The generated tree is committed, marked linguist-generated, stamped DO-NOT-EDIT and excluded from lint — regeneration overwrites it wholesale, so fixes belong in client.py or in the spec.
• Only the innermost transport call was swapped. The retry policy, its wait strategy and every return shape are unchanged.
• httpx transport failures are translated to their requests equivalents inside the retried callable, so callers catching ConnectionError / Timeout still work and transport-error retry still counts.
• Requests carry only the fields this client sets. The generated builders write every spec default, and sending a default pins a value the server would otherwise choose.
• The status endpoint arrives in the execute reply, and only its path is taken. Joining an absolute one against the base URL would have let the reply decide which host receives the deployment key.
• The key is read per request rather than captured when the transport is built, so assigning api_key takes effect on the next call as it did when every call built its own header.
• The new parameters (c291e36 for structure_file, ed89066 for check_execution_status) are keyword-only, named exactly as the API names them, and unset by default — an unset parameter is not sent, so the request is byte-for-byte unchanged for every existing call shape. timeout and include_metadata still fall back to the constructor values; a per-request timeout selects the execution mode for that request. execution_id stays out: it is read from the endpoint URL the server handed back.

Can this PR break any existing features

Two deliberate behaviour changes, both narrow:

• File handles opened by structure_file are now closed after the request. The previous client leaked them.
• The status URL is rebuilt from the spec route plus the execution id rather than concatenating the server-supplied path. test_status_url_matches_the_released_client pins that the resulting request is identical.

Everything else is asserted equal to 1.5.3. New parameters are keyword-only and unset by default, so no released call shape changes.

Release

Dispatch Release Tag and Publish Package with version_bump = minor, which publishes 1.6.0.

This PR removes the unstract console script (python -m unstract.clone replaces it), so the published packaging surface shrinks and a patch release would take the command away from anyone on ~=1.5.3. The workflow reads __version__ as the last released version and applies the chosen bump, so the source stays at 1.5.3 and the dispatch input is what decides — patch would ship 1.5.4 and must not be used. test_a_shrinking_packaging_surface_cannot_ship_as_a_patch fails if a patch release ever lands on this surface.

Notes on Testing

372 tests. tests/test_compat.py compares this client against 1.5.3, vendored at tests/baseline/client_1_5_3.py and refreshed via tools/refresh_baseline.sh:

• constructor parameters, defaults, order, public method signatures and class attributes, read out of the released source by AST
• what goes out on the wire: send-only guards, multipart values, URLs, and that api_timeout (a backend execution mode, not a socket timeout) never reaches the transport
• httpx → requests exception translation, asserted on the exact class rather than with a bare pytest.raises — a bare one accepts a superclass, and it hid two real mismatches
• the exact dict each method returns, by running both clients over the same responses across 14 status/body cases
• that falsy values (0, False, "") are sent rather than filtered out as absent
• that a status endpoint naming another host is polled on the configured origin, and that a rotated api_key reaches the next request

The existing retry suite is unchanged apart from its patch target. A live round trip against a deployment is still outstanding.

Related Issues or PRs

• UN-4010 (sub-task of UN-4008)
• UN-4009 [MISC] Generate and commit the API deployment OpenAPI spec in-repo unstract#2237 — the spec this transport is generated from
• UN-4008 [FEAT] Add the unstract CLI to run extractions and API deployments from the terminal unstract-cli#2 — the CLI this unblocks

Dependencies Versions

Adds httpx. requests stays, as the exception types callers catch. Actions in test.yml are pinned to commit SHAs, matching clone-tests.yml.

🤖 Generated with Claude Code

https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ

[chandrasekharan-zipstack]

Backported a fix found by the sibling client's live run: httpx.ReadTimeout was falling into the TimeoutException catch-all and surfacing as requests.Timeout, where the published client raises requests.ReadTimeout. A caller catching ReadTimeout by name would have stopped matching. The translation-table test passed either way because pytest.raises is subclass-tolerant; it now asserts the exact class and fails on the previous code. 310 tests pass.

The live round trip for this client is still outstanding — it needs staging credentials.

[greptile-apps]

Greptile Summary

The PR replaces hand-built API deployment requests with an OpenAPI-generated httpx transport while preserving the public response and retry contracts.

• Adds support for the deployment API’s remaining execution and status parameters.
• Restricts status polling to the configured deployment origin and preserves path-prefix deployments.
• Translates httpx transport exceptions into the requests exception classes existing callers expect.
• Pins CI actions, adds generated-SDK drift validation, and commits the specification and generated transport.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
src/unstract/api_deployments/client.py	Replaces the request transport, adds complete parameter forwarding and connection lifecycle management, and safely constrains status polling to the configured origin.
.github/workflows/test.yml	Pins action references to immutable commits and adds a generated-SDK drift check.
pyproject.toml	Adds the generated transport dependencies and intentionally removes the top-level console script in favor of module invocation.
specs/docstudio-oss.json	Defines the execution and status contracts used to generate the committed transport.
tests/test_compat.py	Exercises released-client compatibility, request serialization, exception translation, origin restriction, and API-key rotation.

Sequence Diagram

sequenceDiagram
  participant Caller
  participant Client as APIDeploymentsClient
  participant Transport as Generated httpx transport
  participant API as Deployment API
  Caller->>Client: structure_file(files, options)
  Client->>Transport: generated multipart request
  Transport->>API: POST configured deployment URL
  API-->>Client: execution result or status endpoint
  alt execution pending
    Caller->>Client: check_execution_status(endpoint)
    Client->>Client: rebuild URL on configured origin
    Client->>Transport: generated status request
    Transport->>API: GET status with execution_id
    API-->>Caller: normalized execution result
  else execution complete
    Client-->>Caller: normalized execution result
  end

Loading

_{Reviews (5): Last reviewed commit: "fix(client): poll the deployment URLs th..." | Re-trigger Greptile}

[ritwik-g]

Standardized review — verdict: REQUEST CHANGES

Critical: 0 · High: 7 · Medium: 11 · Low: 8 · Lenses run: 17/17

Reviewed against a fixed 17-lens rubric (unstract:standard-review, plugin v0.30.1) at d67dda4b, diffed from merge-base 79f8d099 — 31 files, matching GitHub's count. sdk_docstudio/** was treated as generated and reviewed only for whether the facade depends on details regeneration could change.

The engineering here is strong and worth saying plainly: the generator is pinned and version-checked, gen_sdk.sh fails on generator warnings, the generated tree is stamped DO-NOT-EDIT and lint-excluded, the sdk-drift job uses git add -N so a newly-created file cannot slip past the diff, deps are upper-bounded with stated reasons, and the 1190-line parity suite is genuinely load-bearing. The _status_url origin-pinning is careful work.

The High findings cluster in exactly two places: the new parameters this PR exists to add, and the console-script removal.

Things that came out clean — stated so the silence is legible

• A dedicated security pass found no findings at confidence ≥ 8. Notable verified negatives: follow_redirects=True (client.py:342) is safe because httpx 0.28.1 pops Authorization on any non-same-origin redirect — stricter than requests, which compares hostname only and preserves the header across an https→http downgrade. Multipart filename injection isn't reachable (ntpath.basename + httpx's HTML5 escaping). verify defaults True on both layers and is never disabled. The five added packages all resolve from PyPI with sha256, no typosquats, and h11 is at 0.16.0 (at-or-above the request-smuggling fix).
• Greptile's open P1 on client.py:333 ("cross-origin polling leaks credentials") does not reproduce at HEAD. _status_url strips scheme and netloc before geturl(). I tested https://attacker.example/steal, //attacker.example/steal, https://user:pw@attacker.example/x and http://169.254.169.254/latest/meta-data/ — every one resolves back to the configured origin. That is the same mechanism you used to refute the sibling P1 on :339, which Greptile conceded. Your thread is yours to close; flagging only that it's answered.
• The two test changes I scrutinised hardest are legitimate. tests/test_cli_top_level.py's deletion is justified — the wiring those 50 lines exercised survives in tests/clone/test_cli.py:49-127 through the same monkeypatch.setattr("unstract.clone.cli.run_clone", …) seam. tests/test_retry.py has identical test and assertion counts on both sides (52 and 91); every hunk is either a patch-target move or a fixture gaining ?execution_id=. Mutation-checking the parity gate (renaming a public method) produced 33 failures, so it is not warn-only.

Unanchored findings

These have no RIGHT-side line in this diff to attach to.

• [High] [Lens 7] pyproject.toml (the deleted [project.scripts] block) — a published console script and a public module are removed with no version bump, no deprecation shim, and a release workflow that defaults to patch. unstract = "unstract.cli:main" is gone and src/unstract/cli.py is deleted, so from unstract.cli import main now raises ImportError too. __version__ is untouched at 1.5.3 and .github/workflows/main.yml:11 defaults version_bump to patch — so if the release is cut on the default, 1.5.4 ships both breaks to everyone pinned unstract-client~=1.5.3, and a CI pipeline running unstract clone … gets command not found on a routine patch upgrade. Minimally: state the required bump (major, or at least minor) somewhere the release dispatcher will see it. Better: keep [project.scripts] unstract pointing at a two-line shim that prints "moved to the unstract-cli package; use python -m unstract.clone clone …" and exits non-zero, removed next major.
• [Medium] [Lens 1] The PR description does not mention the console-script removal. "Can this PR break any existing features" says "Two deliberate behaviour changes, both narrow" and lists only file-handle closing and status-URL rebuilding. This is the most user-visible change in the diff, and anyone generating release notes from the body will miss it.
• [Low] [Lens 1, 16] README.md:76-77 still describes async/sync selection as api_timeout=0 / api_timeout > 0. After this diff the decision is made by the effective per-request timeout (client.py:586), which a caller can now override.
• PR title — this repo states no title convention in writing (no CLAUDE.md, no CONTRIBUTING, no PR template), so there is nothing to judge against.
• The description says "372 tests"; the suite collects 377. Cosmetic.
• The description notes "A live round trip against a deployment is still outstanding." That round trip is what settles the two open questions below.

Open questions

1. custom_data — does the deployment echo it back anywhere in the execute or status response, and under which key? No response schema declares it.
2. timeout — is -1 meant to be treated as async by this client (widening the branch to <= 0), or is the docstring the thing to narrow? See the finding at client.py:586; this is a decision, not a defect I should pick for you.

Assumption

specs/docstudio-oss.json here is byte-identical to the copy in Zipstack/unstract#2237 — I diffed them. The response-shape findings raised on that PR therefore apply to the models generated here, and fixing them upstream will move this PR's generated tree.

Lens checklist (17/17)

1 see findings + Unanchored · 2 see findings · 3 see findings · 4 Clean — dedicated security pass plus the redirect/multipart/supply-chain negatives above · 5 N/A — no migrations or persisted state · 6 N/A — no concurrency primitives · 7 see findings · 8 see findings · 9 Clean · 10 see findings · 11 Clean — CI improved here: action SHAs pinned, sdk-drift gate added · 12 N/A · 13 see findings; the deletion and the retry-suite edit were both adjudicated legitimate above · 14 see findings · 15 see findings · 16 see findings · 17 see the gen_sdk.sh provenance finding

Posted as COMMENT, not REQUEST_CHANGES — the merge decision is yours, not the review's.

[ritwik-g]

FOLLOWUP review — verdict: REQUEST CHANGES (one High remains)

Prior findings: 23 · RESOLVED 18 · PARTIALLY RESOLVED 4 · NOT RESOLVED 1
New findings: 1 High (carried over) · 4 Medium · 3 Low
Scope change: NO — the delta is confined to the files the findings named, plus the spec refresh they asked for.

Verified against 893e6b6 from merge-base 79f8d099. Status here is what the code shows, not what the replies claim — every fix below was exercised, and the ones that mattered were mutation-tested (break the fix, confirm a test goes red, restore).

What genuinely landed

The substantive work is real and I want to be specific about it, because most of it is verified by execution rather than by reading:

• custom_data now emits b'{"a": 1, "b": "x"}' — valid JSON, json.loads round-trips. Fixed in the facade, so gen_sdk.sh can't overwrite it. Mutation → 3 failures.
• timeout widened to <= 0, coherent across the branch, both docstrings, the README retry table and the spec default. Measured: -1→3 attempts, 0→3, 300→1. Mutation → 2 failures.
• Error text now surfaces for every non-2xx through both methods — 400/403/404/413 all report the server's detail, and degenerate bodies (null, bare string, list, {}) report rather than raise. Mutation → 11 failures.
• The AttributeError crash is guarded, and the vendored spec is byte-identical to upstream Zipstack/unstract@eddd4b74 (sha256 e453d4f7…), with the generated tree regenerated to zero drift under the CI gate's own check.
• The README command runs — python -m unstract.clone clone … parses and stops at the documented --source-key env var.
• File handles: IsADirectoryError/PermissionError now raise APIDeploymentsClientException with zero leaked handles.
• LocalProtocolError: 4 attempts → 1, as InvalidHeader.
• The packaging test, the explicit-None regression test, the per-part Content-Type assertion and the lifecycle (close()/__enter__/__exit__ under a double-checked lock) are all present and mutation-verified.

That's 18 of 23 fully closed, several with better fixes than I suggested — the _operations() source-level guard and the monkeypatched-warn diagnostics test are both improvements on what I proposed.

Why this still can't be approved

One High is untouched — the console-script removal still ships as a patch release. Details in the inline comment on pyproject.toml. This is the only round-1 finding with no code change at all, and it's the one that reaches users who never read this PR.

Four new Mediums, of which two reach a running deployment: the submit-works-but-poll-fails gap on short ingress URLs, and the 429/409 statuses dropped from the spec while the client still has bespoke Retry-After handling for 429.

Residuals on the partially-resolved four

• #4 — check_execution_status still lets the error text double as extraction_result (client.py:850); the new test asserts only result["error"], so it can't see it.
• #9 — the parsed=None claim is refuted by the dropped statuses above.
• #18 — "returned under each result item's metadata.custom_data" is now specific and falsifiable, which is better, but nothing in this repo backs it: custom_data appears in the spec exactly once, in the request schema.
• #22 — the accepted-divergence list in the suite docstring reads as closed at three items; at least two more (the <= 0 retry boundary, the constructor exception class) are discoverable only inside individual tests. The claim that the <= 0 change was declared "in the suite's module docstring" isn't supported — timeout doesn't appear in lines 1-18.

Three Lows worth a line each

include_metrics is still sent while metrics was deleted from FileResult — one of the two is wrong. The packaging manifest is blind to top-level modules (I re-created src/unstract/cli.py and the test still passed). And the spec's sha256 is a comment enforced by nothing, where BASELINE_SHA256 gets a real assertion — the exact distinction that comment draws.

---

CI is green across the board (Greptile, lint, clone-tests, sdk-drift). Once the version question is settled this is close.

Posted as COMMENT. The three sibling PRs in this chain — Zipstack/unstract#2237, unstract-llm-whisperer#722 and llm-whisperer-python-client#35 — came out clean on their followup and I've approved those.

[chandrasekharan-zipstack]

Round-2 residuals from the review summary, all in 3716ab0 — no inline threads for these, so recording them here.

#4 residual — error text doubling as extraction_result. On the status endpoint the hand-built envelope puts the reason under message, the same key a success puts the result under, so a failed poll returned it as both error and extraction_result. It is now reported once (client.py, check_execution_status), and test_the_reason_an_error_carries_is_reported asserts extraction_result in ("", None) on every error case. Mutation-verified: restoring the double assignment → 1 failed.

#18 residual — the custom_data round trip. The claim is true server-side but is not declared anywhere in this repo, so it is now annotated rather than deleted: structure_file's docstring says the round trip is server behaviour and that the spec carries the field on the request only. test_the_custom_data_round_trip_is_documented_as_undeclared pins both halves — that the spec really does carry custom_data on ExecuteRequest and on no response schema, and that the docstring says so. Mutation-verified: softening the wording → 1 failed.

#22 residual — the accepted-divergence list. The module docstring of tests/test_compat.py now also names the <= 0 retry boundary and the exception class a malformed deployment URL raises. ACCEPTED_DIVERGENCES + test_the_accepted_divergences_are_all_listed_up_front keep the list and the docstring in step. Mutation-verified: dropping a phrase from the docstring → 1 failed.

Correction to an earlier reply on the generated-models thread (PRRT_kwDOMMVaCs6b8Atr): that reply claimed nothing lands in the undeclared parsed=None bucket. The reasoning was right for the statuses named but the sets were not exhaustive, as pointed out on the spec thread. The declared sets are now pinned explicitly in DECLARED_ERROR_STATUSES, and every status with bespoke client handling is asserted to be declared, so the claim is checked rather than asserted in prose.

Two backend shapes the spec does not model, found while verifying the fixtures and recorded here for the upstream owner rather than acted on in this repo: the SdkError body {"error", "type": "sdk_error"} (backend/middleware/exception.py:39-42) and the middleware's 500 {"detail": "…"} (:113-114, served without a content_type). Neither is reachable through APIView.dispatch on these two endpoints in the normal case, and _error_text reads both correctly if they ever are, so nothing is broken — but they are undeclared.

Full suite: 419 passed. ruff check src/ clean, sdk-drift regenerates to an identical tree, and specs/docstudio-oss.json is still byte-identical to upstream eddd4b74 (now asserted by test_the_vendored_spec_is_the_revision_the_generator_pins).