Trent/vigilcare-clinical
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
0d0bba19e618a3d3f40f566fe244275de134f385
vigilcare-clinical/docs/clinical-platform-gap-analysis.md
T

41 KiB
Raw Blame History

VigilCare Clinical Platform — Gap Analysis

Comprehensive gap analysis of the VigilCareClinical system covering data integrity, API surface, infrastructure reliability, security posture, observability, and test coverage. Items are ordered by impact on correctness and patient safety first, then operational reliability, then API completeness, then observability and polish.

Each item includes why it matters and how to fix it at an implementation-ready level.

Priority legend

Tier Meaning
P0 Data integrity or clinical correctness bug; fix before expanding clinical workflows
P1 Silent misconfiguration, transaction safety hole, or data loss risk; fix soon after P0
P2 Blocks common admin/integration workflows or degrades operational reliability
P3 Security hardening, compliance, or defense-in-depth; no immediate patient-safety impact
P4 API completeness, consistency, and developer experience
P5 Observability and test coverage; does not change clinical outcomes but makes incidents diagnosable

Part A — Data Integrity & Correctness

P0 — MRN generation race condition

Problem

PatientService.RegisterAsync generates MRNs via MRN-{count+1:D6} where count is a SELECT COUNT(*). Two concurrent registrations can read the same count and generate duplicate MRNs. The unique index on Patient.Mrn catches this at the database level, but the exception surfaces as an unhandled DbUpdateException, not a controlled retry or user-friendly error.

Why fix

MRN is the primary patient identifier across clinical systems. Duplicate MRN attempts that surface as 500 errors during FHIR bulk-import or concurrent admissions will halt ingest pipelines and require manual intervention.

How to fix

  1. Replace count-based generation with a PostgreSQL sequence: CREATE SEQUENCE mrn_seq START WITH 1 INCREMENT BY 1.
  2. In PatientService.RegisterAsync, call SELECT nextval('mrn_seq') to get the next MRN atomically.
  3. Format as MRN-{sequence:D6}.
  4. Extract MRN prefix/format to PatientOptions for configurability.
  5. Handle DbUpdateException with unique violation check as a fallback (retry once with next sequence value).

Files: PatientService.cs:197-200, new migration for mrn_seq, optional PatientOptions.cs.

Dependency: None.

P0 — Sepsis bundle creation race condition (TOCTOU)

Problem

SepsisBundleService.CreateAsync checks AnyAsync(b => b.EncounterId == encounterId && b.ComplianceStatus == InProgress) before inserting a new bundle. Two SOFA_SEPSIS alerts arriving simultaneously for the same encounter can both pass this check and create duplicate bundles, resulting in duplicate sepsis bundle elements and compliance tracking.

Why fix

Duplicate bundles for the same sepsis episode create conflicting compliance timelines, confuse clinician dashboards, and may trigger duplicate paging/escalation workflows. In a clinical setting this means duplicate nurse pages for the same patient.

How to fix

  1. Replace AnyAsync check with an idempotent INSERT pattern matching the approach used for alert creation: 
    INSERT INTO sepsis_bundles (...)
SELECT ... WHERE NOT EXISTS (
  SELECT 1 FROM sepsis_bundles
  WHERE encounter_id = @encounterId AND compliance_status = 'IN_PROGRESS'
)
  2. Check rowsAffected == 0 to detect concurrent creation; return existing bundle instead of creating a new one.
  3. Wrap bundle + elements creation in a single transaction with SERIALIZABLE isolation or use FOR UPDATE on the encounter row.

Files: SepsisBundleService.cs:24-29, SepsisBundleConfiguration.cs (add unique filtered index on (encounter_id) WHERE compliance_status = 'IN_PROGRESS').

Dependency: None.

P0 — Trend alert matching uses fragile LIKE pattern

Problem

TrendDetector.TryCreateAlertAsync uses LIKE '%{observationCode}%' to check for existing open trend alerts. The pattern %HEART_RATE% could match a hypothetical HEART_RATE_VARIABILITY alert, and %TEMP% could match TEMP_C and TEMP_F. This bypasses deduplication and creates spurious alerts, or worse, suppresses alerts for the wrong vital sign.

Why fix

Trend alerts fire for the 5 most critical vitals (HR, RR, SBP, Temp, SpO2). False suppression means a rapid deterioration goes unnotified; false creation means alert fatigue on a clinical floor.

How to fix

  1. Change the deduplication query to use exact match on a structured field rather than LIKE on the Details text column.
  2. Option A: Add an ObservationCode column to ClinicalAlert (nullable, indexed) and match on it directly.
  3. Option B: Use Details LIKE 'Rapid deterioration: {observationCode} %' with a prefix match instead of substring.
  4. Prefer Option A — it also benefits analytics queries that currently parse alert details text.

Files: TrendDetector.cs:102-113, ClinicalAlert.cs (optional new column), ClinicalAlertConfiguration.cs, migration.

