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.

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 a generated httpx transport while preserving the public response and retry behavior. It also exposes the deployment API’s additional request parameters and hardens status polling and CI dependency integrity.

• Generates and commits an OpenAPI-based deployment transport with a drift check.
• Adds keyword-only execution and status parameters while omitting unset fields.
• Translates httpx transport exceptions into the requests exception classes callers already handle.
• Constrains status polling to the configured origin and pins workflow actions to immutable commits.
• Removes the conflicting unstract console-script registration and documents module-based clone invocation.

Confidence Score: 5/5

The PR appears safe to merge because no blocking failure remains.

No blocking failure remains.

Important Files Changed

Filename	Overview
src/unstract/api_deployments/client.py	Routes deployment calls through the generated transport, forwards newly supported parameters, preserves retry and response compatibility, and reconstructs polling URLs on the configured origin.
specs/docstudio-oss.json	Defines the deployment execute and status contracts used to generate the committed transport.
src/unstract/api_deployments/sdk_docstudio/client.py	Provides the generated authenticated httpx client used by the compatibility wrapper.
tests/test_compat.py	Exercises public API and wire compatibility, parameter forwarding, exception translation, credential rotation, and same-origin status polling.
.github/workflows/test.yml	Pins third-party actions to immutable commits and adds generated-SDK drift detection.
pyproject.toml	Adds bounded generated-transport dependencies, excludes generated code from linting, and removes the conflicting console-script entry.
uv.lock	Locks the newly introduced transport dependencies and updated project dependency constraints.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant Client as APIDeploymentsClient
    participant SDK as Generated transport
    participant API as Configured deployment origin
    Caller->>Client: structure_file(files, options)
    Client->>SDK: Build multipart request
    Client->>API: Execute with bearer token
    API-->>Client: status_api with execution_id
    Caller->>Client: check_execution_status(status_api)
    Client->>Client: Extract execution_id and discard supplied origin
    Client->>API: Poll reconstructed same-origin URL
    API-->>Caller: Normalized result dictionary

Loading

_{Reviews (3): Last reviewed commit: "UN-4010 [MISC] pin the origin against ev..." | 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]

[High] [Lens 7, 3, 13] — custom_data puts a Python repr on the wire, not JSON, and will 400

Typed Any and documented as arbitrary data, but the value reaches sdk_docstudio/models/execute_request.py:130-132 which emits str(self.custom_data).encode(). So custom_data={"a": 1, "b": "x"} goes out as:

b"{'a': 1, 'b': 'x'}"

Single quotes — not valid JSON.

The server rejects this. custom_data = JSONField(required=False, allow_null=True) with validate_custom_data requiring isinstance(value, dict) (Zipstack/unstract backend/api_v2/serializers.py:270,321-327). DRF's JSONField json.loads a multipart string value — which fails on the repr — and if it didn't, the value arrives as a non-dict string and the validator rejects it. Either path is a 400.

So the headline new parameter is unusable in its documented dict form, and it is only usable at all for values whose str() is already what the server wants, i.e. plain strings.

Verified both sides — emitted bytes captured through a patched _send; json.loads on them fails; backend validator read directly.

Fix: json.dumps non-str values in the facade before handing them to ExecuteRequest (the generated encoder must not be edited — gen_sdk.sh overwrites it), and add a custom_data={"a": 1} case to the multipart parity tables. grep -rn custom_data tests/ currently returns nothing outside tests/baseline/.

[ritwik-g]

[High] [Lens 3, 8] — timeout=-1, the spec's own async default, takes the non-retrying branch — but the right fix is a decision, not obvious

This branch tests == 0. Three artifacts disagree:

• this PR's own docstring at :498 — "Execution mode — 0 or below runs asynchronously"
• the spec — timeout is {"default": -1, "minimum": -1, "maximum": 300}, and the generated description says "With the default timeout of -1 the call returns as soon as the execution is queued"
• the code — only exactly 0 takes the retried path

