Preserve OCR outputs on save failures and fix Welcome action inputs - #1
Conversation
Signed-off-by: Lukas Geiger <lukas@um-bruch.org>
Signed-off-by: Lukas Geiger <lukas@um-bruch.org>
Signed-off-by: Lukas Geiger <lukas@um-bruch.org>
lukisch
left a comment
There was a problem hiding this comment.
Zweitmodell-Review: CI grün (pytest 3.10 bis 3.12, Smoke ubuntu/macos), keine Blocker.
OCR-Ausgabe, Sammel-PDF und Job-Manifest werden in ein privates Staging-Verzeichnis geschrieben, per Wiedereinlesen auf Seitenzahl geprüft und erst dann ersetzt; unvollständige oder leere OCR-Seiten, Mehrfachseiten pro Bild und Merge-Ausgabe auf Quelldatei/Hardlink werden abgewiesen. Archivierung kopiert vollständig und löscht danach; Teilerfolge werden separat gemeldet. Sehr gründlich getestet (19 Tests in test_save_safety.py). Keine Credentials oder Nutzerpfade im Diff.
Hinweise:
welcome.yml: Wechsel auf Unterstrich-Eingaben (repo_token,issue_message,pr_message) zufirst-interaction@v3ist für v3 passend. Ob die Eingaben wirklich greifen, zeigt erst der nächste Lauf auf dem Basis-Branch (die PR-eigene Version läuft hier nicht). Der PR bündelt damit zwei fachfremde Änderungen (Workflow und OCR-Sicherheit); ein Split würde ein Revert vereinfachen.- Bei hartem Abbruch bleiben
pdftopdfocr-output-*-Verzeichnisse neben der Ausgabe liegen (bei OneDrive-Exportordnern im Sync). InSAVE_SAFETY.mdals Grenze benannt. validate_pdf_outputöffnet das Staging-PDF erneut, bei sehr großen Batches zusätzliche I/O; vertretbar.translations.jsonbekommt ein Newline am Dateiende, daher ein Diff-Rauschen von einer Zeile.
Generated by Claude Code
Konflikte in CHANGELOG, merge_ocr_outputs und _ocr_pdf aufgeloest: gestagte Ausgabe des PR beibehalten, BS-11-Namenssanitisierung (Path.name, .pdf) und valid_paths uebernommen; BS-11-Tempfile-Test an Staging-Design angepasst. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
Review (Zweitmodell, Sonnet 5.5): Diff gelesen. Staging in privatem Verzeichnis + os.replace, Seitenzahl-Verifikation vor Publikation, Cleanup ohne Maskierung des Primärfehlers: ok. Konflikte mit master (BS-11) aufgelöst: Staging des PR bleibt, Namenssanitisierung (Path.name, .pdf) und valid_paths aus BS-11 übernommen; BS-11-Tempfile-Test an das Staging-Design angepasst (kein System-Tempfile mehr). Lokal 173 passed, 1 skipped; CI grün. |
A two-page OCR job previously reported success after an empty OCR response silently omitted one page. OCR, merge, and manifest writes also wrote directly over existing targets, so a later save failure could destroy the previous output.
This change requires every rendered source page/frame to produce exactly one PDF page. It stages complete output beside the target, reopens PDF stages to verify their page count, closes lazy sources before publication, and replaces the target only after successful writing and validation. Failed disk fallbacks stay in the private stage directory.
Merge inputs and their filesystem aliases cannot be replaced by the merge target. Individual results are archived by completing a staged copy before removing the original. The GUI distinguishes a saved collective PDF from later archiving errors; API callers can collect warnings or inspect MergeArchiveError.merged_path. Manifest failures show an error without success feedback. Cleanup failures are reported separately from processing/publication status.
Validation: 163 passed, 1 skipped (the intentionally unversioned AUFGABEN.txt check); Ruff, compileall, and git diff --check passed. Seven new behavior counterexamples failed on unchanged master; the final suite adds 19 regression cases. An independent isolated review passed 30 cases on the first commit and 8 focused cases on the final delta, including real Windows sharing/readonly failures, TIFF page order, pikepdf lazy-source lifetime, Unicode/serialization failures, and malformed or wrong-page PDF stages. Review is bound to 74bde99.
Version remains 1.1.4. No executable, release, Store submission, or general OCR-accuracy claim. External path races and batch/archival multi-file transactions remain outside this change; SAVE_SAFETY.md records the limits.
The final follow-up commit 7d21427 fixes three Welcome action input names to match the official first-interaction v3 metadata. Its independent delta review passed; product sources are unchanged and the full local suite still passed 163 tests with one expected skip. Five product checks and assignment passed on predecessor 74bde99. The original Welcome run failed on the base branch's hyphenated input names: pull_request_target reads the base workflow, so pushing this fix does not establish a successful Welcome run. No extra comment or issue was posted to test it.
Draft pending current-head CI and the repository's required review/merge by a different model class. The independent technical reviews above do not assert that separate merge requirement is satisfied.