Dependency: None.

P1 — Order result → sepsis bundle update lacks spanning transaction

Problem

OrderService.RecordResultAsync updates the order status to Resulted, then calls SepsisBundleService.OnOrderResultedAsync as a separate operation. If the bundle update fails (e.g., database timeout), the order is marked as resulted but the bundle element remains Pending. The bundle may then be incorrectly marked NonCompliant by SepsisBundleMonitorService even though the order was completed on time.

Why fix

Sepsis bundle compliance is a CMS/Joint Commission quality metric. A false NonCompliant due to a transient failure triggers incorrect escalation and skews compliance reporting.

How to fix

  1. Wrap both operations in a single IDbContextTransaction: 
    using var tx = await _db.Database.BeginTransactionAsync();
// update order status
// call bundle service
await tx.CommitAsync();
  2. If OnOrderResultedAsync fails, the entire transaction rolls back — order stays in previous state for retry.
  3. Add explicit error logging when bundle element is not found for an order (currently silent no-op at SepsisBundleService:120).

Files: OrderService.cs:100-120, SepsisBundleService.cs:114-162.

Dependency: None.

P1 — FHIR bundle processing has no rollback on partial failure

Problem

FhirBundleProcessor processes transaction bundles by iterating entries and calling individual service methods (patient upsert, encounter upsert, observation ingest). If entry 3 of 5 fails, entries 1-2 are already persisted. FHIR R4 transaction semantics require all-or-nothing: either all entries succeed or none do.

Why fix

EHR integration engines (Mirth, Rhapsody) send transaction bundles expecting atomic semantics. Partial writes create orphaned records — an encounter without its patient, observations without their encounter — that break referential integrity assumptions downstream.

How to fix

  1. Wrap the entire bundle processing loop in a single IDbContextTransaction.
  2. On any entry failure, roll back the transaction and return a FHIR OperationOutcome with per-entry diagnostics.
  3. Collect outbox events during processing but only write them after successful commit.
  4. Add a batch mode (non-atomic, per-entry results) as a separate code path if needed.

Files: FhirBundleProcessor.cs:60-80, FhirIngestController.cs:186-192.

Dependency: None.

Part B — Infrastructure & Reliability

P1 — Kafka replication factor hardcoded to 1

Problem

KafkaTopicProvisioner creates all topics with ReplicationFactor = 1. A single broker failure loses all unconsumed messages on those topics — including alert.generated, observation.recorded, and sepsis.bundle.created.

Why fix

Clinical alert delivery is safety-critical. Losing alert.generated messages means nurses are not paged for critical vitals. Losing observation.recorded means scoring services miss data points, potentially delaying sepsis detection.

How to fix

  1. Make replication factor configurable via KafkaTopicOptions.ReplicationFactor (default 3 for production, 1 for dev/test).
  2. Add MinInSyncReplicas to topic config (recommended: 2 with RF=3).
  3. Validate on startup: if ReplicationFactor > broker count, log a warning and fall back to broker count.
  4. Update docker-compose with a comment noting RF=1 is dev-only.

Files: KafkaTopicProvisioner.cs:38, KafkaTopicOptions.cs, appsettings.json, appsettings.Development.json.

Dependency: None.

P2 — No health check endpoints

Problem

The API has no /health or /ready endpoints. There is no startup probe, no liveness check, and no readiness check for any dependency (PostgreSQL, Redis, Kafka, RabbitMQ, Elasticsearch, MinIO).

Why fix

Without health checks: Kubernetes/container orchestrators cannot detect unhealthy instances and route traffic away. Load balancers send requests to instances with dead database connections. Monitoring systems cannot distinguish "service down" from "service unhealthy."

How to fix

  1. Add Microsoft.Extensions.Diagnostics.HealthChecks and provider packages:
    • AspNetCore.HealthChecks.NpgSql (PostgreSQL)
    • AspNetCore.HealthChecks.Redis (Redis)
    • AspNetCore.HealthChecks.Kafka (Kafka)
    • AspNetCore.HealthChecks.RabbitMQ (RabbitMQ)
    • AspNetCore.HealthChecks.Elasticsearch (Elasticsearch)
  2. Register health checks in Program.cs with tags: startup, liveness, readiness.
  3. Map endpoints:
    • GET /health/live — liveness (is the process alive?)
    • GET /health/ready — readiness (are all dependencies reachable?)
    • GET /health/startup — startup (has initial provisioning completed?)
  4. Expose health check results to Prometheus via AspNetCore.HealthChecks.Publisher.Prometheus.

Files: Program.cs, VigilCareClinicalAPI.csproj (new packages), optional HealthChecksConfiguration.cs.

Dependency: None.

P2 — Kafka consumer poison pill causes infinite retry

Problem

