Skip to content
Draft
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
2e003dd
Implement Ignixa FHIRPath provider seam
Copilot Aug 25, 2026
af627ca
Fix Ignixa FHIRPath seam review findings
Copilot Aug 25, 2026
923c7d0
Fix FHIRPath seam regressions
Copilot Aug 25, 2026
90a67d6
Fix final FHIRPath seam review findings
Copilot Aug 25, 2026
471ff45
Cover composite search parameter expressions
Copilot Aug 25, 2026
001c1cf
Apply verified FHIRPath review fixes
Copilot Aug 28, 2026
d6e5a2b
Fix FHIRPath residual review findings
Copilot Aug 28, 2026
537a338
Update FHIRPath seam ADR status
Copilot Aug 28, 2026
66faf8a
Add FHIRPath ingestion A/B workloads
Copilot Aug 28, 2026
35af5ba
Fix ingestion A/B review findings
Copilot Aug 28, 2026
d6be456
Keep E2E comparison labels stable
Copilot Aug 28, 2026
18f0aeb
Fix ingestion performance review findings
Copilot Aug 28, 2026
826c39f
Fix AB runner re-review residuals
Copilot Aug 28, 2026
504363f
Revert ingestion A/B runner changes
Copilot Aug 31, 2026
731a30d
Consolidate Ignixa SDK architecture decision
Copilot Aug 31, 2026
87e0f1f
Remove FHIRPath cache hit lock contention
Copilot Sep 1, 2026
7a53813
Address FHIRPath review and SMART test failure
Copilot Sep 1, 2026
aff792e
Fix SMART count guard and containment rationale
Copilot Sep 1, 2026
a859f86
Document verified SMART indexing delta
Copilot Sep 1, 2026
afd2f43
Update Ignixa SDK to 0.6.73
Copilot Sep 2, 2026
0eecd5a
Add SQL vNext SDK CI lanes
Copilot Sep 2, 2026
592ee86
Fix SQL vNext CI deployment architecture
Copilot Sep 2, 2026
92da037
Validate SQL vNext E2E topology
Copilot Sep 2, 2026
5a300bf
Harden SQL vNext topology validation
Copilot Sep 2, 2026
33b75b7
Improve SQL topology failure diagnostics
Copilot Sep 2, 2026
851c1bd
Fix SQL vNext validation findings
Copilot Sep 2, 2026
b49ed94
Add CI deployment recovery mode
Copilot Sep 2, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<Hl7FhirLegacyVersion>5.11.0</Hl7FhirLegacyVersion>
<DotNetSdkPackageVersion>10.0.10</DotNetSdkPackageVersion>
<OpenIddictPackageVersion>6.2.0</OpenIddictPackageVersion>
<IgnixaPackageVersion>0.0.163</IgnixaPackageVersion>
<IgnixaPackageVersion>0.6.68</IgnixaPackageVersion>
</PropertyGroup>
<!-- SDK Packages -->
<Choose>
Expand Down
2 changes: 1 addition & 1 deletion docs/arch/adr-2607-ignixa-import-phase0.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public enum FhirSdkProvider

- `CoreFeatureConfiguration.FhirSdkProvider` defaults to `Firely`. There is no `Hybrid` mode and no runtime fallback from Ignixa to Firely — shadow comparison is a testing technique, not a production mode. (Superseded in part by [ADR 2608](adr-2608-ignixa-fhirpath-seam.md): this setting became a nested section — `Default`, plus a nullable override per seam — once FHIRPath needed to roll out independently of import. The default, the absence of `Hybrid`, and the no-fallback rule are unchanged.)
- Selecting `Ignixa` means every feature seam already migrated uses its Ignixa implementation; every other seam keeps using Firely until that seam is migrated in its own PR.
- Startup logs the configured provider and the seams it currently controls (`FhirSdkProviderStartupLogger`: `"FHIR SDK provider configured: {FhirSdkProvider}; migrated seams: Import."`), so the global setting never creates a false impression that the whole server has moved.
- Startup logs the default and each effective seam provider (`FhirSdkProviderStartupLogger`: `"FHIR SDK providers configured: Default={DefaultProvider}; Import={ImportProvider}; FHIRPath={FhirPathProvider}. FHIRPath Patch remains Firely-backed."`), so the setting never creates a false impression that the whole server has moved.
- We do not introduce an `IFhirSdkProvider` facade — it would accumulate unrelated serialization, validation, FHIRPath, and persistence responsibilities. Each migrated feature keeps its existing narrow contract (Phase 0 reuses `IImportResourceParser` unchanged) or introduces one narrow contract if none exists.

**Phase 0 migrates only `$import` parsing.**
Expand Down
43 changes: 27 additions & 16 deletions docs/arch/adr-2608-ignixa-fhirpath-seam.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ These are pinned by characterization tests written against the Firely provider b

