Replays Self-Serve Bulk Delete System

Hi there 👋

🌥️ Tech	🌤️ Feat
6 high, 27 medium, 48 low	1 mitigated

An unusually broad Merge Request: the headline requirement is the Replays Self-Serve Bulk Delete System, but the diff also bundles many unrelated changes (an autofix-automation default flip, a browser-reporting endpoint refactor, grouping-parameterization cleanup, a GitHub/GitLab PR-comment template consolidation, taskworker deadline additions across deliver_webhooks/check_auth, a workflow-engine detector refactor, placeholder migrations, replay breadcrumb-summarization, and a large Explore dashboards/insights UI migration).

Note on review completeness: one portion of this review was delivered from prose without a final submission call and is flagged partial — the browser-reporting collector findings are valid but were less fully verified than the others. Please treat that area with slightly more scrutiny.

Overall the MR is directionally correct but not ready to merge. Multiple high-severity issues were found:

• Replay bulk-delete (the core requirement): the ThreadPoolExecutor blob-deletion path swallows worker exceptions, leaving orphaned blobs with no retry; no replay bulk-delete option is registered for rollout gating; and the max_segment_id=None fix is never exercised by tests.
• Dashboard flag: the use-table-widget-visualization flag renders a hardcoded empty table, discarding real query results the moment the flag is enabled.
• Webhooks: drain_mailbox_parallel gets a 120s taskworker deadline shorter than its own 180s internal loop deadline.
• Auth sync: the new 60s deadline can interrupt a batch mid-way while last_synced was already advanced, silently dropping identities from the sync for ~24h.
• Workflow engine: the Redis buffer read-process-delete path has a TOCTOU window that can re-fire actions on retry, and update() is non-atomic with audit-after-commit.
• Replay breadcrumbs: fetch_error_details pairs event IDs to nodestore data via zip(error_ids, events.values()), which is unreliable given get_multi dedup/caching behavior.
• grouptype: should_create_group deletes the wrong Redis key so the noise-reduction counter is never reset.

Lower-severity but widespread issues include several Flex-refactor layout regressions (integrationLayout flex-basis, traceTabsAndVitals alignment, traceWaterfall height, eventAttachments ellipsis), a debounce in the trace-name search bar that never coalesces, dead code left after the ParameterizationRegexExperiment removal, audit-log gaps for owner changes and preprod analytics recording before gating, plus multiple test-coverage gaps. A large fraction of the file set is clean (config registries, migrations no-ops, parameterization core, table-widget fixtures, explore page-params parity, project test updates).

Status: REQUEST_CHANGES — several findings are blocking for the core requirement and multi-file robustness concerns.

---

These might need a close look

• 🔴 src/sentry/replays/usecases/delete.py (L75)
  pool.map(_delete_if_exists, ...) results are never consumed and ThreadPoolExecutor.__exit__ only waits; combined with _delete_if_exists catching only NotFound, transient storage/network errors are silently swallowed. A failed blob delete is dropped, the Kafka archive event is published, and the task marks the job completed — leaving orphaned blobs that are never retried. Consume the iterator (e.g. as_completed) so worker failures trigger the retry policy.
• 🔴 src/sentry/hybridcloud/tasks/deliver_webhooks.py (L239)
  drain_mailbox_parallel gets processing_deadline_duration=120, but both drain paths set their internal loop deadline to BATCH_SCHEDULE_OFFSET = 180s. The 120s taskworker deadline is shorter than the task's own 180s loop deadline, so a parallel task is killed by the taskworker before reaching its internal stop condition — the internal deadline logic is unreachable and batches can be abandoned/retried while still deliverable, and the parallel path is inconsistent with the sequential path (300s). Raise the parallel deadline to ≥180s or shorten the internal deadline.
• 🔴 src/sentry/tasks/auth/check_auth.py (L77)
  check_auth_identities now has processing_deadline_duration=60 and processes its whole chunk_size=100 batch synchronously with outbound SSO refresh network I/O per item. If the worker is interrupted mid-batch, check_auth (line 52) already advanced last_synced=now for the entire batch, so the un-verified remainder won't be selected again for ~24h (AUTH_CHECK_INTERVAL) — the sync is silently incomplete. Shrink the effective batch per task or only advance last_synced for identities actually verified.
• 🔴 src/sentry/workflow_engine/processors/delayed_workflow.py (L652)
  process_delayed_workflows reads the entire Redis buffer, fires actions, and only afterwards deletes keys — there is no atomic claim/lease on the buffer. The new Retry(times=5, delay=5) + 60s deadline means a run exceeding the soft time limit after firing but before cleanup re-reads and re-fires the same actions; two workers have the same TOCTOU window. filter_recently_fired_workflow_actions narrows but doesn't eliminate this. Claim the batch atomically (e.g. Redis Lua SMOVE/RENAME to a processing key, or a short TTL lease) before firing.
