fix: filter out orphaned tool results from history and current context#269
Conversation
Summary of ChangesHello @Darley-Wey, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a crucial mechanism to maintain the integrity of tool-related data within conversation history. By proactively identifying and removing tool results that have become disassociated from their corresponding tool uses due to history truncation, it ensures that the Kiro API receives well-formed requests, thereby enhancing the robustness and reliability of the system's interaction with external tools. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly addresses a bug where orphaned tool_result entries could cause API errors after history truncation. The new filterOrphanedToolResults function effectively cleans up both the history and the current message. The added unit test is comprehensive and validates the fix. I've included a couple of suggestions to improve performance by using in-place slice filtering, which is more memory-efficient.
| filtered := make([]KiroToolResult, 0, len(ctx.ToolResults)) | ||
| for _, tr := range ctx.ToolResults { | ||
| if validToolUseIDs[tr.ToolUseID] { | ||
| filtered = append(filtered, tr) | ||
| continue | ||
| } | ||
| log.Debugf("kiro-openai: dropping orphaned tool_result in history[%d]: toolUseId=%s (no matching tool_use)", i, tr.ToolUseID) | ||
| } | ||
| ctx.ToolResults = filtered |
There was a problem hiding this comment.
For better performance, you can filter the ctx.ToolResults slice in-place. This avoids allocating a new filtered slice on each call, which is more memory-efficient, especially for long histories. This is a common Go idiom for slice filtering.
n := 0
for _, tr := range ctx.ToolResults {
if validToolUseIDs[tr.ToolUseID] {
ctx.ToolResults[n] = tr
n++
} else {
log.Debugf("kiro-openai: dropping orphaned tool_result in history[%d]: toolUseId=%s (no matching tool_use)", i, tr.ToolUseID)
}
}
ctx.ToolResults = ctx.ToolResults[:n]| filtered := make([]KiroToolResult, 0, len(currentToolResults)) | ||
| for _, tr := range currentToolResults { | ||
| if validToolUseIDs[tr.ToolUseID] { | ||
| filtered = append(filtered, tr) | ||
| continue | ||
| } | ||
| log.Debugf("kiro-openai: dropping orphaned tool_result in currentMessage: toolUseId=%s (no matching tool_use)", tr.ToolUseID) | ||
| } | ||
| if len(filtered) != len(currentToolResults) { | ||
| log.Infof("kiro-openai: dropped %d orphaned tool_result(s) from currentMessage", len(currentToolResults)-len(filtered)) | ||
| } | ||
| currentToolResults = filtered |
There was a problem hiding this comment.
Similar to the history filtering, currentToolResults can also be filtered in-place to avoid allocating a new slice. This improves efficiency by reducing memory allocations.
originalLen := len(currentToolResults)
n := 0
for _, tr := range currentToolResults {
if validToolUseIDs[tr.ToolUseID] {
currentToolResults[n] = tr
n++
} else {
log.Debugf("kiro-openai: dropping orphaned tool_result in currentMessage: toolUseId=%s (no matching tool_use)", tr.ToolUseID)
}
}
if n < originalLen {
log.Infof("kiro-openai: dropped %d orphaned tool_result(s) from currentMessage", originalLen-n)
}
currentToolResults = currentToolResults[:n]There was a problem hiding this comment.
Pull request overview
This PR fixes malformed Kiro API requests caused by truncated history leaving tool_result entries that reference tool_use IDs from dropped assistant turns. It ensures only valid tool result entries (those with a matching retained tool_use) are sent onward.
Changes:
- Collect valid
toolUseIdvalues from retained assistant history. - Filter out orphaned
tool_resultentries from both retained history contexts and the current message tool results. - Add a focused unit test covering mixed/fully-orphaned history contexts and current tool results.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| internal/translator/kiro/openai/kiro_openai_request.go | Adds post-truncation filtering to drop orphaned tool results from history and current tool results before building the Kiro payload. |
| internal/translator/kiro/openai/kiro_openai_request_test.go | Adds a unit test validating orphan filtering behavior for both history and current tool results. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Superseded by fork migration. This PR has been moved to KooshaPari/cliproxyapi-plusplus as #210. Please close this upstream PR to keep the queue clean. |
When history is truncated, tool_result entries may reference tool_use IDs from assistant turns that were dropped. This causes Kiro API to return 'Improperly formed request' errors.
This fix: