feat(GALE): Invalidate derived data on merge#118378
Conversation
saponifi3d
left a comment
There was a problem hiding this comment.
just the bot comment and lgtm! excited to see the fun cases this uncovers 🤣
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a997f36. Configure here.
| "organizations:hard-delete-derived-data-invalidation", | ||
| group.project.organization, | ||
| ): |
There was a problem hiding this comment.
Bug: A NameError occurs if all groups to be merged are missing, as the group variable is accessed before it is assigned.
Severity: HIGH
Suggested Fix
The group variable should be defined before its use on line 213. One approach is to fetch the to_object_id's group and use it for the feature flag check when the from_object_ids list becomes empty. Alternatively, ensure the logic accessing group is only executed when group is guaranteed to be defined.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/sentry/tasks/merge.py#L212-L214
Potential issue: In the `merge_groups` task, if all group IDs in `from_object_ids` fail
to be found in the database (e.g., due to a race condition where a group is deleted
after the task is queued), the `group` variable will never be assigned. When the
`from_object_ids` list becomes empty, the code proceeds to a final `else` block where it
attempts to access `group.project.organization`. This access will raise a `NameError`
because `group` is unbound, causing the task to crash.

Experimental option for handling derived data after groups have been merged - hard delete the derived data so that when the next
GroupActionLogEntry(GALE) is processed, derived data will replay all of the GALE entries from epoch and regenerate the result to include the merged actions as well (as opposed to continuing from the current cursor and not considering the merged in group's actions).We may not go with this long term but it's not hooked up to anything yet and is the easiest option to implement.