chore(comp workflows): Hint for Distributions #102761
Conversation
github-actions
bot
added
the
Scope: Frontend
aayush-se
force-pushed
the
comp-workflows/selection-hint
branch
from
cc523db to
251b04c
Compare
![sentry[bot]](https://avatars.githubusercontent.com/u/1396951?s=60&v=4){kind=small}
| const [x1, x2] = boxSelectOptions.xRange; | ||
|
|
||
| let startTimestamp = Math.floor(x1 / 60_000) * 60_000; | ||
| const endTimestamp = Math.ceil(x2 / 60_000) * 60_000; | ||
| startTimestamp = Math.min(startTimestamp, endTimestamp - 60_000); |
There was a problem hiding this comment.
Bug: boxSelectOptions.xRange is treated as timestamps but contains categorical axis positions, leading to incorrect date formatting.
Severity: CRITICAL | Confidence: 1.00
🔍 Detailed Analysis
The code at static/app/views/explore/components/attributeBreakdowns/content.tsx:144-151 incorrectly assumes boxSelectOptions.xRange contains timestamp values in milliseconds. However, xRange actually holds categorical axis positions (indices) from the chart's x-axis, which is of type 'category'. This leads to x1 and x2 being small numbers (e.g., 0-40). When these small numbers are divided by 60_000 and then passed to moment(), they result in epoch dates like "Jan 1 1970 00:00" being displayed in the selection hint UI.
💡 Suggested Fix
Modify the logic at static/app/views/explore/components/attributeBreakdowns/content.tsx:144-151 to correctly interpret boxSelectOptions.xRange as categorical indices, not timestamps, when formatting the selection hint.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: static/app/views/explore/components/attributeBreakdowns/content.tsx#L144-L148
Potential issue: The code at
`static/app/views/explore/components/attributeBreakdowns/content.tsx:144-151`
incorrectly assumes `boxSelectOptions.xRange` contains timestamp values in milliseconds.
However, `xRange` actually holds categorical axis positions (indices) from the chart's
x-axis, which is of type 'category'. This leads to `x1` and `x2` being small numbers
(e.g., 0-40). When these small numbers are divided by `60_000` and then passed to
`moment()`, they result in epoch dates like "Jan 1 1970 00:00" being displayed in the
selection hint UI.
Did we get this right? 👍 / 👎 to inform future reviews.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
|
||
| const CHARTS_COLUMN_COUNT = 3; | ||
| const CHARTS_PER_PAGE = CHARTS_COLUMN_COUNT * 4; | ||
| const PERCENTILE_FUNCTION_PREFIXES = ['p50', 'p75', 'p90', 'p95', 'p99', 'avg']; |
There was a problem hiding this comment.
Bug: Avg erroneously treated as percentile indicator bug
The constant PERCENTILE_FUNCTION_PREFIXES incorrectly includes 'avg' as a percentile function. Average (avg) is a mean calculation, not a percentile function. This causes the selection hint to incorrectly show "and is greater than or equal to avg(...)" for average functions, when it should only show the time range without the threshold clause. Only actual percentile functions (p50, p75, p90, p95, p99) should include the "greater than or equal to" language. This is a known issue that was previously fixed in #72046 ("Don't mark avg as a percentile").
| startDate, | ||
| endDate, | ||
| formattedFunction | ||
| ) |
There was a problem hiding this comment.
Bug: Percentile Selection Hint Lacks Numeric Thresholds
The selection hint for percentile functions displays the function name (e.g., "p50") instead of an actual value threshold. The message "Selection is data between %s - %s and is greater than or equal to %s" uses formattedFunction (the function name like "p50(span.duration)") as the third parameter, but this doesn't convey meaningful information to users. The hint should either display an actual numeric threshold value from the y-axis selection, or the boxSelectOptions should capture and provide yRange data to make this message accurate. Currently, boxSelectOptions only captures xRange (time) but not the y-axis value threshold needed for percentile comparisons.
There was a problem hiding this comment.
Valid (and im surprised bugbot suggests things like this), but lets get feedback first and see
- Include hint for distributions which explains how the cohorts are selected For non-percentile/avg functions, we compare the time segments directly <img width="1044" height="198" alt="image" src="https://github.com/user-attachments/assets/70c1f230-b960-43e7-aad8-8e74a4384cf2" /> For percentile/avg functions, we also segment by the numerical attr <img width="1052" height="210" alt="image" src="https://github.com/user-attachments/assets/b16a2a2f-722f-412c-89fd-684524380caf" />
- Include hint for distributions which explains how the cohorts are selected For non-percentile/avg functions, we compare the time segments directly <img width="1044" height="198" alt="image" src="https://github.com/user-attachments/assets/70c1f230-b960-43e7-aad8-8e74a4384cf2" /> For percentile/avg functions, we also segment by the numerical attr <img width="1052" height="210" alt="image" src="https://github.com/user-attachments/assets/b16a2a2f-722f-412c-89fd-684524380caf" />
- Include hint for distributions which explains how the cohorts are selected For non-percentile/avg functions, we compare the time segments directly <img width="1044" height="198" alt="image" src="https://github.com/user-attachments/assets/70c1f230-b960-43e7-aad8-8e74a4384cf2" /> For percentile/avg functions, we also segment by the numerical attr <img width="1052" height="210" alt="image" src="https://github.com/user-attachments/assets/b16a2a2f-722f-412c-89fd-684524380caf" />
For non-percentile/avg functions, we compare the time segments directly
For percentile/avg functions, we also segment by the numerical attr
