-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: prevent API error messages from polluting chat history #7250
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Remove error message from API conversation history when assistant provides no response.
Keep error display to user via say("error") but do not add to LLM context.
This fixes the issue where API errors disrupt conversation flow.
Fixes #7249
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing my own code is like debugging in production - technically possible but morally questionable.
| role: "assistant", | ||
| content: [{ type: "text", text: "Failure: I did not provide a response." }], | ||
| }) | ||
| // Don't add error messages to the API conversation history as it pollutes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we add a test case to verify that error messages are NOT added to the API conversation history when an empty assistant message is received? This would help prevent regression of this issue.
| content: [{ type: "text", text: "Failure: I did not provide a response." }], | ||
| }) | ||
| // Don't add error messages to the API conversation history as it pollutes | ||
| // the context sent to the LLM. The error is already shown to the user via say("error"). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding the issue number to the comment for future reference:
| // the context sent to the LLM. The error is already shown to the user via say("error"). | |
| // Don't add error messages to the API conversation history as it pollutes | |
| // the context sent to the LLM (fixes #7249). The error is already shown to the user via say("error"). | |
| // This prevents the issue where API errors disrupt the conversation flow. |
Summary
This PR fixes issue #7249 where API error messages were being added to the chat history, disrupting the conversation flow with the LLM.
Problem
When an API returns an empty assistant message or encounters an error, the system was adding a hardcoded error message "Failure: I did not provide a response." to the API conversation history. This error message would then be sent to the LLM in subsequent requests, polluting the context and disrupting the natural conversation flow.
Solution
say("error")so users are still informed about the issueTesting
npx vitest run core/task/__tests__/Task.spec.ts- all tests pass ✅Impact
This change ensures that API-level errors are properly displayed to users without contaminating the LLM's conversation context, resulting in more coherent and consistent interactions.
Fixes #7249
Important
Fixes issue #7249 by preventing API error messages from being added to conversation history in
Task.ts, ensuring cleaner LLM interactions.apiConversationHistoryinrecursivelyMakeClineRequests()inTask.tsto prevent context pollution.say("error").core/task/__tests__/Task.spec.ts- all tests pass.This description was created by
for 025e605. You can customize this summary. It will automatically update as commits are pushed.