[Arch] Manual shipment creation re-implements ingest pipeline's orchestration shape #55

New issue

Closed

opened 2026-07-01 07:26:44 +02:00 by bullitt · 1 comment

bullitt commented

2026-07-01 07:26:44 +02:00

Owner

Severity: Medium | Category: Domain

main.py:903-932 (api_create_shipment) performs find-existing → db.upsert_shipment → db.add_event → finalize_shipment, mirroring ingest.py:346-374's same shape, hand-copied instead of shared.

Recommendation: Have main.api_create_shipment call the same underlying function ingest.py uses internally, with a signature that accommodates "pre-extracted fields" vs "raw email" callers.

Full detail: docs/architecture-review.md § C, Finding "Manual shipment creation re-implements..."

**Severity:** Medium | **Category:** Domain `main.py:903-932` (`api_create_shipment`) performs find-existing → `db.upsert_shipment` → `db.add_event` → `finalize_shipment`, mirroring `ingest.py:346-374`'s same shape, hand-copied instead of shared. **Recommendation:** Have `main.api_create_shipment` call the same underlying function `ingest.py` uses internally, with a signature that accommodates "pre-extracted fields" vs "raw email" callers. Full detail: `docs/architecture-review.md` § C, Finding "Manual shipment creation re-implements..."

bullitt added the

arch

label

2026-07-01 07:27:45 +02:00

bullitt commented

2026-07-01 08:43:38 +02:00

Author

Owner

Fixed on branch architecture-rework, commit f1d8f19: added ingest.upsert_shipment_with_event(), shared by _process_email_inner and main.api_create_shipment. As a bonus, the manual-creation path is now also atomic (it wasn't before — a gap not covered by the earlier db.transaction() fix since that only touched the email path). New regression test: test_manual_shipment_creation_atomic_on_event_failure. Full suite: 259 passed.

Fixed on branch `architecture-rework`, commit f1d8f19: added `ingest.upsert_shipment_with_event()`, shared by `_process_email_inner` and `main.api_create_shipment`. As a bonus, the manual-creation path is now also atomic (it wasn't before — a gap not covered by the earlier `db.transaction()` fix since that only touched the email path). New regression test: `test_manual_shipment_creation_atomic_on_event_failure`. Full suite: 259 passed.

bullitt closed this issue

2026-07-01 08:43:59 +02:00

bullitt referenced this issue from a commit

2026-07-01 09:15:22 +02:00

refactor: share ingest.py's upsert+event orchestration with manual creation (fixes #55)