jonah/safe-ai-tools

Fork 0

feat: German invoices and corpus eval (0.5.0, stacked on #6) #7

Open

jonah wants to merge 4 commits from feat/nl-de-corpus into feat/invoice-to-csv

jonah commented

2026-10-06 22:21:15 +02:00

Owner

Stacked on #6: merge that first, then retarget this to main.

What

German support: labels, month names, VAT rates by supplier country (NL 0/9/21, DE 0/7/19).
Table rows with euro signs, two-digit years, dd/mm/yyyy dates settled by another date of the same document, customer blocks hidden, supplier found above its address.
An unreadable due date now goes to review instead of being left empty.
eval --corpus DIR scores a pdf/ + csv/ folder and lists top review reasons.

Measured on the generated corpus (200 NL + 200 DE, one layout per language)

Before 0/400 accepted. Now every field is right on every invoice read (NL 190, DE 175 once checksums are set aside); 0 silent errors; also 0 on 6 fresh clutter seeds. As generated, ~95% of IBANs and VAT numbers fail their checksums, so almost everything goes to review for that reason; that is the generator, not the reader. One layout per language: this does not give a real-world rate.

Checked

340 hermetic tests, ruff, import-linter, pre-commit.

🤖 Generated with Claude Code

Stacked on #6: merge that first, then retarget this to main. ## What - German support: labels, month names, VAT rates by supplier country (NL 0/9/21, DE 0/7/19). - Table rows with euro signs, two-digit years, dd/mm/yyyy dates settled by another date of the same document, customer blocks hidden, supplier found above its address. - An unreadable due date now goes to review instead of being left empty. - eval --corpus DIR scores a pdf/ + csv/ folder and lists top review reasons. ## Measured on the generated corpus (200 NL + 200 DE, one layout per language) Before 0/400 accepted. Now every field is right on every invoice read (NL 190, DE 175 once checksums are set aside); 0 silent errors; also 0 on 6 fresh clutter seeds. As generated, ~95% of IBANs and VAT numbers fail their checksums, so almost everything goes to review for that reason; that is the generator, not the reader. One layout per language: this does not give a real-world rate. ## Checked 340 hermetic tests, ruff, import-linter, pre-commit. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

jonah added 1 commit

2026-10-06 22:21:15 +02:00

feat: German invoices and a corpus eval (0.5.0) 23cea49070

Currency signs in table rows, German labels and months, two-digit years, slash dates settled by the document, customer blocks, supplier above its address, VAT rates by country, refuse an unreadable due date, eval --corpus.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

jonah commented

2026-10-06 22:22:02 +02:00

Author

Owner

🤖 Auto-review status · jonah/safe-ai-tools PR #7

📋 Review posted — fixes required, awaiting human action.

Last updated: 2026-10-06T20:30:10.968Z

**🤖 Auto-review status** · `jonah/safe-ai-tools` PR #7 📋 **Review posted** — fixes required, awaiting human action. *Last updated: 2026-10-06T20:30:10.968Z*

jonah reviewed

2026-10-06 22:30:10 +02:00

jonah left a comment

[STATUS: FIXES_REQUIRED]

Review of feat/nl-de-corpus vs feat/invoice-to-csv (1 commit: 23cea49, 21 files, +452/−50).

Verification run: pytest -m "not integration" → 340 passed · ruff check . → clean · lint-imports → 4/4 contracts kept.

Must fix

Silent gold corruption in the corpus loader — eval/corpus.py:68,83,87: _invoice reads the VAT rate, subtotal, and VAT amount from rows[0] only, then stamps that single rate onto every line (vat_rate=rate) and emits exactly one VatLine. The format is per-row ("header fields repeated", and the CSV carries a BTW Tarief/MwSt Satz column per line item), yet every other line field is read from r. Verified: a gold CSV with rates [9, 21] loads as gold rates [9, 9] — a correct extraction then scores as wrong, silently, which is the worst failure mode for an accuracy harness. Read r[names["rate"]] per row and group VatLines by rate (the pattern already exists in data/invoices/generate.py:_vat_block), or assert single-rate at load time.
Malformed corpus crashes with raw tracebacks; missing PDFs are miscounted — eval/corpus.py:99 raises bare StopIteration on an unknown CSV header; corpus.py:62 raises ValueError on an empty/unreadable gold date; corpus.py:67 raises IndexError on a header-only CSV. None are caught by eval/__main__.main (only FileNotFoundError is), so the user gets a stack trace instead of the parser.error usage path used everywhere else in that CLI. Conversely, corpus.py:101 never checks the paired pdf/<stem>.pdf exists — a missing PDF flows into ingest.py:23 (except Exception → IngestError) and is scored as an extraction failure (unreadable PDF (FileNotFoundError) in the review-reasons table), silently inflating the refusal counts that CHANGELOG reports. Fail fast with a clear message on corpus-shape errors; check the PDF is present at load.
New scoring logic ships untested — eval/score.py:101-110,135-137: Report.reasons, _label (including its layout not recognised special case), and the top-10 markdown table are new behavior with zero assertions — test_eval_score.py was only touched for an error-string rename. Project rule §1.VIII / ponytail: non-trivial logic leaves one runnable check. One small test: a Rejected with ("iban_checksum_failed: x", "layout not recognised: …") → correct reasons dict + table rows.

Should fix (low)

Mislabelled count — eval/score.py:48,136: reasons counts reason occurrences, not invoices — one invoice missing both iban and supplier counts 2 under missing_field, but the comment and the column header say "Invoices". Rename the column or count distinct invoices per label.
Grounding gap for unpadded short-year dates — ground.py:35-36: full-year forms exist padded and unpadded (3.5.2026), two-digit-year forms only padded (03.05.25). parse_date accepts 3.5.25, so such an invoice extracts fine and then fails grounding → spurious review (fail-safe, but noise). Add f"{day.day}-{day.month}-{yy}" / f"{day.day}.{day.month}.{yy}" for symmetry.
CHANGELOG's "Measured" numbers aren't reproducible from this repo — the generator that emits the 400-invoice pdf/+csv/ corpus is on no branch and in no directory (git grep across all refs: only the loader exists). README documents --corpus DIR but there's no way to regenerate the corpus the reported 190/175/0-silent-errors figures came from. Commit the generator or link where it lives.

Checked and clean

_own customer-block hiding (header.py:114-130) — traced all branches: label-only line hides exactly _MAX_BLOCK lines, block-end line resets and stays visible, old drop-any-customer-line behavior preserved; German/customer tests cover it.
slash_order + _slash_date order settling (numbers.py:69-92) — contradictory evidence → None → refuse; tests cover all four cases.
Two-decimal-first amount_convention (numbers.py:36-45) — 2025.0006 can no longer count as EN cents; fallback to the old 3–4 decimal pass preserved.
Due-date hard reject (header.py:109-110) — documented behavior change, fails safe (review, never accept), ExtractError message is user-safe.
Country-scoped VAT rates (validate.py:79-86) — missing/unknown VAT number skips only the rate check; the identifier/missing-field problem still fires, exactly as the CHANGELOG claims.
Version bumps consistent (VERSION, both pyproject.toml, uv.lock all 0.5.0); euro-sign stripping in table.py can only cause an extra grounding review, never a silent wrong output.

[STATUS: FIXES_REQUIRED] Review of `feat/nl-de-corpus` vs `feat/invoice-to-csv` (1 commit: `23cea49`, 21 files, +452/−50). **Verification run:** `pytest -m "not integration"` → 340 passed · `ruff check .` → clean · `lint-imports` → 4/4 contracts kept. ### Must fix - **Silent gold corruption in the corpus loader** — `eval/corpus.py:68,83,87`: `_invoice` reads the VAT rate, subtotal, and VAT amount from `rows[0]` only, then stamps that single rate onto *every* line (`vat_rate=rate`) and emits exactly one `VatLine`. The format is per-row ("header fields repeated", and the CSV carries a `BTW Tarief`/`MwSt Satz` column per line item), yet every other line field is read from `r`. Verified: a gold CSV with rates `[9, 21]` loads as gold rates `[9, 9]` — a correct extraction then scores as wrong, silently, which is the worst failure mode for an accuracy harness. Read `r[names["rate"]]` per row and group `VatLine`s by rate (the pattern already exists in `data/invoices/generate.py:_vat_block`), or `assert` single-rate at load time. - **Malformed corpus crashes with raw tracebacks; missing PDFs are miscounted** — `eval/corpus.py:99` raises bare `StopIteration` on an unknown CSV header; `corpus.py:62` raises `ValueError` on an empty/unreadable gold date; `corpus.py:67` raises `IndexError` on a header-only CSV. None are caught by `eval/__main__.main` (only `FileNotFoundError` is), so the user gets a stack trace instead of the `parser.error` usage path used everywhere else in that CLI. Conversely, `corpus.py:101` never checks the paired `pdf/<stem>.pdf` exists — a missing PDF flows into `ingest.py:23` (`except Exception` → `IngestError`) and is scored as an extraction failure (`unreadable PDF (FileNotFoundError)` in the review-reasons table), silently inflating the refusal counts that CHANGELOG reports. Fail fast with a clear message on corpus-shape errors; check the PDF is present at load. - **New scoring logic ships untested** — `eval/score.py:101-110,135-137`: `Report.reasons`, `_label` (including its `layout not recognised` special case), and the top-10 markdown table are new behavior with zero assertions — `test_eval_score.py` was only touched for an error-string rename. Project rule §1.VIII / ponytail: non-trivial logic leaves one runnable check. One small test: a `Rejected` with `("iban_checksum_failed: x", "layout not recognised: …")` → correct `reasons` dict + table rows. ### Should fix (low) - **Mislabelled count** — `eval/score.py:48,136`: `reasons` counts *reason occurrences*, not invoices — one invoice missing both `iban` and `supplier` counts 2 under `missing_field`, but the comment and the column header say "Invoices". Rename the column or count distinct invoices per label. - **Grounding gap for unpadded short-year dates** — `ground.py:35-36`: full-year forms exist padded *and* unpadded (`3.5.2026`), two-digit-year forms only padded (`03.05.25`). `parse_date` accepts `3.5.25`, so such an invoice extracts fine and then fails grounding → spurious review (fail-safe, but noise). Add `f"{day.day}-{day.month}-{yy}"` / `f"{day.day}.{day.month}.{yy}"` for symmetry. - **CHANGELOG's "Measured" numbers aren't reproducible from this repo** — the generator that emits the 400-invoice `pdf/`+`csv/` corpus is on no branch and in no directory (`git grep` across all refs: only the loader exists). README documents `--corpus DIR` but there's no way to regenerate the corpus the reported 190/175/0-silent-errors figures came from. Commit the generator or link where it lives. ### Checked and clean - `_own` customer-block hiding (`header.py:114-130`) — traced all branches: label-only line hides exactly `_MAX_BLOCK` lines, block-end line resets and stays visible, old drop-any-customer-line behavior preserved; German/customer tests cover it. - `slash_order` + `_slash_date` order settling (`numbers.py:69-92`) — contradictory evidence → `None` → refuse; tests cover all four cases. - Two-decimal-first `amount_convention` (`numbers.py:36-45`) — `2025.0006` can no longer count as EN cents; fallback to the old 3–4 decimal pass preserved. - Due-date hard reject (`header.py:109-110`) — documented behavior change, fails safe (review, never accept), `ExtractError` message is user-safe. - Country-scoped VAT rates (`validate.py:79-86`) — missing/unknown VAT number skips only the rate check; the identifier/missing-field problem still fires, exactly as the CHANGELOG claims. - Version bumps consistent (`VERSION`, both `pyproject.toml`, `uv.lock` all 0.5.0); euro-sign stripping in `table.py` can only cause an extra grounding review, never a silent wrong output.

jonah commented

2026-10-06 23:37:02 +02:00

Author

Owner

🤖 Auto-review status · jonah/safe-ai-tools PR #7

📋 Review posted — fixes required, awaiting human action.

Last updated: 2026-10-06T21:40:54.752Z

**🤖 Auto-review status** · `jonah/safe-ai-tools` PR #7 📋 **Review posted** — fixes required, awaiting human action. *Last updated: 2026-10-06T21:40:54.752Z*

jonah reviewed

2026-10-06 23:40:54 +02:00

jonah left a comment

[STATUS: FIXES_REQUIRED]

Review of feat/nl-de-corpus vs feat/invoice-to-csv — 2 commits (23cea49, 3552156), 22 files. The new commit (3552156) is small and its core fix is sound, but all three must-fix items from the previous review are still unaddressed, plus one misleading artifact in the new commit.

Verification run: pytest -m "not integration" → 341 passed (340 + 1 new) · ruff check . → clean · lint-imports → 4/4 contracts kept.

Must fix — unaddressed from previous review (re-verified, all still reproduce)

Silent gold corruption in the corpus loader — eval/corpus.py:67-87: unchanged since 23cea49. _invoice still reads rate/subtotal from rows[0] only and stamps it on every line (vat_rate=rate, line 83) and into a single VatLine (line 87). Reproduced just now: a gold CSV with per-row rates [9, 21] loads as [9, 9] with one wrong VatLine — a correct extraction scores as wrong, silently. test_eval_corpus.py only uses single-rate fixtures, so it can't catch this. Read r[names["rate"]] per row and group VatLines by rate, or assert single-rate at load.
Malformed corpus crashes with raw tracebacks; missing PDFs miscounted — eval/corpus.py unchanged. Verified: unknown CSV header → StopIteration (line 99); header-only CSV → IndexError (line 67); main catches only FileNotFoundError (eval/__main__.py:44), so all escape to a stack trace instead of the parser.error path. And corpus.py:101-102 still never checks pdf/<stem>.pdf exists (grep exists() eval/ → none) — a missing PDF loads fine and is scored as an extraction failure, inflating the refusal counts the CHANGELOG reports.
New scoring logic still ships untested — eval/score.py:101-110,135-137: Report.reasons, _label (incl. the layout not recognised special case), and the top-10 table still have zero assertions (grep reasons\|_label test_eval_score.py → no output). The test file's only change on this branch is an error-string rename. One small test with a Rejected carrying ("iban_checksum_failed: x", "layout not recognised: …") asserting the reasons dict and table rows.

Should fix — unaddressed from previous review

Mislabelled count — eval/score.py:48,136: reasons counts reason occurrences (a Counter over all reasons of all rejected invoices — one invoice missing two fields under missing_field counts 2), but the comment says "how many invoices" and the column header says "Invoices". Rename the column or count distinct invoices per label.
Grounding gap for unpadded short-year dates — ground.py:35-36: full-year forms exist padded and unpadded (day.month.year), two-digit-year forms only padded (% 100:02d). parse_date accepts the unpadded form → extracts fine, fails grounding, spurious review. Add f"{day.day}-{day.month}-{yy}" / f"{day.day}.{day.month}-{yy}" for symmetry.
CHANGELOG "Measured" numbers are still unverifiable — and got more specific — CHANGELOG.md:21-24 rewritten with new figures (DE 175→200 read, NL 24/200 and DE 26/200 accepted, "87% VAT fail", "DE 181+"), but the pdf/+csv/ corpus generator is still in no directory on no ref (git grep "Leverancier Naam" across all refs → only the loader and its tests; data/invoices/generate.py writes pdfs/+gold.jsonl, a different format). The DE jump 175→200 is not explained by the one-line IBAN fix (the old 25 DE refusals were postcode-driven extraction failures, unrelated to IBAN) — it comes from the regenerated corpus, which nobody can reproduce. Commit the generator or link where it lives. Related: eval/__main__.py:45 tells --corpus users "See data/invoices/generate.py", which cannot generate that format.

New issues in `3552156`

Comment claims coverage the code doesn't have — rules/header.py:136: # "DE73 4811799" in a VAT number is not the start of an IBAN, but _VAT (line 64) is DE\d{9} contiguous — it doesn't match a spaced VAT. Verified: USt-IdNr.: DE73 4811799 | … on an IBAN line still yields two candidates (['DE734811799S6586704660', 'DE38362125644960532269']) → _iban returns None → review. Fail-safe, not silent, but if PDF extraction or the invoice prints the VAT grouped/spaced the fix silently doesn't apply while the comment says it does. Either match the spaced form too or reword the comment to the contiguous case.

Checked clean (new commit)

The _VAT.sub masking itself is correct: a real NL (18) or DE (22) IBAN can never match _VAT (the (?![A-Za-z0-9]) lookahead fails mid-IBAN — verified), a compact-VAT line now yields exactly one candidate, and the added test test_a_german_vat_number_on_the_same_line_is_not_taken_for_an_iban covers the fix with a full read_header assertion.
New CHANGELOG validator claim checks out: ust_idnr_de("DE136695976") → True, btw_nl("NL820646660B01") → True; the corpus-shaped NL550455977B28 still fails as claimed.

[STATUS: FIXES_REQUIRED] Review of `feat/nl-de-corpus` vs `feat/invoice-to-csv` — 2 commits (`23cea49`, `3552156`), 22 files. The new commit (`3552156`) is small and its core fix is sound, but **all three must-fix items from the previous review are still unaddressed**, plus one misleading artifact in the new commit. **Verification run:** `pytest -m "not integration"` → 341 passed (340 + 1 new) · `ruff check .` → clean · `lint-imports` → 4/4 contracts kept. ### Must fix — unaddressed from previous review (re-verified, all still reproduce) - **Silent gold corruption in the corpus loader** — `eval/corpus.py:67-87`: unchanged since `23cea49`. `_invoice` still reads `rate`/`subtotal` from `rows[0]` only and stamps it on every line (`vat_rate=rate`, line 83) and into a single `VatLine` (line 87). Reproduced just now: a gold CSV with per-row rates `[9, 21]` loads as `[9, 9]` with one wrong `VatLine` — a correct extraction scores as wrong, silently. `test_eval_corpus.py` only uses single-rate fixtures, so it can't catch this. Read `r[names["rate"]]` per row and group `VatLine`s by rate, or `assert` single-rate at load. - **Malformed corpus crashes with raw tracebacks; missing PDFs miscounted** — `eval/corpus.py` unchanged. Verified: unknown CSV header → `StopIteration` (line 99); header-only CSV → `IndexError` (line 67); `main` catches only `FileNotFoundError` (`eval/__main__.py:44`), so all escape to a stack trace instead of the `parser.error` path. And `corpus.py:101-102` still never checks `pdf/<stem>.pdf` exists (`grep exists() eval/` → none) — a missing PDF loads fine and is scored as an extraction failure, inflating the refusal counts the CHANGELOG reports. - **New scoring logic still ships untested** — `eval/score.py:101-110,135-137`: `Report.reasons`, `_label` (incl. the `layout not recognised` special case), and the top-10 table still have zero assertions (`grep reasons\|_label test_eval_score.py` → no output). The test file's only change on this branch is an error-string rename. One small test with a `Rejected` carrying `("iban_checksum_failed: x", "layout not recognised: …")` asserting the `reasons` dict and table rows. ### Should fix — unaddressed from previous review - **Mislabelled count** — `eval/score.py:48,136`: `reasons` counts reason *occurrences* (a `Counter` over all reasons of all rejected invoices — one invoice missing two fields under `missing_field` counts 2), but the comment says "how many invoices" and the column header says "Invoices". Rename the column or count distinct invoices per label. - **Grounding gap for unpadded short-year dates** — `ground.py:35-36`: full-year forms exist padded *and* unpadded (`day.month.year`), two-digit-year forms only padded (`% 100:02d`). `parse_date` accepts the unpadded form → extracts fine, fails grounding, spurious review. Add `f"{day.day}-{day.month}-{yy}"` / `f"{day.day}.{day.month}-{yy}"` for symmetry. - **CHANGELOG "Measured" numbers are still unverifiable — and got more specific** — `CHANGELOG.md:21-24` rewritten with new figures (DE 175→200 read, NL 24/200 and DE 26/200 accepted, "87% VAT fail", "DE 181+"), but the `pdf/`+`csv/` corpus generator is still in no directory on no ref (`git grep "Leverancier Naam"` across all refs → only the loader and its tests; `data/invoices/generate.py` writes `pdfs/`+`gold.jsonl`, a different format). The DE jump 175→200 is *not* explained by the one-line IBAN fix (the old 25 DE refusals were postcode-driven extraction failures, unrelated to IBAN) — it comes from the regenerated corpus, which nobody can reproduce. Commit the generator or link where it lives. Related: `eval/__main__.py:45` tells `--corpus` users "See data/invoices/generate.py", which cannot generate that format. ### New issues in `3552156` - **Comment claims coverage the code doesn't have** — `rules/header.py:136`: `# "DE73 4811799" in a VAT number is not the start of an IBAN`, but `_VAT` (line 64) is `DE\d{9}` *contiguous* — it doesn't match a spaced VAT. Verified: `USt-IdNr.: DE73 4811799 | …` on an IBAN line still yields two candidates (`['DE734811799S6586704660', 'DE38362125644960532269']`) → `_iban` returns `None` → review. Fail-safe, not silent, but if PDF extraction or the invoice prints the VAT grouped/spaced the fix silently doesn't apply while the comment says it does. Either match the spaced form too or reword the comment to the contiguous case. ### Checked clean (new commit) - The `_VAT.sub` masking itself is correct: a real NL (18) or DE (22) IBAN can never match `_VAT` (the `(?![A-Za-z0-9])` lookahead fails mid-IBAN — verified), a compact-VAT line now yields exactly one candidate, and the added test `test_a_german_vat_number_on_the_same_line_is_not_taken_for_an_iban` covers the fix with a full `read_header` assertion. - New CHANGELOG validator claim checks out: `ust_idnr_de("DE136695976")` → True, `btw_nl("NL820646660B01")` → True; the corpus-shaped `NL550455977B28` still fails as claimed.

jonah added 1 commit

2026-10-07 00:48:57 +02:00

feat: diagnose command that prints masked shapes of refused invoices 4b37b771eb

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

jonah added 1 commit

2026-10-07 00:52:01 +02:00

feat: diagnose explains how invoice-date labels were read 5e25e1b80f

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

This pull request can be merged automatically.

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin feat/nl-de-corpus:feat/nl-de-corpus

git switch feat/nl-de-corpus

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.