Add review stage coverage and fix XML tax deduplication

- Reviewer now sends all pages to the review VLM alongside extracted JSON.

- XML builder deduplicates account.tax records by rate.

- XML builder raises on confidence below min_confidence_threshold.

- Adds fractional tax-rate support with stable external IDs.

- Adds expected_invoice.xml fixture, reviewer tests, and XML regression tests.

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."""
+    """Convert raw OCR text into a validated ExtractedInvoice.
-    system_prompt = _load_prompt("extraction_system.txt")
+
+    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. "

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."""
+    """Have a vision model review the extracted invoice against the image.
-    system_prompt = _load_prompt("review_system.txt")
+
+    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,

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
+    # Taxes: emit one record per unique tax rate.
-    tax_ids_used: list[str] = []
+    tax_rates: dict[str, Decimal] = {}
     for line in invoice.line_items:
-        rate = line.tax_rate
+        if line.tax_rate == 0:
-        if rate == 0 and "purchase_vat_0" not in tax_ids_used:
+            tax_rates["purchase_vat_0"] = Decimal("0")
-            tax_ids_used.append("purchase_vat_0")
         else:
-            tid = _tax_external_id(rate)
+            tid = _tax_external_id(line.tax_rate)
-            if tid not in tax_ids_used:
+            tax_rates[tid] = line.tax_rate
-                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"
 
-    if "purchase_vat_0" in tax_ids_used:
+    for tid, rate in sorted(tax_rates.items(), key=lambda item: item[0]):
-        tax_rec = etree.SubElement(data, "record", id="purchase_vat_0", model="account.tax")
+        tax_rec = etree.SubElement(data, "record", id=tid, model="account.tax")
-        etree.SubElement(tax_rec, "field", name="name").text = "Purchase VAT 0%"
+        etree.SubElement(tax_rec, "field", name="name").text = f"Purchase VAT {_rate_text(rate)}%"
-        etree.SubElement(tax_rec, "field", name="amount").text = "0"
+        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"
 

tests/fixtures/expected_invoice.xml

@@ -0,0 +1,33 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<odoo>
+  <data noupdate="0">
+    <record id="vendor_acme_supplies_078953f5" model="res.partner">
+      <field name="name">Acme Supplies</field>
+      <field name="supplier_rank">1</field>
+      <field name="vat">GB123456789</field>
+    </record>
+    <record id="purchase_vat_20" model="account.tax">
+      <field name="name">Purchase VAT 20%</field>
+      <field name="amount">20</field>
+      <field name="amount_type">percent</field>
+      <field name="type_tax_use">purchase</field>
+    </record>
+    <record id="invoice_inv_2024_001_20240501_8047d436" model="account.move">
+      <field name="move_type">in_invoice</field>
+      <field name="partner_id" ref="vendor_acme_supplies_078953f5"/>
+      <field name="invoice_date">2024-05-01</field>
+      <field name="date">2024-05-01</field>
+      <field name="invoice_date_due">2024-06-01</field>
+      <field name="ref">INV-2024-001</field>
+      <field name="currency_id" ref="base.EUR"/>
+      <field name="narration">Net 30</field>
+    </record>
+    <record id="invoice_inv_2024_001_20240501_8047d436_line_1" model="account.move.line">
+      <field name="move_id" ref="invoice_inv_2024_001_20240501_8047d436"/>
+      <field name="name">Consulting services</field>
+      <field name="quantity">10.00</field>
+      <field name="price_unit">100.00</field>
+      <field name="tax_ids" eval="[(6, 0, [ref('purchase_vat_20')])]"/>
+    </record>
+  </data>
+</odoo>

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"

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:
+def _fixture_invoice() -> ExtractedInvoice:
-    settings = Settings()
+    return ExtractedInvoice(
-    invoice = 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("<?xml version=")
     assert 'model="res.partner"' in xml
@@ -43,6 +51,60 @@ def test_build_odoo_xml() -> 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 "<field name=\"amount\">19.79</field>" 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)