diff --git a/src/odoo_ocr/pipeline/extractor.py b/src/odoo_ocr/pipeline/extractor.py index 6acc81a..f9e055f 100644 --- a/src/odoo_ocr/pipeline/extractor.py +++ b/src/odoo_ocr/pipeline/extractor.py @@ -3,31 +3,29 @@ from __future__ import annotations import logging -from pathlib import Path from PIL import Image from odoo_ocr.clients import BaseVLMClient +from odoo_ocr.clients.base import load_prompt from odoo_ocr.config import Settings from odoo_ocr.schemas import ExtractedInvoice logger = logging.getLogger(__name__) -def _load_prompt(name: str) -> str: - from odoo_ocr.clients.base import BaseVLMClient - - return BaseVLMClient._load_prompt_file(Path("prompts") / name) - - async def extract_invoice( raw_text: str, pages: list[Image.Image], client: BaseVLMClient, settings: Settings, ) -> ExtractedInvoice: - """Convert raw OCR text into a validated ExtractedInvoice.""" - system_prompt = _load_prompt("extraction_system.txt") + """Convert raw OCR text into a validated ExtractedInvoice. + + Only the first page image is sent; the full multi-page OCR text is + included in the user prompt, so later pages are still covered. + """ + system_prompt = load_prompt("extraction_system.txt") user_prompt = ( f"OCR text extracted from the invoice:\n```\n{raw_text}\n```\n\n" "Now extract the structured invoice fields from the image and OCR text. " diff --git a/src/odoo_ocr/pipeline/reviewer.py b/src/odoo_ocr/pipeline/reviewer.py index 2e846b8..39bd7e9 100644 --- a/src/odoo_ocr/pipeline/reviewer.py +++ b/src/odoo_ocr/pipeline/reviewer.py @@ -3,31 +3,29 @@ from __future__ import annotations import logging -from pathlib import Path from PIL import Image from odoo_ocr.clients import BaseVLMClient +from odoo_ocr.clients.base import load_prompt from odoo_ocr.config import Settings from odoo_ocr.schemas import ExtractedInvoice, ReviewResult logger = logging.getLogger(__name__) -def _load_prompt(name: str) -> str: - from odoo_ocr.clients.base import BaseVLMClient - - return BaseVLMClient._load_prompt_file(Path("prompts") / name) - - async def review_invoice( invoice: ExtractedInvoice, pages: list[Image.Image], client: BaseVLMClient, settings: Settings, ) -> ReviewResult: - """Have a vision model review the extracted invoice against the image.""" - system_prompt = _load_prompt("review_system.txt") + """Have a vision model review the extracted invoice against the image. + + All pages of the document are sent, so multi-page invoices are reviewed + against their full original image, not just page one. + """ + system_prompt = load_prompt("review_system.txt") user_prompt = ( "Extracted invoice JSON:\n```json\n" f"{invoice.model_dump_json(indent=2)}" @@ -37,7 +35,7 @@ async def review_invoice( response = await client.complete( system_prompt=system_prompt, user_prompt=user_prompt, - images=[pages[0]] if pages else [], + images=pages, response_format=ReviewResult, temperature=0.1, model=settings.models.review, diff --git a/src/odoo_ocr/pipeline/xml_builder.py b/src/odoo_ocr/pipeline/xml_builder.py index 13b73cb..d1d1e69 100644 --- a/src/odoo_ocr/pipeline/xml_builder.py +++ b/src/odoo_ocr/pipeline/xml_builder.py @@ -48,8 +48,16 @@ def _external_id_invoice(invoice: ExtractedInvoice) -> str: return f"invoice_{number}_{invoice.invoice_date.replace('-', '')}_{short}" +def _rate_text(rate: Decimal) -> str: + """Render a tax rate without trailing zeros, e.g. 15 -> '15', 19.79 -> '19.79'.""" + q = rate.quantize(Decimal("0.01")) + if q == q.to_integral_value(): + return str(int(q)) + return f"{q:.2f}".rstrip("0") + + def _tax_external_id(rate: Decimal) -> str: - return f"purchase_vat_{int(rate)}" + return f"purchase_vat_{_rate_text(rate).replace('.', '_')}" def _currency_ref(currency: str, settings: Settings) -> str: @@ -62,10 +70,22 @@ def _to_str(value: Decimal) -> str: def build_odoo_xml( invoice: ExtractedInvoice, - review: ReviewResult | None, + review: ReviewResult, settings: Settings, ) -> str: - """Generate Odoo data-import XML for the invoice as a vendor bill.""" + """Generate Odoo data-import XML for the invoice as a vendor bill. + + Raises: + ValueError: when the review confidence is below the configured minimum + threshold, because final XML should not be generated for uncertain + extractions. + """ + if review.confidence < settings.review.min_confidence_threshold: + raise ValueError( + f"Review confidence {review.confidence:.2f} is below the minimum threshold " + f"{settings.review.min_confidence_threshold:.2f}; refusing to generate XML." + ) + vendor_id = _external_id_vendor(invoice.vendor_name, invoice.vendor_vat) invoice_id = _external_id_invoice(invoice) @@ -73,7 +93,7 @@ def build_odoo_xml( data = etree.SubElement(odoo, "data", noupdate="0") # Human-review warning comment - if review and review.confidence < settings.review.high_confidence_threshold: + if review.confidence < settings.review.high_confidence_threshold: data.append( etree.Comment( f" HUMAN REVIEW ADVISED: confidence={review.confidence:.2f}, " @@ -90,26 +110,19 @@ def build_odoo_xml( if invoice.vendor_vat: etree.SubElement(vendor_rec, "field", name="vat").text = invoice.vendor_vat - # Taxes - tax_ids_used: list[str] = [] + # Taxes: emit one record per unique tax rate. + tax_rates: dict[str, Decimal] = {} for line in invoice.line_items: - rate = line.tax_rate - if rate == 0 and "purchase_vat_0" not in tax_ids_used: - tax_ids_used.append("purchase_vat_0") + if line.tax_rate == 0: + tax_rates["purchase_vat_0"] = Decimal("0") else: - tid = _tax_external_id(rate) - if tid not in tax_ids_used: - tax_ids_used.append(tid) - tax_rec = etree.SubElement(data, "record", id=tid, model="account.tax") - etree.SubElement(tax_rec, "field", name="name").text = f"Purchase VAT {int(rate)}%" - etree.SubElement(tax_rec, "field", name="amount").text = str(rate) - etree.SubElement(tax_rec, "field", name="amount_type").text = "percent" - etree.SubElement(tax_rec, "field", name="type_tax_use").text = "purchase" + tid = _tax_external_id(line.tax_rate) + tax_rates[tid] = line.tax_rate - if "purchase_vat_0" in tax_ids_used: - tax_rec = etree.SubElement(data, "record", id="purchase_vat_0", model="account.tax") - etree.SubElement(tax_rec, "field", name="name").text = "Purchase VAT 0%" - etree.SubElement(tax_rec, "field", name="amount").text = "0" + for tid, rate in sorted(tax_rates.items(), key=lambda item: item[0]): + tax_rec = etree.SubElement(data, "record", id=tid, model="account.tax") + etree.SubElement(tax_rec, "field", name="name").text = f"Purchase VAT {_rate_text(rate)}%" + etree.SubElement(tax_rec, "field", name="amount").text = str(rate) etree.SubElement(tax_rec, "field", name="amount_type").text = "percent" etree.SubElement(tax_rec, "field", name="type_tax_use").text = "purchase" diff --git a/tests/fixtures/expected_invoice.xml b/tests/fixtures/expected_invoice.xml new file mode 100644 index 0000000..601f6c9 --- /dev/null +++ b/tests/fixtures/expected_invoice.xml @@ -0,0 +1,33 @@ + + + + + Acme Supplies + 1 + GB123456789 + + + Purchase VAT 20% + 20 + percent + purchase + + + in_invoice + + 2024-05-01 + 2024-05-01 + 2024-06-01 + INV-2024-001 + + Net 30 + + + + Consulting services + 10.00 + 100.00 + + + + diff --git a/tests/test_reviewer.py b/tests/test_reviewer.py new file mode 100644 index 0000000..75a6e94 --- /dev/null +++ b/tests/test_reviewer.py @@ -0,0 +1,85 @@ +"""Tests for the vision review stage.""" + +from __future__ import annotations + +import pytest +from PIL import Image + +from odoo_ocr.config import Settings +from odoo_ocr.pipeline.reviewer import review_invoice +from odoo_ocr.schemas import ExtractedInvoice, ReviewResult +from tests.conftest import FakeVLMClient + + +def make_invoice() -> ExtractedInvoice: + return ExtractedInvoice( + vendor_name="Acme", + invoice_number="INV-1", + invoice_date="2024-01-01", + ) + + +@pytest.mark.asyncio +async def test_review_returns_parsed_result() -> None: + client = FakeVLMClient( + responses={ + "invoice review assistant": { + "valid": True, + "confidence": 0.95, + "issues": [], + "corrected_invoice": None, + } + } + ) + result = await review_invoice( + make_invoice(), [Image.new("RGB", (10, 10))], client, Settings() + ) + assert isinstance(result, ReviewResult) + assert result.valid is True + assert result.confidence == 0.95 + + +@pytest.mark.asyncio +async def test_review_sends_all_pages() -> None: + client = FakeVLMClient( + responses={ + "invoice review assistant": { + "valid": True, + "confidence": 0.90, + "issues": [], + "corrected_invoice": None, + } + } + ) + pages = [Image.new("RGB", (10, 10)) for _ in range(3)] + await review_invoice(make_invoice(), pages, client, Settings()) + assert client.calls[-1]["image_count"] == len(pages) + + +@pytest.mark.asyncio +async def test_review_preserves_corrected_invoice() -> None: + corrected = { + "vendor_name": "Acme", + "invoice_number": "INV-1", + "invoice_date": "2024-01-01", + "line_items": [], + "subtotal": 100.0, + "tax_total": 0.0, + "total": 100.0, + } + client = FakeVLMClient( + responses={ + "invoice review assistant": { + "valid": True, + "confidence": 0.95, + "issues": [{"field": "total", "severity": "warning", "message": "rounded"}], + "corrected_invoice": corrected, + } + } + ) + result = await review_invoice( + make_invoice(), [Image.new("RGB", (10, 10))], client, Settings() + ) + assert result.corrected_invoice is not None + assert result.corrected_invoice.total == 100.0 + assert result.issues[0].severity == "warning" diff --git a/tests/test_xml_builder.py b/tests/test_xml_builder.py index bf75a4e..3c7bea0 100644 --- a/tests/test_xml_builder.py +++ b/tests/test_xml_builder.py @@ -1,15 +1,20 @@ """Tests for the Odoo XML builder.""" +from __future__ import annotations + from decimal import Decimal +from pathlib import Path + +import pytest +from lxml import etree from odoo_ocr.config import Settings from odoo_ocr.pipeline.xml_builder import build_odoo_xml from odoo_ocr.schemas import ExtractedInvoice, InvoiceLineItem, ReviewResult -def test_build_odoo_xml() -> None: - settings = Settings() - invoice = ExtractedInvoice( +def _fixture_invoice() -> ExtractedInvoice: + return ExtractedInvoice( vendor_name="Acme Supplies", vendor_vat="GB123456789", invoice_number="INV-2024-001", @@ -30,9 +35,12 @@ def test_build_odoo_xml() -> None: tax_total=Decimal("200.00"), total=Decimal("1200.00"), ) - review = ReviewResult(valid=True, confidence=0.95) - xml = build_odoo_xml(invoice, review, settings) + +def test_build_odoo_xml() -> None: + settings = Settings() + invoice = _fixture_invoice() + xml = build_odoo_xml(invoice, ReviewResult(valid=True, confidence=0.95), settings) assert xml.startswith(" None: assert "in_invoice" in xml +def test_xml_parses_with_lxml() -> None: + invoice = _fixture_invoice() + xml = build_odoo_xml(invoice, ReviewResult(valid=True, confidence=0.95), Settings()) + + root = etree.fromstring(xml.encode("utf-8")) + models = {record.get("model") for record in root.iter("record")} + assert models == {"res.partner", "account.tax", "account.move", "account.move.line"} + + +def test_xml_matches_expected_fixture() -> None: + invoice = _fixture_invoice() + xml = build_odoo_xml(invoice, ReviewResult(valid=True, confidence=0.95), Settings()) + + expected = (Path(__file__).parent / "fixtures" / "expected_invoice.xml").read_text( + encoding="utf-8" + ) + + def _normalize(doc: str) -> dict: + root = etree.fromstring(doc.encode("utf-8")) + + def _el(element) -> dict: + return { + "tag": element.tag, + "attrs": dict(element.attrib.items()), + "text": (element.text or "").strip(), + "children": [_el(child) for child in element], + } + + return _el(root) + + assert _normalize(xml) == _normalize(expected) + + +def test_fractional_tax_rate_ids_are_distinct() -> None: + invoice = ExtractedInvoice( + vendor_name="X", + invoice_number="1", + invoice_date="2024-01-01", + line_items=[ + InvoiceLineItem(description="A", tax_rate=Decimal("19.79")), + InvoiceLineItem(description="B", tax_rate=Decimal("15.2")), + InvoiceLineItem(description="C", tax_rate=Decimal("15.7")), + ], + ) + xml = build_odoo_xml(invoice, ReviewResult(valid=True, confidence=0.95), Settings()) + + assert 'id="purchase_vat_19_79"' in xml + assert 'id="purchase_vat_15_2"' in xml + assert 'id="purchase_vat_15_7"' in xml + assert xml.count('model="account.tax"') == 3 + assert "Purchase VAT 19.79%" in xml + assert "19.79" in xml + + def test_xml_adds_review_comment_for_low_confidence() -> None: settings = Settings() invoice = ExtractedInvoice( @@ -58,3 +120,39 @@ def test_xml_adds_review_comment_for_low_confidence() -> None: xml = build_odoo_xml(invoice, review, settings) assert "HUMAN REVIEW ADVISED" in xml + + +def test_duplicate_tax_rates_emit_one_record() -> None: + """Multiple lines sharing the same tax rate must produce a single account.tax record.""" + invoice = ExtractedInvoice( + vendor_name="X", + invoice_number="1", + invoice_date="2024-01-01", + line_items=[ + InvoiceLineItem(description="A", tax_rate=Decimal("20")), + InvoiceLineItem(description="B", tax_rate=Decimal("20")), + InvoiceLineItem(description="C", tax_rate=Decimal("0")), + ], + ) + xml = build_odoo_xml(invoice, ReviewResult(valid=True, confidence=0.95), Settings()) + + assert xml.count('id="purchase_vat_20"') == 1 + assert xml.count('id="purchase_vat_0"') == 1 + assert xml.count('model="account.tax"') == 2 + + +def test_xml_refuses_low_confidence() -> None: + settings = Settings() + invoice = ExtractedInvoice( + vendor_name="X", + invoice_number="1", + invoice_date="2024-01-01", + line_items=[], + subtotal=Decimal("0"), + tax_total=Decimal("0"), + total=Decimal("0"), + ) + review = ReviewResult(valid=False, confidence=0.70) + + with pytest.raises(ValueError, match="below the minimum threshold"): + build_odoo_xml(invoice, review, settings)