• 🔴 src/sentry/replays/endpoints/project_replay_summarize_breadcrumbs.py (L118)
  fetch_error_details pairs ids to data via zip(error_ids, events.values()), but get_multi dedups ids (dict.fromkeys) and appends cache-only ids at the end, so dict order does not reliably match input order. An ErrorEvent can be built with the wrong id/title/message/timestamp, producing incorrect error context (and wrong chronological interleaving) sent to Seer. Look each node id up explicitly: for event_id, node_id in zip(error_ids, node_ids): data = events.get(node_id); ....
• 🔴 static/app/views/dashboards/widgetCard/chart.tsx (L173)
  When use-table-widget-visualization is enabled, TableWidgetVisualization is rendered with columns={[]} and an empty tableData even though real results are in scope (result.data, result.meta, derived fields/fieldAliases). Enabling the flag renders a completely empty dashboard table — a functional regression. Either wire the real data in or don't gate the component behind an enforceable flag until it's functional.

Worth checking

• 🟡 src/sentry/replays/usecases/delete.py (L74)
  delete_matched_rows spins up a fresh ThreadPoolExecutor(max_workers=100) per row sequentially (up to limit executors serially), so only segments within a single replay are deleted concurrently. For large max_segment_id values this can blow past processing_deadline_duration=300, forcing repeated retries. Consider a single shared pool for the whole batch.
• 🟡 src/sentry/options/defaults.py (L2627)
  No replay bulk-delete option is registered anywhere (searched replay-bulk-delete — no matches). The bulk-delete feature ships fully ungated (no option/feature-flag guard). If this is meant to be safely rollout-gated, an option following the standard register(...) pattern is missing. (The only defaults.py change — removing grouping.experiments.parameterization.traceparent — is verified safe/unreferenced.)
• 🟡 src/sentry/projectoptions/defaults.py (L205)
  sentry:autofix_automation_tuning default change ("low"→"off") uses a plain default= with no epoch_defaults, so per WellKnownProjectOption.get_default it silently changes the effective value for existing projects that never set the option, not just new ones. If only the forward-looking default is intended, register with epoch_defaults (bump LATEST_EPOCH). The coordinated change to constants.py/serializers/_is_issue_fixable is otherwise coherent.
• 🟡 tests/sentry/api/endpoints/test_browser_reporting_collector.py (L68)
  Test-coverage regression for the collector endpoint: (a) the old test_logs_request_data_if_option_enabled (patching logger.info and asserting browser_report_received) was deleted — there is now no assertion that the option-enabled endpoint logs, and test_404s_by_default does not assert that no logging/metrics occur when disabled; (b) no positive test posts with application/reports+json, so the BrowserReportsJSONParser path is unexercised (only the invalid-content-type failure case is); (c) the 'body must be a list' 422 branch is untested. Restore the logger/metrics gating assertions and add media-type/array-validation tests.
• 🟡 src/sentry/issues/endpoints/browser_reporting_collector.py (L93)
  logger.info("browser_report_received", extra={"request_body": request.data}) logs the complete request body from an unauthenticated (permission_classes = ()), @csrf_exempt, open-CORS endpoint with no rate limit — a log/JSON injection and metric-inflation abuse surface. Truncate/sanitize the logged body (or log a bounded summary) and consider a rate limit.
• 🟡 src/sentry/grouping/parameterization.py (L338)
  Dead code left after removing ParameterizationRegexExperiment: _handle_regex_match (lines 338-345) is defined but never called (the only loop body calls experiment.run(...)), and ParameterizationRegex.compiled_pattern (lines 41-48) is now unused (only consumer was the removed experiment). Remove both and their now-dangling re/re.Pattern usage — keeping them misleadingly suggests regex experiments are still supported.
• 🟡 src/sentry/grouping/parameterization.py (L158)
  The duration pattern \b(\d+ms) | (\d+(\.\d+)?s)\b is compiled with (?x) which ignores the unescaped whitespace, yielding \b(\d+ms)|(\d+(\.\d+)?s)\b — the leading \b applies only to the ms branch and the trailing \b only to the s branch. "1234msfoo" over-matches, and the semantics are inconsistent with every other pattern. Use a single group, e.g. \b(?:\d+ms|\d+(?:\.\d+)?s)\b, with positive/negative tests.
