Phase C PP XML importer: deferred follow-ups from PR #72 review

[williaby]

Follow-ups identified during the /pr-review + /pr-fix of #72 (merged in 1e4fd7c) and deliberately deferred, either because they need a live PostgreSQL to verify, change behavior/semantics in a way that warrants a deliberate decision, or are cross-cutting cleanups. None blocked the merge.

Importer semantics and data integrity

• ImportSummary field split. securities and accounts count entities present in the source (created or matched), while the other fields count rows newly inserted. PR feat(patch): PP XML round-trip (importer, CLI, Alembic, account-transactions) #72 corrected the docstring; a cleaner design splits the ambiguous fields into *_seen vs *_created so callers can distinguish "18 in file, 0 new" from "18 new". (src/security_master/patch/pp_xml_import.py)
• ISIN-less securities are re-created on every import. Dedup is gated on a non-null ISIN; securities without one are never matched (NULL ISINs are distinct in Postgres). Either add a secondary match key (e.g. name + currency) or keep the documented #EDGE and surface a count of unmatched-by-identity securities. (_get_or_create_security)
• export-xml --config-name does not scope the dataset. generate_complete_backup exports all securities/accounts/portfolios; the option currently scopes only the header (version/baseCurrency). Either scope the queries by config or update the option help to state the data set is global. (src/security_master/patch/pp_xml_export.py, src/security_master/cli.py)
• Soft-polymorphic pp_transaction_units has no referential integrity. transaction_id is not a real FK and transaction_type is a free String(20). viewonly=True resolved the mapper error in feat(patch): PP XML round-trip (importer, CLI, Alembic, account-transactions) #72, but a CHECK (transaction_type IN ('ACCOUNT','PORTFOLIO')) (minimum) would enforce the discriminator. (src/security_master/storage/pp_models.py)

Tests (require the CI Postgres job to verify)

• Idempotency for all entities. test_import_is_idempotent_for_securities asserts only the security count. Extend it to assert accounts, portfolios, bookmarks, and account-transactions counts after a double import. (tests/integration/test_pp_xml_roundtrip.py)
• BigInteger price-overflow regression test. Lock the Integer -> BigInteger fix by persisting a price_value > 2_147_483_647 (above the ~$21 INT4 boundary) and reading it back. Nearly every real security exceeds this.

Ergonomics and types

• CLI error surfacing. Wrap importer/exporter domain errors in click.ClickException so a malformed broker file yields a one-line stderr message and exit 1 rather than a raw traceback. (src/security_master/cli.py)
• Stringly-typed identifiers. uuid: str (should be UUID) and transaction_type/cross_entry_type/unit_type as free strings over closed PP sets (use enum/Literal) so invalid values fail at parse time, not persist time. (src/security_master/patch/pp_xml_import.py)

Org integration

• Rebind security/snyk from the williaby org to byronwilliamscpa. The check ERRORs on feat(patch): PP XML round-trip (importer, CLI, Alembic, account-transactions) #72 as a non-required, post-transfer leftover. Triage the flagged dependency and rebind. (See also the sonar-project.properties williaby pin, intentionally held until the SonarCloud project is created under the new org.)

---

Source: review of #72. Generated with Claude Code.