[damccorm]

Note: this PR was auto-generated by tooling. Before moving it out of draft, the author has done a full review of the PR, specifically evaluating it on the following criteria:

• The PR is an appropriate fix for the issue (e.g. we're not changing a test to avoid a bug)
• The PR is a simple fix to the issue (e.g. we're not adding infra when we could just modify a test)
• The PR is not covering for other problems (e.g. if we're bumping timeouts, it is not because we're covering for a performance problem)

Full details of the methodology of generation and review can be found here - https://docs.google.com/document/d/14lj7ECVov4T4EdwmIzNlWrRGE3lLDMOwX5xUMGzaLoc/edit?usp=sharing

If you have concerns with the PR or with the underlying methodology used to generate this, please comment below - they will be addressed by the author not the tooling :)

Summary

• Fixes a flake in :sdks:go:test:prismValidatesRunner (TestTriggerAfterSynchronizedProcessingTime failing with expected 2 collections, but got: 3 in https://github.com/apache/beam/actions/runs/43083145592 and https://github.com/apache/beam/actions/runs/43099848241).
• In Prism's single-node architecture (strategy.go), TriggerAfterSynchronizedProcessingTime fires immediately upon receiving data (shouldFire returns !ts.finished) and marks the trigger finished.
• Previously, TriggerAfterSynchronizedProcessingTime in sdks/go/test/integration/primitives/windowinto.go wrapped trigger.AfterSynchronizedProcessingTime() in trigger.Repeat(...) (unlike the other leaf trigger tests in windowinto.go), which reset ts.finished = false after each firing and caused the GBK to fire once per upstream bundle. Whenever stage.Execute channel-split the initial 3-element TestStream bundle (1.0, 2.0, 3.0) into two pre-combined bundles in Docker mode, Repeat(AfterSynchronizedProcessingTime()) fired twice for window [0, 10000) and once for window [10000, 20000) (3 panes instead of 2).
• Test trigger.AfterSynchronizedProcessingTime() directly without trigger.Repeat(...) so it fires once per window and deterministically emits 2 panes.

Test Plan

• go test ./pkg/beam/runners/prism/...
• go test -v ./test/integration/primitives -run "TestTrigger.*"

Verification

• Triggered beam_PreCommit_GoPrism.yml workflow run on branch: 43369539492

[damccorm]

/gemini review

[github-actions]

📋 Review Summary

This PR fixes a flake in TestTriggerAfterSynchronizedProcessingTime when running on Prism in Docker mode. By tracking afterProcessingTimeState in TriggerAfterSynchronizedProcessingTime, the trigger now waits for processing time to advance past the initial TestStream clock time (and past previous firing times on reset) instead of firing unconditionally on every split bundle.

🔍 General Feedback

• The implementation cleanly reuses afterProcessingTimeState and aligns well with TriggerAfterProcessingTime's state lifecycle (onElement, shouldFire, onFire, reset).
• Unit tests in strategy_test.go cover both single and repeated TriggerAfterSynchronizedProcessingTime scenarios.

---

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

[codecov]

Codecov Report

❌ Patch coverage is 90.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.77%. Comparing base (73b1202) to head (f83ffeb).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
...pkg/beam/runners/prism/internal/engine/strategy.go	90.00%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #41228      +/-   ##
============================================
+ Coverage     57.76%   57.77%   +0.01%     
  Complexity     2617     2617              
============================================
  Files          1202     1202              
  Lines        171675   171702      +27     
  Branches       2163     2163              
============================================
+ Hits          99162    99198      +36     
+ Misses        69851    69838      -13     
- Partials       2662     2666       +4     

Flag	Coverage Δ
go	29.25% <90.00%> (+0.04%)	⬆️
python	80.55% <ø> (-0.01%)	⬇️

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.

[damccorm]

/gemini review

[github-actions]

📋 Review Summary

This PR resolves a flake in TestTriggerAfterSynchronizedProcessingTime on the Prism runner by testing trigger.AfterSynchronizedProcessingTime() directly without wrapping it in trigger.Repeat(...). Removing the Repeat wrapper ensures the trigger finishes after firing once per window regardless of how upstream TestStream bundles are split across channels.

🔍 General Feedback

• The change aligns TriggerAfterSynchronizedProcessingTime with the other leaf trigger integration tests in sdks/go/test/integration/primitives/windowinto.go (such as TriggerElementCount, TriggerAfterProcessingTime, and TriggerNever) and makes the expected 2-pane count deterministic across bundle boundaries.

---

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