feat: Add observability, PostgreSQL persistence, and test coverage

[rsalus]

Summary

Adds production-ready infrastructure for the Gateway service: observability logging for data flow visibility, PostgreSQL persistence replacing in-memory stores, and CI test coverage enforcement.

Changes

• Observability — Signal logging in FhirDataAggregator (bundle counts) and EncounterProcessor (data validation flags) without exposing PHI
• EF Core Infrastructure — GatewayDbContext with WorkItemEntity and RegisteredPatientEntity, initial migration, snake_case table naming
• Postgres Stores — PostgresWorkItemStore and PostgresPatientRegistry implementing existing interfaces with EF Core
• DI Registration — AddGatewayPersistence extension method, Aspire integration via AddNpgsqlDbContext
• CI Coverage — coverlet.collector package, 60% threshold enforcement in GitHub Actions

Test Plan

Added ~60 new tests covering all persistence components. Alba integration tests verify data persists across HTTP requests.

---

Results: Tests 352 ✓ · Build 0 errors
Design: docs/designs/2026-02-02-observability-persistence-coverage.md

🤖 Generated with Claude Code

[coderabbitai]

Actionable comments posted: 7

🤖 Fix all issues with AI agents

In `@apps/gateway/Gateway.API/Services/Polling/AthenaPollingService.cs`:
- Around line 119-133: The current startup wait in AthenaPollingService uses
MigrationHealthCheck.WaitForMigrationAsync(nameof(GatewayDbContext),
stoppingToken) which can block indefinitely; change this to a bounded wait by
creating a configurable timeout (e.g., from settings) and using a linked
CancellationTokenSource that combines stoppingToken with a timeout token, call
WaitForMigrationAsync with the linked token, and catch both
OperationCanceledException (when stoppingToken requested) and
TimeoutException/TaskCanceledException to log a clear error (including the
timeout value and context like GatewayDbContext and
MigrationHealthCheck.WaitForMigrationAsync) and fail startup or return
appropriately; ensure the timeout value is configurable and documented and that
you dispose the linked CancellationTokenSource.

In `@docs/designs/2026-02-02-observability-persistence-coverage.md`:
- Around line 217-279: Update the document by replacing all occurrences of
"Github" or "github" with the correct product name "GitHub" (case-sensitive)
throughout the text and headings—e.g., in the workflow reference block and the
"Branch Protection" heading—so every mention uses the capitalized form "GitHub".
- Around line 91-118: The CREATE TABLE blocks for work_items and
registered_patients incorrectly use inline "INDEX" clauses; remove those INDEX
lines from the table definitions and add separate CREATE INDEX statements after
the tables (e.g., create indexes for idx_work_items_encounter and
idx_work_items_status on work_items(encounter_id, status as appropriate) and
idx_registered_patients_registered on registered_patients(registered_at)), or
alternatively annotate the doc to state that EF migrations will create those
indexes instead of inline SQL, ensuring the example uses valid PostgreSQL syntax
and references the work_items and registered_patients tables and the index names
idx_work_items_encounter, idx_work_items_status, and
idx_registered_patients_registered.

