-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Adding IsRequired property to all parameters #1457
Adding IsRequired property to all parameters #1457
Conversation
dotnet/src/Extensions/Planning.SequentialPlanner/FunctionViewExtensions.cs
Show resolved
Hide resolved
/// Whether the parameter is required. | ||
/// </summary> | ||
public bool IsRequired { get; set; } = true; | ||
|
||
/// <summary> | ||
/// Creates a parameter view, using information from an instance of this class. | ||
/// </summary> |
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.
#1195 is proposing some changes in the area. Might use this to help speed up things over there.
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.
LGTM - but I'm curious about using string.join as well.
…hoang/semantic-kernel into required-function-params
Closing in favor of Stephen's PR: #1195 Will an issue to re-evaluate how we specify required / optional parameters to the Planner. It seems like Stephen's PR will handle native functions well, where all parameters are required but have a DefaultValue of null when optional. But some native functions specify (optional) in the parameter description, whereas we're specifying (required) in the description of rest api parameters when necessary. The AI could get confused for parameters with descriptions that don't contain either postfix, so we should pick one flag or the other for the planner's functionView. |
Motivation and Context
Extending parameter objects to include an
IsRequired
field for better planner resiliency.Description
Also, includes some logic changes to Copilot PlanViewer to show INPUT value.
Contribution Checklist
dotnet format