Skip to content
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

fix: Derive playground values instead of storing them in state #5067

Merged
merged 2 commits into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 3 additions & 8 deletions app/src/pages/playground/PlaygroundInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,19 @@ import React from "react";
import { Flex, Text, View } from "@arizeai/components";

import { usePlaygroundContext } from "@phoenix/contexts/PlaygroundContext";
import {
selectDerivedInputVariables,
selectInputVariableKeys,
} from "@phoenix/store";
import { assertUnreachable } from "@phoenix/typeUtils";

import { useDerivedPlaygroundVariables } from "./useDerivedPlaygroundVariables";
import { VariableEditor } from "./VariableEditor";

export function PlaygroundInput() {
const variables = usePlaygroundContext(selectDerivedInputVariables);
const variableKeys = usePlaygroundContext(selectInputVariableKeys);
const { variableKeys, variablesMap } = useDerivedPlaygroundVariables();
const setVariableValue = usePlaygroundContext(
(state) => state.setVariableValue
);
const templateLanguage = usePlaygroundContext(
(state) => state.templateLanguage
);

if (variableKeys.length === 0) {
let templateSyntax = "";
switch (templateLanguage) {
Expand Down Expand Up @@ -59,7 +54,7 @@ export function PlaygroundInput() {
// change rapidly for a given variable
key={i}
label={variableKey}
value={variables[variableKey]}
value={variablesMap[variableKey]}
onChange={(value) => setVariableValue(variableKey, value)}
/>
);
Expand Down
9 changes: 3 additions & 6 deletions app/src/pages/playground/PlaygroundOutput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,7 @@ import { useCredentialsContext } from "@phoenix/contexts/CredentialsContext";
import { usePlaygroundContext } from "@phoenix/contexts/PlaygroundContext";
import { useChatMessageStyles } from "@phoenix/hooks/useChatMessageStyles";
import type { ToolCall } from "@phoenix/store";
import {
ChatMessage,
generateMessageId,
selectDerivedInputVariables,
} from "@phoenix/store";
import { ChatMessage, generateMessageId } from "@phoenix/store";
import { assertUnreachable } from "@phoenix/typeUtils";

import {
Expand All @@ -26,6 +22,7 @@ import {
import { isChatMessages } from "./playgroundUtils";
import { TitleWithAlphabeticIndex } from "./TitleWithAlphabeticIndex";
import { PlaygroundInstanceProps } from "./types";
import { useDerivedPlaygroundVariables } from "./useDerivedPlaygroundVariables";

interface PlaygroundOutputProps extends PlaygroundInstanceProps {}

Expand Down Expand Up @@ -221,7 +218,7 @@ function PlaygroundOutputText(props: PlaygroundInstanceProps) {
const templateLanguage = usePlaygroundContext(
(state) => state.templateLanguage
);
const templateVariables = usePlaygroundContext(selectDerivedInputVariables);
const { variablesMap: templateVariables } = useDerivedPlaygroundVariables();
const markPlaygroundInstanceComplete = usePlaygroundContext(
(state) => state.markPlaygroundInstanceComplete
);
Expand Down
2 changes: 1 addition & 1 deletion app/src/pages/playground/__tests__/playgroundUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const expectedPlaygroundInstanceWithIO: PlaygroundInstance = {
provider: "OPENAI",
modelName: "gpt-3.5-turbo",
},
input: { variableKeys: [], variablesValueCache: {} },
input: { variablesValueCache: {} },
tools: [],
toolChoice: "auto",
template: {
Expand Down
49 changes: 49 additions & 0 deletions app/src/pages/playground/playgroundUtils.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { getTemplateLanguageUtils } from "@phoenix/components/templateEditor/templateEditorUtils";
import { TemplateLanguage } from "@phoenix/components/templateEditor/types";
import {
DEFAULT_CHAT_ROLE,
DEFAULT_MODEL_PROVIDER,
Expand All @@ -8,6 +10,7 @@ import {
createPlaygroundInstance,
generateMessageId,
} from "@phoenix/store";
import { assertUnreachable } from "@phoenix/typeUtils";
import { safelyParseJSON } from "@phoenix/utils/jsonUtils";

import {
Expand Down Expand Up @@ -240,3 +243,49 @@ export const isChatMessages = (
): messages is ChatMessage[] => {
return chatMessagesSchema.safeParse(messages).success;
};

export const extractVariablesFromInstances = ({
instances,
templateLanguage,
}: {
instances: PlaygroundInstance[];
templateLanguage: TemplateLanguage;
}) => {
const variables = new Set<string>();
const utils = getTemplateLanguageUtils(templateLanguage);
instances.forEach((instance) => {
const instanceType = instance.template.__type;
// this double nested loop should be okay since we don't expect more than 4 instances
// and a handful of messages per instance
switch (instanceType) {
case "chat": {
// for each chat message in the instance
instance.template.messages.forEach((message) => {
// extract variables from the message content
const extractedVariables =
message.content == null
? []
: utils.extractVariables(message.content);
extractedVariables.forEach((variable) => {
variables.add(variable);
});
});
break;
}
case "text_completion": {
const extractedVariables = utils.extractVariables(
instance.template.prompt
);
extractedVariables.forEach((variable) => {
variables.add(variable);
});
break;
}
default: {
assertUnreachable(instanceType);
}
}
});

return Array.from(variables);
};
49 changes: 49 additions & 0 deletions app/src/pages/playground/useDerivedPlaygroundVariables.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { useMemo } from "react";

import { usePlaygroundContext } from "@phoenix/contexts/PlaygroundContext";
import { isManualInput } from "@phoenix/store";

import { extractVariablesFromInstances } from "./playgroundUtils";

/**
* Get the variable keys from all instances in the playground, using the
* template language to determine the syntax of the variables
*/
export const useDerivedPlaygroundVariableKeys = () => {
const { instances, templateLanguage } = usePlaygroundContext((state) => ({
instances: state.instances,
templateLanguage: state.templateLanguage,
}));
const variableKeys = useMemo(() => {
return extractVariablesFromInstances({
instances,
templateLanguage,
});
}, [instances, templateLanguage]);

return variableKeys;
};

/**
* Get the variable values and keys from all instances in the playground
*
* Variables are recomputed whenever _anything_ in the playground instances change
* or when the template language changes. This can be optimized in the future.
*/
export const useDerivedPlaygroundVariables = () => {
const variableKeys = useDerivedPlaygroundVariableKeys();
const variableValueCache = usePlaygroundContext((state) =>
isManualInput(state.input) ? state.input.variablesValueCache : {}
);
const variablesMap = useMemo(() => {
return variableKeys.reduce(
(acc, key) => {
acc[key] = variableValueCache[key] || "";
return acc;
},
{} as Record<string, string>
);
}, [variableKeys, variableValueCache]);

return { variableKeys, variablesMap };
};
90 changes: 5 additions & 85 deletions app/src/store/playground/playgroundStore.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,11 @@ import { create, StateCreator } from "zustand";
import { devtools } from "zustand/middleware";

import { TemplateLanguages } from "@phoenix/components/templateEditor/constants";
import { getTemplateLanguageUtils } from "@phoenix/components/templateEditor/templateEditorUtils";
import { TemplateLanguage } from "@phoenix/components/templateEditor/types";
import {
DEFAULT_CHAT_ROLE,
DEFAULT_MODEL_PROVIDER,
} from "@phoenix/constants/generativeConstants";
import { assertUnreachable } from "@phoenix/typeUtils";

import {
GenAIOperationType,
Expand Down Expand Up @@ -100,7 +98,7 @@ export function createPlaygroundInstance(): PlaygroundInstance {
// Default to auto tool choice as you are probably testing the LLM for it's ability to pick
toolChoice: "auto",
// TODO(apowell) - use datasetId if in dataset mode
input: { variablesValueCache: {}, variableKeys: [] },
input: { variablesValueCache: {} },
output: undefined,
activeRunId: null,
isRunning: false,
Expand Down Expand Up @@ -135,13 +133,10 @@ export const createPlaygroundStore = (
operationType: "chat",
inputMode: "manual",
input: {
// to get a record of visible variables and their values,
// use usePlaygroundContext(selectDerivedInputVariables). do not render variablesValueCache
// directly or users will see stale values.
variablesValueCache: {
question: "",
},
variableKeys: ["question"],
// variablesValueCache is used to store the values of variables for the
// manual input mode. It is indexed by the variable key. It keeps old
// values when variables are removed so that they can be restored.
variablesValueCache: {},
},
templateLanguage: TemplateLanguages.Mustache,
setInputMode: (inputMode: PlaygroundInputMode) => set({ inputMode }),
Expand Down Expand Up @@ -252,7 +247,6 @@ export const createPlaygroundStore = (
return instance;
}),
});
get().calculateVariables();
},
runPlaygroundInstances: () => {
const instances = get().instances;
Expand Down Expand Up @@ -295,49 +289,6 @@ export const createPlaygroundStore = (
},
setTemplateLanguage: (templateLanguage: TemplateLanguage) => {
set({ templateLanguage });
// Re-compute variables when the template language changes
get().calculateVariables();
},
calculateVariables: () => {
const instances = get().instances;
const variables = new Set<string>();
const utils = getTemplateLanguageUtils(get().templateLanguage);
instances.forEach((instance) => {
const instanceType = instance.template.__type;
// this double nested loop should be okay since we don't expect more than 4 instances
// and a handful of messages per instance
switch (instanceType) {
case "chat": {
// for each chat message in the instance
instance.template.messages.forEach((message) => {
// extract variables from the message content
const extractedVariables =
message.content == null
? []
: utils.extractVariables(message.content);
extractedVariables.forEach((variable) => {
variables.add(variable);
});
});
break;
}
case "text_completion": {
const extractedVariables = utils.extractVariables(
instance.template.prompt
);
extractedVariables.forEach((variable) => {
variables.add(variable);
});
break;
}
default: {
assertUnreachable(instanceType);
}
}
});
set({
input: { ...get().input, variableKeys: [...Array.from(variables)] },
});
},
setVariableValue: (key: string, value: string) => {
const input = get().input;
Expand All @@ -356,34 +307,3 @@ export const createPlaygroundStore = (
};

export type PlaygroundStore = ReturnType<typeof createPlaygroundStore>;

/**
* Selects the variable keys from the playground state
* @param state the playground state
* @returns the variable keys
*/
export const selectInputVariableKeys = (state: PlaygroundState) => {
if (isManualInput(state.input)) {
return state.input.variableKeys;
}
return [];
};

/**
* Selects the derived input variables from the playground state
* @param state the playground state
* @returns the derived input variables
*/
export const selectDerivedInputVariables = (state: PlaygroundState) => {
if (isManualInput(state.input)) {
const input = state.input;
const variableKeys = input.variableKeys;
const variablesValueCache = input.variablesValueCache;
const valueMap: Record<string, string> = {};
variableKeys.forEach((key) => {
valueMap[key] = variablesValueCache?.[key] || "";
});
return valueMap;
}
return {};
};
7 changes: 2 additions & 5 deletions app/src/store/playground/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ type DatasetInput = {

type ManualInput = {
variablesValueCache: Record<string, string | undefined>;
variableKeys: string[];
};

type PlaygroundInput = DatasetInput | ManualInput;
Expand Down Expand Up @@ -231,18 +230,16 @@ export interface PlaygroundState extends PlaygroundProps {
*/
setTemplateLanguage: (templateLanguage: TemplateLanguage) => void;
/**
* Calculate the variables used across all instances
* Set the value of a variable in the input
*/
calculateVariables: () => void;

setVariableValue: (key: string, value: string) => void;
}

/**
* Check if the input is manual
*/
export const isManualInput = (input: PlaygroundInput): input is ManualInput => {
return "variablesValueCache" in input && "variableKeys" in input;
return "variablesValueCache" in input;
};

/**
Expand Down
Loading