From c519c3c8f547804bf386e7c9264a50268ba7f8a5 Mon Sep 17 00:00:00 2001 From: Tony Powell Date: Mon, 18 Nov 2024 16:51:27 -0500 Subject: [PATCH] fix: Hide Output Schema (response format) button on unsupported provider instances Resolves #5385 Resolves #5384 --- .../playground/InvocationParametersForm.tsx | 59 +++++---- .../pages/playground/ModelConfigButton.tsx | 6 +- .../playground/PlaygroundChatTemplate.tsx | 74 ++++++++--- .../pages/playground/PlaygroundTemplate.tsx | 6 +- ...ChatTemplateResponseFormatQuery.graphql.ts | 118 ++++++++++++++++++ 5 files changed, 212 insertions(+), 51 deletions(-) create mode 100644 app/src/pages/playground/__generated__/PlaygroundChatTemplateResponseFormatQuery.graphql.ts diff --git a/app/src/pages/playground/InvocationParametersForm.tsx b/app/src/pages/playground/InvocationParametersForm.tsx index db24de02f9..4bb2b0004d 100644 --- a/app/src/pages/playground/InvocationParametersForm.tsx +++ b/app/src/pages/playground/InvocationParametersForm.tsx @@ -172,13 +172,6 @@ type InvocationParametersFormProps = { instanceId: number; }; -/** - * Azure openai has user defined model names but our invocation parameters query will never know - * what they are. We will just pass in a stub model name and the query will fallback to the set - * of invocation parameters that are defaults to our azure client. - */ -const AZURE_MODEL_NAME = "AZURE_DEPLOYMENT"; - export const InvocationParametersForm = ({ instanceId, }: InvocationParametersFormProps) => { @@ -195,8 +188,13 @@ export const InvocationParametersForm = ({ const filterInstanceModelInvocationParameters = usePlaygroundContext( (state) => state.filterInstanceModelInvocationParameters ); + /** + * Azure openai has user defined model names but our invocation parameters query will never know + * what they are. We will just pass in an empty model name and the query will fallback to the set + * of invocation parameters that are defaults to our azure client. + */ const modelNameToQuery = - model.provider !== "AZURE_OPENAI" ? model.modelName : AZURE_MODEL_NAME; + model.provider !== "AZURE_OPENAI" ? model.modelName : null; const { modelInvocationParameters } = useLazyLoadQuery( graphql` @@ -245,24 +243,6 @@ export const InvocationParametersForm = ({ { input: { providerKey: model.provider, modelName: modelNameToQuery } } ); - useEffect(() => { - // filter invocation parameters to only include those that are supported by the model - if (modelInvocationParameters) { - filterInstanceModelInvocationParameters({ - instanceId: instance.id, - modelSupportedInvocationParameters: - modelInvocationParameters as Mutable< - typeof modelInvocationParameters - >, - filter: constrainInvocationParameterInputsToDefinition, - }); - } - }, [ - filterInstanceModelInvocationParameters, - instance.id, - modelInvocationParameters, - ]); - const onChange = useCallback( (field: InvocationParameter, value: unknown) => { const existingParameter = instance.model.invocationParameters.find( @@ -295,6 +275,33 @@ export const InvocationParametersForm = ({ [instance, updateInstanceModelInvocationParameters] ); + useEffect(() => { + // filter invocation parameters to only include those that are supported by the model + // This will remove configured values that are not supported by the newly selected model + // Including invocation parameters managed outside of this form, like response_format + if (modelInvocationParameters) { + filterInstanceModelInvocationParameters({ + instanceId: instance.id, + modelSupportedInvocationParameters: + modelInvocationParameters as Mutable< + typeof modelInvocationParameters + >, + filter: constrainInvocationParameterInputsToDefinition, + }); + } + }, [ + filterInstanceModelInvocationParameters, + instance.id, + modelInvocationParameters, + ]); + + // It is safe to render this component if the model name is not set for non-azure models + // Hooks will still run to filter invocation parameters to only include those supported by the model + // but no form fields will be rendered if the model name is not set + if (model.modelName === null && model.provider !== "AZURE_OPENAI") { + return null; + } + const fieldsForSchema = modelInvocationParameters .filter( (field) => diff --git a/app/src/pages/playground/ModelConfigButton.tsx b/app/src/pages/playground/ModelConfigButton.tsx index 0f00b31183..c503194958 100644 --- a/app/src/pages/playground/ModelConfigButton.tsx +++ b/app/src/pages/playground/ModelConfigButton.tsx @@ -331,11 +331,9 @@ function ModelConfigDialogContent(props: ModelConfigDialogContentProps) { onChange={onModelNameChange} /> )} - {instance.model.modelName ? ( + - ) : ( - <> - )} + diff --git a/app/src/pages/playground/PlaygroundChatTemplate.tsx b/app/src/pages/playground/PlaygroundChatTemplate.tsx index f43a5aa9eb..83af768ad9 100644 --- a/app/src/pages/playground/PlaygroundChatTemplate.tsx +++ b/app/src/pages/playground/PlaygroundChatTemplate.tsx @@ -1,4 +1,5 @@ import React, { PropsWithChildren, useCallback, useState } from "react"; +import { useLazyLoadQuery } from "react-relay"; import { DndContext, KeyboardSensor, @@ -13,6 +14,7 @@ import { useSortable, } from "@dnd-kit/sortable"; import { CSS } from "@dnd-kit/utilities"; +import { graphql } from "relay-runtime"; import { css } from "@emotion/react"; import { @@ -44,6 +46,7 @@ import { import { assertUnreachable } from "@phoenix/typeUtils"; import { safelyParseJSON } from "@phoenix/utils/jsonUtils"; +import { PlaygroundChatTemplateResponseFormatQuery } from "./__generated__/PlaygroundChatTemplateResponseFormatQuery.graphql"; import { ChatMessageToolCallsEditor } from "./ChatMessageToolCallsEditor"; import { RESPONSE_FORMAT_PARAM_CANONICAL_NAME, @@ -87,6 +90,37 @@ export function PlaygroundChatTemplate(props: PlaygroundChatTemplateProps) { if (!playgroundInstance) { throw new Error(`Playground instance ${id} not found`); } + // We don't care about the model name for Azure OpenAI + const modelNameQueryInput = + playgroundInstance.model.provider !== "AZURE_OPENAI" + ? (playgroundInstance.model?.modelName ?? null) + : null; + const { modelInvocationParameters } = + useLazyLoadQuery( + graphql` + query PlaygroundChatTemplateResponseFormatQuery($input: ModelsInput!) { + modelInvocationParameters(input: $input) { + __typename + ... on InvocationParameterBase { + invocationName + canonicalName + } + } + } + `, + { + input: { + providerKey: playgroundInstance.model.provider, + modelName: modelNameQueryInput, + }, + } + ); + + const supportsResponseFormat = modelInvocationParameters?.some( + (p) => + p.canonicalName === RESPONSE_FORMAT_PARAM_CANONICAL_NAME || + p.invocationName === RESPONSE_FORMAT_PARAM_NAME + ); const hasTools = playgroundInstance.tools.length > 0; const hasResponseFormat = playgroundInstance.model.invocationParameters.find( @@ -167,25 +201,27 @@ export function PlaygroundChatTemplate(props: PlaygroundChatTemplateProps) { borderBottomWidth={hasTools || hasResponseFormat ? "thin" : undefined} > - + {supportsResponseFormat ? ( + + ) : null}