Skip to content

Commit

Permalink
fix: Hide Output Schema (response format) button on unsupported provi…
Browse files Browse the repository at this point in the history
…der instances

Resolves #5385
Resolves #5384
  • Loading branch information
cephalization committed Nov 18, 2024
1 parent ecef524 commit c519c3c
Show file tree
Hide file tree
Showing 5 changed files with 212 additions and 51 deletions.
59 changes: 33 additions & 26 deletions app/src/pages/playground/InvocationParametersForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand All @@ -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<InvocationParametersFormQuery>(
graphql`
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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) =>
Expand Down
6 changes: 2 additions & 4 deletions app/src/pages/playground/ModelConfigButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -331,11 +331,9 @@ function ModelConfigDialogContent(props: ModelConfigDialogContentProps) {
onChange={onModelNameChange}
/>
)}
{instance.model.modelName ? (
<Suspense>
<InvocationParametersForm instanceId={playgroundInstanceId} />
) : (
<></>
)}
</Suspense>
</Flex>
</Form>
</View>
Expand Down
74 changes: 55 additions & 19 deletions app/src/pages/playground/PlaygroundChatTemplate.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React, { PropsWithChildren, useCallback, useState } from "react";
import { useLazyLoadQuery } from "react-relay";
import {
DndContext,
KeyboardSensor,
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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<PlaygroundChatTemplateResponseFormatQuery>(
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(
Expand Down Expand Up @@ -167,25 +201,27 @@ export function PlaygroundChatTemplate(props: PlaygroundChatTemplateProps) {
borderBottomWidth={hasTools || hasResponseFormat ? "thin" : undefined}
>
<Flex direction="row" justifyContent="end" gap="size-100">
<Button
variant="default"
size="compact"
aria-label="output schema"
icon={<Icon svg={<Icons.PlusOutline />} />}
disabled={hasResponseFormat}
onClick={() => {
upsertInvocationParameterInput({
instanceId: id,
invocationParameterInput: {
valueJson: createOpenAIResponseFormat(),
invocationName: RESPONSE_FORMAT_PARAM_NAME,
canonicalName: RESPONSE_FORMAT_PARAM_CANONICAL_NAME,
},
});
}}
>
Output Schema
</Button>
{supportsResponseFormat ? (
<Button
variant="default"
size="compact"
aria-label="output schema"
icon={<Icon svg={<Icons.PlusOutline />} />}
disabled={hasResponseFormat}
onClick={() => {
upsertInvocationParameterInput({
instanceId: id,
invocationParameterInput: {
valueJson: createOpenAIResponseFormat(),
invocationName: RESPONSE_FORMAT_PARAM_NAME,
canonicalName: RESPONSE_FORMAT_PARAM_CANONICAL_NAME,
},
});
}}
>
Output Schema
</Button>
) : null}
<Button
variant="default"
aria-label="add tool"
Expand Down
6 changes: 4 additions & 2 deletions app/src/pages/playground/PlaygroundTemplate.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from "react";
import React, { Suspense } from "react";

import {
Button,
Expand Down Expand Up @@ -50,7 +50,9 @@ export function PlaygroundTemplate(props: PlaygroundTemplateProps) {
}
>
{template.__type === "chat" ? (
<PlaygroundChatTemplate {...props} />
<Suspense>
<PlaygroundChatTemplate {...props} />
</Suspense>
) : (
"Completion Template"
)}
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit c519c3c

Please sign in to comment.