All 7 Kafka consumer services (SepsisEngine, News2Scoring, GcsScoring, TrendAnalyzer, WarningAlert, SofaScoring, EsIndexer) share the same error handling pattern: on exception, log error, delay 2000ms, retry. A malformed message (corrupt JSON, unknown observation code causing unhandled exception) will block the consumer indefinitely — no other messages on that partition are processed.

Why fix

A single bad observation record from a misconfigured device or FHIR integration halts all downstream scoring for that partition. NEWS2, SOFA, qSOFA, and trend alerts stop computing for all patients whose observations land on the blocked partition.

How to fix

  1. Add a retry counter per message (track in memory or via Kafka headers).
  2. After MaxRetries (configurable, default 3), log at Error level with full message payload and commit the offset to skip the poison pill.
  3. Optionally publish to a dead-letter topic ({topic}.dlq) for manual replay.
  4. Add a Prometheus counter kafka_consumer_poison_pills_total{consumer_group, topic}.

Files: All consumer services in BackgroundServices/: SepsisEngineService.cs, News2ScoringService.cs, GcsScoringService.cs, TrendAnalyzerService.cs, WarningAlertService.cs, SofaScoringService.cs, EsIndexerService.cs. Extract shared retry logic to a KafkaConsumerBase<T> helper.

Dependency: None.

P2 — Outbox relay has no dead-letter or max retry limit

Problem

OutboxRelayService retries failed publishes every 1000ms with no maximum retry count and no dead-letter mechanism. If Kafka is down for an extended period, the outbox table grows unbounded. When Kafka recovers, a flood of stale events may overwhelm consumers.

Why fix

Extended Kafka outages are common during upgrades or broker failures. Unbounded outbox growth degrades PostgreSQL query performance (the unprocessed-events index grows). Stale clinical alerts published hours late may trigger incorrect escalations.

How to fix

  1. Add MaxRetryCount and RetryBackoffMs to outbox configuration.
  2. Add a retry_count and last_error column to OutboxEvent.
  3. After MaxRetryCount exceeded, mark event as FAILED (new status column or nullable FailedAt timestamp).
  4. Add backoff: delay = min(RetryBackoffMs * 2^retryCount, MaxBackoffMs).
  5. Add GET /api/v1/ops/outbox?status=failed admin endpoint for manual inspection/replay.
  6. Prometheus metrics: outbox_pending_total, outbox_failed_total.

Files: OutboxRelayService.cs:58-139, OutboxEvent.cs, OutboxEventConfiguration.cs, migration, appsettings.json.

Dependency: None.

P2 — ThresholdCacheLoader crashes startup on Redis failure

Problem

ThresholdCacheLoader runs once at startup and loads all alert thresholds into Redis. If Redis is unavailable, the service throws an unhandled exception, which may crash the entire application depending on host configuration. There is no retry logic.

Why fix

Redis restarts during deployment are common. A transient Redis blip at exactly the wrong moment prevents the entire clinical API from starting, even though Redis will be available seconds later.

How to fix

  1. Wrap the Redis write loop in a retry with exponential backoff (3 attempts, 2s/4s/8s).
  2. On final failure, log at Error level but allow the application to start — the observation ingest pipeline already has a Redis-miss fallback that loads thresholds from PostgreSQL.
  3. Optionally add a background retry that re-attempts cache population after 30 seconds.

Files: ThresholdCacheLoader.cs.

Dependency: None.

P2 — DataLake writer partial commit inconsistency

Problem

DataLakeWriterService flushes Parquet files per partition. If 5 of 6 partitions flush successfully but one fails, the service commits Kafka offsets for the 5 successful partitions and clears their buffers. The failed partition's buffer is also cleared (line 176) even though its data was not written to MinIO. Those events are lost — they won't be re-consumed because the surrounding offsets advanced.

Why fix

Data lake completeness is essential for clinical analytics, research datasets, and regulatory reporting. Silently dropped observations create gaps in longitudinal patient records.

How to fix

  1. Do not clear buffers on flush failure: only clear the buffer for partitions that flushed successfully.
  2. Do not commit offsets for failed partitions: track per-partition flush success and only commit offsets for successful ones.
  3. Add a retry counter per partition buffer; after MaxFlushRetries, log at Error with partition/offset range and clear (accept data loss with explicit audit trail) or halt the consumer for that partition.
  4. Prometheus metric: datalake_flush_failures_total{topic, partition}.

Files: DataLakeWriterService.cs:144-176, DataLakeOptions.cs.

Dependency: None.

Part C — API Completeness & Consistency

P2 — Missing input validators for 4 request types

Problem

Four request types used by controllers have no FluentValidation validator:

  1. TransitionStatusRequest (encounter status changes) — no validation of DischargeDiagnosis length.
  2. RecordOrderResultRequest (order results) — no validation of ResultSummary length or content.
  3. FhirPatientUpsertRequest — no validation of FHIR-mapped fields before database write.
  4. FhirEncounterUpsertRequest — no validation of department/type enum mappings.

