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

[editor] Use Prompt ID Instead of Index for Client-side State Actions #666

Merged
merged 3 commits into from
Dec 29, 2023
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
20 changes: 19 additions & 1 deletion python/src/aiconfig/editor/client/src/Editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import EditorContainer, {
AIConfigCallbacks,
} from "./components/EditorContainer";
import { Flex, Loader, MantineProvider } from "@mantine/core";
import { AIConfig, Prompt } from "aiconfig";
import { AIConfig, ModelMetadata, Prompt } from "aiconfig";
import { useCallback, useEffect, useMemo, useState } from "react";
import { ufetch } from "ufetch";
import { ROUTE_TABLE } from "./utils/api";
Expand Down Expand Up @@ -50,6 +50,12 @@ export default function Editor() {
[]
);

const deletePrompt = useCallback(async (promptName: string) => {
return await ufetch.post(ROUTE_TABLE.DELETE_PROMPT, {
prompt_name: promptName,
});
}, []);

const runPrompt = useCallback(async (promptName: string) => {
return await ufetch.post(ROUTE_TABLE.RUN_PROMPT, {
prompt_name: promptName,
Expand All @@ -66,12 +72,24 @@ export default function Editor() {
[]
);

const updateModel = useCallback(
async (_promptName?: string, _modelData?: string | ModelMetadata) => {
// return await ufetch.post(ROUTE_TABLE.UPDATE_MODEL,
// prompt_name: promptName,
// model_data: modelData,
// });
},
[]
);

const callbacks: AIConfigCallbacks = useMemo(
() => ({
addPrompt,
deletePrompt,
getModels,
runPrompt,
save,
updateModel,
updatePrompt,
}),
[save, getModels, addPrompt, runPrompt]
Expand Down
145 changes: 116 additions & 29 deletions python/src/aiconfig/editor/client/src/components/EditorContainer.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
import PromptContainer from "./prompt/PromptContainer";
import { Container, Group, Button, createStyles, Stack } from "@mantine/core";
import {
Container,
Group,
Button,
createStyles,
Stack,
Flex,
} from "@mantine/core";
import { showNotification } from "@mantine/notifications";
import { AIConfig, Prompt, PromptInput } from "aiconfig";
import { AIConfig, ModelMetadata, Prompt, PromptInput } from "aiconfig";
import { useCallback, useMemo, useReducer, useRef, useState } from "react";
import aiconfigReducer, { AIConfigReducerAction } from "./aiconfigReducer";
import {
Expand All @@ -11,8 +18,12 @@ import {
clientPromptToAIConfigPrompt,
} from "../shared/types";
import AddPromptButton from "./prompt/AddPromptButton";
import { getDefaultNewPromptName } from "../utils/aiconfigStateUtils";
import {
getDefaultNewPromptName,
getPrompt,
} from "../utils/aiconfigStateUtils";
import { debounce, uniqueId } from "lodash";
import PromptMenuButton from "./prompt/PromptMenuButton";

type Props = {
aiconfig: AIConfig;
Expand All @@ -25,9 +36,14 @@ export type AIConfigCallbacks = {
prompt: Prompt,
index: number
) => Promise<{ aiconfig: AIConfig }>;
deletePrompt: (promptName: string) => Promise<void>;
getModels: (search: string) => Promise<string[]>;
runPrompt: (promptName: string) => Promise<void>;
save: (aiconfig: AIConfig) => Promise<void>;
updateModel: (
promptName?: string,
modelData?: string | ModelMetadata
) => Promise<void /*{ aiconfig: AIConfig }*/>;
updatePrompt: (
promptName: string,
promptData: Prompt
Expand Down Expand Up @@ -80,7 +96,7 @@ export default function EditorContainer({
const onSave = useCallback(async () => {
setIsSaving(true);
try {
await callbacks.save(clientConfigToAIConfig(aiconfigState));
await callbacks.save(clientConfigToAIConfig(stateRef.current));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why wouldn't we want to re-render if client aiconfig changes?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will always re-render if aiconfigState changes. This stateRef is a stable ref to use in the callbacks since otherwise using aiconfigState in the callback means the callback logic is actually using the state from when the callback was constructed & memoized (which could technically be stale)

Copy link
Contributor

@rossdanlm rossdanlm Dec 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok this makes sense! Why do we need to update for this diff? Is it because the index before was always guaranteed to be correct as the index --> ui mapping?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't necessarily know if it had to be update here, but it was a small enough change and related to using proper source of truth (like id instead of index)

} catch (err: any) {
showNotification({
title: "Error saving",
Expand All @@ -90,7 +106,7 @@ export default function EditorContainer({
} finally {
setIsSaving(false);
}
}, [aiconfigState, callbacks.save]);
}, [callbacks.save]);

const debouncedUpdatePrompt = useMemo(
() =>
Expand All @@ -103,18 +119,18 @@ export default function EditorContainer({
);

const onChangePromptInput = useCallback(
async (promptIndex: number, newPromptInput: PromptInput) => {
async (promptId: string, newPromptInput: PromptInput) => {
const action: AIConfigReducerAction = {
type: "UPDATE_PROMPT_INPUT",
index: promptIndex,
id: promptId,
input: newPromptInput,
};

dispatch(action);

try {
const prompt = clientPromptToAIConfigPrompt(
aiconfigState.prompts[promptIndex]
getPrompt(stateRef.current, promptId)!
);
const serverConfigRes = await debouncedUpdatePrompt(prompt.name, {
...prompt,
Expand All @@ -138,10 +154,10 @@ export default function EditorContainer({
);

const onChangePromptName = useCallback(
async (promptIndex: number, newName: string) => {
async (promptId: string, newName: string) => {
const action: AIConfigReducerAction = {
type: "UPDATE_PROMPT_NAME",
index: promptIndex,
id: promptId,
name: newName,
};

Expand All @@ -150,23 +166,68 @@ export default function EditorContainer({
[dispatch]
);

const debouncedUpdateModel = useMemo(
() =>
debounce(
(promptName?: string, modelMetadata?: string | ModelMetadata) =>
callbacks.updateModel(promptName, modelMetadata),
250
),
[callbacks.updateModel]
);

const onUpdatePromptModelSettings = useCallback(
async (promptIndex: number, newModelSettings: any) => {
async (promptId: string, newModelSettings: any) => {
dispatch({
type: "UPDATE_PROMPT_MODEL_SETTINGS",
index: promptIndex,
id: promptId,
modelSettings: newModelSettings,
});
// TODO: Call server-side endpoint to update model settings
// TODO: Call server-side endpoint to update model
},
[dispatch]
);

const onUpdatePromptModel = useCallback(
async (promptId: string, newModel?: string) => {
dispatch({
type: "UPDATE_PROMPT_MODEL",
id: promptId,
modelName: newModel,
});

try {
const prompt = clientPromptToAIConfigPrompt(
getPrompt(stateRef.current, promptId)!
);
const currentModel = prompt.metadata?.model;
let modelData: string | ModelMetadata | undefined = newModel;
if (newModel && currentModel && typeof currentModel !== "string") {
modelData = {
...currentModel,
name: newModel,
};
}

await debouncedUpdateModel(prompt.name, modelData);

// TODO: Consolidate
} catch (err: any) {
showNotification({
title: "Error updating prompt model",
message: err.message,
color: "red",
});
}
},
[dispatch, debouncedUpdateModel]
);

const onUpdatePromptParameters = useCallback(
async (promptIndex: number, newParameters: any) => {
async (promptId: string, newParameters: any) => {
dispatch({
type: "UPDATE_PROMPT_PARAMETERS",
index: promptIndex,
id: promptId,
parameters: newParameters,
});
// TODO: Call server-side endpoint to update prompt parameters
Expand Down Expand Up @@ -223,9 +284,30 @@ export default function EditorContainer({
[callbacks.addPrompt, dispatch]
);

const onDeletePrompt = useCallback(
async (promptId: string) => {
dispatch({
type: "DELETE_PROMPT",
id: promptId,
});

try {
const prompt = getPrompt(stateRef.current, promptId)!;
await callbacks.deletePrompt(prompt.name);
} catch (err: any) {
showNotification({
title: "Error deleting prompt",
message: err.message,
color: "red",
});
}
},
[callbacks.deletePrompt, dispatch]
);

const onRunPrompt = useCallback(
async (promptIndex: number) => {
const promptName = aiconfigState.prompts[promptIndex].name;
async (promptId: string) => {
const promptName = getPrompt(stateRef.current, promptId)!.name;
try {
await callbacks.runPrompt(promptName);
} catch (err: any) {
Expand All @@ -241,8 +323,6 @@ export default function EditorContainer({

const { classes } = useStyles();

// TODO: Implement editor context for callbacks, readonly state, etc.

return (
<>
<Container maw="80rem">
Expand All @@ -259,16 +339,23 @@ export default function EditorContainer({
{aiconfigState.prompts.map((prompt: ClientPrompt, i: number) => {
return (
<Stack key={prompt._ui.id}>
<PromptContainer
index={i}
prompt={prompt}
onChangePromptInput={onChangePromptInput}
onChangePromptName={onChangePromptName}
onRunPrompt={onRunPrompt}
onUpdateModelSettings={onUpdatePromptModelSettings}
onUpdateParameters={onUpdatePromptParameters}
defaultConfigModelName={aiconfigState.metadata.default_model}
/>
<Flex mt="md">
<PromptMenuButton
promptId={prompt._ui.id}
onDeletePrompt={() => onDeletePrompt(prompt._ui.id)}
/>
<PromptContainer
prompt={prompt}
getModels={callbacks.getModels}
onChangePromptInput={onChangePromptInput}
onChangePromptName={onChangePromptName}
onRunPrompt={onRunPrompt}
onUpdateModel={onUpdatePromptModel}
onUpdateModelSettings={onUpdatePromptModelSettings}
onUpdateParameters={onUpdatePromptParameters}
defaultConfigModelName={aiconfigState.metadata.default_model}
/>
</Flex>
<div className={classes.addPromptRow}>
<AddPromptButton
getModels={callbacks.getModels}
Expand Down
Loading