Skip to content

[BACK-4387] Update TIDE category boundaries to be inclusive rounded values.#232

Open
lostlevels wants to merge 4 commits intomasterfrom
jp-tide-ranking-rounding
Open

[BACK-4387] Update TIDE category boundaries to be inclusive rounded values.#232
lostlevels wants to merge 4 commits intomasterfrom
jp-tide-ranking-rounding

Conversation

@lostlevels
Copy link
Copy Markdown
Contributor

Updates the TIDE report query predicates to query on rounded percents and be inclusive thresholds instead of exclusive. See comments in code for more details.

This commits adds categorizing a field based on its whole number rounded
percent values. It adds an "extra" call against the unrounded value in the
query in order to be indexable (see comments in `availableCategories`.
@lostlevels lostlevels requested a review from ewollesen April 29, 2026 14:41
return bson.A{
bson.M{field: bson.M{"$lt": 0.70 - halfAPercent}}, // < because round(0.695) = round(69.5%) = 70% so values less than 69.5% round to < 70%.
bson.M{"$expr": bson.M{
"$lte": bson.A{
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be $lt.

While no test case for cgm use time was provided in the ticket, it might be worth adding one. Or modifying one of the existing ones to show this distinction.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably a good idea to add tests around all the boundaries I reckon.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually correct. The CGM wear time is one of the few categories that uses an exclusive bounds. Added test for it.

@lostlevels lostlevels requested a review from ewollesen May 7, 2026 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants