diff --git a/src-ui/src/app/data/ocr-template.ts b/src-ui/src/app/data/ocr-template.ts index 933bbc072..e3e0a1270 100644 --- a/src-ui/src/app/data/ocr-template.ts +++ b/src-ui/src/app/data/ocr-template.ts @@ -104,7 +104,8 @@ export interface OcrZoneRunResult { template: string zone: string custom_field: string - value: string | number | null + value: string | number | boolean | null + reason?: string | null } export interface OcrZoneRunResponse { diff --git a/src/documents/tests/test_api_ocr_templates.py b/src/documents/tests/test_api_ocr_templates.py index e54da0f50..748f3c669 100644 --- a/src/documents/tests/test_api_ocr_templates.py +++ b/src/documents/tests/test_api_ocr_templates.py @@ -8,6 +8,7 @@ from unittest.mock import patch from django.contrib.auth.models import Permission from django.contrib.auth.models import User from django.db import IntegrityError +from PIL import Image from rest_framework import status from rest_framework.test import APITestCase @@ -947,14 +948,16 @@ class TestOcrTemplatesAPI(DirectoriesMixin, APITestCase): ) extract.assert_not_called() - @patch("documents.zone_ocr.run_zone_extraction") + @patch("documents.zone_ocr.subprocess.run") def test_run_zone_ocr_reports_only_templates_visible_to_the_document_owner( self, - extract, + run, ): owner = User.objects.create_user(username="document_owner") document = DocumentFactory(content=CONTENT, owner=owner) - document.source_path.write_bytes(b"test PDF") + with Image.new("RGB", (100, 100), "white") as image: + image.save(document.source_path, format="PNG") + run.return_value.stdout = "extracted" for name, template_owner in [ ("Owned", owner), ("Unowned", None), @@ -987,7 +990,7 @@ class TestOcrTemplatesAPI(DirectoriesMixin, APITestCase): [result["template"] for result in response.json()["results"]], ["Owned", "Unowned", "Shared"], ) - extract.assert_called_once() + self.assertEqual(run.call_count, 3) def test_shared_editor_cannot_transfer_ownership_or_change_sharing(self): editor = User.objects.create_user(username="shared_editor") @@ -1035,13 +1038,15 @@ class TestOcrTemplatesAPI(DirectoriesMixin, APITestCase): self.assertEqual(response.json(), {"error": "Document file not found"}) extract.assert_not_called() - @patch("documents.zone_ocr.run_zone_extraction") - def test_run_zone_ocr_reports_completed_results(self, extract): + @patch("documents.zone_ocr.subprocess.run") + def test_run_zone_ocr_reports_completed_results(self, run): document = DocumentFactory( content=CONTENT, mime_type="application/pdf", ) - document.source_path.write_bytes(b"test PDF") + with Image.new("RGB", (100, 100), "white") as image: + image.save(document.source_path, format="PNG") + run.return_value.stdout = "Extracted title" template = OcrTemplate.objects.create( name="Template", match="Northwind", @@ -1070,17 +1075,61 @@ class TestOcrTemplatesAPI(DirectoriesMixin, APITestCase): "template": template.name, "zone": "Title zone", "custom_field": "Title", - "value": document.title, + "value": "Extracted title", + "reason": None, }, ], }, ) - extract.assert_called_once() + run.assert_called_once() - @patch("documents.zone_ocr.run_zone_extraction") - def test_run_zone_ocr_reports_all_target_types_and_missing_values(self, extract): + @patch("documents.zone_ocr.subprocess.run") + def test_run_zone_ocr_does_not_report_an_old_title_when_extraction_is_empty( + self, + run, + ): + document = DocumentFactory(title="Existing title", content=CONTENT) + with Image.new("RGB", (100, 100), "white") as image: + image.save(document.source_path, format="PNG") + template = OcrTemplate.objects.create( + name="Template", + match="Northwind", + source_width=100, + source_height=100, + ) + OcrTemplateZone.objects.create( + template=template, + name="Title", + target="title", + x=0, + y=0, + width=20, + height=20, + ) + run.return_value.stdout = "" + response = self.client.post(f"/api/documents/{document.pk}/run-zone-ocr/") + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json()["results"][0]["reason"], "no_value") + self.assertIsNone(response.json()["results"][0]["value"]) + document.refresh_from_db() + self.assertEqual(document.title, "Existing title") + + @patch("documents.zone_ocr.subprocess.run") + def test_run_zone_ocr_reports_all_target_types_and_missing_values(self, run): document = DocumentFactory(content=CONTENT, archive_serial_number=123) - document.source_path.write_bytes(b"test PDF") + with Image.new("RGB", (100, 100), "white") as image: + image.save(document.source_path, format="PNG") + run.side_effect = [ + subprocess.CompletedProcess([], 0, stdout=value) + for value in [ + "New invoice", + "", + "orphan", + "New title", + "456", + "2026-04-13", + ] + ] template = OcrTemplate.objects.create( name="Test", source_width=100, @@ -1121,12 +1170,12 @@ class TestOcrTemplatesAPI(DirectoriesMixin, APITestCase): self.assertEqual( values, { - "Present": "INV-123", + "Present": "New invoice", "Missing": None, "Unassigned": None, - "Title": document.title, - "ASN": 123, - "Created": document.created.isoformat(), + "Title": "New title", + "ASN": 456, + "Created": "2026-04-13", }, ) diff --git a/src/documents/tests/test_zone_ocr.py b/src/documents/tests/test_zone_ocr.py index ed16da926..7e0f7db9e 100644 --- a/src/documents/tests/test_zone_ocr.py +++ b/src/documents/tests/test_zone_ocr.py @@ -303,6 +303,70 @@ class TestSavedZoneExtraction(DirectoriesMixin, TestCase): self.document.refresh_from_db() self.assertEqual(self.document.title, "Second") + @patch("documents.zone_ocr.subprocess.run") + def test_skipped_results_never_report_existing_document_values(self, run): + zones = [ + self.zone("Empty"), + self.zone("Rejected", validation_regex="accepted"), + self.zone("Invalid", "asn"), + self.zone("Unassigned", "custom_field"), + ] + run.side_effect = [ + MagicMock(stdout=value) for value in ["", "rejected", "abc", "orphan"] + ] + results = _process_template(self.document, self.page, self.template, zones) + self.assertEqual( + [result.reason for result in results], + [ + "no_value", + "regex_mismatch", + "invalid_value", + "unassigned", + ], + ) + self.assertEqual([result.value for result in results], [None] * 4) + self.assertEqual(self.document.title, "Original") + + @patch("documents.zone_ocr.subprocess.run") + def test_failed_template_reports_rollback_and_next_template_still_runs(self, run): + DocumentFactory(archive_serial_number=42) + self.zone("Title") + self.zone("Conflict", "asn") + second = OcrTemplate.objects.create( + name="Second", + source_width=100, + source_height=50, + ) + first = self.template + self.template = second + self.zone("Second") + run.side_effect = [ + MagicMock(stdout=value) for value in ["Rolled back", "42", "Applied"] + ] + results = run_zone_extraction( + self.document, + self.page, + templates=[first, second], + ) + self.assertEqual( + [result.reason for result in results], + ["template_failed", "template_failed", None], + ) + self.assertEqual([result.value for result in results], [None, None, "Applied"]) + self.document.refresh_from_db() + self.assertEqual(self.document.title, "Applied") + self.assertIsNone(self.document.archive_serial_number) + + @patch("documents.zone_ocr.subprocess.run") + def test_literal_format_does_not_write_when_every_zone_failed(self, run): + zone = self.zone("Title") + self.template.combine_formats = {"title": "Invoice {Title}"} + run.return_value.stdout = "" + results = _process_template(self.document, self.page, self.template, [zone]) + self.assertEqual(results[0].reason, "no_value") + self.assertIsNone(results[0].value) + self.assertEqual(self.document.title, "Original") + @patch("documents.zone_ocr.subprocess.run") def test_saved_extraction_matches_preview_and_rejects_invalid_value(self, run): zone = self.zone( @@ -326,15 +390,13 @@ class TestSavedZoneExtraction(DirectoriesMixin, TestCase): class TestCombineFieldValue(SimpleTestCase): def test_default_format_preserves_zone_order_and_duplicate_names(self): - zones = [ - (OcrTemplateZone(name="Same"), value) for value in ["one", None, "two"] - ] + zones = [("Same", value) for value in ["one", None, "two"]] self.assertEqual(_combine_field_value("", zones), "one two") def test_format_preserves_existing_missing_token_cleanup(self): zones = [ - (OcrTemplateZone(name="First"), "one"), - (OcrTemplateZone(name="Second"), None), + ("First", "one"), + ("Second", None), ] self.assertEqual( _combine_field_value("{ First } / {Second} / {Unknown}", zones), diff --git a/src/documents/views.py b/src/documents/views.py index 828c725df..741194ade 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -9,6 +9,7 @@ import zipfile from collections import defaultdict from collections import deque from collections.abc import Callable +from dataclasses import asdict from datetime import datetime from datetime import timedelta from http import HTTPStatus @@ -2352,40 +2353,10 @@ class DocumentViewSet( from documents.zone_ocr import run_zone_extraction - run_zone_extraction(document, None, templates=templates) - - results = [] - builtin_labels = {"title": "Title", "asn": "ASN", "created": "Created"} - builtin_values = { - "title": document.title, - "asn": document.archive_serial_number, - "created": document.created.isoformat(), - } - field_values = { - instance.field_id: instance.value - for instance in document.custom_fields.select_related("field") - } - for template in templates: - for zone in template.zones.all(): - if ( - zone.target == OcrTemplateZone.TargetType.CUSTOM_FIELD - and zone.custom_field_id - ): - field_name = zone.custom_field.name - value = field_values.get(zone.custom_field_id) - else: - field_name = builtin_labels.get(zone.target, zone.target) - value = builtin_values.get(zone.target) - results.append( - { - "template": template.name, - "zone": zone.name, - "custom_field": field_name, - "value": value, - }, - ) - - return Response({"status": "completed", "results": results}) + results = run_zone_extraction(document, None, templates=templates) + return Response( + {"status": "completed", "results": [asdict(result) for result in results]}, + ) @extend_schema( operation_id="documents_delete_version", diff --git a/src/documents/zone_ocr.py b/src/documents/zone_ocr.py index 1fbfebdde..320518965 100644 --- a/src/documents/zone_ocr.py +++ b/src/documents/zone_ocr.py @@ -50,6 +50,29 @@ class FieldUpdate: custom_field: CustomField | None = None +@dataclass +class ZoneOcrResult: + template: str + zone: str + custom_field: str + value: object = None + reason: str | None = None + + +def _zone_result( + template: OcrTemplate, + zone: OcrTemplateZone, + *, + value: object = None, + reason: str | None = None, +) -> ZoneOcrResult: + if zone.target == OcrTemplateZone.TargetType.CUSTOM_FIELD: + label = zone.custom_field.name if zone.custom_field_id else "(no field)" + else: + label = {"title": "Title", "asn": "ASN", "created": "Created"}[zone.target] + return ZoneOcrResult(template.name, zone.name, label, value, reason) + + def get_templates_for_document(document: Document) -> list[OcrTemplate]: """ The enabled templates that match the document's content and that its owner @@ -72,7 +95,7 @@ def run_zone_extraction( document: Document, original_file: Path | None, templates: list[OcrTemplate] | None = None, -) -> None: +) -> list[ZoneOcrResult]: """ Run zone-based OCR extraction for the given templates, or for every template that matches the document when none are given. @@ -80,7 +103,7 @@ def run_zone_extraction( if templates is None: templates = get_templates_for_document(document) if not templates: - return + return [] # Resolve the document file: prefer archive (PDF/A), then source, then signal arg doc_path = _resolve_doc_path(document, original_file) @@ -89,8 +112,13 @@ def run_zone_extraction( "Zone OCR: no accessible file for document %d", document.pk, ) - return + return [ + _zone_result(template, zone, reason="missing_file") + for template in templates + for zone in template.zones.all() + ] + results = [] for template in templates: zones = list(template.zones.all()) if not zones: @@ -104,13 +132,17 @@ def run_zone_extraction( ) try: - _process_template(document, doc_path, template, zones) + results.extend(_process_template(document, doc_path, template, zones)) except Exception: logger.exception( "Zone OCR: error processing template '%s' for document %d", template.name, document.pk, ) + results.extend( + _zone_result(template, zone, reason="template_failed") for zone in zones + ) + return results def _resolve_doc_path( @@ -148,7 +180,7 @@ def _process_template( doc_path: Path, template: OcrTemplate, zones: list[OcrTemplateZone], -) -> None: +) -> list[ZoneOcrResult]: """Extract and combine zone values before applying a template's updates.""" pages_needed: set[int] = { _resolve_page_idx(zone.page, document.page_count) for zone in zones @@ -164,19 +196,22 @@ def _process_template( document.page_count, ) - grouped: dict[str, list[tuple[OcrTemplateZone, str | None]]] = {} + grouped: dict[str, list[tuple[OcrTemplateZone, ZoneOcrResult]]] = {} + results = [] for zone in zones: page_idx = _resolve_page_idx(zone.page, document.page_count) extracted = None + reason = "missing_page" if page_idx in page_images: - result = _extract_zone( + extraction = _extract_zone( page_images[page_idx], zone, zone.zone_source_width or template.source_width, zone.zone_source_height or template.source_height, tmp_path, ) - extracted = result.value + extracted = extraction.value + reason = None if extracted else "no_value" if ( extracted is not None @@ -190,22 +225,39 @@ def _process_template( zone.validation_regex, ) extracted = None + reason = "regex_mismatch" - grouped.setdefault(_field_key(zone), []).append((zone, extracted)) + result = _zone_result( + template, + zone, + value=extracted if reason is None else None, + reason=reason, + ) + results.append(result) + grouped.setdefault(_field_key(zone), []).append((zone, result)) updates = [] combine_formats = template.combine_formats or {} for key, field_zones in grouped.items(): + accepted = [result for _, result in field_zones if result.reason is None] + if not accepted: + continue value = _combine_field_value( combine_formats.get(key, ""), - field_zones, + [(zone.name, result.value) for zone, result in field_zones], ) - if not value: - continue - update = _prepare_field_update(field_zones[0][0], value) + target_zone = field_zones[0][0] + update = _prepare_field_update(target_zone, value) if value else None if update is not None: updates.append(update) + for result in accepted: + result.value = update.value if update else None + if update is None: + result.reason = ( + "unassigned" if key == "custom_field" else "invalid_value" + ) _apply_field_updates(document, updates) + return results def _field_key(zone: OcrTemplateZone) -> str: @@ -220,7 +272,7 @@ def _field_key(zone: OcrTemplateZone) -> str: def _combine_field_value( fmt: str, - field_zones: list[tuple[OcrTemplateZone, str | None]], + field_zones: list[tuple[str, str | None]], ) -> str: """Combine the OCR values of all zones targeting one field. @@ -228,7 +280,7 @@ def _combine_field_value( and literal text is kept; separators left dangling by an empty token are cleaned up. Without a format, the zone values are joined in order by a space. """ - values = {zone.name: value or "" for zone, value in field_zones} + values = {name: value or "" for name, value in field_zones} if not fmt: return " ".join(value for _, value in field_zones if value).strip()