The existing 9 validators cover other request types thoroughly.

Why fix

Unvalidated inputs can cause database constraint violations that surface as 500 errors instead of 422s. FHIR upsert requests from integration engines may contain malformed data that is difficult to debug without validation error messages.

How to fix

  1. Create TransitionStatusRequestValidator: validate DischargeDiagnosis max length (500), NewStatus is valid enum.
  2. Create RecordOrderResultRequestValidator: validate ResultSummary max length, non-empty.
  3. Create FhirPatientUpsertRequestValidator: validate identifier system/value presence, gender mapping.
  4. Create FhirEncounterUpsertRequestValidator: validate class mapping, department code mapping, period dates.
  5. Register all in DI (auto-registration via FluentValidation.DependencyInjectionExtensions if not already configured).

Files: New files in Validators/, Program.cs (DI registration if needed).

Dependency: None.

P4 — No patient update endpoint

Problem

PatientsController has POST (register) but no PUT/PATCH. Patient demographics (blood type, allergies, emergency contact, name corrections) cannot be updated without direct database access.

Why fix

Patient data corrections are a daily workflow. Allergies discovered during an encounter, emergency contact changes, and name typos all require update capability. FHIR upsert handles external system updates, but internal admin workflows have no path.

How to fix

  1. Add UpdatePatientRequest record with optional fields: firstName, lastName, dateOfBirth, gender, bloodType, allergies, emergencyContactName, emergencyContactPhone.
  2. Add UpdatePatientRequestValidator (same rules as registration, all fields optional).
  3. Add PatientService.UpdateAsync(Guid id, UpdatePatientRequest) — load, apply non-null fields, save.
  4. Add PATCH /api/v1/patients/{id} with [AuthorizePermission(PatientsWrite)].
  5. Emit ClinicalAuditLog entry with before/after JSON.

Files: PatientsController.cs, PatientService.cs, new UpdatePatientRequest.cs, new UpdatePatientRequestValidator.cs.

Dependency: None.

P4 — Pagination inconsistencies across list endpoints

Problem

List endpoints use three different pagination strategies:

  • 1-based page/pageSize (most controllers): page=1, pageSize=20
  • 0-based page (AnalyticsController.PatientSearch): page=0
  • Cursor-based (SOFA, NEWS2, Observations): varying default limits (20, 20, 50)

AlertThresholdsController.List() has no pagination at all — returns every threshold in one response. No endpoint supports sorting parameters.

Why fix

Inconsistent pagination confuses integrators and dashboard developers. Missing pagination on thresholds is fine today (small dataset) but will break if observation codes expand. Missing sort parameters force client-side sorting.

How to fix

  1. Standardize page-based endpoints to 1-based pagination with consistent defaults (page=1, pageSize=20, maxPageSize=100).
  2. Fix AnalyticsController.PatientSearch to use 1-based pagination (breaking change — document in release notes).
  3. Standardize cursor-based endpoints to a consistent default limit (20).
  4. Add pagination to AlertThresholdsController.List() (or document that the dataset is bounded and pagination is unnecessary).
  5. Add optional sortBy and sortDirection query parameters to list endpoints where ordering matters (alerts, observations, encounters).

Files: AnalyticsController.cs:103, AlertThresholdsController.cs:37-44, ObservationsController.cs:80, all list endpoints for sort params.

Dependency: None.

P4 — Missing list/get-by-id endpoints

Problem

Several resources lack expected REST endpoints:

  1. SepsisBundles: No list endpoint — only get-by-encounter. No way to query all active bundles across the hospital.
  2. qSOFA: Only "current" endpoint — no history, unlike NEWS2/SOFA/GCS which all have history endpoints.
  3. AlertThresholds: No get-by-id — only list-all and get-by-code.
  4. ReconciliationAlerts: No API surface at all — backend-only data quality checks.

Why fix

Clinical dashboards need a hospital-wide view of active sepsis bundles for charge nurse/supervisor workflows. qSOFA history is needed for trend visualization. ReconciliationAlerts are invisible to operators without SQL access.

How to fix

  1. Add GET /api/v1/sepsis-bundles?status=IN_PROGRESS&page=1&pageSize=20 — list with status filter.
  2. Add GET /api/v1/encounters/{encounterId}/qsofa/history — mirror NEWS2/SOFA history pattern with cursor pagination.
  3. Add GET /api/v1/alert-thresholds/{id} for admin detail views.
  4. Add GET /api/v1/reconciliation-alerts?resolved=false&page=1&pageSize=20 with checkType filter.

Files: SepsisBundlesController.cs, QsofaController.cs, AlertThresholdsController.cs, new ReconciliationAlertsController.cs, corresponding service methods.

Dependency: None.

P4 — No delete operations across entire API

Problem

The API has zero DELETE endpoints. The system is entirely append-only/immutable. While this is appropriate for clinical records (observations, alerts, scores), it's problematic for configuration entities like alert thresholds and for test/dev workflows.

