UN-4011 [FEAT] Support every extraction parameter via a generated transport by chandrasekharan-zipstack · Pull Request #35 · Zipstack/llm-whisperer-python-client

chandrasekharan-zipstack · 2026-08-11T17:34:43Z

What

LLMWhispererClientV2 builds its requests from a transport generated off the API's OpenAPI spec instead of assembling them by hand, and gains the six extraction parameters the service accepts that had no argument to travel through: allow_rotated_text, watermark_angle_threshold, ignore_vertical_text, derotate_threshold, checkbox_confidence_threshold and min_table_width.

Why

Every parameter the service accepts had to be added to this client by hand, so it lagged the API. Generating the transport from the spec the service now commits (Zipstack/unstract-llm-whisperer#722) makes the wire format follow the API rather than a hand-maintained copy of it.

How

specs/llmwhisperer.json + tools/gen_sdk.sh regenerate src/unstract/llmwhisperer/sdk_llmwhisperer/ with a pinned generator. The tree is committed, marked linguist-generated, stamped DO-NOT-EDIT, and excluded from ruff, docformatter, mypy and pre-commit — regeneration overwrites it wholesale, so a fix applied there is lost on the next run.
Only the generated _get_kwargs builders are used. Responses are read as raw JSON exactly as before, so no generated response model sits on any code path.
The six new parameters (02485e1) are keyword-only, named exactly as the service names them, and unset by default — an unset parameter is not sent, so the query string is byte-for-byte unchanged for every existing call shape. url_in_post is deliberately not among them: in URL mode the URL travels in the body, and whether to say so is this client's decision rather than a caller's.
Headers are read per request rather than held by the transport, so assigning headers or rotating the key reaches the next call the way it did when every call passed them itself.
close() and context-manager support hand the pooled sockets back. The client keeps working afterwards — the next request opens a new pool.

Unchanged and deliberately untouched: the retry policy and its wait strategy, the wall-clock deadline handling, the wait_for_completion poll loop, the deprecated-parameter resolver, the exception hierarchy, and every return shape.

Can this PR break any existing features

Three things a naive transport swap would break, each handled:

Exception types. Callers catch requests.ConnectionError and requests.Timeout by name and the httpx classes are not subclasses. They are translated at the seam, inside the retried call — the retry predicate matches on those same types, so translating around the retry loop would silently disable transport-error retry. requests.ConnectTimeout is both a ConnectionError and a Timeout, so a connect timeout maps to it rather than to a plain Timeout.
Redirects. The previous transport followed them by default; httpx does not. Without follow_redirects a 30x from a proxy or an http→https upgrade surfaces as API error: empty response body.
Injected defaults. The generated builders write every spec-declared parameter. Requests carry only what the client set — sending a default pins a value the service would otherwise choose.

Remaining differences, all wire-irrelevant: query-parameter order is alphabetical rather than insertion order, the webhook JSON body uses compact separators and a different key order (same object), and User-Agent is now python-httpx/....

Notes on Testing

273 unit tests. tests/unit/compat_test.py compares this client against released 2.8.0, vendored at tests/baseline/client_v2_2_8_0.py and pinned by SHA-256 so the comparison cannot drift, refreshed via tools/refresh_baseline.sh. Both run over the same responses:

the outgoing request (method, path, query, body) for all 14 call shapes, including every whisper parameter at once and all three input modes
the returned value across 6 status codes, and error handling across 5 body shapes including empty and non-JSON, so the published client's own rough edges are preserved rather than quietly improved
the wait_for_completion poll loop end to end
constructor parameters, defaults and order, all 11 public signatures, class attributes, and the deprecated-parameter resolver compared statement by statement
retry, deadline capping and deadline-stops-retries at the new seam; exception translation across 9 httpx classes
that each new parameter is absent from the wire unless requested, and reaches it when given a falsy or off value — a truthiness filter would drop those and hand the decision back to the service silently
that a key rotated after the first call reaches the next one, and that the transport can be released and reused

Live round trip. Both clients — this one and the released one vendored under its own module name — were run against the real staging service over the same seven call shapes with every request recorded at the transport layer: usage, a garbage hash sent to status/retrieve/detail, a bad API key, a synchronous extract, and an asynchronous extract followed by a status poll and a retrieve. Wire output was identical on every call in both upload modes; return values matched except for what the service varies between two runs of the same document (per-run timings, confidence_metadata, font_info character metrics).

That run found one divergence the offline suite could not see: httpx.ReadTimeout fell into the TimeoutException catch-all and surfaced as requests.Timeout, where the released client raises requests.ReadTimeout — so a caller catching ReadTimeout by name would have stopped matching. pytest.raises is subclass-tolerant, so the translation test passed either way; it now asserts the exact class and fails on the previous code.

Note on pre-commit: ruff, ruff-format and mypy pass on this branch. docformatter, trailing-whitespace and end-of-file-fixer do not, and they are fixer hooks — running them rewrites 16 expected-output fixtures under tests/test_data/, where trailing whitespace is the thing being asserted in layout-preserving mode. These commits are therefore made with --no-verify. The hook config wants narrowing to exclude those fixtures; separate change.

Related Issues or PRs

UN-4011 (sub-task of UN-4008)
Zipstack/unstract-llm-whisperer#722 — 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
LW-406 [FIX] send whisper params under the names the service reads #34 — merged; this branch now targets main

Dependencies Versions / Env Variables

Adds httpx. requests stays, as the exception types callers catch.

🤖 Generated with Claude Code

https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ

The client now builds its requests from a transport generated off the API's OpenAPI spec instead of assembling them by hand, and sends them over httpx. The retry policy, the deadline handling, the poll loop, the deprecated-parameter resolver and every return shape are unchanged; only the innermost transport call was swapped. Three things a naive swap would have broken, and what keeps them working: - Callers catch requests.ConnectionError and requests.Timeout by name. The httpx equivalents are not subclasses, so they are translated at the seam — inside the retried call, because the retry predicate matches on those same types. requests.ConnectTimeout is both a ConnectionError and a Timeout, so a connect timeout maps to it rather than to a plain Timeout. - The previous transport followed redirects; httpx does not by default. Without it a 30x from a proxy surfaces as "API error: empty response body". - The generated builders write every spec-declared parameter. Requests carry only what the client actually set: sending a default pins a value the service would otherwise choose. url_in_post exists only in URL mode, and the URL itself travels in the body, not also on the query string. Query values are rendered the way the previous transport rendered them, since httpx lowercases booleans. The generated tree is committed but never hand-edited — tools/gen_sdk.sh overwrites it wholesale from specs/llmwhisperer.json with a pinned generator, so fixes belong in client_v2.py or in the spec. It is marked linguist-generated and excluded from lint, formatting and type checking for the same reason. Testing: tests/unit/compat_test.py compares this client against the vendored baseline at tests/baseline — the request that goes out for all 14 call shapes, the value returned across 6 status codes and 5 error bodies, the poll loop, the constructor and public signatures by AST, the retry and deadline behaviour, and exception translation. 234 unit tests pass. A live round trip is still outstanding. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ

httpx.ReadTimeout was landing in the TimeoutException catch-all and coming back out as requests.Timeout. Callers that catch requests.ReadTimeout by name stopped matching. The translation table test used pytest.raises, which is subclass-tolerant and passed either way; it now asserts the exact class. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ

The service takes six OCR parameters this client has no argument for -- allow_rotated_text, watermark_angle_threshold, ignore_vertical_text, derotate_threshold, checkbox_confidence_threshold and min_table_width -- so a caller who needs one cannot reach it at all. They are added as keyword-only arguments named exactly as the service names them. Each defaults to unset and an unset parameter is not sent, so the service still picks its own default and the query string is unchanged for every existing call shape. url_in_post stays out: in URL mode the URL travels in the body, and whether to say so is this client's decision, not a caller's. The signature-parity test now exempts keyword-only parameters, since none is reachable from a released call shape.

The spec now carries what the walk could not infer: which parameters are required, the closed sets the service validates against, the error body it returns, and the binary media types three endpoints answer with. Two of those broke generation quietly. A response whose content type the generator does not recognise is dropped with a warning; so is an entire endpoint whose parameter default its own enum forbids -- and the run still exits 0, so the client came out missing the extraction endpoint with every gate green. The generator's output is now checked for warnings before anything is written, and the three binary content types are mapped to the one it understands rather than being softened in the spec. The unwrapped-operation list is checked against the spec before being subtracted from it: an entry excusing an operation the spec no longer declares would otherwise keep passing forever. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ

The parameter was renamed server-side, and a service older than v2.64.2 reads only the previous spelling: the separator silently falls back to the default instead of failing, which is the kind of thing a caller finds in the output rather than in an error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ

Two unrelated drifts under the same seam. The published client asked for no compression -- `Accept-Encoding: identity`, added by the layer below `requests`, not by any code here -- and httpx asks for gzip. A service response this client has never decoded is not something a transport swap should start requesting; `custom_headers` still overrides. Three httpx failures also reached callers as httpx classes, which nothing downstream catches: a redirect loop, an undecodable body, and any future RequestError that is not a TransportError. Two more mapped to a class the published client never raised for them, since requests had no write or pool timeout. The class decides retries too, so an unsendable URL now stops instead of being attempted four more times. Headers are compared over a real socket, because the transport adds them below anything the client can be asked for. The list of failures is now a walk of httpx's own exception tree rather than a list that stops growing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ

The generated tree is committed, so an edit inside it reviews like any other change and then vanishes on the next regeneration -- as does a spec change nobody ran the generator over. Regenerating in CI and diffing is what notices either one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ

The baseline was a pre-release commit pinned by a version string in its own header comment, which an edit to the file can rewrite as easily as the code below it. It is now taken from the published wheel — what callers actually have installed — and pinned by a digest that no edit can restate.

The generated transport is written against one httpx minor series; an upgrade needs a regeneration and a test run, not a resolver decision taken at install time in someone else's environment.

A query string carries no null, so a caller passing None got the literal string "None" sent as the value. These are overrides the service defaults when absent, and absent is what None asks for.

They did not, and had not for some time. Three things were in the way: - ruff and docformatter disagreed about where a multi-line docstring's closing quotes belong, so each run flipped every docstring back and pre-commit could never converge. D209 is now off; docformatter decides. - the pinned hook ran ruff 0.3.4 while the dev group installed 0.11.9, and the two disagree on import order. Both are pinned to one version now. - mypy could not read `requests` or `pkg_resources` without their stubs, so it reported the imports as errors and checked nothing that used them. The transport-failure translation became a table because the chain of `except` clauses had grown past the complexity limit; the branches, their order and their reasons are unchanged. `Any` is left alone where it is the honest annotation for a service that takes and returns arbitrary JSON.

The spec advertised one region-neutral URL that does not resolve; it now lists the two regions that serve the API. Documentation only -- the generated SDK takes its base URL from the caller, and regenerating against this spec produces no change.

The committed spec covers the whole service while the client wraps part of it, and nothing said so: a reader comparing the two had no way to tell a deliberate omission from a gap. Point at the list the tests already enforce rather than restating it here, where it would go stale.

A comment that describes what the code used to do stops being checkable once that state is gone.

The formatter exclusions were global, so detect-private-key and gitleaks skipped the generated tree and the vendored baseline. They are per hook now, on the hooks whose fix would be lost on the next refresh. InvalidURL is one of the three httpx families outside RequestError; requests raised its own, so it is translated. The docstring names the other two as propagating. The drift gate also sees a newly created file now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ

It shells out to ruff for post-processing. Finding none, it warns and exits 0, and the warning gate reports that as a spec it could not parse -- a clean regeneration on a runner without a global ruff failed with a message pointing at the wrong thing entirely. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ

greptile-apps · 2026-08-17T16:09:24Z

Greptile Summary

The PR replaces manually assembled Client V2 requests with generated OpenAPI request builders while preserving the existing response contract and exposing six additional extraction parameters.

Adds a lazily created, reusable httpx.Client with per-request headers, exception translation, redirect support, deterministic cleanup, and safe recreation after closure.
Commits the generated SDK, source specification, regeneration tooling, and drift/API-surface CI checks.
Expands compatibility, request-wire, retry, lifecycle, and extraction-parameter coverage.

Confidence Score: 5/5

The PR appears safe to merge because the previously reported header and transport-lifecycle failures are resolved and no blocking failure remains.

No blocking failure remains.

Important Files Changed

Filename	Overview
src/unstract/llmwhisperer/client_v2.py	Integrates generated request builders, adds optional extraction parameters, translates httpx failures, reads headers per request, and implements synchronized reusable transport cleanup; the previously reported lifecycle and header issues are resolved.
tests/unit/compat_test.py	Adds broad compatibility checks for request wire format, signatures, responses, exception translation, header rotation, transport closure, and concurrent first access.
tests/unit/client_v2_test.py	Updates isolated request, retry, deadline, and parameter tests for the httpx transport seam.
specs/llmwhisperer.json	Adds the service OpenAPI specification used to generate request builders and parameter definitions.
tools/gen_sdk.sh	Adds pinned regeneration tooling for the committed SDK transport.
.github/workflows/ci_test.yaml	Adds generated-SDK drift detection and public API compatibility checks without introducing an eligible follow-up finding.
pyproject.toml	Adds the runtime packages and tool configuration required by the generated httpx-based transport.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    Caller[Client V2 operation] --> Builder[Generated OpenAPI kwargs builder]
    Builder --> Facade[Filter parameters and apply current headers]
    Facade --> Transport{Cached httpx transport available?}
    Transport -->|No| Create[Create transport under lock]
    Transport -->|Yes| Send[Send request]
    Create --> Send
    Send --> Translate[Translate transport exceptions]
    Translate --> Response[Preserve existing response contract]
    Close[close or context exit] --> Release[Close pool and clear cached transport]
    Release --> Transport

_{Reviews (5): Last reviewed commit: "UN-4011 [FIX] acquire the transport hand..." | Re-trigger Greptile}

The transport held its own copy of the headers, so a key rotated after the first call went on being sent with the old value. They are read per request now, the way every call read them before. `close()` and context-manager support give the pooled sockets back; the previous transport had none to give. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ

`close()` clears it, which mypy reads as assigning None to a Client. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ

The compat suite pins the surface against a vendored baseline file, which only moves when someone remembers to re-vendor it. griffe compares against the latest release tag instead, so the reference point moves on its own. It reads signatures, not requests: it catches a renamed module-level name or a changed parameter, and cannot see what goes out on the wire. The compat suite owns that half; the two are not redundant. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D2bC9Q9MPFeyArNgsSkAkZ

2.8.1 sends page_separator under both spellings for services older than v2.64.2. Comparing against 2.8.0 certified a client that sends only one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D2bC9Q9MPFeyArNgsSkAkZ

The generated builder only emits parameters the spec declares, so the deprecated misspelling the released 2.8.1 sends could not go out at all -- against a service older than v2.64.2, which reads only that spelling, page separation was silently lost. Picks up the spec that now declares it, and widens the send filter to match. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D2bC9Q9MPFeyArNgsSkAkZ

ritwik-g

Standardized review — verdict: `REQUEST CHANGES`

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

Reviewed against a fixed 17-lens rubric (unstract:standard-review, plugin v0.30.1) at b9e12ab, diffed from merge-base 22a4ede9 — 73 files, matching GitHub's count. sdk_llmwhisperer/** was treated as generated and reviewed only for what it publishes and for where the facade depends on it.

The compatibility work here is the most thorough of the four PRs in this batch, and several things came out clean under deliberate attack — worth stating so the silence is legible:

No public symbol is removed, renamed, or has a changed signature or default. whisper()'s 26 released parameters keep their names, order and defaults; the six additions are keyword-only, so no released call shape is reachable. Both guards are real and were mutation-tested: compat_test.py:706-718 compares each of the 11 public methods against the vendored 2.8.1 AST, and griffe check … -X exits 0 against tag v2.8.1. Renaming whisper_detail and moving wait_timeout's default were each caught.
The two modified test files lost nothing. tests/unit/client_v2_test.py: 30 → 30 tests, 57 → 57 asserts, 2 → 2 parametrize tables. tests/integration/client_v2_test.py: 8 → 8, 52 → 52, 4 → 4. An AST comparison confirms no test removed, no argvalue table altered, no per-test assert count changed. Every unit edit is the same mechanical patch-target move (requests.Session.send → _send), and the integration diff is 100% formatter reflow. Classification: legitimate — and note the patch target moved down a layer, so _build_request, _send_request and the retry predicate are all still real code in these tests.
page_separator works end to end. _build_request emits page_separator=%3C%3C%3C&page_seperator=%3C%3C%3C with the default and the same custom value under both keys, across the file, stream and URL paths; _SEND_ONLY declares both and the generated builder drops neither.
./tools/gen_sdk.sh reproduces the committed SDK byte-identically, so sdk-drift is green on arrival.

The findings cluster in three places: the httpx migration's edges (header casing, error translation, URL building, lifecycle), the spec this client is generated from, and two gaps in an otherwise strong parity suite.

Unanchored findings

[Low] [Lens 17] CONTRIBUTING.md:100-113's project structure no longer describes the repo — the listing omits sdk_llmwhisperer/, specs/, tools/ and tests/baseline/, and never tells a contributor the first is generated. Made stale by tools/gen_sdk.sh:17. The DO-NOT-EDIT stamps and sdk-drift cover the rule itself, so this is incompleteness rather than a wrong rule. (Not in this diff, hence here.)
The PR description is stale on the baseline it names. It says the suite compares against released 2.8.0 vendored at tests/baseline/client_v2_2_8_0.py; the branch actually vendors 2.8.1 at client_v2_2_8_1.py (commit 0ad23f3 moved it and the body wasn't updated). It also says "273 unit tests" where 275 collect, and describes neither CI job (sdk-drift, api-surface), the README additions, nor the head commit b9e12ab. The wire-parity and "unset parameters are not sent" claims I checked do hold — the code is right and the description is behind it.
PR title — this repo states no title convention in writing (CONTRIBUTING.md has no PR-title section; the template carries only What/Why/How), so there is nothing to judge against.

Open questions

api-surface vs sdk-drift — what should a maintainer do when a spec change legitimately removes a generated symbol? The two gates will contradict each other from the next release tag onward.
Concurrency — is a single LLMWhispererClientV2 expected to be shared across threads? The previous per-request Session made the question moot; the pooled client and public close() make it live.

Assumption

specs/llmwhisperer.json is byte-identical to the copy in Zipstack/unstract-llm-whisperer#722 — I diffed them — except for the hand-added page_seperator parameter noted in the findings. The spec-level 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 see findings — the header-casing finding is the security-relevant one; a dedicated security pass found no findings at confidence ≥ 8 — empty within scope, not by exclusion. It examined the header-casing issue on its merits and scored it below the bar; I've filed it as a correctness regression accordingly · 5 N/A — no migrations or persisted state · 6 see findings · 7 see findings · 8 see findings · 9 Clean · 10 Clean · 11 see findings · 12 N/A — no prompts, model config or agent loops touched · 13 see findings; both modified test files adjudicated legitimate above · 14 see findings · 15 see findings · 16 see findings · 17 see findings + Unanchored

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

One pre-existing weakness the security pass surfaced, explicitly not this PR's: follow_redirects=True (client_v2.py:310) means the unstract-key header survives a redirect to a foreign host, because httpx strips only Authorization/Cookie cross-origin. But requests.Session.rebuild_auth strips only Authorization too, and the released 2.8.1 client used Session.send() with redirects on by default — so this is unchanged by the diff and out of scope for this review. Worth its own ticket.

Operational note (not a finding): the CA-bundle environment variables change with the transport, from REQUESTS_CA_BUNDLE/CURL_CA_BUNDLE to SSL_CERT_FILE/SSL_CERT_DIR. Anyone pinning a custom CA today will need to move the variable.

Header merge is now case-insensitive, so a custom header overrides the default it collides with instead of travelling alongside it -- which for an overridden `Unstract-Key` put the configured key on the wire too. `httpx.LocalProtocolError` translates to a non-retryable `InvalidHeader` rather than the retryable catch-all: an API key with a stray newline can never be sent, and reporting it as a network fault burned the whole backoff. Its message is replaced so the rejected value -- the credential -- is not reproduced. `build_request` runs through the same translation seam, so a malformed `base_url` raises the `requests` class callers catch. The lazy transport build is locked, `close()` swaps under that lock, and the bare `RuntimeError` httpx raises on a closed transport becomes the exception every method documents. The parity suite compared wire query strings with blank values dropped on both sides, so a change that stopped sending a present-but-empty parameter passed; it now keeps them. The spec is re-copied from upstream at the revision `tools/gen_sdk.sh` records, which un-forks the local page separator edit and declares 402/415/500/503 on every operation -- the generated client returned `None` for all of them. `lines` is no longer required, `mode` is gone from the five operations that never read it, and the `/whisper` mode enum no longer advertises a mode the service does not handle. Also: mypy sees the generated tree's types instead of excluding it, which is what checks the facade's calls into it; the griffe gate is scoped to the hand-written modules so a regeneration is not read as a public API break; `UNSET` no longer leaks into `whisper()`'s signature; `httpx` is bound to the single minor series the transport is generated against; and the package ships a `py.typed` marker.

The accessor read the field again on its way out, so a `close()` landing between the two reads returned the `None` it had been cleared to and the caller dereferenced it -- an `AttributeError`, which is in neither family this client promises. It now returns the client it actually acquired, so a call racing a `close()` gets a closed transport and the documented exception instead.

github-actions · 2026-08-28T05:51:24Z

ritwik-g

FOLLOWUP review — verdict: `APPROVE`

Prior findings: 22 · RESOLVED 19 · PARTIALLY RESOLVED 3 · NOT RESOLVED 0
New findings: 1 Medium · 3 Low. Scope change: NO.

Verified against 124efc6. Status here is what the code shows, not what the replies claim. Suite 287 passed (was 275); mypy . clean over 64 files.

All four Highs fixed, and measured rather than read

Header merge — httpx.Headers(...) with per-item assignment. Raw header list confirms {"Unstract-Key":"override"} → override only, real key gone; {"accept-encoding":"gzip"} → one header; and a third casing (UNSTRACT-KEY) collapses too. The tests drive a real loopback socket returning pairs, so a duplicate can't hide inside a dict. Reverting the merge fails exactly those two tests.
LocalProtocolError — 4 transport attempts → 1, raising InvalidHeader with a message that names the cause and no longer contains the key. Deleting the _TRANSLATIONS row reds test_translation_decides_what_gets_retried[raised5-False].
Blank query params — keep_blank_values=True at all seven sites. Re-ran my exact mutation (and v != ""): 4 failed, 283 passed, naming pages_to_extract/use_webhook/webhook_metadata. Previously 0 failures.
Spec statuses — 402/415/500/503 on all 19 operations, _parse_response returns Error for each. The vendored spec is byte-identical to upstream unstract-llm-whisperer@750f941e (same sha256), and a real gen_sdk.sh run followed by the CI gate's own drift check shows no drift.

Declining api_key.strip() is reasonable — it changes what a configured key resolves to. And the 124efc6 lazy-init change is correct: I checked specifically for a new race or double-close and found neither. Testing the two-bytecode window with a descriptor rather than a thread race was the right call.

One new Medium worth acting on

The API key still reaches logs through the chained __cause__. _REPLACED_MESSAGES correctly keeps it out of str(e) — verified — but raise … from e preserves httpx.LocalProtocolError: Illegal header value b'sk-secret\n', and default traceback formatting renders the chain. So logger.exception(...) / exc_info=True / any uncaught propagation still writes the plaintext key. Only "%s" % e is clean.

That closes the half of my original note about str(e) and leaves open the more common path by which a library exception gets logged. Given this codebase has already had a real header-credential-leak incident, it's worth raise … from None with a redacted detail, or scrubbing the cause's args before chaining. Not a merge blocker — the trigger is a malformed key — but I'd rather it not sit.

Two residuals and two Lows

429 / Retry-After stay undeclared. Waived, and I agree — I checked the LLMWhisperer service myself and it has no 429 path; the facade's handling defends against an infra-level limiter. The only residual is that the generated surface returns None on a 429 from a proxy in front of a deployment.
The minor-release note didn't land. The reply said it would go "in the PR description"; the body has no "minor"/"patch"/"2.8.1" text, and __init__.py is still 2.8.1. A patch dispatch would ship the httpx>=0.28,<0.29 hard cap as 2.8.2. Plausibly a silent gh pr edit --body no-op — that failure mode is known on these repos — but worth re-checking before you cut the release.
The narrowed griffe gate no longer watches unstract/llmwhisperer/__init__.py, where LLMWhispererClientV2 is re-exported and __version__ lives. Mutation-verified: renaming get_llmw_py_client_version was caught by the old package-scope gate and is silent under the per-module loop. Worth adding that module to the loop.
The 13 test-file type errors were suppressed with # type: ignore rather than resolved — fine for test code, but the error_message() return type (str | dict) is now permanently unchecked at those call sites.

Everything else is genuinely closed: InvalidURL translated, thread-safe lifecycle, mypy seam live (the File(payload=bytes) mismatch is fixed and the check that catches it recurring now runs), README page-separator note corrected, UNSET out of the public signature, py.typed shipped, refresh_baseline.sh staged before the digest, streaming branch pinned, all four spec-inherited defects resolved.

CI green (api-surface, sdk-drift, test, Greptile).

chandrasekharan-zipstack and others added 3 commits August 11, 2026 23:04

Base automatically changed from LW-406-deprecate-misspelled-params to main August 12, 2026 09:14

chandrasekharan-zipstack and others added 12 commits August 12, 2026 20:43

build: bound httpx to the series the transport is generated against

e663ada

The generated transport is written against one httpx minor series; an upgrade needs a regeneration and a test run, not a resolver decision taken at install time in someone else's environment.

fix(client): treat an explicit None as unset on the optional OCR params

54d376b

A query string carries no null, so a caller passing None got the literal string "None" sent as the value. These are overrides the service defaults when absent, and absent is what None asks for.

docs: state the header rationale without reference to a prior state

94fef82

A comment that describes what the code used to do stops being checkable once that state is gone.

chandrasekharan-zipstack self-assigned this Aug 13, 2026

chandrasekharan-zipstack marked this pull request as ready for review August 17, 2026 16:06

greptile-apps Bot reviewed Aug 17, 2026

View reviewed changes

Comment thread src/unstract/llmwhisperer/client_v2.py Outdated

Comment thread src/unstract/llmwhisperer/client_v2.py Outdated

chandrasekharan-zipstack mentioned this pull request Aug 17, 2026

UN-4008 [FEAT] Add the unstract CLI to run extractions and API deployments from the terminal Zipstack/unstract-cli#2

Open

chandrasekharan-zipstack changed the title ~~refactor(client): issue requests through a generated transport~~ UN-4011 [FEAT] Support every extraction parameter via a generated transport Aug 17, 2026

chandrasekharan-zipstack and others added 2 commits August 17, 2026 23:05

UN-4011 [FIX] declare the transport handle nullable

cea9d46

`close()` clears it, which mypy reads as assigning None to a Client. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ

chandrasekharan-zipstack requested review from hari-kuriakose, jaseemjaskp, johnyrahul and ritwik-g August 19, 2026 08:00

Merge remote-tracking branch 'origin/main' into work/griffe-baseline

b57775d

chandrasekharan-zipstack and others added 3 commits August 19, 2026 14:33

ritwik-g reviewed Aug 25, 2026

View reviewed changes

greptile-apps Bot reviewed Aug 28, 2026

View reviewed changes

Comment thread src/unstract/llmwhisperer/client_v2.py Outdated

ritwik-g approved these changes Aug 28, 2026

View reviewed changes

Navigation Menu

Uh oh!

UN-4011 [FEAT] Support every extraction parameter via a generated transport - #35

UN-4011 [FEAT] Support every extraction parameter via a generated transport#35
chandrasekharan-zipstack wants to merge 24 commits into
mainfrom
feat/generated-transport

chandrasekharan-zipstack commented Aug 11, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Aug 17, 2026 •

edited

Loading

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

Uh oh!

Uh oh!

ritwik-g left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Aug 28, 2026

Uh oh!

ritwik-g left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

chandrasekharan-zipstack commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

How

Can this PR break any existing features

Notes on Testing

Related Issues or PRs

Dependencies Versions / Env Variables

Uh oh!

greptile-apps Bot commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

Uh oh!

Uh oh!

ritwik-g left a comment

Choose a reason for hiding this comment

Standardized review — verdict: REQUEST CHANGES

Unanchored findings

Open questions

Assumption

Lens checklist (17/17)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Aug 28, 2026

Uh oh!

ritwik-g left a comment

Choose a reason for hiding this comment

FOLLOWUP review — verdict: APPROVE

All four Highs fixed, and measured rather than read

One new Medium worth acting on

Two residuals and two Lows

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chandrasekharan-zipstack commented Aug 11, 2026 •

edited

Loading

greptile-apps Bot commented Aug 17, 2026 •

edited

Loading

Standardized review — verdict: `REQUEST CHANGES`

FOLLOWUP review — verdict: `APPROVE`