[acrites]

Before this PR, we add an id to the IdTracker and then delete it even if the delete operation on the ordered list state happened before the add. Added a test to demonstrate this error (and the fix).

[github-actions]

📋 Review Summary

This PR fixes a bug in WindmillOrderedList.persistDirectly where pendingAdds were committed to IdTracker before pendingDeletes, causing range deletions in the same work item to overwrite sub-range deletion tracking or wipe out ID tracking for elements added after clearRange. Processing pendingDeletes before pendingAdds matches the logical order of operations since clearRange already evicts prior in-memory pendingAdds in the deleted range.

🔍 General Feedback

• The fix in WindmillOrderedList is clean and accurately preserves IdTracker state across overlapping clearRange and add operations within the same work item.
• The new unit test testOrderedListDeleteRangeThenAddPersist thoroughly covers both partial sub-range deletions followed by adds and full 5-minute bucket clears followed by adds.

---

💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.83%. Comparing base (76f6e6d) to head (ea431d5).
⚠️ Report is 232 commits behind head on master.

Additional details and impacted files

@@              Coverage Diff              @@
##             master   #40659       +/-   ##
=============================================
+ Coverage     56.16%   75.83%   +19.66%     
  Complexity     2288     2288               
=============================================
  Files          1124      259      -865     
  Lines        178336    14459   -163877     
  Branches       1489     1487        -2     
=============================================
- Hits         100170    10965    -89205     
+ Misses        75645     2792    -72853     
+ Partials       2521      702     -1819     

Flag	Coverage Δ
go	?
java	75.83% <ø> (+0.04%)	⬆️
python	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

Assigning reviewers:

R: @shunping added as fallback since no labels match configuration
Backup reviewer: Abacn (if the main reviewer is unresponsive, you can escalate to them by tagging their username)

Note: If you would like to opt out of this review, comment assign to next reviewer.

Available commands:

• stop reviewer notifications - opt out of the automated review tooling
• remind me after tests pass - tag the comment author after tests pass
• waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

[shunping]

Lgtm, thanks!

[shunping]

I agree that we need to apply pendingDeletes before pendingAdds. When clearRange() is called, it already clears any existing pendingAdds that fall within the deleted range (see the logic in clearRange()).

Therefore, if there is a remaining pendingAdd that overlaps with a pendingDelete range, it means the add operation occurred after the clear operation. Applying the deletes first ensures that these subsequent additions are correctly preserved.