In `@docs/plans/2026-02-01-patient-registry-polling.md`:
- Around line 237-388: The plan currently references InMemoryPatientRegistry and
the DI registration line services.AddSingleton<IPatientRegistry,
InMemoryPatientRegistry>() but may be outdated if a Postgres-backed registry is
intended; update the docs (sections mentioning InMemoryPatientRegistry,
AddSingleton, IPatientRegistry and the Integration Points table) to either (a)
explicitly state that InMemoryPatientRegistry is an interim/testing
implementation and outline the later Postgres implementation, or (b) replace
references with the Postgres-backed implementation class and corresponding DI
registration (e.g., register the Postgres implementation instead of
InMemoryPatientRegistry) so the plan accurately reflects the intended
persistence choice.
- Around line 692-754: The fenced ASCII diagram code blocks in the document are
missing language identifiers (MD040); update each triple-backtick block that
contains the execution graph and parallelization diagrams to include a language
tag such as "text" (e.g., change ``` to ```text) so the linter passes; apply the
same change to the other similar fenced blocks referenced in the comment (the
Execution Graph and the Parallelization Strategy ASCII blocks and any other
untagged backtick blocks).
- Around line 29-31: The table separator row currently uses tight pipes like
"|-----------|---------|----------|" which violates MD060; update it to use
spaced separators that match the header spacing (e.g., change to "| --- | --- |
--- |") for the table with header "Component | Current | Required" and make the
same adjustment for the other tables mentioned so all separator rows
consistently include single spaces around pipes.

In `@docs/plans/2026-02-02-observability-persistence-coverage.md`:
- Around line 441-444: Update the REFACTOR step wording to read more clearly and
consistently: change the bullet "Apply: Extract common test fixtures if needed"
to "Apply: Extract common test fixtures, if needed" (or "Apply: Extract common
test fixtures to be run, if needed" if you prefer), and ensure the next line is
consistently formatted as "Run: `dotnet test` - MUST STAY GREEN" (separate
sentence/line starting with "Run:") so the phrase "needed Run" is no longer
adjacent and the instruction flows correctly.

🧹 Nitpick comments (5)

docs/integration/testing-in-athena-sandbox.md (1)

61-63: Add language specifiers to fenced code blocks.

The HTTP request examples would benefit from language specifiers for better syntax highlighting and markdown compliance.

📝 Proposed fix to add language specifiers

 **FHIR R4:**
 
 HTTP
 
-```
+```http
 GET https://api.preview.platform.athenahealth.com/fhir/r4/Patient?ah-practice=Organization/a-1.Practice-195900&name=Sandboxtest

athenaOne:

HTTP

- +http
GET https://api.preview.platform.athenahealth.com/v1/195900/patients?lastname=Sandboxtest
GET https://api.preview.platform.athenahealth.com/v1/195900/patients?lastname=Sandbox-Test

Also applies to: 69-72

docs/designs/2026-02-02-observability-persistence-coverage.md (1)

123-136: Add a language tag to the fenced block.

The tree snippet is missing a language identifier, which markdownlint flags. Use ```text for clarity.

docs/plans/2026-02-02-observability-persistence-coverage.md (1)

481-489: Add a language tag to the diagram block.

The ASCII diagram lacks a fenced code language; use ```text to satisfy markdownlint and improve readability.

apps/gateway/Gateway.API.Tests/Data/GatewayDbContextTests.cs (1)

17-18: Consider sealing the test class to match “sealed by default.”

This aligns with the project’s C# standard while keeping the test surface tight.

Suggested change

-public class GatewayDbContextTests
+public sealed class GatewayDbContextTests

As per coding guidelines: apps/gateway/**/*.cs: Types: Sealed by default.

apps/gateway/Gateway.API/Data/MigrationHealthCheck.cs (1)

15-39: Seal the health check class and replace arrow-bodied members.
This file deviates from the repo’s C# standards (sealed-by-default and no arrow code).

🔧 Suggested update

-public class MigrationHealthCheck : IHealthCheck
+public sealed class MigrationHealthCheck : IHealthCheck
 {
     private static readonly ConcurrentDictionary<string, bool> s_completedMigrations = new();

@@
-    public static void MarkComplete(string contextName) =>
-        s_completedMigrations[contextName] = true;
+    public static void MarkComplete(string contextName)
+    {
+        s_completedMigrations[contextName] = true;
+    }
@@
-    public static void RegisterExpected(string contextName) =>
-        s_completedMigrations[contextName] = false;
+    public static void RegisterExpected(string contextName)
+    {
+        s_completedMigrations[contextName] = false;
+    }
@@
-    public static bool IsComplete(string contextName) =>
-        s_completedMigrations.TryGetValue(contextName, out var complete) && complete;
+    public static bool IsComplete(string contextName)
+    {
+        return s_completedMigrations.TryGetValue(contextName, out var complete) && complete;
+    }

As per coding guidelines: apps/gateway/**/*.cs: Sealed by default, ... no arrow code.

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@docs/designs/2026-02-02-observability-persistence-coverage.md`:
- Around line 125-138: The fenced file-tree block in the design doc is missing a
language tag (MD040); update the code fence surrounding the tree (the
triple-backtick block that contains "Gateway.API/" and its nested files like
GatewayDbContext.cs, PostgresWorkItemStore.cs, and PostgresPatientRegistry.cs)
to include a language label (for example "text") so it becomes ```text ... ```,
ensuring the linter no longer flags the missing language.

In `@docs/plans/2026-02-02-observability-persistence-coverage.md`:
- Around line 481-489: The fenced ASCII diagram block is currently untyped which
triggers MD040; update the fenced code block that begins with the diagram
containing "Group A (parallel):          Group B (sequential):" to include a
language identifier (e.g., add "text" directly after the opening ``` fence) so
the block becomes a typed code fence (```text) and satisfies the linter while
preserving the ASCII art.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In
`@apps/gateway/Gateway.API/Data/Migrations/20260203015037_IncreaseWorkItemIdLength.Designer.cs`:
- Around line 54-57: RegisteredPatientEntity.WorkItemId is still configured with
HasMaxLength(32) and character varying(32) which will truncate WorkItem.Id
values; create a follow-up migration that alters registered_patients.WorkItemId
to length 36 and update the EF model configuration for
RegisteredPatientEntity.WorkItemId to .HasMaxLength(36) and
.HasColumnType("character varying(36)"), ensuring the migration class (e.g.,
IncreaseWorkItemIdLength follow-up) performs the ALTER COLUMN/AlterColumn
operation and that the Designer.cs (or Fluent API on RegisteredPatientEntity)
matches WorkItem.Id's length to prevent mismatches.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In @.github/workflows/ci.yml:
- Around line 141-152: In the "Test with coverage" step, update the dotnet run
invocation for apps/gateway/Gateway.API.Tests/Gateway.API.Tests.csproj to
include the --no-build flag (e.g., dotnet run --no-build --project ...),
preserving the existing --configuration Release and all coverage arguments (--
--coverage, --coverage-output-format cobertura, --coverage-output,
--coverage-settings) so the prior build isn't repeated; modify the command
inside that job where dotnet run is invoked.

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@apps/gateway/Gateway.API/Endpoints/FhirEndpoints.cs`:
- Around line 103-110: The SearchEncountersAsync method builds a query using
patientId without URL-encoding, which can break searches for edge-case IDs;
update SearchEncountersAsync to URL-encode patientId before calling BuildQuery
(e.g., encode patientId passed into BuildQuery) so the query string is safe,
similar to how patient search was fixed—make the change in SearchEncountersAsync
where patientId is used and ensure BuildQuery receives the encoded value.
- Around line 54-61: SearchPatientsAsync interpolates the raw name into
BuildQuery causing malformed URLs for special characters; URL-encode the name
before building the query (e.g., call a URL-encoding function on the name
parameter prior to using BuildQuery) so the query string passed to
fhirClient.SearchAsync is safe; update the SearchPatientsAsync flow to encode
the name variable (and handle null/empty appropriately) before calling
BuildQuery/SearchAsync.

🧹 Nitpick comments (2)

apps/gateway/Gateway.API/Endpoints/FhirEndpoints.cs (1)

120-125: Consider URL-encoding practiceId for completeness.

While config-provided values are typically safe, encoding practiceId would make the helper robust against unexpected configuration values.

♻️ Suggested improvement

-            : $"{baseQuery}&ah-practice={practiceId}";
+            : $"{baseQuery}&ah-practice={WebUtility.UrlEncode(practiceId)}";

apps/gateway/Gateway.API.Tests/Endpoints/FhirEndpointsTests.cs (1)

68-98: Missing test for GetPatientAsync non-404 errors.

The endpoint returns ProblemHttpResult for errors other than NOT_FOUND, but there's no test covering this path (e.g., network failure or unauthorized).

🧪 Additional test case

[Test]
public async Task GetPatientAsync_WhenFhirFails_ReturnsProblem()
{
    // Arrange
    var error = FhirError.Network("Connection refused");
    _fhirClient.ReadAsync("Patient", "123", Arg.Any<CancellationToken>())
        .Returns(Result<JsonElement>.Failure(error));

    // Act
    var result = await FhirEndpoints.GetPatientAsync("123", _fhirClient);

    // Assert
    await Assert.That(result.Result).IsTypeOf<ProblemHttpResult>();
}

[coderabbitai]

Actionable comments posted: 3

🤖 Fix all issues with AI agents

In @.github/workflows/ci.yml:
- Around line 154-170: The workflow uses
hashFiles('apps/gateway/coverage/coverage.cobertura.xml') in the step-level if,
which only evaluates checked-in files and will skip the "Check Coverage
Threshold" and "Upload coverage" steps for generated coverage; change the logic
to perform a runtime existence check inside the steps (or propagate an output
from the test step) so the steps run conditionally at execution time: remove or
replace the step-level if using hashFiles and instead in the "Check Coverage
Threshold" step test for the file (e.g., check
./apps/gateway/coverage/coverage.cobertura.xml exists before computing
COVERAGE/COVERAGE_PCT) and similarly in the "Upload coverage" step ensure the
file exists at runtime before calling actions/upload-artifact; keep references
to the same step names ("Check Coverage Threshold", "Upload coverage") and the
COVERAGE/COVERAGE_PCT logic so the threshold check still occurs when the file is
present.

In
`@apps/gateway/Gateway.API/Data/Migrations/20260203030440_IncreaseRegisteredPatientWorkItemIdLength.cs`:
- Around line 25-35: The Down migration in
IncreaseRegisteredPatientWorkItemIdLength can fail if any
registered_patients.WorkItemId is longer than 32; update the Down method to
guard or truncate those values before calling AlterColumn (e.g., use
migrationBuilder.Sql to truncate WorkItemId to 32 chars for rows where
length(WorkItemId) > 32 or add a conditional check that aborts the downgrade
with a clear message). Ensure the change runs before the
migrationBuilder.AlterColumn call in the Down method so the ALTER never
encounters too-long values.

In `@docs/designs/2026-02-02-observability-persistence-coverage.md`:
- Around line 91-117: The SQL schema uses VARCHAR(32) for the WorkItem ID
columns which is inconsistent with the model/migrations that use length 36;
update the CREATE TABLE definitions to use VARCHAR(36) for work_items.id and
registered_patients.work_item_id (and any related index or FK definitions) so
the example matches the model configuration and migrations.

🧹 Nitpick comments (1)

apps/gateway/Gateway.API/Data/Migrations/20260203030440_IncreaseRegisteredPatientWorkItemIdLength.cs (1)

3-3: Clarify the nullable directive with EF Core conventions.

The project enables nullable context (<Nullable>enable</Nullable>), but this migration opts out with #nullable disable. While this is EF Core's default convention for generated migrations, consider whether the team should align this with the project standard. If migrations are regenerated by tooling, keep #nullable disable to prevent warnings; otherwise, #nullable enable aligns with repo policy.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In @.github/workflows/ci.yml:
- Around line 153-167: In the "Check Coverage Threshold" step the parsing of
line-rate into COVERAGE/COVERAGE_PCT can silently fail if the XML is missing or
malformatted; add an explicit guard after extracting COVERAGE to test that it's
non-empty and numeric (e.g., regex check) and if not, emit a clear error like
"Coverage value not found or unparseable" and exit 1 before computing
COVERAGE_PCT or comparing to env.COVERAGE_THRESHOLD so downstream bc/cut
operations won't produce cryptic failures.

[rsalus]

@coderabbitai Approve this PR.

[coderabbitai]

@rsalus Approving the PR now.

✅ Actions performed

Comments resolved and changes approved.