Why fix

Administrators who create test thresholds or misconfigured entries cannot remove them. Draft/test patients created during onboarding clutter the production database. This is acceptable for clinical records but not for configuration data.

How to fix

  1. Add DELETE /api/v1/alert-thresholds/{id} with [AuthorizePermission(ThresholdsWrite)] — hard delete for configuration data.
  2. Document explicitly that clinical entities (patients, encounters, observations, alerts, scores) are immutable by design and do not support deletion (regulatory compliance).
  3. Optionally add a Patient.Status = "inactive" transition endpoint for marking test patients without deletion.

Files: AlertThresholdsController.cs, AlertThresholdService.cs.

Dependency: None.

P4 — FHIR R4 compliance limited to inbound-only facade

Problem

The FHIR implementation supports only Create interactions (POST). The CapabilityStatement correctly declares this, but there are no Read, Search, or Update operations. Only 4 resource types are supported (Patient, Encounter, Observation, MedicationAdministration). There is no FHIR search, no _include/_revinclude, no resource versioning (ETag/If-Match), and no batch bundle mode.

Why fix

EHR integrations commonly need bidirectional data flow. Care coordination systems need to read patient data back in FHIR format. Audit systems query for encounters. Without read operations, downstream systems must use the proprietary REST API instead of standard FHIR.

How to fix (phased)

Phase 1 — Read operations:

  1. Add GET /fhir/Patient/{id} and GET /fhir/Patient?identifier={system}|{value}.
  2. Add GET /fhir/Encounter/{id} and GET /fhir/Encounter?patient={patientId}.
  3. Map internal entities back to FHIR R4 resources using reverse mappers.
  4. Update CapabilityStatement to include Read and SearchType interactions.

Phase 2 — Search and versioning:

  1. Add search parameters: _lastUpdated, _count, _offset.
  2. Add ETag headers based on UpdatedAt or row version.

Files: FhirIngestController.cs, new FhirReadController.cs, FhirMetadataController.cs, new reverse mapper classes.

Dependency: Product decision on FHIR read scope.

Part D — Security & Hardening

P3 — FHIR API key not rotatable and timing-attack vulnerable

Problem

FhirApiKeyOrJwtMiddleware compares the X-Api-Key header against a config value using standard string equality (== config["Fhir:ApiKey"]). This is vulnerable to timing attacks. The API key is stored in appsettings.json in plaintext and cannot be rotated without redeploying the service.

Why fix

FHIR endpoints receive PHI (Protected Health Information). A compromised API key grants full integration-role access to patient data. Timing attacks are low-probability but easily prevented.

How to fix

  1. Replace string equality with CryptographicOperations.FixedTimeEquals() for constant-time comparison.
  2. Support multiple active API keys (array in config) for zero-downtime rotation.
  3. Move API keys to environment variables or a secrets manager (Azure Key Vault, AWS Secrets Manager, HashiCorp Vault).
  4. Add X-Api-Key rotation documentation to the ops runbook.
  5. Optionally add per-key audit logging (which key was used).

Files: FhirApiKeyOrJwtMiddleware.cs:46, FhirOptions.cs, appsettings.json.

Dependency: None.

P3 — JWT signing key not validated on startup

Problem

JwtOptions.SigningKey is read from configuration and used to create a SymmetricSecurityKey. There is no validation that the key meets minimum length requirements (256 bits for HMAC-SHA256). A short or empty key causes a runtime exception on the first authentication attempt, not at startup.

Why fix

Fail-fast on misconfiguration prevents deploying a service that accepts no requests. In development, a missing or weak key wastes debugging time on cryptic SecurityTokenInvalidSignatureException errors.

How to fix

  1. Add a startup validation check in Program.cs after binding JwtOptions: 
    if (string.IsNullOrEmpty(jwtOptions.SigningKey) || 
    Encoding.UTF8.GetByteCount(jwtOptions.SigningKey) < 32)
    throw new InvalidOperationException("JWT SigningKey must be at least 256 bits");
  2. Optionally add IValidateOptions<JwtOptions> implementation for structured validation.

Files: Program.cs, optionally JwtOptions.cs.

Dependency: None.

P3 — No audit of authorization failures

Problem

PermissionAuthorizationHandler returns context.Fail() when a user lacks the required permission, but does not log the attempt or write a ClinicalAuditLog entry. Failed authorization attempts are invisible in both application logs and the audit trail.

Why fix

Security audits and compliance reviews (HIPAA, SOC2) require evidence that unauthorized access attempts are logged. Without this, there is no way to detect credential compromise or privilege escalation attempts.

How to fix

  1. Inject ILogger<PermissionAuthorizationHandler> and log at Warning level on failure: user={username}, role={role}, requiredPermission={permission}, endpoint={resource}.
  2. Optionally write a ClinicalAuditLog entry with action AuthorizationDenied (new enum value) for persistent audit trail.
  3. Add a Prometheus counter: authorization_failures_total{permission, role}.

