Skip to content

fix(render): retain source tile indices for PDF subsets - #171

Open
OrdoAbChao7 wants to merge 1 commit into
StarTrail-org:mainfrom
OrdoAbChao7:fix/pdf-subset-page-numbers
Open

OrdoAbChao7 wants to merge 1 commit into
StarTrail-org:mainfrom
OrdoAbChao7:fix/pdf-subset-page-numbers

Conversation

@OrdoAbChao7

Copy link
Copy Markdown

Description

Preserve selected PDF pages' zero-based source tile indices. A full render maps page 3 to tile_0002.jpg, but pages=[3] previously wrote that page as tile_0000.jpg: pdf2image returns a range starting at page 3, while enumerate(images) restarted at zero.

Fix

Start enumeration at first_page - 1 and use idx + 1 for membership in the one-based page selection. Full-document tile identifiers and source-page mapping remain unchanged. Previously rendered subsets starting after page 1 need regeneration with their associated index metadata to adopt the corrected indices.

Regression tests

Eight offline cases cover full rendering, first/later/final pages, contiguous and sparse subsets, and unsorted input. They verify tiles.json, chunks.json, actual filenames, and selected image bytes against corresponding full-render tiles. Original Poppler integration tests are unchanged.

On upstream c4a0f53052306e009a024a2e344ede8aa5eda407: 5 failed, 3 passed (page 3: expected tile_0002.jpg, got tile_0000.jpg). With the fix: 8 passed. Removing the enumeration offset reproduces five failures; restoring it passes. A separate before/after full-render comparison found identical filenames, manifests, chunk metadata and mock-generated JPEG bytes.

Validation

Windows / Python 3.13.7, with PYTHONPATH=render/src:

  • python -m pytest tests/test_pdf_page_indices.py tests/test_pdf_manifest.py tests/test_chrome_paths.py tests/test_network_idle.py -q -p no:cacheprovider — 22 passed, 1 skipped. Original PDF integration module skips because pdf2image is unavailable.
  • uvx ruff check . — passed.
  • uvx ruff format --check . — passed.
  • git diff --check — passed.

Context

Focused PDF subset fix discussed in the maintainer review of #140, preserving the existing zero-based full-document layout. No linked issue.

@vercel

vercel Bot commented Oct 4, 2026

Copy link
Copy Markdown

@OrdoAbChao7 is attempting to deploy a commit to the andylizf's projects Team on Vercel.

A member of the Team first needs to authorize it.

This branch has not been deployed

No deployments
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