Skip to content

pdf: split the generator into files by responsibility, no byte moves - #152

Merged
donislawdev merged 1 commit into
mainfrom
pdf-package-split
Sep 29, 2026
Merged

donislawdev merged 1 commit into
mainfrom
pdf-package-split

Conversation

@donislawdev

@donislawdev donislawdev commented Sep 29, 2026 •

Copy link
Copy Markdown
Owner

The PDF generator is about to gain a dozen settings (metadata, dates, orientation, mixed paper sizes, page rotation, header version). internal/format/pdf/pdf.go already held seven responsibilities in 522 lines, so it is split first, on its own, where a byte change is easy to rule out.

file holds
pdf.go declaration, Plan, Write
settings.go page count and paper size
document.go objects, cross reference table, trailer
page.go page text, fixed line width, vocabulary
info.go information dictionary, string escaping
padding.go the comment block padding
minimum.go the floor and how a refusal explains it

Blocks moved verbatim, cut by content rather than line number. Nothing is reworded.

No byte moved. The golden values pin one PDF at default settings, which does not reach A5, several pages or an unlabelled file. So the previous and the new binary were compared on 300 requests: five paper sizes, 1, 2 and 24 pages, label on and off, two seeds, and five sizes each (below the floor, the floor, the one byte gap, two above, 64 KB). 100 files with equal SHA-256, 200 refusals with the same exit code and wording.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added PDF document generation with page content, optional labels, and support for A3, A4, A5, Letter, and Legal paper sizes.
    • PDF requests can specify between 1 and 5,000 pages; requests without page or paper-size settings default to one A4 page.

pdf.go held the declaration, the settings, the object layout, the page
text, the information dictionary, the padding and the minimum in 522
lines. Split by that list before new settings arrive, so each lands in
the file that owns it instead of pushing one file past the shape gates.

Blocks moved verbatim, cut by content. Compared against the previous
binary on 300 requests (five paper sizes, 1, 2 and 24 pages, label on
and off, two seeds, five sizes each from below the floor to 64 KB):
100 files with equal SHA-256, 200 refusals with the same exit code and
wording.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Advanced

Run ID: db83a433-bb93-40a1-8b91-52853a8614ef

📥 Commits

Reviewing files that changed from the base of the PR and between 1e06d3d and 6419940.

📒 Files selected for processing (7)
  • internal/format/pdf/document.go
  • internal/format/pdf/info.go
  • internal/format/pdf/minimum.go
  • internal/format/pdf/padding.go
  • internal/format/pdf/page.go
  • internal/format/pdf/pdf.go
  • internal/format/pdf/settings.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📜 Recent review details
⏰ Context from checks skipped due to timeout. (16)
  • GitHub Check: import table of the window binary
  • GitHub Check: coverage gate
  • GitHub Check: test on ubuntu-latest
  • GitHub Check: staticcheck
  • GitHub Check: test on macos-latest
  • GitHub Check: linters
  • GitHub Check: test on windows-latest
  • GitHub Check: semgrep
  • GitHub Check: bill of materials
  • GitHub Check: reference tools actually installed
  • GitHub Check: the Chocolatey packages install and leave
  • GitHub Check: known vulnerabilities
  • GitHub Check: the installer installs and leaves
  • GitHub Check: Analyze (actions)
  • GitHub Check: Analyze (go)
  • GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (9)
Applies to text shown to the user (labels, buttons, tooltips, placeholders, dialogs, errors, status messages, empty states, translations).

⚙️ CodeRabbit configuration file

Files:

  • internal/format/pdf/minimum.go
  • internal/format/pdf/document.go
  • internal/format/pdf/page.go
  • internal/format/pdf/settings.go
  • internal/format/pdf/padding.go
  • internal/format/pdf/info.go
  • internal/format/pdf/pdf.go
These are end-user desktop applications.

⚙️ CodeRabbit configuration file

Files:

  • internal/format/pdf/minimum.go
  • internal/format/pdf/document.go
  • internal/format/pdf/page.go
  • internal/format/pdf/settings.go
  • internal/format/pdf/padding.go
  • internal/format/pdf/info.go
  • internal/format/pdf/pdf.go
Performance is a known weak spot of these projects.

⚙️ CodeRabbit configuration file

Files:

  • internal/format/pdf/minimum.go
  • internal/format/pdf/document.go
  • internal/format/pdf/page.go
  • internal/format/pdf/settings.go
  • internal/format/pdf/padding.go
  • internal/format/pdf/info.go
  • internal/format/pdf/pdf.go
Applies only to code that builds or styles a GUI.

⚙️ CodeRabbit configuration file

Files:

  • internal/format/pdf/minimum.go
  • internal/format/pdf/document.go
  • internal/format/pdf/page.go
  • internal/format/pdf/settings.go
  • internal/format/pdf/padding.go
  • internal/format/pdf/info.go
  • internal/format/pdf/pdf.go
Domain: test file generator (Go; `tfg` CLI and `tfg-gui` Fyne window over one engine).

⚙️ CodeRabbit configuration file

Files:

  • internal/format/pdf/minimum.go
  • internal/format/pdf/document.go
  • internal/format/pdf/page.go
  • internal/format/pdf/settings.go
  • internal/format/pdf/padding.go
  • internal/format/pdf/info.go
  • internal/format/pdf/pdf.go
SECURITY, HIGH PRIORITY.

⚙️ CodeRabbit configuration file

