Skip to content

Conversation

jurajmajerik
Copy link
Contributor

@jurajmajerik jurajmajerik commented Oct 16, 2025

Problem

I've made a mistake when setting up the compute_metric_fingerprint method. Right now, it includes the entire stats_config object in the computation, when all we really care about is the stats.method. Going forward this will be problematic - if we add any new fields to stats_config, this will invalidate existing fingerprints.

Changes

Instead of stats_config, only include stats_config.method in the fingerprint.

This will invalidate timeseries data in ~20 production experiments. I will manually run recalculations for them.

How did you test this code?

Updated tests

@jurajmajerik jurajmajerik requested a review from a team October 16, 2025 15:43
@jurajmajerik jurajmajerik marked this pull request as ready for review October 16, 2025 15:43
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

5 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

@rodrigoi rodrigoi left a comment

Choose a reason for hiding this comment

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

Nod of Approval

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