fix(rest): run batch bundle entries with backend-bounded concurrency

[aacruzgon]

Summary

A batch Bundle was processed one entry at a time, so wall clock was the sum of every entry's storage round trips. Against real S3 that is ~300 ms per entry, which puts a bundle over ~90 entries past the 30 s request timeout: TimeoutLayer drops the handler, the client gets an empty-bodied 408, and the entries that already committed stay committed. A 473-entry US Core fixture took 30.1 s and failed; the same data went through the concurrent transaction path in 5.5 s.

Entries now run through buffered(n), bounded by what the backend says it tolerates. Along the way this fixes a second, quieter defect and removes the structure that produced it: BundleProvider::process_batch was implemented five times and called by none of them, and #311's ifMatch fix had landed in that unreachable half — so batch ifMatch has been silently unconditional over HTTP for two releases.

Fixes #501. Also fixes the batch half of #311.

Why the bound comes from the backend

ResourceStorage::bulk_write_concurrency() already existed on every backend with the right values and the right composite delegation, and process_batch's where clause already reached it — so this needs no new trait method and no widened bound:

Backend	Declares	Effective (default cap 16)
SQLite	1 (default)	1
PostgreSQL / MongoDB / Elasticsearch	8	8
S3	32	16
Composite	its primary's	its primary's

HFS_BATCH_MAX_CONCURRENCY (default 16) is a ceiling: it can lower what a backend declared, never raise it, because only the backend knows what its pool absorbs.

SQLite staying at 1 is honest rather than conservative — its storage calls contain no await points and buffered polls in place without spawning, so a higher bound could not interleave them anyway. This is therefore a no-op on SQLite and on the sqlite/sqlite-elasticsearch CI legs; s3-elasticsearch is the leg that changes.

Changes

helios-rest — the entry loop

• .buffered(n), never buffer_unordered: response entries are positional and nothing in a response entry carries its index, so a scramble would be invisible to clients and to every existing test. A debug_assert! pins the invariant.
• The stream is driven over indices rather than entries.iter() deliberately. A closure whose returned future borrows its argument needs a higher-ranked lifetime that inference cannot supply here, and the resulting error is reported against the route registration in routing::fhir_routes, not against this function.
• Per-entry SMART scope enforcement stays exactly where it was — it is the only authorization gate on POST [base], and is a lock-free, I/O-free function over &Principal, so concurrency cannot change a verdict.
• Per-entry audit moves one nesting level inward, into the entry future, so an entry whose write committed before a timeout still produces its event. Emitting after collection would drop the whole bundle's events on precisely the failure mode being fixed.
• A bundle that writes a StructureDefinition drops to sequential: later entries validate against profiles earlier entries register via upsert_stored_profile, the one cross-entry dependency on this path. The scan keys off request.url through the same parse_request_url the side effect keys off, so the two cannot disagree.
• BatchProgress logs how far a batch got if the handler future is dropped, carrying the bundle-id that per-entry audit events share. The timeout discards the response bundle, so this is otherwise the only trace of what landed.

helios-rest — ifMatch on the batch path

process_batch_entry now evaluates the precondition through the same core::bundle_if_match_gate the backends use: a list is satisfied when any member matches, * requires a current representation, a deleted resource has none, and a malformed value fails closed. The gate runs ahead of write-path validation, because every backend evaluates ifMatch first — a stale precondition carrying an invalid body is a 412, not a 422. Entries without a precondition pay nothing, not even the read.

This is a read-then-write check, not an atomic compare-and-swap. The backends reach an atomic re-check through update_with_match, which lives on VersionedStorage — a trait create_routes does not bound S with, so this path cannot call it. The window is the one handlers::update already carries, with one addition worth naming: entries now run concurrently, so two entries carrying ifMatch for the same id can both pass the gate.

helios-persistence — remove the unreachable half

Deleted BundleProvider::process_batch and all five implementations. Kept supports_atomic_transactions, process_transaction, the composite's sync_bundle_results, and bundle_if_match_gate.

Two things this surfaces. MongoDB's implementation was an unconditional UnsupportedCapability — had anything actually delegated to it, every mongodb-* deployment would have 5xx'd on batch Bundles that succeed today. And S3's carried an unbounded join_all over caller-controlled input, which could never have bounded anything from where it sat.

The trait doc now records why batch is not there — per-entry scope enforcement and per-entry audit are invisible to the persistence tier — so the next reader does not "fix" the asymmetry by wiring the two back together.

Behaviour changes visible to a client

1. Large batches succeed. 473 entries on S3: 30.1 s → 408 becomes an expected ≈9 s → 200.
2. Entries are no longer ordered relative to one another at a bound above 1. FHIR permits this ("the server may process the entries in any order"), but two entries writing the same id, or one reading what another writes, now resolve by timing. Documented in the REST README. Invisible on SQLite.
3. A batch PUT/DELETE with a stale, absent-target, malformed, or *-against-nothing ifMatch now returns a per-entry 412 where it previously returned 200 and wrote. The bundle still returns HTTP 200 — a failed precondition is one entry's outcome, not the batch's.
4. Audit emission order becomes completion order. Position is still carried explicitly by the entry-index detail and the shared bundle-id.

Testing

cargo check --workspace --all-targets clean. cargo check -p helios-persistence --all-features --all-targets clean across sqlite, postgres, mongodb, elasticsearch and s3.

• helios-persistence lib — 767 passed, 0 failed
• helios-persistence transactions_suite — 37 passed, 0 failed
• helios-rest full suite — green, including the R4B leg

Both new suites were verified to have teeth rather than trusted because they were green:

• The ordering test was run against buffer_unordered and fails (caught by the debug_assert!).
• The ifMatch suite was run with both gate call sites removed: 8 of 13 fail. The 5 that still pass are the must-succeed cases, which guard the opposite direction.

