-
-
Notifications
You must be signed in to change notification settings - Fork 505
.pr_agent_accepted_suggestions
| PR 283 (2026-05-05) |
[correctness] PreProcessor path truncated
PreProcessor path truncated
In validate_template_json(), the preProcessors special-case builds the displayed key using only error.path[2], which truncates deeper nested option paths (e.g., morphKernel) and can mislead users about the exact failing field.validate_template_json() special-cases errors under preProcessors to display the preProcessor name, but it currently uses only error.path[2] as the suffix. For nested option errors like preProcessors[0].options.morphKernel, this truncates the path to just ...options.
parse_validation_error() now correctly returns the full formatted path (e.g., preProcessors[0].options.morphKernel), as validated by the new test, but the preProcessors branch bypasses that full key.
- src/utils/validations.py[61-85]
- Start from the already-formatted
keyreturned byparse_validation_error(error). - Replace the
preProcessors[{index}]prefix withpreProcessors.{preProcessorName}(keep the remainder of the path intact). - Example logic:
display_key = key.replace(f"preProcessors[{idx}]", f"preProcessors.{name}", 1)-
table.add_row(display_key, msg)This preserves.options.morphKernelwhile still showing the preProcessor name.
| PR 282 (2026-05-04) |
[correctness] `_pN` suffix breaks matching
`_pN` suffix breaks matching
Rendered PDF page names use the pattern `*_p{page}.png` instead of an `OMR_ID + _{pageNumber}` suffix convention (e.g., `_1`, `_2`) needed for suffix-based page-template selection. This makes it impossible to reliably map pages to their intended per-page template definitions using the prescribed naming convention.PDF page render outputs are named with ..._p{n}.png, which does not follow the required OMR_ID + _{pageNumber} suffix convention for page selection/matching.
Multi-page association requires predictable suffixes (e.g., _1, _2) so the system can select the correct page template for each page.
- src/entry.py[210-215]
[correctness] Eval CSV page collision
Eval CSV page collision
For multi-page PDFs, _process_single_image passes the original PDF Path into evaluate_concatenated_response, so EvaluationConfig.conditionally_save_explanation_csv writes all pages into the same “_evaluation.csv”. This mixes page results and makes per-page evaluation output incorrect.Multi-page PDF processing passes the PDF file_path to evaluation, so the evaluation CSV filename is derived from the PDF stem and all pages append into the same file.
-
_process_single_image()already has a uniqueimg_nameper page (e.g.file_p2.png). -
EvaluationConfig.conditionally_save_explanation_csv()derives the CSV name fromfile_path.stem.
- Ensure the evaluation code receives a page-unique identifier (e.g., pass
Path(img_name)or a synthetic Path that encodes page number) sofile_path.stemis unique per rendered page. - file/path references:
- src/entry.py[306-312]
- src/evaluation.py[364-371]
[reliability] Invalid PDF page silent skip
Invalid PDF page silent skip
If pdf_params.pdf_page is out of range, load_omr_image breaks and returns an empty list, and process_files silently processes nothing for that PDF. This makes misconfiguration look like a successful run with missing outputs.Selecting an out-of-range PDF page results in no pages being rendered and no warning/error, so the PDF is effectively ignored.
-
pdf_pagecan be any non-negative integer; there is no max bound in config schema. -
load_omr_image()currentlybreaks onp >= len(doc).
- If
pdf_pageis set andpdf_page >= len(doc), log an error (or raise a user-facing exception) indicating the PDF page is out of range. - If iterating all pages, consider
continue+ warning instead ofbreak(defensive), depending on desired behavior. - file/path references:
- src/entry.py[197-217]
- src/entry.py[380-387]
[reliability] PDF render exceptions unhandled
PDF render exceptions unhandled
PDF open/render is not wrapped in a context manager/try-finally, so exceptions during get_pixmap/reshape can leave the document unclosed and abort processing of remaining files. This reduces batch robustness when encountering a malformed/encrypted PDF.Exceptions during PDF rendering can bypass doc.close() and stop processing of subsequent inputs.
-
fitz.open()/page.get_pixmap()/ array reshape can raise for malformed/encrypted PDFs. -
process_files()does not handle exceptions fromload_omr_image().
- Use a context manager:
with fitz.open(...) as doc:(or try/finally ensuringdoc.close()). - Catch rendering exceptions per-file (or per-page) inside
load_omr_image()or around its call inprocess_files(); log the error and continue with other files. - file/path references:
- src/entry.py[189-217]
- src/entry.py[380-398]
| PR 269 (2026-03-07) |
[correctness] Sample forces GUI flow
Sample forces GUI flow
The sample sets show_image_level=5, which triggers OpenCV window creation and a blocking wait-for-'q' loop during processing. This can hang/fail in headless environments and is a poor default for a reference sample.The new community sample sets outputs.show_image_level to 5, which causes interactive GUI windows and blocking waits during processing. This makes the sample unsuitable for headless environments and inconvenient as a default.
CropPage (used by this template) calls InteractionUtils.show(...) when show_image_level >= 5, and InteractionUtils.show uses OpenCV GUI primitives (namedWindow/imshow) and blocks waiting for q.
- samples/community/DomBrzezinski/config.json[1-11]
| PR 268 (2026-03-05) |
[correctness] Sample template overflows
Sample template overflows
The new community RTL sample template will raise an exception during template validation because the custom 7-choice horizontal block extends past the configured page width. This makes the sample unusable as-is for anyone trying to run it.The new sample samples/community/EricSoOSU/template.json currently fails template validation because MCQBlock_RTL_Custom overflows the page width.
Template.validate_parsed_labels raises if block_end_x >= page_width. For a horizontal block, width is bubblesGap*(len(bubbleValues)-1)+bubbleDimensions[0].
- samples/community/EricSoOSU/template.json[1-22]
Pick one:
- Reduce
origin[0](e.g., from 50 to 20), or - Reduce
bubblesGap(e.g., from 40 to 35), or - Increase
pageDimensions[0](e.g., from 300 to 320+) Ensureorigin[0] + (bubblesGap*(len(bubbleValues)-1)+bubbleDimensions[0]) < pageDimensions[0].