• 🟡 src/sentry/hybridcloud/tasks/deliver_webhooks.py (L85)
  schedule_webhook_delivery loops over up to BATCH_SIZE=1000 records, each doing a replica read plus a primary UPDATE of up to MAX_MAILBOX_DRAIN=300 rows. With processing_deadline_duration=30 the tick can be killed under load; also scheduled_mailboxes.count() on line 134 evaluates the full annotated queryset before the [:BATCH_SIZE] slice. Confirm 30s suffices for the worst case or raise it (siblings use 60-120s+).
• 🟡 src/sentry/tasks/auth/check_auth.py (L149)
  The except Exception branch sets is_valid = False with the comment 'to ensure security we count any kind of error as an invalidation event', but the flag write only runs when sso:linked != is_linked; since is_linked=True here, the common already-linked case skips the write and sso:invalid is never set. The security rationale in the comment is not honored. Drive the flag update off sso:invalid/is_valid too.
• 🟡 src/sentry/workflow_engine/endpoints/validators/base/detector.py (L88)
  update() is not atomic and audits after commit: instance.save() (line 88) commits first, then create_audit_entry(... DETECTOR_EDIT ...) (line 90) runs outside any transaction. If audit creation fails, the detector is silently updated with no audit trail. Wrap the save + audit in the same transaction.atomic(router.db_for_write(Detector)) block used by create().
• 🟡 src/sentry/workflow_engine/endpoints/validators/base/detector.py (L90)
  Owner changes are persisted but the audit payload is data=instance.get_audit_log_data() which returns only {"name": ...}. An admin cannot tell from the audit log that ownership changed or who owned it before/after. Extend get_audit_log_data() (and/or the audit event) to include owner_user_id/owner_team_id.
• 🟡 src/sentry/workflow_engine/processors/delayed_workflow.py (L672)
  In both get_condition_query_groups (line 672) and get_groups_to_fire (line 419), workflow_env = workflows_to_envs[workflow_id] if workflow_id else None can raise KeyError when a workflow referenced by a buffered event was deleted (since fetch_workflows_envs only returns existing workflows), aborting the whole batch. Use workflows_to_envs.get(workflow_id) and skip/log when absent.
• 🟡 tests/sentry/replays/test_project_replay_summarize_breadcrumbs.py (L58)
  All tests @patch(... "make_seer_request"), so the signed request is never exercised. Add a test with a mocked requests.post asserting the URL (SEER_AUTOFIX_URL/v1/automation/summarize/replay/breadcrumbs), the content-type header, and that the Rpcsignature is computed over the exact request body (and only present when the shared secret applies).
• 🟡 src/sentry/replays/endpoints/project_replay_summarize_breadcrumbs.py (L100)
  on_results=functools.partial(analyze_recording_segments, error_events) fires per pagination page and merges the full error_events list into every page's logs, repeating all error messages across pages and re-summarizing the same errors N times. Disable pagination here or distribute errors only to the page containing their timestamped segment.
• 🟡 static/app/views/replays/detail/ai/index.tsx (L86)
  The 'Project not found' guard runs before the isPending branch and fires whenever project is undefined — but useProjectFromId returns undefined both when the project doesn't exist and while the async ProjectsStore is still loading (e.g. deep-link). The AI tab can flash a hard error then swap to the spinner. Gate on the project-store being loaded (initiallyLoaded/fetching).
• 🟡 src/sentry/issues/grouptype.py (L696)
  Pre-existing (adjacent to the added noise_config): should_create_group increments key = f"grouphash:{grouphash}:{project.id}" but on threshold-cross deletes client.delete(grouphash) — the raw fingerprint string, not the incremented key — so the noise-reduction counter is never reset. Fix to client.delete(key). The newly-added NoiseConfig(ignore_limit=5) itself is correct and consistent with the registry.
• 🟡 src/sentry/preprod/api/endpoints/organization_preprod_artifact_assemble.py (L81)
  analytics.record("preprod_artifact.api.assemble", ...) is the first statement in post(), before the feature-flag gate (404) and schema validation (400), so every rejected/not-found request also emits an analytics event — any authenticated user with ProjectReleasePermission can inflate metrics. Move the record after the gates, or document that all attempts are intentionally recorded.
• 🟡 static/app/views/explore/hooks/useGetTraceItemAttributeValues.tsx (L88)
  The no-values-for-numbers short-circuit guards on the hook's type prop, but both production callers set type: 'string'; numeric attributes (tag kind MEASUREMENT) reach this function with the string-typed hook and fall through to issue a request with attributeType: 'string' — the opposite of the intended optimization. The spec only passes because it constructs the hook with type: 'number', which matches no real caller. Gate on the tag's kind (e.g. tag.kind === FieldKind.MEASUREMENT) and update the spec accordingly.
