Skip to content

Replays Self-Serve Bulk Delete System#16

Open
ShashankFC wants to merge 32 commits into
replays-delete-vulnerablefrom
replays-delete-stable
Open

Replays Self-Serve Bulk Delete System#16
ShashankFC wants to merge 32 commits into
replays-delete-vulnerablefrom
replays-delete-stable

Conversation

@ShashankFC

@ShashankFC ShashankFC commented Jan 22, 2026

Copy link
Copy Markdown
Contributor

Test 5

Summary by CodeRabbit

Release Notes

  • New Features

    • Added AI-powered feedback summaries in the feedback list.
    • Introduced table widget visualization for dashboards with customizable columns.
    • Added owner assignment support for workflow detectors.
    • Enhanced replay summaries with error context integration.
  • Bug Fixes

    • Improved PR comment formatting with better issue title and environment display.
    • Refined feedback summary generation with stricter length and format constraints.
    • Disabled autofix automation tuning by default for improved stability.

Replicated from ai-code-review-evaluation/sentry-coderabbit#5


Note

Medium Risk
Touches multiple product areas (AI pipelines, replay summarization, dashboards rendering) and modifies request validation and async task deadlines; regressions could affect user-facing summaries, table rendering behind a flag, and background processing behavior.

Overview
Adds AI summarization surfaces: a new frontend FeedbackSummary component + hook calling /feedback-summary/, and tighter LLM prompting/format constraints for feedback summary generation.

Enhances replay breadcrumb summarization by pulling error_ids for a replay, fetching event details from nodestore, and interleaving error messages with breadcrumb logs before sending to Seer (with a query param to disable error context).

Introduces a new TableWidgetVisualization (GridEditable-based) with default cell renderers, fixtures, stories, and tests, and gates dashboard table rendering behind the use-table-widget-visualization feature flag.

Operational/cleanup changes: improved merged PR bot issue formatting (truncate titles + include environment), adds strict serializer validation to the browser reporting collector, adds detector owner support in workflow validators, sets taskworker processing_deadline_duration on several control-plane tasks, tweaks replay deletion to handle null max_segment_id, changes default autofix tuning to off, removes unused parameterization regex experiment/option, adds snuba equations passthrough, updates replay analytics event names/zoom tracking, and updates devservices with new snuba-metrics + tracing mode consumers.

Written by Cursor Bugbot for commit ea188e2. Configure here.

armenzg and others added 30 commits June 20, 2025 12:49
…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
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
![Screenshot 2025-06-20 at 12 00
54 PM](https://github.com/user-attachments/assets/97ca72da-0a52-4446-9825-cd4fcb505adf)

after
![Screenshot 2025-06-20 at 11 59
29 AM](https://github.com/user-attachments/assets/a4284d2b-c9f5-442f-b010-7fe72a598e39)
### 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.
mrduncan and others added 2 commits June 20, 2025 13:20
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.
@ShashankFC

Copy link
Copy Markdown
Contributor Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 7 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

units: {},
},
}}
/>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Feature-flagged table renders no widget data

High Severity

When organization.features includes use-table-widget-visualization, tableResultComponent renders TableWidgetVisualization with hardcoded empty columns and empty tableData. This drops all real result.data and result.meta, so the table appears blank despite successful query results.

Fix in Cursor Fix in Web

const queryKey = useMemo(
() => ['use-trace-item-attribute-keys', queryOptions],
[queryOptions]
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Trace attribute cache key misses organization scope

Medium Severity

useTraceItemAttributeKeys builds queryKey from only queryOptions and omits the org identifier. Since queryFn fetches /organizations/{slug}/trace-items/attributes/, cached results can be reused across organizations with matching params, returning wrong attribute keys.

Additional Locations (1)

Fix in Cursor Fix in Web

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")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Age/timestamp mutual exclusion bypasses zero values

Low Severity

BrowserReportSerializer checks mutual exclusivity with truthy tests on self.initial_data.get("age") and get("timestamp"). If either field is 0, validation treats it as absent, so reports containing both age and timestamp can pass incorrectly.

Fix in Cursor Fix in Web

# Check if we need to yield any error messages that occurred before this event
while error_idx < len(error_events) and error_events[error_idx][
"timestamp"
] < event.get("timestamp", 0):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Error timestamp type breaks replay summarization

High Severity

fetch_error_details stores data["timestamp"] as-is from nodestore, then gen_request_data compares it with replay event timestamps using <. Error event timestamps are commonly ISO strings, so mixed string/number comparison can raise at runtime and fail the summarize endpoint.

Additional Locations (1)

Fix in Cursor Fix in Web

message=data.get("message", ""),
)
for event_id, data in zip(error_ids, events.values())
if data is not None

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Error details can attach to wrong IDs

Medium Severity

fetch_error_details pairs error_ids with events.values() via zip, but get_multi returns a mapping keyed by node id. This assumes value iteration order matches requested IDs, which is not guaranteed by mapping contract, so titles/messages can be associated with the wrong error_id.

Fix in Cursor Fix in Web

)

error_ids = response[0].get("error_ids", []) if response else []

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Error context skipped when fields are filtered

Medium Severity

error_ids are read from process_raw_response(...) using client-provided field filters. If the request omits error_ids in field, response[0] won’t contain it, so fetch_error_details gets an empty list and error context is silently dropped.

Fix in Cursor Fix in Web

# 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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reports accepted without any timestamp field

Low Severity

BrowserReportSerializer makes both age and timestamp optional, but there is no validation requiring at least one of them. Reports missing both fields now validate and are counted, even though they lack required temporal context.

Fix in Cursor Fix in Web

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.