[robertwb]

A tree-based shuffle with combining tables is waseful in this case.

[github-actions]

📋 Review Summary

This PR introduces combiners.ToIterable and refactors ToList and ToDict to inherit from a shared ToSingleElementCollection base class that uses AsIter (for GlobalWindows) or GroupByKey (for non-global windows) instead of CombineGlobally, avoiding unnecessary partial-combine overhead. The approach is clean and well-tested across empty, non-empty, and windowed cases.

🔍 General Feedback

• In ToSingleElementCollection._expand, checking pcoll.windowing.windowfn == window.GlobalWindows() rather than pcoll.windowing.is_default() causes GlobalWindows PCollections configured with non-default triggers (e.g., Repeatedly(AfterCount(...)) with .without_defaults()) to take the single-impulse DoOnce + AsIter path instead of GroupByKey, dropping the trigger/accumulation strategy and breaking per-pane firing.

---

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

[github-actions]

🟠 High: Checking pcoll.windowing.windowfn == window.GlobalWindows() also matches PCollections in GlobalWindows that have non-default triggers or accumulation modes (for example, WindowInto(GlobalWindows(), trigger=Repeatedly(AfterCount(N)), accumulation_mode=AccumulationMode.DISCARDING) used with .without_defaults()). Taking the DoOnce >> Create([None]) side-input branch in that case replaces the output windowing strategy with default GlobalWindows and only fires once rather than emitting one collection per trigger pane via GroupByKey (which previously worked via CombineGlobally(...).without_defaults()).

Using pcoll.windowing.is_default() ensures that GlobalWindows with custom triggers fall through to the GroupByKey path when without_defaults() is used.

Suggested change

	if pcoll.windowing.windowfn == window.GlobalWindows():
	if pcoll.windowing.is_default():

[robertwb]

Yes. Done.

[codecov]

Codecov Report

❌ Patch coverage is 96.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 57.53%. Comparing base (9ff540f) to head (1ca4ac4).
⚠️ Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
sdks/python/apache_beam/transforms/combiners.py	96.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #41054      +/-   ##
============================================
- Coverage     57.53%   57.53%   -0.01%     
  Complexity     2492     2492              
============================================
  Files          1205     1205              
  Lines        171468   171485      +17     
  Branches       2103     2103              
============================================
+ Hits          98652    98656       +4     
- Misses        70164    70177      +13     
  Partials       2652     2652              

Flag	Coverage Δ
python	80.25% <96.00%> (-0.02%)	⬇️

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: @claudevdm for label python.
Backup reviewer: tvalentyn (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).