Repository navigation
fix(experiments): Classify NOT_AN_AGGREGATE as a metric config error - #90715
Merged
Merged
Conversation
|
😎 Merged successfully - details. |
Contributor
🤖 CI report
|
Contributor
|
Reviews (1): Last reviewed commit: "fix(experiments): Classify NOT_AN_AGGREG..." | Re-trigger Greptile |
There was a problem hiding this comment.
Contained fix within the owning team's own domain (experiment query error classification/retry logic), with unit tests covering the new classification and decorator branches; no risky-territory concerns (no schema, API, auth, billing, or CI changes).
- Author wrote 11% of the modified lines and has 25 merged PRs in these paths (familiarity MODERATE).
- 👍 on the PR from greptile-apps[bot].
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 52L, 2F substantive, 105L/3F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1c-medium (105L, 3F, single-area, fix) |
| stamphog 2.0.0b4 | .stamphog/policy.yml @ 26cc538 · reviewed head 26cc538 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
CHQueryErrorNotAnAggregatefrom experiment queries (~53/week, error tracking) is user-authored HogQL referencing a row-level column (person_id,timestamp) outside an aggregate — every tracked occurrence carriescontains_user_hogql, and affected metrics get edited by their owners shortly after. It's treated as a platform error: captured to error tracking, shown as a generic failure, and retried 3–17× per metric by the recalculation and timeseries workers.Changes
classify_experiment_query_errormapsNOT_AN_AGGREGATEtovalidation_error, which is already inNON_RETRYABLE_ERROR_TYPES— workers fail it permanently instead of retrying.ValidationError("references an event column outside an aggregate function — wrap the value in an aggregation") for user-facing callers, re-raises raw for internal ones, and skipscapture_exception.validation_error-class failures.How did you test this code?
Tests: classifier cases for raw code 215 and the wrapped error; decorator converts with actionable message and no capture (user-facing), re-raises raw with no capture (internal). Ran error handling (27) and recalculation activity (38) tests. mypy on changed files.