feat(sof): SQL-on-FHIR v2 integration — runners, export, conformance#66
Open
mauripunzueta wants to merge 50 commits into
Open
feat(sof): SQL-on-FHIR v2 integration — runners, export, conformance#66mauripunzueta wants to merge 50 commits into
mauripunzueta wants to merge 50 commits into
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.
`sof-server`'s patient-compartment filter previously needed
`data/search-parameters-{ver}.json` at runtime (joined against the
`get_compartment_params` lookup) to know which FHIRPath expression
links each resource type to the Patient compartment. The Docker image
ships with `include_data: false` and `cargo test` runs with the
per-crate CWD, so in both contexts the filter silently fell back to
the embedded minimal parameter set and went no-op on anything with a
non-trivial compartment path (e.g. `Appointment.participant.actor`).
Bake the join into the helios-fhir crate at code-gen time:
- Extend `fhir_gen` (no side-binary): new
`generate_compartment_expressions_file` helper joins
`compartmentdefinition-*.json` against `search-parameters.json` from
the same per-version resources dir and emits
`crates/fhir/src/compartment_expressions/{ver}.rs` exposing
`get_compartment_param_expressions(compartment, resource_type)
-> &'static [(name, fhirpath)]`. Output is deterministic
(BTreeMap-ordered) and lives in a small, reviewable file separate
from the giant per-version r4.rs/r4b.rs/r5.rs/r6.rs.
- New `--compartments-only` CLI flag on `fhir_gen` regenerates JUST
these tables without re-running the full per-version generator
(which produces a 100k+ line diff). The full generator still emits
the tables as part of a normal run.
- `helios_sof::compartment::resource_in_patient_compartment` now
consults the compiled-in tables directly. The
`default_search_param_registry`/`OnceLock`/`HFS_DATA_DIR` lazy-loader
added in the previous compartment commit is gone — no runtime data
file is read by the SoF compartment path.
- `helios_sof::filter_resources_by_patient_and_group` drops its
`&SearchParameterRegistry` argument. sof-server's `handlers.rs` and
HFS REST's `execute_view_inline` drop the registry plumbing.
Tests:
- New `patient_compartment_includes_appointment_via_nested_participant_actor`
in `helios_sof::compartment::tests` — the exact case the old
hardcoded `(subject|patient)` allowlist couldn't reach.
- Existing compartment + sof_run integration tests stay green
(92 sof lib + 26 REST sof_run); they now exercise the real
compiled-in expressions regardless of CWD.
Closes the remaining shipping concern from audit item #3.
) Per the SoF v2 spec, "Server SHOULD return OperationOutcome if requested patients absent" (same for `group`). Previously both inline paths silently returned an empty result set when the requested target wasn't in the bundle. - `filter_resources_by_patient_and_group` now returns `PatientGroupFilterOutcome { resources, warnings }`. The warnings field carries one message per absent `patient` / `group` reference (target Patient or Group resource not in the inline bundle). The filter still runs and returns partial results — warnings are advisory. - sof-server `run_view_definition_handler` accumulates warnings from both filter passes (source-bundle branch + provided-resources branch) and attaches them as `Warning:` HTTP headers (RFC 7234 §5.5, warn-code 199) on the response. Works for every output format (CSV/JSON/NDJSON/Parquet) without altering the body. - HFS REST `execute_view_inline` does the same via a new `build_response_with_warnings` helper. - Audit-doc updated to reflect the inline-path fix; the in-DB runner path remains a follow-up (would need a SearchProvider::read probe before streaming). Tests: - `test_inline_run_emits_warning_for_absent_patient_target` in crates/rest/tests/sof_run.rs — confirms the Warning header is populated when a requested Patient isn't in the supplied bundle. - `test_filter_with_unresolvable_group_returns_empty` updated to assert the new warning text is present.
Audit item #3 was previously fixed for the inline `$viewdefinition-run` path (sof-server + HFS REST inline) via compile-time `get_compartment_param_expressions` tables. The in-DB runner path (HFS storage-backed SoF runner, SQLite + Postgres) still used a hand-rolled `WHERE (subject.reference=? OR patient.reference=?)` JSON-path clause, so nested compartment links like `Appointment.participant.actor` were unreachable. This commit closes that gap on both runners. Approach: the search_index table already has pre-evaluated SearchParameter values populated at resource-write time. For compartment filtering, look up the search-param names that link the resource type to the compartment via `helios_fhir::compartment_params(version, "Patient", resource_type)` and emit an `EXISTS (SELECT 1 FROM search_index ...)` clause against those names — no FHIRPath evaluation at query time, no FHIRPath→SQL compiler. - Add `helios_fhir::compartment_params(version, compartment, resource_type)` as a public version-dispatching helper; HFS REST compartment-search handler now uses it too (removes the local duplicate). - SQLite runner (`crates/persistence/src/sof/sqlite.rs`): replace the hardcoded subject/patient/group JSON-path clause with `compartment_filter_sql` driving an EXISTS against search_index. Patient resources matching their own id stay a special case (compartment owner). Resources not in the compartment at all emit `1=0` (spec: "SHALL NOT return resources from patient compartments outside provided list"). - Postgres runner (`crates/persistence/src/sof/postgres.rs`): same rewrite, $N parameter syntax. - Group filtering on both runners: resolve each `Group/{id}` via an extra DB read, extract `member.entity` Patient references via the shared `helios_sof::resolve_group_members_to_patient_refs` (same helper the inline path uses), fold into the patient filter, then clear the group_refs. Mirrors the inline path's spec-correct semantics rather than the old runner's non-spec literal `Patient.group.reference` match. Tests: - New `test_run_view_definition_appointment_compartment_runner` in crates/rest/tests/sof_run.rs — exercises Appointment.participant.actor through the SQLite runner end-to-end. The exact case the old SQL filter could not handle. - New `test_pg_appointment_compartment_runner` in crates/persistence/tests/sof_pg_runner.rs — same for Postgres (testcontainers). - Update `create_test_server_with_indb` in REST tests to point `data_dir` at the workspace `data/` so the search_index is populated with the real SearchParameter set (the new EXISTS filter depends on it). - Rewrite `test_run_view_definition_group_filter` to seed a real Group with `member.entity = Patient/p-grouped` (the old test seeded a non-spec `Patient.group.reference` field that only worked because of the old hardcoded JSON-path filter).
#7) #6 — sof-server's router only published the type-level URL (`/ViewDefinition/$viewdefinition-run`). The SoF v2 spec lists three valid endpoints (system, type, instance); HFS REST already wired all three. This brings sof-server in line: - `POST/GET /$viewdefinition-run` — new system-level alias routes to the same handler as the type-level URL. - `POST/GET /ViewDefinition/$viewdefinition-run` — unchanged. - `POST/GET /ViewDefinition/{id}/$viewdefinition-run` — new route that always returns a clear 400 (see #7). #7 — sof-server is stateless, so the instance-level form (which infers the ViewDefinition from a stored `{id}`) has no meaning. Previously the URL fell through to a routing 404 or returned a misleading 501. Now: - New `instance_level_not_supported` handler returns `400 Bad Request` with an OperationOutcome explaining the stateless limitation and pointing at the supported alternative (inline `viewResource` via POST). - CapabilityStatement's `operation.documentation` enumerates the supported scopes (system + type) and explicitly notes that `viewReference` and instance-level invocation are unavailable. Output formats (`application/json`, `application/x-ndjson`, `text/csv`, `application/octet-stream`) are also listed correctly in `format` (partial closeout of audit item #11). Tests: - `test_capability_statement_structure` (in-handler unit) extended to assert the documentation mentions both scopes + viewResource, and that all four output formats appear in `format`. - `test_instance_level_returns_bad_request` (in-handler unit) confirms the new handler returns `BadRequest` with the stateless explanation and the alternative pointer. - `test_system_level_route_runs_view_definition` (integration) sends a full Parameters body to `POST /$viewdefinition-run` and asserts a 200 NDJSON response with the seeded row. - `test_instance_level_returns_400_with_stateless_explanation` (integration) hits `POST /ViewDefinition/some-id/$viewdefinition-run` and asserts a 400 OperationOutcome with the right text.
The SoF v2 spec's content-negotiation table lists `application/octet-stream`
as the Parquet response MIME. sof-server was returning the
non-standard `application/parquet` on every Parquet response path. HFS
REST already emits the spec value; this brings sof-server in line.
- All three sof-server Parquet response paths
(`crates/sof/src/handlers.rs` small-file direct return,
`crates/sof/src/handlers.rs` standard processing, and
`crates/sof/src/streaming.rs::stream_single_parquet_response` for
large streaming) now emit:
Content-Type: application/octet-stream
Content-Disposition: attachment; filename="output.parquet"
Matches the HFS REST byte-for-byte. (The multi-file ZIP wrapper
already used `application/zip` and is unchanged.)
- `ContentType::from_string` accepts `application/octet-stream` as the
spec input value for the `_format` parameter; `application/parquet`
is kept as a permissive alias for back-compat with clients that
still send the old shape.
- Doc-comment headers in handlers.rs and server.rs updated to list the
spec MIME types.
Test:
- `test_parquet_response_uses_octet_stream_content_type` (integration)
asserts the response Content-Type is `application/octet-stream`, the
Content-Disposition names a `.parquet` file, and the body starts
with the PAR1 magic.
Per the SoF v2 spec, "invalid ViewDefinition or processing failure" maps to 422 Unprocessable Entity. HFS REST already does this. sof-server had a stray special case in `parse_view_definition_for_version` that mapped `SofError::InvalidViewDefinition → ServerError::BadRequest` (400), out of sync with the rest of the run pipeline (which already returned 422 via the default `From<SofError>` impl). - Remove the special case in `parse_view_definition_for_version`; `?`-conversion via `From<SofError>` routes through `ProcessingError` → 422. - Update the common.rs test stub's serde-deserialize-error mapping to match (was 400; now 422). Tests: - `test_invalid_view_definition_maps_to_422` (in-handler unit) feeds a type-mismatched select to `parse_view_definition_for_version`, confirms the returned ServerError is `ProcessingError`, and renders it via `into_response` to assert the HTTP status is 422. - `test_invalid_view_definition_returns_422` (integration) POSTs the same bad body through the full handler and asserts the response status is 422 with an OperationOutcome.
Before this commit, sof-server enforced a `1..=10000` `_limit` cap (`crates/sof/src/models.rs::validate_query_params`) but HFS REST had no such validation, so the same `_limit=20000` request was accepted on one binary and rejected on the other. - Add `validate_limit` in `crates/rest/src/handlers/sof/run.rs` with the same bounds and error messages as sof-server. Called from `execute_view` after format validation, before either path (`execute_view_inline` or the in-DB runner). - The cap remains a deployment-policy safety net (the spec leaves `_limit` unbounded); the value is now shared between binaries so raising or removing it is a coordinated change. Tests: - `test_run_view_definition_limit_zero_returns_400` — `?_limit=0` rejected with 400 + "greater than 0" message (matches sof-server). - `test_run_view_definition_limit_exceeds_cap_returns_400` — `?_limit=10001` rejected with 400 + "cannot exceed 10000" message (matches sof-server).
The CapabilityStatement-formats half of audit item #11 was already closed in the audit #6/#7 sweep — `/metadata` now lists all four MIME types sof-server actually serves. This commit closes the remaining half: the SoF v2 `$sql-on-fhir-capabilities` endpoint that publishes truthful supports* flags so clients can negotiate without trial and error. - New `GET /$sql-on-fhir-capabilities` route wired in `server.rs`, backed by `sof_capabilities` handler in `handlers.rs`. - Returns a `Parameters` resource matching HFS REST's shape so clients can use the same response decoder against either binary. For a stateless server the truthful values are: supportsViewDefinitionRun = true supportsViewDefinitionExport = false supportsSqlQueryRun = false supportsInDbRunner = false supportsRelativeReference = false supportsCanonicalReference = false supportsAbsoluteReference = false supportedFormat = ndjson, json, csv, parquet - Test stub in `common.rs` mirrors the production handler. - Fix collateral test failure: `test_run_view_definition_ndjson_output` was still asserting the pre-audit-#8 `application/ndjson` content-type on the NDJSON response. Updated to the spec value `application/x-ndjson` (production has emitted this since #8). Test: - `test_sof_capabilities_endpoint` asserts the endpoint returns `application/fhir+json`, the right resource shape, every expected boolean (with truthful values for a stateless server), and all four `supportedFormat` codes.
The audit suspected our advertised OperationDefinition canonical URLs might be wrong (standard FHIR convention puts no `$` in an OperationDefinition's `url` field — the `$` should only appear in the invocation path). Verified directly against the published SoF v2 spec on build.fhir.org: - OperationDefinition-ViewDefinitionRun.json → url ends in `$viewdefinition-run` - OperationDefinition-ViewDefinitionExport.json → url ends in `$viewdefinition-export` - OperationDefinition-SQLQueryRun.json → url ends in `$sqlquery-run` The SoF v2 spec deliberately uses the `$`-prefixed canonical URL form (deviates from standard FHIR convention, but that's what's published). Our code in `crates/sof/src/handlers.rs` and `crates/rest/src/handlers/capabilities.rs` already emits these exact strings. No code change required — item #12 closed as verified.
The SoF v2 spec binds `_format` to `https://sql-on-fhir.org/ig/ValueSet/OutputFormatCodes` with `extensible` strength. Both impls already accept the spec codes (csv/ndjson/parquet/json + fhir for $sqlquery-run on HFS REST) plus some permissive MIME aliases, but neither declared the binding in their capability advertisement. Conformance audit tools couldn't discover it without dereferencing the OperationDefinition. - `/$sql-on-fhir-capabilities` on both binaries now publishes an explicit `formatBinding` parameter pointing at the spec ValueSet and naming the `extensible` strength. - Spec strength is `extensible`, so we don't hard-reject unknown codes; the declaration is advisory metadata that audit tools can find without indirection. Tests: - `test_sof_capabilities_endpoint` (sof-server integration) extended to assert the `formatBinding` block, its `valueSet` URI, and its `extensible` strength. - `test_sof_capabilities_declares_format_binding` (HFS REST integration) asserts the same shape on the HFS REST side.
…14) Per the SoF v2 spec, the `header` parameter "applies only when csv output is requested." Spec gives no requirement to reject it on other formats. sof-server's body-parameter branch was returning 400 with "Header parameter only applies to CSV format" — stricter than the spec. - `crates/sof/src/handlers.rs` body-only branch: when `header` is supplied in the body but the negotiated `_format` isn't CSV, leave the validated format untouched (the `header` is advisory; ignore it) instead of erroring. - `crates/sof/tests/common/mod.rs` stub mirrors the lenient behavior. - `parse_content_type` already handled this correctly internally (only applies `header_param` when content-type == text/csv); this removes the early-return that was rejecting before the mapping ran. Tests: - `test_header_parameter_without_format_is_ignored_on_non_csv` (renamed from `test_header_parameter_without_format`) now asserts 200 + JSON response, matching the new lenient behavior. - `test_csv_header_parameter_with_non_csv_format` already expected the lenient behavior and continues to pass.
Before this commit, `execute_view` validated the `_format` string and
then threw away the resulting `ContentType`; downstream
`format_stream` and `execute_view_inline` re-parsed the format string.
`format_stream` used `.expect("format already validated by
execute_view")` to paper over the redundant work — fragile if any
future bug changed the parser branches.
- `execute_view` now keeps the parsed `ContentType` and threads it
through to both the inline path (`execute_view_inline`) and the
runner path (`format_stream`).
- `execute_view_inline` signature simplifies: takes `ContentType`
instead of `format: &str` + `include_header: bool` (the
CsvWithHeader/Csv distinction encodes the header flag already).
- The runner-path NDJSON-vs-buffered branch matches on
`ContentType::NdJson` instead of string-comparing the format string.
- `format_stream` drops the `.expect` — pure render now, with no
parsing.
Pure code-quality cleanup; no spec or behavior changes. 30 sof_run
integration tests pass unchanged.
`helios_sof::run_view_definition_with_options` already applies `_limit` at the structured-row level (via `apply_pagination_to_result`) before serialization. sof-server's handler then called `apply_result_filtering(output, &validated_params)` on the serialized bytes — which re-parsed, re-truncated, and re-serialized to produce identical output. Fragile in three ways: - JSON path re-parsed + re-serialized the entire record array. - NDJSON path re-parsed every line as JSON. - CSV path's line-split truncation assumed no embedded newlines in quoted fields. - Drop the `apply_result_filtering` call in `crates/sof/src/handlers.rs`. - Delete the dead helper chain in `crates/sof/src/models.rs`: `apply_result_filtering`, `apply_json_filtering`, `apply_csv_filtering`, `apply_pagination_to_records`, `apply_pagination_to_lines`. - Remove the two unit tests that exercised those helpers; end-to-end `_limit` behavior is still exercised by the HTTP-layer tests via the row-level pass. Pure code-quality cleanup — no spec or behavior changes. All sof test suites pass unchanged.
Audit pass of $viewdefinition-export against the SQL-on-FHIR v2 OperationDefinition. Three behavioral fixes plus a parameter cleanup, and the audit log migrates from a single-op draft to a shared spec-inconsistencies doc covering run/export/sqlquery-run. - Failed result URL returns 500 (was 200) per spec status-code table; manifest body still carries `status=failed` + OperationOutcome. - Instance-level endpoint rejects any query/body input parameter with 400 — spec scopes every input parameter to system+type only. - Enforce XOR on `view.viewResource` vs `view.viewReference` (was silently preferring `viewResource` when both were supplied). - Remove `_limit` from $viewdefinition-export — not defined in the spec's input table (unlike $viewdefinition-run, which keeps it). docs/spec-audit-viewdefinition-run.md is superseded by spec-inconsistencies.md, which covers all three SoF operations.
Aligns the date lowBoundary/highBoundary cases with FHIR/sql-on-fhir-v2#357, which corrected the expected output type of date.lowBoundary()/highBoundary() from date to dateTime.
Sync repeat-operator test coverage from FHIR/sql-on-fhir-v2#356 and fix the failing "unionAll inside repeat" case: expand_repeat_combinations now applies select.union_all() branches in each child's context after nested selects, matching the handling already present for forEach and top-level select nodes.
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 (FHIR/sql-on-fhir-v2#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 FHIR/sql-on-fhir-v2#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 FHIR/sql-on-fhir-v2#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).
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)