fix(Segment membership): Zero out (segment, env) pairs that stopped matching#7600
Conversation
…atching `refresh_project_segment_counts` builds the upsert list from `compute_segment_counts_for_project`, which only returns pairs with at least one match. Combined with `bulk_create(update_conflicts=True)` -- which only touches rows it's given -- a pair that previously matched but no longer does keeps the stale non-zero count forever. Saw this in practice on staging: editing a segment rule so that a previously-matching env dropped to zero left the old count visible in the UI through the next refresh cycle. Append explicit zero-count rows for every (segment, env) pair the project already has in the table but didn't appear in this run's result. The next bulk_create then drives those down to zero. Pairs that have never matched stay absent (no row pre-population). Log the zeroed-row count alongside the existing membership_counts__count on `refresh.project.completed` so this path is observable. beep boop
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the segment membership count refresh task to explicitly zero out stale membership counts for segment-environment pairs that no longer match, preventing stale counts from lingering. It also adds corresponding unit tests and updates the observability documentation. The reviewer suggested a performance optimization to filter the existing pairs by count__gt=0 to avoid redundant database updates for rows that have already been zeroed out.
Filter `existing_pairs` by `count__gt=0` so steady-state refreshes don't re-write zeros over zeros. Per gemini-code-assist on #7600. beep boop
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7600 +/- ##
=======================================
Coverage 98.51% 98.51%
=======================================
Files 1436 1436
Lines 54363 54405 +42
=======================================
+ Hits 53553 53595 +42
Misses 810 810 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Per matthewelwell on #7600: zero-out rows just for the next refresh to read them isn't useful — we're not surfacing history anywhere — so drop them outright. The frontend already renders no chip when a membership row is absent, which is the same UX the explicit zero was reaching for. beep boop
github-actions
Bot
added
api
Docker builds report
|
Visual Regression19 screenshots compared. See report for details. |
Per matthewelwell on #7600: comment was carrying old context, and the `if stale_ids else (0, {})` guard is unnecessary since `.delete()` on an empty `id__in` filter already returns `(0, {})` without issuing SQL. beep boop
github-actions
Bot
added
docs
3 participants
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Contributes to #5663.
refresh_project_segment_countsonly upserts pairs returned by compute (count > 0), so a pair that previously matched but no longer does keeps its stale non-zero count forever. Append explicit zero rows for every(segment, env)pair already in the table that didn't appear in this run; pairs that have never matched stay absent.How did you test this code?
Added
test_refresh_project_segment_counts__previously_matching_pair_drops_to_zero__row_zeroedandtest_refresh_project_segment_counts__never_matched_pair__no_row_written.