Measured with _send stubbed to 503 and max_retries=2: timeout=-1 → 1 attempt; timeout=0 → 3. On -1 the caller gets {'status_code': 503, 'pending': False, 'execution_status': '', 'error': '', 'extraction_result': ''} — which a while pending: loop reads as a finished execution with no result. The comment at :591 ("a retry would execute it twice") is false for -1, which only queues.

Blast radius is narrow and worth stating: api_timeout still defaults to 300, so existing callers are unaffected and the body's "byte-for-byte unchanged for every existing call shape" holds. This bites only callers passing a negative timeout — which is what the new docstring and the spec default tell them to do.

Which side is authoritative is genuinely open, so I'm not asserting the fix. == 0 is carried verbatim from tests/baseline/client_1_5_3.py, and this PR's stated goal is byte-level parity with 1.5.3 — so widening to <= 0 is a behaviour change this PR has not declared and the parity suite does not cover. Either:

• widen the branch to <= 0, declare it as a third deliberate change, and add -1 to test_a_requested_timeout_selects_the_execution_mode (test_compat.py:445, which today covers only 0 and 300); or
• narrow the docstring at :498 to describe what this client does, and fix test_compat.py:308 ("-1/0 mean async") to match.

The two cannot both stand.

[ritwik-g]

[High] [Lens 3] — Every HTTP error status except 401 is reduced to error: "", and the new parameters are what make those statuses reachable

response_data.get("message", {}) assumes the success envelope. A drf-standardized-errors body — {"type": …, "errors": [{"code", "detail", "attr"}]} — has no message key, so response_message is {} and every field downstream resolves to "". Only 401 is special-cased at :624 to dig into errors[0].detail; check_execution_status (:735 → :752-762) doesn't even have that.

So a 400/403/404/409/413/502/504 returns a dict whose error and execution_status are both empty — structurally identical to a successful-but-empty execution, differing only in status_code, which the poll-loop logic at :654 and :767 consults only for the 2xx/retryable split. The server sent the reason and the client throws it away.

The baseline sent only timeout, include_metadata and files, so a 400 on a working deployment was near-unreachable. This PR adds ten server-validated parameters — precisely the inputs a user gets wrong. A mistyped hitl_queue_name is now a routine 400 carrying "Queue 'nope' does not exist", and the client reports nothing.

Verified — five probes through structure_file and five through check_execution_status; every non-401 case returned error: ''.

Fix: extract the error text once for any non-2xx — prefer "; ".join(e["detail"] for e in body["errors"]) when errors is a list, fall back to body.get("message") when it's a string, then response.text[:N] — and call it from both methods. That also removes the need for the 401 special case.

[ritwik-g]

[High] [Lens 3, 7] — The spec this PR ships declares an error body whose shape crashes the client that consumes it

The committed spec declares ErrorResponse {message: Any, status: str} for 400/401/403/404/409/429/500 on execute. If the server ever returns that declared shape, response_data.get("message", {}) at :618 yields a string, and :637 calls .get() on it — AttributeError: 'str' object has no attribute 'get', uncaught, propagating out of structure_file. Callers catching the documented exceptions (ConnectionError, Timeout, APIDeploymentsClientException) do not catch it.

The two artifacts in this PR contradict each other about the wire format: the spec says errors carry {status, message}, while this file's own 401 branch at :624 reads errors[0].detail — the drf-standardized-errors shape. Both cannot be right, and the spec is the artifact gen_sdk.sh regenerates from, so the wrong one is the durable one.

Verified — a stubbed 404 with {"status":"ERROR","message":"API deployment not found"} raises the AttributeError. Through check_execution_status the same body doesn't crash but mis-maps: execution_status: 'ERROR', extraction_result: 'not found', error: '' — the error text lands in the result field.

Confidence: High that the path crashes on the declared shape; Medium on how often the server emits it — the 401 branch suggests drf-standardized-errors is the live shape, which makes this latent rather than active.

Fix: guard the unwrap (isinstance(..., dict)), and separately reconcile the spec's error schemas upstream in Zipstack/unstract#2237 — I've raised the root cause there.

[ritwik-g]

