Skip to content

feat(preprod): Sort insight diff by total potential savings#112476

Merged
mtopo27 merged 2 commits intomasterfrom
mtopo27/eme-997-sort-insight-diff
Apr 8, 2026
Merged

feat(preprod): Sort insight diff by total potential savings#112476
mtopo27 merged 2 commits intomasterfrom
mtopo27/eme-997-sort-insight-diff

Conversation

@mtopo27
Copy link
Copy Markdown
Contributor

@mtopo27 mtopo27 commented Apr 8, 2026

Summary

  • Sort insights by total_savings_change descending within each tab of the insight diff, so the biggest opportunities surface first
  • Applies per-tab, meaning "unresolved" and "resolved" tabs sort by their recalculated values
  • Affects both the build comparison page and the issue details insight diff section

Fixes EME-997

Test plan

  • Open a build comparison page with multiple insights and verify they are sorted by savings descending
  • Switch between tabs (All, New, Unresolved, Resolved) and confirm each tab is independently sorted
  • Verify the issue details insight diff section also shows sorted insights

Sort insights by total_savings_change descending within each tab
so the biggest opportunities surface first.

Fixes EME-997
@mtopo27 mtopo27 requested a review from a team as a code owner April 8, 2026 16:18
@linear-code
Copy link
Copy Markdown

linear-code bot commented Apr 8, 2026

@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Apr 8, 2026
Comment on lines +153 to +155
for (const insights of Object.values(byTab)) {
insights.sort((a, b) => b.total_savings_change - a.total_savings_change);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The sorting logic incorrectly orders the "Resolved" tab by placing the smallest savings first, because it performs a descending sort on negative total_savings_change values.
Severity: MEDIUM

Suggested Fix

Update the sorting logic to handle the "Resolved" tab as a special case. For insights in the "Resolved" tab, sort in ascending order of total_savings_change (e.g., a.total_savings_change - b.total_savings_change) to ensure the largest savings (most negative numbers) appear first.

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:
static/app/views/preprod/buildComparison/main/insightComparisonSection.tsx#L153-L155

Potential issue: The code at `insightComparisonSection.tsx:153~155` applies a uniform
descending sort based on `total_savings_change` across all insight tabs. For the
"Resolved" tab, insights have negative `total_savings_change` values to represent
savings. Sorting negative numbers in descending order (e.g., -100 is greater than -5000)
causes insights with the smallest savings to appear at the top of the list. This is
contrary to the intended behavior of showing the "biggest opportunities" first. As a
result, users will see trivial fixes prioritized over major ones in the "Resolved" tab,
which degrades the feature's usefulness.

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

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 1 potential issue.

Fix All in Cursor

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Descending sort reverses order for negative resolved values
    • Updated insight sorting to use descending absolute total_savings_change, so resolved items with larger savings now rank first.

Create PR

Or push these changes by commenting:

@cursor push 63e368251c
Preview (63e368251c)
diff --git a/static/app/views/preprod/buildComparison/main/insightComparisonSection.tsx b/static/app/views/preprod/buildComparison/main/insightComparisonSection.tsx
--- a/static/app/views/preprod/buildComparison/main/insightComparisonSection.tsx
+++ b/static/app/views/preprod/buildComparison/main/insightComparisonSection.tsx
@@ -151,7 +151,9 @@
     }
 
     for (const insights of Object.values(byTab)) {
-      insights.sort((a, b) => b.total_savings_change - a.total_savings_change);
+      insights.sort(
+        (a, b) => Math.abs(b.total_savings_change) - Math.abs(a.total_savings_change)
+      );
     }
 
     return {

This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.

Reviewed by Cursor Bugbot for commit b900add. Configure here.

Resolved insights have negative total_savings_change values, so
ascending sort puts the biggest savings first in that tab.
@mtopo27 mtopo27 merged commit a77fb6b into master Apr 8, 2026
64 checks passed
@mtopo27 mtopo27 deleted the mtopo27/eme-997-sort-insight-diff branch April 8, 2026 18:38
george-sentry pushed a commit that referenced this pull request Apr 9, 2026
## Summary
- Sort insights by `total_savings_change` descending within each tab of
the insight diff, so the biggest opportunities surface first
- Applies per-tab, meaning "unresolved" and "resolved" tabs sort by
their recalculated values
- Affects both the build comparison page and the issue details insight
diff section

Fixes EME-997

## Test plan
- [ ] Open a build comparison page with multiple insights and verify
they are sorted by savings descending
- [ ] Switch between tabs (All, New, Unresolved, Resolved) and confirm
each tab is independently sorted
- [ ] Verify the issue details insight diff section also shows sorted
insights
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants