feat: add skip_preflight and REJECTED job status (0.47.0) - #353
Open
gavinchen2025 wants to merge 4 commits into
Open
feat: add skip_preflight and REJECTED job status (0.47.0)#353gavinchen2025 wants to merge 4 commits into
gavinchen2025 wants to merge 4 commits into
Conversation
Two gaps kept the client behind the server on the job pre-flight check. REJECTED job status ------------------- JobStatus and JobProcessingStatus were closed enums without REJECTED, so get_job on a rejected job raised, and one rejected job in a page failed the whole list_jobs call, since the page is validated as a single list. Both enums now carry REJECTED plus a _missing_ hook that preserves an unrecognised server-side value verbatim, so the next status the server adds is not a client break. The hook cannot come from the spec - a generated enum is closed - so both files stay in .genignore with a guard test. skip_preflight -------------- Added to CreateWorkflow and UpdateWorkflow to opt a workflow out of the check, and to WorkflowInformation to read the setting back. Modelled exactly on reprocess_all: OptionalNullable[bool] = UNSET on the request models, so an unset value is omitted rather than sent as false, which is what makes the server's "omit means unchanged" contract work on update. Sending false opts back in. BodyCreateJob takes it as a plain argument. The spec types request_data as a string and describes the payload only in contentSchema, so there is no model to generate; the field folds into that JSON instead, and carries no MultipartFormMetadata so it never becomes a form part of its own. An explicit argument wins over a value already in the string. The fold splices rather than re-encoding, so every other byte of the caller's payload survives verbatim - a json.loads/json.dumps round trip preserves values but not their representation. It runs on assignment and through model_copy as well as construction: the field is invisible on the wire, so a dropped fold would reach the server as nothing at all and raise nothing, running the job with preflight enabled while the caller believes otherwise. Re-running it on an already-correct payload is a no-op. Also exports Unset from unstructured_client.types. UNSET was exported but not its type, leaving no public way to test for unset-ness - pydantic deep-copies the default per instance, so an identity check against the singleton silently fails open. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 21 files
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
Resolutions:
_version.py kept 0.47.0. main released 0.46.2 after this branch point;
0.47.0 supersedes it. Both __version__ and __user_agent__.
CHANGELOG.md kept both entries, 0.47.0 above 0.46.2. 0.46.2 shipped to
RELEASES.md PyPI, so dropping its entries would lose release history.
RELEASES.md is oldest-first, so 0.46.2 precedes 0.47.0 there.
.genignore honored main's deletion.
test_regen... honored main's deletion.
The Speakeasy generation contract is being wound down, so the
file protects nothing. Its removal also makes every test that
asserted membership in it vacuous, so those are gone with it.
What was NOT dropped: the behaviour those guards sat next to.
test_skip_preflight_models.py carries the nine tests that
verify what the models actually do - REJECTED and the enums'
forward tolerance, skip_preflight on all four models, the
Unset export, and the request_data fold across construction,
assignment and model_copy, including its byte-preservation and
no-op properties. Those are the real cover: if a regeneration
or a careless edit drops any of it, these fail. The deleted
assertions only checked that a path appeared in a text file.
Verified on the merged tree: 295 unit + 89 contract passing, pylint 10.00/10,
mypy clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Setting skip_preflight back to None -- by assignment or model_copy -- left the
previously folded "skip_preflight": true inside request_data. The object then
said "no preference" while the wire still carried true, so the job ran with
preflight off after the caller had cleared the flag. Silent, and in the
permissive direction.
The early return lumped Unset and None together. That is right when
request_data is untouched caller bytes, but after a fold it no longer is. The
two are different statements and now behave differently:
Unset never mentioned -> passthrough, byte for byte, including a
skip_preflight the caller wrote into the
JSON themselves
None explicitly no preference -> clears a folded key; the explicit
argument wins, as it already does for
True and False
Widening the existing branch to cover both would have introduced the opposite
bug: stripping a key the caller wrote and never asked us to touch. None on a
non-JSON payload also stays silent rather than raising -- nothing was folded
in, so there is nothing to undo.
Also makes the splice preserve surrounding whitespace. The comment claimed it
"leaves every other byte exactly as handed to us" while rstrip() discarded the
indentation before the closing brace and any trailing newline. The splice now
keeps content, the gap before the brace, and anything after it as separate
pieces and reassembles them, so the claim is true.
README: correct the enum containment note. It said `some_string in
shared.JobStatus` is true for any string; that holds only on 3.12. CPython
changed __contains__ twice, so across the supported versions the same
expression raises TypeError on 3.11, returns True on 3.12 and returns False on
3.13. Verified on all three. __members__ is stable everywhere and stays the
recommendation. Also documents the None-clears-versus-unset-passthrough split
above.
No behaviour change to the enums themselves: forward tolerance on
deserialization works identically on 3.11, 3.12 and 3.13.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
The None path removed the folded key by re-encoding the whole document, so
clearing the flag rewrote unrelated configuration: 1e5 became 100000.0, 1.50
became 1.5, and precision past a double was lost. The insert path goes out of
its way to preserve the caller's bytes, so having the clear path discard them
contradicted the code's own contract.
The fold writes a fixed fragment, so clearing can be its exact inverse rather
than a re-encode. `_drop_skip_preflight` removes that fragment from the raw
text, searching from the right because the fold always appends after existing
content. The candidate is parsed and compared against the expected payload
before being accepted, so a fragment that happens to match a *nested*
`skip_preflight` is rejected instead of silently corrupting the document. A
caller who wrote the key with their own spacing matches no fragment and falls
back to a re-encode, which is correct but not byte-preserving.
Fold-then-clear is now a true round trip: '{\n "x": 1e5\n}\n' comes back
identical, as do a 21-digit decimal, non-ASCII, an empty object and '{ }'.
Narrow in practice - it needs a hand-written request_data, since a json.dumps
payload is already normalized before the SDK sees it, plus a non-canonical
number and a fold-then-clear transition. Fixed for the consistency reason
above rather than the precision one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Shadow auto-approve: would not auto-approve. This PR does not meet the repository auto-approval settings.
Re-trigger cubic
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two gaps kept the client behind the server on the job pre-flight check.
REJECTED job status
JobStatus and JobProcessingStatus were closed enums without REJECTED, so get_job on a rejected job raised, and one rejected job in a page failed the whole list_jobs call, since the page is validated as a single list. Both enums now carry REJECTED plus a missing hook that preserves an unrecognised server-side value verbatim, so the next status the server adds is not a client break.
The hook cannot come from the spec - a generated enum is closed - so both files stay in .genignore with a guard test.
skip_preflight
Added to CreateWorkflow and UpdateWorkflow to opt a workflow out of the check, and to WorkflowInformation to read the setting back. Modelled exactly on reprocess_all: OptionalNullable[bool] = UNSET on the request models, so an unset value is omitted rather than sent as false, which is what makes the server's "omit means unchanged" contract work on update. Sending false opts back in.
BodyCreateJob takes it as a plain argument. The spec types request_data as a string and describes the payload only in contentSchema, so there is no model to generate; the field folds into that JSON instead, and carries no MultipartFormMetadata so it never becomes a form part of its own. An explicit argument wins over a value already in the string.
The fold splices rather than re-encoding, so every other byte of the caller's payload survives verbatim - a json.loads/json.dumps round trip preserves values but not their representation. It runs on assignment and through model_copy as well as construction: the field is invisible on the wire, so a dropped fold would reach the server as nothing at all and raise nothing, running the job with preflight enabled while the caller believes otherwise. Re-running it on an already-correct payload is a no-op.
Also exports Unset from unstructured_client.types. UNSET was exported but not its type, leaving no public way to test for unset-ness - pydantic deep-copies the default per instance, so an identity check against the singleton silently fails open.