Conversation
…fairs/vets-api into rcassity-debug-AcroForm-warnings
| doc = HexaPDF::Document.open(pdf_path) | ||
| field = doc.acro_form&.field_by_name(SIGNATURE_FIELD_NAME) | ||
| form = doc.acro_form | ||
| raise 'No AcroForm found in PDF template.' if form.nil? |
There was a problem hiding this comment.
maybe we could put this inside the flipper since it is changing behavior?
There was a problem hiding this comment.
Let me revert that change
jweissman
left a comment
jweissman
left a comment
There was a problem hiding this comment.
lgtm and seems safe to enable (although maybe we could put the new raise inside the flipper for now to make sure we don't conflate our debugging with introducing a new issue?)
There was a problem hiding this comment.
Pull request overview
This PR adds opt-in debug logging for AcroForm PDF processing to help diagnose problematic PDF fields that generate warnings in staging. The feature is controlled by the acroform_debug_logs feature flag and aims to make AcroForm warnings traceable by logging template paths and identifying fields missing appearance streams.
Changes:
- Added feature flag
acroform_debug_logsto enable debug logging for AcroForm processing - Implemented logging in 5 PDF processing modules to log template paths when the flag is enabled
- Added field inspection logic in one module to detect and log problematic fields (missing AP on Btn fields)
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| config/features.yml | Adds the acroform_debug_logs feature flag configuration |
| lib/pdf_fill/filler.rb | Adds template path logging in fill_form_with_hexapdf when flag is enabled |
| modules/survivors_benefits/lib/survivors_benefits/pdf_fill/va21p534ez.rb | Adds template path logging in signature_overlay_coordinates_for when flag is enabled |
| modules/medical_expense_reports/lib/medical_expense_reports/pdf_fill/va21p8416.rb | Adds template path logging and problematic field detection in signature_overlay_coordinates when flag is enabled |
| modules/increase_compensation/lib/increase_compensation/pdf_stamper.rb | Adds template path logging in signature_overlay_coordinates and changes form field key from 'statementOfTruthSignature' to 'statement_of_truth_signature' |
| modules/decision_reviews/lib/decision_reviews/pdf_template_stamper.rb | Adds template path logging in fill_and_write_pdf when flag is enabled |
| if Flipper.enabled?(:acroform_debug_logs) | ||
| Rails.logger.info("SurivorsBenefits::PdfFill::Va21p534ez HexaPDF template: #{pdf_path}") | ||
| end |
There was a problem hiding this comment.
The new logging behavior when the acroform_debug_logs feature flag is enabled lacks test coverage. The existing tests for signature_overlay_coordinates stub the method, which means the new logging code is not exercised. Add tests that verify the logging behavior both when the flag is enabled and disabled, following the pattern established in the codebase for feature flag testing.
| if Flipper.enabled?(:acroform_debug_logs) | ||
| Rails.logger.info("IncreaseCompensation::PdfStamper HexaPDF template: #{pdf_path}") | ||
| end |
There was a problem hiding this comment.
The new logging behavior when the acroform_debug_logs feature flag is enabled lacks test coverage. The minimal existing tests for PdfStamper don't exercise the signature_overlay_coordinates method. Add tests that verify the logging behavior both when the flag is enabled and disabled, following the pattern established in the codebase for feature flag testing.
| if Flipper.enabled?(:acroform_debug_logs) | ||
| Rails.logger.info("DecisionReviews::PdfTemplateStamper #{@template_path}") | ||
| end |
There was a problem hiding this comment.
The new logging behavior when the acroform_debug_logs feature flag is enabled lacks test coverage. The existing tests for PdfTemplateStamper don't verify this logging behavior. Add tests that verify the logging occurs when the flag is enabled and doesn't occur when disabled, following the pattern established in the codebase for feature flag testing.
| # | ||
|
|
||
| def fill_form_with_hexapdf(template_path, output_path, hash_data) | ||
| Rails.logger.info("PdfFill::Filler HexaPDF template: #{template_path}") if Flipper.enabled?(:acroform_debug_logs) |
There was a problem hiding this comment.
The new logging behavior when the acroform_debug_logs feature flag is enabled lacks test coverage. The existing tests for fill_form_with_hexapdf don't verify this logging behavior. Add tests that verify the logging occurs when the flag is enabled and doesn't occur when disabled, following the pattern established in the codebase for feature flag testing.
Keep your PR as a Draft until it's ready for Platform review. A PR is ready for Platform review when it has a teammate approval and tests, linting, and settings checks pass CI. See these tips on how to avoid common delays in getting your PR merged.
Summary
This PR adds opt-in debug logging around AcroForm PDF processing to surface problematic or unsupported fields encountered at runtime. While investigating staging behavior after recent changes, we observed hundreds of repeated warnings related to AcroForm fields (e.g.,
Btnfields without appearance streams). These logs were difficult to trace back to specific templates or fields.The goal of this PR is to make these issues actionable and traceable without impacting normal production log volume.
What’s happening today
In staging, we are seeing repeated warnings such as:
WARNING: AcroForm field 'Btn' with no AP not implementedRadio = FALSE, Pushbutton = FALSEThese warnings:
What this PR changes
Introduces a feature-flagged debug mode (
acroform_debug_logs)When enabled, logs:
Keeps existing behavior unchanged when the flag is disabled
Avoids throwing errors or changing PDF generation behavior
This keeps production safe while allowing deeper inspection in staging or during investigations. Pods will shutdown after these warnings. The warnings are not causing the pod to shutdown, assuming there is a job or service causing memory leaks to due PDF failures.
Why this matters
This PR is intentionally diagnostic, not a behavioral change.
How to test
Enable the feature flag:
Trigger PDF generation for affected forms
Confirm logs now include:
Disable the flag and confirm logging returns to baseline behavior
Scope / Risk
Related issue(s)
Testing done
Screenshots
Note: Optional
What areas of the site does it impact?
(Describe what parts of the site are impacted andifcode touched other areas)
Acceptance criteria
Requested Feedback
(OPTIONAL)What should the reviewers know in addition to the above. Is there anything specific you wish the reviewer to assist with. Do you have any concerns with this PR, why?