-
Notifications
You must be signed in to change notification settings - Fork 852
Skip adding messages to context when invocation fails with exception #2600
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
Skip adding messages to context when invocation fails with exception #2600
Conversation
…cation fails Co-authored-by: westey-m <[email protected]>
| // Add the request and response messages to the recent messages queue | ||
| foreach (var message in context.RequestMessages) | ||
| // Only add messages to the recent messages queue if the invocation succeeded | ||
| if (context.InvokeException is null) |
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.
@copilot Instead of having an if around all the code, just have an if that checks if InvokeException is not null and return if so.
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.
Refactored to use early return pattern in commit 70b0e76.
westey-m
left a comment
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.
Please fix requested item
Co-authored-by: westey-m <[email protected]>
Could you clarify what needs to be fixed? The early return pattern was implemented in commit 70b0e76 as requested. The current code checks if |
Motivation and Context
Addresses review feedback from #2505: #2505 (comment)
The
ContextualFunctionProviderwas adding failed invocation messages to its context history, polluting the conversation state with messages from unsuccessful operations.Description
Modified
InvokedAsyncto checkcontext.InvokeExceptionand use an early return pattern when invocations fail:Implementation Details
InvokeExceptionis not nullTest Coverage
InvokedAsync_ShouldNotAddMessages_WhenExceptionIsPresent_Asyncto verify messages from failed invocations are excluded from context historyContribution Checklist
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.