`FirelyFhirPathProvider` and `FirelyCompiledFhirPath` live in Core beside the interfaces. Core already references `Hl7.Fhir.Base`, which contains the engine, `EvaluationContext`, and `AddFhirExtensions`, so this adds no dependency and avoids touching the four version-specific `*.FirelySdk` projects that Phase 0 created. Final cutover deletes two files.

**The seam owns symbol-table registration.** `FhirModule` currently calls `FhirPathCompiler.DefaultSymbolTable.AddFhirExtensions()` in two places; that global mutation is what puts `resolve()` into the engine. Leaving it there means a provider constructed outside full server startup — exactly what the characterization tests do — cannot compile `resolve()` expressions. `FirelyFhirPathProvider` performs the registration itself, and `FhirModule` drops both calls.
**The seam owns evaluation symbol-table registration.** `FhirModule` previously called `FhirPathCompiler.DefaultSymbolTable.AddFhirExtensions()` in two places; that global mutation is what puts `resolve()` into the engine. Leaving evaluation registration there means a provider constructed outside full server startup — exactly what the characterization tests do — cannot compile `resolve()` expressions. `FirelyFhirPathProvider` therefore performs the guarded, idempotent registration itself, and `FhirModule` drops both calls. Separately, `SearchModule` unconditionally performs the same guarded registration at the composition root, including in Ignixa mode, because FHIRPath Patch remains Firely-backed until Phase 7.

### Provider selection

Expand All @@ -51,28 +51,39 @@ The provider is a process-wide ambient, following the pattern `ModelInfoProvider
```csharp
public static class FhirPathProvider
{
private static Func<IFhirPathProvider> _factory = static () => new FirelyFhirPathProvider();
private static Lazy<IFhirPathProvider> _instance = new(() => _factory());
private static Lazy<IFhirPathProvider> _instance = CreateLazy(static () => new FirelyFhirPathProvider());

public static IFhirPathProvider Instance => _instance.Value;
public static IFhirPathProvider Instance => Volatile.Read(ref _instance).Value;

public static void SetProviderFactory(Func<IFhirPathProvider> factory)
{
_factory = EnsureArg.IsNotNull(factory, nameof(factory));
_instance = new Lazy<IFhirPathProvider>(() => _factory());
Func<IFhirPathProvider> providerFactory = EnsureArg.IsNotNull(factory, nameof(factory));
Interlocked.Exchange(ref _instance, CreateLazy(providerFactory));
}

private static Lazy<IFhirPathProvider> CreateLazy(Func<IFhirPathProvider> factory)
=> new(
() => factory() ?? throw new InvalidOperationException("The FHIRPath provider factory returned null."),
LazyThreadSafetyMode.ExecutionAndPublication);
}
```

Two properties matter. The default is Firely, so **nothing has to call the setter for current behaviour to hold** — roughly a thousand unit tests that construct converters directly need no fixture change, unlike `ModelInfoProvider`, which throws when unset. And resolution is lazy behind a `Lazy<T>`, so the provider is built after `ModelInfoProvider` is set and two threads cannot race into two expression caches. `SetProviderFactory` replaces the `Lazy`, so a pre-registration read cannot latch Firely permanently.
Two properties matter. The default is Firely, so **nothing has to call the setter for current behaviour to hold** — roughly a thousand unit tests that construct converters directly need no fixture change, unlike `ModelInfoProvider`, which throws when unset. Resolution is lazy behind a `Lazy<T>`, so the provider is built after `ModelInfoProvider` is set and two threads cannot race into two expression caches. `SetProviderFactory` validates and captures the factory, then atomically replaces the lazy generation; readers observe either the complete old generation or the complete new one, so a pre-registration read cannot latch Firely permanently. A factory that returns `null` is rejected when its generation is first resolved.

`SearchModule` is the single composition point — it already takes `FhirServerConfiguration` and owns the feature area, where `FhirModule` is parameterless:

```csharp
FhirPathProvider.SetProviderFactory(
_configuration.CoreFeatures.FhirSdkProvider.EffectiveFhirPath == FhirSdkProvider.Ignixa
? () => new IgnixaFhirPathProvider(new IgnixaSchemaContext(ModelInfoProvider.Instance))
: () => new FirelyFhirPathProvider());
// FHIRPath Patch remains Firely-backed even when evaluation uses Ignixa.
ElementNavFhirExtensions.PrepareFhirSymbolTableFunctions();

Func<IFhirPathProvider> providerFactory = _configuration.CoreFeatures.FhirSdkProvider.EffectiveFhirPath switch
{
FhirSdkProvider.Firely => () => new FirelyFhirPathProvider(),
FhirSdkProvider.Ignixa => () => new IgnixaFhirPathProvider(new IgnixaSchemaContext(ModelInfoProvider.Instance)),
var provider => throw new InvalidOperationException($"Unsupported FHIR SDK provider: {provider}."),
};

