Byte-account hosted tool-call/result content in compaction sizing - #1008
Conversation
computeContentByteCount (used by group ByteCount and, via /4, by the token estimate for non-text content) counted FunctionResultContent and DataContent payloads, but MCPServerToolResult/CodeInterpreterToolResult (and the call variants) fell through to default: return 0. A 10k-byte hosted tool result therefore contributed ~2 bytes, so byte/token-based triggers (TokensExceed, ContextWindowStrategy) under-counted history dominated by hosted code- interpreter / MCP outputs - exactly the large content that most needs compaction - and could fail to fire. Account these types by recursing into Outputs/Inputs and counting the identity strings, mirroring the function-call/result cases.
github-actions
Bot
added
area:agent
github-actions
Bot
added
the
pending-auto-risk
There was a problem hiding this comment.
🟡 Changes recommended
Tool-call grouping logic still doesn’t recognize the new hosted tool-call content types, and the new byte-count branches for hosted tool calls are untested.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR fixes under-counting in compaction sizing by ensuring hosted tool call/result content (MCP server + code interpreter) contributes its nested input/output bytes to group byte/token estimates, so byte/token-based compaction triggers fire appropriately for large hosted tool payloads.
Changes:
- Extend
computeContentByteCountto account forMCPServerTool{Call,Result}ContentandCodeInterpreterTool{Call,Result}Content, recursing intoInputs/Outputs. - Add a regression test ensuring hosted tool result outputs are byte-accounted comparably to
FunctionResultContent.
File summaries
| File | Description |
|---|---|
| agent/compaction/index.go | Adds byte counting for hosted tool call/result content (including recursive counting of nested contents). |
| agent/compaction/index_test.go | Adds a regression test to catch under-counting of hosted tool result output bytes. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| case *message.CodeInterpreterToolCallContent: | ||
| total := stringByteCount(typed.CallID) | ||
| for _, input := range typed.Inputs { | ||
| total += computeContentByteCount(input) | ||
| } |
| // A hosted tool result carrying 10k bytes of Outputs must be byte-accounted | ||
| // like the equivalent FunctionResultContent, so token/byte-based compaction | ||
| // triggers see the payload that most needs compacting. | ||
| fnMsg := &message.Message{Role: message.RoleTool, Contents: message.Contents{ | ||
| &message.FunctionResultContent{CallID: "c1", Result: big}, | ||
| }} | ||
| mcpMsg := &message.Message{Role: message.RoleTool, Contents: message.Contents{ | ||
| &message.MCPServerToolResultContent{CallID: "c1", Outputs: message.Contents{&message.TextContent{Text: big}}}, | ||
| }} | ||
| ciMsg := &message.Message{Role: message.RoleTool, Contents: message.Contents{ | ||
| &message.CodeInterpreterToolResultContent{CallID: "c1", Outputs: message.Contents{&message.TextContent{Text: big}}}, | ||
| }} | ||
| prefix := textMessage(message.RoleUser, "hi") | ||
|
|
||
| fnBytes := compaction.CreateMessageIndex([]*message.Message{prefix, fnMsg}, nil).TotalByteCount() | ||
| mcpBytes := compaction.CreateMessageIndex([]*message.Message{prefix, mcpMsg}, nil).TotalByteCount() | ||
| ciBytes := compaction.CreateMessageIndex([]*message.Message{prefix, ciMsg}, nil).TotalByteCount() | ||
|
|
||
| if mcpBytes < fnBytes { | ||
| t.Errorf("MCP tool-result bytes = %d, want >= FunctionResult bytes %d (Outputs undercounted)", mcpBytes, fnBytes) | ||
| } | ||
| if ciBytes < fnBytes { | ||
| t.Errorf("code-interpreter result bytes = %d, want >= FunctionResult bytes %d (Outputs undercounted)", ciBytes, fnBytes) | ||
| } | ||
| } |
|
Scope: internal-only Changed Go contract: None. The change is confined to the unexported Upstream evidence reviewed: This is a Go-only internal byte/token-estimation heuristic ( Result: out of scope for cross-repo parity review — the change fixes an accounting bug in an unexported Go-only helper with no user-visible API surface change. It does correct an internal under-counting bug that could affect compaction trigger timing, but it brings hosted tool-call/result accounting in line with the framework's own existing pattern ( Warning Firewall blocked 3 domainsThe following domains were blocked by the firewall during workflow execution:
[!TIP] tools:
github:
mode: gh-proxySee GitHub Tools for more information on To allow these domains, add them to the network:
allowed:
- defaults
- "api.github.com"
- "github.com"
- "patchdiff.githubusercontent.com"See Network Configuration for more information.
|
github-actions
Bot
added
risk:medium
Problem
computeContentByteCount(agent/compaction/index.go) computes each group'sByteCountand, viacomputeTokenCount's default branch (… / 4), the token estimate for non-text content. It accountsFunctionResultContent(fmt.Sprint(Result)) andDataContent(len(Data)), butMCPServerToolResultContent/CodeInterpreterToolResultContent(and the call variants) hitdefault: return 0.So a
MCPServerToolResultContent/CodeInterpreterToolResultContentcarrying 10,000 bytes ofOutputscontributes only ~2 bytes (just the CallID), whereas the equivalentFunctionResultContentcontributes 10,004. Byte/token-based triggers (TokensExceed, and thereforeContextWindowStrategy) under-count history dominated by hosted code-interpreter / MCP outputs — precisely the large content that most needs compaction — so compaction can fail to fire when it should.Fix
Account the hosted tool-call/result types by recursing into their
Outputs/Inputs(summingcomputeContentByteCount) and counting their identity strings, mirroring the function-call/result cases.defaultstays0for genuinely unknown types.Test
TestMessageIndex_CountsHostedToolResultBytesbuilds an MCP result and a code-interpreter result each carrying a 10k-byteOutputstext and asserts theirTotalByteCountis at least that of the equivalentFunctionResultContent. Fails before the fix (2 bytes vs 10004), passes after.