| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
DateTimeOffset.UnixEpoch is not available on .NET Framework 4.7.2, so the WithAggregatedUsage copy tests failed to compile for that target framework. Use an explicit DateTimeOffset instead; the specific instant is irrelevant, the value only needs to be non-default so the copy assertion is meaningful. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
This PR fixes under-reporting of token usage for non-streaming components that internally loop and re-invoke an inner agent/chat client, by accumulating response.Usage across iterations and returning a response that carries the aggregated totals (without mutating/aliasing caller-owned UsageDetails instances).
Changes:
Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.
Show a summary per file| File | Description |
|---|---|
| dotnet/src/Shared/Usage/UsageAggregationExtensions.cs | New shared helper to merge/accumulate usage (null-aware) and return response copies carrying aggregated usage. |
| dotnet/eng/MSBuild/Shared.props | Adds InjectSharedUsage plumbing to inject the shared usage helper into selected projects. |
| dotnet/src/Microsoft.Agents.AI/Microsoft.Agents.AI.csproj | Enables InjectSharedUsage for the main .NET library. |
| dotnet/src/Microsoft.Agents.AI.Workflows/Microsoft.Agents.AI.Workflows.csproj | Enables InjectSharedUsage for Workflows. |
| dotnet/src/Microsoft.Agents.AI/Harness/Loop/LoopAgent.cs | Accumulates usage across iterations and returns aggregated usage even when returning only the last response. |
| dotnet/src/Microsoft.Agents.AI/Harness/ToolApproval/ToolApprovalAgent.cs | Accumulates usage across auto-approval reinvocations, including the iteration-cap final turn. |
| dotnet/src/Microsoft.Agents.AI/ChatClient/MessageInjectingChatClient.cs | Accumulates usage across injected-message loop iterations in the non-streaming path. |
| dotnet/src/Microsoft.Agents.AI.Workflows/MessageMerger.cs | Switches usage merging to the shared helper (fixing prior null-addition/aliasing issues). |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/Shared/UsageAggregationExtensionsTests.cs | New unit tests covering merge semantics, additional-count merging, non-mutation, and response-copy fidelity. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/Loop/LoopAgentTests.cs | Adds coverage for usage aggregation across iterations and transcript/last-response modes. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/ToolApproval/ToolApprovalAgentTests.cs | Adds coverage for usage aggregation across auto-approval loops and the iteration cap path (non-streaming + streaming). |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/MessageInjectingChatClientTests.cs | Adds coverage for usage aggregation across injected-message loops (non-streaming + streaming). |
| dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/MessageMergerTests.cs | Adds coverage ensuring merged usage and AdditionalCounts aggregate correctly. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/PerServiceCallChatHistoryPersistingChatClientTests.cs | Adds usage pass-through assertions for this single-call decorator. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ApprovalNotRequiredFunctionBypassingChatClientTests.cs | Adds usage pass-through assertions (including streaming update containing both approval + usage content). |
Sorry, something went wrong.
There was a problem hiding this comment.
Completed passes: 5 | Result: No high-severity findings
Scope: full PR (3 commit(s)): 2d23301a4b52, a20aed55286c, 0f98c350af78
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Motivation & Context
Several components re-invoke an inner agent or chat client multiple times within what the caller sees as a single run. Each inner call reports its own usage, but the looping component returned only the final call's response, so every earlier call's tokens were dropped. Callers reading response.Usage therefore under-reported the true cost of a run, and the error grew with the number of iterations.
Three non-streaming loops were affected:
Accurate token accounting matters for cost attribution, billing and budget enforcement, and the components most affected are precisely those that can iterate many times.
Streaming paths were audited and were already correct: UsageContent updates are forwarded to the caller and summed by the terminal ToAgentResponse() / ToChatResponse() conversion. That is the same contract FunctionInvokingChatClient relies on, which is why no aggregation is added there — doing so would double count.
Description & Review Guide
What are the major changes?
Adds an internal shared helper, UsageAggregationExtensions, compiled into Microsoft.Agents.AI and Microsoft.Agents.AI.Workflows via a new InjectSharedUsage property:
The three loops now accumulate usage across every iteration and return a copy carrying the total. ToolApprovalAgent's MaxAutoApprovalIterations cap branch is included: it takes an extra final turn outside the accumulating loop and previously returned early, discarding the aggregate entirely. That is the highest-spend path by definition, since the cap exists to bound a runaway loop.
Microsoft.Agents.AI.Workflows' MessageMerger is refactored onto the shared helper. This also fixes a latent bug in its local merge, which used raw long? addition — so null + 5 evaluated to null and silently zeroed a count whenever one side omitted a field — and which returned caller-owned instances by reference.
What is the impact of these changes?
response.Usage now reflects the whole run for these components rather than just the final inner call. Reported token counts will increase for multi-iteration runs; this is the correction, not a regression. There are no public API changes — the helper is internal — and no behavioural change to message content, ordering, or streaming. Usage continues to be reported only on .Usage and is never duplicated as UsageContent in messages.
What do you want reviewers to focus on?
Whether the aggregation boundaries are right: each loop should count every inner call exactly once, with no double counting where components nest, since FunctionInvokingChatClient sits directly above MessageInjectingChatClient in the ChatClientAgent pipeline.
Related Issue
Closes #7537
Contribution Checklist