Skip to content

fix(bulk-submit): key entry results by their manifest output file - #466

Open
angela-helios wants to merge 1 commit into
mainfrom
fix/457-bulk-entry-file-key
Open

fix(bulk-submit): key entry results by their manifest output file#466
angela-helios wants to merge 1 commit into
mainfrom
fix/457-bulk-entry-file-key

Conversation

@angela-helios

Copy link
Copy Markdown
Contributor

Closes #457

Line numbers restart in every manifest output file, but bulk_entry_results's primary key had no file discriminator — so in any multi-file manifest, only the first file ingested and every other died wholesale on a UNIQUE violation at its first entry (single-file smoke tests never trip it; the #448 Synthea submission with 20 files did, instantly).

  • The worker names the file it is ingesting (BulkProcessingOptions::file_url, threaded per-file), both backends store it, and both primary keys include it.
  • SQLite: schema v15 with a table rebuild (PKs are immutable there); pre-migration rows keep ''.
  • Postgres: in-place ADD COLUMN IF NOT EXISTS + a re-key guarded on the constraint's current shape, so it is idempotent.
  • The insert is now an upsert on that key: the worker re-fetches a whole file after a transient failure, and the retry must overwrite its own earlier rows instead of colliding with them (found live when my test harness's file server dropped mid-ingest and the retry loop compounded the collisions).

Tests: new multi-file regression (two files, both starting at line 1, both must ingest and both results stored); persistence lib 759 green; rest bulk_submit 13 green. A full-scale e2e (17 Synthea patients, 20 NDJSON files, 11,286 resources through $bulk-submit) is running against this build — result to follow as a comment.

Line numbers restart in every output file, but bulk_entry_results' key
was (tenant, submitter, submission, manifest, line_number) — so in any
multi-file manifest, every file after the first collided on its first
entry and failed wholesale. The worker now names the file it is
ingesting (BulkProcessingOptions::file_url), the stores key results by
it, and both schemas carry it in the primary key: SQLite via a v15
table rebuild (pre-migration rows keep ''), Postgres via an in-place
add-column + re-key guarded on the constraint's current shape.

The insert is also an upsert now: the worker re-fetches a whole file
after a transient failure, and the retry must overwrite its own earlier
rows instead of colliding with them.

Closes #457
@angela-helios

Copy link
Copy Markdown
Contributor Author

Full-scale e2e against this build: the #448 Synthea submission — 20 NDJSON files, 11,286 resources, every file starting at line 1 — now ingests completely. The status manifest reports one output per file (AllergyIntolerance 26, CarePlan 54, Claim 1,073, Condition 491, DiagnosticReport 1,045, Observation 1,377, Procedure 1,906, …) with no error outcomes, and every resource type queries back with the expected counts. Before the fix the same submission died at the first line of the second file.

@angela-helios

Copy link
Copy Markdown
Contributor Author

The terminology_integration::test_in_modifier_fails_open_on_hts_unavailable failure is unrelated to this PR — it is the #449 runner-proxy signature hitting a third surface (the HTS client detours through the egress proxy on some pool runners and hangs instead of failing fast). Root-cause fix in #469; re-running the failed job meanwhile, which should pass on any runner without the proxy env.

@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants