feat(sof): SQL-on-FHIR v2 integration — runners, export, conformance#66
Merged
Conversation
Documents current implementation status for all 6 phases (all unstarted), reusable assets from crates/sof and persistence, phase-by-phase gaps, and 5 issues requiring resolution before implementation begins.
Implements the full SQL-on-FHIR v2 IG against the HFS FHIR server. ## Persistence layer - `SofRunner` trait + `ViewFilters` (since, patient, group, limit) in `helios-persistence::core::sof_runner` - `RawSqlRunner` trait for `$sql-query-run` in `core::raw_sql` - SQLite in-DB runner (`SqliteInDbRunner`): compiles ViewDefinitions to parameterised SQL via `compiler.rs`; supports flat columns, forEach, forEachOrNull, unionAll, and runtime filter injection - PostgreSQL in-DB runner (`PostgresInDbRunner`): same compiler, JSONB operators, streaming via `tokio::sync::mpsc` + `spawn_blocking` - `inject_before_order_by` bug fix: compiler emits `\nORDER BY` but both runners searched for ` ORDER BY`; filter conditions were appended after the ORDER BY clause and silently ignored — fixed in both dialects - Auto-fallback: backends expose `sof_runner()` returning `Option<Arc<dyn SofRunner>>`; `Uncompilable` views fall back to `InProcessRunner` - Integration tests: `sof_sqlite_runner.rs`, `sof_pg_runner.rs` ## REST layer - `InProcessRunner`: universal FHIRPath fallback backed by `SearchProvider` - `$viewdefinition-run` handler: NDJSON / JSON / CSV / Parquet output, `_since` / `patient` / `group` / `_limit` filters, `X-HFS-Runner` header, auto-fallback on `Uncompilable` (`HFS_SOF_DEFAULT_RUNNER=auto`) - `$viewdefinition-export` handler: async job submission (POST), polling (GET), cancellation (DELETE), shard download; backed by `ExportJobController` trait - `InMemoryController` + `InMemorySink`: in-process export with DashMap job registry, Semaphore concurrency cap, row sharding via `planner` - Parquet export fix: `format_rows` was missing the `"parquet"` arm in `InMemoryController`, silently returning NDJSON bytes in `.parquet` shards - `$sql-query-run` handler: tenant-scoped raw SELECT via `RawSqlRunner`, DDL/DML blocked by `sqlparser` validation - `/$sql-on-fhir-capabilities` handler: CapabilityStatement for SoF ops - `HFS_SOF_DEFAULT_RUNNER` env var; capabilities, routing, AppState wired ## Tests Handler-level integration tests (no Docker required): - `sof_run.rs`: happy path, formats, limit, error cases, runner override, filter tests (_since / patient / group), auto-fallback test - `sof_export.rs`: submit→poll→download, cancel, multi-shard, Parquet magic - `sof_capabilities.rs`: CapabilityStatement shape - `sof_sql_query.rs` / `sof_sql_query_sqlite.rs`: raw SQL execution SoF v2 official conformance suite: - Vendored 22 fixture files from `FHIR/sql-on-fhir-v2/tests/` into `crates/rest/tests/conformance/sof_v2/` - `sof_conformance.rs`: 125 pass, 9 skipped (`%rowIndex` not yet implemented), 0 fail; runs on every PR, no Docker
SOF (SQL-on-FHIR) integration is now always compiled in. The `sof`
feature was on by default in both `helios-rest` and `helios-hfs` and
gated nothing useful: `helios-sof` was already an unconditional
dependency of both crates, `sqlparser` was already non-optional in
`helios-hfs`, and no documented or CI path turned the feature off.
Removing it strips ~80 `#[cfg(feature = "sof")]` attributes plus a
no-op `create_sof_routes()` fallback and aligns the source with how
the server is actually shipped.
Changes:
- Drop `sof` from `default = [...]` and delete the feature definitions
in `crates/rest/Cargo.toml` and `crates/hfs/Cargo.toml`.
- Promote `sqlparser` to a non-optional dep in `helios-rest`
(matches `helios-hfs`, which was already non-optional).
- Strip all `#[cfg(feature = "sof")]` / `#[cfg(not(feature = "sof"))]`
attributes from `crates/rest/src/{config,state,lib}.rs`,
`handlers/{mod,capabilities}.rs`, `handlers/sof/run.rs`,
`routing/fhir_routes.rs`, and the 6 `tests/sof_*.rs` files.
- Delete the no-op `create_sof_routes()` fallback in `fhir_routes.rs`.
- Tidy stale doc comments referencing "when the feature is enabled".
- Add the missing `tokio-stream` dep to `helios-persistence`
(pre-existing build break: `crates/persistence/src/sof/{sqlite,
postgres}.rs` import `tokio_stream::wrappers::ReceiverStream` but
the crate never declared the dep).
Out-of-tree impact: anyone previously building with
`--no-default-features --features R4,sqlite` (deliberately excluding
`sof`) will now silently get SOF compiled in.
Implements the remaining audit items against the v2 OperationDefinitions
for $viewdefinition-run, $viewdefinition-export, and $sqlquery-run.
Security
- Tenant auth on export status/cancel/download — guessable job IDs no
longer cross tenant boundaries; mismatched tenants get 404 (not 403)
to avoid leaking existence.
- Parameter binding for $sqlquery-run (spec MUST): :name placeholders
are rewritten to driver-native positional binds; values are bound via
ToSql / rusqlite params, never interpolated. New BoundValue enum
covers Bool/Int/Decimal/Text/Date/DateTime/Null.
Interop
- $sqlquery-run URL renamed to spec spelling /$sqlquery-run with
Library type-/instance-level routes.
- Export completion manifest uses spec field names (exportId, status,
location, cancelUrl, _format, exportStartTime, exportEndTime,
exportDuration) and grouped output.part entries (name/location/rowCount).
- /metadata advertises viewdefinition-run unconditionally and gates
viewdefinition-export / sqlquery-run on runtime wiring.
- Async export uses the 303 → result-URL pattern: status URL returns
303 See Other to /_operations/export/{id}/$result on completion.
- Prefer: respond-async is required at export kickoff (400 otherwise).
- DELETE cancellation returns 202 Accepted (was 204).
- Retry-After: 5 on running polls.
Functional
- viewReference (relative form ViewDefinition/{id}) accepted in run +
export; canonical / absolute rejected with descriptive 400 and
disclosed in $sql-on-fhir-capabilities.
- Stored ViewDefinition lookup at /ViewDefinition/{id}/$viewdefinition-run
reads from storage instead of the prior Phase-1 stub.
- Body-level parameters (_format, _limit, _since, patient, group,
header) parsed from Parameters bodies with body-over-query precedence.
- Inline resource[] in Parameters body drives the in-process runner
directly, bypassing storage.
- Multi-value patient/group (spec 0..*): ViewFilters fields are now
Vec<String>; runners expand to OR clauses; handlers accept
comma-separated query values and repeated Parameters entries.
- view[] multi-view export: ExportTask carries Vec<NamedView>, manifest
emits one output entry per view with view.name in output.part.
- clientTrackingId accepted at submit and echoed in the manifest.
- _format=json and header on export.
- _format=fhir on $sqlquery-run returns a FHIR Parameters resource per
the spec SQL→FHIR type table; composite types return 422.
- $viewdefinition-run streams NDJSON responses (Body::from_stream over
a tokio mpsc bridge) so large views don't have to buffer fully;
RowStream is now 'static.
Tests
- sof_run: viewReference (relative + canonical-rejected), inline
resources, multi-value patient filter.
- sof_export: manifest field conformance, tenant isolation, Prefer
enforcement, clientTrackingId echo, json format, multi-view.
- sof_sql_query_sqlite: parameter binding happy path, SQL-injection
regression, missing-value 400, _format=fhir.
All 66 SoF tests pass; SoF v2 conformance suite still at 0 failures
(9 known %rowIndex skips).
… PostgreSQL Replaces the in-memory FHIRPath fallback for $viewdefinition-run with native JSON queries on both backends. Both pass the full SQL-on-FHIR v2 conformance corpus (125/125 fixtures). Compiler / IR - Two-stage IR: PlanNode (scan / lateral-unnest / filter / project / union / recurse) + SqlExpr (json-path / cast / binop / case / scalar-from-chain / collection-agg / where-scalar / where-exists / reference-key / boundary). - Dialect trait abstraction (PgDialect, SqliteDialect) for placeholders, json navigation, json_agg, casts, regex. - compile_path lowers FHIRPath to SqlExpr; compile_view assembles the PlanNode tree per ViewDefinition. PostgreSQL - New PgInDbRunner streams rows via tokio_postgres::query_raw + bounded mpsc channel. - Type-aware comparisons / arithmetic — text JSON projections need ::numeric / 'true'-'false' literals to type-check under PG strict typing. - jsonb_typeof type-guards on every unnest source so FHIR singletons (object-shaped contact.name) wrap into one-element arrays. - coalesce(jsonb_agg(...), '[]'::jsonb) for empty collection columns. - Recursive CTE: split seed vs step branches and fold multi-path steps into a single lateral so rec_0 is referenced once (PG requirement). Recursive UNION ALL branches get _recurse_N aliases. - Boolean / Int / Decimal constants bound as text strings so they compare cleanly against ->> JSON-text projections; explicit ::numeric casts in arithmetic / ordering contexts. SQLite - json_each / json_extract / json_type chains with json_array(json(X)) type-guards for singleton object navigation. - Custom UDFs (sqlite_udfs.rs) for fhir_last_segment. Conformance - crates/rest/tests/sof_conformance.rs (SQLite, PASS_FLOOR=125) and crates/rest/tests/sof_conformance_postgres.rs (PG, PG_PASS_FLOOR=125) drive every fixture through the live HTTP endpoint. - The PG suite uses the same testcontainers + github.run_id labelling pattern as the existing PG integration tests; CI's self-hosted runner has Docker available, so it runs alongside everything else. - Conformance fixtures consumed from crates/sof/tests/sql-on-fhir-v2/ (the local copy under crates/rest/tests/conformance/ is removed). Removes the in-memory in_process runner and its module wiring.
# Conflicts: # Cargo.lock # crates/hfs/Cargo.toml # crates/rest/Cargo.toml # crates/rest/src/config.rs # crates/rest/src/handlers/mod.rs # crates/rest/src/lib.rs # crates/rest/src/routing/fhir_routes.rs # crates/rest/src/state.rs
`pub mod inline;` in sof/mod.rs already gates the module on `feature = "sqlite"`, so the inner `#![cfg(...)]` tripped Rust 1.91's new clippy::duplicated_attributes lint and broke CI.
- Bump lettre 0.11.21 → 0.11.22 to clear RUSTSEC-2026-0141. The advisory only affects the boring-tls backend; we use tokio1-rustls-tls, but the patched release closes the report cleanly. - Add three rustls-webpki advisories (RUSTSEC-2026-0098/0099/0104) to the CI --ignore list. They affect rustls-webpki 0.101.7 pulled in by aws-smithy-http-client 1.1.10 (rustls 0.21.x line, no longer patched). None of the three paths are reachable through the AWS SDK; rationale is documented inline next to the existing hickory entries.
The previous comment claimed SofRunner abstracted over an in-process FHIRPath runner and an in-DB runner. Only in-DB runners exist (SqliteInDbRunner, PgInDbRunner); when no backend provides one the handler returns 501 instead of falling back. Inline resource: parameters are materialised into a transient in-memory SQLite backend so they reuse the same in-DB pipeline. [skip ci]
…ator Inline `resource:` parameters previously materialised into a transient in-memory SQLite backend, which made the REST `$viewdefinition-run` handler depend on the sqlite cargo feature. Builds without sqlite (e.g. R4 + postgres only) failed to compile. Switch the inline path to the same in-process FHIRPath evaluator `sof-server` uses (`helios_sof::run_view_definition_with_options`), decoupling the REST layer from any storage backend for inline runs. The persistent path still dispatches to the backend's in-DB SOF runner and streams rows incrementally. - Promote `parse_view_definition[_for_version]`, `create_bundle_from_resources[_for_version]`, `filter_resources_by_patient_and_group`, and `filter_resources_by_since` to public `helios-sof` API. `sof-server`'s private wrappers now delegate to these. - Replace `build_inline_runner` call in `execute_view` with a new `execute_view_inline` that parses, filters, bundles, and evaluates inline resources in-process, returning a buffered response. Behavior matches `sof-server` for CSV/JSON/NDJSON/Parquet. - Drop the now-unused `helios_persistence::sof::inline` module.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Three independent consolidations of SoF v2 spec parsing/handling logic
that was previously duplicated across helios-sof, helios-persistence, and
helios-rest. Net diff: -388 lines.
Phase A — REST output formatters
helios-sof: promote `format_output`/`format_csv`/`format_json`/
`format_ndjson`/`format_parquet` to `pub`, add `rows_to_processed_result`.
helios-rest: delete `stream_to_csv`/`stream_to_json_array`/
`stream_to_ndjson`/`stream_to_parquet` (~165 lines of hand-rolled
CSV escaping, Parquet schema inference, and JSON object building).
`format_stream` now drains the `RowStream` once and dispatches through
the shared formatters. The streaming NDJSON `mpsc` path is unchanged
(it's the only true streaming path). Adds a small `content_type_headers`
helper to deduplicate the two header-mapping sites.
Phase B — Shared `constant[]` parsing
helios-sof: new `constants.rs` with `ConstantValue` enum + `parse_constant_from_json`
+ `ConstantValue::to_evaluation_result`. The four per-version
`ViewDefinitionConstantTrait::to_evaluation_result` impls in `traits.rs`
(~110 lines each, including `@`/`@T` prefixing and decimal precision
handling) collapse to a small per-version mapping into the neutral
`ConstantValue` plus a delegated call. Fixes an R6 bug where
`Integer64` was downcasting to `Integer`.
helios-persistence: gains a `helios-sof` dep (FHIR-version features
pass through). `populate_constants` in `compile_view.rs` delegates the
`value[X]` field walk to the shared parser, then lifts `ConstantValue`
into the compiler's `LitValue`.
Phase C — Shared Parameters extractor
helios-sof: new `params.rs` with `ExtractedRunParams` +
`extract_run_params_from_json` + `body_has_view_definition`. Single
source of truth for the `_format`/`_limit`/`_since`/`patient`/`group`/
`viewResource`/`viewReference`/`resource`/`header`/Parquet-options
field list and their accepted JSON shapes. Accepts both raw JSON and
FHIR-typed serde output (`name` as string or `{value: ...}`).
helios-rest: `BodyParams`, `extract_body_params`, `body_has_view`, and
the inline body-walking in `resolve_view_from_body` are gone — `run.rs`
uses the shared extractor end-to-end (~155 lines removed).
helios-sof: `extract_all_parameters` in `models.rs` keeps its strict
validation pass (`_limit` bounds, RFC 3339 `_since`, `compression`
allowed values, `header` boolean shape) that REST didn't have; doc
comment notes the relationship and future-convergence path.
Out of scope (justified inline in `MEMORY.md`-adjacent plan doc):
select-tree walking (engines produce structurally different artifacts),
where-path validation (compile-time vs runtime — narrow surface), and
FHIRPath compilation/evaluation (engines lower to different targets).
Verified: cargo fmt --all, cargo clippy with CI flags clean. Tests:
helios-sof lib 48/48 (+23 new), persistence sqlite_runner 22/22, REST
sof_run 18/18, REST sof_export 14/14, REST sof_conformance 1/1 (full
SoF v2 spec), sof-server parameter-body/validation/combination tests
all pass.
smunini
reviewed
May 15, 2026
…heck Replaces the hand-maintained `ARRAY_ROOTS` list in `compile_view.rs` (which only inspected the first path segment and ignored the resource root type) with a walk over the per-version `helios_fhir::*::get_field_type` tables. `CompileEnv` now carries `resource_type` and `fhir_version`. The walker tracks the current parent FHIR type starting from `ViewDefinition.resource`, looking up each segment's `(type, is_collection)` pair, and returns true when any non-terminal segment crosses a `0..*` element. Opaque segments (function calls, brackets) and unknown fields short-circuit to "no info" rather than guess. `build_plan` and `compile_view_definition_dialect` take a `FhirVersion` argument; `SqliteInDbRunner` / `PgInDbRunner` carry the version (default R4) with a `with_fhir_version` builder.
Replaces the static list of polymorphic root field names with a spec-driven check against the per-version `FIELD_TYPES` table emitted by `fhir-gen`. `extend_path` now takes `&CompileEnv` and, on a `Field(prev).ofType(T)` pair, confirms the typed variant `prev+T` exists in the schema before collapsing the steps to `Field(prev+T)`. Resource-rooted paths walk from `env.resource_type` through `lookup_field_type` to resolve the parent FHIR type at `prev`; sub-scope paths (rooted at a `where`/`forEach` iter alias, where the parent type isn't tracked) fall back to a parent-free scan via the new `field_exists_anywhere` helper. `lookup_field_type` moves from `compile_view.rs` to `sof/mod.rs` as `pub(super)` so both compilers share it; `field_exists_anywhere` joins it there.
The mod-level docs called `compiler` a "legacy string-pattern" compiler awaiting replacement by the IR pipeline, but that migration already happened — `compiler` is now a thin façade over `build_plan` + `emit_plan` and remains the entry point used by both backend runners. Replace the stage-ladder narrative with a description of the actual pipeline order. [skip ci]
Removes the file-scope missing-docs suppressions from error.rs, the SoF compiler IR/dialect/compile_view modules, and the SQLite/Postgres chain and filter builders. Every previously-undocumented public field, variant, method, and trait-method now carries a one-line doc.
smunini
reviewed
May 17, 2026
…adata Consolidates several spots that hand-rolled FHIR shape knowledge to instead consult the generated FIELD_TYPES tables and primitive-type list, following the same pattern as the recent POLYMORPHIC_ROOTS removal in compile_path. helios-fhir gains four reusable helpers so callers don't reimplement the FhirVersion dispatch each time: - get_field_type(version, parent, field) - field_exists_anywhere(version, field) - field_types(version) for whole-table enumeration - is_primitive_type(name) Behavioral fixes: - FHIRPatch handlers in both sqlite and postgres backends silently dropped every value[x] variant except a hard-coded handful (valueString, valueBoolean, valueInteger, valueDecimal, valueCode). Now any value[A-Z]* key is accepted, restoring valueQuantity / valueReference / valueDateTime / etc. for FHIRPath patches. - polymorphic_access::is_choice_element no longer always returns false in the no-context path; it consults the default version's FIELD_TYPES so common polymorphic bases (value, effective, onset, …) resolve correctly. - evaluator::could_be_typed_polymorphic_field now answers authoritatively via get_field_type when the parent resourceType is known, falling back to the suffix-validity heuristic only when the parent is unknown. - polymorphic_access::get_polymorphic_fields enumerates the spec-declared typed variants for the resource's parent type and intersects with the JSON keys present, instead of relying purely on a prefix scan that can match unrelated fields. Refactor cleanups: - The duplicated FhirVersion-dispatch wrappers in persistence/sof/mod.rs and fhirpath/type_inference.rs are now thin aliases to the canonical helios_fhir helpers. - fhirpath/fhir_type_hierarchy.rs replaces its HashSet of primitive names (and the buggy full-lowercase normalization that missed dateTime, base64Binary, positiveInt, unsignedInt) with a call to helios_fhir::is_primitive_type. type_inference::is_system_primitive delegates the FHIR-primitive arms similarly.
URL paths now match the spec (/export/{id}/{status,result,file}
instead of /_operations/export/...). The status endpoint 303s on both
success AND failure, with the result endpoint serving 500 +
OperationOutcome for failed jobs. In-progress polls return a Parameters
body with exportId/status (not OperationOutcome); X-Progress carries a
real percentage tracked per view completion. `output` is grouped per
view with repeating `location` parts as the spec defines, and the
non-spec `rowCount` and `progress` parts are dropped. The `source`
input parameter is now rejected with 400. Content-Location, 303
Location, and the manifest's `location`/`cancelUrl` are absolute URLs
prefixed with state.base_url(). Empty datasets yield zero output
entries; the result response carries an Expires header at
completed_at + 24h.
- Route GET on /ViewDefinition/$viewdefinition-run (and instance form). Per spec, GET is permitted for simple invocations using viewReference in the query string; viewResource/resource remain POST-only. - Enforce _format as required (1..1). HFS now consults the Accept header per spec precedence (_format > Accept) and returns 400 OperationOutcome when both are missing. The streaming path no longer silently coerces unknown formats to NDJSON. sof-server pre-checks the body, query, and Accept header and returns 400 when none provides a format. - HFS now returns 501 NotImplemented when the source parameter is supplied; previously it was silently dropped. source-based ETL is the province of the stateless sof-server, not the storage-backed HFS.
Register the operation at the spec's system level (POST /$viewdefinition-export) alongside the existing type and instance levels. Replace the 202 kick-off body — previously an informational OperationOutcome — with a Parameters resource carrying exportId, status=accepted, location (absolute status URL), and an echoed clientTrackingId when supplied.
The previous implementation accepted a raw SQL string in a custom `query`
parameter and ran it directly against the storage backend's physical
schema. The spec defines `$sqlquery-run` as executing a SQLQuery Library
profile whose `content` carries base64-encoded SQL, whose
`relatedArtifact[type=depends-on]` declares ViewDefinitions to
materialize as named tables, and whose `Library.parameter` declares
bound `:name` placeholders.
Replace the handler end-to-end:
- New `helios_sof::sqlquery` module: parses a SQLQuery Library (Library.type
validated against LibraryTypesCodes#sql-query; sql-name label regex
enforced; required parameter.type; depends-on duplicate-label check;
inline VD resources rejected), opens a per-request in-memory rusqlite
engine, binds `:name` parameters via native rusqlite parameter binding
(no string interpolation), and formats output (csv/json/ndjson/parquet
via the shared format_output path; `_format=fhir` derives value[X]
from the VD-declared column type, promotes BIGINT inferences to
valueInteger64, and rounds valueInstant to millisecond precision).
- New REST handler at `crates/rest/src/handlers/sof/sqlquery.rs`: wires
the three spec routes (system, type, instance). queryReference and
queryResource on the instance route conflict with the path id and are
rejected. `source` is out of scope and returns 422. Canonical and
relative `Type/{id}` depends-on references both resolve via storage;
Library by canonical URL picks the newest by meta.lastUpdated.
Dialect selection prefers `application/sql;dialect=sqlite` over bare
`application/sql`. SELECT-only validation via sqlparser. Per-request
row cap and watchdog-cancelled timeout.
- Capability statement advertises `supportsSqlQueryRun=true` and
`supportsCanonicalReference=true` unconditionally.
Remove the non-conforming implementation and its scaffolding:
delete `crates/persistence/src/core/raw_sql.rs` and
`crates/persistence/src/raw_sql/`, the `RawSqlRunner` field on
AppState, the `HFS_SOF_READONLY_URL` init block, and the four
`HFS_SOF_SQL_QUERY_*` config flags. New flags
`HFS_SOF_SQLQUERY_MAX_ROWS`, `_MAX_SOURCE_ROWS_PER_VD`, `_MAX_VDS`,
and `_TIMEOUT_SECS` replace them.
Output deviation: flat formats return raw bytes with the format MIME
type rather than a Binary resource. Documented in the handler.
Drive-by: update `sof_run.rs` and `sof_conformance.rs` tests to include
`_format=ndjson` in URLs that were missing it (broken since the prior
commit made `_format` strictly required for `$viewdefinition-run`).
- `_format` now resolves from body > URL query > Accept header (spec is `1..1` but allows any transmission); mirrors `$viewdefinition-run`. - `source` (0..1) is logged and ignored instead of returning 422 — the request still fails naturally if no Library was supplied. - `Accept: application/fhir+json` opts flat formats into the spec's `Binary | Parameters` shape by wrapping bytes as `Binary.data`. - `queryReference` parsing is tightened to `valueReference.reference` only (spec types the parameter as `Reference`). - Reject `queryResource` + `queryReference` together at system/type level with 400 (instance route already rejected). - Refresh stale `$sql-on-fhir-capabilities` doc comment.
Aligns the operation with the SoF v2 spec across seven conformance gaps
surfaced in the latest spec audit:
- Body-form Parameters input parity: parse `_format`, `header`, `patient`,
`group`, `_since`, and `clientTrackingId` from the request body in
addition to the query string. Query wins on conflict.
- Canonical viewReference resolution via SearchProvider (ViewDefinition.url
+ optional `|version`), in line with the `supportsCanonicalReference`
capability advertised by `$sql-on-fhir-capabilities`.
- Failed-job result: 200 + Parameters manifest with `status=failed` and an
`error` part carrying the OperationOutcome, replacing the bare 500. The
spec's status enum includes `failed`; the manifest is the canonical
channel for terminal states.
- `estimatedTimeRemaining` (valueInteger) on in-progress poll bodies,
derived from elapsed time and reported percent.
- Unknown parameter rejection: `deny_unknown_fields` on the query struct
plus a body parameter allowlist; both surface as 400 OperationOutcome
with code `not-supported` and the offending name in `diagnostics`.
- Patient/Group reference validation at kick-off: 404 OperationOutcome
listing any relative `Patient/{id}` / `Group/{id}` reference that fails
to resolve. Absolute external refs are skipped.
- Order-robust shard merge in the completion manifest: group locations by
`view_name` preserving first-seen order, so a controller emitting files
non-contiguously per view still produces one `output` entry per view.
Tests: rewires `test_export_failed_status_returns_303_then_failed_manifest`
to the new failure shape, plus five new tests covering unknown
query/body params, body-form input precedence, query-wins-on-conflict,
and missing patient-ref 404.
- viewReference now accepts canonical and absolute URLs (with `|version`
or spec-narrative `@version` suffix), not just `ViewDefinition/{id}`.
Resolution reuses the canonical-or-relative helper shared with
$sqlquery-run, lifted into handlers/sof/references.rs. The capability
statement's `supportsCanonicalReference: true` is now truthful.
- Register embedded `url` and `version` search params for ViewDefinition
and Library. Without these, the canonical-resolution code paths in
$viewdefinition-run, $viewdefinition-export, and $sqlquery-run all
returned zero matches because the FHIR base bundle doesn't cover
ViewDefinition (an R5+ SoF resource).
- `source` parameter now returns 400 + `not-supported` (was 501), via a
new RestError::NotSupported variant — matches the spec's error-code
examples for refused parameters. NotImplemented (501) stays for
features that aren't wired up yet.
- Instance-level `POST /ViewDefinition/{id}/$viewdefinition-run` rejects
bodies whose `viewResource`/`viewReference` refers to a different
ViewDefinition than the path id. Matching or id-less bodies pass
through as no-op overrides.
- Wire system-level `POST/GET /$viewdefinition-run` route, matching the
pattern already used by $viewdefinition-export and $sqlquery-run.
The SoF v2 PostgreSQL conformance test was POSTing to `/ViewDefinition/$viewdefinition-run` without `_format` or an `Accept` header, so every fixture returned HTTP 400 once the handler started requiring an explicit format. Matches the SQLite sibling at `sof_conformance.rs:344`.
…paths The SoF v2 $viewdefinition-run spec sets `patient` at 0..1 and `group` at 0..* with union semantics for multi-group. Four extractors disagreed on that cardinality, so the strict POST path on sof-server silently dropped all but the last `group` entry, and the HFS REST inline FHIRPath path applied only the first patient/group reference. - Add `helios_sof::split_csv_refs` shared comma-split helper; replace the duplicated `split_refs` in `crates/rest/src/handlers/sof/run.rs`. - `filter_resources_by_patient_and_group` now takes `&[String]` for both refs; patient unions across entries. Group still errors when non-empty (audit item #2 will turn that into 400 not-supported separately). - `ExtractedParameters.patient` / `.group` change from `Option<String>` to `Vec<String>`; `process_parameter` accumulates instead of overwriting, matching the shared permissive extractor in `params.rs`. - sof-server `handlers.rs` builds patient/group Vecs (body precedence, comma-split fallback for the query string) and passes slices to the filter at both call sites (source-bundle and provided-resources). - HFS REST `execute_view_inline` drops `.first()` and passes all refs through. The runner path (`ViewFilters`) was already multi-ref. Tests: - New unit test for `split_csv_refs`. - New `test_extract_multiple_group_parameters_accumulate` proving the strict extractor accumulates every group entry. - New `test_inline_run_applies_all_patient_refs` integration test proving multi-patient refs union through the inline FHIRPath path. Closes audit item #1 from /Users/slm/.claude/plans/analyze-intensly-this-specification-snuggly-fairy.md
…audit #3) The patient/group filter in `filter_resources_by_patient_and_group` used a hand-rolled allowlist (`Observation|Condition|MedicationRequest| Procedure|Encounter` plus a `.patient` reference catch-all) that both leaked out-of-compartment resources for types not in the allowlist and dropped in-compartment resources for unrecognised types. The fix drives the scan off the FHIR spec's `CompartmentDefinition-patient.json` (already code-generated as `get_compartment_params`) plus the search- parameter registry's FHIRPath expressions. Spec-data refactor (prerequisite — no circular dep): - Move `SearchParameterRegistry`, `SearchParameterDefinition`, `SearchParameterLoader`, `SearchParameterStatus`/`Source`, `SearchParamType`, `LoaderError`/`RegistryError` from helios-persistence to a new helios-fhir::search module. Strip the unused tokio broadcast machinery (no subscribers existed). Adjust `resolve_param_type` to take `&[&str]` so it doesn't need persistence's `SearchValue` type. - Persistence keeps the runtime/index code (`extractor`, `writer`, `reindex`, `converters`) and re-exports the moved types so its public API stays stable. `resolve_param_type` becomes a thin adapter that maps `&[SearchValue]` to `&[&str]` before delegating. Compartment filter: - New `crates/sof/src/compartment.rs`: - `resource_in_patient_compartment` enumerates the spec-defined compartment params for `(Patient, resource_type)`, resolves each to a FHIRPath expression via the registry, evaluates it against the resource JSON, and matches the resulting Reference(s) against the requested patient set. - `resolve_group_members_to_patient_refs` walks Group resources in the inline bundle and pulls their `member.entity` Patient refs. - `default_registry(FhirVersion)` lazy-loads a process-wide registry from `${HFS_DATA_DIR:-./data}/search-parameters-{ver}.json`, falling back to embedded minimal params. - `filter_resources_by_patient_and_group` now takes `FhirVersion` + `&SearchParameterRegistry`. Group filtering unions resolved group- member patient refs into the effective patient set instead of erroring with 501. - Callers (sof-server `handlers.rs`, HFS REST `run.rs::execute_view_inline`) thread the registry through using `default_search_param_registry`. Tests: - 14 helios-fhir search tests (registry, loader, errors). - 4 new compartment.rs tests (AllergyIntolerance via patient ref, Patient identity, out-of-compartment Library, Group→Patient resolution). - 2 updated handlers.rs tests for the new signature. - New REST integration test `test_inline_run_patient_compartment_ allergyintolerance` exercising the FHIRPath-driven path end to end. Closes audit item #3 from crates/sof/docs/spec-audit-viewdefinition-run.md.
The audit-item-#3 compartment fix already replaced the unimplemented `group` path with actual resolution against `Group.member.entity` in the inline bundle, which makes the prior `501 NotImplemented` refusal unreachable. This commit closes out the remaining cleanup: - Drop the stale "group ... not yet supported" doc comment on the $viewdefinition-run handler. Group is honored now. - Remove the dead `SofError::InvalidViewDefinition → ServerError::NotImplemented` arm in sof-server's `filter_resources_by_patient_and_group` wrapper — the underlying filter no longer emits that variant for group. - Replace the leftover "Item #2 is a separate fix" comment in the handler with a description of the new group resolution path. - Add `test_inline_run_group_resolves_member_patients` covering the full inline pipeline: a `group=Group/g1` ref → resolved against an inline Group → its Patient members union into the effective compartment set → only those Patients survive the filter. - Mark item #2 as fixed in the spec-audit doc. The SHOULD-emit-OperationOutcome-when-target-absent gap remains tracked as audit item #5.
Commit e5d6b2d normalized the NDJSON response content-type to application/x-ndjson per the SoF v2 spec, but two assertions in test_format_parameter_body.rs still expected the legacy application/ndjson. Input parameters retain the legacy form to keep exercising the parser's tolerance.
…v2 PR #353 Brings $sqlquery-run and $viewdefinition-run into compliance with the upcoming SQL-on-FHIR v2 spec (HL7/sql-on-fhir#353): - Add optional `_limit` to $sqlquery-run as a soft cap; truncates the final result set silently (returning fewer rows than requested is not an error per the PR). Body wins over URL query string. - Relax `_format` from `1..1` to `0..1` on both operations, defaulting to `ndjson` when neither `_format` nor a usable `Accept` header is supplied. `_format` still takes precedence over `Accept`. $viewdefinition-run and $viewdefinition-export already use the spec-aligned parameter shapes (closes #343, #344) — no changes needed there.
The in-DB compiler's BoundaryKind::DateTime emit only padded length=10
("YYYY-MM-DD") inputs, returning NULL for shorter forms. Commit 27232ec
aligned the fn_boundary fixture with HL7/sql-on-fhir#357, which
declares the column type as `dateTime` even when the source FHIRPath
expression is a `date` field. That dropped two fixtures (`date lowBoundary`,
`date highBoundary` — Patient.birthDate = "1970-06", length 7) from the
conformance suite, taking the pass count from 125 to 124 against the
floor of 125.
FHIRPath `lowBoundary()`/`highBoundary()` preserves source precision:
a date input stays date-only, a dateTime input pads to full instant.
The compiler's `column_type_hint` can't distinguish the two when the
spec author writes `type: dateTime` over a date-typed source.
Extend the DateTime emit to dispatch on input length:
- length 4 ("YYYY") → date semantics, pad to "YYYY-01-01"/"-12-31"
- length 7 ("YYYY-MM") → date semantics, pad to month start/last day
- length 10 ("YYYY-MM-DD") → datetime semantics (unchanged)
Bumps PASS_FLOOR from 125 to 126 (the suite now passes the previously-
failing date boundary cases AND keeps the existing datetime cases green).
Mirrors the SQLite ratchet from the prior commit. The PostgreSQL in-DB compiler shares the boundary emit logic, so the dateTime partial-date fix lifts both backends from 125 to 126 passing fixtures.
# Conflicts: # Cargo.lock
…2 PR #349) Per the FHIRPath specification, join() on an empty input collection returns an empty collection, not an empty string. Update the evaluator and refresh the vendored SoF v2 conformance tests to apply PR HL7/sql-on-fhir#349: remove the two shareable fhirpath.json join tests (which mis-specified the empty case as "") and extend the experimental fn_join.json tests with an empty-input patient expecting null.
The in-DB SQL compiler's JoinAggregate lowering wrapped the string aggregate in coalesce(..., ''), forcing join() on an empty input collection to yield "". Per the FHIRPath spec (SoF v2 PR #349), the empty case must yield empty (NULL). string_agg/group_concat over zero rows already returns NULL, so the coalesce is dropped — fixing both the SQLite and PostgreSQL dialects (shared emit site). Lower the SQLite and PostgreSQL conformance PASS_FLOOR from 126 to 124: PR #349 removed two join() fixtures from the upstream fhirpath.json corpus, shrinking the total fixture count (not a compiler regression).
…stic [skip ci] Entry F of spec-inconsistencies.md conflated HTTP chunked transfer encoding (transport framing, format-agnostic per RFC 9112) with application-level incremental result production, and falsely claimed both ops stream identically. Rewrite with an accurate per-op/per-binary behavior matrix, separate the two concepts, and cover $viewdefinition-export's async-bulk model.
…f silent truncation When the underlying RowStream errors mid-flight, $viewdefinition-run silently truncated its response. The NDJSON streaming path broke out of the loop, ending the chunked body cleanly so the client received a truncated 200 with no way to detect it; the buffered csv/json/parquet paths returned partial Vecs and swallowed formatter failures into an empty 200. streaming_ndjson_response now yields an io::Error into the body channel so hyper aborts the chunked transfer. drain_stream and format_stream now return Result and propagate row/formatter errors as a RestError, yielding a real error status since the response is not yet committed on the buffered path. Adds unit tests covering both abort and clean-stream behavior.
…nked encoding sof-server materialized the full parquet file (and the multi-file ZIP) in memory, then copied it into 64 KB pieces and sent it via Body::from_stream — emitting Transfer-Encoding: chunked even though the size was already known. That dropped Content-Length, added a full-payload memcpy, and the streaming paths also skipped attach_filter_warnings, silently losing patient/group absence warnings. Buffered parquet (single file and multi-file ZIP) is now returned directly with Content-Length, matching HFS REST, and both paths attach filter warnings. streaming.rs is renamed to parquet_zip.rs and reduced to the ZIP builder; the dead cosmetic-chunking helpers are removed. Updates docs/spec-inconsistencies.md entry D (formerly F) to the post-fix reality, renumbers the entries A,D,E,F,G -> A,B,C,D,E, and replaces fragile file:line citations with function references.
# Conflicts: # Cargo.lock
…114) Three spec-compliance items from the v2 OperationDefinition audit (`crates/sof/docs/spec-inconsistencies.md`): 1. Absent `patient` / `group` references are now `400 Bad Request` with an `OperationOutcome`, per the spec's error table — the previous "200 OK + Warning: 199 header" path is gone. New `SofError::ReferencedResourceNotFound` carries the absence; the filter in `filter_resources_by_patient_and_group` returns Err instead of an outcome wrapper, dropping `PatientGroupFilterOutcome`. 2. `sof-server` now accepts a bare `ViewDefinition` body as an alternative to a `Parameters` wrapper (the HFS REST handler already did). Other operation parameters must come from the query string in that shape. 3. The HFS REST system-level route at `/$viewdefinition-run` was already wired (verified during audit) — capability output and docs continue to advertise it. Updates the see-also section of `spec-inconsistencies.md` to record item #1 as a resolved deviation and notes the bare-ViewDefinition shortcut as a deliberate-but-spec-silent convenience.
5 tasks
* fix(sof): align $viewdefinition-run with v2 spec error & body shapes Three spec-compliance items from the v2 OperationDefinition audit (`crates/sof/docs/spec-inconsistencies.md`): 1. Absent `patient` / `group` references are now `400 Bad Request` with an `OperationOutcome`, per the spec's error table — the previous "200 OK + Warning: 199 header" path is gone. New `SofError::ReferencedResourceNotFound` carries the absence; the filter in `filter_resources_by_patient_and_group` returns Err instead of an outcome wrapper, dropping `PatientGroupFilterOutcome`. 2. `sof-server` now accepts a bare `ViewDefinition` body as an alternative to a `Parameters` wrapper (the HFS REST handler already did). Other operation parameters must come from the query string in that shape. 3. The HFS REST system-level route at `/$viewdefinition-run` was already wired (verified during audit) — capability output and docs continue to advertise it. Updates the see-also section of `spec-inconsistencies.md` to record item #1 as a resolved deviation and notes the bare-ViewDefinition shortcut as a deliberate-but-spec-silent convenience. * fix(sof): tighten $viewdefinition-export spec compliance - Reject unsupported `_format` values at submit time with 400 + OperationOutcome (not-supported). Previously unknown formats silently downgraded to NDJSON in the serializer while the manifest echoed the bogus value. - Reject duplicate view names in a multi-view submission with 400 + OperationOutcome (invalid). The completion manifest groups files by view name, so colliding names would silently merge separate views' shards into a single `output` entry. - Capture `failed_at` once at the failure transition (added to `JobStatus::Failed`) and use it in the failed-job manifest so `exportEndTime` / `exportDuration` are stable across multiple polls of the result URL. Previously the result handler called `chrono::Utc::now()` on each poll. Tests: - `test_export_unknown_format_returns_400` - `test_export_duplicate_view_names_returns_400` - `test_export_failed_manifest_export_end_time_is_stable` - `test_export_cancel_after_completion_preserves_completed_state` (locks in the existing controller behavior that a cancel on a completed job is a no-op rather than overwriting state) * fix(pysof): cover SofError::ReferencedResourceNotFound in match `ReferencedResourceNotFound(String)` was added to `SofError` in 66f701d (the $viewdefinition-run v2 spec alignment) but the exhaustive match in `lib_coverage_tests.rs` was not updated, breaking `cargo test --workspace` (used by the Code Coverage CI job). Local `cargo test` from the repo root misses this because pysof is excluded from default workspace members.
Tightens the $sqlquery-run handler and its shared canonical-URL resolver
to better match the OperationDefinition-SQLQueryRun spec:
- `source` parameter now returns 400 BadRequest instead of warn-and-ignore;
the server can't honor an external data source, so accepting the request
silently was misleading.
- Server hard-cap (sof_sqlquery_max_rows) silently truncates the result
set per SoF v2 PR #353 ("Servers MAY enforce a maximum value, silently
capping client-supplied limits"). Per-VD source-row caps stay as hard
errors because truncating depends-on tables changes query semantics.
- Canonical-URL lookups that match no registered resource now return 404
("Library or ViewDefinition not found") instead of 422, aligning with
the spec's error mapping and the relative-reference path. Affects both
$sqlquery-run and $viewdefinition-run via the shared resolver.
- /$sql-on-fhir-capabilities now advertises supportsAbsoluteReference:true
(we already resolve absolute http(s) canonicals via SearchProvider).
- Dropped the misleading Accept: application/fhir+xml → fhir mapping;
the formatter only emits JSON.
- Added a "Type fidelity" section to the handler module doc explaining
that synthesised columns (no matching VD column) fall back to SQLite
storage-class inference under _format=fhir.
- Added clarifying comment on the Library.parameter[use=out] skip.
Tests updated/added: silent-truncate engine test, canonical-not-found
404 test for both operations, source-parameter 400 test.
The SoF v2 IG does not ship declarative JSON test fixtures for $sqlquery-run the way it does for ViewDefinition. The closest upstream reference is HL7/sql-on-fhir:sof-js/tests/server/sql.test.js — a JavaScript/Bun suite that drives a live HTTP server. Ports those test scenarios into a Rust integration suite that runs in CI alongside the rest of our tests. Adds crates/rest/tests/sqlquery_spec_conformance.rs (8 tests) focused on gaps not covered by sof_sqlquery.rs: - OperationOutcome `issue[0].code` assertions on the not-found, invalid-parameter, and processing-error paths. - _format=fhir round-trip for a VD-declared boolean column. - Empty-result Parameters output: confirms the `parameter` key is omitted (matching FHIR JSON convention and sof-js's expectation). - Documented deviation: SoF v2 PR #353 makes _format default to ndjson, while the older sof-js test asserts a 400 with `required` when _format is omitted. The deviation is intentional; the conformance test pins our behavior. Also fixes the empty-Parameters output in sof::sqlquery::output to omit the `parameter` key when there are zero rows (FHIR JSON convention for empty repeating elements).
# Conflicts: # Cargo.lock # crates/fhir/src/lib.rs # crates/persistence/src/error.rs # crates/rest/src/handlers/capabilities.rs # crates/rest/src/handlers/compartment.rs # crates/rest/src/lib.rs # crates/rest/src/state.rs
Conflict resolutions:
- crates/persistence/Cargo.toml, crates/rest/Cargo.toml: union of both
sides' FHIR version feature pass-throughs (helios-sof from this branch,
helios-audit from main)
- crates/fhirpath/{fhir_type_hierarchy,type_inference}.rs: took main's
spec-derived per-version FhirPrimitiveTypeProvider implementation over
this branch's hand-maintained union list
- crates/persistence/src/search/loader.rs: kept this branch's re-export
shim; ported main's FhirVersion::default_enabled() Default impl into
the moved loader (crates/fhir/src/search/loader.rs)
- crates/rest/src/handlers/capabilities.rs: union of trait bounds
(SearchProvider from main, 'static from this branch)
# Conflicts: # Cargo.lock
…on behavior)
Implements the reconciled cross-operation rules from spec PR #365
(issues #358-#363), assuming it merges as proposed:
- $viewdefinition-export completion (#363): the status poll now returns
200 OK with the manifest Parameters in the body (Expires header moved
onto the poll); failed jobs return 500 + OperationOutcome directly on
the poll. The 303 See Other redirect and the separate
/export/{job-id}/result route/handler are removed.
- FHIR content negotiation on run operations (#358/#362): new shared
module crates/sof/src/fhir_format.rs used by both HFS REST and
sof-server. Accept: application/fhir+json with a flat _format wraps
the payload in a serialized Binary envelope (base64 data, bare native
media type); application/fhir+xml is rejected with 406 +
OperationOutcome (new ServerError::NotAcceptable). $sqlquery-run now
shares the same helpers and gained the fhir+xml 406 check.
- _format=fhir on $viewdefinition-run (#360): both servers render a
typed Parameters resource (one row per result row, value[x] driven by
the declared column.type, NULL parts omitted, collections repeated as
parts, empty results omit parameter). Accept: application/fhir+json
with no _format selects this format.
- Bundle unwrapping (#359): the shared lib extractor
(crates/sof/src/params.rs) now unwraps a Bundle supplied as a
`resource` value one level into its entries — previously only
sof-server's own extractor did this, so the HFS REST path ran the
view against zero resources. Spec worked Example 5 added as a
regression test.
- Parquet media type: responses use application/vnd.apache.parquet
(run, sqlquery, export downloads); application/parquet and
application/octet-stream remain accepted as request aliases. The HFS
REST CompressionLayer now skips parquet/zip responses (both parquet
identifiers), matching the sof-server predicate added in 58b15f8.
- Capability declarations: sof-server /metadata and
$sql-on-fhir-capabilities list fhir and the new parquet media type.
Deferred (out of scope): $sqlquery-export; _format=fhir on exports.
…t operations Completes the four SQL-on-FHIR data operations per spec PR HL7/sql-on-fhir#365 (Common Operation Behavior). $sqlquery-export (new): - Routes at system (/$sqlquery-export), type (/Library/$sqlquery-export), and instance (/Library/{id}/$sqlquery-export) level; shares the /export/{job-id}/* status, cancel, and download flow with $viewdefinition-export, including 200 OK + manifest completion. - Kick-off resolves everything synchronously so client errors surface before the 202: query 1..* (name / queryReference / queryResource / per-query parameters bindings), Library parsing and SELECT-only validation, depends-on cap, depends-on ViewDefinition resolution (preferring inline `view` table sources matched by canonical url or label, then storage), and Library.parameter binding. Instance level scopes all input parameters to system/type per the spec, so Libraries used there must carry parameter defaults. - Execution generalizes the export controller with an ExportWork enum (Views | SqlQueries). SQL jobs materialize table sources into an in-memory SQLite engine via the controller's SofRunner (patient / group / _since filters apply to the sources), run the SQL under the same row caps and watchdog timeout as $sqlquery-run, refine output column types from the ViewDefinition schemas, and shard through the existing planner/sink. Flat formats serialize via helios_sof::format_output, byte-identical to the run operation. _format=fhir on both export operations: - Each output is a file of newline-delimited FHIR Parameters resources, one per result row, columns as typed top-level parameter entries (value[x] from declared column types, NULLs omitted, collections repeated per element), per the spec's FHIR Format section. - Files are written as shard-N.fhir.ndjson and downloaded with Content-Type: application/fhir+ndjson. - Serializers in helios_sof::fhir_format::format_view_fhir_ndjson (view rows) and helios_sof::sqlquery::format_fhir_ndjson_rows (query results, reusing the typed value[x] mapping). Discovery: $sql-on-fhir-capabilities reports supportsSqlQueryExport and HFS /metadata declares the sqlquery-export operation when an export controller is wired. Tests: 8 new integration tests in crates/rest/tests/sof_export.rs (happy path, fhir format for both operations, instance level, missing query 422, missing Prefer 400, unknown Library 404, non-SELECT SQL 400) plus unit tests for the new serializers.
Four fixes found by auditing the implementation against spec PR #365: - Unsupported _format → 400 (was 415 on the sof-server body path): SofError::UnsupportedContentType now maps to ServerError::BadRequest, per the spec's "SHALL be rejected with 400 Bad Request + OperationOutcome". The stub tests that entrenched 415 are corrected, and a new in-bin test locks the 400 against the production handler. - _format=json export shards are now downloaded with Content-Type: application/json (previously fell through to the ndjson branch); test asserts the header. - Export cancellation race: a job cancelled mid-run could be resurrected to Completed/Failed by its background task, making post-DELETE polls return 200 instead of the spec-required 404. Completion/failure/progress writebacks now go through set_status_if_running, which leaves a Cancelled job untouched. Deterministic unit test added using a blocking mock SofRunner. - HFS_EXPORT_PRESIGN_TTL_SECS default raised 3600 → 86400 so out-of-the-box S3 deployments honor the spec's >= 24h output.location validity (matching the Expires header already advertised on the completion poll).
…ription delivery [skip ci] - SQL-on-FHIR async export job state (InMemoryController) should move to database-backed storage, following the Bulk Data $export pattern. Until then, SoF async operations must not be clustered: job state is process-local, so restarts invalidate status URLs and a second instance cannot see jobs submitted to the first. - Same review flagged Subscriptions WebSocket delivery as the only other instance-affine state: connected clients are tracked in process memory, so multi-instance delivery needs a shared fan-out channel. Bulk Data $export (DB-backed job store), search pagination (stateless), and HTS/search-parameter caches (perf-only, DB authoritative) were checked and are cluster-safe.
Per SoF PR HL7/sql-on-fhir#365 (Common Operation Behavior — Asynchronous Delivery), each status poll's headers govern that poll's response, including the 200 OK completion manifest. The status handler now rejects Accept: application/fhir+xml (without fhir+json) with 406 Not Acceptable + OperationOutcome, matching the run operations' behavior, instead of silently returning JSON.
…xport) Align our SoF implementation to the final state of PR #365 after John Grimes' 13 follow-up commits. Behavioral change — remove _format=fhir from the export operations (commit 8c21fc4), reverting the export half of bbcaa0a: - $viewdefinition-export / $sqlquery-export now reject _format=fhir with 400; the export _format binds to the new ExportOutputFormatCodes set (csv, ndjson, parquet, json). A newline-delimited Parameters file has no established media type or consumer; fhir is a run-operation format. - Delete the now-dead export-fhir code: format_view_fhir_ndjson + FHIR_NDJSON_MIME, format_fhir_ndjson_rows + its re-export, the .fhir.ndjson extension and application/fhir+ndjson download content-type, and the column-type refinement that only fed it. - Flip the two export-fhir tests to assert 400. Doc/metadata alignment: - Async pattern rename to "Asynchronous Bulk Data Request Pattern" (2c7d3d8) in export.rs and spec-inconsistencies.md. - Document the run operations' return as Binary (raw stream) with Parameters as the _format=fhir exception (86c178b) in sqlquery.rs. - capability.rs: declare the run/export value-set split via a second formatBinding (ExportOutputFormatCodes); strengthen the capability test to assert it. - Add a Resolution summary to spec-inconsistencies.md recording how issues #358-363 settled. The run-side Accept/envelope behaviour (b8b9014) and run-only fhir support were already correct and unchanged. cargo fmt + clippy (CI flags) clean; helios-sof and helios-rest SoF test suites pass.
Upgrade vulnerable crates to patched versions: - postgres-protocol 0.6.11 -> 0.6.12 (RUSTSEC-2026-0179, -0180) - tokio-postgres 0.7.13 -> 0.7.18 (RUSTSEC-2026-0178) - pyo3 0.27.2 -> 0.29.0 (RUSTSEC-2026-0176, -0177) tokio-postgres 0.7.18 pulls postgres-types 0.2.14, which requires newer serde, so bump the workspace pins serde 1.0.220 -> 1.0.221 and serde_json 1.0.143 -> 1.0.144. Bump pysof's pythonize and pyo3-build-config to 0.29 to allow the pyo3 upgrade.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Full SQL-on-FHIR v2 IG implementation integrated into the HFS FHIR server, including in-DB runners for SQLite and PostgreSQL, the async export pipeline, raw SQL query execution, and the official SoF v2 conformance test suite.
compiler.rs; supports flat columns,forEach/forEachOrNull,unionAll, and runtime filter injection (_since,patient,group,limit) for both SQLite and PostgreSQL dialects.InProcessRunner: universal FHIRPath fallback for anySearchProvider-backed storage; auto-fallback onSofError::UncompilablewhenHFS_SOF_DEFAULT_RUNNER=auto.$viewdefinition-run(NDJSON / JSON / CSV / Parquet,X-HFS-Runnerheader),$viewdefinition-export(async jobs, polling, cancellation, shard download),$sql-query-run(tenant-scoped raw SELECT),/$sql-on-fhir-capabilities.inject_before_order_bysearched for" ORDER BY"(space-prefix) but the compiler emits"\nORDER BY"(newline-prefix) — filter conditions were appended after the clause and silently ignored. Fixed in both SQLite and PostgreSQL runners.InMemoryController::format_rowswas missing the"parquet"arm, silently writing NDJSON bytes into.parquetshard files.FHIR/sql-on-fhir-v2/tests/vendored;sof_conformance.rsruns 125 cases (9 skipped —%rowIndexnot yet implemented), no Docker required.Test plan
cargo test -p helios-persistence --features sqlite— SQLite in-DB runner + compiler testscargo test -p helios-rest --features sof,sqlite— all handler tests including conformance (125 pass, 9 skip, 0 fail)cargo clippy --all-targets --all-features -- -D warnings …— cleancargo test -p helios-persistence --features postgres(requires Docker)