Repository navigation
PDFCLOUD-6570 Add PDF-to-PostScript conversion - #55
datalogics-tsmith wants to merge 6 commits into
Conversation
- Export the supported PostScript language levels for client callers. - Validate uploaded PDFs, page ranges, scale values, and output names. - Serialize the payload into the fields expected by the service. Assisted-by: Codex
- Expose sync and async helpers with typed conversion options. - Cover request serialization, validation, responses, and timeouts. - Bump the feature release version to 1.4.0. Assisted-by: Codex
- Exercise both language levels through sync and async clients. - Assert output metadata, page count, and language-level markers. - Confirm the service rejects invalid levels, ranges, and scales. Assisted-by: Codex
- Add runnable sync and async examples for PostScript levels 3 and 2. - Verify generated headers, language levels, page counts, and metadata. - Link the route and examples from the public documentation indexes. Assisted-by: Codex
- Verify default output serialization through both client transports. - Isolate live options and assert observable PostScript markers. - Derive live scale boundary cases from the payload schema. 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
datalogics-kam
left a comment
There was a problem hiding this comment.
Focused review by Codex using the repository's pr-review-auditor guidance. Scope: the new PostScript page-range validation and opportunities to reuse existing Pydantic compositions.
Please replace the duplicated page-range parser/checks with a composition of the existing constrained range types and the smallest endpoint-specific adaptations. This follows AGENTS.md's requirement to favor declarative Pydantic validation and keep custom validators shape-focused.
Validation at 1efad81: the focused PostScript and shared graphic-payload suites passed (146 tests, Python 3.11); the committed diff passed git diff --check. A separate composition probe accepted and serialized the documented selections all, 1, 1,3-5, last, and 14-last. Live tests and the full interpreter matrix were not run for this focused validation-design review.
| ) | ||
|
|
||
|
|
||
| def _validate_postscript_page_range(value: str) -> str: |
There was a problem hiding this comment.
[P2] Compose page-range validation from the existing Pydantic types
This introduces another regex/parser and manual range checks even though PageNumber, _AscendingPageRangeTuple, and AscendingPageRange already express positive page numbers, valid endpoint combinations, parsing, and ascending-order validation. The documented page selections can reuse those types with the existing comma-list parser/serializer and an all alternative. AGENTS.md explicitly prefers declarative Pydantic composition and native constraints over bespoke normalization/checking code.
Please build the payload field from those reusable Annotated aliases, unions, and constrained collections, following _AscendingPageRangeTuple: let Pydantic validate each endpoint, use a short BeforeValidator only for input adaptation, and retain an AfterValidator only for relations native constraints cannot express. Add the verified PostScript-specific syntax through narrow adapters or declarative constraints; preserve its accepted syntax and wire behavior without duplicating the shared rules or broadening other endpoints. Cover the composed payload and local rejection through both clients, including the endpoint-specific differences.
There was a problem hiding this comment.
Addressed in b92ea91. The PostScript field now parses into a list of the existing AscendingPageRange union, which composes PageNumber and _AscendingPageRangeTuple. The PostScript-specific adapter is limited to splitting/serializing the comma-delimited wire value and preserving the established user-facing validation message.
datalogics-kam
left a comment
datalogics-kam
left a comment
There was a problem hiding this comment.
Follow-up review by Codex using the repository's pr-review-auditor guidance, against 1efad816aab02cd62f5936cabbb3ac56c1013253.
Please address the two inline findings: the unsupported all,1 success case and missing assertions for the effects of rotation, shrink-to-fit, and annotation controls. The page-range finding is an acceptance criterion for the declarative composition already requested in the existing review; it does not prescribe retaining or modifying the separate regex/parser.
Validation on this unchanged head: the earlier review run passed 46 focused unit tests and 24 live tests on Python 3.11, plus Ruff, basedpyright, and diff checks. A live probe nevertheless showed that all,1 passes local validation and receives HTTP 400. The additional composition probe confirms that AscendingPageRange excludes all, while a standalone all alternative outside the collection can preserve that keyword and reject a mixed all selection. No source files were changed. The full local interpreter/example matrices were not rerun; current-head CI passed those checks.
| pytest.param("1-3", id="ascending-range"), | ||
| pytest.param("6-last", id="range-to-last"), | ||
| pytest.param("1, 3-5, 12, 14-last", id="mixed-selection"), | ||
| pytest.param("all,1", id="all-with-selection"), |
There was a problem hiding this comment.
[P2] Reject mixed all selections instead of certifying them as supported
This case passes the current payload model, but a live /postscript request with page_range="all,1" returns HTTP 400. Change it to a rejection case and verify that both clients fail before transport, consistent with AGENTS.md:122-127.
This complements the existing request to reuse the constrained page-range types: treat standalone all as an alternative to the collection of page selections, rather than a permitted collection member. AscendingPageRange itself excludes all; the surrounding composition can enforce this distinction without another parser. Preserve verified valid syntax and serialization while adding this regression coverage.
There was a problem hiding this comment.
Addressed in b92ea91. all is now a standalone union branch, so all,1 is rejected locally. The payload suite covers the rejection, and the sync and async client tests both verify that this input short-circuits before transport.
| 3, | ||
| 1, | ||
| ) | ||
| if expected_marker is not None: |
There was a problem hiding this comment.
[P2] Assert the effects of rotation, fitting, and annotation options
The rotate=True, shrink_to_fit=True, and print_annotations=False cases all supply expected_marker=None, so this branch skips the option-specific assertion. Both sync and async tests then check only generic metadata, the header, language level, and page count. Default output would satisfy those checks even if the service ignored the requested flags.
Use inputs where each flag changes the result and assert that effect through both transports: for example, printable annotations for the annotation toggle and suitable page geometry/scaling for rotation and fitting. This follows TESTING_GUIDELINES.md:232-235, which requires validating that the server honors optional arguments.
There was a problem hiding this comment.
Addressed in b92ea91.
print_annotationsnow has distinct sync and async live tests using a deterministic PDF with a printable annotation. Each test requests both values and verifies that the normalized PostScript output changes.- I investigated
rotateandshrink_to_fitdirectly against the current production service atapi.pdfrest.comusing five varied PDFs. The service accepts both options, but the tested outputs retain the same bounding boxes and rasterize identically with each option enabled or disabled. The previous smoke cases therefore could not prove those options worked and have been removed. - The production API defect is tracked in PDFCLOUD-6597. Live effect assertions for these two options can be restored after the service honors them.
|
I had Codex update the skill, see #56 |
- Compose PostScript page ranges from shared ascending selection types and reject combinations that mix all with explicit selections. - Replace annotation option smoke coverage with sync and async assertions against a deterministic printable annotation fixture. Assisted-by: Codex
This branch was successfully deployed
2 participants
PDFCLOUD-6570
Why this change
The Python SDK did not expose pdfRest's PDF-to-PostScript operation, requiring callers to assemble requests outside the typed client. This adds a supported sync and async workflow for converting uploaded PDFs into print-oriented PostScript.
What changed (high level)
The new
convert_pdf_to_postscripthelpers accept uploadedPdfRestFileresources and delegate request validation and serialization to a dedicated Pydantic payload model. A publicPostScriptLeveltype makes the supported language levels discoverable, while the existing file-operation machinery resolves the generated resource into a typed response.Documentation and runnable sync and async examples demonstrate the upload, conversion, download, and verification flow. The package version moves to 1.4.0 for the additive public API.
Behavior changes
Callers can now select PostScript level 2 or 3, page ranges, binary or text-safe output, scaling, rotation, shrink-to-fit behavior, printable annotations, and an optional output prefix. Omitting the output prefix leaves server-generated naming intact.
Invalid MIME types, multiple input resources, unsupported language levels, malformed page ranges, non-positive or non-finite scales, and invalid output prefixes fail locally before transport. There are no breaking changes to existing APIs.
Validation
Focused validation passed with 46 unit tests and 24 live sync/async tests against the local pdfRest service. The live suite isolates each route option and verifies concrete PostScript markers for text-safe output, language level, page selection, and the legal scale boundary.
The Python 3.10–3.14 non-live Nox matrix passed with 2,969 tests and 10 expected skips per interpreter at 95% overall coverage. The class-function coverage gate checked 165 functions and passed its 90% threshold. Ruff, configured basedpyright, strict MkDocs, package build, and diff checks also passed. The final rebase was patch-identical by
git range-diff, followed by another focused unit and live run.The pre-commit file checks passed except that its basedpyright subprocess could not discover installed dependencies in the sandbox; an equivalent full configured basedpyright run completed with zero errors or warnings.
Risks and follow-ups
The public API is additive and uses the existing request and response infrastructure, so migration risk is low. Live assertions use deterministic checked-in PDFs; rotation, shrink-to-fit, and annotation toggles are isolated and exercised through both transports, while binary-output and scale effects have direct output assertions. CI should provide the final clean-environment verification of the pre-commit hook and interpreter matrix.