Skip to content

Commit

Permalink
feat(prompts): Display dirty state on playground instance (#5961)
Browse files Browse the repository at this point in the history
* feat(prompts): Derive dirty state from playground instance

* Refactor dirty checking into hook

* Clear prompt on compare

* Manually track dirty state via store actions

* Keep prompt selected when cloning instance

* Fix types
  • Loading branch information
cephalization authored Jan 9, 2025
1 parent ef5ce56 commit b34c106
Show file tree
Hide file tree
Showing 13 changed files with 80 additions and 20 deletions.
1 change: 1 addition & 0 deletions app/src/pages/playground/ChatMessageToolCallsEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ export function ChatMessageToolCallsEditor({
),
},
},
dirty: true,
});
},
[messageId, playgroundInstanceId, templateMessages, updateInstance]
Expand Down
19 changes: 9 additions & 10 deletions app/src/pages/playground/ModelSupportedParamsFetcher.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,13 @@ export const ModelSupportedParamsFetcher = ({
}: {
instanceId: number;
}) => {
const modelProvider = usePlaygroundContext(
(state) =>
state.instances.find((instance) => instance.id === instanceId)?.model
.provider
);
const modelName = usePlaygroundContext(
(state) =>
state.instances.find((instance) => instance.id === instanceId)?.model
.modelName
);
const instances = usePlaygroundContext((state) => state.instances);
const instance = instances.find((instance) => instance.id === instanceId);
if (!instance) {
throw new Error("Instance not found");
}
const modelProvider = instance.model.provider;
const modelName = instance.model.modelName;
const modelConfigByProvider = usePreferencesContext(
(state) => state.modelConfigByProvider
);
Expand Down Expand Up @@ -88,6 +85,7 @@ export const ModelSupportedParamsFetcher = ({
},
}
);
const promptId = instance.prompt?.id;
useEffect(() => {
updateModelSupportedInvocationParameters({
instanceId,
Expand All @@ -100,6 +98,7 @@ export const ModelSupportedParamsFetcher = ({
instanceId,
updateModelSupportedInvocationParameters,
modelConfigByProvider,
promptId,
]);
return null;
};
4 changes: 4 additions & 0 deletions app/src/pages/playground/PlaygroundChatTemplate.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ export function PlaygroundChatTemplate(props: PlaygroundChatTemplateProps) {
messages: newMessages,
},
},
dirty: true,
});
}}
>
Expand Down Expand Up @@ -343,6 +344,7 @@ function SortableMessageItem({
),
},
},
dirty: true,
});
},
[message.id, playgroundInstanceId, template.messages, updateInstance]
Expand Down Expand Up @@ -396,6 +398,7 @@ function SortableMessageItem({
),
},
},
dirty: true,
});
}}
/>
Expand Down Expand Up @@ -464,6 +467,7 @@ function SortableMessageItem({
),
},
},
dirty: true,
});
}}
/>
Expand Down
2 changes: 2 additions & 0 deletions app/src/pages/playground/PlaygroundChatTemplateFooter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ export function PlaygroundChatTemplateFooter({
}),
],
},
dirty: true,
});
}}
>
Expand All @@ -143,6 +144,7 @@ export function PlaygroundChatTemplateFooter({
],
},
},
dirty: true,
});
}}
>
Expand Down
4 changes: 4 additions & 0 deletions app/src/pages/playground/PlaygroundDatasetExamplesTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,7 @@ export function PlaygroundDatasetExamplesTable({
updateInstance({
instanceId,
patch: { experimentId: chatCompletion.experiment.id },
dirty: null,
});
break;
case "ChatCompletionSubscriptionResult":
Expand Down Expand Up @@ -552,6 +553,7 @@ export function PlaygroundDatasetExamplesTable({
patch: {
experimentId: response.chatCompletionOverDataset.experimentId,
},
dirty: null,
});
setExampleDataForInstance({
instanceId,
Expand Down Expand Up @@ -581,6 +583,7 @@ export function PlaygroundDatasetExamplesTable({
updateInstance({
instanceId: instance.id,
patch: { experimentId: null },
dirty: null,
});
if (activeRunId === null) {
continue;
Expand Down Expand Up @@ -638,6 +641,7 @@ export function PlaygroundDatasetExamplesTable({
updateInstance({
instanceId: instance.id,
patch: { experimentId: null },
dirty: null,
});
const variables = {
input: getChatCompletionOverDatasetInput({
Expand Down
3 changes: 3 additions & 0 deletions app/src/pages/playground/PlaygroundOutput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ export function PlaygroundOutput(props: PlaygroundOutputProps) {
patch: {
spanId: chatCompletion.span.id,
},
dirty: null,
});
return;
}
Expand Down Expand Up @@ -279,6 +280,7 @@ export function PlaygroundOutput(props: PlaygroundOutputProps) {
patch: {
spanId: response.chatCompletion.span.id,
},
dirty: null,
});
if (errors) {
notifyError({
Expand Down Expand Up @@ -315,6 +317,7 @@ export function PlaygroundOutput(props: PlaygroundOutputProps) {
patch: {
spanId: null,
},
dirty: null,
});
}, [instanceId, updateInstance]);

Expand Down
1 change: 1 addition & 0 deletions app/src/pages/playground/PlaygroundOutputMoveButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ export const PlaygroundOutputMoveButton = ({
messages: [...instance.template.messages, ...messages],
},
},
dirty: true,
});
cleanupOutput();
}}
Expand Down
21 changes: 12 additions & 9 deletions app/src/pages/playground/PlaygroundTemplate.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,13 @@ export function PlaygroundTemplate(props: PlaygroundTemplateProps) {
const index = instances.findIndex((instance) => instance.id === instanceId);
const prompt = instance?.prompt;
const promptId = prompt?.id;
const dirty = instance?.dirty;

const onChangePrompt = useCallback(
async (promptId: string | null) => {
if (!promptId) {
updateInstance({
instanceId,
patch: {
prompt: null,
},
});
const patch = { prompt: null };
updateInstance({ instanceId, patch, dirty: false });
return;
}

Expand All @@ -52,6 +49,7 @@ export function PlaygroundTemplate(props: PlaygroundTemplateProps) {
patch: {
...response.instance,
},
dirty: false,
});
}
},
Expand Down Expand Up @@ -94,7 +92,11 @@ export function PlaygroundTemplate(props: PlaygroundTemplateProps) {
<ModelSupportedParamsFetcher instanceId={instanceId} />
</Suspense>
<ModelConfigButton {...props} />
<SaveButton instanceId={instanceId} setDialog={setDialog} />
<SaveButton
instanceId={instanceId}
setDialog={setDialog}
dirty={dirty}
/>
{instances.length > 1 ? <DeleteButton {...props} /> : null}
</Flex>
}
Expand Down Expand Up @@ -143,9 +145,10 @@ function DeleteButton(props: PlaygroundInstanceProps) {
type SaveButtonProps = {
instanceId: number;
setDialog: (dialog: React.ReactNode) => void;
dirty?: boolean;
};

function SaveButton({ instanceId, setDialog }: SaveButtonProps) {
function SaveButton({ instanceId, setDialog, dirty }: SaveButtonProps) {
const instance = usePlaygroundContext((state) =>
state.instances.find((instance) => instance.id === instanceId)
);
Expand All @@ -168,7 +171,7 @@ function SaveButton({ instanceId, setDialog }: SaveButtonProps) {
<TooltipTrigger delay={100}>
<TriggerWrap>
<Button
// TODO(apowell): Make variant "primary" when instance is "dirty", aka different from selected prompt
variant={dirty ? "primary" : undefined}
size="S"
onPress={onSave}
>
Expand Down
2 changes: 2 additions & 0 deletions app/src/pages/playground/PlaygroundTool.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ export function PlaygroundTool({
: t
),
},
dirty: true,
});
},
[instanceTools, playgroundInstanceId, tool.id, updateInstance]
Expand Down Expand Up @@ -165,6 +166,7 @@ export function PlaygroundTool({
tools: newTools,
toolChoice,
},
dirty: true,
});
}}
/>
Expand Down
1 change: 1 addition & 0 deletions app/src/pages/playground/PlaygroundTools.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ export function PlaygroundTools(props: PlaygroundToolsProps) {
patch: {
toolChoice: choice,
},
dirty: true,
});
}}
toolNames={toolNames}
Expand Down
3 changes: 3 additions & 0 deletions app/src/pages/playground/__tests__/playgroundUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ const baseTestPlaygroundInstance: PlaygroundInstance = {
tools: [],
toolChoice: "auto",
spanId: null,
dirty: false,
template: {
__type: "chat",
messages: [],
Expand All @@ -86,6 +87,7 @@ const expectedPlaygroundInstanceWithIO: PlaygroundInstance = {
tools: [],
toolChoice: "auto",
spanId: "fake-id",
dirty: false,
template: {
__type: "chat",
// These id's are not 0, 1, 2, because we create a playground instance (including messages) at the top of the transformSpanAttributesToPlaygroundInstance function
Expand Down Expand Up @@ -994,6 +996,7 @@ describe("getVariablesMapFromInstances", () => {
tools: [],
toolChoice: "auto",
spanId: null,
dirty: false,
template: {
__type: "chat",
messages: [],
Expand Down
25 changes: 24 additions & 1 deletion app/src/store/playground/playgroundStore.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ export const DEFAULT_INSTANCE_PARAMS = () =>
output: undefined,
spanId: null,
activeRunId: null,
dirty: false,
}) satisfies Partial<PlaygroundInstance>;

export function createPlaygroundInstance(): PlaygroundInstance {
Expand Down Expand Up @@ -313,6 +314,7 @@ export const createPlaygroundStore = (initialProps: InitialPlaygroundState) => {
return {
...instance,
...patch,
dirty: true,
};
}
return instance;
Expand All @@ -330,6 +332,7 @@ export const createPlaygroundStore = (initialProps: InitialPlaygroundState) => {
if (instance.id === instanceId) {
return {
...instance,
dirty: true,
model: {
...instance.model,
...patch,
Expand Down Expand Up @@ -359,6 +362,7 @@ export const createPlaygroundStore = (initialProps: InitialPlaygroundState) => {
) {
return {
...instance,
dirty: true,
messages: [
...instance.template.messages,
{ role: DEFAULT_CHAT_ROLE, content: "{question}" },
Expand All @@ -369,14 +373,15 @@ export const createPlaygroundStore = (initialProps: InitialPlaygroundState) => {
}),
});
},
updateInstance: ({ instanceId, patch }) => {
updateInstance: ({ instanceId, patch, dirty }) => {
const instances = get().instances;
set({
instances: instances.map((instance) => {
if (instance.id === instanceId) {
return {
...instance,
...patch,
...(dirty != undefined ? { dirty } : {}),
};
}
return instance;
Expand Down Expand Up @@ -444,6 +449,7 @@ export const createPlaygroundStore = (initialProps: InitialPlaygroundState) => {
if (instance.id === instanceId) {
return {
...instance,
dirty: true,
model: { ...instance.model, invocationParameters },
};
}
Expand All @@ -470,6 +476,7 @@ export const createPlaygroundStore = (initialProps: InitialPlaygroundState) => {
if (instance.id === instanceId) {
return {
...instance,
dirty: true,
model: {
...instance.model,
invocationParameters: instance.model.invocationParameters.map(
Expand All @@ -490,6 +497,7 @@ export const createPlaygroundStore = (initialProps: InitialPlaygroundState) => {
if (instance.id === instanceId) {
return {
...instance,
dirty: true,
model: {
...instance.model,
invocationParameters: [
Expand Down Expand Up @@ -517,6 +525,7 @@ export const createPlaygroundStore = (initialProps: InitialPlaygroundState) => {
if (instance.id === instanceId) {
return {
...instance,
dirty: true,
model: {
...instance.model,
invocationParameters:
Expand All @@ -532,6 +541,20 @@ export const createPlaygroundStore = (initialProps: InitialPlaygroundState) => {
}),
});
},
setDirty: (instanceId: number, dirty: boolean) => {
const instances = get().instances;
set({
instances: instances.map((instance) => {
if (instance.id === instanceId) {
return {
...instance,
dirty,
};
}
return instance;
}),
});
},
...initialProps,
});
return create(devtools(playgroundStore));
Expand Down
Loading

0 comments on commit b34c106

Please sign in to comment.