[High] [Lens 7, 16] — This command does not work

Run verbatim against the worktree:

$ python -m unstract.clone --source-url https://dev.example.com --source-org org_dev123 \
    --target-url https://qa.example.com --target-org org_qa456 --dry-run
Usage: python -m unstract.clone [OPTIONS] COMMAND [ARGS]...
Error: No such option '--source-url'.

unstract.clone.cli:cli is a @click.group() with a single @cli.command("clone") (src/unstract/clone/cli.py:69-74), so the flags must follow the word clone. The repo's own test gets this right — tests/clone/test_cli.py:66-69 invokes with ["clone", "--source-url", …].

This PR removes the unstract console script and points every user here, so a user upgrading past this release loses unstract clone … and is handed a replacement that errors out. python -m unstract.clone --help at :111 does work, so they can discover Commands: clone and recover — but the copy-pasteable example is broken.

The deleted tests/test_cli_top_level.py was the only test exercising a documented CLI entry point end to end, which is why this went unnoticed.

Fix: write the commands as python -m unstract.clone clone …, or collapse cli to a bare @click.command() so the module invocation matches the documented one (then tests/clone/test_cli.py drops the "clone" argument).

[ritwik-g]

[Low] [Lens 15, 16] — Two small things in this script

.gen-venv is not gitignored. VENV="$REPO/.gen-venv" creates it inside the repo, and .gitignore ignores .venv exactly — not .venv* or .gen-venv. Anyone who runs the script gets a multi-megabyte untracked directory in git status and in every subsequent git add sweep. Add .gen-venv/.

The documented verification command has exactly the blind spot the PR's own CI comment names. The header at :12 tells a developer to check with ./tools/gen_sdk.sh && git diff --stat src/unstract/api_deployments/sdk_docstudio. But .github/workflows/test.yml:62-66 states — and works around — that "a diff alone cannot see a file the generator has newly created, which is exactly what a spec growing an endpoint does." So the documented local command reports clean for the one case the gate exists to catch, and the developer only finds out in CI. Document the same git add -N -- <out> && git diff … the workflow uses.

[ritwik-g]

[Low] [Lens 16] — "None means the body was not JSON" is false for a body that is the JSON literal null

A response body of null parses successfully and response.json() returns None, which :368-371 then reports to the caller as "Invalid JSON response from API" — a misleading message for a well-formed body.

Minor, but the docstring states the mapping as exhaustive when it isn't.

[ritwik-g]

[Low] [Lens 10] — The missing-execution_id error embeds the whole status endpoint, signature included

f"No {key} in {url!r}." puts the full endpoint into an exception message that the README's own example prints via print(e). _forwarded_query's docstring at :110-114 states the endpoint may carry "a signature" — so this can write a signed token into a caller's logs.

UNVERIFIED — does the deployment ever return a status_api carrying a signature or token query parameter? The docstring asserts it; I could not confirm it. Please check before treating this as real.

Fix: report the path without the query, or redact query values.

[ritwik-g]

[Low] [Lens 2] — This suite is a migration harness framed as a permanent invariant

1190 lines assert byte-identical wire behaviour against 1.5.3. That is exactly right for this migration. But nothing states when it retires or re-baselines, so the next deliberate behaviour change must edit or delete large parts of it — and the likely outcome is the baseline drifting to whatever makes the suite green, which is the failure mode tools/refresh_baseline.sh:5-7 was written to prevent.

No failure mode today; it's a maintenance judgement.

Fix: one sentence at the top stating the suite's purpose and when it is expected to be dropped or re-baselined.

[ritwik-g]

[Low] [Lens 16] — This docstring points at an unstract CLI that this same diff deletes

"The unstract CLI wraps the orchestrator directly rather than this module." No unstract CLI exists in this package any more — src/unstract/cli.py is deleted in this diff and grep for unstract.cli across src/ tests/ tools/ .github/ returns no hits. A reader lands here looking for the supported invocation and is sent to a command that is not installed.

Fix: drop the second sentence, or name the external package explicitly ("the separate unstract-cli package").