feat(export): produce every export format, choose one when downloading - #99
Merged
Conversation
Summary: - add opt-in "One PDF per page" and "One JPEG per page" export options - render each book page into its own print-ready PDF with the same page boxes, embedded fonts, and FOGRA51 output intent as the whole book - rasterize the exported PDF to 300 PPI JPEGs with PDFium and Sharp - store both bundles as ZIPs next to the PDF and preflight report and serve them from /api/exports/:id - record the bundle object keys so project deletion cleans them up Rationale: - print shops and photo services ask for one file per page, so organizers had to split the exported book themselves - bundles are built only after preflight passes, and only when asked for, so a rejected or plain export costs nothing extra - ZIP entries are stored rather than deflated because PDF and JPEG payloads are already compressed Tests: - bun run format:check, lint, typecheck, build - 43 test files and 355 unit tests passed - repository.integration and contributor-drafts were not run: they need Docker services that this environment cannot start, and contributor-drafts already fails on origin/main here - end-to-end suite not run locally for the same reason; CI covers it Closes #91 AI-Assisted: true AI-Agent: claude-code AI-Model: anthropic/claude-opus-5
Closed
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Summary: - ignore .vscode/** in oxfmt Rationale: - .vscode/PythonImportHelper-v2-Completion.json landed on main in f14a8f7 and fails format:check, so verify never reaches the real checks - the file is regenerated editor state, so formatting it would only break again on the next write Tests: - bun run format:check Related to #91 AI-Assisted: true AI-Agent: claude-code AI-Model: anthropic/claude-opus-5
patriksimms
marked this pull request as ready for review
August 29, 2026 16:36
Owner
Author
|
Measured trade-offs while reviewing the final diff, for the human reviewer:
|
Summary: - recapture the before/after export step from the running application - replace the JPEG sample with a page taken from a real exported bundle Rationale: - the first pair was captured from an isolated component because no Docker services were available; the app shell was missing from it - the new pair comes from an actual export against Postgres and RustFS, so the downloads shown are real artifacts Tests: - e2e capture run against the seeded demo project on both main and this branch Related to #91 AI-Assisted: true AI-Agent: claude-code AI-Model: anthropic/claude-opus-5
Summary: - drop the per-format switches; one export produces the book, one PDF per page, one JPEG per page, and the preflight report - list all four as labelled downloads on the completed export - build the per-page PDFs by copying the preflighted book's pages instead of rendering every page a second time - deflate the output intent profile once per export instead of once per produced page Rationale: - asking which formats to produce before the render made organizers predict what a printer would want; the choice belongs at download time - always producing the bundles makes their cost matter, and copying pages cuts a 40-page bundle from 5.2 s to 0.5 s while guaranteeing the files are the very pages preflight accepted - the profile is 2 MiB, so compressing it per page dominated the export Tests: - SAKEKEEP_E2E_PORT=3100 bun run format:check, lint, typecheck, test, test:e2e, build - 367 unit tests pass; contributor-drafts fails identically on origin/main in this environment - 22 end-to-end tests passed, 2 intentionally skipped Related to #91 AI-Assisted: true AI-Agent: claude-code AI-Model: anthropic/claude-opus-5
patriksimms
changed the title
Summary: - keep main's cover/standalone layout rework in `requiredCuts` while this branch's page-subset signature stays - regenerate the export ZIP columns migration on top of main's layout role migration so the drizzle journal stays a single chain - move this branch's standalone page fixtures onto layout-backed pages Rationale: - main dropped the `pageType`/`title`/`background` standalone page shape in favour of layouts carrying a role, so the split-page tests had to follow - two migrations claimed idx 5; regenerating ours as idx 6 keeps the snapshot chain valid instead of hand-merging the journal Tests: - bun run typecheck - bun run format:check - bun run lint - bun run test (contributor-drafts Blob assertion fails on main too) AI-Assisted: true AI-Agent: claude-code AI-Model: anthropic/claude-opus-5
Summary: - render, zip, and upload each page bundle in one streaming pass instead of collecting every page and the finished archive first - claim the export object keys as tombstones before uploading and hand them over to the export row in the same commit that writes it - hold the orphan sweep off tombstones younger than an hour Rationale: - `renderBookPagePdfs` and `renderPageJpegs` returned full arrays, so a book export held every page PDF, every 300 PPI JPEG, and both complete ZIPs at once; a long book could exhaust the server heap. Pages are now produced lazily, the archive is emitted as it grows, and a multipart upload consumes it, so peak cost is one page plus one upload part rather than the bundle - nothing discovers a stored object except its export row, so a failure in a bundle upload or in `recordExport` stranded files that neither `deleteProject` nor `cleanupOrphanedObjects` could ever reach - reserving keys up front covers a crash as well as a thrown error, but only works if the sweep cannot delete a write that is still uploading, hence the grace period; `deleteProject` already deletes eagerly, so it loses nothing Tests: - bun run typecheck && bun run format:check && bun run lint - bun run test (contributor-drafts Blob assertion fails on main too) - bun run test:e2e - verified a 12 MiB streamed bundle round-trips through the local object store in two multipart parts Addresses the review comments on server/export-service.ts:88 and :119. Related to #91 AI-Assisted: true AI-Agent: claude-code AI-Model: anthropic/claude-opus-5
…n object Summary: - claim a tombstone by deleting its row and acting only on what came back - `recordExport` now takes its reservation back before writing the export row and fails with 409 if any key was already claimed - `deleteProject` and the sweep follow the same rule, returning whatever they could not delete instead of dropping the row regardless - correct the README description of what the grace period does Rationale: - ordering the sweep behind a one-hour grace period was not exclusion: the sweep selected keys, then deleted objects in a separate step, so an export committing in between produced an export row whose files were gone. An export running past the grace period had the same effect - a row that both parties must delete gives real mutual exclusion for free — `DELETE ... RETURNING` is atomic, so whoever commits first owns the keys and the loser sees them missing and backs off - the sweep previously dropped tombstones for objects it had failed to delete only on success, but `deleteProject` dropped them unconditionally, which could discard a row the sweep had just put back; both now re-insert failures with the age they had, so a failing object keeps its place in the queue - the grace period stays, but only as the window in which an export is certain to win its own keys; correctness no longer rests on it Tests: - bun run typecheck && bun run format:check && bun run lint - bun run test (contributor-drafts Blob assertion fails on main too) - bun run test:e2e - new integration tests: a swept reservation refuses the export row, and a sweep racing `recordExport` on the same keys leaves exactly one winner Addresses the review comment on server/repository.ts:649 and README.md:198. Related to #91 AI-Assisted: true AI-Agent: claude-code AI-Model: anthropic/claude-opus-5
…delete Summary: - claim a tombstone by stamping `claimed_at` instead of deleting the row - drop the row only once the delete came back clean; clear the stamp when it did not, so a refused object is retried immediately - treat a claim older than fifteen minutes as abandoned and offer it again - `recordExport` clears only unclaimed tombstones, so a key a deleter has taken on still fails the export with 409 Rationale: - claiming by deleting the row lost the only record that the object still needed deleting: a process that died between that commit and the object store call stranded the object with nothing left to retry it - the row has to outlive the claim to be a work queue, but then a deleter that dies leaves the row claimed forever, hence the lease - `claimed_at` is still a single atomic update, so the mutual exclusion the previous commit relied on is unchanged; only the bookkeeping moved - a claim in progress is as good as a completed one from the export's side, because the object may already be gone by the time it looks Tests: - bun run typecheck && bun run format:check && bun run lint - bun run test (contributor-drafts Blob assertion fails on main too) - bun run test:e2e - new integration tests: a claimed tombstone survives a sweep and is retried once its lease expires, and an export refuses to record while a deleter holds its keys - replayed the whole migration chain onto an empty database Addresses the review comment on server/repository.ts:652. Related to #91 AI-Assisted: true AI-Agent: claude-code AI-Model: anthropic/claude-opus-5
Summary: - carry the claim stamp out of `claimTombstones` and condition the row delete on it, so a holder that overran its lease cannot settle the claim that took over from it - stop clearing `claimed_at` on a failed delete; the object is retried by whoever claims it next Rationale: - the settle step matched on object key alone, so a stalled deleter returning after its lease had expired would clear the newer holder's stamp, and an unclaimed tombstone is one `recordExport` may take ownership of — while that newer deleter was still removing the files - a re-claim can only happen a lease after the previous one, so no key ever carries the same stamp twice and the stamp alone tells two holders apart; no extra column is needed for the token - handing a refused object straight back had the same shape of problem: it returned a key a deleter had already been at to the unclaimed state. Leaving the stamp makes "once claimed, never unclaimed" an invariant, which is what lets `recordExport` treat an unclaimed tombstone as safe to take - the cost is that a refused delete waits out the lease instead of retrying at once, which is nothing next to the sweep's own schedule Tests: - bun run typecheck && bun run format:check && bun run lint - bun run test (contributor-drafts Blob assertion fails on main too) - bun run test:e2e - new integration test: a taken-over claim gets a fresh stamp and the previous holder's settle matches nothing; a live claim survives a sweep untouched Addresses the review comment on server/repository.ts:698. Related to #91 AI-Assisted: true AI-Agent: claude-code AI-Model: anthropic/claude-opus-5
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
1 participant
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.
Outcome
One export now produces every format, and the organizer picks what to download
afterwards instead of predicting it beforehand:
The only pre-export option left is the crop-marks switch, because that one
changes what gets rendered. Closes #91.
How it works
preflighted book with
splitBookPagePdfs, so each file is the very page thereport accepted. A copied page loses catalog-level data, so the FOGRA51
output intent and PDF/X metadata are re-applied per document.
src/server/page-raster.tsrasterizes the same book PDF with PDFium(WebAssembly, no native dependency) and encodes with Sharp.
src/server/zip.tsbundles entries withfflate, stored rather thandeflated because PDF and JPEG payloads are already compressed.
produced page.
and report. Their object keys are recorded on the export row, so
deleteProjectcleans them up.GET /api/exports/:id?file=page-pdfs|page-jpegsserves them, and returns 404for an export created before this change.
Cost of producing everything
Measured on a text-only fixture book, per export:
Two things worth a decision:
book makes the export request correspondingly slow.
own 1.7 MiB output intent — that is what makes each file print-ready on its
own. Dropping the intent would shrink a 40-page bundle from 83 MiB to about
16 MiB, at the cost of the pages no longer being PDF/X-targeted.
Visual evidence
Before/after of the export step, attached to #91 and committed under
visual-artifacts/issues/91/. Both were captured from the running applicationagainst Postgres and RustFS, with a real export in the "after" shot, so the
download buttons point at artifacts that actually exist. The sample JPEG is
page 2 taken straight out of the exported
sakekeep-pages-jpeg.zip.Validation
bun run format:check,bun run lint,bun run typecheck,bun run buildsplitter, the rasterizer, the ZIP writer, the export service wiring, and the
panel switches
repository.integration.test.tsandcontributor-drafts.test.tscould notrun locally (no Docker);
contributor-draftsalready fails the same way onorigin/mainhere. CI runs both plus the end-to-end suite.Deployment
No changes outside the repository beyond the usual migration step: the
exportstable gains two nullable columns(
drizzle/20260829161535_fast_master_chief.sql), applied by the existingdb:migratejob. Two new runtime dependencies (@hyzyla/pdfium,fflate) areplain node_modules packages, so the Docker image needs no extra system
packages.