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

Fix parameter assignment in ActionPlanner #1530

Merged

Conversation

lemillermicrosoft
Copy link
Member

@lemillermicrosoft lemillermicrosoft commented Jun 16, 2023

Motivation and Context

This pull request fixes a bug in the ActionPlanner class where the plan parameters were not assigned correctly from the input parameters. It also updates some formatting

Fixes #997

Description

  • Fix parameter assignment in ActionPlanner.CreatePlanFromParameters method
    • Use plan.Parameters instead of plan.State to assign the input parameters to the plan object
    • This fixes a bug where the plan state was overwritten by the input parameters, causing unexpected behavior and errors

Contribution Checklist

@github-actions github-actions bot added .NET Issue or Pull requests regarding .NET code kernel Issues or pull requests impacting the core kernel labels Jun 16, 2023
This commit fixes a bug where the plan state was incorrectly updated
instead of the plan parameters. It also adds descriptions for the
SKFunction attributes of the ActionPlanner class, to make them more
readable and understandable. Additionally, it formats the parameters
of the functions to follow the coding style guidelines.
@lemillermicrosoft lemillermicrosoft removed this pull request from the merge queue due to a manual request Jun 16, 2023
@lemillermicrosoft lemillermicrosoft added this pull request to the merge queue Jun 16, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 16, 2023
@lemillermicrosoft lemillermicrosoft added this pull request to the merge queue Jun 16, 2023
Merged via the queue into microsoft:main with commit c8ba55e Jun 16, 2023
@lemillermicrosoft lemillermicrosoft deleted the 616_action_substep branch June 16, 2023 21:51
markwallace-microsoft pushed a commit to markwallace-microsoft/semantic-kernel that referenced this pull request Jun 23, 2023
### Motivation and Context
This pull request fixes a bug in the ActionPlanner class where the plan
parameters were not assigned correctly from the input parameters. It
also updates some formatting

Fixes microsoft#997 

### Description
- Fix parameter assignment in ActionPlanner.CreatePlanFromParameters
method
- Use plan.Parameters instead of plan.State to assign the input
parameters to the plan object
- This fixes a bug where the plan state was overwritten by the input
parameters, causing unexpected behavior and errors

### Contribution Checklist
<!-- Before submitting this PR, please make sure: -->
- [x] The code builds clean without any errors or warnings
- [x] The PR follows SK Contribution Guidelines
(https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
- [x] The code follows the .NET coding conventions
(https://learn.microsoft.com/dotnet/csharp/fundamentals/coding-style/coding-conventions)
verified with `dotnet format`
- [ ] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone 😄

---------

Co-authored-by: Lee Miller <lemillermicrosoft@users.noreply.github.com>
@lemillermicrosoft lemillermicrosoft added this to the Sprint 33 milestone Jun 27, 2023
shawncal pushed a commit to shawncal/semantic-kernel that referenced this pull request Jul 6, 2023
### Motivation and Context
This pull request fixes a bug in the ActionPlanner class where the plan
parameters were not assigned correctly from the input parameters. It
also updates some formatting

Fixes microsoft#997 

### Description
- Fix parameter assignment in ActionPlanner.CreatePlanFromParameters
method
- Use plan.Parameters instead of plan.State to assign the input
parameters to the plan object
- This fixes a bug where the plan state was overwritten by the input
parameters, causing unexpected behavior and errors

### Contribution Checklist
<!-- Before submitting this PR, please make sure: -->
- [x] The code builds clean without any errors or warnings
- [x] The PR follows SK Contribution Guidelines
(https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
- [x] The code follows the .NET coding conventions
(https://learn.microsoft.com/dotnet/csharp/fundamentals/coding-style/coding-conventions)
verified with `dotnet format`
- [ ] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone 😄

---------

Co-authored-by: Lee Miller <lemillermicrosoft@users.noreply.github.com>
golden-aries pushed a commit to golden-aries/semantic-kernel that referenced this pull request Oct 10, 2023
### Motivation and Context
This pull request fixes a bug in the ActionPlanner class where the plan
parameters were not assigned correctly from the input parameters. It
also updates some formatting

Fixes microsoft#997 

### Description
- Fix parameter assignment in ActionPlanner.CreatePlanFromParameters
method
- Use plan.Parameters instead of plan.State to assign the input
parameters to the plan object
- This fixes a bug where the plan state was overwritten by the input
parameters, causing unexpected behavior and errors

### Contribution Checklist
<!-- Before submitting this PR, please make sure: -->
- [x] The code builds clean without any errors or warnings
- [x] The PR follows SK Contribution Guidelines
(https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
- [x] The code follows the .NET coding conventions
(https://learn.microsoft.com/dotnet/csharp/fundamentals/coding-style/coding-conventions)
verified with `dotnet format`
- [ ] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone 😄

---------

Co-authored-by: Lee Miller <lemillermicrosoft@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kernel Issues or pull requests impacting the core kernel .NET Issue or Pull requests regarding .NET code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Input confusion when ActionPlanner works with substep
3 participants