PDFCLOUD-5839 Re-align tests with API by datalogics-cgreen · Pull Request #42 · pdfrest/pdfrest-python

datalogics-cgreen · 2026-06-02T15:16:31Z

Why this change

This branch closes two validation gaps in the SDK’s live and payload-level coverage.

First, convert_to_pdfa now accepts case-insensitive output_type strings at validation time while keeping the public type surface opinionated around the canonical PdfAType literals. That preserves IDE guidance and stricter typing for callers without forcing runtime inputs to already match the canonical case.

Second, the live signing suite was treating logo_opacity=0.0 as invalid even though zero opacity is a valid boundary value. That left the branch without a test proving the lower bound is accepted.

What changed

For PDF/A conversion, the payload validator now normalizes string output_type values case-insensitively before enforcing the existing PdfAType literal. The canonical allowed values are derived from get_args(PdfAType) so the validation logic does not maintain a second hard-coded list.

The PDF/A live suite was updated to add explicit sync and async lowercase input coverage, while preserving the canonical uppercase coverage already driven from PdfAType.

For signing, the live tests now separate the invalid-opacity coverage from the zero-opacity boundary. Negative values remain the lower-bound failure case, and new sync and async live tests assert that logo_opacity=0.0 succeeds.

Behavior changes

convert_to_pdfa now accepts inputs such as "pdf/a-2b" and normalizes them to the canonical PDF/A literal before request serialization. Invalid values still fail validation or server-side checks as before.

The sign-PDF live suite now reflects the intended boundary behavior for logo_opacity: values below 0.0 are rejected, while exactly 0.0 is treated as valid.

Validation

Validated locally with:

uv run pytest -n auto --maxschedchunk 2 tests/test_convert_to_pdfa.py
uv run ruff check tests/test_convert_to_pdfa.py
uv run basedpyright tests/test_convert_to_pdfa.py tests/live/test_live_convert_to_pdfa.py

Not run locally:

tests/live/test_live_convert_to_pdfa.py
tests/live/test_live_sign_pdf.py

Those live suites require pdfRest credentials and service access.

Risks and follow-ups

The main runtime change is intentionally narrow and limited to PDF/A output_type normalization. The larger risk is drift between live API behavior and the SDK’s expectations, which is why the added live coverage matters here.

This branch does not add broader unit coverage for lowercase PDF/A inputs beyond the existing focused request/validation tests, and it does not rerun the full nox matrix as part of this iteration.

netlify · 2026-06-02T15:16:39Z

✅ Deploy Preview for pdfrest-python ready!

Name	Link
🔨 Latest commit	`619a011`
🔍 Latest deploy log	https://app.netlify.com/projects/pdfrest-python/deploys/6a203be2eeac350008f193d2
😎 Deploy Preview	https://deploy-preview-42--pdfrest-python.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

datalogics-kam · 2026-06-02T19:08:09Z

Had to close and open to get the tests to run, go figure.

datalogics-kam

Changes to the PDF/A support.

Don't change the client interface to be open ended for PdfAType. It should still strictly declare canonical types. Type hinting is a hint, so the user can still pass a lower case str, and the low level validation will fix it.
I agree on the less strict validation before going to pdfRest, just avoid duplicating lists of values by using get_args.

datalogics-kam · 2026-06-02T19:15:33Z

        file: PdfRestFile | Sequence[PdfRestFile],
        *,
-        output_type: PdfAType,
+        output_type: PdfAOutputType,


This is a tough one; changing the name of the type is a breaking change to the interface.
Changing to a type that has | str removes type checking and IDE assistance.

It's totally okay that that the typing system of this interface is more strict than pdfRest. It can guide the user toward canonical use while still allowing non-conforming inputs.

If the user is using types strictly, and the IDE is assisting in choosing values, it'll suggest upper case and all is good. In this case, Literal works kind of like EnumStr, in that there's one best way to write it.

If the user isn't using types, they can put any string they want here, and the payload class will validate it. Even if it's lower case.

Please remove changes to the client interface here.

datalogics-kam · 2026-06-02T19:16:49Z

-#: PDF/A conformance targets accepted by ``convert_to_pdfa``.
+#: Canonical PDF/A conformance targets accepted by ``convert_to_pdfa``.
 PdfAType = Literal["PDF/A-1b", "PDF/A-2b", "PDF/A-2u", "PDF/A-3b", "PDF/A-3u"]
+#: Caller-facing ``convert_to_pdfa`` input type. Values are matched


Please just take out PdfAOutputType. Saying | str is as wide open as just str, so this isn't really constraining anything.