FhirPathProvider.SetProviderFactory(providerFactory);

services.AddSingleton<IFhirPathProvider>(_ => FhirPathProvider.Instance);
```
Expand Down Expand Up @@ -107,7 +118,7 @@ FHIRPath changes search index *content*, where import parsing does not, so the t

**Out, and permanently marked so:**

- **FHIRPath Patch.** The six `Operation*.cs` files select nodes and then mutate the returned nodes: `ElementModelExtensions.ToElementNode` is `(element is ElementNode el) ? el : ElementNode.FromElement(element)`, and `OperationDelete` then calls `Target.Parent.Remove(Target)`. Firely returns the input tree's own `ElementNode` instances, so the mutation lands. An Ignixa provider returns adapter-wrapped nodes, fails the type test, receives a **detached copy**, and patches the copy — the operation reports success and the resource is unchanged. Ignixa structurally cannot honour node identity across the adapter boundary. Patch is Phase 7 in ADR 2607 regardless; these files keep `Hl7.FhirPath` and are whitelisted in the seam test with this reason.
- **FHIRPath Patch node selection.** The six `Operation*.cs` files select nodes and then mutate the returned nodes: `ElementModelExtensions.ToElementNode` is `(element is ElementNode el) ? el : ElementNode.FromElement(element)`, and `OperationDelete` then calls `Target.Parent.Remove(Target)`. Firely returns the input tree's own `ElementNode` instances, so the mutation lands. An Ignixa provider returns adapter-wrapped nodes, fails the type test, receives a **detached copy**, and patches the copy — the operation reports success and the resource is unchanged. Ignixa structurally cannot honour node identity across the adapter boundary. Patch is Phase 7 in ADR 2607 regardless; these operation files keep `Hl7.FhirPath` and are whitelisted in the seam test with this reason. `PatchPayload` only compares immutable scalar values, so it uses the provider seam.
- **The three AST consumers.** They perform type inference at definition time, never evaluate against a resource, and porting them to Ignixa's AST plus `FhirPathAnalyzer` is a large visitor rewrite with no runtime benefit. They keep `Hl7.FhirPath` for `FhirPathCompiler` and are whitelisted.
- **`ITypedElement` and `EvaluationContext` stay in the seam signatures.** Both ship in `Hl7.Fhir.Base`, which Core keeps for the element model regardless, so abstracting `EvaluationContext` alone removes no dependency while adding churn. They get replaced together when the element model moves.

Expand All @@ -119,11 +130,11 @@ A test asserts that no file imports `Hl7.FhirPath` outside the Firely provider a

Stated explicitly because three different policies exist today and none is written down: the extension path uses a shared static 500-entry LRU, `TypedElementSearchIndexer` uses a private unbounded dictionary, and Ignixa keeps its own static unbounded AST and delegate caches. Expressions are influenced by user input through custom search parameters, so unbounded caching is a slow leak.

Each provider owns a **bounded** compile cache. `TypedElementSearchIndexer` keeps holding `ICompiledFhirPath` handles in its own dictionary keyed by search parameter — the `Compile`-returns-a-handle shape means the hot path never consults a string-keyed cache at all, which is strictly better than today and independent of the LRU size. R4 alone ships roughly 1,400 search-parameter expressions, so routing them through a 500-entry LRU would thrash every extract cycle.
Each provider owns a **bounded** compile cache sized to hold the generated corpus without thrashing (R4 alone ships roughly 1,400 search-parameter expressions). `TypedElementSearchIndexer` does not retain a second raw-expression cache: each extraction obtains an `ICompiledFhirPath` handle through the selected provider, whose bounded cache is the single cache-policy authority.

### Failure handling

`TypedElementSearchIndexer` catches all exceptions from expression evaluation, logs a warning, and yields an empty index entry set. Ignixa throws `NotSupportedException` for unimplemented functions where Firely would return empty, so that catch is the exact mechanism by which a conformance gap becomes silent index drift. In Ignixa mode, evaluation failure is surfaced as a metric and is a bake-in gate, not a swallowed warning.
`TypedElementSearchIndexer` catches non-cancellation exceptions from expression evaluation, logs a warning, emits a failure metric, and yields an empty index entry set; cancellation continues to propagate. Ignixa throws `NotSupportedException` for unimplemented functions where Firely would return empty, so that catch is the exact mechanism by which a conformance gap becomes silent index drift. In Ignixa mode, the metric is a bake-in gate, not a swallowed warning.

## Status

Expand All @@ -146,12 +157,12 @@ Proposed
- **A process-wide static cannot express per-server configuration.** `TestFhirServerFactory` caches multiple in-process servers, so two in-proc servers configured with different providers cross-contaminate — which is precisely the shape an Ignixa-versus-Firely E2E comparison would take. Constructor injection into `TypedElementSearchIndexer` covers the parity corpus; a genuine per-server FHIRPath provider would require removing the ambient, which in turn requires the ~1,000 direct-`new` converter tests to gain a fixture. Deferred, and the E2E constraint is documented at the static.
- **`$patch` remains Firely-backed until Phase 7** even when the flag says Ignixa. The startup log names the seams the setting actually controls, so this does not silently mislead operators.
- **Reshaping the config node is a breaking change.** An existing scalar `"FhirSdkProvider": "Firely"` binds to the new object type as *nothing*, so an operator who had set `Ignixa` silently reverts to Firely. The direction is fail-safe, and Phase 0's flag is opt-in and unreleased, but the startup log must make the effective values unambiguous.
- **A prerequisite lands in another repository.** The Ignixa adapters passed `Value` through untranslated in both directions, so Firely's `P.DateTime` reached Ignixa's comparison helpers — which narrow operands through a `string`/`DateTime`/`DateTimeOffset` switch and fall through to `null` — turning every date comparison into an empty result instead of a boolean, silently. Fixed in [ignixa-fhir#398](https://github.com/brendankowitz/ignixa-fhir/pull/398); enabling Ignixa in production is blocked on a package release containing it.
- **A prerequisite landed in another repository.** The Ignixa adapters passed `Value` through untranslated in both directions, so Firely's `P.DateTime` reached Ignixa's comparison helpers — which narrow operands through a `string`/`DateTime`/`DateTimeOffset` switch and fall through to `null` — turning every date comparison into an empty result instead of a boolean, silently. Fixed in [ignixa-fhir#398](https://github.com/brendankowitz/ignixa-fhir/pull/398) and released in Ignixa 0.6.68, which this change consumes; date-comparison parity is covered by the provider context-and-resolver tests.

### Neutral Effects

- `%context` is not bound by name in Ignixa's `GetEnvironmentVariable`; it falls through to the generic environment dictionary. The evaluation-context bridge binds it explicitly, along with `%resource` and `%rootResource` and the `ElementResolver` that backs `resolve()` — which appears 76 times across the R4, R4B, and R5 search parameters and is the single highest-risk behaviour in the bridge.
- `TypedElementSearchIndexer` moving onto the seam changes two behaviours that were never deliberate: it gains the `ToScopedNode()` wrap the extension path always applied, and it loses its unbounded private cache. Both are pinned by characterization tests before the move.
- `TypedElementSearchIndexer` moving onto the seam changes two behaviours that were never deliberate: it gains the `ToScopedNode()` wrap the extension path always applied, and its unbounded private cache is replaced by the provider's bounded cache. Both are pinned by characterization tests.
- Custom search parameters are validated through Firely's AST tooling (out of scope) but indexed through Ignixa, so an accept-versus-index mismatch is possible. The parity corpus covers generated parameters; custom-parameter parity is a bake-in observation, not a pre-merge gate.

### Delivery
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,38 @@ public class FhirSdkProviderConfigurationTests
public void GivenDefaultConfiguration_WhenProviderRead_ThenFirelyIsSelected()
{
var configuration = new CoreFeatureConfiguration();
Assert.Equal(FhirSdkProvider.Firely, configuration.FhirSdkProvider);
Assert.Equal(FhirSdkProvider.Firely, configuration.FhirSdkProvider.EffectiveImport);
Assert.Equal(FhirSdkProvider.Firely, configuration.FhirSdkProvider.EffectiveFhirPath);
}

[Fact]
public void GivenIgnixaConfigured_WhenProviderRead_ThenIgnixaIsSelected()
{
var configuration = new CoreFeatureConfiguration
{
FhirSdkProvider = FhirSdkProvider.Ignixa,
FhirSdkProvider = new FhirSdkProviderConfiguration
{
Default = FhirSdkProvider.Ignixa,
},
};
Assert.Equal(FhirSdkProvider.Ignixa, configuration.FhirSdkProvider);
Assert.Equal(FhirSdkProvider.Ignixa, configuration.FhirSdkProvider.EffectiveImport);
Assert.Equal(FhirSdkProvider.Ignixa, configuration.FhirSdkProvider.EffectiveFhirPath);
}

[Fact]
public void GivenSeamOverrides_WhenProvidersRead_ThenSelectionsAreIndependent()
{
var configuration = new CoreFeatureConfiguration
{
FhirSdkProvider = new FhirSdkProviderConfiguration
{
Default = FhirSdkProvider.Firely,
Import = FhirSdkProvider.Ignixa,
},
};

Assert.Equal(FhirSdkProvider.Ignixa, configuration.FhirSdkProvider.EffectiveImport);
Assert.Equal(FhirSdkProvider.Firely, configuration.FhirSdkProvider.EffectiveFhirPath);
}
}
}
Loading
Loading