Skip to content

fix: fill radio button groups with the chosen option - #736

Merged
marcvergees merged 2 commits into
fireform-core:developmentfrom
Anandb71:fix-radio-button-groups
Oct 26, 2026
Merged

marcvergees merged 2 commits into
fireform-core:developmentfrom
Anandb71:fix-radio-button-groups

Conversation

@Anandb71

Copy link
Copy Markdown
Contributor

Description

Radio button groups are never filled with the right option. The buttons of a group share one field name, and write_pdf treated each button like a checkbox. For an answer of "Yes", every button got its own on state and the last one won. Any other answer turned the whole group off. The schema also sent each group to the model as a free-form {"type": "string"}, so the model never saw the options.

On OSHA's fillable recordkeeping package (https://www.osha.gov/sites/default/files/OSHA-RK-Forms-Package.pdf, page 10 is the Form 301 injury and illness incident report), with the model's answer patched in:

Field Model answer development This PR
5) Male / Female Female nothing checked Female
8) Was employee treated in an emergency room? Yes No Yes
9) Was employee hospitalized overnight as an in-patient? No nothing checked No
13) Time of event: AM / PM / Check if time cannot be determined PM nothing checked PM

I checked this by reading each button's /AS back, and by rendering the page with PDFium, the engine behind the preview iframe.

Radio buttons were asked for in #142 and #493, both closed. The current filler sends them down the checkbox path, which cannot pick one option of a group. None of the benchmark PDFs has a radio group, which is probably why it didn't show up.

This change:

  • template.create_template() reads each radio group's options, the state each of its buttons turns on (/Male, /Female). It gives them to the model as an enum plus "Off", the same way checkboxes get "Yes" / "Off". The new helpers are read_radio_options() and add_radio_options(), next to the tooltip ones.
  • filler.write_pdf() gives every button of a group the chosen option's state. pypdf then turns on the button that has that state and turns the others off. "Off", "" or a missing answer leaves the group empty. Checkboxes are unchanged.

Forms without radio groups get exactly the same schema as before. I compared create_template() output (schema, tables and groups) on development and on this branch for the 11 benchmark PDFs, data/inputs/, and 9 more FEMA ICS forms (210, 211, 214, 215, 215A, 218, 220, 221, 225). All 21 are identical, so the benchmark is not affected.

Not covered: geometry.row_lists() still reads some rows of 3 or more radio buttons as a one-column table, so those groups keep a string column. On the OSHA 300 log, that is the classification and injury-type buttons of every case row after the first (18 groups). That needs a change in the geometry, so I left it out of this PR.

Related: #142, #493

Type of change

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

How Has This Been Tested?

Repro on development (5bab407) with the form the new tests write: a text field and two radio groups, Gender (Male/Female) and ER (Yes/No), laid out the way Acrobat writes them. The model's answer {"Gender": "Female", "ER": "Yes"} gives:

Gender: Male off, Female off
ER:     Yes off,  No on

With the change: Gender has Female on, and ER has Yes on.

  • New test_radio_groups_offer_their_options: the schema gives Gender the enum ["Male", "Female", "Off"] and ER the enum ["Yes", "No", "Off"]. The text field stays a string. Fails on development with KeyError: 'enum'.
  • New test_fill_turns_on_the_chosen_radio_button: runs fill() with the model call patched. The schema sent to the model has the options, and the filled PDF has Female and Yes on, with the other buttons off. Fails on development with KeyError: 'enum'. With only the schema half of the fix, it fails on the PDF instead: {'/Female': '/Off'} != {'/Female': '/Female'}.
  • New test_radio_group_answered_off_stays_unselected: "Off" and "" leave both groups empty. This passes before and after the change, and guards the new branch in write_pdf.
  • OSHA 301 end to end, as in the table above, plus the schema comparison over the 21 forms.

python -m pytest tests/ → 316 passed. ruff check app/ → All checks passed.

Test Configuration:

  • Python version: 3.11.17
  • Docker/Compose version: not needed (the new tests write their own PDF and patch the model call)
  • OS: Linux

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 (none needed)
  • 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
  • The Lint workflow check passes on this PR ✅
  • The Tests workflow check passes on this PR ✅

The buttons of a radio group share one field name. write_pdf treated
each button like a checkbox, so an answer of "Yes" gave every button
its own on state and the last one won, and any other answer left the
group empty. On OSHA Form 301, "Was employee treated in an emergency
room?" answered "Yes" came out as "No". The schema also gave the model
a free-form string, so it never saw the options.

A radio group is now offered its buttons' states as an enum, plus
"Off", the way checkboxes get "Yes" and "Off". write_pdf gives the
whole group the chosen state, and pypdf turns on the button that has
it. Groups that geometry reads as table columns are unchanged.
Copilot AI balanced review requested due to automatic review settings October 24, 2026 15:37

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

