Repository navigation
feat(preprod): Add treemap diff UI - #105331
Conversation
github-actions
Bot
added
the
Scope: Frontend
8d774a7 to
85fbc7c
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| if (query.isLoading) { | ||
| return <LoadingIndicator />; | ||
| } | ||
|
|
||
| const diffItems = query.data?.diff_items; | ||
|
|
||
| if (!diffItems || diffItems.length === 0) { | ||
| return <EmptyStateWarning small>No diff found.</EmptyStateWarning>; | ||
| } | ||
|
|
||
| return <TreemapDiffSection diffItems={diffItems} />; |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
0520276 to
0a3405e
Compare
| return <TreemapDiffSection diffItems={diffItems} />; | ||
| } | ||
|
|
||
| type SectionProps = { |
There was a problem hiding this comment.
what's the difference between SectionProps and ContentProps?
There was a problem hiding this comment.
Nothing currently, I've made it so one aliases the other
| const headMetricId = event.occurrence?.evidenceData?.headSizeMetricId; | ||
| const baseMetricId = event.occurrence?.evidenceData?.baseSizeMetricId; | ||
|
|
||
| if (baseMetricId && headMetricId) { | ||
| return { | ||
| baseMetricId, | ||
| headMetricId, | ||
| }; | ||
| } | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
Bug: The getMetricIds function expects headSizeMetricId and baseSizeMetricId in event.occurrence.evidenceData, but these fields are never populated, causing the X-ray diff UI to never render.
Severity: CRITICAL | Confidence: High
🔍 Detailed Analysis
The getMetricIds function in the new EventXrayDiff component attempts to extract headSizeMetricId and baseSizeMetricId from event.occurrence.evidenceData. However, these camelCase fields are not defined in the type system and are not populated by the backend for size-related issues. As a result, getMetricIds will always return undefined. This causes the EventXrayDiff component to consistently return null, preventing the new X-ray diff UI from ever rendering for users. The feature will fail silently without any error indication.
💡 Suggested Fix
Ensure the backend populates event.occurrence.evidenceData.headSizeMetricId and event.occurrence.evidenceData.baseSizeMetricId. Alternatively, if the data exists elsewhere or with different naming (e.g., snake_case), update the frontend to source the metric IDs from the correct location and handle any naming discrepancies.
🤖 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/components/events/eventXrayDiff.tsx#L77-L88
Potential issue: The `getMetricIds` function in the new `EventXrayDiff` component
attempts to extract `headSizeMetricId` and `baseSizeMetricId` from
`event.occurrence.evidenceData`. However, these camelCase fields are not defined in the
type system and are not populated by the backend for size-related issues. As a result,
`getMetricIds` will always return `undefined`. This causes the `EventXrayDiff` component
to consistently return `null`, preventing the new X-ray diff UI from ever rendering for
users. The feature will fail silently without any error indication.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 8223334
| /> | ||
| </ErrorBoundary> | ||
| </InterimSection> | ||
| ); |
There was a problem hiding this comment.
Duplicate section headers when rendering treemap diff
The EventXrayDiffSection wraps its content in InterimSection with title={t('X-Ray diff')}, which renders a section header. However, TreemapDiffSection (rendered inside via EventXrayDiffContent) already includes its own <Heading as="h2">X-Ray Diff</Heading> and a <Separator>. This results in duplicate "X-Ray diff" headings appearing in the UI when the treemap successfully renders.
Additional Locations (1)
There was a problem hiding this comment.
Will solve this later
This branch was successfully deployed
2 participants
This causes the x-ray to show up in the issues feed for size issues: