Skip to content

FIX : Incorrect value mapping when filling multi-page PDFs in FireForm.#240

Open
Aryama-srivastav wants to merge 2 commits intofireform-core:mainfrom
Aryama-srivastav:#238
Open

FIX : Incorrect value mapping when filling multi-page PDFs in FireForm.#240
Aryama-srivastav wants to merge 2 commits intofireform-core:mainfrom
Aryama-srivastav:#238

Conversation

@Aryama-srivastav
Copy link

Description

This PR fixes incorrect value mapping when filling multi-page PDFs in FireForm.

Previously, the field-value index was reset inside the per-page loop in [filler.py], which caused page 2+ to restart from the first extracted value. As a result, multi-page forms could repeat early values and misalign later fields.
This change makes multi-page assignment sequential and stable:

  • initialize index once before iterating pages
  • continue incrementing index across all pages
  • preserve existing single-page behavior

Fixes #238

Type of change

Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • Test A (multi-page regression path):

Ran:
python -m pytest [test_filler_multi_page.py] -q

Verified output:
. [100%]
1 passed in 3.21s

Verified behavior:

  • page 1 gets first value
  • page 2 gets second value
  • no reset/reuse of first value on later pages

Test Configuration:

Firmware version: N/A
Hardware: Local development machine (Windows)
Python: 3.13
Shell: PowerShell

Checklist:

My code follows the style guidelines of this project:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Copilot AI review requested due to automatic review settings March 13, 2026 06:29
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a bug in FireForm’s PDF filling flow where multi-page PDFs incorrectly restarted the answer index on each page, causing field/value misalignment (Issue #238).

Changes:

  • Move the answer cursor initialization outside the per-page loop to keep assignment sequential across pages.
  • Add a regression test to validate value indexing across multiple pages.
  • Update the sample input text used for local/manual runs.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/filler.py Ensures the answer index (i) is initialized once and incremented across all pages.
src/test/test_filler_multi_page.py Adds a multi-page regression test using dummy PDF/page/annotation objects and monkeypatching.
src/inputs/input.txt Updates the sample input content (now includes a dated form-style example).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

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.

[BUG]: Multi-page PDF filling resets answer index and misaligns fields

2 participants