Files:

  • internal/format/pdf/minimum.go
  • internal/format/pdf/document.go
  • internal/format/pdf/page.go
  • internal/format/pdf/settings.go
  • internal/format/pdf/padding.go
  • internal/format/pdf/info.go
  • internal/format/pdf/pdf.go
These apps are QA/developer tools.

⚙️ CodeRabbit configuration file

Files:

  • internal/format/pdf/minimum.go
  • internal/format/pdf/document.go
  • internal/format/pdf/page.go
  • internal/format/pdf/settings.go
  • internal/format/pdf/padding.go
  • internal/format/pdf/info.go
  • internal/format/pdf/pdf.go
Go code.

⚙️ CodeRabbit configuration file

Files:

  • internal/format/pdf/minimum.go
  • internal/format/pdf/document.go
  • internal/format/pdf/page.go
  • internal/format/pdf/settings.go
  • internal/format/pdf/padding.go
  • internal/format/pdf/info.go
  • internal/format/pdf/pdf.go
All code in this repository is written by an AI coding agent (Claude Code).

⚙️ CodeRabbit configuration file

Files:

  • internal/format/pdf/minimum.go
  • internal/format/pdf/document.go
  • internal/format/pdf/page.go
  • internal/format/pdf/settings.go
  • internal/format/pdf/padding.go
  • internal/format/pdf/info.go
  • internal/format/pdf/pdf.go
🔇 Additional comments (7)
internal/format/pdf/settings.go (1)

1-70: LGTM!

internal/format/pdf/pdf.go (1)

17-17: LGTM!

internal/format/pdf/minimum.go (1)

1-29: LGTM!

internal/format/pdf/page.go (1)

1-115: LGTM!

internal/format/pdf/info.go (1)

1-27: LGTM!

internal/format/pdf/document.go (1)

1-62: LGTM!

internal/format/pdf/padding.go (1)

1-101: LGTM!


📝 Walkthrough

Walkthrough

The PDF implementation is split into new helpers for settings, page content, metadata, padding, minimum-size calculation, and document serialization. The previous implementation is removed from pdf.go, where retained code still references removed helpers and definitions.

Changes

PDF generation

Layer / File(s) Summary
PDF settings and size helpers
internal/format/pdf/settings.go, internal/format/pdf/minimum.go, internal/format/pdf/pdf.go
Adds page-count and paper-size parsing, sorted supported names, and minimum-size messaging. Removes the former settings definitions and label-related messaging helpers from pdf.go.
Page content and document metadata
internal/format/pdf/page.go, internal/format/pdf/info.go
Adds page headings, seeded body text, optional footer labels, and PDF information dictionary generation with string escaping.
PDF serialization and padding
internal/format/pdf/document.go, internal/format/pdf/padding.go, internal/format/pdf/pdf.go
Adds PDF object serialization and cancellable comment padding. Removes the former implementation from pdf.go; retained code still references removed helpers and definitions.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to 64199

The PDF package builds after the split, with no identified issue requiring a fix before merge.

🚥 Pre-merge checks | ✅ 14
✅ Passed checks (14 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: splitting the PDF generator into files by responsibility without changing byte output. It is specific enough for release notes and is within the length lim…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Tests For Changed Behavior ✅ Passed The PR is a pure file split. The changed-file inventory contains only seven production Go files and no test additions, updates, deletions, or skips. Direct comparison of the base implementation with t…
No Secrets Or Debug Leftovers ✅ Passed No secrets or debug leftovers were introduced. The diff changes only seven Go files under internal/format/pdf; it adds no CLAUDE.md, AGENTS.md, .claude, or .env paths. Scans of added lines found no cr…
No Hardcoded Ui Styling ✅ Passed The pull request changes only Go files in internal/format/pdf, which implement PDF generation and settings. It does not add or modify GUI code in XAML, Slint, Fyne, Tkinter, or WPF. The hardcoded UI…
No Obvious Performance Problems ✅ Passed No clear performance problem is introduced. The authoritative diff only relocates the PDF generator code. Function-body comparison shows the moved implementations are identical to the base revision. T…
Desktop Robustness ✅ Passed The PR only splits existing PDF code into files. Write retains the existing context check and calls the existing cancellable writeComment; no new filesystem, working-directory, network, background…
Safe File Parsing ✅ Passed The changed PDF package only reads scalar request properties with strconv.Atoi and strings.ToLower, builds PDF bytes, and writes them through io.Writer. It adds no file reads, path resolution, d…
System Changes Are Reversible ✅ Passed PASS: The pull request only splits PDF generation into package files. The changed code writes PDF bytes to the provided io.Writer and uses in-memory buffers, strings, context cancellation, and request…
Clear User-Facing Text ✅ Passed The PR only relocates the PDF implementation. The before/after literal comparison found no changed user-facing strings; the only head-only literals are import paths. Error messages, hints, labels, pro…
No Resource Leaks ✅ Passed No resource leak was introduced. The changed PDF code only builds bounded in-memory document data and writes to the caller-owned io.Writer. pages is limited to 5000, padding uses a fixed 32 KiB bu…
Scope, Duplication And Docs ✅ Passed The PR is a scoped PDF refactor. The diff removes the existing PDF helpers from pdf.go and places them in the seven responsibility-based files named in the title and description. Plan, Write, registra…

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@donislawdev
donislawdev merged commit a632cd4 into main Sep 29, 2026
22 checks passed
@donislawdev
donislawdev deleted the pdf-package-split branch September 29, 2026 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant