PDFCLOUD-5548 Add "Add to PDF" client methods by datalogics-cgreen · Pull Request #13 · pdfrest/pdfrest-python

datalogics-cgreen · 2026-01-20T15:46:46Z

Adds client support for pdfRest “add to PDF” operations (attachments, images, text) across sync/async APIs, including new payload/type models and request wiring.

Expands unit and live test coverage for each endpoint with validation, request customization, error handling, and async flows; add a small graphics test helper and PNG fixture to support image cases.

datalogics-kam

I was looking mostly for "code that Codex shouldn't have changed", so please consider this a pass 1.

Thanks for your effort here.

datalogics-cgreen · 2026-02-11T22:44:30Z

A lot of changes to _internal.py are now eradicated.

datalogics-kam

There are cases here where it could be tightened up with Pydantic.

Also, use good typing in the client interface. Sure, our system will let them violate type and send "3" to a parameter and Pydantic will fix it...but the type system should guide the coder to using accurate types.

datalogics-kam

Getting closer, but a few things to fix

Custom validation of RGB/CMYK is actually not needed (and it would remove two functions that caused coverage threshold failure)
In the public interface, numbers should not have str alternatives in the typing. Yes, it would work if mistyped in the code, but the type hinting should be accurate.

datalogics-kam

I think it's pretty ok in design, but I fall back on Codex for reviewing tests, and so could you ask Codex to fix up the tests?

Maybe supply it a ducky in other graphic formats.

Thanks!

Review Summary

Thanks for the updates here. I validated the MIME-coverage question and have one concrete testing gap to call out.

Findings

Image MIME variant coverage is not exhaustive yet (unit + live).
- PdfAddImagePayload allows image/jpeg, image/png, image/tiff, and image/gif (/Users/kam/src/dl/pdfrest-python/src/pdfrest/models/_internal.py:1163).
- Current unit tests cover jpeg, gif, and png, but I did not find a positive-path case for tiff.
  - jpeg: /Users/kam/src/dl/pdfrest-python/tests/test_add_image_to_pdf.py:25
  - gif: /Users/kam/src/dl/pdfrest-python/tests/test_add_image_to_pdf.py:274
  - png: /Users/kam/src/dl/pdfrest-python/tests/test_add_image_to_pdf.py:91
- Live tests currently exercise a single uploaded PNG fixture and do not enumerate all accepted image MIME variants:
  - /Users/kam/src/dl/pdfrest-python/tests/live/test_live_add_image_to_pdf.py:24

What I’m not flagging

I am not flagging the float/string coercion point as a correctness bug. Pydantic coercion there is valid unless strict validation is explicitly intended.
I am not flagging commit-subject style in this review.

Suggested follow-up

Add one unit positive test for image/tiff in test_add_image_to_pdf.py.
Optionally add a live parameterized MIME matrix (jpeg/png/tiff/gif) if we want full server-parity coverage for each accepted literal.

Assisted-by: Codex

Use explicit channel tuple types for add-text colors: - `text_color_rgb` -> `tuple[RgbChannel, RgbChannel, RgbChannel]` - `text_color_cmyk` -> `tuple[CmykChannel, CmykChannel, CmykChannel, CmykChannel]` Assisted-by: Codex

Assisted-by: Codex

- Remove `int` in favor of `float` wherever both were accepted - `page` requires literal `"all"`, not any `str` Assisted-by: Codex

- Rename to "is_right_to_left" for readability - Keep "is_rtl" alias Assisted-by: Codex

- Pydantic models get `model_dump_json` - Everything else gets `to_json` Assisted-by: Codex

Add missing sync and async tests for add_text_to_pdf local validation: - reject non-PDF input files - reject page values below 1 - reject text objects that provide both RGB and CMYK colors Assisted-by: Codex

Add high-value live assertions for add_image_to_pdf and add_text_to_pdf (sync + async): verify output file size is positive and warning is None. Assisted-by: Codex

Validators are outmoded by other validation Update tests Assisted-by: Codex

Assisted-by: Codex

datalogics-cgreen mentioned this pull request Jan 20, 2026

Pdfcloud 5464 add attachment #12

Closed

datalogics-gwalczak changed the title ~~Pdfcloud 5464 add to pdf~~ PDFCLOUD-5464 add to pdf Jan 21, 2026

datalogics-cgreen changed the title ~~PDFCLOUD-5464 add to pdf~~ PDFCLOUD-5548 add to pdf Jan 30, 2026

datalogics-cgreen changed the title ~~PDFCLOUD-5548 add to pdf~~ PDFCLOUD-5548 Add "Add Text", "Add Image" client methods Jan 30, 2026

datalogics-cgreen changed the title ~~PDFCLOUD-5548 Add "Add Text", "Add Image" client methods~~ PDFCLOUD-5548 Add "Add to PDF" client methods Jan 30, 2026

datalogics-cgreen force-pushed the pdfcloud-5464-add-to-pdf branch from 7c16b85 to 0a3602e Compare February 6, 2026 19:31

