Skip to content

fix(nimbus): skip data points with None bounds in get_max_metric_value#15517

Merged
yashikakhurana merged 2 commits into
mozilla:mainfrom
SAY-5:fix/abs-none-typeerror-15396
May 6, 2026
Merged

fix(nimbus): skip data points with None bounds in get_max_metric_value#15517
yashikakhurana merged 2 commits into
mozilla:mainfrom
SAY-5:fix/abs-none-typeerror-15396

Conversation

@SAY-5
Copy link
Copy Markdown
Contributor

@SAY-5 SAY-5 commented May 5, 2026

Because

  • A data point in relative_uplift results can have explicit null (or missing) lower/upper while still being a non-empty dict (e.g. only point is set). The existing if not data_point: continue guard treats these as truthy and proceeds, then abs(None) raises TypeError: bad operand type for abs(): 'NoneType' (Sentry EXPERIMENTER-PROD-16X).

This commit

  • Skips the data point when either bound is None before calling abs() in ResultsManager.get_max_metric_value.
  • Adds a parameterized test case covering a data point with None lower/upper alongside a sibling branch with valid bounds, asserting the valid bounds still drive the max.

Fixes #15396

Copy link
Copy Markdown
Contributor

@moibra05 moibra05 left a comment

Choose a reason for hiding this comment

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

thanks for taking a look at this! changes look simple and enough to fix this issue, nice work 👍

Copy link
Copy Markdown
Contributor

@yashikakhurana yashikakhurana left a comment

Choose a reason for hiding this comment

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

Hey @SAY-5 thank you for your contribution, I noticed one of the test case is failing

FAILED experimenter/jetstream/tests/test_results_manager.py::TestExperimentResultsManager::test_get_max_metric_value_2 - AssertionError: 0 != 0.7 

would you be able to take a look and fix that please 🙏

Signed-off-by: SAY-5 <saiasish.cnp@gmail.com>
@SAY-5
Copy link
Copy Markdown
Contributor Author

SAY-5 commented May 5, 2026

Fixed the failing test.

@yashikakhurana yashikakhurana self-requested a review May 6, 2026 21:39
Copy link
Copy Markdown
Contributor

@yashikakhurana yashikakhurana left a comment

Choose a reason for hiding this comment

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

Thank you @SAY-5 looks good 🎉

@yashikakhurana yashikakhurana enabled auto-merge May 6, 2026 21:39
@yashikakhurana yashikakhurana added this pull request to the merge queue May 6, 2026
Merged via the queue into mozilla:main with commit 68324f3 May 6, 2026
25 checks passed
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.

TypeError: bad operand type for abs(): 'NoneType'

3 participants