• 🟡 static/app/views/codecov/tests/onboardingSteps/addUploadToken.tsx (L36)
  handleGenerateClick only calls setShowTokenDetails(true); on the truncated view showTokenDetails is already true, so 'Regenerate' (line 92) is a no-op — showFullToken stays false and the full token can never be shown again after 'Done' without a reload. Reset setShowFullToken(true) (and ideally generate a distinct token value rather than reusing FULL_TOKEN).
• 🟡 static/app/views/codecov/tests/onboardingSteps/addUploadToken.tsx (L73)
  The alert says 'Please copy this token to a safe place - it won't be shown again.' but there is no copy button or clipboard action anywhere in the component — the copy step implied by the task flow is missing. Add a copy action beside the full-token CodeSnippet, or flag with a TODO if this is still a hardcoded sample.
• 🟡 static/app/views/performance/newTraceDetails/traceDrawer/details/span/eapSections/attributes.tsx (L65)
  HIDDEN_ATTRIBUTES (['is_segment', 'project_id', 'received']) is only applied inside the non-empty-search branch; when there's no query the early return sorted; bypasses the filter, so these internal fields still render by default — exactly what the constant was introduced to prevent. Make the exclusion unconditional and independent of searchQuery.
• 🟡 static/app/views/performance/newTraceDetails/traceTabsAndVitals.tsx (L93)
  The Flex refactor dropped align-items: center and gap: ${space(1)} from the tabs/vitals container. The Flex component defaults to align-items: stretch, so Tabs and TraceContextVitals now stretch vertically instead of being centered and spacing is no longer guaranteed. Add align="center" and gap={space(1)} to preserve the layout.
• 🟡 static/app/views/performance/newTraceDetails/traceWaterfall.tsx (L712)
  The Flex refactor dropped height: 100% (and changed flex-basis: auto → flex-basis: 0%) on the waterfall wrapper. flex: 1 should still grow in the parent flex column, but the flex-basis change is a subtle sizing behavior change for the TraceGrid inside. Visually verify the waterfall (and issuesTraceWaterfall.tsx variant) still fills the available height; add height: '100%' if there's any scroll/fill regression.
• 🟡 static/app/components/events/eventAttachments.tsx (L143)
  The Flex refactor dropped overflow: hidden from the attachment name-cell wrapper. Old FlexCenter grid cell carried overflow: hidden, letting the 1fr first column shrink so Name's nowrap + text-overflow: ellipsis truncates. The new <Flex align="center"> has default overflow: visible, reverting to min-content width of the full nowrap filename — long attachment names will stop ellipsizing. Constrain the cell (min-width: 0/overflow: hidden).
• 🟡 static/app/views/settings/organizationIntegrations/detailedView/integrationLayout.tsx (L247)
  flex: 1 (shorthand → flex: 1 1 0%) was replaced by <Flex.Item grow={1}>, which per the FlexItem implementation only sets flex-grow: 1 while keeping flex-basis: auto. The Description/FeatureList column's base size is now derived from content rather than remaining space, which can overflow or shift the Metadata column for wide content. Use <Flex.Item flex={1}>/basis={0} to preserve flex-basis: 0%, or confirm auto is acceptable.
• 🟡 static/app/views/insights/pages/transactionNameSearchBar.tsx (L130)
  const getSuggestedTransactions = debounce(async query => {...}, DEFAULT_DEBOUNCE_DURATION, {leading: true}) is recreated on every render (not wrapped in useCallback/useMemo). Each keystroke calls setSearchString (re-render) then the debounced fn, so the debounce timer resets every keystroke and with leading: true fires one network request per keystroke ≥3 chars. Memoize the debounced function.
• 🟡 static/app/views/insights/pages/transactionNameSearchBar.tsx (L152)
  With requests firing on every keystroke (no cancellation/ordering guard), an older query's response can resolve after a newer one and setSearchResults renders stale suggestions (race condition). Also catch (_) { throw new Error(...) } re-throws inside the async debounced fn invoked without .catch() in handleSearchChange, producing an unhandled promise rejection on any fetch failure with no user-facing error state. Track a request sequence/AbortController and surface a graceful error instead of throwing.

Small things (take or leave)

• 🔵 src/sentry/replays/usecases/delete.py (L170)
  has_more = len(rows) == limit reports True when total matches are an exact multiple of the limit, scheduling one extra chained run that fetches zero rows. Not incorrect (offset advances monotonically), but it adds a wasted Snuba round-trip and an inaccurate 'has more' signal.