datalogics-kam · 2026-06-02T19:18:59Z

        response = client.convert_to_pdfa(
            input_file,
-            output_type="PDF/A-3b",
+            output_type="pdf/a-3b",


There's a new test that tests lowercase; I would suggest that this revert to uppercase as it was before.

datalogics-kam · 2026-06-02T19:19:06Z

        response = await client.convert_to_pdfa(
            input_file,
-            output_type="PDF/A-2u",
+            output_type="pdf/a-2u",


There's a new test that tests lowercase; I would suggest that this revert to uppercase as it was before.

datalogics-kam · 2026-06-02T19:35:59Z

+PDFA_OUTPUT_TYPES: tuple[PdfAType, ...] = (
+    "PDF/A-1b",
+    "PDF/A-2b",
+    "PDF/A-2u",
+    "PDF/A-3b",
+    "PDF/A-3u",
+)


Pllease avoid duplication here; it's possible to get the values from the literal

PDFA_OUTPUT_TYPES: tuple[PdfAType, ...] = get_args(PdfAType)

(you'll have to import get_args from typing)

datalogics-cgreen · 2026-06-02T22:03:09Z

This:

using Any casts only where tests intentionally bypass the public
type hints.

needs to come out of the commit message on bea3204.

Otherwise, I think it's improved.

- Normalize PDF/A `output_type` values case-insensitively during payload validation before enforcing the canonical `PdfAType` literals. - Derive the accepted canonical values from `get_args(PdfAType)` instead of maintaining a separate hard-coded list. - Update unit and live PDF/A tests to cover lowercase inputs. Assisted-by: Codex

- Add sync and async live tests proving `logo_opacity=0.0` succeeds. - Keep invalid-opacity cases focused on out-of-range values by supplying required signature names in the overridden payload. Assisted-by: Codex

datalogics-kam

Review findings from Codex PR Review Auditor. I did not rerun live tests because the previously attached live server was incorrect; the comments below are based on the diff, policy files, and focused mocked/unit coverage.

datalogics-kam · 2026-06-03T18:37:55Z

+    output_type: Annotated[
+        PdfAType,
+        Field(serialization_alias="output_type"),
+        BeforeValidator(_normalize_pdfa_output_type),


[P2] This changes local payload validation/serialization, but the branch only adds live coverage. Please add sync and async MockTransport tests, or payload-model tests plus client assertions, showing lowercase output_type normalizes to canonical PDF/A-2b in the outbound /pdfa body.

datalogics-kam · 2026-06-03T18:37:55Z

+                        "type": "new",
+                        "name": "live-logo-opacity-zero",
+                        "location": make_signature_location(),
+                        "logo_opacity": 0.0,


[P1] This succeeds only by overriding the serialized body with extra_body; callers using signature_configuration={..., "logo_opacity": 0.0} still hit the SDK's gt=0 validation. If zero is now valid, update the payload constraint and unit tests, then exercise the public argument path instead of bypassing it.

I think what Codex is saying here is that the logo opacity is still gt=0 in _internal.py, at line 1024, but should be ge=0 plus appropriate tests.

datalogics-kam

The PDF/A work looks a lot better, but I ran the PR Review Auditor skill and it found a few things; please address those.

datalogics-cgreen changed the title ~~Pdfcloud 5839 tests~~ PDFCLOUD-5839 Re-align tests with API Jun 2, 2026

datalogics-cgreen marked this pull request as ready for review June 2, 2026 15:17

datalogics-kam self-requested a review June 2, 2026 18:29

datalogics-kam self-assigned this Jun 2, 2026

datalogics-kam closed this Jun 2, 2026

datalogics-kam reopened this Jun 2, 2026

datalogics-kam temporarily deployed to ci-live June 2, 2026 19:02 — with GitHub Actions Inactive

datalogics-cgreen force-pushed the pdfcloud-5839-tests branch from a3a16ca to 54e584d Compare June 2, 2026 19:28

datalogics-cgreen temporarily deployed to ci-live June 2, 2026 19:28 — with GitHub Actions Inactive

datalogics-kam requested changes Jun 2, 2026

View reviewed changes

datalogics-cgreen force-pushed the pdfcloud-5839-tests branch from 54e584d to c4f7195 Compare June 2, 2026 20:40

datalogics-cgreen had a problem deploying to ci-live June 2, 2026 20:40 — with GitHub Actions Error

datalogics-cgreen force-pushed the pdfcloud-5839-tests branch from c4f7195 to b7bb76a Compare June 2, 2026 21:48

datalogics-cgreen had a problem deploying to ci-live June 2, 2026 21:48 — with GitHub Actions Error

datalogics-cgreen force-pushed the pdfcloud-5839-tests branch from b7bb76a to 18b4748 Compare June 2, 2026 21:55

datalogics-cgreen temporarily deployed to ci-live June 2, 2026 21:55 — with GitHub Actions Inactive

datalogics-cgreen added 2 commits June 3, 2026 09:35

test_live_sign_pdf.py: Cover zero logo opacity separately

619a011

- Add sync and async live tests proving `logo_opacity=0.0` succeeds. - Keep invalid-opacity cases focused on out-of-range values by supplying required signature names in the overridden payload. Assisted-by: Codex

datalogics-cgreen force-pushed the pdfcloud-5839-tests branch from 18b4748 to 619a011 Compare June 3, 2026 14:36

datalogics-cgreen temporarily deployed to ci-live June 3, 2026 14:36 — with GitHub Actions Inactive

datalogics-kam reviewed Jun 3, 2026

View reviewed changes

datalogics-kam requested changes Jun 3, 2026

View reviewed changes

Conversation

datalogics-cgreen commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why this change

What changed

Behavior changes

Validation

Risks and follow-ups

Uh oh!

netlify Bot commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for pdfrest-python ready!

Uh oh!

datalogics-kam commented Jun 2, 2026

Uh oh!

datalogics-kam left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

datalogics-cgreen commented Jun 2, 2026

Uh oh!

datalogics-kam left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

datalogics-kam left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

datalogics-cgreen commented Jun 2, 2026 •

edited

Loading

netlify Bot commented Jun 2, 2026 •

edited

Loading