datalogics-cgreen marked this pull request as ready for review February 6, 2026 22:36

datalogics-cgreen assigned datalogics-kam Feb 6, 2026

datalogics-cgreen marked this pull request as draft February 10, 2026 17:51

datalogics-cgreen force-pushed the pdfcloud-5464-add-to-pdf branch from 915d5a5 to 36bf801 Compare February 10, 2026 21:13

datalogics-cgreen marked this pull request as ready for review February 10, 2026 21:17

datalogics-kam requested changes Feb 11, 2026

View reviewed changes

datalogics-cgreen force-pushed the pdfcloud-5464-add-to-pdf branch 2 times, most recently from 935272e to e6e9bff Compare February 11, 2026 22:43

datalogics-kam requested changes Feb 11, 2026

View reviewed changes

datalogics-kam reviewed Feb 12, 2026

View reviewed changes

Comment thread src/pdfrest/models/_internal.py Outdated

datalogics-cgreen force-pushed the pdfcloud-5464-add-to-pdf branch from 14af4c4 to dc01981 Compare February 12, 2026 19:17

datalogics-kam requested changes Feb 12, 2026

View reviewed changes

datalogics-kam requested changes Feb 13, 2026

View reviewed changes

datalogics-cgreen added 10 commits February 13, 2026 14:30

Add Add Attachment

b66b058

Assisted-by: Codex

Add tests of Add Attachment

5642140

Assisted-by: Codex

Add Add Image methods

d77d6d1

Assisted-by: Codex

Add tests of Add Image tool

f7b9703

Assisted-by: Codex

Add Add Text methods

3e348de

Assisted-by: Codex

Add tests of Add Text

06e624b

Assisted-by: Codex

Add to PDF tests: Fix testing guideline infractions

3b41fb1

Assisted-by: Codex

Replace test image file

af80d43

Ensure text_objects is stringified in testing

8b4be78

Assisted-by: Codex

Ensure string values within text_object string

aec924c

Assisted-by: Codex

datalogics-cgreen added 13 commits February 13, 2026 14:30

tests: fix instances where file ID generation attempted prefix of 3

5fceeeb

tests: add async validation parity for add-to-pdf endpoints

38d8daa

Assisted-by: Codex

models: Type add-text color fields as fixed channel tuples

dd4e596

Use explicit channel tuple types for add-text colors: - `text_color_rgb` -> `tuple[RgbChannel, RgbChannel, RgbChannel]` - `text_color_cmyk` -> `tuple[CmykChannel, CmykChannel, CmykChannel, CmykChannel]` Assisted-by: Codex

_internal.py: Replace page number validator with stricter type check

6885f4c

Assisted-by: Codex

public.py: Tighten Add Text typing

6c3a78f

- Remove `int` in favor of `float` wherever both were accepted - `page` requires literal `"all"`, not any `str` Assisted-by: Codex

Rename public-facing is_rtl to is_right_to_left

eb2eefd

- Rename to "is_right_to_left" for readability - Keep "is_rtl" alias Assisted-by: Codex

Replace json.dumps

a68e908

- Pydantic models get `model_dump_json` - Everything else gets `to_json` Assisted-by: Codex

tests: cover add-text local validation branches

24dd1d4

Add missing sync and async tests for add_text_to_pdf local validation: - reject non-PDF input files - reject page values below 1 - reject text objects that provide both RGB and CMYK colors Assisted-by: Codex

tests: strengthen live add-to-pdf response assertions

bd49bed

Add high-value live assertions for add_image_to_pdf and add_text_to_pdf (sync + async): verify output file size is positive and warning is None. Assisted-by: Codex

_internal.py: Remove RGB, CMYK validators

0d5939a

Validators are outmoded by other validation Update tests Assisted-by: Codex

public.py: Reject str values where only float should be expected

60d15c1

Assisted-by: Codex

tests: Add output to Add Image, Add Text test payloads

3f9d6de

Assisted-by: Codex

tests: Round out test image formats with new inputs

5e04b1e

Assisted-by: Codex

datalogics-cgreen force-pushed the pdfcloud-5464-add-to-pdf branch from 3043301 to 5e04b1e Compare February 13, 2026 20:38

datalogics-kam approved these changes Feb 13, 2026

View reviewed changes

datalogics-kam merged commit ea5eb66 into pdfrest:main Feb 13, 2026
14 checks passed

datalogics-cgreen deleted the pdfcloud-5464-add-to-pdf branch February 13, 2026 21:32

Conversation

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

Uh oh!

Uh oh!

datalogics-kam left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

datalogics-cgreen commented Feb 11, 2026

Uh oh!

datalogics-kam left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

datalogics-kam left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

datalogics-kam left a comment

Choose a reason for hiding this comment

Review Summary

Findings

What I’m not flagging

Suggested follow-up

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

datalogics-cgreen commented Jan 20, 2026 •

edited

Loading