• 🔵 src/sentry/replays/usecases/delete.py (L91)
  Comment typo: # deleting the segements. should read segments.
• 🔵 tests/sentry/replays/tasks/test_delete_replays_bulk.py (L97)
  test_run_bulk_replay_delete_job_completion passes max_segment_id: None but mocks delete_matched_rows, so the new None guard in _make_recording_filenames is never invoked — the core fix has no direct test coverage. Add a direct unit test of delete_matched_rows/_make_recording_filenames with both None and an integer value.
• 🔵 devservices/config.yml (L26)
  The new snuba-metrics dependency uses mode: containerized-metrics-dev, an odd naming divergence from the existing containerized/containerized-profiles modes. Confirm against the snuba repo that this is a valid mode the snuba devservices tooling accepts, otherwise the tracing mode will fail to bring up the metrics stack.
• 🔵 src/sentry/issues/endpoints/browser_reporting_collector.py (L55)
  validate_timestamp/validate_age check the other field via truthiness (if self.initial_data.get("age")), so a 0 value for the other field is not detected and a report carrying both fields passes validation. Use "age" in self.initial_data / "timestamp" in self.initial_data and add a 0-valued edge-case test.
• 🔵 src/sentry/grouping/parameterization.py (L282)
  matches_counter — the only signal consumed by normalize_message_for_grouping for the grouping.value_trimmed_from_message metrics and get_successful_experiments analytics — has no test assertions for the regex path (correct keys/counts) nor for the experiment-gating negative case. The deletion of test_parameterize_regex_experiment* further reduces coverage of the counting path.
• 🔵 src/sentry/integrations/github/integration.py (L394)
  After routing both providers through the shared get_merged_pr_single_issue_template, GitHubPRCommentWorkflow.format_comment_subtitle (and its GitLab twin) is now completely unused (zero call sites, repo-wide). Remove both dead provider-specific helpers; the str | None handling distinction between them is now untestable cruft.
• 🔵 tests/sentry/integrations/gitlab/tasks/test_pr_comment.py (L393)
  The GitHub suite asserts the debounce-cache-release/error-metric contracts for missing-org/project/repo/integration paths, but the GitLab suite covers only api_error, rate_limited_error, and no_issues. Since the missing-entity branches live in the shared pr_comment_workflow task, mirror at least the release-the-cache assertions in the GitLab suite so shared-handler regressions can't slip through provider-specific coverage.
• 🔵 src/sentry/integrations/source_code_management/commit_context.py (L143)
  MERGED_PR_SINGLE_ISSUE_TEMPLATE ends with a hard \n and both providers join with "\n", producing \n\n between issues. Intentional per the tests, but redundant and couples the template to the join strategy. Also _truncate_title (line 578) has no direct unit test despite new truncation behavior.
• 🔵 src/sentry/hybridcloud/tasks/deliver_webhooks.py (L205)
  In the sequential drain_mailbox, deliver_message(record) returns without raising when payload.attempts >= MAX_ATTEMPTS, yet the loop unconditionally runs delivered += 1 — discarded webhooks are tallied as delivered (and 4xx/ApiConflictError drops are also counted as 'ok'). The parallel path records attempts_exceed without incrementing delivered. Use a separate discards/drops counter so delivered reflects true deliveries.
• 🔵 src/sentry/tasks/auth/check_auth.py (L99)
  check_auth_identities unconditionally enqueues another check_auth, creating a drain-driven check_auth → check_auth_identities → check_auth chain with no delay/backoff; while stale identities remain (provider failures or backlogs) the two tasks ping-pong as fast as scheduling allows — effectively a busy loop. Add a small delay/countdown or rely on the scheduler, and document the intended concurrency.
• 🔵 src/sentry/workflow_engine/processors/delayed_workflow.py (L652)
  The new no_slow_condition_groups logging branch (lines 654-663) is not exercised by any test, and there is no end-to-end test of process_delayed_workflows covering buffer-fetch → Snuba aggregation → slow-condition evaluation → firing → cleanup, nor of the early-return-on-empty-condition_groups path that leaves the buffer uncleaned. Add a pipeline-state test asserting buffer keys are deleted only on successful processing.
• 🔵 tests/sentry/workflow_engine/endpoints/test_organization_detector_index.py (L43)
  The detector index endpoint returns results via self.paginate(...), yet the test file has no pagination test (per-page limits, cursor/offset, Link headers, empty next-page). Filters and sorting are covered; pagination is not.