Files: PermissionAuthorizationHandler.cs, AuditAction.cs (new enum value), AuditService.cs.

Dependency: None.

P3 — Elasticsearch security disabled in deployment

Problem

docker-compose.yml sets xpack.security.enabled=false and xpack.security.http.ssl.enabled=false on the Elasticsearch container. The ES instance accepts unauthenticated requests from any container on the network. The patient_encounters index contains PHI (patient names, MRNs, encounter details).

Why fix

Any compromised container on the Docker network can read/write/delete clinical data in Elasticsearch. Even in development, this creates a risk of accidental data exposure if the Docker network is bridged to a shared network.

How to fix

  1. Enable xpack.security.enabled=true in docker-compose.
  2. Set ELASTIC_PASSWORD via Docker secrets or .env file.
  3. Update ElasticsearchOptions to include Username, Password, and UseTls fields.
  4. Configure the .NET ElasticClient with basic auth credentials.
  5. Document that production deployments must use TLS + authentication.

Files: docker-compose.yml:67, new ElasticsearchOptions.cs fields, EsIndexerService.cs, ElasticIndexProvisioner.cs.

Dependency: None.

P3 — No token refresh or revocation mechanism

Problem

JWT tokens are issued with a configurable expiration but there is no refresh token flow and no token revocation/blacklist. A compromised token remains valid until natural expiration. There is no POST /auth/refresh or POST /auth/revoke endpoint.

Why fix

Clinical sessions may last entire shifts (8-12 hours). Short token lifetimes require frequent re-authentication, disrupting clinical workflows. Long lifetimes without revocation mean a stolen token grants extended access. Compromised accounts cannot be locked out until the token expires.

How to fix

  1. Add refresh token support: issue a long-lived opaque refresh token stored in the database alongside the access token.
  2. Add POST /api/v1/auth/refresh — validate refresh token, issue new access token.
  3. Add POST /api/v1/auth/revoke — invalidate refresh token and optionally blacklist the access token (via Redis TTL set matching remaining token lifetime).
  4. Add LastLoginAt update on token refresh (already exists on ClinicalUser).

Files: AuthController.cs, AuthService.cs, ClinicalUser.cs (add RefreshToken, RefreshTokenExpiresAt), migration.

Dependency: None.

Part E — Observability & Operations

P5 — No request/response timing metrics

Problem

The API has Prometheus metrics for clinical events (alerts, bundles, consumer lag) but no HTTP request timing histograms. There is no way to measure API latency, identify slow endpoints, or set SLOs.

Why fix

Clinical dashboards and FHIR integrations depend on API responsiveness. Without latency metrics, there is no baseline for alerting on degradation, and performance regressions go undetected until users report them.

How to fix

  1. Add prometheus-net.AspNetCore middleware: app.UseHttpMetrics() in Program.cs.
  2. This automatically provides http_request_duration_seconds histogram with labels: method, controller, action, status_code.
  3. Add Grafana dashboard panels for p50/p95/p99 latency per endpoint.
  4. Set initial SLO targets (e.g., observation ingest p95 < 200ms).

Files: Program.cs:276 (add app.UseHttpMetrics() before app.MapMetrics()), VigilCareClinicalAPI.csproj (package).

Dependency: None.

P5 — Background service errors not metricked

Problem

Kafka consumer services, outbox relay, bundle monitor, and RabbitMQ workers log errors but do not increment Prometheus counters on failure. The only background service metrics are sepsis_bundle_compliance_total and kafka_consumer_lag. There are no failure-rate metrics for any background service.

Why fix

Log-based alerting requires parsing structured logs. Metrics-based alerting (Prometheus + Alertmanager) is standard in production Kubernetes deployments and enables rate-of-change alerts ("consumer errors spiking") that are impossible with log grep.

How to fix

  1. Add counters per background service:
    • kafka_consumer_errors_total{consumer_group, topic, error_type}
    • outbox_relay_failures_total{reason}
    • rabbitmq_worker_errors_total{queue, error_type}
    • datalake_flush_failures_total{topic, partition}
  2. Add processing duration histograms:
    • kafka_consumer_processing_seconds{consumer_group}
    • outbox_relay_batch_seconds
  3. Increment counters in existing catch blocks (minimal code change).

Files: All background services, new BackgroundServiceMetrics.cs static class for metric definitions.

Dependency: None.

P5 — Thin test coverage for concurrent operations and background services

Problem

Test coverage analysis reveals:

  • No concurrent operation tests: No tests for simultaneous alert creation, parallel observation ingest, or race conditions in deduplication logic.
  • Thin background service tests: Kafka consumer behavior, outbox relay failure recovery, and RabbitMQ worker retry logic are not directly tested.
  • No performance tests: No benchmarks for observation ingest throughput, scoring latency, or alert pipeline end-to-end timing.
  • No chaos tests: No fault injection for database/Redis/Kafka/RabbitMQ failures.

