Replays Self-Serve Bulk Delete System#11
Conversation
This validates both the [Working Draft](https://www.w3.org/TR/reporting-1/#concept-reports) and the [Editor's Draft](https://w3c.github.io/reporting/#concept-reports) formats. Fixes [ID-730 - Accept current and upcoming data model](https://linear.app/getsentry/issue/ID-730/accept-current-and-upcoming-data-model).
…o 'low' (#93927)" This reverts commit 8d04522. Co-authored-by: roaga <47861399+roaga@users.noreply.github.com>
Missed in the initial commit, leading to some relevant logs being unannotated.
We have had a few tasks get killed at 10% rollout.
Also add a test, so that this doesn't happen again
Fixes DE-129 and DE-156 --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
These transitions should be matching
…` (#93946) Use `project_id` on the replay record instead of the URL (where it does not always exist). --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
Also fixed `replay.view_html` -> `replay.view-html` --------- Co-authored-by: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com>
…948) gets `npx @typescript/native-preview` passing again
This adds mode for all things tracing. This encompasses transactions/metrics/spans. Taken from https://github.com/getsentry/sentry/blob/feeaf393deeca8b97675bff23039c6320270aab5/src/sentry/runner/commands/devserver.py#L370
The conditions associated with a DCG can change over time, and it's good if we can be completely confident that they're consistent within a given task execution.
This is unused and most regex experiments have required broader changes to ensure that regexes are evaluated in a specific order (ex: traceparent). Removing this for now to simplify the code and very slightly improve runtime performance.
From some testing (on feedback lists of all different lengths), this prompt seems to work better. It doesn't write overly long sentences and also does a better job at "summarizing" versus just mentioning a few specific topics and leaving out others.
Just remove a couple custom Flex* classes in favor of the Flex primitive
This has been killed a few times. Refs SENTRY-42M7
…n table (#93892) <!-- Describe your PR here. --> [ticket](https://linear.app/getsentry/issue/ID-156/grouping-info-remove-type-field-from-ui) The Type field in the Grouping Info section of the issue details page was redundant. This removes the Type row from all variant types while keeping the underlying data structure intact. before  after 
### Changes Related to this PR: getsentry/sentry#93810. This is part 1 of the change, which is pulling out the new component and just adding it to the repo. Also includes some simplification of the logic in the base component. Part 2 will be replacing tables in widgets. ### Before/After There is no UI change as the table is not being used yet. There is a new story page for the component.
…93943) to prevent this issue from becoming too noisy, add a noise config
Unfortunately, 'event_data' went from being the variable for current event context to being the complete parsed data from Redis, and we continued logging it per group. That's more data than we should be logging even arguably once, let alone per group.
Co-authored-by: Abdullah Khan <abdullahkhan@PG9Y57YDXQ.local>
Adds some simple analytics to our endpoint so we can begin building a dashboard in Amplitude.
Previously, explore supported multiple y axis per chart, so each visualize supported multiple y axis. That functionality has since been removed for simplicity so update the types here to match. Keep in mind that saved queries still store them as an array so when serializing/deserializing, we still need to treat it as an array.
We'll need the `useGetTraceItemAttributeKeys` hook in other places so refactoring it so that it can exported.
- getsentry/sentry#93894 removed usage - getsentry/sentry-options-automator#4243 removed the last override
When the max segment ID is null the process fails. We should exit early since if there aren't any segments to delete there's nothing to do.
|
@coderabbitai review |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis pull request introduces service configuration updates, refactors the Visualize API from array-based to single-axis, enhances feedback summarization with AI features, updates PR comment formatting across integrations, adds error context to replay breadcrumbs, and modernizes UI layout components by replacing custom wrappers with a shared Flex component. Additionally, it removes deprecated grouping experiments, updates automation tuning defaults, and extends workflow detector capabilities with owner management. Changes
Sequence Diagram(s)sequenceDiagram
participant Endpoint as Browser Reporting<br/>Endpoint
participant Serializer as BrowserReportSerializer
participant Parser as BrowserReportsJSONParser
participant Validation as Validation<br/>Pipeline
participant Metrics as Metrics
Endpoint->>Parser: Parse request body
Parser->>Endpoint: Return raw data
Endpoint->>Validation: Validate report array
rect rgb(200, 220, 255)
Note over Validation: Per-report validation
Validation->>Serializer: Serialize each report
Serializer->>Serializer: Check required fields,<br/>validate type/url/timestamp
end
alt All valid
Validation->>Metrics: Increment metrics<br/>per report type
Metrics->>Endpoint: Success (200)
else Any invalid
Validation->>Endpoint: Return 422<br/>with error details
end
sequenceDiagram
participant Replay as Replay Endpoint
participant DB as Nodestore
participant Seer as Seer Service
participant Response
Replay->>DB: Fetch error IDs from replay
Replay->>DB: Retrieve error events<br/>(title, message, timestamp)
rect rgb(220, 255, 220)
Note over Replay: Error Context Integration
Replay->>Replay: Interleave errors<br/>with breadcrumbs<br/>chronologically
end
Replay->>Seer: analyze_recording_segments<br/>(error_events, segments)
Seer->>Seer: Generate summary<br/>from merged logs
Seer->>Response: Return AI summary
sequenceDiagram
participant UI as Explore Chart
participant Config as Visualize Config
participant Query as Query Builder
rect rgb(255, 240, 200)
Note over UI: Old: Array-based yAxes
UI->>Config: visualize.yAxes = ['expr1', 'expr2']
Config->>Query: Multiple series per chart
end
rect rgb(200, 255, 220)
Note over UI: New: Single yAxis
UI->>Config: visualize.yAxis = 'expr1'
Config->>Query: Single series per chart
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Areas requiring extra attention:
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
static/app/views/performance/newTraceDetails/traceDrawer/details/span/eapSections/attributes.tsx (1)
59-70: Inconsistent filtering: hidden attributes only filtered during search.The current logic filters out hidden attributes only when a search query is present (lines 65-69), but returns all attributes including hidden ones when there's no search query (lines 61-63). This means
'is_segment','project_id', and'received'will be visible to users until they start searching, which is likely unintended.Apply this diff to filter hidden attributes consistently regardless of search state:
const sortedAndFilteredAttributes = useMemo(() => { - const sorted = sortAttributes(attributes); + const sorted = sortAttributes(attributes).filter( + attribute => !HIDDEN_ATTRIBUTES.includes(attribute.name) + ); if (!searchQuery.trim()) { return sorted; } return sorted.filter( attribute => - !HIDDEN_ATTRIBUTES.includes(attribute.name) && attribute.name.toLowerCase().trim().includes(searchQuery.toLowerCase().trim()) ); }, [attributes, searchQuery]);static/app/views/settings/organizationAuditLog/auditLogList.tsx (1)
437-440: Fix invalidwhite-spacevalue to avoid unintended wrapping
white-space: no-wrap;is not a valid CSS value and will be ignored by browsers, soMonoDetailtext may wrap unexpectedly. This is likely meant to prevent wrapping.You can correct this with:
-const MonoDetail = styled('code')` - font-size: ${p => p.theme.fontSizeMedium}; - white-space: no-wrap; -`; +const MonoDetail = styled('code')` + font-size: ${p => p.theme.fontSizeMedium}; + white-space: nowrap; +`;static/app/views/explore/hooks/useTopEvents.tsx (1)
19-21: Update the outdated comment.The comment mentions "when there are no multiple y-axes chart," but the code no longer checks for multiple y-axes since the multi-yAxis logic was removed. This could confuse future developers.
Apply this diff to update the comment:
- // We only support top events in aggregates mode for - // when there are no multiple y-axes chart and there is at least one group by. + // We only support top events in aggregates mode when there is at least one group by.
🧹 Nitpick comments (37)
static/app/views/settings/project/projectOwnership/codeOwnerFileTable.tsx (1)
110-165: Layout refactor toFlexis sound; consider deduplicating repeated propsWrapping each cell’s content in
<Flex align="center" gap={space(1)}>keeps table semantics intact and improves alignment; behavior should remain unchanged.You could optionally reduce duplication by extracting a small wrapper, e.g.:
+const TableCellFlex = (props: React.PropsWithChildren) => ( + <Flex align="center" gap={space(1)} {...props} /> +);and then using
<TableCellFlex>in place of each repeated<Flex align="center" gap={space(1)}>instance.src/sentry/migrations/0920_convert_org_saved_searches_to_views_revised.py (1)
13-15: No‑op migration body looks correct and safeTurning
convert_org_saved_searches_to_viewsinto a documented no‑op is a reasonable way to keep the migration in the chain while avoiding the faulty data logic, given that this migration was never run and a corrected version lives in0921_convert_org_saved_searches_to_views_rerevised.py. No further changes needed here.static/app/views/profiling/profileSummary/index.tsx (2)
338-346: Explicit typing improves type safety.The addition of explicit return type annotation and parameter types is a solid improvement for type safety and maintainability.
For complete consistency, consider adding explicit parameter typing to line 340 as well:
Apply this diff for consistency:
const flamegraphFrameFilter = useMemo((): ((frame: Frame) => boolean) => { if (frameFilter === 'all') { - return () => true; + return (frame: Frame) => true; } if (frameFilter === 'application') { return (frame: Frame) => frame.is_application; } return (frame: Frame) => !frame.is_application; }, [frameFilter]);
635-636: Replace @ts-expect-error with proper prop typing.The
@ts-expect-errordirective suppresses a legitimate TypeScript error. Instead of suppressing it, properly type the styled component props.Apply this diff to fix the typing:
-// @ts-expect-error TS(7008): Member 'hideRegressions' implicitly has an 'any' t... Remove this comment to see the full error message -const ProfileVisualizationContainer = styled('div')<{hideRegressions}>` +const ProfileVisualizationContainer = styled('div')<{hideRegressions: boolean}>` display: grid; /* false positive for grid layout */ /* stylelint-disable */ grid-template-areas: ${p => p.hideRegressions ? "'visualization'" : "'visualization digest'"}; grid-template-columns: ${p => (p.hideRegressions ? `100%` : `60% 40%`)}; flex: 1 1 100%; `;src/sentry/snuba/ourlogs.py (1)
159-171: Parameter propagation looks correct.The
equationsparameter is properly forwarded to the downstream function using keyword argument syntax, consistent with the other parameters.Optional observation: The
queryfunction (line 37) also has anequationsparameter that doesn't appear to be used within the function body. You may want to verify whether this is intentional or if there's additional wiring needed.src/sentry/replays/usecases/delete.py (1)
87-91: Good None-guard; minor comment nitShort-circuiting when
row["max_segment_id"] is Noneis a sensible way to avoidrange(None + 1)failures and unnecessary storage deletes for replays with no segments. This matches the semantics implied by the Snubamax(segment_id)aggregation.Nit: consider fixing the comment typo and making it a bit clearer that it is
max_segment_idthat can be null, e.g.:- # Null segment_ids can cause this to fail. If no segments were ingested then we can skip - # deleting the segements. + # A null max_segment_id indicates no segments were ingested, so we can skip deleting segments.tests/sentry/replays/tasks/test_delete_replays_bulk.py (1)
95-99: Test data now reflects None max_segment_id; consider adding a direct usecase testSwitching
max_segment_idtoNonehere is consistent with the updated delete logic and MatchedRow type, and it ensures the completion path can tolerate rows with no segments.Note that in this test
delete_matched_rowsis patched, so the new_make_recording_filenamesNone-guard isn’t exercised end-to-end. Consider adding a small unit test insrc/sentry/replays/usecases/delete.pythat asserts_make_recording_filenames(..., {"max_segment_id": None, ...})returns an empty list and does not attempt any deletes, to prevent regressions in the future.src/sentry/projectoptions/defaults.py (1)
193-209: Default changed to "off"; consider reusing the shared constant.The new default
"off"is consistent withDEFAULT_AUTOFIX_AUTOMATION_TUNING_DEFAULTand the tests. To avoid future drift, you could import and use that constant here instead of a raw string, e.g.:-from sentry.constants import TARGET_SAMPLE_RATE_DEFAULT +from sentry.constants import TARGET_SAMPLE_RATE_DEFAULT, DEFAULT_AUTOFIX_AUTOMATION_TUNING_DEFAULT @@ -# Should autofix run automatically on new issues -register(key="sentry:autofix_automation_tuning", default="off") +# Should autofix run automatically on new issues +register( + key="sentry:autofix_automation_tuning", + default=DEFAULT_AUTOFIX_AUTOMATION_TUNING_DEFAULT, +)src/sentry/hybridcloud/tasks/deliver_webhooks.py (3)
79-86: Explicit 30s deadline forschedule_webhook_deliverylooks reasonable; just ensure it matches expected scheduling costThe added
processing_deadline_duration=30caps this scheduler task’s runtime, which is good for fairness and avoiding worker starvation, especially withBATCH_SIZEup to 1000 mailboxes. As long as typical runs complete comfortably under 30s at your scale, this is fine; if you see repeated deadline hits, consider either dialing backBATCH_SIZEor nudging the deadline up slightly.
155-162: Aligndrain_mailboxdeadline withBATCH_SCHEDULE_OFFSETexpectations
drain_mailboxalready has its own in-function deadline via:
- Line 188:
deadline = timezone.now() + BATCH_SCHEDULE_OFFSET- Line 192: early exit once
timezone.now() >= deadline.With
processing_deadline_duration=300, the effective max runtime becomesmin(BATCH_SCHEDULE_OFFSET, 300s). That’s probably desired, but ifBACKOFF_INTERVALis ever configured above 5 minutes, the Taskworker deadline will win and the in-code “delivery_deadline” path might not fire. If that’s not intended, consider either:
- Documenting that
BACKOFF_INTERVALshould not exceed 5 minutes, or- Basing
processing_deadline_durationdirectly onBATCH_SCHEDULE_OFFSET.total_seconds()so they remain coupled.
233-240: 120s deadline fordrain_mailbox_parallelis consistent but worth sanity‑checking against batch size and worker_threadsThe new
processing_deadline_duration=120caps the parallel drain, which is good for preventing long‑running mailboxes from monopolizing a worker. Since the inner loop processes at mostworker_threadsrecords per iteration and respects its owndeadline = timezone.now() + BATCH_SCHEDULE_OFFSET(line 294), the same “min(external, internal)” runtime behavior applies here.It’s probably safe, but keep an eye on telemetry: if parallel drains regularly approach or exceed 120s (especially with higher
worker_threads), you may want to either:
- Slightly increase the deadline, or
- Trim
worker_threads/ batch sizes so that each task comfortably completes within 120s.src/sentry/tasks/auth/check_auth.py (1)
72-79: 60s deadline is fine, but there’s a subtle interaction withlast_syncedupdates that’s worth revisitingAdding
processing_deadline_duration=60is a good safeguard, but note the flow:
check_auth(line 44–58) updateslast_syncedfor the wholeidentity_ids_listbefore enqueuingcheck_auth_identities.check_auth_identities(lines 80–99) then loops overauth_identity_idsand callscheck_single_auth_identityper ID, with no internal time cutoff.If the task hits the 60s processing deadline mid‑loop, some identities in the chunk will have
last_syncedbumped but will never actually be refreshed until they age back past the cutoff again. That was already a risk on crashes, but introducing a hard time limit makes it more systematic under load.Consider one of:
- Moving the
last_syncedupdate intocheck_auth_identitiesas identities are processed, or- Reducing
chunk_size/ dynamically checking elapsed time insidecheck_auth_identitiesand re‑queueing any remaining IDs in a follow‑up task, so no identity is permanently “marked processed” without being refreshed.Also, for consistency, you may want to decide whether the deprecated
check_auth_identitytask should get a similar deadline or be retired once this path is fully rolled out.static/app/utils/analytics/replayAnalyticsEvents.tsx (1)
116-117: Consider adding specific parameters to zoom events.While the current
Record<string, unknown>type works, adding explicit parameters likescaleorzoom_levelwould make the analytics data more actionable and align with the specificity of other events in this file (e.g.,replay.play-pauseincludescontext,mobile,play, etc.).Apply this diff to add more specific typing:
- 'replay.timeline.zoom-in': Record<string, unknown>; - 'replay.timeline.zoom-out': Record<string, unknown>; + 'replay.timeline.zoom-in': { + new_scale: number; + }; + 'replay.timeline.zoom-out': { + new_scale: number; + };Then update the tracking calls in
static/app/components/replays/timeAndScrubberGrid.tsxto include the scale value:trackAnalytics('replay.timeline.zoom-out', { organization, new_scale: newScale, }); trackAnalytics('replay.timeline.zoom-in', { organization, new_scale: newScale, });Also applies to: 158-159
static/app/views/alerts/list/rules/row.tsx (1)
269-278: Remove unnecessary nested Flex wrapper.The inner
Flexat lines 270-272 wraps onlyCombinedAlertBadgeand adds no layout value since the outerFlexalready providesalign="center".Apply this diff to simplify the structure:
<Flex align="center"> - <Flex align="center"> - <CombinedAlertBadge rule={rule} /> - </Flex> + <CombinedAlertBadge rule={rule} /> {!isUptime && !isCron && ( <MarginLeft> <AlertRuleStatus rule={rule} /> </MarginLeft> )} </Flex>src/sentry/grouping/parameterization.py (1)
335-350: Remove now-unused_handle_regex_matchinparametrize_w_experimentsWith all experiments funneled through
experiment.run(content, _incr_counter), the_handle_regex_matchhelper insideparametrize_w_experimentsis dead code and duplicates the logic already used inparametrize_w_regex. Dropping it will reduce confusion without changing behavior.- def _handle_regex_match(match: re.Match[str]) -> str: - # Find the first (should be only) non-None match entry, and sub in the placeholder. For - # example, given the groupdict item `('hex', '0x40000015')`, this returns '<hex>' as a - # replacement for the original value in the string. - for key, value in match.groupdict().items(): - if value is not None: - self.matches_counter[key] += 1 - return f"<{key}>" - return "" - - for experiment in self._experiments: + for experiment in self._experiments: if not should_run(experiment.name): continue content = experiment.run(content, _incr_counter)src/sentry/preprod/__init__.py (1)
1-1: Consider explicit exports for better maintainability.Wildcard imports make it unclear which symbols are being exported and can lead to namespace pollution. Consider using an explicit
__all__list to document the public API.Apply this diff to make exports explicit:
-from .analytics import * # NOQA +from .analytics import PreprodArtifactApiAssembleEvent + +__all__ = ["PreprodArtifactApiAssembleEvent"]src/sentry/issues/endpoints/browser_reporting_collector.py (1)
126-126: Remove redundantstr()call.The
typefield is already validated as a ChoiceField with string choices, sobrowser_report["type"]is guaranteed to be a string. The explicitstr()conversion is unnecessary.Apply this diff:
metrics.incr( "browser_reporting.raw_report_received", - tags={"browser_report_type": str(browser_report["type"])}, + tags={"browser_report_type": browser_report["type"]}, sample_rate=1.0, # XXX: Remove this once we have a ballpark figure )src/sentry/feedback/usecases/feedback_summaries.py (1)
51-54: Consider adding validation for prompt constraints.The parse_response function doesn't verify that the LLM output adheres to the constraints specified in the prompt (55-word maximum, 2-sentence maximum, "Users..." prefix). While LLMs typically follow instructions, adding validation could help catch edge cases and provide better error handling.
Example validation to consider:
def parse_response(text): cleaned = re.sub(r"\s+", " ", text).strip() # Validate word count word_count = len(cleaned.split()) if word_count > 55: logger.warning(f"Summary exceeds 55 words: {word_count} words") # Validate sentence count sentence_count = len(re.split(r'[.!?]+', cleaned.strip())) if sentence_count > 2: logger.warning(f"Summary exceeds 2 sentences: {sentence_count} sentences") return cleanedstatic/app/views/feedback/feedbackListPage.tsx (1)
125-129: Consider a slightly larger gap for better visual separation.The current gap of
space(1)(typically 8px) between the summary and list might be too small given that the summary is a visually distinct AI-generated component. Consider usingspace(1.5)orspace(2)for clearer separation.const SummaryListContainer = styled('div')` display: flex; flex-direction: column; - gap: ${space(1)}; + gap: ${space(2)}; `;static/app/components/feedback/feedbackSummary.tsx (1)
15-21: Redundant feature flag check.The feature flag is checked both in the hook's
enabledcondition (line 36-38 of useFeedbackSummary.tsx) and here at line 16. While this defensive approach is safe, the check here is redundant since the hook won't fetch data when the flag is disabled.Consider simplifying to:
if ( - !organization.features.includes('user-feedback-ai-summaries') || tooFewFeedbacks || isError ) { return null; }static/app/views/explore/spans/spansTab.tsx (1)
346-356: LGTM with optional refactor suggestion.The change correctly adapts to the single-axis model. The code wraps the single
yAxisin an array and immediately flatMaps over it, which works correctly.Consider simplifying the series construction to avoid the wrap/unwrap pattern:
- const dedupedYAxes = [visualize.yAxis]; - const series = dedupedYAxes - .flatMap(yAxis => timeseriesResult.data[yAxis]) - .filter(defined); + const series = (timeseriesResult.data[visualize.yAxis] ?? []).filter(defined);This would eliminate the intermediate array and flatMap while achieving the same result.
src/sentry/integrations/source_code_management/commit_context.py (1)
142-144: New PR comment helpers look solid; consider clarifying title-length semanticsThe
_truncate_title/ template helpers and environment extraction are implemented safely and readably. Two small nits you may want to consider:
ISSUE_TITLE_MAX_LENGTHcurrently caps the pre‑ellipsis length; the final rendered title can exceed that length by up to 3 chars once"..."is appended. If you need a hard cap on the total length (e.g. provider limits), you might want to subtract the ellipsis length from the slice or rename the constant to reflect “max base length.”get_environment_infologs atinfoon any exception; if this ends up noisy in practice, downgrading todebug(or adding a dedicated sampling key) might be preferable, since failure to fetch environment is non-critical.Otherwise this is a nice, reusable abstraction for PR comment formatting.
Also applies to: 577-607
static/app/views/explore/hooks/useAddToDashboard.tsx (1)
50-58: Dashboard add flow is correctly migrated to single yAxis; invariant on visualizes still implicitUsing
const yAxis = visualizes[visualizeIndex]!.yAxisand passingyAxis: [yAxis]into the discover query cleanly adapts this hook to the single‑axis model while preserving the existing fields/sorts behavior.This function continues to assume that
visualizes[visualizeIndex]always exists (the non‑null assertion, pluschartTypeaccess below). If there’s any path wherevisualizescould be empty or the index out of range, it may be worth adding a small guard/early return to avoid runtime errors, but if the invariant is guaranteed by the caller, the current code is fine.Also applies to: 64-71
static/app/views/explore/toolbar/toolbarSaveAs.tsx (1)
61-62: Single-yAxis migration looks good; consider deduping alert options and making compare invariant explicitThe switch to a single
yAxisper visualize is wired through consistently here:
visualizeYAxes = visualizes.map(v => v.yAxis)feeds both alert URLs and the compare route.- Chart options now treat each chart as having exactly one axis (
[chart.yAxis]), which matches the new Visualize shape.- The compare route payload uses
yAxes: [visualizeYAxes[0]!], preserving the existing multi-query API contract while adapting to a single local axis.Two small follow-ups you might want to consider:
- Alert submenu duplication:
alertsUrlsnow maps directly overvisualizeYAxeswithout deduplication. If multiple charts share the same aggregate, users will see repeated “An Alert for <metric>” entries where previouslydedupeArrayavoided this. A lightweight fix would be:const uniqueYAxes = Array.from(new Set(visualizeYAxes)); const alertsUrls = uniqueYAxes.map((yAxis, index) => { /* ... */ });
- Compare route assumptions: Both
visualizeYAxes[0]!andvisualizes[0]!.chartTypeassume there is at least one visualize. If that invariant isn’t strictly guaranteed upstream, a simple guard (e.g. early return or disabling the compare button) would prevent a runtime crash.Functionally, though, the single‑axis migration is sound.
Also applies to: 72-95, 152-175, 296-298
static/app/views/explore/utils.tsx (1)
283-295: Explore utils are correctly adapted to single yAxis; minor cleanup possibleThe changes in both helpers align with the new Visualize shape:
viewSamplesTargetnow derives the underlying field fromvisualize.yAxisviaparseFunction, preserving the previous “first argument becomes column” behavior while avoiding any yAxes iteration.computeVisualizeSampleTotalsnow effectively aggregates series for a single axis per visualize, which matches how charts are defined elsewhere after the refactor.Given each
Visualizenow has only one axis,dedupedYAxes = [visualize.yAxis];is a bit of leftover structure. You could simplify to:const series = (data[visualize.yAxis] ?? []).filter(defined);(or whatever the concrete type for
data[visualize.yAxis]is), which would be slightly clearer without changing behavior.Functionally, the current code is sound.
Also applies to: 433-443
static/app/views/codecov/tests/onboardingSteps/addUploadToken.tsx (1)
6-6: Token rows Flex usage looks fine; minor duplication opportunityThe nested
<Flex justify="space-between" gap={space(1)}>blocks correctly lay out the key/token pair against the action button in both full and truncated token states, matching the intended “left text, right button” behavior.If you want to reduce repetition, you could extract a tiny
TokenRowhelper that takes{token, buttonLabel, buttonPriority, onClick}and renders this Flex structure once, but that’s purely optional given the small size here.Also applies to: 77-97
tests/js/fixtures/tabularColumns.ts (1)
1-9: TabularColumnsFixture helper looks goodStraightforward wrapper around
TabularColumnFixture; types are tight and behavior is clear. If you want to trim noise, you can omit the explicitparam: Partial<TabularColumn>in themapcallback and rely on inference, but that's purely optional.static/app/views/dashboards/widgets/tableWidget/tableWidgetVisualization.spec.tsx (1)
14-106: Test coverage for TableWidgetVisualization looks solidThe spec exercises the key paths: default rendering, custom column order/display names, formatter behavior for duration/percentage/rate, and custom head/body renderers. This gives good confidence in the new table widget behavior.
If you find these tests getting brittle over time, you might consider asserting on column labels via more semantic queries (e.g.
getByRole('columnheader', {name: /.../})), but what you have here is fine for now.static/app/views/dashboards/widgets/tableWidget/defaultTableCellRenderers.tsx (1)
24-42: Align on field key for alignment and tighten types in default renderersThe default renderers look good overall; a few small tweaks would make them more robust and consistent:
- Use the field key for alignment, not the display name
Right now alignment is derived from
column.name:const align = fieldAlignment(column.name, column.type as ColumnValueType);If callers supply a custom
name(e.g."Count of Span Duration"), this may diverge from the actual field key (e.g."count(span.duration)"). Using the key is safer:- const align = fieldAlignment(column.name, column.type as ColumnValueType); + const align = fieldAlignment(String(column.key), column.type as ColumnValueType);
- Avoid over‑narrowing the unit to
string
unitis currently cast tostring:const unit = tableData.meta.units?.[columnKey] as string;That drops potential type information (e.g. a
DurationUnitenum) and can misrepresentundefined. Prefer preserving the declared unit type:- const unit = tableData.meta.units?.[columnKey] as string; + const unit = tableData.meta.units?.[columnKey];
- Optional: reuse
CellWrapperfor body cells for consistent layoutIf you want body cells to share the same nowrap/full‑width behavior as headers, you could wrap the rendered value similarly, using the same alignment logic on the field key. That’s not strictly necessary, but can improve visual consistency.
All of these are incremental and safe to defer, but they’ll make the defaults a bit more future‑proof.
Also applies to: 75-83, 95-104
static/app/views/dashboards/widgets/tableWidget/tableWidgetVisualization.tsx (1)
91-99: Guard column derivation against missing meta
columnOrderfalls back to deriving columns fromtableData.meta.fields:const columnOrder: TabularColumn[] = columns ?? Object.keys(tableData?.meta.fields).map((key: string) => ({ key, name: key, width: -1, type: tableData?.meta.fields[key], }));Assuming
TabularData.meta.fieldsis always present this is fine, but if a caller ever passes a partially populatedtableData(e.g. in a loading or error state),tableData.metabeingundefinedwould causeObject.keysto throw.You can make this a bit more defensive at low cost:
- Object.keys(tableData?.meta.fields).map((key: string) => ({ + Object.keys(tableData?.meta?.fields ?? {}).map((key: string) => ({ key, name: key, width: -1, - type: tableData?.meta.fields[key], + type: tableData?.meta?.fields?.[key], }));This keeps the existing behavior when meta is present, but degrades gracefully to an empty column set otherwise.
static/app/views/dashboards/widgets/tableWidget/tableWidgetVisualization.stories.tsx (3)
13-33: Solid introductory story; minor copy nitsThe basic example is clear and correctly wires
TableWidgetVisualizationwithsampleHTTPRequestTableData. Consider tightening the copy as a low-priority polish:
- Line 27: "Below is the the most basic example..." → remove duplicated "the".
- Line 29: Add a period at the end of the sentence.
These are purely editorial and optional.
35-89: Good coverage of data/columns behavior; consider prettifying JSON outputThe story clearly illustrates:
- Empty
datahandling viatableWithEmptyData.- Optional
columnsoverride vsmeta.fieldsfallback.- Custom column naming and ordering.
One optional improvement for readability in the snippet:
-${JSON.stringify(tableWithEmptyData)} +${JSON.stringify(tableWithEmptyData, null, 2)}This will render nicely formatted, multi-line JSON in
<CodeSnippet language="json">, making the example easier to scan.
91-128: Custom cell rendering example is clear and aligned with intended APIThe story effectively demonstrates:
- Header customization via
renderTableHeadCell.- Body customization via
renderTableBodyCell.- The pattern of returning
undefinedto fall back to default rendering for selected columns.As a tiny polish, you could add a period at the end of the sentence on lines 113–115 (“allowing for partial custom rendering.”), but that’s optional.
static/app/views/explore/types.tsx (1)
1-21: Clarify base-props docs vs actual usageThe shared
UseTraceItemAttributeBasePropsshape looks good, but the doc comments (“currently only supports LOGS” / “string and number”) may already be outdated given SPANS and new hooks using this type. Consider updating the comments (or tightening the type) so consumers aren’t misled about supported datasets and attribute types.static/app/views/explore/hooks/useGetTraceItemAttributeValues.spec.tsx (1)
13-16: Tests match new hook behavior; optional cleanupThe tests correctly validate that string attributes hit the
/values/endpoint and number attributes always return an empty array without issuing a request. In the “number type” test, the mock response is never used; you could drop that mock entirely to make the intent (no network call) even clearer.Also applies to: 30-33, 57-101, 103-148
static/app/views/explore/hooks/useGetTraceItemAttributeValues.tsx (1)
12-28: AlignUseGetTraceItemAttributeValuesPropswith base props usageThe hook behavior (early-return for functions/numbers, query construction, and filtering out undefined values) looks solid. However,
UseGetTraceItemAttributeValuesPropsextendsUseTraceItemAttributeBasePropsbut only usesprojectIds, ignoring theprojects?: Project[]field that the base type advertises. That differs fromuseTraceItemAttributeKeys, which does respectprojects.To avoid surprising consumers who might pass
projectshere, consider either:
- Wiring
projectsthrough, similar to the keys hook:-export function useGetTraceItemAttributeValues({ - traceItemType, - projectIds, - datetime, - type = 'string', -}: UseGetTraceItemAttributeValuesProps) { +export function useGetTraceItemAttributeValues({ + traceItemType, + projectIds, + datetime, + type = 'string', + projects, +}: UseGetTraceItemAttributeValuesProps) { ... - const queryKey = traceItemAttributeValuesQueryKey({ + const queryKey = traceItemAttributeValuesQueryKey({ orgSlug: organization.slug, attributeKey: tag.key, search: queryString, - projectIds: projectIds ?? selection.projects, + projectIds: + projectIds ?? + (projects ? projects.map(project => project.id) : selection.projects), datetime: datetime ?? selection.datetime, traceItemType, type, }); @@ - [ - api, - type, - organization.slug, - projectIds, - selection.projects, - selection.datetime, - datetime, - traceItemType, - ] + [ + api, + type, + organization.slug, + projectIds, + projects, + selection.projects, + selection.datetime, + datetime, + traceItemType, + ]
- Or, if you don’t intend this hook to accept
projects, stop extending the base props and define a leaner props type.Either approach will make the API surface less ambiguous.
Also applies to: 78-104, 118-127
tests/sentry/replays/test_project_replay_summarize_breadcrumbs.py (1)
283-331:test_get_request_datacorrectly validates interleaving; minor robustness opportunityThis test does a good job of pinning the desired ordering between two breadcrumb events and two
ErrorEvents at different timestamps, ensuringget_request_data()yields[error-before, log1, log2, error-after]. That gives solid protection around the chronological merge logic.If you later change
get_request_data()to avoid in-place sorting (e.g., switching tosorted()as suggested in the endpoint file), this test will continue to hold without modification, which is ideal.If you want to go a step further, you could also assert that the original
error_eventslist ordering is preserved after the call (i.e., the helper doesn’t mutate its inputs), but that’s optional and only needed if callers start reusing the list elsewhere.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (106)
devservices/config.yml(4 hunks)src/sentry/constants.py(1 hunks)src/sentry/feedback/usecases/feedback_summaries.py(1 hunks)src/sentry/grouping/parameterization.py(2 hunks)src/sentry/hybridcloud/tasks/deliver_webhooks.py(3 hunks)src/sentry/integrations/github/integration.py(1 hunks)src/sentry/integrations/gitlab/integration.py(1 hunks)src/sentry/integrations/source_code_management/commit_context.py(2 hunks)src/sentry/issues/endpoints/browser_reporting_collector.py(4 hunks)src/sentry/issues/grouptype.py(1 hunks)src/sentry/migrations/0917_convert_org_saved_searches_to_views.py(1 hunks)src/sentry/migrations/0920_convert_org_saved_searches_to_views_revised.py(1 hunks)src/sentry/options/defaults.py(0 hunks)src/sentry/preprod/__init__.py(1 hunks)src/sentry/preprod/analytics.py(1 hunks)src/sentry/preprod/api/endpoints/organization_preprod_artifact_assemble.py(2 hunks)src/sentry/projectoptions/defaults.py(1 hunks)src/sentry/replays/endpoints/project_replay_summarize_breadcrumbs.py(5 hunks)src/sentry/replays/usecases/delete.py(2 hunks)src/sentry/snuba/ourlogs.py(2 hunks)src/sentry/tasks/auth/check_auth.py(1 hunks)src/sentry/workflow_engine/endpoints/validators/base/detector.py(4 hunks)src/sentry/workflow_engine/processors/delayed_workflow.py(6 hunks)src/sentry/workflow_engine/processors/workflow.py(1 hunks)static/app/components/codeSnippet.tsx(2 hunks)static/app/components/codecov/branchSelector/branchSelector.tsx(2 hunks)static/app/components/codecov/datePicker/dateSelector.tsx(2 hunks)static/app/components/codecov/integratedOrgSelector/integratedOrgSelector.tsx(4 hunks)static/app/components/codecov/repoPicker/repoSelector.tsx(2 hunks)static/app/components/core/button/styles.chonk.tsx(1 hunks)static/app/components/events/eventAttachments.tsx(2 hunks)static/app/components/events/groupingInfo/groupingVariant.tsx(1 hunks)static/app/components/events/interfaces/spans/newTraceDetailsHeader.tsx(4 hunks)static/app/components/feedback/feedbackSummary.tsx(1 hunks)static/app/components/feedback/list/useFeedbackSummary.tsx(1 hunks)static/app/components/group/times.tsx(3 hunks)static/app/components/replays/breadcrumbs/breadcrumbItem.tsx(1 hunks)static/app/components/replays/timeAndScrubberGrid.tsx(4 hunks)static/app/components/scrollCarousel.tsx(1 hunks)static/app/utils/analytics/replayAnalyticsEvents.tsx(2 hunks)static/app/views/alerts/list/rules/alertRuleStatus.tsx(3 hunks)static/app/views/alerts/list/rules/row.tsx(3 hunks)static/app/views/codecov/tests/onboardingSteps/addUploadToken.tsx(2 hunks)static/app/views/dashboards/widgetCard/chart.tsx(4 hunks)static/app/views/dashboards/widgets/common/types.tsx(1 hunks)static/app/views/dashboards/widgets/tableWidget/defaultTableCellRenderers.tsx(1 hunks)static/app/views/dashboards/widgets/tableWidget/fixtures/sampleHTTPRequestTableData.ts(1 hunks)static/app/views/dashboards/widgets/tableWidget/tableWidgetVisualization.spec.tsx(1 hunks)static/app/views/dashboards/widgets/tableWidget/tableWidgetVisualization.stories.tsx(1 hunks)static/app/views/dashboards/widgets/tableWidget/tableWidgetVisualization.tsx(1 hunks)static/app/views/explore/charts/index.tsx(4 hunks)static/app/views/explore/components/traceItemSearchQueryBuilder.tsx(2 hunks)static/app/views/explore/contexts/pageParamsContext/aggregateFields.tsx(4 hunks)static/app/views/explore/contexts/pageParamsContext/index.spec.tsx(17 hunks)static/app/views/explore/contexts/pageParamsContext/index.tsx(4 hunks)static/app/views/explore/contexts/pageParamsContext/sortBys.tsx(2 hunks)static/app/views/explore/contexts/pageParamsContext/visualizes.spec.tsx(3 hunks)static/app/views/explore/contexts/pageParamsContext/visualizes.tsx(4 hunks)static/app/views/explore/hooks/useAddToDashboard.tsx(2 hunks)static/app/views/explore/hooks/useAnalytics.tsx(5 hunks)static/app/views/explore/hooks/useExploreAggregatesTable.tsx(1 hunks)static/app/views/explore/hooks/useExploreTimeseries.tsx(3 hunks)static/app/views/explore/hooks/useGetTraceItemAttributeKeys.tsx(1 hunks)static/app/views/explore/hooks/useGetTraceItemAttributeValues.spec.tsx(4 hunks)static/app/views/explore/hooks/useGetTraceItemAttributeValues.tsx(3 hunks)static/app/views/explore/hooks/useTopEvents.tsx(1 hunks)static/app/views/explore/hooks/useTraceItemAttributeKeys.tsx(2 hunks)static/app/views/explore/spans/spansTab.tsx(1 hunks)static/app/views/explore/tables/aggregateColumnEditorModal.spec.tsx(14 hunks)static/app/views/explore/tables/aggregateColumnEditorModal.tsx(4 hunks)static/app/views/explore/toolbar/index.spec.tsx(11 hunks)static/app/views/explore/toolbar/toolbarSaveAs.tsx(2 hunks)static/app/views/explore/toolbar/toolbarSortBy.tsx(1 hunks)static/app/views/explore/toolbar/toolbarVisualize.tsx(6 hunks)static/app/views/explore/types.tsx(1 hunks)static/app/views/explore/utils.spec.tsx(1 hunks)static/app/views/explore/utils.tsx(2 hunks)static/app/views/feedback/feedbackListPage.tsx(4 hunks)static/app/views/insights/common/components/chartActionDropdown.tsx(1 hunks)static/app/views/insights/pages/transactionNameSearchBar.tsx(2 hunks)static/app/views/organizationStats/teamInsights/teamMisery.tsx(3 hunks)static/app/views/performance/newTraceDetails/traceDrawer/details/span/eapSections/attributes.tsx(2 hunks)static/app/views/performance/newTraceDetails/traceTabsAndVitals.tsx(4 hunks)static/app/views/performance/newTraceDetails/traceWaterfall.tsx(3 hunks)static/app/views/profiling/profileSummary/index.tsx(1 hunks)static/app/views/replays/detail/ai/index.tsx(2 hunks)static/app/views/settings/dynamicSampling/organizationSampleRateInput.tsx(4 hunks)static/app/views/settings/organizationAuditLog/auditLogList.tsx(2 hunks)static/app/views/settings/organizationIntegrations/detailedView/integrationLayout.tsx(2 hunks)static/app/views/settings/project/projectOwnership/codeOwnerFileTable.tsx(4 hunks)static/gsAdmin/views/instanceLevelOAuth/instanceLevelOAuthDetails.tsx(3 hunks)tests/js/fixtures/tabularColumn.ts(1 hunks)tests/js/fixtures/tabularColumns.ts(1 hunks)tests/sentry/api/endpoints/test_browser_reporting_collector.py(4 hunks)tests/sentry/api/endpoints/test_project_details.py(1 hunks)tests/sentry/api/serializers/test_project.py(1 hunks)tests/sentry/grouping/test_parameterization.py(1 hunks)tests/sentry/integrations/github/tasks/test_pr_comment.py(6 hunks)tests/sentry/integrations/gitlab/tasks/test_pr_comment.py(6 hunks)tests/sentry/migrations/test_0917_convert_org_saved_searches_to_views.py(0 hunks)tests/sentry/replays/tasks/test_delete_replays_bulk.py(1 hunks)tests/sentry/replays/test_project_replay_summarize_breadcrumbs.py(5 hunks)tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py(1 hunks)tests/sentry/workflow_engine/endpoints/test_organization_detector_index.py(1 hunks)tests/sentry/workflow_engine/processors/test_delayed_workflow.py(6 hunks)tests/snuba/api/endpoints/test_organization_events_stats.py(4 hunks)
💤 Files with no reviewable changes (2)
- src/sentry/options/defaults.py
- tests/sentry/migrations/test_0917_convert_org_saved_searches_to_views.py
| class BrowserReportSerializer(serializers.Serializer[Any]): | ||
| """Serializer for validating browser report data structure.""" | ||
|
|
||
| body = serializers.DictField() | ||
| type = serializers.ChoiceField(choices=BROWSER_REPORT_TYPES) | ||
| url = serializers.URLField() | ||
| user_agent = serializers.CharField() | ||
| destination = serializers.CharField() | ||
| attempts = serializers.IntegerField(min_value=1) | ||
| # Fields that do not overlap between specs | ||
| # We need to support both specs | ||
| age = serializers.IntegerField(required=False) | ||
| timestamp = serializers.IntegerField(required=False, min_value=0) | ||
|
|
||
| def validate_timestamp(self, value: int) -> int: | ||
| """Validate that age is absent, but timestamp is present.""" | ||
| if self.initial_data.get("age"): | ||
| raise serializers.ValidationError("If timestamp is present, age must be absent") | ||
| return value | ||
|
|
||
| def validate_age(self, value: int) -> int: | ||
| """Validate that age is present, but not timestamp.""" | ||
| if self.initial_data.get("timestamp"): | ||
| raise serializers.ValidationError("If age is present, timestamp must be absent") | ||
| return value |
There was a problem hiding this comment.
Consider validating that at least one of age or timestamp is present.
The W3C specs referenced in the comments indicate two different standards: Working Draft uses timestamp while Editor's Draft uses age. The current validators ensure mutual exclusion (both fields cannot be present simultaneously), but neither validator ensures at least one is present.
If both fields are optional and absent, no validation error will be raised, which may lead to incomplete report data.
Apply this diff to add a class-level validator:
def validate_age(self, value: int) -> int:
"""Validate that age is present, but not timestamp."""
if self.initial_data.get("timestamp"):
raise serializers.ValidationError("If age is present, timestamp must be absent")
return value
+
+ def validate(self, attrs: dict[str, Any]) -> dict[str, Any]:
+ """Ensure at least one of age or timestamp is present."""
+ if "age" not in attrs and "timestamp" not in attrs:
+ raise serializers.ValidationError(
+ "Either 'age' or 'timestamp' must be present per W3C Reporting API specs"
+ )
+ return attrs🤖 Prompt for AI Agents
In src/sentry/issues/endpoints/browser_reporting_collector.py around lines 36 to
60, add a class-level validation to ensure at least one of age or timestamp is
provided: implement a validate(self, attrs) method that checks if neither
attrs.get("age") nor attrs.get("timestamp") is present and raises a
serializers.ValidationError indicating that one of age or timestamp must be
supplied (attach error to a non-field error or to both fields for clarity); keep
the existing per-field mutual-exclusion validators intact.
| analytics.record( | ||
| "preprod_artifact.api.assemble", | ||
| organization_id=project.organization_id, | ||
| project_id=project.id, | ||
| user_id=request.user.id, | ||
| ) |
There was a problem hiding this comment.
Handle potential AttributeError when accessing user ID.
Line 85 accesses request.user.id without checking if the user is authenticated. For anonymous users or certain authentication types, request.user might not have an id attribute, causing an AttributeError.
Apply this diff to safely access the user ID:
analytics.record(
"preprod_artifact.api.assemble",
organization_id=project.organization_id,
project_id=project.id,
- user_id=request.user.id,
+ user_id=getattr(request.user, 'id', None),
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| analytics.record( | |
| "preprod_artifact.api.assemble", | |
| organization_id=project.organization_id, | |
| project_id=project.id, | |
| user_id=request.user.id, | |
| ) | |
| analytics.record( | |
| "preprod_artifact.api.assemble", | |
| organization_id=project.organization_id, | |
| project_id=project.id, | |
| user_id=getattr(request.user, 'id', None), | |
| ) |
🤖 Prompt for AI Agents
In src/sentry/preprod/api/endpoints/organization_preprod_artifact_assemble.py
around lines 81 to 86, the analytics call uses request.user.id without guarding
against anonymous or non-standard user objects; replace that direct access with
a safe retrieval (e.g., use getattr(request.user, "id", None) or check
hasattr(request.user, "id") and pass None when absent) so the analytics.record
call never raises AttributeError for unauthenticated users.
| background: ${p => | ||
| p.transparentMask | ||
| ? 'linear-gradient(to right, rgba(255, 255, 255, 0), rgba(255, 255, 255, 1))' | ||
| ? `linear-gradient(to right, transparent, ${p.theme.background})` |
There was a problem hiding this comment.
Inconsistency with LeftMask causes incorrect gradient interpolation.
The RightMask uses the CSS transparent keyword, which is equivalent to rgba(0, 0, 0, 0) (transparent black). This creates incorrect color interpolation, especially noticeable in light themes where the gradient will transition through grey tones instead of smoothly fading from the theme background to transparent.
The LeftMask correctly uses Color(p.theme.background).alpha(0).rgb().string() to create a transparent version of the theme background color. Both masks should use the same approach for visual consistency and correct color interpolation.
Apply this diff to fix the inconsistency:
- ? `linear-gradient(to right, transparent, ${p.theme.background})`
+ ? `linear-gradient(to right, ${Color(p.theme.background).alpha(0).rgb().string()}, ${p.theme.background})`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ? `linear-gradient(to right, transparent, ${p.theme.background})` | |
| ? `linear-gradient(to right, ${Color(p.theme.background).alpha(0).rgb().string()}, ${p.theme.background})` |
🤖 Prompt for AI Agents
In static/app/components/scrollCarousel.tsx around line 204, the RightMask
currently uses the CSS keyword "transparent" which interpolates from transparent
black and causes wrong gradient colors in light themes; change the right-side
gradient stop to use the same transparentized theme background as LeftMask by
replacing "transparent" with Color(p.theme.background).alpha(0).rgb().string()
(i.e., build a transparent version of the theme background color) so both masks
use identical color interpolation and visual behavior.
| assert set(data.keys()) == {x[0] for x in expected_message_counts[:5]}.union({"Other"}) | ||
|
|
||
| for index, (message, count) in enumerate(expected_message_counts[:5]): | ||
| assert [{"count": count}] in data[message]["data"][0] |
There was a problem hiding this comment.
Fix incorrect assertion pattern.
The assertions directly access data[message]["data"][0] (a single tuple), checking if a list is inside it. This doesn't match the pattern used in the spans tests and will likely fail or give incorrect results.
Apply this diff to fix the assertion pattern:
for index, (message, count) in enumerate(expected_message_counts[:5]):
- assert [{"count": count}] in data[message]["data"][0]
+ assert [{"count": count}] in [attrs for _, attrs in data[message]["data"]]
assert data[message]["order"] == index
other = data["Other"]
assert other["order"] == 5
- assert [{"count": 10}] in other["data"][0]
+ assert [{"count": 10}] in [attrs for _, attrs in other["data"]]Also applies to: 3189-3189
🤖 Prompt for AI Agents
In tests/snuba/api/endpoints/test_organization_events_stats.py around lines 3184
and 3189, the test currently does `assert [{"count": count}] in
data[message]["data"][0]` which is wrong because `data[message]["data"][0]` is a
tuple; update both assertions to check inside the tuple's values instead — e.g.
assert [{"count": count}] in data[message]["data"][0][1] — so you're asserting
the expected dict list is present in the tuple's payload rather than in the
tuple itself.
20 participants
Test 5
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Replicated from ai-code-review-evaluation/sentry-coderabbit#5