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

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

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 ShashankFC requested a review from Copilot January 30, 2026 10:31

Copilot AI 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.

Pull request overview

This PR implements several enhancements across the codebase, including improvements to workflow engine processing, detector owner assignment, replay error context integration, and various UI/UX refinements.

Changes:

  • Enhanced workflow engine to optimize slow condition queries and improve logging
  • Added owner assignment support for detectors with validation for users and teams
  • Integrated error context into replay summaries for better debugging insights
  • Refactored multiple components to use standardized Flex layout component
  • Updated default autofix automation tuning from "low" to "off"

Reviewed changes

Copilot reviewed 106 out of 106 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/sentry/workflow_engine/processors/delayed_workflow.py Optimized slow condition query handling by pre-fetching and logging missing conditions
src/sentry/workflow_engine/endpoints/validators/base/detector.py Added owner field validation and persistence for detector creation/updates
src/sentry/replays/endpoints/project_replay_summarize_breadcrumbs.py Integrated error event details into replay breadcrumb summaries
tests/sentry/workflow_engine/processors/test_delayed_workflow.py Updated tests to pass dcg_to_slow_conditions parameter
tests/sentry/replays/test_project_replay_summarize_breadcrumbs.py Added comprehensive tests for error context integration
static/app/views/explore/* Refactored visualization handling from arrays to single yAxis field
src/sentry/projectoptions/defaults.py Changed default autofix automation tuning to "off"

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

toJSON(): BaseVisualize {
const json: BaseVisualize = {
yAxes: this.yAxes,
yAxes: [this.yAxis],

Copilot AI Jan 30, 2026

Copy link

Choose a reason for hiding this comment

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

The toJSON method converts a single yAxis back to an array format (yAxes), while the constructor accepts a single yAxis string. This mismatch creates confusion and could lead to bugs when serializing/deserializing. Consider either keeping the internal representation as an array or updating toJSON to match the constructor's expectations.

Copilot uses AI. Check for mistakes.
Comment on lines +78 to +81
static fromJSON(json: BaseVisualize): Visualize[] {
return json.yAxes.map(
yAxis => new Visualize(yAxis, {label: '', chartType: json.chartType})
);

Copilot AI Jan 30, 2026

Copy link

Choose a reason for hiding this comment

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

The fromJSON method now returns an array of Visualize objects instead of a single instance, which is a breaking change to the API. This inconsistency with the toJSON method (which returns BaseVisualize with yAxes array) could cause confusion. Consider documenting this behavior or ensuring calling code handles the array return type correctly.

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

Copilot AI Jan 30, 2026

Copy link

Choose a reason for hiding this comment

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

The code assumes that all values in events.values() are dictionaries with the expected keys. If nodestore returns data in an unexpected format or if required fields are missing, this could raise KeyError or return incorrect data. Add validation to ensure data is a dictionary before accessing keys.

Suggested change
if data is not None
if isinstance(data, dict)

Copilot uses AI. Check for mistakes.
Comment on lines +152 to +158
# 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):
error = error_events[error_idx]
yield generate_error_log_message(error)
error_idx += 1

Copilot AI Jan 30, 2026

Copy link

Choose a reason for hiding this comment

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

Using event.get("timestamp", 0) could cause incorrect chronological ordering if events legitimately have a timestamp of 0 or if the timestamp is missing. This could result in errors being inserted at the wrong position in the log sequence. Consider using a more robust default or validating that timestamp exists.

Suggested change
# 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):
error = error_events[error_idx]
yield generate_error_log_message(error)
error_idx += 1
# Check if we need to yield any error messages that occurred before this event.
# Do not use a default timestamp value, as that can mis-order events when the
# timestamp is missing.
event_ts = event.get("timestamp")
if event_ts is not None:
while error_idx < len(error_events) and error_events[error_idx]["timestamp"] < event_ts:
error = error_events[error_idx]
yield generate_error_log_message(error)
error_idx += 1

Copilot uses AI. Check for mistakes.
@@ -68,7 +67,7 @@ export function useAddToDashboard() {
query: search.formatString(),
version: 2,
dataset,

Copilot AI Jan 30, 2026

Copy link

Choose a reason for hiding this comment

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

The yAxis is converted back to an array format here for the EventView, which creates inconsistency with the new single yAxis approach used throughout the codebase. This suggests the refactoring may not be complete or that EventView requires array format. Consider documenting why this conversion is necessary or updating EventView to accept single yAxis.

Suggested change
dataset,
dataset,
// Explore uses a single yAxis value, but EventView/NewQuery still expects an array.
// Wrap the single yAxis to satisfy the EventView API without changing Explore's model.

Copilot uses AI. Check for mistakes.
feedbacks,
):
feedbacks_string = "\n".join(f"- {msg}" for msg in feedbacks)
feedbacks_string = "\n------\n".join(feedbacks)

Copilot AI Jan 30, 2026

Copy link

Choose a reason for hiding this comment

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

The separator ------ is a magic string that could be extracted as a named constant for better maintainability and to make its purpose clearer.

Copilot uses AI. Check for mistakes.
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.