Clippy warning count on helios-persistence --all-features --all-targets is unchanged at 930 against the pre-change tree, with no dead-code warnings left behind.

I also ran to_batch.jq over every Inferno fixture directly: all ten rewritten bundles have zero duplicate PUT targets, so concurrent execution cannot produce a write conflict, and uscore_bundle_asserted-date.json is the only fixture carrying a StructureDefinition — it correctly drops to sequential, at a cost of 2 entries.

Not reproducible in cargo test. batch_conformance.rs builds its router with create_routes rather than the full app, so no TimeoutLayer is installed, and SQLite's storage calls never yield.

Verified against real S3 — Inferno US Core run 31016157176

Run on fix/489-s3-decline-unsupported-transactions (the stacked branch, which contains these three commits plus the loader that routes S3 to batch), because that is the only configuration where Inferno exercises this code path. 30/30 jobs green, against a baseline on the same branch (run 30973069238, the run issue #501 quotes) where all six s3-elasticsearch jobs died at the load step:

Bundle	Entries	Baseline	This run
uscore_bundle_patient_355	473	HTTP 408 @ 30.09s	HTTP 200 @ 9.42s
uscore_bundle_patient_85	267	HTTP 408 @ 30.06s	HTTP 200 @ 5.48s

The log confirms the path taken: Server does not advertise 'transaction'; rewriting bundles to batch. All 15 fixtures loaded.

Correctness, not just speed. US Core v8.0.0, latest result per test_id:

Backend	Loaded via	Pass	Skip	Fail	Error
s3-elasticsearch	batch, 16-way concurrent	329	204	9	2
sqlite-elasticsearch	transaction	329	204	9	2
sqlite	transaction	326	207	9	2
mongodb	transaction	365	168	9	2
postgres	transaction	25	511	8	0

S3's pass set is identical to sqlite-elasticsearch's at the test-id level, not merely equal in count — 0 tests pass only on S3, 0 only on sqlite-ES, and zero S3-only failures. The same fixtures loaded through the concurrent batch loop produce exactly the same conformance outcome as loading them through a transaction: no lost entries, no ordering corruption, no unresolved references.

All 10 named failures are inside the workflow's 17-entry omit list, so the gate's "0 failures" is genuine rather than an artifact of #491.

Two observations from the run that are not caused by this PR, recorded because the run makes them visible: postgres reports success while passing 25 of 545 tests (#491, with #490 as the root cause), and the baseline shows Inferno had never actually executed on s3-elasticsearch — every previous run stopped at the load step, so this is the first conformance number that backend has produced in CI.

Tests re-homed rather than dropped

• The nine batch ifMatch scenarios in if_match_suite move, one for one, to crates/rest/tests/batch_if_match.rs, where they run against the loop the server executes. The five transaction scenarios stay, with their SQLite and PostgreSQL macro expansions. Coverage narrows from two backends to one, which is the right trade now that the comparison is backend-independent and separately unit-tested in core::preconditions.
• Transaction/batch Bundles ignore fhirVersion content negotiation and persist entries as compile-time default (R4); HFS_DEFAULT_FHIR_VERSION ineffective #350's version-stamping guarantee had a batch half asserted through process_batch. It is now batch_entries_stamp_the_configured_default in default_version_fallback.rs; the transaction half stays in the SQLite unit test, with a pointer between them.
• s3_tests::test_aws_bundle_bulk_export_and_submit seeded via process_batch; it now seeds through ResourceStorage::create, since its subject is the export/submit feed.

Notes

• fix(s3): decline transaction Bundles S3 cannot honour, and bound bundle concurrency #500 is stacked on top of this PR. batch: entries are processed sequentially and BundleProvider::process_batch is dead on every backend — a batch over ~90 entries times out on S3 #501 blocks fix(s3): decline transaction Bundles S3 cannot honour, and bound bundle concurrency #500 — that PR declines transaction Bundles on S3 and directs callers to batch, and that advice is only sound once batch can carry the load. This lands first.
• HFS_BATCH_MAX_CONCURRENCY multiplies per concurrent request: twenty simultaneous batches at 16 put 320 storage operations in flight. This is a per-request bound, not a server-wide budget. A global semaphore would touch export, submit, subscriptions and seeding, and is deliberately out of scope.
• Auditing amplifies with concurrency — the same event count compressed into a much shorter window. This is why the effective S3 bound is 16 rather than its declared 32. Operators running large batches with HFS_AUDIT_BACKEND=database against the same bucket should prefer file/cloudwatch/none. Bounding the audit spawns is a separate concern.
• Three adjacent defects found while grounding this, filed rather than folded in: batch: PATCH entries return 405 and methods are case-sensitive — the batch and transaction method matchers disagree #502 (batch PATCH returns 405 while the transaction path accepts it, and methods are case-sensitive only in batch), batch: parse_request_url never strips the query string, so every conditional entry is dispatched against a resource type containing the query #503 (parse_request_url never strips the query string, so every conditional entry is dispatched and scope-checked against a resource type containing the query), batch: every per-entry error carries OperationOutcome code processing — a 403, 404 and 405 are indistinguishable without parsing English #504 (every per-entry error carries OperationOutcome code processing).
• ifNoneExist is not implemented on the batch path and is untouched here — parse_bundle_entry is its only reader and its only caller is process_transaction. There is no conditional-create race to fix in batch because batch has no conditional create. Whoever adds it will need to document the concurrency semantics.

[codecov]

Codecov Report

❌ Patch coverage is 94.26752% with 18 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
crates/rest/src/handlers/batch.rs	93.97%	18 Missing ⚠️

📢 Thoughts on this report? Let us know!