Skip to content

Commit f986282

Browse files
committed
Update summaries in the clinic service before triggering the EHR sync
1 parent 738187a commit f986282

24 files changed

Lines changed: 1504 additions & 138 deletions

POSTPROCESS_MIGRATION_CHECKLIST.md

Lines changed: 100 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,9 @@ Consequences vs merging at enqueue: `JellyfishQuietDelayMaximum` is no longer ne
8585
a shared row, so nothing can be starved), and a long jellyfish upload recalculates roughly once per
8686
90s quiet window rather than once at the end.
8787

88-
**Processor** (Quantity 15, Frequency 30s): `updateSummaries` (all 3 types) → `triggerEHRSync`
89-
(`SyncEHRDataForPatient`, at-least-once) → `Delete`.
88+
**Processor** (Quantity 15, Frequency 30s): `updateSummaries` (all 3 types) → `updateClinicSummaries`
89+
(cgm/bgm changes only, at-least-once — Step 16a) → `triggerEHRSync` (`SyncEHRDataForPatient`,
90+
at-least-once) → `Delete`.
9091
Failures → Failing (exponential 1m/5s jitter/1h cap).
9192

9293
**Logging**: INFO — item lifecycle + data-shape changes (processor start/finish: userId, reasons,
@@ -667,6 +668,103 @@ bump the abbott plugin's platform pin (it pins the exact commit `9ee577a1c`, whi
667668
run the remediation (work user ids → `dates.outdatedSince`), confirm the runners pick them up
668669
- [ ] Done — **⏸ approval — PR1 checkpoint**
669670

671+
### Step 16a — Clinic summary updates move from clinic-worker CDC into the work item (2026-08-26)
672+
673+
The clinic service stores a copy of each patient's cgm/bgm summaries, and the EHR sync generates
674+
reports **from that copy** — so the copy must be current before the sync fires. Today clinic-worker's
675+
`patientsummary` CDC consumer maintains it (Mongo change stream → Kafka `data.summary`
676+
`UpdatePatientSummary`/`DeletePatientSummary`). Moved into the processor as a step strictly between
677+
`updateSummaries` and `triggerElectronicHealthRecordSync`.
678+
679+
- [x] **Clinic client bumped** `v0.0.0-20250122…``v0.0.0-20260814105914-911a077531db` (the old pin
680+
predates `PatientSummaryV1`/`CgmStatsV1`/`DeletePatientSummary` and the ranges-based schema).
681+
Generated names lost their unsuffixed aliases (`Clinic``ClinicV1`, `Patient``PatientV1`,
682+
`EHRSettings``EhrSettingsV1`, …) — mechanical renames in clinics/ehr/prescription/notifications
683+
+ `clinics/test/clinics.go` rebuilt against the new `EhrSettingsV1` shape
684+
- [x] `clinics/summaries.go`: `NewPatientSummary(cgm, bgm) *clinic.PatientSummaryV1` — the
685+
clinic-worker mapping (`patientsummary/models.go`) ported onto native `summary/types`, keeping
686+
the UI eligibility rules verbatim (percentages only with records; TIR percent gated on >70%
687+
single-day use or >24h CGM use; GMI only >70% use; percent deltas only when the reconstructed
688+
previous period also qualifies; BGM stdev/CoV only ≥30 records and ≥7 days). Two clinic-worker
689+
bugs fixed in the port: dead `IsZero()` guards on `time.UnixMilli` values (absent dates were
690+
sent as 1970-01-01 with `Has*: true` — now omitted with `Has*: false`), and ~40 unguarded
691+
`period.Delta` dereferences (nil delta panicked the consumer — now treated as a zero delta).
692+
Type aliases `types.CGMSummary`/`BGMSummary`/`ContinuousSummary` added for the generic
693+
instantiations
694+
- [x] `clinics.Client` gains `UpdatePatientSummary` (200/204/404 → success; 204 = not a patient of
695+
any clinic, the common case) and `DeletePatientSummary` (200/204 → success), mirroring
696+
clinic-worker's accepted statuses plus the `SyncEHRDataForPatient` 404 precedent
697+
- [x] **Only an actual change is reported** (requirement). The `Summarizers` seam returns
698+
`SummariesUpdate{CGM, BGM, UpdatedTypes, Deleted}` (by value — a pointer invited nil checks its
699+
callers never needed): each cgm/bgm update is classified against a pre-read (`GetSummary`
700+
before `UpdateSummary`) — recalculation always advances `Dates.LastUpdatedDate`, the
701+
no-data-modified branch never does, and deletion is a non-nil→nil transition (recording the
702+
deleted summary's id for the clinic delete). The legacy outdated-flag clear (`SetNotOutdated`)
703+
is deliberately classified as no change. Continuous summaries are calculated but never
704+
classified or reported — the clinic model has no `con`
705+
- [x] **`LastUpdatedDate` discriminator audited (2026-08-26 review)**: the field is written in
706+
exactly two places. `Dates.Update` sets it, and is reachable only past the
707+
`status.EarliestModified.IsZero()` guard — i.e. only when the fetcher reports data modified
708+
since the watermark. `Dates.Reset` zeroes it on the schema-migration path, but the zero can
709+
never be persisted: the zero watermark makes the follow-on fetch either find data (full
710+
recalculation → `Dates.Update` stamps a fresh value; classified updated, correctly — the
711+
rebuild may change content) or find none (summary deleted; classified deleted). The
712+
no-data-modified branch rewrites the summary without touching it. So "timestamp advanced" ⇔
713+
"the summarizer was given new or modified data (or rebuilt in full)" — the invariant the
714+
classification relies on holds
715+
- [x] **At-least-once across retries**: the pipeline retries from the start, and a retried
716+
calculation reports no further change, so changes are persisted into the work metadata
717+
(`pendingSummaryUpdates` types / `pendingSummaryDeletes` ids) BEFORE they are reported —
718+
the absorb pattern. On the success path that is one extra `ProcessingUpdate` per changing item;
719+
on the failing path the merged metadata rides the failing update for free (including changes
720+
calculated before a mid-`UpdateSummaries` error — the seam returns partial results with the
721+
error). Not cleared after reporting: the item is deleted at the end, and a re-report after an
722+
EHR-sync failure stores the same summary again rather than twice. Remaining window, accepted: a
723+
crash between the summary write and the metadata persist loses that report; the next actual
724+
change reports the full current summary, so updates self-heal — a lost *delete* report does not
725+
- [x] **Retry-interleaving analysis (2026-08-26 review)**: deletions are reported before updates, and
726+
a deletion matches only the summary id it names — never the id of a recreation — so every
727+
single-pod retry interleaving converges (update-then-deleted → delete reported, update skipped
728+
via nil content; deleted-then-recreated → old id unset, new stats stored; pending updates
729+
re-send the CURRENT content re-read each attempt, so a sticky record never pushes stale stats;
730+
the delete loop and the update re-send are idempotent). ONE genuine race was found: a processor
731+
reaped mid-report (>5m stall) whose `UpdatePatientSummary` completes AFTER another pod's retry
732+
pushed newer stats would regress the clinic copy until the user's next change — unlike CDC
733+
there is no partition ordering. CLOSED clinic-side by **tidepool-org/clinic#265**
734+
(2026-08-26): `UpdateSummaryInAllClinics` applies each stats type only when the incoming
735+
`dates.lastUpdatedDate` is not older than the stored one (`$cond` pipeline update, per type,
736+
per patient document; equal dates still apply so re-reports stay idempotent; BSON ordering
737+
missing < null < date means absent stored stats always accept and an undated report replaces
738+
only undated stats). Deploy #265 before or with this step — before it, the race stands as
739+
originally accepted (requires a stalled-then-successful HTTP call spanning the reap grace plus
740+
the retry's full recalculation)
741+
- [x] `updateClinicSummaries` step: deletes first (each id, tolerant of ids the clinic no longer
742+
holds), then one `UpdatePatientSummary` carrying only the changed types' stats (the clinic
743+
service merges per type, leaving the other untouched). An updated type whose summary was
744+
deleted by a later attempt has its deletion recorded and no summary to report — skipped. Not
745+
reason-gated, unlike the EHR sync: every actual change reaches the clinic
746+
- [x] `absorbPending` validates each sibling before absorbing it — `validateIdentity` (the same
747+
check the claimed work gets) on top of the field validation `metadata.Decode` already runs —
748+
so a row listed by group but naming another user in its metadata is left pending to fail on
749+
its own pickup, rather than having that user's change merged into this one and destroyed
750+
- [x] Tests: processor — record→report→sync ordering pinned (`gomock.InOrder`); only changed types in
751+
the payload; no clinic call when nothing changed; delete before update; pre-populated metadata
752+
reports with a no-change recalculation (the retry case); failing metadata carries the
753+
unreported changes (both after a report failure and after a partial calculation failure); three
754+
new Failing entries. clinics — mapping (ids/config/dates, zero-date omission, empty-reason
755+
replacement, every eligibility gate, nil delta, non-day period keys dropped) and both new
756+
client methods (paths, tolerated statuses, error statuses). Metadata parse/validate/round-trip
757+
entries for the new fields
758+
- [x] Done: `GOTEST_PKGS='./data/... ./summary/... ./clinics/... ./work/... ./ehr/... ./prescription/... ./notifications/...' make test-go`
759+
green (84 packages); `go build ./...`/`go vet ./...`/`gofmt`/`goimports -local` clean
760+
- [ ] **Cross-repo consequence**: once this deploys, clinic-worker's `patientsummary` consumer and
761+
the processor both push (benign duplicates — same content, idempotent upsert). Retire the
762+
consumer in clinic-worker with the same gate as clinic-worker#123: the drain endpoint's inline
763+
recalculations (alive until PR2/PR3) reach the clinic only via CDC, so the consumer must
764+
outlive the legacy queue (`tidepool_summary_queue_length` → 0). The patient-create
765+
`populateSummary` path in clinic-worker (`patients/consumer.go`) stays — it pulls, not CDC
766+
- **⏸ approval**
767+
670768
## PR2 — task service retirement
671769

672770
### Step 17 — Remove runners + task-row cleanup

clinics/clinician.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import (
66
api "github.com/tidepool-org/clinic/client"
77
)
88

9-
func IsPrescriber(clinician *api.Clinician) bool {
9+
func IsPrescriber(clinician *api.ClinicianV1) bool {
1010
if clinician == nil {
1111
return false
1212
}

clinics/service.go

Lines changed: 48 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,17 @@ const ErrorCodeClinicClientFailure = "clinic-client-failure"
2323
var ClientModule = fx.Provide(NewClient)
2424

2525
type Client interface {
26-
GetClinic(ctx context.Context, clinicID string) (*clinic.Clinic, error)
27-
GetClinician(ctx context.Context, clinicID, clinicianID string) (*clinic.Clinician, error)
28-
GetEHRSettings(ctx context.Context, clinicId string) (*clinic.EHRSettings, error)
29-
SharePatientAccount(ctx context.Context, clinicID, patientID string) (*clinic.Patient, error)
30-
ListEHREnabledClinics(ctx context.Context) ([]clinic.Clinic, error)
26+
GetClinic(ctx context.Context, clinicID string) (*clinic.ClinicV1, error)
27+
GetClinician(ctx context.Context, clinicID, clinicianID string) (*clinic.ClinicianV1, error)
28+
GetEHRSettings(ctx context.Context, clinicId string) (*clinic.EhrSettingsV1, error)
29+
SharePatientAccount(ctx context.Context, clinicID, patientID string) (*clinic.PatientV1, error)
30+
ListEHREnabledClinics(ctx context.Context) ([]clinic.ClinicV1, error)
3131
SyncEHRData(ctx context.Context, clinicID string) error
3232
SyncEHRDataForPatient(ctx context.Context, patientID string) error
33-
GetPatients(ctx context.Context, clinicId string, userToken string, params *clinic.ListPatientsParams, injectedParams url.Values) ([]clinic.Patient, error)
34-
GetPatient(ctx context.Context, clinicID, patientID string) (*clinic.Patient, error)
33+
GetPatients(ctx context.Context, clinicId string, userToken string, params *clinic.ListPatientsParams, injectedParams url.Values) ([]clinic.PatientV1, error)
34+
GetPatient(ctx context.Context, clinicID, patientID string) (*clinic.PatientV1, error)
35+
UpdatePatientSummary(ctx context.Context, patientID string, patientSummary *clinic.PatientSummaryV1) error
36+
DeletePatientSummary(ctx context.Context, summaryID string) error
3537
}
3638

3739
type config struct {
@@ -76,7 +78,7 @@ func NewClient(authClient auth.ExternalAccessor) (Client, error) {
7678
}, nil
7779
}
7880

79-
func (d *defaultClient) GetClinician(ctx context.Context, clinicID, clinicianID string) (*clinic.Clinician, error) {
81+
func (d *defaultClient) GetClinician(ctx context.Context, clinicID, clinicianID string) (*clinic.ClinicianV1, error) {
8082
response, err := d.httpClient.GetClinicianWithResponse(ctx, clinic.ClinicId(clinicID), clinic.ClinicianId(clinicianID))
8183
if err != nil {
8284
return nil, err
@@ -94,7 +96,7 @@ func (d *defaultClient) GetClinician(ctx context.Context, clinicID, clinicianID
9496
return response.JSON200, nil
9597
}
9698

97-
func (d *defaultClient) GetClinic(ctx context.Context, clinicID string) (*clinic.Clinic, error) {
99+
func (d *defaultClient) GetClinic(ctx context.Context, clinicID string) (*clinic.ClinicV1, error) {
98100
response, err := d.httpClient.GetClinicWithResponse(ctx, clinic.ClinicId(clinicID))
99101
if err != nil {
100102
return nil, err
@@ -112,11 +114,11 @@ func (d *defaultClient) GetClinic(ctx context.Context, clinicID string) (*clinic
112114
return response.JSON200, nil
113115
}
114116

115-
func (d *defaultClient) ListEHREnabledClinics(ctx context.Context) ([]clinic.Clinic, error) {
117+
func (d *defaultClient) ListEHREnabledClinics(ctx context.Context) ([]clinic.ClinicV1, error) {
116118
offset := 0
117119
batchSize := 1000
118120

119-
clinics := make([]clinic.Clinic, 0)
121+
clinics := make([]clinic.ClinicV1, 0)
120122
for {
121123
response, err := d.httpClient.ListClinicsWithResponse(ctx, &clinic.ListClinicsParams{
122124
EhrEnabled: pointer.FromBool(true),
@@ -148,7 +150,7 @@ func (d *defaultClient) ListEHREnabledClinics(ctx context.Context) ([]clinic.Cli
148150
return clinics, nil
149151
}
150152

151-
func (d *defaultClient) GetEHRSettings(ctx context.Context, clinicId string) (*clinic.EHRSettings, error) {
153+
func (d *defaultClient) GetEHRSettings(ctx context.Context, clinicId string) (*clinic.EhrSettingsV1, error) {
152154
response, err := d.httpClient.GetEHRSettingsWithResponse(ctx, clinicId)
153155
if err != nil {
154156
return nil, err
@@ -163,10 +165,10 @@ func (d *defaultClient) GetEHRSettings(ctx context.Context, clinicId string) (*c
163165
return response.JSON200, nil
164166
}
165167

166-
func (d *defaultClient) SharePatientAccount(ctx context.Context, clinicID, patientID string) (*clinic.Patient, error) {
168+
func (d *defaultClient) SharePatientAccount(ctx context.Context, clinicID, patientID string) (*clinic.PatientV1, error) {
167169
permission := make(map[string]interface{}, 0)
168170
body := clinic.CreatePatientFromUserJSONRequestBody{
169-
Permissions: &clinic.PatientPermissions{
171+
Permissions: &clinic.PatientPermissionsV1{
170172
Note: &permission,
171173
View: &permission,
172174
},
@@ -222,7 +224,37 @@ func (d *defaultClient) SyncEHRDataForPatient(ctx context.Context, patientID str
222224
return nil
223225
}
224226

225-
func (d *defaultClient) GetPatient(ctx context.Context, clinicID, patientID string) (*clinic.Patient, error) {
227+
func (d *defaultClient) UpdatePatientSummary(ctx context.Context, patientID string, patientSummary *clinic.PatientSummaryV1) error {
228+
response, err := d.httpClient.UpdatePatientSummaryWithResponse(ctx, clinic.PatientId(patientID), *patientSummary)
229+
if err != nil {
230+
return err
231+
}
232+
if response.StatusCode() != http.StatusOK && response.StatusCode() != http.StatusNoContent && response.StatusCode() != http.StatusNotFound {
233+
err = errors.Preparedf(ErrorCodeClinicClientFailure,
234+
"Unexpected status code from clinic service",
235+
"unexpected response status code %v from %v", response.StatusCode(), response.HTTPResponse.Request.URL)
236+
err = errors.WithMeta(err, response.HTTPResponse)
237+
return err
238+
}
239+
return nil
240+
}
241+
242+
func (d *defaultClient) DeletePatientSummary(ctx context.Context, summaryID string) error {
243+
response, err := d.httpClient.DeletePatientSummaryWithResponse(ctx, clinic.SummaryId(summaryID))
244+
if err != nil {
245+
return err
246+
}
247+
if response.StatusCode() != http.StatusOK && response.StatusCode() != http.StatusNoContent {
248+
err = errors.Preparedf(ErrorCodeClinicClientFailure,
249+
"Unexpected status code from clinic service",
250+
"unexpected response status code %v from %v", response.StatusCode(), response.HTTPResponse.Request.URL)
251+
err = errors.WithMeta(err, response.HTTPResponse)
252+
return err
253+
}
254+
return nil
255+
}
256+
257+
func (d *defaultClient) GetPatient(ctx context.Context, clinicID, patientID string) (*clinic.PatientV1, error) {
226258
response, err := d.httpClient.GetPatientWithResponse(ctx, clinic.ClinicId(clinicID), clinic.PatientId(patientID))
227259
if err != nil {
228260
return nil, err
@@ -237,7 +269,7 @@ func (d *defaultClient) GetPatient(ctx context.Context, clinicID, patientID stri
237269
return response.JSON200, nil
238270
}
239271

240-
func (d *defaultClient) GetPatients(ctx context.Context, clinicId string, userToken string, params *clinic.ListPatientsParams, injectedParams url.Values) ([]clinic.Patient, error) {
272+
func (d *defaultClient) GetPatients(ctx context.Context, clinicId string, userToken string, params *clinic.ListPatientsParams, injectedParams url.Values) ([]clinic.PatientV1, error) {
241273
response, err := d.httpClient.ListPatientsWithResponse(ctx, clinicId, params, func(ctx context.Context, req *http.Request) error {
242274
if len(injectedParams) != 0 {
243275
q := req.URL.Query()

0 commit comments

Comments
 (0)