@marcvergees

Copy link
Copy Markdown
Member

Hi @Anandb71,

Thanks for the PR, but we cannot accept this change.

Your points do not hold up regarding how our repository and pipeline actually handle form fields. In our experience and throughout all our real-world use cases, the current implementation works completely fine and as expected.

Please make sure to properly familiarize yourself with the codebase, how our schemas are structured, and test with actual real-world production cases before asserting issues that do not exist in practice. Don't use AI for creating false problems.

Closing this PR for now.

Marc

@Anandb71

Copy link
Copy Markdown
Contributor Author

Thanks for taking a look, and fair enough if it doesn't fit your pipeline. For context, I hit this on OSHA's public Form 301: answering "Yes" to the ER question gets filled in as "No", because the radio group's export values don't match the option that was picked. Happy to share the test PDF and the steps to reproduce it if that would help. Otherwise, no worries.

@marcvergees

Copy link
Copy Markdown
Member

Hi @Anandb71, thanks for bringing it up, could you show us this edge example case in which it didn't work out.

Thanks,
M

@Anandb71

Copy link
Copy Markdown
Contributor Author

Sure, here it is. It only uses FireForm's own write_pdf() and the public OSHA form, no LLM call.

PDF: OSHA recordkeeping forms package, https://www.osha.gov/sites/default/files/OSHA-RK-Forms-Package.pdf (page 10 is Form 301). Field 301 ER is question 8, "Was employee treated in an emergency room?". It's a radio group with two buttons: one whose on-state is /Yes and one whose on-state is /No.

Steps (from the root of a FireForm checkout, after pip install -r requirements.txt):

  1. Save the script below as repro.py.
  2. git checkout development && python repro.py
  3. git fetch https://github.com/Anandb71/FireForm fix-radio-button-groups && git checkout FETCH_HEAD && python repro.py
import os, sys, urllib.request
sys.path.insert(0, os.getcwd())
from pypdf import PdfReader
from app.services.form_filler.filler import write_pdf

URL = "https://www.osha.gov/sites/default/files/OSHA-RK-Forms-Package.pdf"
req = urllib.request.Request(URL, headers={"User-Agent": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 Chrome/120 Safari/537.36"})
open("osha.pdf", "wb").write(urllib.request.urlopen(req).read())

answer = {"301 ER": "Yes", "301 Gender": "Female"}   # what the model returns
write_pdf("osha.pdf", answer, "filled.pdf")

for a in PdfReader("filled.pdf").pages[9]["/Annots"]:
    a = a.get_object(); p = a.get("/Parent")
    if p is not None and p.get_object().get("/T") in answer:
        on = [s for s in a["/AP"]["/N"] if s != "/Off"][0]
        print(p.get_object()["/T"], "V =", p.get_object().get("/V"), "|", on, "button AS =", a.get("/AS"))

Observed on development (cbd52e2):

301 Gender V = /Off | /Male button AS = /Off
301 ER V = /No | /Yes button AS = /Off
301 Gender V = /Off | /Female button AS = /Off
301 ER V = /No | /No button AS = /No

Expected (and what the PR branch gives):

301 Gender V = /Female | /Male button AS = /Off
301 ER V = /Yes | /Yes button AS = /Yes
301 Gender V = /Female | /Female button AS = /Female
301 ER V = /Yes | /No button AS = /Off

Rendered with PDFium (left: development, right: PR branch), question 8 shows No checked on development for a "Yes" answer.
er-field-main
er-field-pr-branch

Why it happens: both buttons share the name 301 ER, and write_pdf handles each one like a checkbox. For "Yes" it sets the field to that button's own on-state: first /Yes, then /No for the second button. The last one wins, so the PDF says No. Any answer other than "Yes" (like "Female") sets the group to /Off, so Gender ends up empty. The PR sets every button of the group to the chosen option instead (/Yes, /Female), and pypdf then turns on the matching button.

I should also correct how I put it earlier: the problem isn't that the export values don't match. Each button gets its own on-state, so the last button in the group wins.

Happy to reopen #736, or to cut it down to just the write_pdf change if you'd rather keep the schema/enum part separate. Whatever works best for you.

@marcvergees marcvergees reopened this Oct 26, 2026
@marcvergees

Copy link
Copy Markdown
Member

Hi @Anandb71, since you seem new in this community, I'm happy to welcome you and to invite you to hop on our discord channel so that you can follow our weekly meetings and discussions.

I've just reopen your request and merged it.

Thanks for your contribution,
Marc

@marcvergees
marcvergees merged commit 004ee30 into fireform-core:development Oct 26, 2026
2 checks passed
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.

3 participants