Add background PDF generation for published document versions#1065
Merged
SachaProbo merged 1 commit intomainfrom Apr 23, 2026
Merged
Add background PDF generation for published document versions#1065SachaProbo merged 1 commit intomainfrom
SachaProbo merged 1 commit intomainfrom
Conversation
c9b16b7 to
096c21f
Compare
096c21f to
265ade0
Compare
Contributor
There was a problem hiding this comment.
8 issues found across 16 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="e2e/console/document_version_test.go">
<violation number="1" location="e2e/console/document_version_test.go:1509">
P2: This subtest never creates any signatures, so `withSignatures: true` does not exercise the signature-page export path.</violation>
</file>
<file name="pkg/server/api/mcp/v1/specification.yaml">
<violation number="1" location="pkg/server/api/mcp/v1/specification.yaml:5350">
P2: `DocumentVersion.file_id` should use the shared `GID` schema instead of a raw `string|null` so this ID-like field stays consistent with the API’s canonical identifier contract.</violation>
</file>
<file name="apps/console/src/components/documents/PdfDownloadDialog.tsx">
<violation number="1" location="apps/console/src/components/documents/PdfDownloadDialog.tsx:66">
P2: `withSignatures` is derived from `isPublished` only on the first render, so publish/version transitions can submit a stale signature setting.</violation>
</file>
<file name="pkg/server/api/console/v1/document_resolvers.go">
<violation number="1" location="pkg/server/api/console/v1/document_resolvers.go:1640">
P3: Return the matched domain error here instead of the wrapped service error to avoid leaking the internal `cannot export document PDF` prefix in the client-facing GraphQL message.</violation>
</file>
<file name="pkg/coredata/document_version.go">
<violation number="1" location="pkg/coredata/document_version.go:579">
P1: This lock-based claim is released before processing starts, so multiple workers can pick the same published version and generate duplicate PDFs.</violation>
</file>
<file name="pkg/probo/document_service.go">
<violation number="1" location="pkg/probo/document_service.go:2126">
P2: Published versions without a stored PDF ignore `WithSignatures`, so downloads can be missing the signature page until the background job finishes.</violation>
<violation number="2" location="pkg/probo/document_service.go:2683">
P2: Don't hold a DB connection across HTML/PDF rendering here; load the data first and render after releasing the connection.
(Based on your team's feedback about splitting data fetching and PDF generation.) [FEEDBACK_USED]</violation>
</file>
<file name="pkg/probo/document_pdf_worker.go">
<violation number="1" location="pkg/probo/document_pdf_worker.go:64">
P1: This claim path releases the `FOR UPDATE SKIP LOCKED` lock before any claimed/processing state is saved, so concurrent workers can pick the same document version and generate/upload the PDF twice.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
c5b8e93 to
57f9e39
Compare
e46bcec to
1d8652c
Compare
Contributor
There was a problem hiding this comment.
2 issues found across 14 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pkg/coredata/migrations/20260416T160000Z.sql">
<violation number="1" location="pkg/coredata/migrations/20260416T160000Z.sql:20">
P2: Keep the `pdf_attempt_count` default until all existing `document_versions` writers are updated.</violation>
</file>
<file name="pkg/probo/document_service.go">
<violation number="1" location="pkg/probo/document_service.go:2165">
P2: When exporting generated PDFs with both signatures and watermark enabled, the signature page is appended after watermarking and remains unwatermarked.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
1d8652c to
6eee642
Compare
Move PDF generation from synchronous publish flow to a background polling job. Published versions with file_id IS NULL are picked up by the job, which generates the PDF, uploads to S3, and links the file. Export PDF now serves stored files for published versions (with optional signature page and watermark) and generates on the fly for drafts. Signed-off-by: Sacha Al Himdani <sacha@getprobo.com>
6eee642 to
25c590f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Move PDF generation from synchronous publish flow to a background polling job. Published versions with file_id IS NULL are picked up by the job, which generates the PDF, uploads to S3, and links the file. Export PDF now serves stored files for published versions (with optional signature page and watermark) and generates on the fly for drafts.
Summary by cubic
Move publication PDF generation to a background worker and serve stored PDFs for published versions. This speeds up publishing and makes exports more reliable.
New Features
document-pdf-worker: runs every 30s; claims published versions withfile_id IS NULLandpdf_attempt_count < 3, increments attempts, generates the PDF, uploads to S3, and setsfile_id; started byprobod.pdfutils.MergePDFs) and watermark; if not stored yet, generate on the fly. Drafts → generate on the fly with optional signatures. Watermark is applied after signatures are merged. Added E2E tests for signatures on drafts and published versions.docgen.RenderSignaturePageHTML) appended to PDFs; respects document orientation.pdfutils.watermarkpdftopdfutilsand addedMergePDFs.Migration
file_idandpdf_attempt_counttodocument_versions(file_idreferencesfiles(id)). The worker backfills PDFs for existing published versions automatically.Written for commit 25c590f. Summary will update on new commits.