[gezerui888]

I changed the court-form verifier to reject missing required checkbox data and missing fields that must remain blank. Text-only PDFs and forms with no selected checkboxes now fail. I also corrected the blank-field list to match the supplied SC-100 form. AcroForm read and field dereference errors now reach the test runner.

I added direct regression tests against the shipped PDF, including its field schema and a filled-form roundtrip. A lint guard rejects skip and xfail paths in the two patched binary verifiers. The README records the remaining writable-input check and the change in score meaning.

The dev dependency and sample Dockerfile both pin pypdf 6.21.4. CI's dependency audit reported 103 advisories with the old 5.1.0 pin and none after this update.

Tests:

• make verify: 6,687 passed, four existing skips, 93.26% coverage
• 752 custom lint tests passed
• 97 court-form regression cases passed, including real PDF extraction and missing, wrong, and correct field values
• The unchanged test.sh returns rewards [0, 1, 0, 0, 0] for blank, correct, wrong-Q10, unchecked, and text-only PDFs
• The correct PDF passes all 47 cases in a standalone Python 3.13 verifier image with the sample dependency pins
• Both extraction-error tests fail before the follow-up change
• CI's frozen dependency install, uv lock --check, and pip-audit passed

Fixes #234

cc @uipreliga

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	⚠️ Failed 2026-10-30T19:24:46.476728Z We couldn’t complete this review. Comment @codex review to try again.	6c6c6f2	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[uipreliga]

Review: coder_eval — pr:243 (3 files) axis:1,2,3,4,5,6,7,8

