Repository navigation
Conversation
Reject zero-rate transfer reconstructions as recoverable parse failures, cover manager recovery, and format schema compatibility tests.
Thread known profiles through every investment construction path and reject ambiguous schemas without guessing stale aliases.
Model holding and transaction share columns independently and preserve canonical stored amounts for rounded transfers.
- Introduced tests for entity source coverage to validate manager behavior with unknown descendants and unclassifiable entities. - Added transaction lifecycle tests to ensure correct loading and mutation of transaction data. - Enhanced investment values tests to verify proper handling of profiled investment records. - Created tests for read completeness to check for duplicate and unknown rows during loading. - Implemented relationship completeness tests to validate handling of relationships in various scenarios. - Developed schema cache binding tests to ensure proper handling of schema changes and metadata remapping. - Added transaction compatibility tests to verify correct behavior with different transaction types.
… with MappingProxyType
Assert optional manager lookups before accessing their fields in scoped-load and schema-cache tests. Cast the intentionally mutated report mappings so the runtime immutability checks remain valid under mypy src tests.
marcomc
left a comment
marcomc
left a comment
There was a problem hiding this comment.
Inline notes explain the scope and purpose of the main read-completeness blocks.
| """Load all manager state inside the caller-owned read snapshot.""" | ||
| ents = self.ents | ||
| # Completeness is scoped to this manager's explicitly supported entities. | ||
| records = db_accessor.query_objects(list(ents)) |
There was a problem hiding this comment.
This intentionally measures only entity types this manager can construct. New or unsupported subtypes remain outside this feature's completeness contract, avoiding speculative subtype handling.
| self.add(obj) | ||
| except UnsupportedInvestmentSchemaError: | ||
| raise | ||
| except (AssertionError, KeyError, ValueError, RuntimeError) as exc: |
There was a problem hiding this comment.
A malformed supported row is skipped to preserve the existing best-effort read, but the report records it. Without this accounting, callers could mistake a partial result for a complete one.
| table_present = self._table_exists(table_name) | ||
| if not metadata_present and not table_present: | ||
| return RelationshipStorage.ABSENT | ||
| if not metadata_present or not table_present: |
There was a problem hiding this comment.
This distinguishes truly absent optional storage from a mismatched schema. If metadata, table, or required columns disagree, reporting an empty relationship map as complete would hide missing data.
| ) | ||
| tag_column = f"Z_{tag_ent}TAGS" if tag_ent is not None else None | ||
| candidates: list[tuple[str, set[str]]] = [] | ||
| # Core Data can create several *_TAGS tables; select only the Transaction–Tag link. |
There was a problem hiding this comment.
Core Data can have several *_TAGS join tables, including links for other entity types. Resolving the table from the live Transaction and Tag entity IDs and checking its columns avoids hardcoding IDs or selecting an unrelated tag link.
| with self._raw_read_transaction(): | ||
| if not self._admission_depth: | ||
| # Cached entity IDs and column profiles must still match the live schema. | ||
| self._verify_schema_identity() |
There was a problem hiding this comment.
The accessor caches entity IDs and the investment column profile at open. Rechecking them inside the read snapshot prevents those cached mappings from interpreting rows after the store's schema has changed.
| def complete(self) -> bool: | ||
| return ( | ||
| self.observed | ||
| and self.source_count == self.parsed_count |
There was a problem hiding this comment.
Completeness requires every observed supported row to parse and every relationship report to be complete. This makes skipped rows visible instead of letting a partial in-memory map look authoritative.
Reject malformed record identity and boolean values before publishing a complete read report. Preserve nullable transaction descriptions and classify invalid descriptions as per-row skips.
Use the existing read admission for record lookups so cached aliases cannot be applied after a schema change. Preserve the scale of finite zero holding quantities and keep record identity validation aligned with the prior contract.
This branch has not been deployed
1 participant
Scoped read completeness
Problem
The API used best-effort parsing and exposed limited record load errors, but did not account for source versus parsed rows or transaction relationship storage. A caller could mistake a partial read for a complete one. Separate manager reloads could also represent different SQLite generations.
What changed
Why this approach
The change keeps the existing managers, model constructors, and supported schema-profile mappings. It adds evidence around their existing best-effort parsing instead of replacing the parsing path. Entity metadata identifies transaction tag storage, avoiding a fixed numeric join-table name; ambiguous layouts fail closed. Existing category and refund table names remain limited to their known layouts. This PR does not add writes or consumer-specific snapshot policy.
This draft depends on PR #37. Because its base is upstream
main, the current diff includes those prerequisite compatibility commits. After PR #37 is resolved, the branch can be reconciled for a focused review. The latest v1.1.0 release notes include the earlier schema-version work from PR #40.Validation
get()results needed existence assertions, and intentional runtime mutation checks needed an explicit type cast. The follow-up changes only those tests.mypy src testsand the CI unit-test command pass on Python 3.10 and 3.11: 105 tests on each version.