• 🔵 src/sentry/migrations/0917_convert_org_saved_searches_to_views.py (L42)
  Both 0917 and 0920 no-op placeholder migrations retain hints={"tables": ["sentry_groupsearchview", "sentry_savedsearch"]} even though the RunPython functions are pure no-ops that never touch those tables. Harmless but misleading; consider removing the hints (same applies to 0920 at line 42).
• 🔵 src/sentry/feedback/usecases/feedback_summaries.py (L52)
  make_input_prompt instructs the model 'AT MOST 55 words' / 'AT MOST two sentences', but parse_response only collapses whitespace — nothing validates or enforces these limits server-side. An over-length summary is cached and returned verbatim to the UI with no backstop. Validate/truncate, or at minimum emit a feedback.summaries.too_long metric.
• 🔵 src/sentry/feedback/usecases/feedback_summaries.py (L44)
  In generate_summary, the guard is if response: (truthy), but parse_response can normalize a whitespace-only response to "", which is then returned, cached, and served as summary: "" to the UI. Reject/normalize empty results so the endpoint doesn't persist an empty summary.
• 🔵 static/app/components/feedback/list/useFeedbackSummary.tsx (L40)
  enabled: Boolean(normalizedDateRange) && organization.features.includes(...): normalizeDateTimeParams(selection.datetime) returns an object so Boolean(...) is always true — the date-range guard is a no-op. Remove it or test for a populated range (e.g. normalizedDateRange.start || normalizedDateRange.end || normalizedDateRange.statsPeriod).
• 🔵 static/app/components/feedback/list/useFeedbackSummary.tsx (L63)
  The final return {summary: data.summary, ...} dereferences data without a guard. useApiQuery can return data === undefined in fall-through states (e.g. disabled + previously-cached data), which would throw inside the hook. Guard with data?.summary ?? null (and data?.numFeedbacksUsed ?? 0 for tooFewFeedbacks).
• 🔵 tests/sentry/replays/test_project_replay_summarize_breadcrumbs.py (L139)
  Each test inserts a single error event, so the fetch_error_details path with multiple errors — and critically the missing-nodestore-node case — is unverified (such a test would have surfaced the zip bug above). Add a unit test of fetch_error_details (multi events + one missing) and an endpoint test where a referenced error's node is absent.
• 🔵 src/sentry/replays/endpoints/project_replay_summarize_breadcrumbs.py (L84)
  process_raw_response(snuba_response, fields=request.query_params.getlist("field")) strips columns to the requested fields; a caller passing e.g. ?field=id drops error_ids and silently disables error context even when enable_error_context is true. Pass fields=[] or validate error_ids is always included, and document the interaction.
• 🔵 src/sentry/replays/endpoints/project_replay_summarize_breadcrumbs.py (L136)
  error_events.sort(...) mutates the list captured by the functools.partial closure and shared across pagination pages. Idempotent today, but sort a copy (sorted(error_events, key=...)) so the function doesn't mutate its input.
• 🔵 src/sentry/replays/endpoints/project_replay_summarize_breadcrumbs.py (L152)
  gen_request_data uses strict error_events[error_idx]["timestamp"] < event.get("timestamp", 0); same-second errors (common with coarse recording-segment timestamps) are ordered after the equal-timestamp event. Consider <= for more faithful chronological interleaving.
• 🔵 src/sentry/replays/endpoints/project_replay_summarize_breadcrumbs.py (L53)
  Minor cleanups: the get docstring ('Return a collection of replay recording segments.') is now inaccurate given error fetching; ErrorEvent["category"] is always the literal "error" and never read; and fetch_error_details silently swallows nodestore failures (beyond SDK capture) — consider an error-level log.
• 🔵 static/app/utils/analytics/replayAnalyticsEvents.tsx (L116)
  The new replay.timeline.zoom-in/zoom-out event schemas are loose Record<string, unknown> and the tracking calls only pass {organization} — no distinguishing payload (scale/duration delta). Capture the resulting timelineScale/maxScale and type the events concretely, consistent with sibling replay events.
• 🔵 static/app/components/replays/timeAndScrubberGrid.tsx (L35)
  handleZoomOut/handleZoomIn call trackAnalytics unconditionally after computing newScale; though buttons are disabled at boundaries (timelineScale === 1 / === maxScale), tracking fires even if the new value is clamped to the current value (e.g. if maxScale shrinks between renders). Track only when the scale actually changes.
• 🔵 static/app/views/replays/detail/ai/index.tsx (L124)
  chapterData computes end: period_end * 1000 per chapter but the render loop only consumes {title, start, breadcrumbs} — the end field is dead. Either drop it or use it to bound breadcrumb filtering.