Scope: pr:243 (3 files) axis:1,2,3,4,5,6,7,8 · branch fix/court-form-missing-fields (PR #243 head; local main 021534ce) · 6c6c6f2 · 2026-10-30T21:25Z · workflow variant

Change class: complex — changes grading semantics of a vendored verifier (skip→fail on missing checkbox data, missing blank-field now fails, swaps a field id), so correctness depends on the real PDF's field set

PR #243 correctly closes the court-form-filling skip loophole: no-op or blank-key PDFs now fail and do not score 1.0, all 20 EMPTY_FIELDS names exist in the shipped sc100-blank.pdf, and the change adds no security or architecture problems. The real risk is that the new hard-fail gate has no anchor to the real form, because every regression test replaces the extraction fixtures and builds its inputs from the verifier's own lists, so a field-name typo or an extraction regression would silently set reward 0 for every agent. The bottom line is merge-worthy, but add two anchoring tests before or soon after the merge.

Summary

Axis	Score	🔴	🟠	🟡	🔵	Top Issue
1. Code Quality & Style	9.1 / 10	0	0	1	4	Regression test uses a string-embedded generated module run through pytest subprocesses, unlike the sibling in-process verifier test
2. Type Safety	10 / 10	0	0	0	0	—
3. Test Health	9 / 10	0	0	2	0	No test checks the changed EMPTY_FIELDS names / new field-must-exist gate against the shipped sc100-blank.pdf
4. Security	10 / 10	0	0	0	0	—
5. Architecture & Design	10 / 10	0	0	0	0	—
6. Error Handling & Resilience	9.9 / 10	0	0	0	1	Bare except: in pdf_field_values catches BaseException on the path that now decides the must-be-blank hard-fail
7. API Surface & Maintainability	9.9 / 10	0	0	0	1	The patched court-form verifier has no in-file modification notice, but its patched sibling has one
8. Evaluation Harness Quality	9.9 / 10	0	0	0	1	Scores for this task change meaning across the patch, with no version marker and no re-measured typical score

Overall Score: 9.7 / 10 · Weakest Axis: Test Health at 9 / 10
Totals: 🔴 0 · 🟠 0 · 🟡 3 · 🔵 7 across 8 axes.

Blockers

None.

Non-blocking, but please consider before merge

1. [Axis 1] Regression test uses a string-embedded generated module run through pytest subprocesses, unlike the sibling in-process verifier test (tests/test_skillsbench_court_form_verifier.py:90) — The test passes fixture bodies as Python source strings (line 33: '{name: "" for name, _ in EMPTY_FIELDS[1:]}'). It appends them to a copy of the verifier (line 90: _VERIFIER.read_text() + f"\n\n@pytest.fixture\ndef {fixture_name}():\n return {values}\n"). The override depends on a later def shadowing the earlier fixture of the same name. Then each case starts a full python -m pytest subprocess with a throwaway pytest.ini and PYTEST_DISABLE_PLUGIN_AUTOLOAD, and the test parses the summary text. That is more machinery than the goal ("a missing box or field fails, it does not skip") needs. Ruff and pyright cannot see the embedded code, so a typo in a values string shows only as an exit-code or summary mismatch. The sibling tests/test_skillsbench_3d_scan_calc_verifier.py already sets the pattern: it loads the verifier with importlib.util.spec_from_file_location and calls test methods directly. Use that pattern here. Load the module once. Parametrize over module.EMPTY_FIELDS / REQUIRED_CHECKBOXES / UNCHECKED_CHECKBOXES, then call for example module.TestEmptyFields().test_field_empty({}, name, desc) inside pytest.raises(AssertionError) (or pytest.raises(pytest.fail.Exception) for TestCheckboxes). An in-process call still proves that no Skipped is raised. The values then become real dicts, and the subprocess, the temporary pytest.ini and the source concatenation go away. Minor point on the same line: read_text() has no encoding=, but the write_text beside it uses encoding="utf-8".
2. [Axis 3] No test checks the changed EMPTY_FIELDS names / new field-must-exist gate against the shipped sc100-blank.pdf (tests/test_skillsbench_court_form_verifier.py:47) — Every fixture dict is built from the verifier's own lists, for example line 47 '{name: "" for name, _ in EMPTY_FIELDS}', and line 54 "{pattern: expected for pattern, _, expected, _ in REQUIRED_CHECKBOXES}". Because of this the 'blank-fields' and 'correct-boxes' cases pass for any field name. If someone puts back the nonexistent SC-100[0].Page1[0].CaptionRight[0].County[0].County[0] (the field this PR removes), or adds any other typo'd name, all 9 tests still pass. In production, after this PR, that field fails TestEmptyFields for EVERY agent and sets reward.txt to 0. So the score-changing part of the change (one missing name = automatic fail) has no anchor to the real form. The sibling test does have this anchor: test_expected_values_match_the_shipped_stl in tests/test_skillsbench_3d_scan_calc_verifier.py ties the verifier constants to environment/scan_data.stl. I ran pypdf 5.1.0 on court-form-filling/environment/sc100-blank.pdf. It found 155 fields. All 20 EMPTY_FIELDS names are present, County[0].County[0] is absent, and every REQUIRED_CHECKBOXES / UNCHECKED_CHECKBOXES pattern matches at least one /Btn field. This means the PR is correct today, but nothing keeps it correct. Add a test that opens environment/sc100-blank.pdf with PdfReader(...).get_fields() and asserts {n for n, _ in EMPTY_FIELDS} <= fields.keys() and that each checkbox pattern matches a field. Add pypdf as a dev dependency so the test runs in CI. pypdf is not in pyproject.toml now, and a pytest.importorskip guard would never run in CI.
3. [Axis 3] Now that missing extraction data fails the test instead of skipping it, no test runs the real extraction on a correctly filled PDF (tests/test_skillsbench_court_form_verifier.py:90) — Line 90 replaces the fixture by appending source text: _VERIFIER.read_text() + f"\n\n@pytest.fixture\ndef {fixture_name}():\n return {values}\n",. Because of this, extract_checkbox_values, parse_xfa_checkboxes and the real pdf_field_values fixture never run. Before this PR, an extraction failure gave pytest.skip. Now it gives pytest.fail, and a fail sets reward 0. So a silent extraction failure on a correctly filled form (an XFA-only fill, a pypdf version change, or flattened output) now scores 0, and no test shows that a correct form still extracts. The sample PDF has an /XFA stream, which is the extraction path the removed skip was for. I checked this locally: I filled sc100-blank.pdf with PdfWriter.update_page_form_field_values (pypdf 5.1.0, the version in the Dockerfile) using the six expected checkbox states. extract_checkbox_values then returned all six values, and all 20 EMPTY_FIELDS survived the round-trip. This took milliseconds and did not need pdftotext. Add one test that fills the shipped blank form in tmp_path, calls extract_checkbox_values and the field-value logic directly, and asserts that TestCheckboxes, TestUncheckedCheckboxes and TestEmptyFields pass. This shows that the new fail-on-absent gate does not reject a correct answer.

Nits

1. [Axis 1] Expected pytest summary counts hardcode the verifier's list lengths (tests/test_skillsbench_court_form_verifier.py:29) — The expected summaries "6 failed" / "6 passed" (lines 27-28), ("TestEmptyFields", "pdf_field_values", "{}", 1, "20 failed") (line 29) and "1 failed, 19 passed" (line 35) repeat len(REQUIRED_CHECKBOXES)=6, len(UNCHECKED_CHECKBOXES)=6 and len(EMPTY_FIELDS)=20 as magic numbers. If one verifier entry is added or removed, all of these strings break, and the error does not point to the cause. Derive the counts from the loaded verifier module (for example f"{len(EMPTY_FIELDS)} failed"), or assert per-case outcomes so that no count is needed (see the in-process recommendation above).
2. [Axis 1] Parametrize ids are a separate parallel list, not attached to each case (tests/test_skillsbench_court_form_verifier.py:73) — ids=[ "missing-required-boxes", "unchecked-boxes", ... "unexpected-checked-boxes", ] (lines 73-83) must stay aligned by position with the nine 5-tuples above it (lines 27-71). Several of those tuples are near-identical (for example, three "TestEmptyFields", "pdf_field_values" cases), so if someone reorders or inserts a case, the wrong label goes on a case and nothing reports it. Use pytest.param(..., id="one-missing-field") on each case.
3. [Axis 1] README no longer says that the court-form-filling verifier still reads an agent-writable input (tasks/samples/skillsbench/README.md:19) — Before this PR, the README said: "The other verifiers are unchanged and can still read agent-writable inputs". The PR changes it to The \dialogue-parser` verifier is unchanged and takes part of its ground truth/from the agent-writable `/app/script.txt`.(lines 19-20), so now only dialogue-parser is named. But the patched court-form-filling verifier still readsINPUT_FILE = Path("/root/sc100-blank.pdf")` (test_outputs.py line 20) for its input-vs-output identity check (lines 375-379), and the agent can write that file. The impact is small, but the README now discloses less than it did. Add one clause that names the blank-form read, so the list of agent-writable inputs stays complete. Cross-axis: 4, 8.
4. [Axis 1] Edited README lines are not re-wrapped to the paragraph's width (tasks/samples/skillsbench/README.md:63) — The surrounding prose wraps at about 80 columns, but two edited lines are longer: line 63 (97 chars) \3d-scan-calc` and `court-form-filling`, the local patches described above). Budget roughly 1–3and line 22 (90 chars)The `3d-scan-calc` patch closes only the `scan_data.stl` vector. Grading still runs in the`. Re-wrap both paragraphs. This is cosmetic only.
5. [Axis 6] Bare except: in pdf_field_values catches BaseException on the path that now decides the must-be-blank hard-fail (tasks/samples/skillsbench/court-form-filling/verifier/test_outputs.py:352) — Lines 350-353: try: / value = str(value.get_object()) / except: / value = str(value). This is pre-existing and the PR does not change it, but TestEmptyFields now depends on pdf_field_values for presence and blankness (new line 476 assert field_name in pdf_field_values). The bare except: also catches KeyboardInterrupt/SystemExit. On a real get_object() failure it falls back to str(IndirectObject(...)), which is always 3 or more characters. A must-be-blank field then fails with the confusing message "should be empty, got: 'IndirectObject(...)'" and the original error is lost. Narrow the handler to except Exception: (CodeQL py/catch-base-exception). Better, drop the fallback and let a dereference failure propagate so the verifier shows the real cause. Impact is low because the fallback can only make the verdict fail and cannot make it pass.
6. [Axis 7] The patched court-form verifier has no in-file modification notice, but its patched sibling has one (tasks/samples/skillsbench/court-form-filling/verifier/test_outputs.py:1) — The PR changes the vendored upstream file in three ways: it removes the two pytest.skip(...) branches, it changes value = pdf_field_values.get(field_name, "") to assert field_name in pdf_field_values, ..., and it replaces the page1_county entry with page1_court_addr3. The file still opens with the unchanged upstream docstring (line 1: """, line 2: Tests for court-form-filling task (SC-100 Small Claims Court form).). Nothing in the file says it was changed. The sibling patched verifier tasks/samples/skillsbench/3d-scan-calc/verifier/test_outputs.py line 1 has # Modified from benchflow-ai/skillsbench@95b3e60: grade against fixed EXPECTED_* (issue #233). Apache-2.0 §4(b), the license the README names, says modified files must carry a prominent notice of the change. The README note does not do this inside the file. Without the notice, someone who re-syncs from upstream can overwrite the #234 fix and not see it. Add a matching header line, for example # Modified from benchflow-ai/skillsbench@95b3e60: fail on missing checkbox data / blank-field keys; fix EMPTY_FIELDS names (issue #234).
7. [Axis 8] Scores for this task change meaning across the patch, with no version marker and no re-measured typical score (tasks/samples/skillsbench/README.md:94) — The patch changes the score for the same agent output, on purpose. A PDF with no extractable checkbox data, or with a must-be-blank field missing, used to score 1.0 because pytest skips still exit 0 and test.sh writes reward 1. It now scores 0.0, and TrialDateCourtAdd3 is checked for the first time. The task_id skillsbench-court-form-filling and the YAML did not change, so runs graded before and after the patch look the same in task.json and in reports. The README table still shows | \court-form-filling` | 1.00 | (line 94), and that number was measured with the old, more lenient verifier. Re-measure the typical score with the patched verifier, or add a note that scores from before issue #234 are not comparable. Nightly and cross-repo impact: none. The skillsbench samples are not in any experiment or nightly config. The verifier is copied from $TASK_DIR at grade time (cp -r "$TASK_DIR/verifier" /verifier`), not baked into the Dockerfile, so no image rebuild is needed, and the task.json, run.json and report schemas do not change.

What's Missing

Parallel paths:

• 🔵 The PR removes the if not checkbox_data: pytest.skip(...) guard, which was the only signal of an extraction failure. The parallel extraction path was not updated: extract_checkbox_values (test_outputs.py lines 65-85) still wraps the pypdf read in except Exception: pass. sc100-blank.pdf is AES-encrypted, so a pypdf error now turns silently into an empty dict. Examples are a DependencyError when cryptography is absent, or a parse error on an unusual output. Before the PR that error gave a skip. Now it gives six TestCheckboxes failures that say "not found. Available checkboxes: []", and the real cause is lost. Make the handler re-raise, or call pytest.fail with the exception text, so that an extraction error and a truly unchecked form are reported differently. (trigger: tasks/samples/skillsbench/court-form-filling/verifier/test_outputs.py) (restates: Axis 6: Bare except: in pdf_field_values catches BaseException on the path that now decides the must-be-blank hard-fail)
• 🔵 The sibling verifier patch for #233 (3d-scan-calc/verifier/test_outputs.py line 1) carries a # Modified from benchflow-ai/skillsbench@95b3e60: ... header. The court-form patch did not copy this convention, so the file still looks like an unmodified upstream copy. Separately, the README now documents two local verifier patches, but the pyproject.toml comment at line 244 still says that the vendored skillsbench tasks are "kept verbatim". Make the in-file notice and the pyproject comment agree with the README. (trigger: tasks/samples/skillsbench/court-form-filling/verifier/test_outputs.py) (restates: Axis 7: The patched court-form verifier has no in-file modification notice, but its patched sibling has one)

Tests:

• 🟡 The PR body says that verifier/test.sh was run by hand and rejected the unchanged form, a text-only PDF, missing checkboxes and a wrong Q10 answer. None of these checks is in the committed tests. The new test replaces the checkbox_data / pdf_field_values fixtures with literal dicts, so the #234 headline case (shipped sc100-blank.pdf copied unchanged to the output path, which scored 1.0 before the PR) is not locked in. Add a test that points the verifier's OUTPUT_FILE at a copy of environment/sc100-blank.pdf, runs the real extract_checkbox_values and pdf_field_values logic, and asserts that TestCheckboxes fails and does not skip. This test needs pypdf and cryptography in the dev dependencies. (trigger: tests/test_skillsbench_court_form_verifier.py) (restates: Axis 3: Now that missing extraction data fails the test instead of skipping it, no test runs the real extraction on a correctly filled PDF)
• 🟡 Nothing ties the changed EMPTY_FIELDS names (County[0].County[0] removed, TrialDateCourtAdd3[0] added) or the checkbox patterns to the shipped PDF. Now a field that is absent fails every agent, so this anchor carries the score. The sibling test test_expected_values_match_the_shipped_stl gives the 3d-scan-calc patch the same kind of anchor. (trigger: tests/test_skillsbench_court_form_verifier.py) (restates: Axis 3: No test checks the changed EMPTY_FIELDS names / new field-must-exist gate against the shipped sc100-blank.pdf)
• 🔵 The defect class here is "a pytest skip counts as a pass": test.sh writes reward 1 whenever pytest exits 0, and skips are included. But the regression test checks "skipped" not in result.stdout only for the three classes it targets. Nothing stops a future pytest.skip from coming back in TestPDFValid, TestRequiredContent or a re-synced upstream copy. Add a small AST or grep test that asserts the patched court-form (and 3d-scan-calc) verifiers contain no reachable pytest.skip call, so the whole file is guarded and not only the patched methods. (trigger: tests/test_skillsbench_court_form_verifier.py)

Downstream consumers:

• 🔵 The scoring formula changed. A PDF with no extractable checkboxes, or with a must-be-blank field absent, now scores 0.0 instead of 1.0, and TrialDateCourtAdd3 is checked for the first time. But README line 94 still shows court-form-filling with a typical score of 1.00, which was measured with the old, more lenient verifier. Nothing marks pre-#234 scores for skillsbench-court-form-filling as not comparable with post-#234 scores. (trigger: tasks/samples/skillsbench/README.md) (restates: Axis 8: Scores for this task change meaning across the patch, with no version marker and no re-measured typical score)
• 🔵 The README's residual-tamper paragraph (line 22, "The 3d-scan-calc patch closes only the scan_data.stl vector...") was rescoped to 3d-scan-calc only. No matching statement was added for the court-form patch. That verifier still reads the agent-writable /root/sc100-blank.pdf for its identity check. It also still treats an absent checkbox as unchecked and any value shorter than 3 characters as blank. The README now discloses less than it did before for this verifier. (trigger: tasks/samples/skillsbench/README.md) (restates: Axis 1: README no longer says that the court-form-filling verifier still reads an agent-writable input)

Harness & Lint Improvements

Static checks (lint / type):

• [ce-lint] CE070, patched-vendored-verifier provenance. This is a whole-tree @pytest.mark.lint class in tests/test_custom_lint.py, not a per-AST rule. CE069 is in flight in a worktree, so 070 is the next free number. Check this with the id-uniqueness assert in tests/lint/runner.py. Invariant: every tasks/samples/skillsbench/<slug>/verifier/test_outputs.py that has a regression test tests/test_skillsbench_<slug_underscored>_verifier.py must start with a # Modified from benchflow-ai/skillsbench@<sha>: ... (issue #N). header. The reverse must also hold: a file with that header must have a regression test. A regression test is the mechanical proof that a file was patched locally. On main today the check holds for 3d-scan-calc. On pr-243 it fails for court-form-filling, because the PR adds the regression test but the header is missing. Prevents: A7 'patched court-form verifier has no in-file modification notice' (test_outputs.py:1). It also covers the re-sync risk in that finding: an upstream re-sync that overwrites the #234 fix now fails make verify, because the header and the regression test go out of step.
• [ce-lint] CE071, no silent pass in a locally patched binary-reward verifier. The scope is the files that carry the CE070 # Modified from header. Unpatched upstream files stay verbatim and are not checked. The check fails on (a) any pytest.skip(...) / pytest.importorskip(...) call, because verifier/test.sh writes reward 1 when pytest exits 0, and a skip exits 0, so a skip counts as a pass. It also fails on (b) a bare except: and an except BaseException:. For (b), run ruff check --isolated --select E722,BLE001 on those files from inside the lint test. The repo-wide ruff config cannot do this, because extend-exclude = ["tasks/samples"] keeps all vendored files out, and they must stay unformatted. Running ruff that way over tasks/samples/skillsbench today gives 4 hits. One of them is court-form-filling/verifier/test_outputs.py:352. The same file is covered by (a), so the PR's skip removal is also locked in. Prevents: A6 'Bare except: in pdf_field_values catches BaseException' (test_outputs.py:352). It also stops a later edit or a partial re-sync from bringing back the pytest.skip branches this PR removed, which would make a missing-checkbox PDF pass again. That reversal is the gap that A3 (fail-on-absent untested) and A8 (score meaning changed) are about.
• [ce-lint] CE072, no nested pytest run in tests/. Add the rule to _ALSO_SCAN_TESTS, or scan tests/ only. The rule flags a subprocess.run / subprocess.Popen / create_subprocess_exec call whose argv list literal contains the adjacent constants "-m", "pytest". It also flags a <Path>.read_text(...) + <str> BinOp whose result goes to write_text, which writes out a changed copy of a module's source. The fix is the in-process pattern from tests/test_skillsbench_3d_scan_calc_verifier.py: load the module with importlib.util.spec_from_file_location and call the test methods directly, or use pytester. Use # noqa: CE072 for a test that really has to check pytest's CLI. main has no hits today, so the rule needs no baseline sweep. Rule file: tests/lint/rules/ce072_no_nested_pytest_subprocess.py, imported and listed in ALL_RULES in tests/lint/runner.py. Prevents: A1/A2/A5 'regression test uses a string-embedded generated module run through pytest subprocesses' (tests/test_skillsbench_court_form_verifier.py:88-112, :90). The rewrite this rule forces also removes A1/A3/A7 'expected summary counts hardcode list lengths' (lines 27-29, 35), because the pytest summary text is not parsed any more.
• [ce-lint] CE073, no positional ids= list in pytest.mark.parametrize. Scope: tests/. The rule flags ids= when its value is a List or Tuple literal of string constants. Computed ids such as ids=[r.id for r in ALL_RULES] are derived from the values, so the rule allows them. The fix is pytest.param(..., id="...") on each case. tests/ has 29 such literal lists in 14 files today. Following the CLAUDE.md cap-rule policy, ship the rule with a per-file baseline set below the current count, or migrate the 29 in the same change. Do not exempt new files: the new court-form test is the case the rule exists for. Prevents: A1 'Parametrize ids are a separate parallel list, not attached to each case' (tests/test_skillsbench_court_form_verifier.py:73-83). This is the 'positional list[i] <-> list[i] coupling' shape the review procedure names as statically detectable.
• [ce-lint] CE074, skillsbench README discloses every agent-writable verifier input. This is a whole-tree doc-surface class in tests/test_custom_lint.py. The test collects the absolute-path string constants under the agent-writable roots (/root/, /app/) that are passed to Path(...) in each tasks/samples/skillsbench/*/verifier/test_outputs.py. It then asserts that the README's disclosure paragraph names every such path, or the task slug together with the file basename. Output paths that the agent is supposed to write (for example /root/sc100-filled.pdf) go in a short per-task allowlist with a reason. Prevents: A1 'README no longer says that the court-form-filling verifier still reads an agent-writable input' (README.md:19). With this test, the PR's rewrite of the disclosure sentence would fail on /root/sc100-blank.pdf (INPUT_FILE, test_outputs.py:20).
• [ce-lint] Optional and low value: add CE008 (read_text without encoding=) to _ALSO_SCAN_TESTS in tests/test_custom_lint.py, so it scans tests/ too. Today CE008 scans src/ only, and that is how the new test's _VERIFIER.read_text() passed. Cost: tests/ has 89 CE008 hits now, so the widening needs one mechanical encoding="utf-8" sweep. Another way is ruff PLW1514 (unspecified-encoding), once it leaves preview. Prevents: The minor detail in A1 at tests/test_skillsbench_court_form_verifier.py:90: read_text() has no encoding, while the write_text(..., encoding="utf-8") next to it has one. If CE072 forces the in-process rewrite, this line goes away anyway.

Harness improvements (not statically reachable):

• Make 'the verifier constants exist in the shipped artefact' a required test for every patched skillsbench verifier. The 3d-scan-calc test test_expected_values_match_the_shipped_stl already does this. For court-form-filling: open environment/sc100-blank.pdf with PdfReader(...).get_fields(), then assert {n for n, _ in EMPTY_FIELDS} <= fields.keys() and that each REQUIRED_CHECKBOXES / UNCHECKED_CHECKBOXES pattern matches at least one /Btn field. Add pypdf[crypto] (pinned to the Dockerfile's 5.1.0) to the dev group. Make sure cryptography is in the same group the test env installs, because the blank form is AES-encrypted. Do not guard the test with importorskip, because then it never runs in CI. CE070 can check statically that the test file exists, but not what it asserts. Why not static: Field names come from parsing an encrypted PDF at runtime. No AST or grep check can tell whether a string like CaptionRight[0].County[0].County[0] is a real AcroForm field. Prevents: A3/A8 'No test checks the changed EMPTY_FIELDS names / field-must-exist gate against the shipped sc100-blank.pdf' (tests/test_skillsbench_court_form_verifier.py:47). If someone adds back the nonexistent County field, every agent now gets reward 0, and all 9 current tests still pass.
• Add a known-good answer control for each patched binary-reward verifier. Build a correct answer from the shipped input in tmp_path. For court-form-filling: fill sc100-blank.pdf with PdfWriter.update_page_form_field_values and the six expected checkbox states. Then run the REAL fixtures/extraction (extract_checkbox_values, pdf_field_values) and assert that TestCheckboxes, TestUncheckedCheckboxes and TestEmptyFields all pass. Pair this with the existing known-bad cases, so that each fail-on-absent gate has a test that trips it and a test that it accepts. The verifier reported this to take milliseconds, and it needs no pdftotext. Why not static: The question is whether the real extraction path returns data for a correctly filled form. That depends on runtime behaviour of the pypdf version and the XFA/AcroForm round-trip. The current tests replace those fixtures, so this behaviour is never checked. Prevents: A3 'Now that missing extraction data fails the test instead of skipping it, no test runs the real extraction on a correctly filled PDF' (tests/test_skillsbench_court_form_verifier.py:90). A silent extraction regression would score a correct answer 0.
• Record a digest of the grading inputs in task.json. That is the verifier dir for skillsbench-style tasks and the criteria block in general. Use the existing path_utils.digest_tree, which today is used only for the reference-solution shield. Report tooling can then mechanically mark runs graded by different verifier versions as not comparable, and the README's 'typical score' table can carry the digest it was measured with. Note: this changes the task.json schema. That schema is a cross-repo contract for the coder-eval-uipath pipeline, so add the field there and document it in docs/REPORT_SCHEMA.md. Why not static: Score comparability depends on which grader bytes ran for each run. Only a runtime capture can provide this. No static check can tell whether a stored score was produced before or after a verifier patch. Prevents: A8 'Scores for this task change meaning across the patch, with no version marker and no re-measured typical score' (tasks/samples/skillsbench/README.md:94).
• Hardcoded pytest-summary counts ("20 failed", "1 failed, 19 passed") have no static gate. A count inside an expected-output string looks like any other string literal. Do not add one. The fix is structural: the in-process rewrite that CE072 forces asserts each case's outcome directly, so no counts are left. The README re-wrap (A1, README.md:63/22) is cosmetic. A markdownlint MD013 gate on one vendored README would cost more than it gives, so do not add that gate either. Why not static: Telling a magic count from a real expected string needs semantic judgment. Line width in hand-written README prose is a style choice, not a defect class. Prevents: A1/A3/A7 'Expected pytest summary counts hardcode the verifier's list lengths' (tests/test_skillsbench_court_form_verifier.py:27-29, 35). A1 'Edited README lines are not re-wrapped' is a deliberate no-gate.

Top 5 Priority Actions

1. Add a round-trip test in tests/test_skillsbench_court_form_verifier.py (the gap is at line 90, which replaces the real checkbox_data/pdf_field_values fixtures). It fills environment/sc100-blank.pdf in tmp_path with pypdf 5.1.0 using the six expected checkbox states, runs the real extract_checkbox_values and field-value logic, and asserts that TestCheckboxes, TestUncheckedCheckboxes and TestEmptyFields pass. This proves that the new fail-on-absent gate does not give a correct answer reward 0.
2. Add a test that opens environment/sc100-blank.pdf with PdfReader(...).get_fields() and asserts {n for n, _ in EMPTY_FIELDS} <= fields.keys() and that each REQUIRED_CHECKBOXES/UNCHECKED_CHECKBOXES pattern matches a /Btn field (tests/test_skillsbench_court_form_verifier.py:47 builds fixtures from EMPTY_FIELDS itself, so a typo such as the removed County[0].County[0] would pass every test and fail every agent). Add pypdf[crypto] (or pypdf plus cryptography) to the dev group that CI installs, because the PDF is AES-encrypted.
3. Re-measure the court-form-filling typical score with the patched verifier, or add a note that scores from before issue #234 cannot be compared with later scores (tasks/samples/skillsbench/README.md:94 still shows 1.00 from the old lenient verifier). The same agent output that scored 1.0 before can now score 0.0 under the same task_id.
4. Rewrite the regression test to load the verifier in-process with importlib.util.spec_from_file_location and call the test methods directly inside pytest.raises, as tests/test_skillsbench_3d_scan_calc_verifier.py does. This replaces the source concatenation and the nine pytest subprocesses at tests/test_skillsbench_court_form_verifier.py:90. Expect pytest.fail.Exception for absent checkboxes and AssertionError for wrong values. Derive counts from the module (lines 27-35), and use pytest.param(..., id=...) in place of the parallel ids list (line 73).
5. Add a # Modified from benchflow-ai/skillsbench@95b3e60: ... (issue #234). header at tasks/samples/skillsbench/court-form-filling/verifier/test_outputs.py:1, matching the 3d-scan-calc sibling, so an upstream re-sync does not overwrite the fix. In the same file, narrow the bare except: at line 352 to except Exception: or remove the fallback. Also restore the README:19 clause that says this verifier still reads the agent-writable /root/sc100-blank.pdf.

---

Stats: 0 🔴 · 0 🟠 · 3 🟡 · 7 🔵 across 8 axes reviewed.

[gezerui888]

I replaced the generated pytest subprocesses with direct, parametrized verifier calls. The tests check the shipped PDF's field names and types and roundtrip the six correct checkbox values. They also reject blank and fieldless forms and preserve AcroForm read and field dereference errors. Both error tests failed before the change.

I added the modification notice and CE071 skip/xfail guard, restored the writable-input disclosure, and marked the historical score as non-comparable. CI's dependency audit reported 103 advisories in pypdf 5.1.0. I updated both the dev and Dockerfile pins to 6.21.4, preserved their parity, and checked that the audit passes.

make verify passes: 6,687 tests, the same four existing skips, 93.26% coverage, and 752 custom lint tests. The unchanged test.sh also returns [0, 1, 0, 0, 0] for the five PDF controls. The correct form passes all 47 cases in a standalone Python 3.13 verifier image with the updated dependency pins.

The broader repository-wide lint proposals and grading-digest schema change need separate changes.