Add tool calling support to Model catalog#2687
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
manaswinidas
force-pushed
the
feat/tool-calling-ui
branch
from
9a76dee to
6c9ebd9
Compare
manaswinidas
force-pushed
the
feat/tool-calling-ui
branch
from
6c9ebd9 to
c2113ea
Compare
ppadti
left a comment
ppadti
left a comment
There was a problem hiding this comment.
@manaswinidas we also need to update our OpenAPI spec to have these changes of type in CatalogModel.
And when we close the sidebar - The alignment of logo, label in the card changes
| className="pf-v6-u-p-xs" | ||
| icon={<HelpIcon />} | ||
| <Stack hasGutter> | ||
| {isToolCallingValidated && model.servingConfig?.toolCalling && ( |
There was a problem hiding this comment.
Isn't this condition redundant? because isToolCallingValidated already checks for model.servingConfig?.toolCalling, no?
| name: 'validated-model', | ||
| tasks: ['text-generation'], | ||
| tasks: ['text-generation', 'tool-calling'], | ||
| validatedTasks: ['tool-calling'], |
There was a problem hiding this comment.
would it be better to have this tool-calling as a constant?
| artifacts: CatalogArtifacts[], | ||
| ): boolean => isModelValidated(model) && hasPerformanceArtifacts(artifacts); | ||
|
|
||
| export const hasValidatedToolCalling = (model: CatalogModel): boolean => |
There was a problem hiding this comment.
can we add unit test for this?
| ): boolean => isModelValidated(model) && hasPerformanceArtifacts(artifacts); | ||
|
|
||
| export const hasValidatedToolCalling = (model: CatalogModel): boolean => | ||
| !!model.validatedTasks?.includes('tool-calling') && !!model.servingConfig?.toolCalling; |
There was a problem hiding this comment.
Nit: Just from the readability perspective
can we make it
| !!model.validatedTasks?.includes('tool-calling') && !!model.servingConfig?.toolCalling; | |
| model.validatedTasks?.includes('tool-calling') === true && model.servingConfig?.toolCalling != null; |
Either way its right - no strong opinion though.
| <CheckCircleIcon color="var(--pf-t--global--color--status--success--default)" /> | ||
| } | ||
| > | ||
| Validated by Red Hat |
There was a problem hiding this comment.
We might have to get clarification for this label from backend - whether we get this as a label ot should we construct one?
There was a problem hiding this comment.
The existing "Validated" label is getting a revamp - screenshot here
- Add validatedTasks, ServingConfig, ToolCallingConfig to BFF types, frontend types, and OpenAPI spec - Add Validated configurations card with expandable tool calling section - Update Validated label to 'Validated by Red Hat' with success status - Show validated task labels with green check icon - Gate tool calling features behind TempDevFeature.ToolCallingConfiguration - Use MODEL_CATALOG_TASK_NAME_MAPPING for task label display names - Add hasValidatedToolCalling utility with unit tests - Update BFF, frontend, and Cypress mocks Ref: RHOAIENG-60925 Signed-off-by: manaswinidas <dasmanaswini10@gmail.com>
manaswinidas
force-pushed
the
feat/tool-calling-ui
branch
from
c2113ea to
708b123
Compare
|
Removed " by Red Hat" from the labels - it just says "Validated" now. 
|
Updated screenshot for details page |
2 participants
Description
Add validated configurations card to model details page with expandable tool calling section behind a feature flag. Update
"Validated" label to "Validated by Red Hat" with green check icon, and show validated task labels with check indicators.
How Has This Been Tested?
Switch on the feature flag on the browser console and check for the labels in the model card and the tool calling validation configurations in the catalog details page as shown in the screenshots.
Merge criteria:
DCOcheck)ok-to-testhas been added to the PR.If you have UI changes