Repository navigation
PDFCLOUD-6568 Add PostScript job options support - #51
Conversation
- Accept uploaded .joboptions profiles in sync and async conversion - Validate profile type, extension, size, and file count - Serialize the uploaded profile as the job_options_id request field Assisted-by: Codex
- Upload a known-valid Distiller profile for integration coverage - Verify sync and async conversions return both uploaded input IDs Assisted-by: Codex
- Add a runnable conversion with an uploaded Distiller profile - Include deterministic PostScript and joboptions example assets - Document the workflow in the example inventory and API guides Assisted-by: Codex
- Update project metadata for the joboptions feature release - Keep the locked editable package version in sync Assisted-by: Codex
✅ Deploy Preview for pdfrest-python ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
datalogics-kam
left a comment
There was a problem hiding this comment.
Please add live rejection coverage for malformed job-options profiles through both client transports, as detailed in the inline comment.
Validation applies to this PR: its complete file contents match the previously reviewed revision. The non-live suite passed (2,847 passed, 10 skipped), focused unit tests passed (14), and Ruff and Basedpyright passed. The focused hosted live suite passed (6), and the new example succeeded on Python 3.10. The full live suite and example interpreter matrix were not rerun; validation focused on this feature. No repository files were edited during review.
datalogics-kam
left a comment
There was a problem hiding this comment.
This review was produced by Codex running the repository's pr-review-auditor skill.
Requesting changes for the profile-test coverage gaps:
- P2: Assert an observable profile-controlled property of the generated PDF through both transports; successful conversion and echoed resource IDs do not prove the settings were applied.
- P2: Add invalid-profile-content live tests through both transports, as detailed in the existing inline finding. That finding remains applicable and is not duplicated here.
- P3: Complete client-level extension/size validation parity, empty-list rejection, and immediately-inside size-boundary cases.
These are coverage and repository-policy findings; no production-code defect or introduced workflow regression was demonstrated. The two additional findings are attached to the relevant changed lines below.
Scope and validation: Reviewed all 13 changed files and the applicable TESTING_GUIDELINES.md and AGENTS.md rules. The recreated PR's complete Git tree matches the previously audited revision exactly. That revision passed the Python 3.10-3.14 non-live Nox matrix (2,847 passed and 10 expected skips per interpreter), 14 focused unit tests, Ruff, basedpyright, pre-commit, strict documentation build, packaging, and client/diff coverage gates. Runtime tests were not repeated for this follow-up because the file contents are identical. Live tests and the example were not independently run during that audit; the current PR description reports focused live/example success. No repository source or test files were changed by this review.
- Exercise metadata and cardinality failures through both clients - Verify accepted file sizes at and adjacent to each boundary - Ensure invalid inputs fail before reaching the transport Assisted-by: Codex
- Set the validation profile to request PDF 1.6 output - Download sync and async results to confirm the requested version Assisted-by: Codex
- Upload an invalid Distiller profile as a live-test fixture - Assert the service rejection through sync and async clients Assisted-by: Codex
65e6833 to
b636277
Compare
datalogics-kam
left a comment
There was a problem hiding this comment.
Focused follow-up review by Codex, using the GitHub PR review posting workflow, at b636277eb15950bc3cc2905edda818f90fca254c. This review covers the joboptions MIME handling investigated here; it is not a new full-PR policy audit.
Requesting changes:
- Accept the real MIME types of supported joboptions profiles. Accept
text/plainandapplication/postscriptalongsideapplication/octet-stream, while retaining the case-insensitive.joboptionsextension, size, and single-resource checks. Add distinct sync and async client tests for each accepted MIME type and lower/uppercase extensions, and retain rejection coverage for unrelated MIME types and wrong extensions. - Exercise ordinary uploads in examples and live tests. Remove the forced
application/octet-streamfrom the example and both live profile fixtures; usecreate_from_paths(path). A successful test with a manually selected MIME type does not establish that the normal user workflow works. - Add the requested repository stipulation and enforce it in reviews. Update
AGENTS.md, the associatedTESTING_GUIDELINES.md, and.agents/skills/pr-review-auditor/SKILL.md/references/review-checklist.mdwith the requirement: Users must not have to force or override a MIME type to use a supported SDK interface or make its documented examples work. Require default-path upload coverage through both transports and review examples/fixtures for MIME overrides that conceal overly restrictive validation. Treat this as a newly requested policy improvement, rather than an existing rule violation.
Evidence: On both macOS and Linux, file --mime-type identifies the checked-in valid profile as application/postscript and the malformed text fixture as text/plain. On the tested Linux system, Python 3.11's mimetypes.guess_type("valid.joboptions") returns (None, None), so HTTPX defaults the upload part to application/octet-stream; the example's override is redundant there. Content detection and filename-based upload labeling are different mechanisms. The original PR validator was independently confirmed on Linux to reject text/plain and application/postscript metadata for a correctly named profile.
Validation of the uncommitted candidate fix, not of this unchanged PR head: 74 focused joboptions unit tests passed on macOS and Linux; 7 focused upload-helper tests passed on Linux; the macOS offline Python 3.10–3.14 Nox matrix passed with 2,907 passed and 10 existing skips per interpreter; four focused hosted live tests and the updated example passed; Ruff, basedpyright, and patch whitespace checks passed. Linux live tests were not run because no API key was configured; broader live/example matrices were not rerun for this focused change. The existing review threads were inspected and left unchanged.
Files directly examined for this follow-up: src/pdfrest/models/_internal.py, tests/test_convert_postscript_to_pdf.py, tests/live/test_live_convert_postscript_to_pdf.py, the joboptions example and fixtures, AGENTS.md, TESTING_GUIDELINES.md, and the review skill/checklist. No commits or pushes were made for this review.
- Allow octet-stream, PostScript, and plain-text profile metadata - Exercise accepted types and extension casing through both clients - Use normal uploads in live fixtures and the runnable example - Add review guidance against forced MIME metadata Assisted-by: Codex
datalogics-kam
left a comment
There was a problem hiding this comment.
Tested with Codex, also did additional probes, checked public API, verified testing coverage.
Approved.
PDFCLOUD-6568
Why this change
The
/pdfendpoint supports custom.joboptionsprofiles for PostScript and EPS conversion, but the Python client did not expose that capability. Users therefore could not apply custom Distiller settings through the typed SDK.What changed (high level)
PdfRestClientandAsyncPdfRestClientnow accept an optional uploadedPdfRestFileasjob_optionswhen callingconvert_postscript_to_pdf.The payload model serializes the uploaded resource as
job_options_idand validates that exactly one.joboptionsfile is supplied with supported upload metadata and a size between 1 byte and 5 MiB. Documentation, a runnable example, reusable sample assets, live coverage, and package metadata were updated for the new capability.Behavior changes
Existing PostScript and EPS conversions remain unchanged when
job_optionsis omitted.When supplied, the profile must:
PdfRestFile.joboptionsextensionapplication/octet-stream,application/postscript, ortext/plainMIME typeInvalid file metadata is rejected locally before transport. Profile-content validity remains enforced by the service. Successful responses may contain both the source document ID and profile ID in
input_ids.This change is backward compatible.
Validation
The following validation completed successfully:
/pdftests: 8 passed, including synchronous and asynchronous profile application and malformed-profile rejectionThe complete live suite and complete examples matrix were not rerun locally; live validation was focused on the affected endpoint and the updated example. CI exercises both complete suites.
Risks and follow-ups
The client validates profile metadata but cannot determine whether the contents form a valid Distiller profile. Invalid profile contents continue to produce the service's normal API error, which is covered through both client transports in the focused live suite.
The package version is updated from 1.2.0 to 1.3.0. No migration is required for existing callers.