Well-tested areas include: clinical scoring (qSOFA, SOFA, NEWS2, GCS), alert lifecycle, FHIR ingest, medication correlation, sepsis bundle tracking, and end-to-end scenarios.

Why fix

The concurrent operation gaps directly correspond to P0 race conditions identified in this document (MRN generation, sepsis bundle creation). Without concurrent tests, fixes cannot be verified. Background service resilience is untested, meaning the Kafka poison pill and outbox retry gaps have no regression safety net.

How to fix

  1. Concurrent operation tests (priority — validates P0 fixes):

    • Parallel patient registration with same demographics → verify unique MRN.
    • Parallel SOFA_SEPSIS alerts for same encounter → verify single bundle.
    • Parallel observation ingest with same idempotency key → verify single record.

  2. Background service tests:

    • Test Kafka consumer with malformed message → verify skip after max retries.
    • Test outbox relay with simulated Kafka failure → verify retry and eventual dead-letter.
    • Test PagingWorker with acknowledged alert → verify no escalation.

  3. Performance benchmarks (optional, lower priority):

    • Observation ingest throughput (target: 1000/sec per instance).
    • Alert pipeline latency (observation → alert → page: target < 5s p95).

Files: New test files in VigilCareClinicalAPI.Tests/: ConcurrencyTests.cs, BackgroundServiceTests.cs, optional BenchmarkTests.cs.

Dependency: P0 fixes (concurrent tests validate the fixes).

Part F — Hardcoded Values & Configuration Gaps

P4 — Clinical parameters hardcoded instead of configurable

Problem

Several clinically significant parameters are hardcoded:

Value Location Current
Sepsis bundle deadline SepsisBundleService.cs:39 1 hour
Bundle monitor scan interval SepsisBundleMonitorService.cs:5 5 minutes
qSOFA criterion TTL QsofaDetector.cs:8 1800 seconds
GCS/NEWS2 scoring TTL GcsDetector.cs:8, News2Detector.cs:8 14400 seconds
MRN format pattern PatientService.cs:200 MRN-{count:D6}
Paging worker poll interval PagingWorkerService.cs 2 seconds

Why fix

Different hospitals and clinical settings have different protocols. CMS Sepsis SEP-1 requires a 3-hour bundle, not 1-hour. Facilities operating under different guidelines need to adjust these parameters without code changes.

How to fix

  1. Move sepsis bundle deadline to SepsisOptions.BundleDeadlineHours (default 1, CMS standard 3).
  2. Move bundle monitor scan interval to SepsisOptions.MonitorScanIntervalMinutes.
  3. Move qSOFA TTL to QsofaOptions.CriterionTtlSeconds.
  4. Move GCS/NEWS2 TTL to a shared ScoringOptions.CalculationTtlSeconds.
  5. Move MRN format to PatientOptions.MrnPrefix and MrnDigits.
  6. All via IOptions<T> pattern already established in the codebase.

Files: Respective service files, new/updated options classes, appsettings.json.

Dependency: None.

Summary matrix

# Issue Priority Part Status
1 MRN generation race condition P0 A Open
2 Sepsis bundle creation TOCTOU P0 A Open
3 Trend alert LIKE pattern P0 A Open
4 Order→Bundle transaction gap P1 A Open
5 FHIR bundle no rollback P1 A Open
6 Kafka replication factor = 1 P1 B Open
7 No health check endpoints P2 B Open
8 Kafka consumer poison pill P2 B Open
9 Outbox relay no dead-letter P2 B Open
10 ThresholdCacheLoader crash on Redis P2 B Open
11 DataLake partial commit P2 B Open
12 Missing input validators P2 C Open
13 No patient update endpoint P4 C Open
14 Pagination inconsistencies P4 C Open
15 Missing list/get endpoints P4 C Open
16 No delete operations P4 C Open
17 FHIR R4 read-only facade P4 C Open
18 API key timing attack + rotation P3 D Open
19 JWT key not validated on startup P3 D Open
20 No authorization failure audit P3 D Open
21 Elasticsearch security disabled P3 D Open
22 No token refresh/revocation P3 D Open
23 No request timing metrics P5 E Open
24 Background service error metrics P5 E Open
25 Thin concurrent/resilience tests P5 E Open
26 Clinical params hardcoded P4 F Open

Suggested implementation sequence

