From 968d22b2788e27cb86f22ef8081f1645201e0dec Mon Sep 17 00:00:00 2001 From: Ramesh Padmanabhaiah Date: Tue, 16 Jun 2026 09:49:49 -0700 Subject: [PATCH] Add v2 document storage foundation --- .ai-context/ARCHITECTURE.md | 9 + .ai-context/DECISIONS.md | 10 + CHANGELOG.md | 3 + docs/financial_intelligence_architecture.md | 76 ++++- docs/financial_intelligence_open_questions.md | 31 +- src/bankbuddy/financial/dao.py | 6 +- src/bankbuddy/financial/records.py | 60 +++- src/bankbuddy/financial/storage.py | 240 ++++++++++++++ .../0011_financial_document_storage.sql | 175 ++++++++++ src/bankbuddy/paths.py | 28 ++ tests/test_database.py | 9 +- tests/test_financial_foundation.py | 4 - tests/test_financial_storage.py | 308 ++++++++++++++++++ 13 files changed, 926 insertions(+), 33 deletions(-) create mode 100644 src/bankbuddy/financial/storage.py create mode 100644 src/bankbuddy/migrations/0011_financial_document_storage.sql create mode 100644 tests/test_financial_storage.py diff --git a/.ai-context/ARCHITECTURE.md b/.ai-context/ARCHITECTURE.md index 39b2dd3..2ab2f2b 100644 --- a/.ai-context/ARCHITECTURE.md +++ b/.ai-context/ARCHITECTURE.md @@ -46,6 +46,9 @@ New data homes use: - `bank/inbox`, `bank/processed`, `bank/duplicates`, and `bank/exports` for banking statements and exports. - Matching `tax/` folders for tax document workflows. +- `financial/inbox`, `financial/canonical`, `financial/failed`, + `financial/duplicates`, `financial/review`, `financial/views`, and + `financial/exports` for v2 document-first workflows. ## Banking Import Model @@ -91,3 +94,9 @@ exists. Early v2 work should add `BB_` foundation tables beside existing legacy tables, keep new SQL centralized behind DAO modules, and focus on generic document storage/import, provenance, and inspect/report foundations before adding a user-facing infer workflow. + +V2 document storage should treat `financial/canonical` as the source of truth +and `financial/views` as generated human-readable copies. Domain tables should +link to `BB_DOCUMENT`, `BB_DOCUMENT_OBJECT`, or `BB_DOCUMENT_VIEW` rather than +storing filesystem paths directly. The legacy `bank/` and `tax/` trees remain +available for v1 workflows while v2 matures side by side. diff --git a/.ai-context/DECISIONS.md b/.ai-context/DECISIONS.md index 9f593cc..0de5944 100644 --- a/.ai-context/DECISIONS.md +++ b/.ai-context/DECISIONS.md @@ -44,6 +44,16 @@ as account identity. Imports, repairs, and storage-layout migration expose dry-run paths so parser, duplicate, archive, and migration decisions can be reviewed before writes. +## V2 Canonical Storage And Human Views + +The financial-intelligence v2 model stores authoritative document objects under +`financial/canonical` and exposes user-friendly generated copies under +`financial/views`. The database centralizes path metadata in storage roots, +document objects, and document views; domain facts reference document/object +rows instead of embedding filesystem paths. Human views are copies by default, +not symlinks or hardlinks, so they can be rebuilt, reconciled, and repaired +without making user-browsable folders the heart of the system. + ## Mask Sensitive Account Details Full account numbers can be stored for validation and exact matching, but diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b6e270..492c106 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,9 @@ and versions are tracked in the repo-root `VERSION` file. ### Added +- Added v2 financial document storage roots, document object metadata, + generated human-readable document view metadata, path-resolution helpers, + and read-only managed-file guardrails under the `financial/` namespace. - Added additive financial intelligence v2 foundation tables, seed type dictionaries, indexes, and DAO-based read/write helpers without dropping existing banking or tax tables. diff --git a/docs/financial_intelligence_architecture.md b/docs/financial_intelligence_architecture.md index 06bc8e4..121df8c 100644 --- a/docs/financial_intelligence_architecture.md +++ b/docs/financial_intelligence_architecture.md @@ -39,8 +39,17 @@ The following decisions define the current v2 direction: foundation, while allowing details to evolve during implementation. - Introduce the v2 foundation schema additively beside the existing legacy tables. Do not silently discard existing tables or data. +- Keep v2 documents in a separate `financial/` storage namespace so early v2 + work does not mix with the existing `bank/` and `tax/` trees. - Move imported documents into managed storage when an import activity finishes, whether the import succeeds or fails. +- Treat `financial/canonical` as the authoritative document-object store and + `financial/views` as generated human-readable copies for transparency. +- Store canonical/view path metadata in `BB_STORAGE_ROOT`, + `BB_DOCUMENT_OBJECT`, and `BB_DOCUMENT_VIEW`. Other v2 domain tables should + reference document/object rows rather than storing filesystem paths. +- Do not use symlinks or hardlinks as the default human-view mechanism. Views + are copies that can be reconciled, repaired, or regenerated. - Treat future `infer` work as read-only with respect to source documents unless a later design explicitly says otherwise. - Pause TaxBuddy issue #100 until the v2 document/entity/observation model @@ -262,10 +271,11 @@ W-2, Form 26AS, AIS, mortgage statement, property tax bill, insurance policy. Documents should track: -- original filename/path; -- canonical filename/path when copied into managed storage; +- original filename; - SHA-256 hash; - MIME/file type; +- authoritative storage object metadata; +- generated human-readable view metadata; - document type; - source institution when known; - jurisdiction; @@ -342,6 +352,8 @@ Conceptual ER: ```text BB_DOCUMENT + -> BB_DOCUMENT_OBJECT + -> BB_DOCUMENT_VIEW -> BB_IMPORT_ATTEMPT -> BB_EXTRACTION_RUN -> BB_OBSERVATION @@ -414,16 +426,54 @@ BB_DOCUMENT( document_id, file_hash, original_file_name, - original_path, canonical_file_name, - canonical_path, + source_uri, document_type, - file_type, jurisdiction_code, - period_start, - period_end, tax_year, + document_status, + created_at, + updated_at +) + +BB_STORAGE_ROOT( + storage_root_id, + storage_root_code, + root_kind, + base_path_key, + relative_root, + permissions_mode, + active, + created_at, + updated_at +) + +BB_DOCUMENT_OBJECT( + document_object_id, + document_id, + storage_root_id, + object_key, + object_role, + content_hash, + byte_size, + media_type, + original_file_name, + created_at, + updated_at +) + +BB_DOCUMENT_VIEW( + document_view_id, + document_id, + document_object_id, + storage_root_id, + view_name, + view_key, + materialization_kind, + expected_hash, + byte_size, status, + last_materialized_at, created_at, updated_at ) @@ -431,7 +481,7 @@ BB_DOCUMENT( BB_IMPORT_ATTEMPT( import_attempt_id, document_id, - source_path, + document_object_id, import_status, started_at, finished_at, @@ -750,6 +800,12 @@ Create indexes alongside the v2 schema rather than as an afterthought: BB_DOCUMENT(file_hash) BB_DOCUMENT(document_type, tax_year) BB_DOCUMENT(jurisdiction_code, tax_year) +BB_STORAGE_ROOT(root_kind, active) +BB_DOCUMENT_OBJECT(document_id, object_role) +BB_DOCUMENT_OBJECT(content_hash) +BB_DOCUMENT_VIEW(document_id) +BB_DOCUMENT_VIEW(document_object_id) +BB_DOCUMENT_VIEW(status) BB_IMPORT_ATTEMPT(document_id, import_status) BB_ENTITY(entity_type, status) BB_ENTITY_ATTRIBUTE(entity_id, entity_attribute_type_id) @@ -792,7 +848,9 @@ Responsibilities: - hash files; - identify duplicates; - create document records; -- plan/copy canonical storage; +- plan/copy canonical storage objects; +- materialize human-readable document views; +- reconcile missing or modified generated views against canonical objects; - list/show documents; - record import attempts. - move completed import inputs into managed storage for both successful and diff --git a/docs/financial_intelligence_open_questions.md b/docs/financial_intelligence_open_questions.md index b8ade4b..94e6d56 100644 --- a/docs/financial_intelligence_open_questions.md +++ b/docs/financial_intelligence_open_questions.md @@ -50,28 +50,33 @@ Current bias: needed for tests, provenance, or inspect commands. - Keep debug logs and normal CLI output free of raw document content. -## Managed Document Storage Layout +## Managed Document View Reconciliation -BankBuddy already moves completed imports into managed storage. The v2 model -needs a generic document layout that works for banking, tax, investment, -property, insurance, failed imports, duplicates, and review-needed documents. +BankBuddy v2 separates authoritative canonical document objects from generated +human-readable views. This keeps the database and canonical store authoritative +while preserving the transparent browsable filesystem experience that made v1 +easy to understand. Open questions: -- What is the exact directory layout for successful documents? -- Should failed documents live under a shared failure area or under a - domain-specific path? -- Should exact duplicates continue to be preserved under a duplicates - directory, or eventually be deleted after recording the duplicate attempt? -- How should review-needed documents be named and grouped? +- How should BankBuddy detect that a user edited, moved, or deleted generated + view copies? +- Should view reconciliation run automatically during status/import, or only + through an explicit inspect/repair command? +- What should the repair UX look like when a view is missing but the canonical + object is intact? +- Should exact duplicates continue to be preserved under the v2 duplicates root, + or eventually be deleted after recording the duplicate attempt? Current bias: -- Preserve source documents after every completed import attempt. +- Keep `financial/canonical` as the source of truth. +- Treat `financial/views` as generated copies that can be rebuilt. +- Preserve source documents after every completed import attempt by recording a + canonical object or managed failed/duplicate object. - Keep duplicate preservation for now because it is useful while the product is still young. -- Store canonical paths in the database, but keep directory-shape assumptions - in path/layout services rather than spreading them through domain logic. +- Use explicit inspect/repair commands before adding automatic repair behavior. ## Type Dictionary Seed Values diff --git a/src/bankbuddy/financial/dao.py b/src/bankbuddy/financial/dao.py index c3a762f..ce31a20 100644 --- a/src/bankbuddy/financial/dao.py +++ b/src/bankbuddy/financial/dao.py @@ -118,19 +118,17 @@ def create_document(self, record: DocumentCreate) -> DocumentRecord: file_hash, original_file_name, canonical_file_name, - storage_path, source_uri, document_type, jurisdiction_code, tax_year, document_status - ) values (?, ?, ?, ?, ?, ?, ?, ?, ?) + ) values (?, ?, ?, ?, ?, ?, ?, ?) """, ( record.file_hash, record.original_file_name, record.canonical_file_name, - record.storage_path, record.source_uri, record.document_type, record.jurisdiction_code, @@ -150,7 +148,6 @@ def find_document_by_hash(self, file_hash: str) -> DocumentRecord | None: file_hash, original_file_name, canonical_file_name, - storage_path, source_uri, document_type, jurisdiction_code, @@ -327,7 +324,6 @@ def _document_from_row(row: sqlite3.Row) -> DocumentRecord: file_hash=str(row["file_hash"]), original_file_name=str(row["original_file_name"]), canonical_file_name=row["canonical_file_name"], - storage_path=row["storage_path"], source_uri=row["source_uri"], document_type=row["document_type"], jurisdiction_code=row["jurisdiction_code"], diff --git a/src/bankbuddy/financial/records.py b/src/bankbuddy/financial/records.py index 9b330fa..c239f9a 100644 --- a/src/bankbuddy/financial/records.py +++ b/src/bankbuddy/financial/records.py @@ -57,7 +57,6 @@ class DocumentCreate: file_hash: str original_file_name: str canonical_file_name: str | None = None - storage_path: str | None = None source_uri: str | None = None document_type: str | None = None jurisdiction_code: str | None = None @@ -72,6 +71,65 @@ class DocumentRecord(DocumentCreate): document_id: int = 0 +@dataclass(frozen=True) +class StorageRootRecord: + """A configured v2 document storage root.""" + + storage_root_id: int + storage_root_code: str + root_kind: str + base_path_key: str + relative_root: str + permissions_mode: str + active: bool + + +@dataclass(frozen=True) +class DocumentObjectCreate: + """Input data for a stored document object.""" + + document_id: int + storage_root_code: str + object_key: str + object_role: str + content_hash: str + byte_size: int | None = None + media_type: str | None = None + original_file_name: str | None = None + + +@dataclass(frozen=True) +class DocumentObjectRecord(DocumentObjectCreate): + """Stored document object metadata.""" + + document_object_id: int = 0 + storage_root_id: int = 0 + + +@dataclass(frozen=True) +class DocumentViewCreate: + """Input data for a generated human-readable document view.""" + + document_id: int + document_object_id: int + storage_root_code: str + view_name: str + view_key: str + materialization_kind: str = "copy" + expected_hash: str | None = None + byte_size: int | None = None + status: str = "current" + last_materialized_at: str | None = None + + +@dataclass(frozen=True) +class DocumentViewRecord(DocumentViewCreate): + """Stored generated document view metadata.""" + + document_view_id: int = 0 + storage_root_id: int = 0 + + @dataclass(frozen=True) class EntityCreate: """Input data for creating a v2 entity record.""" diff --git a/src/bankbuddy/financial/storage.py b/src/bankbuddy/financial/storage.py new file mode 100644 index 0000000..435f141 --- /dev/null +++ b/src/bankbuddy/financial/storage.py @@ -0,0 +1,240 @@ +"""Storage DAO and path helpers for financial intelligence documents.""" + +from __future__ import annotations + +from pathlib import Path, PurePosixPath +import sqlite3 + +from bankbuddy.financial.records import ( + DocumentObjectCreate, + DocumentObjectRecord, + DocumentViewCreate, + DocumentViewRecord, + StorageRootRecord, +) +from bankbuddy.paths import AppPaths + + +class FinancialStoragePathError(ValueError): + """Raised when a storage key would escape managed storage.""" + + +class FinancialStorageRootNotFoundError(ValueError): + """Raised when a configured storage root code does not exist.""" + + +class FinancialStorageDAO: + """Persistence boundary for v2 document storage tables.""" + + def __init__(self, conn: sqlite3.Connection) -> None: + self._conn = conn + + def list_storage_roots(self) -> list[StorageRootRecord]: + """Return active and inactive storage roots ordered by code.""" + + rows = self._conn.execute( + """ + select + storage_root_id, + storage_root_code, + root_kind, + base_path_key, + relative_root, + permissions_mode, + active + from BB_STORAGE_ROOT + order by storage_root_code + """ + ).fetchall() + return [_storage_root_from_row(row) for row in rows] + + def get_storage_root(self, storage_root_code: str) -> StorageRootRecord: + """Return one configured storage root by code.""" + + row = self._conn.execute( + """ + select + storage_root_id, + storage_root_code, + root_kind, + base_path_key, + relative_root, + permissions_mode, + active + from BB_STORAGE_ROOT + where storage_root_code = ? + """, + (storage_root_code,), + ).fetchone() + if row is None: + raise FinancialStorageRootNotFoundError( + f"Unknown financial storage root: {storage_root_code}" + ) + return _storage_root_from_row(row) + + def create_document_object( + self, + record: DocumentObjectCreate, + ) -> DocumentObjectRecord: + """Create metadata for a canonical or managed document object.""" + + object_key = validate_storage_key(record.object_key) + root = self.get_storage_root(record.storage_root_code) + cursor = self._conn.execute( + """ + insert into BB_DOCUMENT_OBJECT ( + document_id, + storage_root_id, + object_key, + object_role, + content_hash, + byte_size, + media_type, + original_file_name + ) values (?, ?, ?, ?, ?, ?, ?, ?) + """, + ( + record.document_id, + root.storage_root_id, + object_key, + record.object_role, + record.content_hash, + record.byte_size, + record.media_type, + record.original_file_name, + ), + ) + return DocumentObjectRecord( + document_object_id=int(cursor.lastrowid), + storage_root_id=root.storage_root_id, + **{**record.__dict__, "object_key": object_key}, + ) + + def create_document_view( + self, + record: DocumentViewCreate, + ) -> DocumentViewRecord: + """Create metadata for a generated human-readable document view.""" + + view_key = validate_storage_key(record.view_key) + root = self.get_storage_root(record.storage_root_code) + cursor = self._conn.execute( + """ + insert into BB_DOCUMENT_VIEW ( + document_id, + document_object_id, + storage_root_id, + view_name, + view_key, + materialization_kind, + expected_hash, + byte_size, + status, + last_materialized_at + ) values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + """, + ( + record.document_id, + record.document_object_id, + root.storage_root_id, + record.view_name, + view_key, + record.materialization_kind, + record.expected_hash, + record.byte_size, + record.status, + record.last_materialized_at, + ), + ) + return DocumentViewRecord( + document_view_id=int(cursor.lastrowid), + storage_root_id=root.storage_root_id, + **{**record.__dict__, "view_key": view_key}, + ) + + +def ensure_financial_storage_dirs(paths: AppPaths) -> None: + """Create the v2 financial storage directories.""" + + paths.financial_inbox.mkdir(parents=True, exist_ok=True) + paths.financial_canonical.mkdir(parents=True, exist_ok=True) + paths.financial_failed.mkdir(parents=True, exist_ok=True) + paths.financial_duplicates.mkdir(parents=True, exist_ok=True) + paths.financial_review.mkdir(parents=True, exist_ok=True) + paths.financial_views.mkdir(parents=True, exist_ok=True) + paths.financial_exports.mkdir(parents=True, exist_ok=True) + + +def object_key_for_hash(file_hash: str, suffix: str = "") -> str: + """Return a deterministic canonical object key for a SHA-256 hash.""" + + normalized_hash = file_hash.strip().lower() + if len(normalized_hash) < 4: + raise FinancialStoragePathError("Document hash must contain at least 4 chars.") + normalized_suffix = suffix.strip().lower() + if normalized_suffix and not normalized_suffix.startswith("."): + normalized_suffix = f".{normalized_suffix}" + return ( + f"sha256/{normalized_hash[:2]}/{normalized_hash[2:4]}/" + f"{normalized_hash}{normalized_suffix}" + ) + + +def validate_storage_key(storage_key: str) -> str: + """Return a normalized relative storage key or raise for unsafe input.""" + + normalized_key = storage_key.strip() + if not normalized_key: + raise FinancialStoragePathError("Storage key cannot be empty.") + if "\\" in normalized_key: + raise FinancialStoragePathError("Storage key must use POSIX separators.") + + pure_path = PurePosixPath(normalized_key) + if pure_path.is_absolute(): + raise FinancialStoragePathError("Storage key must be relative.") + + parts = pure_path.parts + if any(part in ("", ".", "..") for part in parts): + raise FinancialStoragePathError( + "Storage key cannot contain empty, current, or parent path segments." + ) + return pure_path.as_posix() + + +def resolve_storage_path( + paths: AppPaths, + storage_root: StorageRootRecord, + storage_key: str, +) -> Path: + """Resolve a storage root and relative key to a local filesystem path.""" + + if storage_root.base_path_key != "app_root": + raise FinancialStoragePathError( + f"Unsupported storage base path: {storage_root.base_path_key}" + ) + + relative_root = validate_storage_key(storage_root.relative_root) + relative_key = validate_storage_key(storage_key) + root_path = paths.root.joinpath(*PurePosixPath(relative_root).parts) + return root_path.joinpath(*PurePosixPath(relative_key).parts) + + +def protect_managed_path(path: Path) -> None: + """Make a managed file or directory read-only as an accidental-delete guard.""" + + if path.is_dir(): + path.chmod(0o555) + return + path.chmod(0o444) + + +def _storage_root_from_row(row: sqlite3.Row) -> StorageRootRecord: + return StorageRootRecord( + storage_root_id=int(row["storage_root_id"]), + storage_root_code=str(row["storage_root_code"]), + root_kind=str(row["root_kind"]), + base_path_key=str(row["base_path_key"]), + relative_root=str(row["relative_root"]), + permissions_mode=str(row["permissions_mode"]), + active=bool(row["active"]), + ) diff --git a/src/bankbuddy/migrations/0011_financial_document_storage.sql b/src/bankbuddy/migrations/0011_financial_document_storage.sql new file mode 100644 index 0000000..acc37a9 --- /dev/null +++ b/src/bankbuddy/migrations/0011_financial_document_storage.sql @@ -0,0 +1,175 @@ +alter table BB_DOCUMENT drop column storage_path; + +alter table BB_IMPORT_ATTEMPT drop column source_path; + +create table BB_STORAGE_ROOT ( + storage_root_id integer primary key, + storage_root_code text not null unique, + root_kind text not null check ( + root_kind in ( + 'canonical', + 'view', + 'inbox', + 'failed', + 'duplicates', + 'review', + 'exports' + ) + ), + base_path_key text not null default 'app_root' check ( + base_path_key in ('app_root') + ), + relative_root text not null unique check ( + length(relative_root) > 0 + and substr(relative_root, 1, 1) <> '/' + and instr(relative_root, '..') = 0 + ), + permissions_mode text not null default 'managed-readonly' check ( + permissions_mode in ('managed-readonly', 'managed-writable') + ), + active integer not null default 1 check (active in (0, 1)), + created_at text not null default current_timestamp, + updated_at text not null default current_timestamp +); + +insert into BB_STORAGE_ROOT ( + storage_root_code, + root_kind, + base_path_key, + relative_root, + permissions_mode, + active +) values + ( + 'FINANCIAL_CANONICAL', + 'canonical', + 'app_root', + 'financial/canonical', + 'managed-readonly', + 1 + ), + ( + 'FINANCIAL_VIEWS', + 'view', + 'app_root', + 'financial/views', + 'managed-readonly', + 1 + ), + ( + 'FINANCIAL_INBOX', + 'inbox', + 'app_root', + 'financial/inbox', + 'managed-writable', + 1 + ), + ( + 'FINANCIAL_FAILED', + 'failed', + 'app_root', + 'financial/failed', + 'managed-readonly', + 1 + ), + ( + 'FINANCIAL_DUPLICATES', + 'duplicates', + 'app_root', + 'financial/duplicates', + 'managed-readonly', + 1 + ), + ( + 'FINANCIAL_REVIEW', + 'review', + 'app_root', + 'financial/review', + 'managed-writable', + 1 + ), + ( + 'FINANCIAL_EXPORTS', + 'exports', + 'app_root', + 'financial/exports', + 'managed-writable', + 1 + ); + +create table BB_DOCUMENT_OBJECT ( + document_object_id integer primary key, + document_id integer not null references BB_DOCUMENT(document_id), + storage_root_id integer not null references BB_STORAGE_ROOT(storage_root_id), + object_key text not null check ( + length(object_key) > 0 + and substr(object_key, 1, 1) <> '/' + and instr(object_key, '..') = 0 + ), + object_role text not null check ( + object_role in ( + 'canonical', + 'failed_original', + 'duplicate_original', + 'review_original', + 'extracted_text' + ) + ), + content_hash text not null, + byte_size integer check (byte_size is null or byte_size >= 0), + media_type text, + original_file_name text, + created_at text not null default current_timestamp, + updated_at text not null default current_timestamp, + unique (storage_root_id, object_key) +); + +create table BB_DOCUMENT_VIEW ( + document_view_id integer primary key, + document_id integer not null references BB_DOCUMENT(document_id), + document_object_id integer not null + references BB_DOCUMENT_OBJECT(document_object_id), + storage_root_id integer not null references BB_STORAGE_ROOT(storage_root_id), + view_name text not null, + view_key text not null check ( + length(view_key) > 0 + and substr(view_key, 1, 1) <> '/' + and instr(view_key, '..') = 0 + ), + materialization_kind text not null default 'copy' check ( + materialization_kind in ('copy') + ), + expected_hash text, + byte_size integer check (byte_size is null or byte_size >= 0), + status text not null default 'current' check ( + status in ('current', 'missing', 'modified', 'stale') + ), + last_materialized_at text, + created_at text not null default current_timestamp, + updated_at text not null default current_timestamp, + unique (storage_root_id, view_key) +); + +alter table BB_IMPORT_ATTEMPT +add column document_object_id integer references BB_DOCUMENT_OBJECT(document_object_id); + +create index idx_BB_STORAGE_ROOT_kind_active +on BB_STORAGE_ROOT(root_kind, active); + +create index idx_BB_DOCUMENT_OBJECT_document_role +on BB_DOCUMENT_OBJECT(document_id, object_role); + +create index idx_BB_DOCUMENT_OBJECT_hash +on BB_DOCUMENT_OBJECT(content_hash); + +create index idx_BB_DOCUMENT_VIEW_document +on BB_DOCUMENT_VIEW(document_id); + +create index idx_BB_DOCUMENT_VIEW_object +on BB_DOCUMENT_VIEW(document_object_id); + +create index idx_BB_DOCUMENT_VIEW_status +on BB_DOCUMENT_VIEW(status); + +create index idx_BB_IMPORT_ATTEMPT_document_object +on BB_IMPORT_ATTEMPT(document_object_id); diff --git a/src/bankbuddy/paths.py b/src/bankbuddy/paths.py index f4874af..5127068 100644 --- a/src/bankbuddy/paths.py +++ b/src/bankbuddy/paths.py @@ -37,6 +37,13 @@ class AppPaths: tax_processed: Path tax_duplicates: Path tax_exports: Path + financial_inbox: Path + financial_canonical: Path + financial_failed: Path + financial_duplicates: Path + financial_review: Path + financial_views: Path + financial_exports: Path database: Path @@ -96,6 +103,13 @@ def resolve_app_paths( tax_processed=resolved_root / "tax" / "processed", tax_duplicates=resolved_root / "tax" / "duplicates", tax_exports=resolved_root / "tax" / "exports", + financial_inbox=resolved_root / "financial" / "inbox", + financial_canonical=resolved_root / "financial" / "canonical", + financial_failed=resolved_root / "financial" / "failed", + financial_duplicates=resolved_root / "financial" / "duplicates", + financial_review=resolved_root / "financial" / "review", + financial_views=resolved_root / "financial" / "views", + financial_exports=resolved_root / "financial" / "exports", database=resolved_root / "database" / DATABASE_NAME, ) @@ -111,6 +125,13 @@ def resolve_app_paths( tax_processed=resolved_root / "tax" / "processed", tax_duplicates=resolved_root / "tax" / "duplicates", tax_exports=resolved_root / "tax" / "exports", + financial_inbox=resolved_root / "financial" / "inbox", + financial_canonical=resolved_root / "financial" / "canonical", + financial_failed=resolved_root / "financial" / "failed", + financial_duplicates=resolved_root / "financial" / "duplicates", + financial_review=resolved_root / "financial" / "review", + financial_views=resolved_root / "financial" / "views", + financial_exports=resolved_root / "financial" / "exports", database=resolved_root / DATABASE_NAME, ) @@ -128,6 +149,13 @@ def ensure_app_dirs(paths: AppPaths) -> None: paths.tax_processed.mkdir(parents=True, exist_ok=True) paths.tax_duplicates.mkdir(parents=True, exist_ok=True) paths.tax_exports.mkdir(parents=True, exist_ok=True) + paths.financial_inbox.mkdir(parents=True, exist_ok=True) + paths.financial_canonical.mkdir(parents=True, exist_ok=True) + paths.financial_failed.mkdir(parents=True, exist_ok=True) + paths.financial_duplicates.mkdir(parents=True, exist_ok=True) + paths.financial_review.mkdir(parents=True, exist_ok=True) + paths.financial_views.mkdir(parents=True, exist_ok=True) + paths.financial_exports.mkdir(parents=True, exist_ok=True) def _resolve_layout(root: Path, layout: AppLayoutMode) -> AppLayout: diff --git a/tests/test_database.py b/tests/test_database.py index e3b602d..cacb433 100644 --- a/tests/test_database.py +++ b/tests/test_database.py @@ -24,6 +24,7 @@ "0008_account_statement_refs", "0009_tax_documents", "0010_financial_intelligence_foundation", + "0011_financial_document_storage", ] @@ -35,6 +36,9 @@ def test_initialize_database_creates_directories_and_schema_table(tmp_path) -> N assert paths.inbox.is_dir() assert paths.processed.is_dir() assert paths.exports.is_dir() + assert paths.financial_canonical.is_dir() + assert paths.financial_views.is_dir() + assert paths.financial_inbox.is_dir() assert paths.database.is_file() with sqlite3.connect(paths.database) as conn: @@ -82,6 +86,9 @@ def test_initialize_database_applies_core_schema_and_seed_categories(tmp_path) - "budgets", "account_statement_refs", "tax_documents", + "BB_STORAGE_ROOT", + "BB_DOCUMENT_OBJECT", + "BB_DOCUMENT_VIEW", }.issubset(table_names) assert migration_versions == EXPECTED_MIGRATION_VERSIONS assert categories == { @@ -326,7 +333,7 @@ def test_tax_documents_schema_tracks_imported_document_metadata(tmp_path) -> Non "imported_at": "TEXT", }.items() <= columns.items() assert "0009_tax_documents" in migration_versions - assert migration_versions[-1] == "0010_financial_intelligence_foundation" + assert migration_versions[-1] == "0011_financial_document_storage" assert dict(row) == { "document_type": "1099-INT", "jurisdiction": "US", diff --git a/tests/test_financial_foundation.py b/tests/test_financial_foundation.py index 15ddb7f..d77a595 100644 --- a/tests/test_financial_foundation.py +++ b/tests/test_financial_foundation.py @@ -102,10 +102,6 @@ def test_financial_dao_round_trips_records_without_inline_sql_callers( file_hash="a" * 64, original_file_name="statement.pdf", canonical_file_name="bank_1234_2026-01-01_2026-01-31.pdf", - storage_path=( - "bank/processed/bank/2026/01/" - "bank_1234_2026-01-01_2026-01-31.pdf" - ), document_type="bank_statement", jurisdiction_code="US", tax_year=2026, diff --git a/tests/test_financial_storage.py b/tests/test_financial_storage.py new file mode 100644 index 0000000..23b0470 --- /dev/null +++ b/tests/test_financial_storage.py @@ -0,0 +1,308 @@ +import stat + +import pytest + +from bankbuddy.database import connect_database +from bankbuddy.database import initialize_database +from bankbuddy.financial.dao import FinancialIntelligenceDAO +from bankbuddy.financial.records import DocumentCreate +from bankbuddy.paths import resolve_app_paths + + +def test_financial_storage_schema_seeds_roots(tmp_path) -> None: + paths = resolve_app_paths(tmp_path / "home") + + initialize_database(paths) + + with connect_database(paths) as conn: + rows = conn.execute( + """ + select storage_root_code, root_kind, base_path_key, relative_root + from BB_STORAGE_ROOT + order by storage_root_code + """ + ).fetchall() + + assert [row["storage_root_code"] for row in rows] == [ + "FINANCIAL_CANONICAL", + "FINANCIAL_DUPLICATES", + "FINANCIAL_EXPORTS", + "FINANCIAL_FAILED", + "FINANCIAL_INBOX", + "FINANCIAL_REVIEW", + "FINANCIAL_VIEWS", + ] + assert { + row["storage_root_code"]: ( + row["root_kind"], + row["base_path_key"], + row["relative_root"], + ) + for row in rows + } == { + "FINANCIAL_CANONICAL": ("canonical", "app_root", "financial/canonical"), + "FINANCIAL_DUPLICATES": ("duplicates", "app_root", "financial/duplicates"), + "FINANCIAL_EXPORTS": ("exports", "app_root", "financial/exports"), + "FINANCIAL_FAILED": ("failed", "app_root", "financial/failed"), + "FINANCIAL_INBOX": ("inbox", "app_root", "financial/inbox"), + "FINANCIAL_REVIEW": ("review", "app_root", "financial/review"), + "FINANCIAL_VIEWS": ("view", "app_root", "financial/views"), + } + + +def test_financial_storage_paths_are_separate_from_bank_and_tax_dirs(tmp_path) -> None: + ( + _, + _, + _, + _, + ensure_financial_storage_dirs, + _, + _, + ) = _storage_api() + paths = resolve_app_paths(tmp_path / "home") + + ensure_financial_storage_dirs(paths) + + assert paths.financial_canonical == paths.root / "financial" / "canonical" + assert paths.financial_views == paths.root / "financial" / "views" + assert paths.financial_inbox == paths.root / "financial" / "inbox" + assert paths.financial_failed == paths.root / "financial" / "failed" + assert paths.financial_duplicates == paths.root / "financial" / "duplicates" + assert paths.financial_review == paths.root / "financial" / "review" + assert paths.financial_exports == paths.root / "financial" / "exports" + assert paths.inbox == paths.root / "bank" / "inbox" + assert paths.tax_inbox == paths.root / "tax" / "inbox" + assert paths.financial_canonical.is_dir() + assert paths.financial_views.is_dir() + + +def test_storage_dao_records_objects_and_views_without_absolute_paths(tmp_path) -> None: + ( + DocumentObjectCreate, + DocumentViewCreate, + FinancialStorageDAO, + _, + _, + object_key_for_hash, + resolve_storage_path, + ) = _storage_api() + paths = resolve_app_paths(tmp_path / "home") + file_hash = "0123456789abcdef" * 4 + + initialize_database(paths) + + with connect_database(paths) as conn: + document = FinancialIntelligenceDAO(conn).create_document( + DocumentCreate( + file_hash=file_hash, + original_file_name="statement.pdf", + document_type="bank_statement", + ) + ) + storage = FinancialStorageDAO(conn) + canonical_root = storage.get_storage_root("FINANCIAL_CANONICAL") + views_root = storage.get_storage_root("FINANCIAL_VIEWS") + object_record = storage.create_document_object( + DocumentObjectCreate( + document_id=document.document_id, + storage_root_code="FINANCIAL_CANONICAL", + object_key=object_key_for_hash(file_hash, ".pdf"), + object_role="canonical", + content_hash=file_hash, + byte_size=1234, + media_type="application/pdf", + original_file_name="statement.pdf", + ) + ) + view_record = storage.create_document_view( + DocumentViewCreate( + document_id=document.document_id, + document_object_id=object_record.document_object_id, + storage_root_code="FINANCIAL_VIEWS", + view_name="bank/by-account", + view_key=( + "bank/bank-of-america/2026/05/" + "bank-of-america_1145_2026-04-22_2026-05-19.pdf" + ), + expected_hash=file_hash, + byte_size=1234, + ) + ) + + assert object_record.storage_root_id == canonical_root.storage_root_id + assert object_record.object_key == f"sha256/01/23/{file_hash}.pdf" + assert view_record.storage_root_id == views_root.storage_root_id + assert view_record.materialization_kind == "copy" + assert resolve_storage_path(paths, canonical_root, object_record.object_key) == ( + paths.financial_canonical / "sha256" / "01" / "23" / f"{file_hash}.pdf" + ) + assert resolve_storage_path(paths, views_root, view_record.view_key) == ( + paths.financial_views + / "bank" + / "bank-of-america" + / "2026" + / "05" + / "bank-of-america_1145_2026-04-22_2026-05-19.pdf" + ) + assert str(paths.root) not in object_record.object_key + assert str(paths.root) not in view_record.view_key + + +def test_v2_document_tables_do_not_keep_filesystem_paths(tmp_path) -> None: + paths = resolve_app_paths(tmp_path / "home") + + initialize_database(paths) + + with connect_database(paths) as conn: + document_columns = { + row["name"] + for row in conn.execute("pragma table_info(BB_DOCUMENT)").fetchall() + } + import_attempt_columns = { + row["name"] + for row in conn.execute("pragma table_info(BB_IMPORT_ATTEMPT)").fetchall() + } + document = FinancialIntelligenceDAO(conn).create_document( + DocumentCreate( + file_hash="fedcba9876543210" * 4, + original_file_name="statement.pdf", + source_uri="external://manual-upload/statement.pdf", + ) + ) + + assert "storage_path" not in document_columns + assert "source_path" not in import_attempt_columns + assert "document_object_id" in import_attempt_columns + assert not hasattr(document, "storage_path") + assert document.source_uri == "external://manual-upload/statement.pdf" + + +def test_storage_dao_normalizes_keys_before_writing(tmp_path) -> None: + ( + DocumentObjectCreate, + _, + FinancialStorageDAO, + _, + _, + _, + _, + ) = _storage_api() + paths = resolve_app_paths(tmp_path / "home") + + initialize_database(paths) + + with connect_database(paths) as conn: + document = FinancialIntelligenceDAO(conn).create_document( + DocumentCreate( + file_hash="1234567890abcdef" * 4, + original_file_name="statement.pdf", + ) + ) + storage = FinancialStorageDAO(conn) + object_record = storage.create_document_object( + DocumentObjectCreate( + document_id=document.document_id, + storage_root_code="FINANCIAL_CANONICAL", + object_key=" sha256/12/34/object.pdf ", + object_role="canonical", + content_hash="1234567890abcdef" * 4, + ) + ) + row = conn.execute( + """ + select object_key + from BB_DOCUMENT_OBJECT + where document_object_id = ? + """, + (object_record.document_object_id,), + ).fetchone() + + assert object_record.object_key == "sha256/12/34/object.pdf" + assert row["object_key"] == "sha256/12/34/object.pdf" + + +def test_storage_keys_reject_absolute_and_parent_traversal_paths(tmp_path) -> None: + ( + DocumentObjectCreate, + _, + FinancialStorageDAO, + FinancialStoragePathError, + _, + _, + _, + ) = _storage_api() + from bankbuddy.financial.storage import validate_storage_key + + paths = resolve_app_paths(tmp_path / "home") + + initialize_database(paths) + + with connect_database(paths) as conn: + document = FinancialIntelligenceDAO(conn).create_document( + DocumentCreate( + file_hash="abcdef" * 10 + "abcd", + original_file_name="statement.pdf", + ) + ) + storage = FinancialStorageDAO(conn) + + with pytest.raises(FinancialStoragePathError): + storage.create_document_object( + DocumentObjectCreate( + document_id=document.document_id, + storage_root_code="FINANCIAL_CANONICAL", + object_key="/absolute/path.pdf", + object_role="canonical", + content_hash="abcdef" * 10 + "abcd", + ) + ) + + with pytest.raises(FinancialStoragePathError): + validate_storage_key("nested/../escape.pdf") + + with pytest.raises(FinancialStoragePathError): + validate_storage_key("../escape.pdf") + + +def test_protect_managed_path_sets_read_only_modes(tmp_path) -> None: + from bankbuddy.financial.storage import protect_managed_path + + managed_file = tmp_path / "financial" / "canonical" / "object.pdf" + managed_file.parent.mkdir(parents=True) + managed_file.write_bytes(b"%PDF-1.4 placeholder") + managed_dir = tmp_path / "financial" / "views" / "bank" + managed_dir.mkdir(parents=True) + + try: + protect_managed_path(managed_file) + protect_managed_path(managed_dir) + + assert stat.S_IMODE(managed_file.stat().st_mode) == 0o444 + assert stat.S_IMODE(managed_dir.stat().st_mode) == 0o555 + finally: + managed_file.chmod(0o644) + managed_dir.chmod(0o755) + + +def _storage_api(): + try: + from bankbuddy.financial.records import DocumentObjectCreate + from bankbuddy.financial.records import DocumentViewCreate + from bankbuddy.financial.storage import FinancialStorageDAO + from bankbuddy.financial.storage import FinancialStoragePathError + from bankbuddy.financial.storage import ensure_financial_storage_dirs + from bankbuddy.financial.storage import object_key_for_hash + from bankbuddy.financial.storage import resolve_storage_path + except ImportError as exc: + pytest.fail(f"Financial storage API is not available: {exc}") + + return ( + DocumentObjectCreate, + DocumentViewCreate, + FinancialStorageDAO, + FinancialStoragePathError, + ensure_financial_storage_dirs, + object_key_for_hash, + resolve_storage_path, + )