• 🔵 src/sentry/preprod/analytics.py (L5)
  The event type is defined once on the class and re-specified as a raw string literal at the call site — the two can drift. Prefer analytics.record(PreprodArtifactApiAssembleEvent.type, ...) for a single source of truth.
• 🔵 src/sentry/preprod/__init__.py (L1)
  from .analytics import * re-exports the event class but also binds the name analytics (from from sentry import analytics inside analytics.py) into the sentry.preprod namespace, pointing at sentry.analytics rather than the sentry.preprod.analytics submodule — a latent source of confusion. Define __all__ in analytics.py or use an explicit import.
• 🔵 src/sentry/preprod/api/endpoints/organization_preprod_artifact_assemble.py (L102)
  find_missing_chunks(...) runs before get_assemble_status(...), diverging from organization_release_assemble.py which checks cached status first. If an assembly completed (state OK cached) but blobs were pruned, a re-poll returns NOT_FOUND instead of the cached completed state. Confirm intentional, else swap the checks.
• 🔵 src/sentry/preprod/api/endpoints/organization_preprod_artifact_assemble.py (L48)
  When a chunks item fails ^[0-9a-f]{40}$, e.path[0] is "chunks" so the message reads 'The chunks field is required and must be provided as an array of 40-character hexadecimal strings' — misleading for a single malformed item. Use e.message for item-level errors or distinguish required-field from item-pattern errors.
• 🔵 src/sentry/preprod/api/endpoints/organization_preprod_artifact_assemble.py (L133)
  update_org_auth_token_last_used(...) is only reached on the happy path after apply_async; NOT_FOUND/poll/validation short-circuits don't refresh the token. Verify the intended semantics — if any successful authenticated request should count, update before the early returns.
• 🔵 static/app/views/explore/contexts/pageParamsContext/visualizes.tsx (L187)
  determineDefaultChartType uses counts[ct] >= counts[acc] iterating AREA→BAR→LINE, so any tie resolves to LINE (the last element). The constructor only passes a single-element array today so it never triggers, but the latent inconsistency would surprise future multi-yAxis reuse. Add a deliberate tie-break rule.
• 🔵 static/app/views/explore/contexts/pageParamsContext/visualizes.tsx (L49)
  this.stack = 'all' unconditionally: previously multi-series bar charts were unstacked. Now that each Visualize is single-yAxis this is internally consistent, but multiple bar overlays rendered together will stack on top of one another — a visible rendering change. Confirm the intended UX; if not desired, use stack: undefined for bar overlays.
• 🔵 static/app/views/explore/contexts/pageParamsContext/visualizes.tsx (L78)
  Visualize.fromJSON(json: BaseVisualize): Visualize[] returns an array despite a singular name; callers must .map() over the result. Correct and necessary for the legacy multi-yAxis migration, but rename (e.g. fromJSONArray/expand) or document the return type to avoid the pre-change single-Visualize footgun.