flowchart TD
    subgraph correctness [Part A — Correctness]
        P0A[P0: MRN sequence]
        P0B[P0: Bundle idempotent INSERT]
        P0C[P0: Trend exact match]
        P1A[P1: Order→Bundle transaction]
        P1B[P1: FHIR bundle rollback]
    end

    subgraph infra [Part B — Infrastructure]
        P1K[P1: Kafka replication factor]
        P2H[P2: Health checks]
        P2P[P2: Poison pill handling]
        P2O[P2: Outbox dead-letter]
        P2T[P2: ThresholdCacheLoader retry]
        P2D[P2: DataLake partial commit]
    end

    subgraph security [Part D — Security]
        P3K[P3: API key hardening]
        P3J[P3: JWT validation]
        P3A[P3: Auth failure audit]
        P3E[P3: ES security]
        P3R[P3: Token refresh]
    end

    subgraph api [Part C — API]
        P2V[P2: Missing validators]
        P4P[P4: Patient update]
        P4G[P4: Pagination/sorting]
        P4L[P4: Missing endpoints]
        P4F[P4: FHIR read ops]
    end

    subgraph obs [Part E — Observability]
        P5M[P5: Request metrics]
        P5B[P5: Background metrics]
        P5T[P5: Concurrent tests]
    end

    P0A --> P5T
    P0B --> P5T
    P0C --> P5T
    P2P --> P5B
    P2O --> P5B

Sprint-sized batches

Batch Items Outcome
1 — Correctness P0 MRN sequence, P0 bundle idempotent INSERT, P0 trend exact match, P1 order→bundle tx, P1 FHIR rollback Race conditions eliminated; clinical data integrity guaranteed
2 — Infrastructure resilience P1 Kafka RF, P2 health checks, P2 poison pill, P2 outbox dead-letter, P2 ThresholdCacheLoader, P2 DataLake commit Production-ready infrastructure; no silent data loss
3 — Security hardening P3 API key, P3 JWT validation, P3 auth audit, P3 ES security, P3 token refresh HIPAA/compliance baseline; audit trail for access
4 — API completeness P2 validators, P4 patient update, P4 pagination, P4 missing endpoints, P4 delete ops, P4 config extraction Admin UI and integration teams unblocked
5 — Observability & testing P5 request metrics, P5 background metrics, P5 concurrent tests, P4 FHIR read Incidents diagnosable; regression safety net for Batch 1 fixes

Testing strategy (cross-cutting)

For each fix, add or extend tests in VigilCareClinicalAPI.Tests/:

  • Concurrency tests (Batch 1): Parallel patient registration, parallel bundle creation, parallel observation ingest with same idempotency key.
  • Transaction rollback tests (Batch 1): Order result failure rolls back bundle update; FHIR bundle entry failure rolls back all entries.
  • Infrastructure resilience tests (Batch 2): Consumer with poison pill message, outbox with simulated Kafka failure, startup with Redis unavailable.
  • Security tests (Batch 3): Timing-safe API key comparison, expired/revoked token rejection, authorization failure audit log entry.
  • API contract tests (Batch 4): New validators return 422 with correct error shapes, pagination parameters respected, new endpoints return expected status codes.
  • Metrics verification tests (Batch 5): Prometheus counter increments on consumer error, request histogram populated after API call.

Out of scope (unless explicitly requested)

  • Full OpenTelemetry distributed tracing (P5 covers Prometheus metrics as interim).
  • Multi-tenancy or organization-scoped data isolation.
  • FHIR Subscription or WebSocket push for real-time updates.
  • HL7v2 ADT message support (current integration is FHIR-only).
  • Rate limiting on public-facing endpoints (API is internal-only today).
  • Database read replicas or CQRS pattern.
  • Kubernetes manifests, Helm charts, or CI/CD pipeline definitions.
  • SMART on FHIR authorization (OAuth2 scopes for EHR launch context).

Success criteria

When complete, the system should support:

Data Integrity (Part A)

  • Concurrent patient registrations produce unique MRNs without 500 errors.
  • Concurrent SOFA_SEPSIS alerts for the same encounter create exactly one bundle.
  • Trend alerts match on exact observation code, not substring.
  • Order results and bundle compliance update atomically.
  • FHIR transaction bundles are all-or-nothing.

Infrastructure (Part B)

  • Kafka topic loss requires losing 2+ brokers (RF=3).
  • Health checks report dependency status; orchestrators route around failures.
  • A malformed Kafka message is dead-lettered after 3 retries, not retried forever.
  • Outbox events have bounded retry with backoff and dead-letter.
  • Startup survives transient Redis outage.
  • Data lake writes are complete or explicitly failed — never silently dropped.

Security (Part D)

  • FHIR API keys can be rotated without downtime.
  • JWT misconfiguration fails at startup, not at first request.
  • Authorization failures are logged and auditable.
  • Elasticsearch requires authentication.

API (Part C)

  • All request types have input validation with 422 error responses.
  • Patient demographics are updatable via API.
  • Pagination is consistent (1-based, sortable) across all list endpoints.
  • Clinical dashboards have API access to sepsis bundles, qSOFA history, and reconciliation alerts.

Observability (Part E)

  • HTTP request latency is measurable via Prometheus histograms.
  • Background service failures are countable and alertable.
  • Concurrent operation tests provide regression safety for P0 fixes.
Reference in New Issue View Git Blame Copy Permalink