Skip to content

Replace iban with nested BankInfo model#7

Merged
emilwojtaszek merged 2 commits into
mainfrom
feature/bank-details
Apr 2, 2026
Merged

Replace iban with nested BankInfo model#7
emilwojtaszek merged 2 commits into
mainfrom
feature/bank-details

Conversation

@emilwojtaszek

@emilwojtaszek emilwojtaszek commented Apr 2, 2026

Copy link
Copy Markdown
Member

Summary

  • Replace flat iban field with nested BankInfo model containing IBAN, SWIFT/BIC, bank name, bank address, routing number, account number, and payment notes
  • Add default=None to all nullable fields across BankInfo, Address, InvoiceLineItem, and InvoiceSchema so partial dicts validate correctly
  • Sync default_invoice_schema.json with Pydantic model (add missing seller_address, buyer_address, update bank_details)
  • Add unit tests for BankInfo, Address, and InvoiceSchema models

Test plan

  • All 44 tests pass (make test)
  • mypy clean (make typecheck)
  • ruff format + lint clean (make format && make lint)
  • Manual: deploy and extract an invoice, verify bank_details nested object in response

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added comprehensive bank details support (IBAN, SWIFT/BIC, routing/account numbers, bank name/address, notes).
    • Added seller and buyer address fields.
    • Extended tax ID support to include EU VAT-style IDs and US EIN/TIN.
    • Made currency and line items required for valid invoices.
  • Documentation

    • README updated to mention bank details in the default extraction description.

Extract full bank/wire transfer details (IBAN, SWIFT/BIC, bank name,
routing number, account number, payment instructions) instead of just
IBAN. Also adds default=None to all nullable fields, syncs the JSON
schema documentation, and adds unit tests for schema models.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Apr 2, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9dcc1159-4336-4d2e-99f5-ec8d4c251c21

📥 Commits

Reviewing files that changed from the base of the PR and between 2b99e82 and 4025a3e.

📒 Files selected for processing (3)
  • tests/integration/test_extract_endpoint.py
  • tests/unit/test_invoice_schema.py
  • tests/unit/test_pdf_parser.py
✅ Files skipped from review due to trivial changes (1)
  • tests/unit/test_pdf_parser.py

📝 Walkthrough

Walkthrough

Invoice schema and Pydantic models expanded for international use: added structured bank_details, seller/buyer addresses, broadened tax ID descriptions, made currency and line_items required, and updated tests and README to reflect these schema changes.

Changes

Cohort / File(s) Summary
Invoice JSON Schema
src/app/schemas/default_invoice_schema.json
Replaced iban with nullable bank_details object (iban, swift_bic, bank_name, bank_address, routing_number, account_number, notes); added seller_address and buyer_address objects; broadened seller_nip/buyer_nip descriptions; added currency and line_items to required.
Pydantic Models
src/app/schemas/invoice.py
Added BankInfo model; replaced iban with `bank_details: BankInfo
Tests
tests/unit/test_invoice_schema.py, tests/integration/test_extract_endpoint.py, tests/unit/test_llm_extractor.py, tests/unit/test_pdf_parser.py
Added unit tests for BankInfo, Address, and InvoiceSchema (round-trip, coercion, None handling). Minor typing/formatting changes in several test functions; no logic/assertion changes.
Docs
README.md
Updated "default extraction" description to mention "bank details" alongside existing invoice fields.

Sequence Diagram(s)

(Skipped — changes are schema/model additions without multi-component sequential control flow.)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 I nibble fields both near and far,
From IBAN trails to a routing star.
Addresses tucked in gentle rows,
Tax IDs wearing country codes.
Hoppy schemas, tidy and bright—bank details land just right! 🥕📜

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 6.25% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Replace iban with nested BankInfo model' accurately summarizes the main structural change in the PR, which is replacing a flat iban field with a new nested BankInfo model.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/bank-details

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
tests/unit/test_pdf_parser.py (1)

200-205: ⚠️ Potential issue | 🟡 Minor

Add type hints to stub method signatures.

The methods pymupdf_rect_stub.__init__ (line 200) and pymupdf_rect_stub.intersects (line 203) in the touched block lack type annotations, which violates the coding guideline requiring type hints on all Python function signatures.

Proposed fix
 class pymupdf_rect_stub:
     """Stub for pymupdf.Rect that supports intersects()."""

-    def __init__(self, bbox):
+    def __init__(self, bbox: tuple[float, float, float, float]) -> None:
         self.x0, self.y0, self.x1, self.y1 = bbox

-    def intersects(self, other):
+    def intersects(self, other: "pymupdf_rect_stub") -> bool:
         return not (
             self.x1 <= other.x0 or other.x1 <= self.x0 or self.y1 <= other.y0 or other.y1 <= self.y0
         )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/unit/test_pdf_parser.py` around lines 200 - 205, The stub methods
pymupdf_rect_stub.__init__ and pymupdf_rect_stub.intersects are missing type
annotations; update __init__ to annotate the bbox parameter (e.g.,
Sequence[float] or Tuple[float, float, float, float]) and the return type as
None, and update intersects to annotate its other parameter as the same rect
type (or "pymupdf_rect_stub") and its return type as bool; ensure you import or
reference typing types (Sequence/Tuple) as needed and keep names consistent with
the class for static checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/integration/test_extract_endpoint.py`:
- Line 148: The test function test_extract_with_custom_pdf_settings is missing
type annotations; update its signature to include parameter and return type
hints for client, auth_header, mock_pdf_parser, and mock_llm_extractor and a
return type of None (e.g., client typed as the project test client type like
FlaskClient or TestClient, or typing.Any if a concrete fixture type is
unavailable); also add any necessary imports (e.g., from typing import Any) so
the annotations are valid.

In `@tests/unit/test_invoice_schema.py`:
- Around line 5-169: Add explicit return type annotations (-> None) to all test
methods listed: test_all_fields_populated, both test_all_none_is_valid methods,
test_us_wire_fields, test_round_trip_json, test_full_address,
test_bank_details_nested, test_bank_details_null_is_valid, and
test_bank_details_from_dict; update each function signature in the Test classes
and module-level tests to include "-> None" (e.g., def
test_all_fields_populated(self) -> None:) so the test definitions have proper
type hints while preserving their bodies and assertions.

---

Outside diff comments:
In `@tests/unit/test_pdf_parser.py`:
- Around line 200-205: The stub methods pymupdf_rect_stub.__init__ and
pymupdf_rect_stub.intersects are missing type annotations; update __init__ to
annotate the bbox parameter (e.g., Sequence[float] or Tuple[float, float, float,
float]) and the return type as None, and update intersects to annotate its other
parameter as the same rect type (or "pymupdf_rect_stub") and its return type as
bool; ensure you import or reference typing types (Sequence/Tuple) as needed and
keep names consistent with the class for static checks.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7f810a03-55f3-411d-aeaa-f00c0495e19b

📥 Commits

Reviewing files that changed from the base of the PR and between 74af48a and 2b99e82.

📒 Files selected for processing (7)
  • README.md
  • src/app/schemas/default_invoice_schema.json
  • src/app/schemas/invoice.py
  • tests/integration/test_extract_endpoint.py
  • tests/unit/test_invoice_schema.py
  • tests/unit/test_llm_extractor.py
  • tests/unit/test_pdf_parser.py

Comment thread tests/integration/test_extract_endpoint.py Outdated
Comment thread tests/unit/test_invoice_schema.py Outdated
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@emilwojtaszek emilwojtaszek merged commit 4b4dd29 into main Apr 2, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant