fixes: PromoteAsync (retry path) does not create clinical entities + Patient deduplication by exact name + DOB is fragile + Batch assignment creates inconsistent interim state

This commit is contained in:
voltsrage
2026-06-27 14:15:40 +08:00
parent 66ae95956a
commit 46c3492bb9
8 changed files with 1195 additions and 1107 deletions
+1 -1
View File
@@ -72,7 +72,7 @@ public class PromotionTests : IAsyncLifetime
var patient = await db.Patients.FirstOrDefaultAsync(p => p.Id == promotion.PatientId); var patient = await db.Patients.FirstOrDefaultAsync(p => p.Id == promotion.PatientId);
patient.Should().NotBeNull(); patient.Should().NotBeNull();
patient!.Mrn.Should().Be(promotion.Mrn); patient!.Mrn.Should().Be(promotion.Mrn);
patient.FullName.Should().Be("Test Patient"); patient.FullName.Should().Be("TEST PATIENT");
patient.DateOfBirth.Should().Be(new DateOnly(1990, 5, 15)); patient.DateOfBirth.Should().Be(new DateOnly(1990, 5, 15));
// Encounter exists // Encounter exists
+38 -12
View File
@@ -386,15 +386,19 @@ public class VerificationTests : IAsyncLifetime
var olderBatch = await BatchSeedHelper.SeedBatchInPendingVerificationAsync( var olderBatch = await BatchSeedHelper.SeedBatchInPendingVerificationAsync(
db, (await BatchSeedHelper.UserIdAsync(db, "entry1"))); db, (await BatchSeedHelper.UserIdAsync(db, "entry1")));
var trackedOlder = await db.DigitizationBatches.FindAsync(olderBatch.Id);
// Make the second batch newer by updating its timestamp trackedOlder!.UpdatedAt = DateTimeOffset.UtcNow.AddHours(-2);
await Task.Delay(100); // Ensure different timestamps await db.SaveChangesAsync();
var newerBatch = await BatchSeedHelper.SeedBatchInPendingVerificationAsync( var newerBatch = await BatchSeedHelper.SeedBatchInPendingVerificationAsync(
db, (await BatchSeedHelper.UserIdAsync(db, "entry2"))); db, (await BatchSeedHelper.UserIdAsync(db, "entry2")));
var trackedNewer = await db.DigitizationBatches.FindAsync(newerBatch.Id);
trackedNewer!.UpdatedAt = DateTimeOffset.UtcNow.AddHours(-1);
await db.SaveChangesAsync();
// Also seed a rejected batch — should NOT appear in verification queue // Also seed a rejected batch — should NOT appear in verification queue
await BatchSeedHelper.SeedBatchInRejectedAsync(db, (await BatchSeedHelper.UserIdAsync(db, "entry1"))); var rejectedBatch = await BatchSeedHelper.SeedBatchInRejectedAsync(
db, (await BatchSeedHelper.UserIdAsync(db, "entry1")));
// Act // Act
var client = await AuthHelper.LoginAsync(_fixture, "verifier1"); var client = await AuthHelper.LoginAsync(_fixture, "verifier1");
@@ -410,11 +414,23 @@ public class VerificationTests : IAsyncLifetime
.GetProperty("data") .GetProperty("data")
.GetProperty("items"); .GetProperty("items");
items.GetArrayLength().Should().Be(2); // Only PendingVerification batches var batchIds = Enumerable.Range(0, items.GetArrayLength())
.Select(i => items[i].GetProperty("batchId").GetString())
.ToList();
// First item should be the older batch (FIFO) batchIds.Should().Contain(olderBatch.Id.ToString());
var firstBatchId = items[0].GetProperty("batchId").GetString(); batchIds.Should().Contain(newerBatch.Id.ToString());
firstBatchId.Should().Be(olderBatch.Id.ToString()); batchIds.Should().NotContain(rejectedBatch.Id.ToString());
foreach (var i in Enumerable.Range(0, items.GetArrayLength()))
{
items[i].GetProperty("status").GetString()
.Should().Be(BatchStatus.PendingVerification.ToDbString());
}
// Older batch should appear before newer batch (FIFO by UpdatedAt)
batchIds.IndexOf(olderBatch.Id.ToString())
.Should().BeLessThan(batchIds.IndexOf(newerBatch.Id.ToString()));
} }
/// <summary> /// <summary>
@@ -432,7 +448,7 @@ public class VerificationTests : IAsyncLifetime
db, (await BatchSeedHelper.UserIdAsync(db, "entry1"))); db, (await BatchSeedHelper.UserIdAsync(db, "entry1")));
// Seed a PendingVerification batch (should NOT appear in entry queue) // Seed a PendingVerification batch (should NOT appear in entry queue)
await BatchSeedHelper.SeedBatchInPendingVerificationAsync( var pendingBatch = await BatchSeedHelper.SeedBatchInPendingVerificationAsync(
db, (await BatchSeedHelper.UserIdAsync(db, "entry2"))); db, (await BatchSeedHelper.UserIdAsync(db, "entry2")));
// Seed an Uploaded batch directly // Seed an Uploaded batch directly
@@ -474,15 +490,25 @@ public class VerificationTests : IAsyncLifetime
.GetProperty("data") .GetProperty("data")
.GetProperty("items"); .GetProperty("items");
// Should contain the rejected batch and uploaded batch, but NOT the PendingVerification batch
items.GetArrayLength().Should().Be(2);
var batchIds = Enumerable.Range(0, items.GetArrayLength()) var batchIds = Enumerable.Range(0, items.GetArrayLength())
.Select(i => items[i].GetProperty("batchId").GetString()) .Select(i => items[i].GetProperty("batchId").GetString())
.ToList(); .ToList();
batchIds.Should().Contain(rejectedBatch.Id.ToString()); batchIds.Should().Contain(rejectedBatch.Id.ToString());
batchIds.Should().Contain(uploadedBatchId.ToString()); batchIds.Should().Contain(uploadedBatchId.ToString());
batchIds.Should().NotContain(pendingBatch.Id.ToString());
var entryStatuses = new[]
{
BatchStatus.Uploaded.ToDbString(),
BatchStatus.InEntry.ToDbString(),
BatchStatus.Rejected.ToDbString()
};
foreach (var i in Enumerable.Range(0, items.GetArrayLength()))
{
entryStatuses.Should().Contain(items[i].GetProperty("status").GetString());
}
} }
// --------------------------------------------------------------- // ---------------------------------------------------------------
@@ -1,7 +1,11 @@
public record PromotionResult( public record PromotionResult(
Guid BatchId, Guid BatchId,
Guid PatientId,
string Mrn,
Guid EncounterId, Guid EncounterId,
Guid[] ObservationIds,
int ObservationsPromoted, int ObservationsPromoted,
int OutboxEventsWritten,
bool IsCorrection, bool IsCorrection,
SupersessionResult? Supersession SupersessionResult? Supersession
); );
@@ -212,6 +212,7 @@ public class BatchService : IBatchService
"BATCH_ALREADY_ASSIGNED"); "BATCH_ALREADY_ASSIGNED");
batch.EnteredByUserId = entryClerkUserId; batch.EnteredByUserId = entryClerkUserId;
batch.Status = BatchStatus.InEntry;
batch.UpdatedAt = DateTimeOffset.UtcNow; batch.UpdatedAt = DateTimeOffset.UtcNow;
_db.DigitizationEvents.Add(new DigitizationEvent _db.DigitizationEvents.Add(new DigitizationEvent
@@ -6,7 +6,6 @@ public class DraftService : IDraftService
private readonly AppDbContext _db; private readonly AppDbContext _db;
private readonly ILogger<DraftService> _logger; private readonly ILogger<DraftService> _logger;
// Statuses that allow data entry to begin or continue
private static readonly HashSet<BatchStatus> _entryAllowedStatuses = new() private static readonly HashSet<BatchStatus> _entryAllowedStatuses = new()
{ {
BatchStatus.Uploaded, BatchStatus.Uploaded,
@@ -335,15 +334,10 @@ public class DraftService : IDraftService
return batch; return batch;
} }
/// <summary>
/// Transitions the batch from Uploaded or Rejected to InEntry on first save.
/// If the batch is already InEntry, this is a no-op.
/// </summary>
private void TransitionToInEntryIfNeeded(DigitizationBatch batch, Guid actorUserId) private void TransitionToInEntryIfNeeded(DigitizationBatch batch, Guid actorUserId)
{ {
if (batch.Status == BatchStatus.InEntry) return; if (batch.Status == BatchStatus.InEntry) return;
// batch.Status is Uploaded or Rejected (validated by LoadBatchForEntryAsync)
var previousStatus = batch.Status.ToDbString(); var previousStatus = batch.Status.ToDbString();
batch.Status = BatchStatus.InEntry; batch.Status = BatchStatus.InEntry;
batch.UpdatedAt = DateTimeOffset.UtcNow; batch.UpdatedAt = DateTimeOffset.UtcNow;
+195 -229
View File
@@ -73,21 +73,8 @@ public class PromotionService : IPromotionService
"Separation of duties: the verifier cannot also approve the same batch.", "Separation of duties: the verifier cannot also approve the same batch.",
"SEPARATION_OF_DUTIES_VIOLATION"); "SEPARATION_OF_DUTIES_VIOLATION");
// --- Validate draft data completeness (corrections reuse the linked live patient) --- // --- Validate draft data completeness ---
if (!batch.SupersedesBatchId.HasValue) ValidateDraftDataForPromotion(batch);
{
if (batch.DraftPatient is null)
throw new ValidationException("Batch has no draft patient data.", "MISSING_DRAFT_PATIENT");
if (string.IsNullOrWhiteSpace(batch.DraftPatient.FullName))
throw new ValidationException("Draft patient must have a full name.", "INVALID_DRAFT_PATIENT");
}
else if (!batch.PatientId.HasValue)
{
throw new ValidationException(
"Correction batch has no linked patient.",
"MISSING_PATIENT");
}
// --- Begin atomic transaction --- // --- Begin atomic transaction ---
await using var transaction = await _db.Database.BeginTransactionAsync(); await using var transaction = await _db.Database.BeginTransactionAsync();
@@ -96,92 +83,21 @@ public class PromotionService : IPromotionService
{ {
var now = DateTimeOffset.UtcNow; var now = DateTimeOffset.UtcNow;
// === Step 1: Create or update Patient ===
Patient patient;
if (batch.SupersedesBatchId.HasValue)
{
patient = await _db.Patients.FirstOrDefaultAsync(p => p.Id == batch.PatientId!.Value)
?? throw new NotFoundException(
$"Patient {batch.PatientId.Value} not found.",
"PATIENT_NOT_FOUND");
}
else
{
patient = await CreateOrUpdatePatientAsync(batch.DraftPatient!, now);
}
// === Step 2: Create or match Encounter (reuse original encounter for corrections) ===
var encounter = await ResolveClinicalEncounterAsync(batch, patient.Id, now);
await EnsureLiveEncounterAsync(encounter, now);
// === Step 3: Insert each DraftObservation as live Observation ===
var (observationIds, outboxCount) = await PromoteObservationsAsync(
batch, patient.Id, encounter.Id, enableRetroactiveAlerts, now);
// === Step 3b: Mirror to live_observations for supersession / digitization history ===
PromoteLiveObservationsAsync(batch, patient.Id, encounter.Id, now);
SupersessionResult? supersessionResult = null;
if (batch.SupersedesBatchId.HasValue)
{
supersessionResult = await SupersedeOriginalBatchAsync(
batch.SupersedesBatchId.Value, batchId, approverUserId, now);
}
// === Step 4: Update batch status to Promoted ===
batch.Status = BatchStatus.Promoted;
batch.ApprovedByUserId = approverUserId; batch.ApprovedByUserId = approverUserId;
batch.PatientId = patient.Id;
batch.PromotedAt = now;
batch.PromotionEncounterId = encounter.Id;
batch.EnableRetroactiveAlerts = enableRetroactiveAlerts;
batch.UpdatedAt = now;
// === Step 5: Write DigitizationEvent === var core = await ExecutePromotionCoreAsync(
var promotionMetadata = new Dictionary<string, object> batch, approverUserId, enableRetroactiveAlerts, now);
{
["patientId"] = patient.Id,
["mrn"] = patient.Mrn,
["encounterId"] = encounter.Id,
["observationCount"] = observationIds.Length,
["outboxEventsWritten"] = outboxCount,
["enableRetroactiveAlerts"] = enableRetroactiveAlerts,
["track"] = batch.Track.ToDbString(),
["isCorrection"] = batch.SupersedesBatchId.HasValue
};
if (supersessionResult is not null) // === Store idempotency record (within same transaction) ===
{
promotionMetadata["supersession"] = new
{
originalBatchId = supersessionResult.OriginalBatchId,
observationsSuperseded = supersessionResult.ObservationsSuperseded,
observationsReplaced = supersessionResult.ObservationsReplaced
};
}
_db.DigitizationEvents.Add(new DigitizationEvent
{
Id = Guid.NewGuid(),
BatchId = batchId,
EventType = batch.SupersedesBatchId.HasValue
? DigitizationEventType.CorrectionPromoted
: DigitizationEventType.Promoted,
ActorUserId = approverUserId,
OccurredAt = now,
MetadataJson = JsonSerializer.Serialize(promotionMetadata)
});
// === Step 6: Store idempotency record (within same transaction) ===
var result = new PromotionResultResponse( var result = new PromotionResultResponse(
BatchId: batchId, BatchId: batchId,
Status: BatchStatus.Promoted.ToDbString(), Status: BatchStatus.Promoted.ToDbString(),
PatientId: patient.Id, PatientId: core.Patient.Id,
Mrn: patient.Mrn, Mrn: core.Patient.Mrn,
EncounterId: encounter.Id, EncounterId: core.Encounter.Id,
ObservationIds: observationIds, ObservationIds: core.ObservationIds,
PromotedAt: now, PromotedAt: now,
OutboxEventsWritten: outboxCount OutboxEventsWritten: core.OutboxCount
); );
if (!string.IsNullOrWhiteSpace(idempotencyKey)) if (!string.IsNullOrWhiteSpace(idempotencyKey))
@@ -191,7 +107,7 @@ public class PromotionService : IPromotionService
200, result, TimeSpan.FromHours(24)); 200, result, TimeSpan.FromHours(24));
} }
// === Step 7: Commit === // === Commit ===
await _db.SaveChangesAsync(); await _db.SaveChangesAsync();
await transaction.CommitAsync(); await transaction.CommitAsync();
@@ -201,8 +117,8 @@ public class PromotionService : IPromotionService
_logger.LogInformation( _logger.LogInformation(
"Batch {BatchId} promoted in {ElapsedMs}ms: Patient {PatientId} (MRN {Mrn}), " + "Batch {BatchId} promoted in {ElapsedMs}ms: Patient {PatientId} (MRN {Mrn}), " +
"Encounter {EncounterId}, {ObsCount} observations, {OutboxCount} outbox events", "Encounter {EncounterId}, {ObsCount} observations, {OutboxCount} outbox events",
batchId, stopwatch.ElapsedMilliseconds, patient.Id, patient.Mrn, encounter.Id, batchId, stopwatch.ElapsedMilliseconds, core.Patient.Id, core.Patient.Mrn,
observationIds.Length, outboxCount); core.Encounter.Id, core.ObservationIds.Length, core.OutboxCount);
return result; return result;
} }
@@ -263,22 +179,133 @@ public class PromotionService : IPromotionService
// Private helpers // Private helpers
// ────────────────────────────────────────────────── // ──────────────────────────────────────────────────
private record PromotionCoreResult(
Patient Patient,
Encounter Encounter,
Guid[] ObservationIds,
int OutboxCount,
SupersessionResult? Supersession);
private static void ValidateDraftDataForPromotion(DigitizationBatch batch)
{
if (!batch.SupersedesBatchId.HasValue)
{
if (batch.DraftPatient is null)
throw new ValidationException("Batch has no draft patient data.", "MISSING_DRAFT_PATIENT");
if (string.IsNullOrWhiteSpace(batch.DraftPatient.FullName))
throw new ValidationException("Draft patient must have a full name.", "INVALID_DRAFT_PATIENT");
}
else if (!batch.PatientId.HasValue)
{
throw new ValidationException(
"Correction batch has no linked patient.",
"MISSING_PATIENT");
}
}
private async Task<PromotionCoreResult> ExecutePromotionCoreAsync(
DigitizationBatch batch, Guid actorUserId, bool enableRetroactiveAlerts, DateTimeOffset now)
{
// === Step 1: Create or update Patient ===
Patient patient;
if (batch.SupersedesBatchId.HasValue)
{
patient = await _db.Patients.FirstOrDefaultAsync(p => p.Id == batch.PatientId!.Value)
?? throw new NotFoundException(
$"Patient {batch.PatientId!.Value} not found.",
"PATIENT_NOT_FOUND");
}
else
{
patient = await CreateOrUpdatePatientAsync(batch.DraftPatient!, now);
}
// === Step 2: Create or match Encounter + LiveEncounter ===
var encounter = await ResolveClinicalEncounterAsync(batch, patient.Id, now);
await EnsureLiveEncounterAsync(encounter, now);
// === Step 3: Insert clinical Observations + OutboxEvents ===
var (observationIds, outboxCount) = await PromoteObservationsAsync(
batch, patient.Id, encounter.Id, enableRetroactiveAlerts, now);
// === Step 4: Mirror to LiveObservations ===
PromoteLiveObservationsAsync(batch, patient.Id, encounter.Id, now);
// === Step 5: Handle supersession for correction batches ===
SupersessionResult? supersessionResult = null;
if (batch.SupersedesBatchId.HasValue)
{
supersessionResult = await SupersedeOriginalBatchAsync(
batch.SupersedesBatchId.Value, batch.Id, actorUserId, now);
}
// === Step 6: Update batch status to Promoted ===
batch.Status = BatchStatus.Promoted;
batch.PatientId = patient.Id;
batch.PromotedAt = now;
batch.PromotionEncounterId = encounter.Id;
batch.EnableRetroactiveAlerts = enableRetroactiveAlerts;
batch.UpdatedAt = now;
// === Step 7: Write DigitizationEvent ===
var promotionMetadata = new Dictionary<string, object>
{
["patientId"] = patient.Id,
["mrn"] = patient.Mrn,
["encounterId"] = encounter.Id,
["observationCount"] = observationIds.Length,
["outboxEventsWritten"] = outboxCount,
["enableRetroactiveAlerts"] = enableRetroactiveAlerts,
["track"] = batch.Track.ToDbString(),
["isCorrection"] = batch.SupersedesBatchId.HasValue
};
if (supersessionResult is not null)
{
promotionMetadata["supersession"] = new
{
originalBatchId = supersessionResult.OriginalBatchId,
observationsSuperseded = supersessionResult.ObservationsSuperseded,
observationsReplaced = supersessionResult.ObservationsReplaced
};
}
_db.DigitizationEvents.Add(new DigitizationEvent
{
Id = Guid.NewGuid(),
BatchId = batch.Id,
EventType = batch.SupersedesBatchId.HasValue
? DigitizationEventType.CorrectionPromoted
: DigitizationEventType.Promoted,
ActorUserId = actorUserId,
OccurredAt = now,
MetadataJson = JsonSerializer.Serialize(promotionMetadata)
});
return new PromotionCoreResult(patient, encounter, observationIds, outboxCount, supersessionResult);
}
private static string NormalizePatientName(string name) =>
string.Join(" ", name.Trim().Split((char[]?)null, StringSplitOptions.RemoveEmptyEntries))
.ToUpperInvariant();
private async Task<Patient> CreateOrUpdatePatientAsync(DraftPatient draft, DateTimeOffset now) private async Task<Patient> CreateOrUpdatePatientAsync(DraftPatient draft, DateTimeOffset now)
{ {
// Attempt to match existing patient by name + DOB (simple dedup) var normalizedName = NormalizePatientName(draft.FullName!);
Patient? existing = null; Patient? existing = null;
if (draft.DateOfBirth.HasValue) if (draft.DateOfBirth.HasValue)
{ {
existing = await _db.Patients existing = await _db.Patients
.FirstOrDefaultAsync(p => .FirstOrDefaultAsync(p =>
p.FullName == draft.FullName && EF.Functions.ILike(p.FullName, normalizedName) &&
p.DateOfBirth == draft.DateOfBirth); p.DateOfBirth == draft.DateOfBirth);
} }
if (existing is not null) if (existing is not null)
{ {
// Update fields that may have new information
existing.Sex = draft.Sex ?? existing.Sex; existing.Sex = draft.Sex ?? existing.Sex;
existing.BloodType = draft.BloodType ?? existing.BloodType; existing.BloodType = draft.BloodType ?? existing.BloodType;
existing.EmergencyContact = draft.EmergencyContact ?? existing.EmergencyContact; existing.EmergencyContact = draft.EmergencyContact ?? existing.EmergencyContact;
@@ -287,20 +314,43 @@ public class PromotionService : IPromotionService
existing.UpdatedAt = now; existing.UpdatedAt = now;
_logger.LogInformation( _logger.LogInformation(
"Matched existing patient {PatientId} (MRN {Mrn}) by name + DOB", "Matched existing patient {PatientId} (MRN {Mrn}) by normalized name + DOB",
existing.Id, existing.Mrn); existing.Id, existing.Mrn);
return existing; return existing;
} }
// Create new patient with generated MRN // No exact normalized match — check for fuzzy near-misses on same DOB
if (draft.DateOfBirth.HasValue)
{
var sameDobPatients = await _db.Patients
.Where(p => p.DateOfBirth == draft.DateOfBirth)
.Select(p => new { p.Id, p.Mrn, p.FullName })
.ToListAsync();
foreach (var candidate in sameDobPatients)
{
var distance = LevenshteinDistance(
normalizedName, NormalizePatientName(candidate.FullName));
if (distance > 0 && distance <= 3)
{
_logger.LogWarning(
"Fuzzy patient match: new name '{NewName}' is {Distance} edits from " +
"existing patient {PatientId} (MRN {Mrn}, name '{ExistingName}') with same DOB. " +
"Creating new patient — consider merging if these are the same person",
normalizedName, distance, candidate.Id, candidate.Mrn, candidate.FullName);
}
}
}
var mrn = await _mrnGenerator.GenerateNextMrnAsync(); var mrn = await _mrnGenerator.GenerateNextMrnAsync();
var patient = new Patient var patient = new Patient
{ {
Id = Guid.NewGuid(), Id = Guid.NewGuid(),
Mrn = mrn, Mrn = mrn,
FullName = draft.FullName!, FullName = normalizedName,
DateOfBirth = draft.DateOfBirth, DateOfBirth = draft.DateOfBirth,
Sex = draft.Sex, Sex = draft.Sex,
BloodType = draft.BloodType, BloodType = draft.BloodType,
@@ -320,6 +370,33 @@ public class PromotionService : IPromotionService
return patient; return patient;
} }
private static int LevenshteinDistance(string a, string b)
{
if (a.Length == 0) return b.Length;
if (b.Length == 0) return a.Length;
var prev = new int[b.Length + 1];
var curr = new int[b.Length + 1];
for (var j = 0; j <= b.Length; j++)
prev[j] = j;
for (var i = 1; i <= a.Length; i++)
{
curr[0] = i;
for (var j = 1; j <= b.Length; j++)
{
var cost = a[i - 1] == b[j - 1] ? 0 : 1;
curr[j] = Math.Min(
Math.Min(curr[j - 1] + 1, prev[j] + 1),
prev[j - 1] + cost);
}
(prev, curr) = (curr, prev);
}
return prev[b.Length];
}
private async Task<Encounter> ResolveClinicalEncounterAsync( private async Task<Encounter> ResolveClinicalEncounterAsync(
DigitizationBatch batch, Guid patientId, DateTimeOffset now) DigitizationBatch batch, Guid patientId, DateTimeOffset now)
{ {
@@ -536,7 +613,6 @@ public class PromotionService : IPromotionService
try try
{ {
// Load the batch with all draft data
var batch = await _db.DigitizationBatches var batch = await _db.DigitizationBatches
.Include(b => b.DraftPatient) .Include(b => b.DraftPatient)
.Include(b => b.DraftEncounter) .Include(b => b.DraftEncounter)
@@ -551,76 +627,13 @@ public class PromotionService : IPromotionService
$"Only approved batches can be promoted. Batch is in '{batch.Status.ToDbString()}' status.", $"Only approved batches can be promoted. Batch is in '{batch.Status.ToDbString()}' status.",
"ILLEGAL_STATUS_TRANSITION"); "ILLEGAL_STATUS_TRANSITION");
// Resolve or create the live encounter ValidateDraftDataForPromotion(batch);
var encounterId = await ResolveEncounterAsync(batch);
// Promote draft observations to live observations
var now = DateTimeOffset.UtcNow; var now = DateTimeOffset.UtcNow;
var liveObservations = batch.DraftObservations.Select(draft => new LiveObservation var enableRetroactiveAlerts = batch.EnableRetroactiveAlerts;
{
Id = Guid.NewGuid(),
EncounterId = encounterId,
PatientId = batch.PatientId,
SourceBatchId = batch.Id,
ObservationCode = draft.ObservationCode,
Value = draft.Value,
Unit = draft.Unit,
RecordedAt = draft.RecordedAt,
Note = draft.Note,
CreatedAt = now,
IsSuperseded = false,
SupersededByBatchId = null,
SupersededAt = null
}).ToList();
_db.LiveObservations.AddRange(liveObservations); var core = await ExecutePromotionCoreAsync(
batch, actorUserId, enableRetroactiveAlerts, now);
// Handle supersession if this is a correction batch
SupersessionResult? supersessionResult = null;
if (batch.SupersedesBatchId.HasValue)
{
supersessionResult = await SupersedeOriginalBatchAsync(
batch.SupersedesBatchId.Value,
batch.Id,
actorUserId,
now);
}
// Update batch status to Promoted
batch.Status = BatchStatus.Promoted;
batch.PromotedAt = now;
batch.PromotionEncounterId = encounterId;
batch.UpdatedAt = now;
// Record promotion event on the correction batch
var promotionMetadata = new Dictionary<string, object>
{
["encounterId"] = encounterId,
["observationsPromoted"] = liveObservations.Count,
["isCorrection"] = batch.SupersedesBatchId.HasValue
};
if (supersessionResult is not null)
{
promotionMetadata["supersession"] = new
{
originalBatchId = supersessionResult.OriginalBatchId,
observationsSuperseded = supersessionResult.ObservationsSuperseded,
observationsReplaced = supersessionResult.ObservationsReplaced
};
}
_db.DigitizationEvents.Add(new DigitizationEvent
{
Id = Guid.NewGuid(),
BatchId = batch.Id,
EventType = batch.SupersedesBatchId.HasValue
? DigitizationEventType.CorrectionPromoted
: DigitizationEventType.Promoted,
ActorUserId = actorUserId,
OccurredAt = now,
MetadataJson = JsonSerializer.Serialize(promotionMetadata)
});
await _db.SaveChangesAsync(); await _db.SaveChangesAsync();
await transaction.CommitAsync(); await transaction.CommitAsync();
@@ -629,20 +642,21 @@ public class PromotionService : IPromotionService
DiagnosticsMetrics.PromotionDuration.Observe(stopwatch.Elapsed.TotalSeconds); DiagnosticsMetrics.PromotionDuration.Observe(stopwatch.Elapsed.TotalSeconds);
_logger.LogInformation( _logger.LogInformation(
"Batch {BatchId} promoted in {ElapsedMs}ms (correction={IsCorrection}, " + "Batch {BatchId} promoted via retry in {ElapsedMs}ms: Patient {PatientId} (MRN {Mrn}), " +
"observations={ObservationCount}, superseded={SupersededCount})", "Encounter {EncounterId}, {ObsCount} observations, {OutboxCount} outbox events",
batchId, batchId, stopwatch.ElapsedMilliseconds, core.Patient.Id, core.Patient.Mrn,
stopwatch.ElapsedMilliseconds, core.Encounter.Id, core.ObservationIds.Length, core.OutboxCount);
batch.SupersedesBatchId.HasValue,
liveObservations.Count,
supersessionResult?.ObservationsSuperseded ?? 0);
return new PromotionResult( return new PromotionResult(
batch.Id, batch.Id,
encounterId, core.Patient.Id,
liveObservations.Count, core.Patient.Mrn,
core.Encounter.Id,
core.ObservationIds,
core.ObservationIds.Length,
core.OutboxCount,
batch.SupersedesBatchId.HasValue, batch.SupersedesBatchId.HasValue,
supersessionResult); core.Supersession);
} }
catch catch
{ {
@@ -714,52 +728,4 @@ public class PromotionService : IPromotionService
now); now);
} }
/// <summary>
/// Resolves the live encounter for promotion. For correction batches, reuses the
/// encounter from the original batch to maintain continuity. For new batches,
/// creates or resolves the encounter from draft data.
/// </summary>
private async Task<Guid> ResolveEncounterAsync(DigitizationBatch batch)
{
// For correction batches, reuse the encounter from the original promoted batch
if (batch.SupersedesBatchId.HasValue)
{
var originalBatch = await _db.DigitizationBatches
.FirstOrDefaultAsync(b => b.Id == batch.SupersedesBatchId.Value);
if (originalBatch?.PromotionEncounterId.HasValue == true)
{
_logger.LogInformation(
"Correction batch {CorrectionBatchId} reusing encounter {EncounterId} " +
"from original batch {OriginalBatchId}",
batch.Id, originalBatch.PromotionEncounterId.Value, originalBatch.Id);
return originalBatch.PromotionEncounterId.Value;
}
}
// For non-correction batches, create or find the encounter from draft data
if (batch.DraftEncounter is null)
throw new ValidationException(
"Batch has no draft encounter data for promotion.",
"MISSING_ENCOUNTER_DATA");
var encounter = new LiveEncounter
{
Id = Guid.NewGuid(),
PatientId = batch.PatientId
?? throw new ValidationException("Patient ID is required for promotion.", "MISSING_PATIENT_ID"),
AdmissionDate = batch.DraftEncounter.AdmissionDate
?? throw new ValidationException("Admission date is required.", "MISSING_ADMISSION_DATE"),
Department = batch.DraftEncounter.Department,
RoomBed = batch.DraftEncounter.RoomBed,
AdmissionReason = batch.DraftEncounter.AdmissionReason,
DischargeDiagnosis = batch.DraftEncounter.DischargeDiagnosis,
Status = batch.DraftEncounter.Status ?? "active",
CreatedAt = DateTimeOffset.UtcNow
};
_db.LiveEncounters.Add(encounter);
return encounter.Id;
}
} }
-859
View File
@@ -1,859 +0,0 @@
# 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:
```sql
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`:
```csharp
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`:
```csharp
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~~ DONE
Implemented: `RefreshToken` entity with DB-backed storage, `POST /api/v1/auth/refresh` (rotate refresh token + issue new access token), `POST /api/v1/auth/logout` (revoke refresh token server-side). Access token reduced to 15 min, refresh token 7 days. Frontend auto-refreshes before expiry, retries on 401, and redirects to login on refresh failure. Logout button in header, sidebar, and mobile nav. Audit logged as `USER_LOGOUT` and `TOKEN_REFRESHED`.
---
# 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 | **Done** |
| 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
```mermaid
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.
+956
View File
@@ -0,0 +1,956 @@
# VigilCare Records Platform — Gap Analysis
Comprehensive gap analysis of the VigilCare Records digitization system covering data integrity, infrastructure reliability, security posture, API completeness, Vue frontend coverage, observability, and test coverage. Analysis compares the current implementation against the [PRD](vigilcare-records-prd.md) and identifies issues ordered by **impact on data correctness and clinical safety first**, then **operational reliability**, then **feature 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 correctness bug; fix before expanding production usage |
| **P1** | Silent misconfiguration, transaction safety hole, or data loss risk; fix soon after P0 |
| **P2** | Blocks common workflows or degrades operational reliability |
| **P3** | Security hardening, compliance, or defense-in-depth; no immediate patient-safety impact |
| **P4** | API/UI completeness, consistency, and developer/operator experience |
| **P5** | Observability and test coverage; does not change outcomes but makes incidents diagnosable |
---
# Part A — Data Integrity & Correctness
---
## P0 — PromoteAsync (retry path) does not create clinical entities
### Problem
`PromotionService` has two code paths for promotion:
1. `ApproveAndPromoteAsync` (called by `POST .../approve`) — creates clinical `Patient`, `Encounter`, `Observation` entities **plus** `LiveEncounter`, `LiveObservation`, and `OutboxEvent` entries. This is the happy path.
2. `PromoteAsync` (called by `POST .../promote` and `PromotionRetryService`) — creates only `LiveEncounter` and `LiveObservation` entries. It does **not** create clinical `Patient`, `Encounter`, or `Observation` records, and does **not** write `OutboxEvent` entries.
When initial promotion fails and the batch is deferred to `APPROVED` status (202 response), the `PromotionRetryService` retries via `PromoteAsync`. On success, the batch is marked `PROMOTED` but the clinical tables that VigilCareClinical depends on are never populated. The batch appears promoted, but observations are invisible to the downstream alert pipeline, ward dashboard, and scoring consumers.
### Why fix
A successfully retried promotion that fails to populate clinical tables defeats the entire purpose of the digitization pipeline. Observations from deferred promotions never reach VigilCareClinical's alert engine — critical values entered during a live capture that was deferred due to transient infrastructure failure will silently disappear from clinical monitoring.
### How to fix
1. **Unify the promotion logic**: `PromoteAsync` should call the same core promotion method as `ApproveAndPromoteAsync`, with the only difference being that approval-specific validation (status check for `Verified`/`AwaitingClinicalApproval`, separation-of-duties) is already done.
2. Extract a shared `ExecutePromotionAsync(batch, actorUserId, enableRetroactiveAlerts)` method that creates Patient, Encounter, Observation, LiveEncounter, LiveObservation, and OutboxEvent in a single transaction.
3. Both `ApproveAndPromoteAsync` and `PromoteAsync` should call this shared method.
4. Ensure the batch's `EnableRetroactiveAlerts` flag (set during `DeferPromotionAsync`) is read and passed to the shared method on retry.
5. Add an integration test: defer promotion (simulate infra failure) → retry succeeds → verify clinical Observation rows exist.
**Files:** `PromotionService.cs:529-658` (PromoteAsync), `PromotionService.cs:24-220` (ApproveAndPromoteAsync).
**Dependency:** None.
---
## P0 — Patient deduplication by exact name + DOB is fragile
### Problem
`PromotionService.CreateOrUpdatePatientAsync` matches existing patients by exact `FullName` string equality and `DateOfBirth`. Two batches for the same physical patient with different name representations ("Maria Santos" vs "MARIA SANTOS" vs "Maria R. Santos" vs "Santos, Maria") create duplicate patient records with distinct MRNs.
### Why fix
MRN is the primary patient identifier. Duplicate patients split their clinical history across multiple MRNs — observations, encounters, and alerts for the same person appear under different identities. This fragments the clinical picture and can lead to missed critical trend alerts (e.g., three potassium readings spread across two patient records don't trigger a trend).
### How to fix
1. **Normalize name comparison**: case-insensitive, whitespace-trimmed comparison as minimum. Consider `ToUpperInvariant().Trim()` normalization.
2. **Add fuzzy matching warning**: if no exact match but a close match exists (e.g., Levenshtein distance < 3 on normalized name + exact DOB match), log a warning and return the match with a flag in the promotion result indicating a fuzzy match was used.
3. **Add a patient merge endpoint** (future): `POST /api/v1/patients/{targetId}/merge/{sourceId}` for administrator-driven deduplication after the fact.
4. **Short term**: at minimum, normalize the comparison to case-insensitive with trimming.
**Files:** `PromotionService.cs:266-295` (CreateOrUpdatePatientAsync).
**Dependency:** None.
---
## P1 — Batch assignment creates inconsistent interim state
### Problem
`BatchService.AssignAsync` sets `EnteredByUserId` on a batch in `UPLOADED` status and acquires a Redis lock, but does **not** transition the batch to `IN_ENTRY` status. The status transition only happens later when `DraftService` processes the first draft save. Between assignment and first save, the batch is in `UPLOADED` status with an assigned user — a state not represented in the PRD's status machine.
If the Redis lock expires (1-hour TTL) before the clerk saves any draft, another assignment call could succeed but the original `EnteredByUserId` is already set on the batch row. The second `AssignAsync` would set a new `EnteredByUserId` without clearing the Redis lock from the first assignment (the lock already expired).
### Why fix
The entry work queue (`GET /api/v1/work-queue/entry`) queries for batches in `UPLOADED`, `IN_ENTRY`, and `REJECTED` statuses. A batch in `UPLOADED` status with `EnteredByUserId` set appears in the entry queue as if it's being worked on, but the status suggests it hasn't been started. Supervisors cannot distinguish between "assigned but not started" and "not yet assigned" from the status alone.
### How to fix
1. **Transition to `IN_ENTRY` during assignment**: `AssignAsync` should set `batch.Status = BatchStatus.InEntry` immediately after acquiring the Redis lock and setting `EnteredByUserId`.
2. **Remove the implicit transition in DraftService**: `DraftService` currently transitions `UPLOADED → IN_ENTRY` on first save. Since assignment now does this, the DraftService check becomes redundant (keep the `IN_ENTRY` status check as a guard).
3. **Handle Redis lock expiry**: add a `PATCH .../reassign` endpoint (or extend `AssignAsync` for `IN_ENTRY` batches) that clears the old Redis lock, acquires a new one, updates `EnteredByUserId`, and writes a `DigitizationEvent` for reassignment.
4. Validate that the work queue entry view only shows `IN_ENTRY` and `REJECTED` batches for the entry clerk, not `UPLOADED`.
**Files:** `BatchService.cs:192-229` (AssignAsync), `DraftService.cs` (first-save transition logic).
**Dependency:** None.
---
## P1 — No duplicate detection on concurrent batch creation
### Problem
`BatchService.CreateAsync` checks for duplicate documents by querying `DocumentSha256` for the same patient within 24 hours. However, two concurrent upload requests with the same file for the same patient can both pass the `AnyAsync` check before either commits. No database-level unique constraint on `(DocumentSha256, PatientId)` with a time window exists.
### Why fix
FHIR integration engines or intake automation scripts submitting the same scan file simultaneously could create duplicate batches. While not a clinical safety issue (duplicate entry would be caught at verification), it wastes verifier time and clutters the work queue.
### How to fix
1. Add a database-level unique partial index: `CREATE UNIQUE INDEX IX_batch_sha256_patient_24h ON digitization_batches (document_sha256, patient_id) WHERE created_at >= NOW() - INTERVAL '24 hours'` — note: PostgreSQL partial indexes with volatile expressions are not directly supported; instead, use an advisory lock or a dedicated deduplication table.
2. Alternative: use a Redis `SET NX` with key `batch:dedup:{sha256}:{patientId}` and 24-hour TTL as a first-line check. The database `AnyAsync` remains as a fallback.
3. Wrap the upload + batch insert in a serializable transaction scope for the dedup check.
**Files:** `BatchService.cs:71-85` (duplicate detection), `DigitizationBatchConfiguration.cs` (add index).
**Dependency:** None.
---
# Part B — Infrastructure & Reliability
---
## P2 — No health check endpoints
### Problem
The API has no `/health`, `/ready`, or `/startup` endpoints. There are no health checks for PostgreSQL, Redis, or MinIO connectivity.
### Why fix
Container orchestrators (Docker Compose health checks, Kubernetes probes) cannot detect unhealthy instances. If PostgreSQL or Redis becomes unreachable after startup, the API continues accepting requests that will fail with 500 errors. Load balancers cannot route traffic away from degraded instances.
### How to fix
1. Add `Microsoft.Extensions.Diagnostics.HealthChecks` and provider packages:
- `AspNetCore.HealthChecks.NpgSql` (PostgreSQL)
- `AspNetCore.HealthChecks.Redis` (Redis)
2. Create a custom `MinioHealthCheck` that calls `BucketExistsAsync`.
3. Register health checks in `Program.cs` with tags: `startup`, `liveness`, `readiness`.
4. Map endpoints:
- `GET /health/live` — is the process alive?
- `GET /health/ready` — are PostgreSQL, Redis, and MinIO reachable?
- `GET /health/startup` — has migration and seed completed?
5. Add health check responses to Prometheus via `AspNetCore.HealthChecks.Publisher.Prometheus`.
6. Update `docker-compose.yml` with health check configuration on the API service.
**Files:** `Program.cs`, `VigilCareRecordsAPI.csproj`, optional `MinioHealthCheck.cs`.
**Dependency:** None.
---
## P2 — No CORS configuration for production deployment
### Problem
The API has no CORS configuration. The Vue frontend works in development via Vite's proxy (`/api → localhost:5217`), but in production where the frontend is served from a different origin (e.g., `https://records.vigilcare.local` vs `https://api.vigilcare.local`), API requests will be blocked by the browser's same-origin policy.
### Why fix
Without CORS, the digitization workstation cannot call the API in any deployment topology where the frontend is served from a different domain, port, or protocol than the API. This blocks every non-dev deployment.
### How to fix
1. Add CORS configuration to `Program.cs`:
```csharp
builder.Services.AddCors(options =>
{
options.AddPolicy("VigilCare", policy =>
{
policy.WithOrigins(builder.Configuration.GetSection("Cors:AllowedOrigins").Get<string[]>()!)
.AllowAnyHeader()
.AllowAnyMethod()
.AllowCredentials();
});
});
```
2. Add `app.UseCors("VigilCare")` before `app.UseAuthentication()`.
3. Add `Cors:AllowedOrigins` to `appsettings.json` (default: `["http://localhost:3028"]` for dev).
4. Document production CORS configuration in README.
**Files:** `Program.cs`, `appsettings.json`.
**Dependency:** None.
---
## P2 — Redis connection failure crashes startup
### Problem
`Program.cs` calls `ConnectionMultiplexer.Connect(...)` synchronously during DI registration. If Redis is unreachable at startup, this throws an unhandled `RedisConnectionException` that crashes the application. There is no retry logic.
### Why fix
Redis is used for batch assignment locks and alert threshold caching — important but not essential for core API functionality. A transient Redis outage during deployment or container restart should not prevent the entire API from starting. The batch assignment lock is a convenience feature; the separation-of-duties enforcement at the service layer is the true guard.
### How to fix
1. Replace synchronous connection with lazy initialization:
```csharp
builder.Services.AddSingleton<IConnectionMultiplexer>(sp =>
{
var config = ConfigurationOptions.Parse(
sp.GetRequiredService<IConfiguration>()["Redis:ConnectionString"]!);
config.AbortOnConnectFail = false;
return ConnectionMultiplexer.Connect(config);
});
```
2. Setting `AbortOnConnectFail = false` allows the multiplexer to be created even if Redis is unreachable — operations will fail gracefully until Redis recovers.
3. In `BatchService.AssignAsync`, catch `RedisConnectionException` and log a warning; optionally fall back to a database-level advisory lock.
4. In `LiveCaptureService`, handle Redis cache miss for thresholds (already falls back to database query).
**Files:** `Program.cs:26-27`, `BatchService.cs:204`.
**Dependency:** None.
---
## P2 — PromotionRetryService does not record failure metrics
### Problem
`PromotionRetryService` logs errors on retry failure but does not increment any Prometheus counter. There is no metric for retry attempts, retry successes, retry failures, or retries exhausted. The existing `DiagnosticsMetrics` class does not have any retry-related metrics.
### Why fix
Operators monitoring Prometheus/Grafana have no visibility into whether deferred promotions are succeeding or failing on retry. A sustained promotion failure (e.g., due to a configuration issue) would only be visible in Seq logs, not in dashboards or alerts. This delays incident detection.
### How to fix
1. Add metrics to `DiagnosticsMetrics`:
```csharp
public static readonly Counter PromotionRetryTotal = Metrics.CreateCounter(
"digitization_promotion_retry_total",
"Total promotion retry attempts.",
new CounterConfiguration { LabelNames = new[] { "outcome" } }); // success, failure, exhausted
```
2. In `PromotionRetryService.RetryPromotionAsync`:
- On success: `DiagnosticsMetrics.PromotionRetryTotal.WithLabels("success").Inc()`.
- On failure: `DiagnosticsMetrics.PromotionRetryTotal.WithLabels("failure").Inc()`.
- On exhausted: `DiagnosticsMetrics.PromotionRetryTotal.WithLabels("exhausted").Inc()`.
3. Add a gauge for pending retry count: `digitization_promotion_pending_retries` (update in MetricsCollectorService).
**Files:** `DiagnosticsMetrics.cs`, `PromotionRetryService.cs:117-175`, `MetricsCollectorService.cs`.
**Dependency:** None.
---
# Part C — Security & Hardening
---
## P3 — JWT signing key not validated on startup
### Problem
`Program.cs` reads `JwtOptions.Secret` from configuration and creates a `SymmetricSecurityKey` without validating minimum length. A short or empty key causes a runtime `SecurityTokenInvalidSignatureException` on the first authentication attempt, not at startup. The dev default key (`VigilCareRecordsDevSecretKeyAtLeast32Chars!`) is 43 characters, which is sufficient, but nothing prevents a production deployment from using a shorter key.
### Why fix
Fail-fast on misconfiguration prevents deploying a service that silently rejects all authentication. In production, a weak key means all JWTs can be forged by an attacker who brute-forces the HMAC.
### How to fix
1. Add a startup validation check in `Program.cs` after binding `JwtOptions`:
```csharp
if (string.IsNullOrEmpty(jwtOptions.Secret) ||
Encoding.UTF8.GetByteCount(jwtOptions.Secret) < 32)
throw new InvalidOperationException(
"JWT Secret must be at least 256 bits (32 bytes).");
```
2. Optionally implement `IValidateOptions<JwtOptions>` for structured validation.
**Files:** `Program.cs:45-58`, optionally `JwtOptions.cs`.
**Dependency:** None.
---
## P3 — No rate limiting on authentication endpoints
### Problem
The `POST /api/v1/auth/login` endpoint has no rate limiting. An attacker can attempt unlimited password guesses against known usernames (seeded users have predictable usernames: `intake1`, `entry1`, `verifier1`, etc.). There is no account lockout mechanism after failed attempts.
### Why fix
The API handles PHI (Protected Health Information). Brute-force attacks on auth endpoints are a standard OWASP risk. With all seeded users sharing the password `password`, a single guess compromises any account.
### How to fix
1. **Rate limiting**: Add `AspNetCoreRateLimit` or the built-in .NET 7+ `AddRateLimiter()`:
```csharp
builder.Services.AddRateLimiter(options =>
{
options.AddFixedWindowLimiter("auth", opt =>
{
opt.Window = TimeSpan.FromMinutes(5);
opt.PermitLimit = 10;
opt.QueueProcessingOrder = QueueProcessingOrder.OldestFirst;
});
});
```
2. Apply `[EnableRateLimiting("auth")]` to login and refresh endpoints.
3. **Account lockout**: Add `FailedLoginAttempts` and `LockedUntil` columns to `User`. After 5 failed attempts in 15 minutes, lock the account for 30 minutes.
4. Return `429 Too Many Requests` on rate limit, `423 Locked` on account lockout.
**Files:** `Program.cs`, `AuthController.cs`, `AuthService.cs`, `User.cs`, migration.
**Dependency:** None.
---
## P3 — Credentials stored in plaintext in appsettings.json
### Problem
`appsettings.json` contains plaintext credentials committed to source control:
- JWT signing key: `VigilCareRecordsDevSecretKeyAtLeast32Chars!`
- PostgreSQL password: `password`
- MinIO access/secret keys: `minioadmin`/`minioadmin`
- Redis connection string (no auth)
### Why fix
Anyone with repository access can extract production credentials if the same configuration pattern is used in deployment. Even for development, committed secrets create a precedent that normalizes insecure practices.
### How to fix
1. Move all secrets to environment variables or a secrets manager.
2. Add `appsettings.Development.json` with dev-only defaults (already gitignored by convention).
3. Add `appsettings.Production.json.example` documenting required secrets.
4. Document environment variable overrides:
- `ConnectionStrings__DefaultConnection`
- `Jwt__Secret`
- `Minio__AccessKey`, `Minio__SecretKey`
5. Add a `.env.example` file for docker-compose that documents required secrets without values.
6. Consider Azure Key Vault or HashiCorp Vault integration for production.
**Files:** `appsettings.json`, new `.env.example`, `docker-compose.yml`.
**Dependency:** None.
---
## P3 — No audit of document access
### Problem
The PRD explicitly requires logging "who viewed a scan and when" as an audit requirement (Section 9: Authentication and Audit). The current implementation generates presigned URLs in `BatchService.GetByIdAsync` and `DocumentStorageService.GetPresignedUrlAsync` without recording who requested the URL or when.
### Why fix
HIPAA and clinical compliance audits require demonstrating that access to PHI (including scanned patient documents) is logged. Without document access logging, there is no evidence of who viewed which patient's scan, making the system non-compliant with the PRD's stated audit requirements.
### How to fix
1. Add a `DigitizationEventType.DocumentAccessed` enum value.
2. In `DigitizationBatchesController.GetById`, after generating the presigned URL, write a `DigitizationEvent`:
```csharp
_db.DigitizationEvents.Add(new DigitizationEvent
{
Id = Guid.NewGuid(),
BatchId = id,
EventType = DigitizationEventType.DocumentAccessed,
ActorUserId = userId,
OccurredAt = DateTimeOffset.UtcNow,
MetadataJson = JsonSerializer.Serialize(new { expiresAt = presignedUrlExpiry })
});
```
3. Consider rate-limiting audit writes: if the same user accessed the same batch within the last 5 minutes, skip the duplicate event (prevents audit noise during entry/verification when the page auto-refreshes the URL).
**Files:** `DigitizationBatchesController.cs`, `DigitizationEventType.cs`, migration (enum update).
**Dependency:** None.
---
# Part D — API Completeness & Consistency
---
## P2 — No FluentValidation for request DTOs
### Problem
The API relies entirely on inline validation in service methods and controllers. There are no `FluentValidation` validators for any of the 15+ request DTOs (`CreateBatchRequest`, `UpsertDraftPatientRequest`, `UpsertDraftEncounterRequest`, `CreateDraftObservationRequest`, `VerifyBatchRequest`, `RejectBatchRequest`, `ApproveRequest`, `RecordObservationsRequest`, `OpenEncounterWithVitalsRequest`, `LoginRequest`, etc.).
Invalid request bodies reach the service layer before being rejected, which means:
- Missing required fields cause `NullReferenceException` instead of 422 responses.
- String length violations hit database constraints instead of validation errors.
- Validation error messages are inconsistent across endpoints.
### Why fix
Consistent request validation is essential for integration reliability. FHIR integration engines, the Vue frontend, and CLI scripts all need predictable error shapes when submitting invalid data. Database constraint violations surfacing as 500 errors instead of 422s make debugging integration issues significantly harder.
### How to fix
1. Add `FluentValidation.AspNetCore` package.
2. Create validators for all request DTOs. Priority validators:
- `LoginRequestValidator`: username required, password required (min 1 char)
- `UpsertDraftPatientRequestValidator`: fullName max 200, DOB not future
- `UpsertDraftEncounterRequestValidator`: admissionDate not future, department valid enum
- `CreateDraftObservationRequestValidator`: observationCode required, value required, recordedAt required and not future
- `VerifyBatchRequestValidator`: fieldChecks non-empty, each status is valid
- `RejectBatchRequestValidator`: reason required, min 10 chars (already enforced in service)
- `RecordObservationsRequestValidator`: observations non-empty, max 10, each has code/value/unit/recordedAt
3. Register validators with auto-discovery: `builder.Services.AddValidatorsFromAssemblyContaining<Program>()`.
4. Add `app.UseFluentValidationExceptionHandler()` or integrate with the existing `ExceptionHandlerMiddleware`.
**Files:** New `Validators/` directory, `Program.cs`, `VigilCareRecordsAPI.csproj`.
**Dependency:** None.
---
## P4 — No user management endpoints (create, update, deactivate)
### Problem
`UsersController` has only `GET /api/v1/users?role=` to list users. There are no endpoints to create users, update user details, change passwords, or deactivate accounts. User management is only possible through the `DataSeeder` or direct database access.
The PRD assigns the Administrator role permission for "User management, batch type config, retroactive alert policy, work-queue reassignment."
### Why fix
In a production deployment, new staff must be onboarded (new data entry clerks, verifiers) and departing staff must be deactivated. Without user management endpoints, every personnel change requires direct database access and a service restart to re-seed, which is unacceptable for a clinical system.
### How to fix
1. Add endpoints to `UsersController`:
- `POST /api/v1/users` — create user (admin only): username, password, fullName, role
- `PATCH /api/v1/users/{id}` — update user details: fullName, role, isActive
- `POST /api/v1/users/{id}/reset-password` — admin password reset
- `POST /api/v1/users/{id}/change-password` — self-service (current + new password)
2. Create `UserService` with `CreateAsync`, `UpdateAsync`, `ResetPasswordAsync`, `ChangePasswordAsync`.
3. Add `CreateUserRequest`, `UpdateUserRequest`, `ChangePasswordRequest` DTOs with FluentValidation validators.
4. Enforce password complexity: min 8 chars, at least 1 uppercase, 1 digit.
5. Log user management actions as `AuthAuditEvent` entries.
**Files:** `UsersController.cs`, new `UserService.cs`, new DTOs in `Models/Records/User/`.
**Dependency:** None.
---
## P4 — No batch cancel/void operation
### Problem
Once a batch is created, it can only move forward through the status machine or be rejected (which returns it to entry). There is no way to permanently cancel or void a batch that was created in error (wrong patient, wrong batch type, test upload). The status machine has no terminal state other than `PROMOTED`.
### Why fix
Intake clerks create batches by uploading scans. Mistakes happen: wrong document scanned, wrong patient linked, test uploads during training. Without a cancel operation, these batches permanently occupy the work queue, cluttering the entry and verification views. Supervisors must use direct database access to clean up.
### How to fix
1. Add `BatchStatus.Cancelled` as a terminal state (alongside `Promoted`).
2. Add allowed transitions: `UPLOADED → CANCELLED`, `IN_ENTRY → CANCELLED`, `REJECTED → CANCELLED`.
3. Add `POST /api/v1/digitization-batches/{id}/cancel` — body: `{ "reason": "..." }` — restricted to `ADMINISTRATOR` role.
4. Write `DigitizationEvent` with `EventType.Cancelled` and the reason.
5. Release Redis assignment lock on cancellation if one exists.
6. `CANCELLED` batches are excluded from work queue queries.
**Files:** `BatchService.cs` (add allowed transitions), `DigitizationBatchesController.cs`, `BatchStatus.cs`, `DigitizationEventType.cs`, migration.
**Dependency:** None.
---
## P4 — Pagination missing `sortBy` and `sortDirection` parameters
### Problem
All list endpoints (`GET /digitization-batches`, work queue endpoints) sort by `CreatedAt DESC` or `UpdatedAt ASC` with no user-configurable sort. The supervisor dashboard cannot sort batches by status, type, patient, or age.
### Why fix
Supervisors managing a work queue of 50+ batches need to sort by different criteria: oldest first for urgency, by type for batch processing, by assigned clerk for workload review. The Vue frontend's QueueDashboardView shows all batches in a flat list with no sort controls.
### How to fix
1. Add `sortBy` and `sortDirection` query parameters to `GET /api/v1/digitization-batches` and all work queue endpoints.
2. Supported sort fields: `createdAt`, `updatedAt`, `status`, `batchType`, `track`.
3. Default: `createdAt DESC`.
4. Validate sort field against allowed list; reject unknown fields with 422.
5. Update `BatchService.ListAsync` and `WorkQueueService` to apply dynamic ordering.
**Files:** `BatchService.cs:167-190`, `WorkQueueService.cs`, `DigitizationBatchesController.cs`, `WorkQueueController.cs`.
**Dependency:** None.
---
# Part E — Vue Frontend Gaps
---
## P2 — No clinical approval view
### Problem
The PRD defines a distinct clinical approval step for high-stakes batch types (vitals, labs, encounter summaries, medications, mixed). The backend implements `GET /api/v1/work-queue/clinical-approval` and `POST .../approve` endpoints. However, the Vue frontend has no dedicated view for clinical approvers. The router assigns `CLINICAL_APPROVER` to the verification routes, meaning approvers use the same `VerificationView` that verifiers use.
The `VerificationForm` component has approve/reject buttons but calls `verifyBatch()` (which posts to `POST .../verify`), not `approveBatch()` (which posts to `POST .../approve` with an `Idempotency-Key`). There is no UI for the `AWAITING_CLINICAL_APPROVAL` queue.
### Why fix
Clinical approvers (typically physicians) have a different workflow from verifiers. They are not comparing data entry against a scan — they are authorizing promotion of clinical data into the live system. Conflating the two roles in one view means clinical approval batches either sit in the verification queue unnoticed or are processed using the wrong action.
### How to fix
1. Add `ApprovalView.vue` at `/approval` route, accessible to `CLINICAL_APPROVER` and `ADMINISTRATOR`.
2. The view loads batches from `GET /api/v1/work-queue/clinical-approval`.
3. Each batch shows: patient summary, encounter context, observations (read-only), verification pass details, verifier name.
4. Approve button calls `POST .../approve` with a generated `Idempotency-Key` header.
5. Reject button calls `POST .../reject` with reason.
6. Show promotion result (patient MRN, encounter ID, observation count) on success.
7. Handle 202 `PROMOTION_DEFERRED` — show a banner: "Approved. Promotion will be retried automatically."
8. Add the route to the router with `meta.roles: ['CLINICAL_APPROVER', 'ADMINISTRATOR']`.
9. Update `AppHeader.vue` navigation to include the approval link.
**Files:** New `views/ApprovalView.vue`, `router/index.ts`, `stores/batches.ts` (add `approveBatch` action), `components/AppHeader.vue`.
**Dependency:** None.
---
## P2 — No live capture view for bedside data entry
### Problem
The PRD describes Track B live capture as a core feature: "Credentialed clinician enters vitals or labs at point of care on a tablet." The backend implements `POST /api/v1/live-capture/encounters/{encounterId}/observations` and `POST /api/v1/live-capture/encounters` with clinician attestation and synchronous critical alert evaluation.
The Vue frontend has no live capture view. The router has no `/live-capture` route. Clinicians (`CLINICIAN` role) can log in but are redirected to `/login` because there is no default route for their role, and no route allows `CLINICIAN` access.
### Why fix
Live capture is the path from paper-to-digital for current patient care. Without a bedside UI, clinicians cannot use the system for real-time vital sign entry — the primary workflow that makes VigilCare actionable in a clinical setting. The backend is fully implemented but inaccessible through the frontend.
### How to fix
1. Add `LiveCaptureView.vue` at `/live-capture`, accessible to `CLINICIAN` and `ADMINISTRATOR`.
2. The view provides:
- Patient search (reuse `PatientSearch` component)
- Active encounter selector (or create-new-encounter flow)
- Compact vitals entry form (optimized for tablet):
- Observation rows for standard codes (HR, Temp, BP, RR, SpO2)
- Password re-confirmation field (PRD requires password or PIN)
- Clinician attestation checkbox
- Submit button that calls `POST /api/v1/live-capture/encounters/{encounterId}/observations`
- Critical alert display: if the response includes synchronous alerts, show them prominently with severity, threshold details, and recommended actions
3. Add `POST /api/v1/live-capture/encounters` flow for creating a new encounter with initial vitals.
4. Optimize for tablet: large touch targets, minimal scrolling, landscape layout.
5. Update router default route for `CLINICIAN` → `/live-capture`.
**Files:** New `views/LiveCaptureView.vue`, `router/index.ts`, `stores/batches.ts` or new `stores/liveCapture.ts`, `components/AppHeader.vue`.
**Dependency:** None.
---
## P4 — EntryForm missing allergy, medication, and discharge fields
### Problem
The `EntryForm.vue` component captures patient demographics (name, DOB, sex, blood type, emergency contact), encounter context (admission date, department, room/bed, admission reason), and observations. However, it does not render fields for:
- **Allergies** (`allergiesJson`, `noKnownAllergies`) — required for `ALLERGY_UPDATE` batches
- **Medications** (`medicationsJson`, `noActiveMedications`) — required for `MEDICATION_LIST` batches
- **Discharge diagnosis** — part of the encounter context
- **Encounter status** (active/discharged) — affects encounter creation during promotion
The backend `DraftService` validates completeness for these batch types and will reject submission with `BATCH_INCOMPLETE`, but the entry clerk has no way to enter the required data.
### Why fix
Two of the seven batch types (`ALLERGY_UPDATE` and `MEDICATION_LIST`) cannot be completed through the UI. Entry clerks working on these batch types will be blocked at submission with a validation error they cannot resolve without backend support.
### How to fix
1. Add conditional field sections to `EntryForm.vue` based on `batchType`:
- **All types**: patient demographics (existing), encounter context (existing)
- **ALLERGY_UPDATE**: allergies list (add/remove items) + `noKnownAllergies` checkbox
- **MEDICATION_LIST**: medications JSON editor (add/remove entries) + `noActiveMedications` checkbox
- **ENCOUNTER_SUMMARY**: discharge diagnosis text field, encounter status toggle
- **MIXED**: all fields visible
2. Update `saveDraftPatient` to include `allergiesJson` and `noKnownAllergies`.
3. Add a medications section that saves to the draft patient's `medicationsJson` field.
4. The verification form should also render these fields for review.
**Files:** `components/EntryForm.vue`, `components/VerificationForm.vue`, `types/index.ts`.
**Dependency:** None.
---
## P4 — No corrections/supersession UI
### Problem
The PRD describes a correction workflow: "A correction creates a new batch with `supersedesBatchId` pointing to the original." The backend fully implements supersession: upload with `supersedesBatchId`, promotion marks original observations as superseded, and `GET /patients/{id}/digitization-history` shows correction chains.
The Vue frontend has no UI to:
- Create a correction batch (upload with `supersedesBatchId`)
- View which batch superseded which
- See superseded observations vs active observations in patient history
- Navigate the correction chain
### Why fix
Corrections are a daily clinical workflow. When a promoted batch has an error (wrong potassium value, incorrect admission date), the only recourse is direct API calls or database access. Without a corrections UI, the audit trail — one of the system's core value propositions — cannot be maintained through normal operator workflow.
### How to fix
1. Add a "Create Correction" button on promoted batch detail views.
2. The button pre-fills `supersedesBatchId` and `patientId` in the upload form.
3. Add a `PatientHistoryView.vue` at `/patients/{id}/history`:
- Timeline showing all batches for the patient
- Correction chains visualized (original → correction arrows)
- Superseded observations shown with strikethrough styling
- Active vs superseded observation counts
4. Add batch detail panel showing supersession info when `supersedesBatchId` is set.
**Files:** New `views/PatientHistoryView.vue`, `components/IntakeView.vue` (correction button), `router/index.ts`, `stores/batches.ts`.
**Dependency:** None.
---
## P4 — No toast notification system or success feedback
### Problem
The Vue frontend has no toast/snackbar notification system. All success and error feedback is either:
- Inline `errorMessage` refs that render as red text below forms
- Implicit (redirect after success with no confirmation)
- Console errors (invisible to users)
Operations like "batch submitted for verification," "batch verified," "observation saved," and "batch rejected" provide no positive feedback to the user.
### Why fix
Clinical data entry is high-stakes work. Entry clerks need immediate confirmation that their saves succeeded, verifiers need confirmation that rejections were sent, and approvers need clear feedback that promotion completed (or was deferred). Silent success breeds anxiety and repeat submissions.
### How to fix
1. Add a toast notification composable (`useToast`) or install `vue-toastification`.
2. Show success toasts for: observation saved, draft auto-saved, batch submitted, batch verified, batch rejected, batch approved.
3. Show error toasts for: API failures, validation errors, auth errors.
4. Show warning toasts for: presigned URL about to expire, batch already assigned.
5. Show info toasts for: promotion deferred (202), session refreshed.
6. Toast position: top-right, auto-dismiss after 4 seconds.
**Files:** New `composables/useToast.ts` or install package, update all views/components.
**Dependency:** None.
---
## P4 — No frontend tests
### Problem
The Vue frontend has zero tests. No unit tests (Vitest), no component tests (Vue Test Utils), and no end-to-end tests (Cypress/Playwright). `package.json` does not include any test dependencies or scripts.
### Why fix
The frontend handles critical clinical workflows: data entry, verification, approval. Untested UI components mean regressions in form validation, API calls, auth flow, and role-based routing go undetected until a user encounters them in production.
### How to fix
1. Add Vitest + Vue Test Utils as dev dependencies.
2. Priority unit tests:
- Auth store: login flow, token refresh, logout, role-based permissions
- Batch store: CRUD operations, error handling, pagination
- Router guards: unauthenticated redirect, role-based access
3. Priority component tests:
- `EntryForm`: renders fields, saves on blur, validates observations, submits
- `VerificationForm`: all-checked enables approve, reject requires reason
- `PatientSearch`: debounced search, selection emits event
- `ObservationRow`: renders observation codes, value validation
4. Add a `test` script to `package.json`: `"test": "vitest"`.
5. Optionally add Playwright for E2E tests of the full login → entry → verify → approve flow.
**Files:** `package.json`, `vitest.config.ts`, new `src/__tests__/` directory.
**Dependency:** None.
---
# Part F — Observability & Test Coverage
---
## P5 — No audit of field-level draft changes
### Problem
The PRD requires "Who changed which draft field (field-level diff in event metadata on save)" as an audit requirement. The current `DraftService` saves draft data (patient, encounter, observations) but does not record which fields changed or the before/after values. The `DigitizationEvent` for `EntryStarted` is written once; subsequent saves produce no events.
### Why fix
In a clinical data quality dispute ("the verifier says the temperature was entered as 38.7 but the original chart shows 37.7"), there is no way to determine whether the entry clerk made the error or whether the value was changed after initial entry. Field-level audit trail is essential for clinical accountability.
### How to fix
1. In `DraftService` save methods, load the existing draft before applying updates.
2. Compare each field; build a `fieldsChanged` list with `{ field, oldValue, newValue }`.
3. If any field changed, write a `DigitizationEvent` with `EventType.DraftFieldUpdated` and the field diff in `MetadataJson`.
4. Debounce at the service level: if a `DraftFieldUpdated` event was written for the same batch within the last 30 seconds, update the existing event's metadata rather than creating a new row (prevents audit noise from auto-save).
**Files:** `DraftService.cs`, `DigitizationEventType.cs` (add `DraftFieldUpdated`).
**Dependency:** None.
---
## P5 — Integration test gaps for deferred promotion and retry
### Problem
The test suite has 45 tests across 5 files: `DraftEntryTests` (7), `VerificationTests` (11), `PromotionTests` (10), `CorrectionSupersessionTests` (5), `LiveCaptureIntegrationTests` (12). Key untested scenarios:
1. **Deferred promotion**: no test for the 202 `PROMOTION_DEFERRED` path — when `ApproveAndPromoteAsync` throws an infrastructure exception, the controller should set batch to `APPROVED` and create a `PromotionAttempt`.
2. **Promotion retry**: no test for `PromotionRetryService` picking up deferred batches and retrying.
3. **Retry exhaustion**: no test for the retry limit being reached (batch stuck in `APPROVED` permanently).
4. **Concurrent batch creation**: no test for two simultaneous uploads with the same SHA-256.
5. **Concurrent assignment**: no test for two `PATCH .../assign` calls for the same batch.
6. **Auth flow**: no tests for login, token refresh, logout, or role-based access control.
7. **Work queue ordering**: no test for FIFO ordering of work queue items.
### Why fix
The deferred promotion path (P0 issue above) is the most critical untested code path. Without integration tests, the fix for the `PromoteAsync` gap cannot be verified. Concurrent operation tests are needed to validate the Redis-based assignment lock and SHA-256 deduplication under contention.
### How to fix
1. **Deferred promotion tests** (validates P0 fix):
- Simulate infrastructure failure during `ApproveAndPromoteAsync` → verify batch is `APPROVED` with `PromotionAttempt` → verify retry creates clinical entities.
- Verify exhausted retries leave batch in `APPROVED` with no `NextRetryAt`.
2. **Concurrent operation tests**:
- Parallel `PATCH .../assign` for same batch → verify exactly one succeeds.
- Parallel upload with same SHA-256 and patient → verify exactly one succeeds.
3. **Auth tests**:
- Login with valid/invalid credentials → verify tokens.
- Refresh with valid/expired/revoked token → verify behavior.
- Access protected endpoint without token → verify 401.
- Access role-restricted endpoint with wrong role → verify 403.
4. **Work queue tests**:
- Submit 3 batches at different times → verify verification queue returns oldest first.
**Files:** New `PromotionRetryTests.cs`, `ConcurrencyTests.cs`, `AuthTests.cs`, `WorkQueueTests.cs` in `VigilCareRecordsAPI.Tests/`.
**Dependency:** P0 fix (PromoteAsync).
---
## P5 — MetricsCollectorService does not track APPROVED or retry-pending batches
### Problem
`MetricsCollectorService` collects `digitization_batches_by_status` gauge for all 8 statuses and `digitization_queue_age_seconds` for the oldest `PENDING_VERIFICATION` batch. However, it does not report:
- Age of the oldest `APPROVED` batch (waiting for promotion retry)
- Count of `PromotionAttempt` records with `NextRetryAt` in the past (overdue retries)
- Count of exhausted retries (`NextRetryAt IS NULL AND NOT Succeeded`)
### Why fix
A batch stuck in `APPROVED` with exhausted retries is invisible in Prometheus dashboards. Operators won't know that a patient's vitals are trapped in limbo unless they query the database directly or check Seq logs.
### How to fix
1. Add gauge: `digitization_promotion_pending_retries` — count of `PromotionAttempt` records where `!Succeeded && NextRetryAt != null`.
2. Add gauge: `digitization_promotion_exhausted_total` — count of batches in `APPROVED` with all `PromotionAttempt` records having `NextRetryAt == null`.
3. Add gauge: `digitization_approval_queue_age_seconds` — age of oldest `APPROVED` batch (mirrors `queue_age_seconds` pattern).
4. Update `MetricsCollectorService.CollectMetricsAsync` to query these metrics.
**Files:** `DiagnosticsMetrics.cs`, `MetricsCollectorService.cs`.
**Dependency:** None.
---
# Summary matrix
| # | Issue | Priority | Part | Status |
|---|-------|----------|------|--------|
| 1 | PromoteAsync missing clinical entities | P0 | A | Done |
| 2 | Patient dedup by exact name+DOB | P0 | A | Done |
| 3 | Batch assignment inconsistent state | P1 | A | Done |
| 4 | Concurrent batch creation race | P1 | A | Open |
| 5 | No health check endpoints | P2 | B | Open |
| 6 | No CORS configuration | P2 | B | Open |
| 7 | Redis failure crashes startup | P2 | B | Open |
| 8 | Promotion retry metrics missing | P2 | B | Open |
| 9 | JWT key not validated on startup | P3 | C | Open |
| 10 | No rate limiting on auth | P3 | C | Open |
| 11 | Credentials in plaintext config | P3 | C | Open |
| 12 | No document access audit | P3 | C | Open |
| 13 | No FluentValidation | P2 | D | Open |
| 14 | No user management endpoints | P4 | D | Open |
| 15 | No batch cancel/void | P4 | D | Open |
| 16 | No sort parameters on lists | P4 | D | Open |
| 17 | No clinical approval view | P2 | E | Open |
| 18 | No live capture view | P2 | E | Open |
| 19 | EntryForm missing allergy/med fields | P4 | E | Open |
| 20 | No corrections/supersession UI | P4 | E | Open |
| 21 | No toast/notification system | P4 | E | Open |
| 22 | No frontend tests | P4 | E | Open |
| 23 | No field-level draft audit | P5 | F | Open |
| 24 | Integration test gaps | P5 | F | Open |
| 25 | MetricsCollector missing retry gauges | P5 | F | Open |
---
## Suggested implementation sequence
```mermaid
flowchart TD
subgraph correctness [Part A — Correctness]
P0A[P0: Unify PromoteAsync]
P0B[P0: Patient dedup normalization]
P1A[P1: Assignment state transition]
P1B[P1: Concurrent batch creation guard]
end
subgraph infra [Part B — Infrastructure]
P2H[P2: Health checks]
P2C[P2: CORS configuration]
P2R[P2: Redis graceful startup]
P2M[P2: Promotion retry metrics]
end
subgraph security [Part C — Security]
P3J[P3: JWT key validation]
P3R[P3: Rate limiting]
P3S[P3: Secrets management]
P3D[P3: Document access audit]
end
subgraph api [Part D — API]
P2V[P2: FluentValidation]
P4U[P4: User management]
P4X[P4: Batch cancel/void]
P4S[P4: Sort parameters]
end
subgraph vue [Part E — Vue Frontend]
P2A[P2: Clinical approval view]
P2L[P2: Live capture view]
P4E[P4: Entry form fields]
P4F[P4: Corrections UI]
P4T[P4: Toast notifications]
P4FT[P4: Frontend tests]
end
subgraph obs [Part F — Observability]
P5D[P5: Field-level draft audit]
P5T[P5: Integration test gaps]
P5M[P5: Metrics collector gaps]
end
P0A --> P5T
P2A --> P4E
P2L --> P4T
P2V --> P4U
```
### Sprint-sized batches
| Batch | Items | Outcome |
|-------|-------|---------|
| **1 — Correctness** | P0 unify PromoteAsync, P0 patient dedup normalization, P1 assignment state transition, P1 concurrent batch creation guard | Promotion retry creates complete clinical records; no duplicate patients from name variation |
| **2 — Infrastructure** | P2 health checks, P2 CORS, P2 Redis graceful startup, P2 promotion retry metrics | Production-deployable infrastructure; orchestration-ready |
| **3 — Security** | P3 JWT validation, P3 rate limiting, P3 secrets management, P3 document access audit | Compliance-ready auth; HIPAA audit trail for document access |
| **4 — API hardening** | P2 FluentValidation, P4 user management, P4 batch cancel, P4 sort parameters | Admin UI and integration teams unblocked; consistent validation |
| **5 — Vue frontend** | P2 clinical approval view, P2 live capture view, P4 entry form fields, P4 corrections UI, P4 toast notifications | All PRD workflows accessible through the UI |
| **6 — Observability** | P5 field-level audit, P5 integration tests, P5 metrics collector gaps, P4 frontend tests | Full audit trail; regression safety net for Batch 1 fixes |
---
## Testing strategy (cross-cutting)
For each fix, add or extend tests in `VigilCareRecordsAPI.Tests/`:
- **Promotion tests** (Batch 1): Deferred promotion → retry → verify clinical entities created; patient name normalization dedup; concurrent assignment race.
- **Infrastructure tests** (Batch 2): Health check endpoints return expected status; CORS headers present on cross-origin requests; app starts with Redis down.
- **Security tests** (Batch 3): Rate limit on login; JWT with short key rejected at startup; document access event written on batch detail.
- **API validation tests** (Batch 4): FluentValidation returns 422 with field-level errors; user CRUD lifecycle; batch cancel transitions.
- **Frontend tests** (Batch 5): Vitest component tests for EntryForm, VerificationForm, approval flow; Playwright E2E for full workflow.
- **Integration tests** (Batch 6): End-to-end deferred promotion → retry → verify clinical entities; field-level audit on draft save.
---
## Out of scope (unless explicitly requested)
- HL7v2 ADT message support (FHIR inbound only in VigilCareClinical)
- OCR or automated field extraction (explicitly excluded in PRD v1)
- Multi-facility federated identity (single-tenant per deployment in v1)
- Full EMR functionality (billing, pharmacy inventory, scheduling)
- SMART on FHIR authorization (OAuth2 scopes for EHR launch context)
- Offline-first PWA for intermittent connectivity (documented as future extension)
- Kubernetes manifests, Helm charts, or CI/CD pipeline definitions
- Grafana dashboard provisioning (infrastructure exists but dashboards are manual)
---
## Success criteria
When complete, the system should support:
**Data Integrity (Part A)**
- Deferred promotion retries create complete clinical entities (Patient, Encounter, Observation) identical to first-attempt promotion.
- Patient matching normalizes name comparison to prevent duplicates from case/spacing variations.
- Batch assignment transitions to `IN_ENTRY` immediately, with no inconsistent interim state.
- Concurrent uploads with same SHA-256 for same patient create exactly one batch.
**Infrastructure (Part B)**
- Health checks report PostgreSQL, Redis, and MinIO status; orchestrators route around failures.
- CORS allows the Vue frontend to call the API from any configured origin.
- Application starts successfully even when Redis is temporarily unreachable.
- Promotion retry attempts are countable via Prometheus metrics.
**Security (Part C)**
- JWT misconfiguration (short key) fails at startup, not at first request.
- Auth endpoints are rate-limited (10 attempts per 5-minute window).
- Secrets are managed via environment variables, not plaintext config files.
- Document access (presigned URL generation) is logged in the audit trail.
**API (Part D)**
- All request DTOs have FluentValidation validators with consistent 422 error shapes.
- Administrators can create, update, and deactivate users via API.
- Erroneously created batches can be cancelled by administrators.
- List endpoints support configurable sorting.
**Vue Frontend (Part E)**
- Clinical approvers have a dedicated approval view with proper promotion flow.
- Clinicians have a bedside live capture view for real-time vital sign entry.
- Entry clerks can complete allergy, medication, and encounter summary batch types.
- Correction batches can be created and tracked through the UI.
- All operations provide toast notification feedback.
**Observability (Part F)**
- Draft field changes are logged with before/after values for clinical accountability.
- Integration tests cover deferred promotion, retry, concurrent operations, and auth flow.
- MetricsCollector tracks promotion retry status for Prometheus dashboards.