[acrites]

In CachingStateIterable, remove() and appendHelper() detect when the cache is only partially loaded (nextToken != null) and call cache.remove(IterableCacheKey.INSTANCE). However, without returning early, execution falls through to the block combination logic which stores a MutatedBlocks with nextToken = null, overwriting the cache with truncated contents.

This PR adds the missing return; statements after cache.remove(...) so that the cache remains evicted and subsequent iterations fetch the full, correct dataset from the runner. It also updates testAppend and testRemove in StateFetchingIteratorsTest with explanatory comments ensuring the partial cache condition is properly triggered before mutations occur.

It also adds a test to BagUserStateTest.java, which demonstrates how this issue can happen at a higher level.

[github-actions]

📋 Review Summary

This PR resolves a bug in CachingStateIterable where remove() and appendHelper() evicted partially-cached state entries (nextToken != null) via cache.remove(IterableCacheKey.INSTANCE) but failed to return early, immediately overwriting the cache with a truncated MutatedBlocks entry. The fix cleanly adds the missing return; statements and corrects the ordering in StateFetchingIteratorsTest so that the partial cache state is actually populated prior to testing mutations.

🔍 General Feedback

• The root cause diagnosis and fix in StateFetchingIterators.java are accurate and minimal.
• Fixing the ordering of iterable.iterator().hasNext() before append()/remove() in StateFetchingIteratorsTest.java ensures the partial-cache eviction path is genuinely exercised by the unit tests.

---

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

[acrites]

Run Java_Examples_Dataflow PreCommit

[github-actions]

Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment assign set of reviewers

[acrites]

assign set of reviewers

I don't think the failing precommit is related.

[github-actions]

Assigning reviewers:

R: @ahmedabu98 for label java.
Backup reviewer: damccorm (if the main reviewer is unresponsive, you can escalate to them by tagging their username)

This pull request likely touches a core component ("core" label). Please review with scrutiny.

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).

[codecov]

Codecov Report

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

Additional details and impacted files

@@              Coverage Diff              @@
##             master   #40825       +/-   ##
=============================================
- Coverage     56.16%        0   -56.17%     
=============================================
  Files          1124        0     -1124     
  Lines        178336        0   -178336     
  Branches       1489        0     -1489     
=============================================
- Hits         100170        0   -100170     
+ Misses        75645        0    -75645     
+ Partials       2521        0     -2521     

Flag	Coverage Δ
go	?
java	?
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.