From 86820e88440a700ed9b06a85b01f3745abb35989 Mon Sep 17 00:00:00 2001 From: Eric Wollesen Date: Thu, 29 Jan 2026 10:54:17 +0100 Subject: [PATCH 1/2] WIP first stab at new bucketing using rounded mg/dL I don't like that there's no proper logging in this code, but the lift required to bring in a logger seems not worth the effort. Maybe someone else can find an easier way? This approach is based on research by Darin and myself into how bucket consistently independent of the incoming units and the units of the viewing clinic. BACK-4158 --- data/blood/glucose/glucose.go | 31 +++++++++++++++++++ summary/types/glucose.go | 58 ++++++++++++++++++++++++++++++----- 2 files changed, 82 insertions(+), 7 deletions(-) diff --git a/data/blood/glucose/glucose.go b/data/blood/glucose/glucose.go index f91f0d671..2a0519151 100644 --- a/data/blood/glucose/glucose.go +++ b/data/blood/glucose/glucose.go @@ -1,7 +1,10 @@ package glucose import ( + "fmt" "math" + "slices" + "strings" "github.com/tidepool-org/platform/pointer" ) @@ -82,6 +85,34 @@ func NormalizeValueForUnits(value *float64, units *string) *float64 { return value } +func ConvertValue(value float64, fromUnits, toUnits string) (float64, error) { + if fromUnits == toUnits { + return value, nil + } + units := Units() + if !slices.Contains(units, fromUnits) { + return 0, fmt.Errorf("unrecognized from units %q not found in %q", fromUnits, units) + } + if !slices.Contains(units, toUnits) { + return 0, fmt.Errorf("unrecognized to units %q not found in %q", fromUnits, units) + } + + switch strings.ToLower(fromUnits + toUnits) { + case strings.ToLower(Mgdl + MmolL): + v := NormalizeValueForUnits(&value, &fromUnits) + // NormalizeValueForUnits will return the original value if the from units aren't + // recognized. + if *v == value { + return 0, fmt.Errorf("unhandled from units: %q", fromUnits) + } + return *v, nil + case strings.ToLower(MmolL + MgdL): + return value * MmolLToMgdLConversionFactor, nil + default: + return 0, fmt.Errorf("unhandled from units: %q", fromUnits) + } +} + func ValueRangeForRateUnits(rateUnits *string) (float64, float64) { if rateUnits != nil { switch *rateUnits { diff --git a/summary/types/glucose.go b/summary/types/glucose.go index 06e9fd1c0..5af9bf286 100644 --- a/summary/types/glucose.go +++ b/summary/types/glucose.go @@ -2,8 +2,8 @@ package types import ( "context" - "errors" "fmt" + "log/slog" "math" "strconv" "strings" @@ -15,6 +15,7 @@ import ( "github.com/tidepool-org/platform/data/blood/glucose" "github.com/tidepool-org/platform/data/types/blood/glucose/continuous" "github.com/tidepool-org/platform/data/types/blood/glucose/selfmonitored" + "github.com/tidepool-org/platform/errors" ) const ( @@ -24,6 +25,7 @@ const ( type Glucose interface { NormalizedValue() float64 + RawValueAndUnits() (float64, string, error) Type() string Time() *time.Time CreatedTime() *time.Time @@ -49,6 +51,17 @@ type SelfMonitoredGlucoseAdapter struct { datum *selfmonitored.SelfMonitored } +func (s SelfMonitoredGlucoseAdapter) RawValueAndUnits() (float64, string, error) { + if s.datum == nil { + return 0, "", errors.New("datum is nil") + } else if s.datum.Value == nil { + return 0, "", errors.New("datum's value is nil") + } else if s.datum.Units == nil { + return 0, "", errors.New("datum's units are nil") + } + return *s.datum.Value, *s.datum.Units, nil +} + func (s SelfMonitoredGlucoseAdapter) NormalizedValue() float64 { return *glucose.NormalizeValueForUnits(s.datum.Value, s.datum.Units) } @@ -75,6 +88,17 @@ type ContinuousGlucoseAdapter struct { datum *continuous.Continuous } +func (c ContinuousGlucoseAdapter) RawValueAndUnits() (float64, string, error) { + if c.datum == nil { + return 0, "", errors.New("datum is nil") + } else if c.datum.Value == nil { + return 0, "", errors.New("datum value is nil") + } else if c.datum.Units == nil { + return 0, "", errors.New("datum units are nil") + } + return *c.datum.Value, *c.datum.Units, nil +} + func (c ContinuousGlucoseAdapter) NormalizedValue() float64 { return *glucose.NormalizeValueForUnits(c.datum.Value, c.datum.Units) } @@ -262,24 +286,44 @@ func (rs *GlucoseRanges) Finalize(days int) { } } +const ( + veryLowBloodGlucoseMgdL int = 54 + lowBloodGlucoseMgdL int = 70 + highBloodGlucoseMgdL int = 180 + veryHighBloodGlucoseMgdL int = 250 + extremeHighBloodGlucoseMgdL int = 350 +) + func (rs *GlucoseRanges) Update(record Glucose) { - normalizedValue := record.NormalizedValue() + rawValue, rawUnits, err := record.RawValueAndUnits() + if err != nil { + // TODO pass in a proper platform logger + slog.Error("unable to update datum", "error", err) + return + } + mgdlValue, err := glucose.ConvertValue(rawValue, rawUnits, glucose.MgdL) + if err != nil { + // TODO pass in a proper platform logger + slog.Error("unable to update datum: conversion error", "error", err) + return + } + mgdlRounded := int(math.Round(mgdlValue)) - if normalizedValue < veryLowBloodGlucose { + if mgdlRounded < veryLowBloodGlucoseMgdL { rs.VeryLow.Update(record) rs.AnyLow.Update(record) - } else if normalizedValue > veryHighBloodGlucose { + } else if mgdlRounded > veryHighBloodGlucoseMgdL { rs.VeryHigh.Update(record) rs.AnyHigh.Update(record) // VeryHigh is inclusive of extreme high, this is intentional - if normalizedValue >= extremeHighBloodGlucose { + if mgdlRounded >= extremeHighBloodGlucoseMgdL { rs.ExtremeHigh.Update(record) } - } else if normalizedValue < lowBloodGlucose { + } else if mgdlRounded < lowBloodGlucoseMgdL { rs.Low.Update(record) rs.AnyLow.Update(record) - } else if normalizedValue > highBloodGlucose { + } else if mgdlRounded > highBloodGlucoseMgdL { rs.AnyHigh.Update(record) rs.High.Update(record) } else { From 3ec22917f221cc1b1771cd31db76d0751035580d Mon Sep 17 00:00:00 2001 From: Eric Wollesen Date: Fri, 30 Jan 2026 08:58:32 +0100 Subject: [PATCH 2/2] multiple changes from code review - break RawValueAndUnits into Value and Units methods - break ConvertValue into MmolLRounded and MgdLRounded - better matching of the defensiveness level of surrounding code - update the thresholds of summary/types.Config BACK-4158 --- data/blood/glucose/glucose.go | 38 +++++--------- data/blood/glucose/glucose_test.go | 35 +++++++++++++ summary/types/glucose.go | 82 +++++++++++++++--------------- summary/types/summary.go | 7 +-- 4 files changed, 90 insertions(+), 72 deletions(-) diff --git a/data/blood/glucose/glucose.go b/data/blood/glucose/glucose.go index 2a0519151..e1536ff32 100644 --- a/data/blood/glucose/glucose.go +++ b/data/blood/glucose/glucose.go @@ -1,9 +1,7 @@ package glucose import ( - "fmt" "math" - "slices" "strings" "github.com/tidepool-org/platform/pointer" @@ -85,32 +83,22 @@ func NormalizeValueForUnits(value *float64, units *string) *float64 { return value } -func ConvertValue(value float64, fromUnits, toUnits string) (float64, error) { - if fromUnits == toUnits { - return value, nil - } - units := Units() - if !slices.Contains(units, fromUnits) { - return 0, fmt.Errorf("unrecognized from units %q not found in %q", fromUnits, units) - } - if !slices.Contains(units, toUnits) { - return 0, fmt.Errorf("unrecognized to units %q not found in %q", fromUnits, units) +func MmolLRounded(value float64, units string) float64 { + mmolLValue := 0.0 + if strings.EqualFold(units, MmolL) { + mmolLValue = value + } else { + normalized := NormalizeValueForUnits(&value, &units) + mmolLValue = *normalized } + return math.Round(mmolLValue*10) / 10 +} - switch strings.ToLower(fromUnits + toUnits) { - case strings.ToLower(Mgdl + MmolL): - v := NormalizeValueForUnits(&value, &fromUnits) - // NormalizeValueForUnits will return the original value if the from units aren't - // recognized. - if *v == value { - return 0, fmt.Errorf("unhandled from units: %q", fromUnits) - } - return *v, nil - case strings.ToLower(MmolL + MgdL): - return value * MmolLToMgdLConversionFactor, nil - default: - return 0, fmt.Errorf("unhandled from units: %q", fromUnits) +func MgdLRounded(value float64, units string) int { + if strings.EqualFold(units, MmolL) { + value *= MmolLToMgdLConversionFactor } + return int(math.Round(value)) } func ValueRangeForRateUnits(rateUnits *string) (float64, float64) { diff --git a/data/blood/glucose/glucose_test.go b/data/blood/glucose/glucose_test.go index 319940730..f770b32e0 100644 --- a/data/blood/glucose/glucose_test.go +++ b/data/blood/glucose/glucose_test.go @@ -218,4 +218,39 @@ var _ = Describe("Glucose", func() { } }) }) + + Describe("MgdLRounded", func() { + It("rounds to 0 decimal places", func() { + got := glucose.MgdLRounded(3.0, "mmol/L") + Expect(got).To(Equal(54)) + got = glucose.MgdLRounded(3.9, "mmol/L") + Expect(got).To(Equal(70)) + got = glucose.MgdLRounded(10, "mmol/L") + Expect(got).To(Equal(180)) + got = glucose.MgdLRounded(13.9, "mmol/L") + Expect(got).To(Equal(250)) + got = glucose.MgdLRounded(19.4, "mmol/L") + Expect(got).To(Equal(350)) + + got = glucose.MgdLRounded(54.49999999999999, "mg/dL") + Expect(got).To(Equal(54)) + got = glucose.MgdLRounded(54.5, "mg/dL") + Expect(got).To(Equal(55)) + }) + }) + + Describe("MmolLRounded", func() { + It("rounds to 1 decimal place", func() { + got := glucose.MmolLRounded(54, "mg/dL") + Expect(got).To(Equal(3.0)) + got = glucose.MmolLRounded(70, "mg/dL") + Expect(got).To(Equal(3.9)) + got = glucose.MmolLRounded(180, "mg/dL") + Expect(got).To(Equal(10.0)) + got = glucose.MmolLRounded(250, "mg/dL") + Expect(got).To(Equal(13.9)) + got = glucose.MmolLRounded(350, "mg/dL") + Expect(got).To(Equal(19.4)) + }) + }) }) diff --git a/summary/types/glucose.go b/summary/types/glucose.go index 5af9bf286..bceee7a1c 100644 --- a/summary/types/glucose.go +++ b/summary/types/glucose.go @@ -3,7 +3,6 @@ package types import ( "context" "fmt" - "log/slog" "math" "strconv" "strings" @@ -25,7 +24,8 @@ const ( type Glucose interface { NormalizedValue() float64 - RawValueAndUnits() (float64, string, error) + Value() float64 + Units() string Type() string Time() *time.Time CreatedTime() *time.Time @@ -51,19 +51,22 @@ type SelfMonitoredGlucoseAdapter struct { datum *selfmonitored.SelfMonitored } -func (s SelfMonitoredGlucoseAdapter) RawValueAndUnits() (float64, string, error) { - if s.datum == nil { - return 0, "", errors.New("datum is nil") - } else if s.datum.Value == nil { - return 0, "", errors.New("datum's value is nil") - } else if s.datum.Units == nil { - return 0, "", errors.New("datum's units are nil") +func (s SelfMonitoredGlucoseAdapter) NormalizedValue() float64 { + return *glucose.NormalizeValueForUnits(s.datum.Value, s.datum.Units) +} + +func (s SelfMonitoredGlucoseAdapter) Units() string { + if s.datum == nil || s.datum.Units == nil { + return "" } - return *s.datum.Value, *s.datum.Units, nil + return *s.datum.Units } -func (s SelfMonitoredGlucoseAdapter) NormalizedValue() float64 { - return *glucose.NormalizeValueForUnits(s.datum.Value, s.datum.Units) +func (s SelfMonitoredGlucoseAdapter) Value() float64 { + if s.datum == nil || s.datum.Value == nil { + return 0 + } + return *s.datum.Value } func (s SelfMonitoredGlucoseAdapter) Type() string { @@ -88,19 +91,22 @@ type ContinuousGlucoseAdapter struct { datum *continuous.Continuous } -func (c ContinuousGlucoseAdapter) RawValueAndUnits() (float64, string, error) { - if c.datum == nil { - return 0, "", errors.New("datum is nil") - } else if c.datum.Value == nil { - return 0, "", errors.New("datum value is nil") - } else if c.datum.Units == nil { - return 0, "", errors.New("datum units are nil") +func (c ContinuousGlucoseAdapter) NormalizedValue() float64 { + return *glucose.NormalizeValueForUnits(c.datum.Value, c.datum.Units) +} + +func (c ContinuousGlucoseAdapter) Units() string { + if c.datum == nil || c.datum.Units == nil { + return "" } - return *c.datum.Value, *c.datum.Units, nil + return *c.datum.Units } -func (c ContinuousGlucoseAdapter) NormalizedValue() float64 { - return *glucose.NormalizeValueForUnits(c.datum.Value, c.datum.Units) +func (c ContinuousGlucoseAdapter) Value() float64 { + if c.datum == nil || c.datum.Value == nil { + return 0 + } + return *c.datum.Value } func (c ContinuousGlucoseAdapter) Type() string { @@ -294,36 +300,30 @@ const ( extremeHighBloodGlucoseMgdL int = 350 ) -func (rs *GlucoseRanges) Update(record Glucose) { - rawValue, rawUnits, err := record.RawValueAndUnits() - if err != nil { - // TODO pass in a proper platform logger - slog.Error("unable to update datum", "error", err) - return - } - mgdlValue, err := glucose.ConvertValue(rawValue, rawUnits, glucose.MgdL) - if err != nil { - // TODO pass in a proper platform logger - slog.Error("unable to update datum: conversion error", "error", err) - return - } - mgdlRounded := int(math.Round(mgdlValue)) +var ( + veryLowBloodGlucose = glucose.MmolLRounded(float64(veryLowBloodGlucoseMgdL), glucose.MmolL) + lowBloodGlucose = glucose.MmolLRounded(float64(lowBloodGlucoseMgdL), glucose.MmolL) + highBloodGlucose = glucose.MmolLRounded(float64(highBloodGlucoseMgdL), glucose.MmolL) + veryHighBloodGlucose = glucose.MmolLRounded(float64(veryHighBloodGlucoseMgdL), glucose.MmolL) +) - if mgdlRounded < veryLowBloodGlucoseMgdL { +func (rs *GlucoseRanges) Update(record Glucose) { + mgdLRounded := glucose.MgdLRounded(record.Value(), record.Units()) + if mgdLRounded < veryLowBloodGlucoseMgdL { rs.VeryLow.Update(record) rs.AnyLow.Update(record) - } else if mgdlRounded > veryHighBloodGlucoseMgdL { + } else if mgdLRounded > veryHighBloodGlucoseMgdL { rs.VeryHigh.Update(record) rs.AnyHigh.Update(record) // VeryHigh is inclusive of extreme high, this is intentional - if mgdlRounded >= extremeHighBloodGlucoseMgdL { + if mgdLRounded >= extremeHighBloodGlucoseMgdL { rs.ExtremeHigh.Update(record) } - } else if mgdlRounded < lowBloodGlucoseMgdL { + } else if mgdLRounded < lowBloodGlucoseMgdL { rs.Low.Update(record) rs.AnyLow.Update(record) - } else if mgdlRounded > highBloodGlucoseMgdL { + } else if mgdLRounded > highBloodGlucoseMgdL { rs.AnyHigh.Update(record) rs.High.Update(record) } else { diff --git a/summary/types/summary.go b/summary/types/summary.go index 4e4f3bd39..f99486447 100644 --- a/summary/types/summary.go +++ b/summary/types/summary.go @@ -20,12 +20,7 @@ const ( SummaryTypeContinuous = "con" SchemaVersion = 6 - lowBloodGlucose = 3.9 - veryLowBloodGlucose = 3.0 - highBloodGlucose = 10.0 - veryHighBloodGlucose = 13.9 - extremeHighBloodGlucose = 19.4 - HoursAgoToKeep = 60 * 24 + HoursAgoToKeep = 60 * 24 OutdatedReasonUploadCompleted = "UPLOAD_COMPLETED" OutdatedReasonDataAdded = "DATA_ADDED"