• 🔵 static/app/views/explore/hooks/useExploreAggregatesTable.tsx (L23)
  UseExploreAggregatesTableOptions exposes queryExtras but useExploreAggregatesTable only destructures {enabled, limit, query} — a caller-supplied queryExtras is silently discarded (the inner hook does consume it, and the timeseries sibling doesn't accept it). Thread it through or remove it from the interface.
• 🔵 static/app/views/explore/tables/aggregateColumnEditorModal.tsx (L63)
  groupBys is memoized from the columns prop, not the live tempColumns state, so a group-by added during the current edit session won't be excluded from the other rows' drop-down lists until the modal closes. Derive it from tempColumns.
• 🔵 static/app/views/explore/tables/aggregateColumnEditorModal.tsx (L66)
  onColumnsChange(tempColumns.map(col => (isVisualize(col) ? col.toJSON() : col))) — isVisualize requires a string yAxis, so a raw BaseVisualize ({yAxes:[...]}) flowing into the modal would be forwarded verbatim with no chartType. The guard's dual role (type-narrowing + serialization decision) is fragile; serialize on isBaseVisualize/instanceof Visualize (low severity, worth documenting).
• 🔵 static/app/views/explore/toolbar/toolbarVisualize.tsx (L47)
  The visible A/B/C chart labels exist solely in the URL-serialized aggregateField ordering and are discarded whenever a Visualize is serialized with toJSON() — labels are never persisted and only rematerialized via the location round-trip. Fine today (matches the spec), but a saved-query/compare feature needing stable chart identity independent of position would hit a trap. Keep it a conscious design decision.
• 🔵 static/app/views/dashboards/widgets/tableWidget/fixtures/sampleHTTPRequestTableData.ts (L8)
  Each data row includes an id: '' key not present in meta.fields/meta.units; since columnOrder derives from Object.keys(meta.fields), the id is never rendered and is misleading fixture data. Remove it or declare it in meta.
• 🔵 static/app/views/dashboards/widgets/tableWidget/defaultTableCellRenderers.tsx (L76)
  if (!tableData?.meta) { return dataRow[column.key]; } is dead code — TableWidgetVisualization requires tableData and always passes it — and returning a raw possibly-non-React-node value is a footgun if a future caller omits tableData. Tighten the prop to be required or remove the fallback. (Related: Object.keys(tableData?.meta.fields) optional-chains on a required prop in tableWidgetVisualization.tsx.)
• 🔵 static/app/views/dashboards/widgets/tableWidget/defaultTableCellRenderers.tsx (L35)
  column.type as ColumnValueType (head) and tableData.meta.units?.[columnKey] as string (body) are unchecked casts across differing types (AttributeValueType | undefined vs ColumnValueType; DataUnit | null vs string). They work only because downstream consumers tolerate the values; prefer narrow type guards (column.type ?? undefined, map DataUnit | null to string | undefined).
• 🔵 static/app/views/performance/newTraceDetails/traceTabsAndVitals.tsx (L46)
  The ResizeObserver callback calls setContainerWidth(clientWidth) with no guard; ResizeObserver can fire for height/count reasons, causing needless re-renders. Use a functional guard: setContainerWidth(prev => prev === width ? prev : width).
• 🔵 static/app/components/group/times.tsx (L37)
  The hidden-xs hidden-sm responsive class moved inside the TextOverflow wrapper rather than on the flex item; on small screens the TimeSince is display:none but the wrapper remains as an (empty, ~0-width) item. Semantics are off — move the class to the wrapper or conditionally omit it.
• 🔵 static/app/views/insights/pages/transactionNameSearchBar.tsx (L174)
  const fullQuery = asRawText ? query : `transaction:"${query}"`; interpolates the user-authored transaction name into the search token without escaping; a name containing " or other search syntax breaks the constructed MutableSearch. Escape/quote the value before embedding.
• 🔵 static/app/views/insights/pages/transactionNameSearchBar.tsx (L92)
  With nothing highlighted (highlightedItemIndex === -1) and ArrowUp pressed, (highlightedItemIndex + transactionCount - 1) % transactionCount yields count - 2, skipping the last suggestion. ArrowUp from an empty selection should wrap to the final item (count - 1).
• 🔵 tests/snuba/api/endpoints/test_organization_events_stats.py (L3184)
  assert [{"count": count}] in data[message]["data"][0] only inspects the first interval bucket, so it would silently pass even if the second bucket were not zero-filled (passes only because all logs land in the first hour). Follow the spans sibling pattern ([attrs for _, attrs in data[message]["data"]]) to also verify zero-fill.
• 🔵 tests/snuba/api/endpoints/test_organization_events_stats.py (L3189)
  assert [{"count": 10}] in other["data"][0] hardcodes the Other bucket count; it only holds because exactly one group falls outside the top 5. Derive it from the data (e.g. sum(... expected_message_counts[5:])) so fixture changes don't silently break it.
• 🔵 tests/snuba/api/endpoints/test_organization_events_stats.py (L3150)
  The new OrganizationEventsStatsTopNEventsLogs only exercises yAxis="count()" top-events on the ourlogs dataset — no meta assertion, no second-bucket zero-fill check, no other yAxis function, unlike the spans class. Acceptable as a sanity check, but a meta assertion and zero-fill check would bring it in line with equivalent coverage.

---

✅ Feature-level checklist

Partially covered

• ⚠️ Replays Self-Serve Bulk Delete System — implement a self-serve bulk delete flow for replays (delete replay recordings/blobs, checkpoint offsets so retries resume, and gate/configure the job).
  The delete path is implemented and directionally correct (idempotent deletes via _delete_if_exists silencing 404s, monotonic offset checkpointing, safe re-processing). However it is not fully compliant: the thread-pool blob deletion silently swallows worker exceptions leaving orphaned blobs with no retry (src/sentry/replays/usecases/delete.py:75), no replay bulk-delete option is registered in defaults.py for rollout gating (src/sentry/options/defaults.py:2627), and the max_segment_id=None fix is never exercised by tests (tests/sentry/replays/tasks/test_delete_replays_bulk.py:97). The per-row sequential thread pools also risk deadline overruns (delete.py:74), and has_more has an off-by-one (delete.py:170).