Skip to content
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

.Net: [Fix] .Net: FunctionCallingStepwisePlannerTests.DoesNotThrowWhenPluginFunctionThrowsNonCriticalExceptionAsync is failing #4542

Merged
merged 8 commits into from
Jan 11, 2024

Conversation

gitri-ms
Copy link
Contributor

@gitri-ms gitri-ms commented Jan 9, 2024

Motivation and Context

Fix for failing integration test (resolves #4534)

When using certain OpenAI models such as gpt-3.5-turbo-1106, the model would sometimes opt to call GetEmailAddressAsync when it wasn't actually needed. This was causing a critical exception to be thrown (which was by design for a different test case, but in this case was being called unexpectedly and causing the test to fail.)

Description

This change modifies the test scenarios to minimize the possibility of a critical exception being thrown by mistake. Now WriteJokeAsync throws the critical exception, so the other test cases (that involve writing/emailing poems) should not encounter it.

Contribution Checklist

Sorry, something went wrong.

@gitri-ms gitri-ms added bug Something isn't working PR: ready for review All feedback addressed, ready for reviews labels Jan 9, 2024
@gitri-ms gitri-ms self-assigned this Jan 9, 2024
@gitri-ms gitri-ms requested a review from a team as a code owner January 9, 2024 22:16
@shawncal shawncal added .NET Issue or Pull requests regarding .NET code kernel Issues or pull requests impacting the core kernel labels Jan 9, 2024
@github-actions github-actions bot changed the title [Fix] .Net: FunctionCallingStepwisePlannerTests.DoesNotThrowWhenPluginFunctionThrowsNonCriticalExceptionAsync is failing .Net: [Fix] .Net: FunctionCallingStepwisePlannerTests.DoesNotThrowWhenPluginFunctionThrowsNonCriticalExceptionAsync is failing Jan 9, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@markwallace-microsoft markwallace-microsoft added this pull request to the merge queue Jan 11, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 11, 2024
@markwallace-microsoft
Copy link
Member

@gitri-ms Could you take a look as the problem still seems to be there, see: https://github.com/microsoft/semantic-kernel/actions/runs/7487880249/job/20381194144#step:6:319

@gitri-ms
Copy link
Contributor Author

@gitri-ms Could you take a look as the problem still seems to be there, see: https://github.com/microsoft/semantic-kernel/actions/runs/7487880249/job/20381194144#step:6:319

Hmm... this looks like a different exception being thrown. Looking...

@gitri-ms gitri-ms added this pull request to the merge queue Jan 11, 2024
Merged via the queue into microsoft:main with commit 93bdce8 Jan 11, 2024
@gitri-ms gitri-ms deleted the failing-test branch January 11, 2024 23:10
Bryan-Roe pushed a commit to Bryan-Roe-ai/semantic-kernel that referenced this pull request Oct 6, 2024
…nPluginFunctionThrowsNonCriticalExceptionAsync is failing (microsoft#4542)

### Motivation and Context

<!-- Thank you for your contribution to the semantic-kernel repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  2. What problem does it solve?
  3. What scenario does it contribute to?
  4. If it fixes an open issue, please link to the issue here.
-->
Fix for failing integration test (resolves microsoft#4534)

When using certain OpenAI models such as `gpt-3.5-turbo-1106`, the model
would sometimes opt to call `GetEmailAddressAsync` when it wasn't
actually needed. This was causing a critical exception to be thrown
(which was by design for a different test case, but in this case was
being called unexpectedly and causing the test to fail.)

### Description

<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->
This change modifies the test scenarios to minimize the possibility of a
critical exception being thrown by mistake. Now `WriteJokeAsync` throws
the critical exception, so the other test cases (that involve
writing/emailing poems) should not encounter it.

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [x] The code builds clean without any errors or warnings
- [x] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [x] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone 😄
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working kernel Issues or pull requests impacting the core kernel .NET Issue or Pull requests regarding .NET code PR: ready for review All feedback addressed, ready for reviews
Projects
Archived in project
7 participants