Skip to content

[dotnet-port-fixes] Aggregate loop usage across iterations - #862

Closed
Michelle Clayton (michelle-clayton-work) wants to merge 1 commit into
mainfrom
dotnet-port-fixes-loop-usage-aggregation-360e094fde178154
Closed

[dotnet-port-fixes] Aggregate loop usage across iterations#862
Michelle Clayton (michelle-clayton-work) wants to merge 1 commit into
mainfrom
dotnet-port-fixes-loop-usage-aggregation-360e094fde178154

Conversation

@michelle-clayton-work

Copy link
Copy Markdown
Contributor

Summary

Align the Go loop harness with the LoopAgent portion of microsoft/agent-framework#7539 so NonStreamingReturnsLastResponseOnly still reports usage from every loop iteration. The loop middleware now accumulates usage across all iterations, removes per-iteration usage from the buffered final response, and reattaches the aggregated usage to the surfaced last response. I also added a regression test that exercises summed token counts and merged AdditionalCounts.

Ported .NET PRs

Upstream commit referenced: ec32e86646469585bf6d7be83b343c88bf606950 from microsoft/agent-framework. This PR ports only the LoopAgent behavior that maps to agent/harness/loop.

Breaking Changes

No. This corrects internal usage accounting for existing loop runs without changing exported Go API.

Tests and Examples

  • Ran go test ./agent/harness/loop
  • Added regression coverage in agent/harness/loop/loop_test.go for aggregated usage in last-response-only non-streaming runs
  • No example changes were needed for this internal behavior fix

Notes

This intentionally scopes the upstream port to the Go loop harness slice of microsoft/agent-framework#7539. The broader .NET PR also updated other looping components that do not map to this narrow Go change set.

Generated by .NET to Go Fixes and Test Porting Agent · gpt54 · 250.2 AIC · ⌖ 14.4 AIC · ⊞ 24.2K ·

Closes #847

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings August 18, 2026 21:21
@github-actions github-actions Bot added area:agent Changes files in the agent area size:large At most 300 changed lines across at most 10 files labels Aug 18, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the loop harness’s non-streaming “return last response only” behavior so token usage is accumulated across all loop iterations, while still surfacing only the final iteration’s response content. It aligns Go behavior with the referenced upstream LoopAgent change and adds a regression test to prevent usage undercounting.

Changes:

  • Accumulate UsageDetails across loop iterations when NonStreamingReturnsLastResponseOnly is enabled.
  • Strip per-iteration UsageContent from the buffered final iteration updates, then attach the aggregated usage to the surfaced final response.
  • Add a regression test asserting summed token counts and merged AdditionalCounts.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
agent/harness/loop/loop.go Aggregates usage across iterations and reattaches it to the final surfaced response update.
agent/harness/loop/loop_test.go Adds regression coverage ensuring usage totals are summed across iterations in last-response-only mode.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@github-actions

Copy link
Copy Markdown
Contributor

Parity Review — ✅ Approved

This PR ports the loop-agent usage aggregation behavior from upstream microsoft/agent-framework#7539 into the Go loop harness.

Scope: Internal implementation only — all new functions (responseUpdatesWithAggregatedUsage, stripUsageFromResponseUpdate, hasResponseUpdateMetadata, isZeroUsageDetails) are unexported. No exported Go API surface changed.

Parity finding: The fix aligns Go behavior with the .NET LoopAgent change in upstream commit ec32e86, which ensures NonStreamingReturnsLastResponseOnly reports accumulated usage across all loop iterations rather than only the final iteration's usage. The Go implementation faithfully reproduces this semantic: it accumulates UsageDetails across iterations, strips per-iteration usage from buffered updates, and reattaches the aggregate to the surfaced last response.

Label actions: public-api-change label not added (no exported API changed). parity-approved added.

Generated by Go API Consistency Review Agent for #862 · sonnet46 · 16.2 AIC · ⌖ 5.57 AIC · ⊞ 5.7K ·

@github-actions github-actions Bot added parity-approved Go API consistency review found no parity issues kind:code Changes production behavior or code kind:tests Changes tests, fixtures, or test infrastructure failed-auto-risk Automatic risk classification was inconclusive or failed risk:medium Contained production impact requiring normal review depth and removed failed-auto-risk Automatic risk classification was inconclusive or failed labels Aug 18, 2026
@qmuntal

Copy link
Copy Markdown
Member

I don't like how this PR ported the .NET implementation. Closing, will port it manually.

@michelle-clayton-work
Michelle Clayton (michelle-clayton-work) deleted the dotnet-port-fixes-loop-usage-aggregation-360e094fde178154 branch August 19, 2026 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:agent Changes files in the agent area kind:code Changes production behavior or code kind:tests Changes tests, fixtures, or test infrastructure parity-approved Go API consistency review found no parity issues risk:medium Contained production impact requiring normal review depth size:large At most 300 changed lines across at most 10 files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[dotnet-port-fixes] Aggregate loop usage across iterations

3 participants