Improve sequential plan parser with support for missing functions. #1615
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
.NET
lemillermicrosoft
force-pushed
the
615_invalid_ask_regex
branch
from
6b7a4e8
to
d6a68e5
Compare
lemillermicrosoft
added
the
PR: in progress
dotnet/src/Extensions/Planning.SequentialPlanner/SequentialPlanParser.cs
Show resolved
Hide resolved
dotnet/src/Extensions/Planning.SequentialPlanner/SequentialPlanParser.cs
Outdated
Show resolved
Hide resolved
lemillermicrosoft
force-pushed
the
615_invalid_ask_regex
branch
2 times, most recently
from
0ea3f7d
to
17eb799
Compare
lemillermicrosoft
added
PR: ready for review
lemillermicrosoft
force-pushed
the
615_invalid_ask_regex
branch
from
4408f7c
to
5665d90
Compare
5 tasks
lemillermicrosoft
force-pushed
the
615_invalid_ask_regex
branch
from
5b0a167
to
3f885ce
Compare
This commit fixes several issues with the plan parsing, execution, and replacement logic in the sequential planner and the semantic kernel. It adds some unit tests to cover the scenarios where the plan xml is not well-formed or contains other text, where the plan has variables with a $ in them that are not associated with an output, and where the plan xml contains a function from an OpenApi plugin. It also fixes a bug in the Plan class, where it would not correctly replace all occurrences of a string in the plan actions.
This commit fixes several issues related to the sequential planner. It improves the plan parsing and error handling by adding a new parameter to the ToPlanFromXml method to optionally ignore missing functions in the plan xml, instead of throwing an exception. It also improves the exception messages and codes for different scenarios, such as invalid plan xml, invalid goal, or unknown error. Additionally, it catches and rethrows planning exceptions in the CreatePlan method, and updates the skprompt file to include a new step for handling unavailable functions. Finally, it adds a missing value to the planning exception enum and updates the documentation accordingly.
This commit fixes a bug in the sequential plan parser that caused an exception when the plan xml was not valid. It adds a regex to extract the <plan> element from the input string, and handles any xml parsing errors gracefully. It also adds a new option to the SequentialPlannerConfig class that allows the planner to create a plan even if some functions are missing from the skills. The missing functions are treated as no-op steps in the plan. This option is useful for testing and debugging purposes, but should be used with caution in production scenarios. The commit also improves the error messages and logging for plan parsing and creation, by including the function and skill names in the exception messages and using consistent quotes for xml strings.
The sequential plan parser was not handling invalid function nodes correctly, and would throw an exception if the allowMissingFunctions parameter was false. This commit adds a test case for this scenario and fixes the bug by checking the function name before creating the step. It also adds a default skill name and description for invalid function nodes, to avoid null reference errors.
This commit adds a new error code and exception message for the scenario when the sequential planner cannot create a plan for a given goal with the available functions. It also modifies the skprompt.txt file to return <noplan /> instead of an empty plan in this case. The commit also includes some minor formatting changes and a new test case for the plan not possible scenario.
This commit fixes a bug in the sequential plan parser that caused the description and steps properties of the plan steps to be null or empty when parsing a plan from XML. The bug was due to a missing assignment of the description property and a wrong constructor call for the plan steps. The commit also adds a unit test to verify the correct behavior of the parser.
This commit adds a check for the ErrorOccurred property of the planResult object returned by the functionFlowFunction. If it is true, it throws a PlanningException with the appropriate error code, message, and inner exception. This ensures that any errors that occur during the plan creation process are properly propagated and handled by the caller.
shawncal
force-pushed
the
615_invalid_ask_regex
branch
from
3f885ce
to
0715cf3
Compare
shawncal
approved these changes
Jun 23, 2023
shawncal
pushed a commit
to shawncal/semantic-kernel
that referenced
this pull request
Jul 6, 2023
…icrosoft#1615) ### Motivation and Context This pull request improves the ability to create sequential plans by parsing the XML plan text and matching the function names and parameters with the registered plugin functions. It also improves the error handling and logging in the sequential planner by adding more specific error codes, messages, and exceptions for different scenarios. The main motivation for these changes is to make the plan creation more robust and flexible, and to avoid throwing exceptions when the plan XML is not well-formed or contains unknown functions. The new option, `AllowMissingFunctions`, can be set to true or false in the SequentialPlannerConfig class, and it determines whether the plan creation will fail or succeed when a function is missing in the skill library. The plan parsing logic has been improved to use a regular expression to extract the <plan> element from the XML string, and to handle any XmlException that may occur when loading the XML document. Related to microsoft#1483 Fixes microsoft#159 ### Description - Added a new method `CreateKernelAndFunctionCreateMocks` in `SequentialPlanParserTests` to create a mock kernel and register mock plugin functions for testing - Added three new test cases in `SequentialPlanParserTests` to test the parsing of plans with OpenApi plugin functions, with different input formats and parameters - Modified `SequentialPlanParser` to handle the case where the plan text is not valid XML by using a regular expression to try to extract the plan XML from the input string and throwing a `PlanningException` if it fails - Added a new error code `CreatePlanError` for cases where the plan creation fails due to invalid goal or plan XML - Added a new option `AllowMissingFunctions` to the `ToPlanFromXml` method, which allows the parser to append missing function nodes as plain text steps instead of throwing an exception - Updated the `skprompt.txt` file with a new instruction to return an empty plan if a plan cannot be created with the available functions - Fixed a bug in the `SequentialPlanParser` where an exception was thrown twice when the plan XML was invalid - Added more specific error messages and exceptions for different scenarios, such as invalid configuration, unknown error, or missing function - Improved the logging of the plan parsing and processing by adding more trace and error messages ### 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` - [x] 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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation and Context
This pull request improves the ability to create sequential plans by parsing the XML plan text and matching the function names and parameters with the registered plugin functions. It also improves the error handling and logging in the sequential planner by adding more specific error codes, messages, and exceptions for different scenarios. The main motivation for these changes is to make the plan creation more robust and flexible, and to avoid throwing exceptions when the plan XML is not well-formed or contains unknown functions. The new option,
AllowMissingFunctions, can be set to true or false in the SequentialPlannerConfig class, and it determines whether the plan creation will fail or succeed when a function is missing in the skill library. The plan parsing logic has been improved to use a regular expression to extract the element from the XML string, and to handle any XmlException that may occur when loading the XML document.Related to #1483
Fixes #159
Description
CreateKernelAndFunctionCreateMocksinSequentialPlanParserTeststo create a mock kernel and register mock plugin functions for testingSequentialPlanParserTeststo test the parsing of plans with OpenApi plugin functions, with different input formats and parametersSequentialPlanParserto handle the case where the plan text is not valid XML by using a regular expression to try to extract the plan XML from the input string and throwing aPlanningExceptionif it failsCreatePlanErrorfor cases where the plan creation fails due to invalid goal or plan XMLAllowMissingFunctionsto theToPlanFromXmlmethod, which allows the parser to append missing function nodes as plain text steps instead of throwing an exceptionskprompt.txtfile with a new instruction to return an empty plan if a plan cannot be created with the available functionsSequentialPlanParserwhere an exception was thrown twice when the plan XML was invalidContribution Checklist
dotnet format