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.

Sign up for GitHub

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

Feat: Add in ability to delete a single output by prompt name in editor #1465

Merged
merged 6 commits into from
Mar 19, 2024
Merged

Feat: Add in ability to delete a single output by prompt name in editor #1465

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0c0f78d
Feat: Add in ability to delete a single output by prompt name in editor
justinkchen Mar 17, 2024
c7c2d17
Update to move delete output button to context menu
justinkchen Mar 17, 2024
c958a1d
Fix client to pass in prompt name
justinkchen Mar 17, 2024
8b4b559
Update styling
justinkchen Mar 17, 2024
cf0cf99
Update client changes and server code changes based on PR comments fo…
justinkchen Mar 19, 2024
c299c4b
Style tweaks
justinkchen Mar 19, 2024
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
2 changes: 1 addition & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,4 +141,4 @@ dist
__pycache__/
models
*.egg-info
.hypothesis
.hypothesis
7 changes: 7 additions & 0 deletions python/src/aiconfig/editor/client/src/LocalEditor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,12 @@ export default function LocalEditor() {
return await ufetch.post(ROUTE_TABLE.CLEAR_OUTPUTS, {});
}, []);

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

const runPrompt = useCallback(
async (
promptName: string,
Expand Down Expand Up @@ -249,6 +255,7 @@ export default function LocalEditor() {
addPrompt,
cancel,
clearOutputs,
deleteOutput,
deleteModelSettings,
deletePrompt,
getModels,
Expand Down
35 changes: 35 additions & 0 deletions python/src/aiconfig/editor/client/src/components/AIConfigEditor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ export type AIConfigCallbacks = {
) => Promise<{ aiconfig: AIConfig }>;
cancel: (cancellationToken: string) => Promise<void>;
clearOutputs: () => Promise<{ aiconfig: AIConfig }>;
deleteOutput: (promptName: string) => Promise<{ aiconfig: AIConfig }>;
deleteModelSettings?: (modelName: string) => Promise<void>;
deletePrompt: (promptName: string) => Promise<void>;
download?: () => Promise<void>;
Expand Down Expand Up @@ -811,6 +812,39 @@ function AIConfigEditorBase({
[deletePromptCallback, dispatch, logEventHandler, showNotification]
);

const deleteOutputCallback = callbacks?.deleteOutput;
const onDeleteOutput = useCallback(
async (promptId: string) => {
if (!deleteOutputCallback) {
// Just no-op if no callback specified. We could technically perform
// client-side updates but that might be confusing
return;
}

dispatch({
type: "DELETE_OUTPUT",
id: promptId,
});
logEventHandler?.("DELETE_OUTPUT");

try {
const prompt = getPrompt(stateRef.current, promptId);
if (!prompt) {
throw new Error(`Could not find prompt with id ${promptId}`);
}
await deleteOutputCallback(prompt.name);
} catch (err: unknown) {
const message = (err as RequestCallbackError).message ?? null;
showNotification({
title: "Error deleting output for prompt",
message,
type: "error",
});
}
},
[deleteOutputCallback, dispatch, logEventHandler, showNotification]
);

const clearOutputsCallback = callbacks?.clearOutputs;
const onClearOutputs = useCallback(async () => {
if (!clearOutputsCallback) {
Expand Down Expand Up @@ -1236,6 +1270,7 @@ function AIConfigEditorBase({
onChangePromptInput={onChangePromptInput}
onChangePromptName={onChangePromptName}
onDeletePrompt={onDeletePrompt}
onDeleteOutput={onDeleteOutput}
onRunPrompt={onRunPrompt}
onUpdatePromptMetadata={onUpdatePromptMetadata}
onUpdatePromptModel={onUpdatePromptModel}
Expand Down
126 changes: 77 additions & 49 deletions python/src/aiconfig/editor/client/src/components/prompt/PromptContainer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@ import { ClientPrompt } from "../../shared/types";
import { getPromptSchema } from "../../utils/promptUtils";
import { Flex, Card, createStyles } from "@mantine/core";
import { PromptInput as AIConfigPromptInput, JSONObject } from "aiconfig";
import { memo, useCallback, useEffect, useMemo, useRef } from "react";
import { memo, useCallback, useContext, useEffect, useMemo, useRef } from "react";
import PromptOutputBar from "./PromptOutputBar";
import PromptName from "./PromptName";
import ModelSelector from "./ModelSelector";
import { DEBOUNCE_MS } from "../../utils/constants";
import { debounce } from "lodash";
import PromptMenuButton from "./PromptMenuButton";
import AIConfigContext from "../../contexts/AIConfigContext";

type Props = {
prompt: ClientPrompt;
Expand All @@ -22,6 +24,8 @@ type Props = {
) => void;
onChangePromptName: (promptId: string, newName: string) => void;
onRunPrompt(promptId: string): Promise<void>;
onDeletePrompt(promptId: string): void;
onDeleteOutput(promptId: string): void;
onUpdateModel: (promptId: string, newModel?: string) => void;
onUpdateModelSettings: (
promptId: string,
Expand Down Expand Up @@ -65,13 +69,16 @@ export default memo(function PromptContainer({
onChangePromptName,
defaultConfigModelName,
onRunPrompt,
onDeletePrompt,
onDeleteOutput,
onUpdateModel,
onUpdateModelSettings,
onUpdateParameters,
onUpdatePromptMetadata,
isRunButtonDisabled = false,
}: Props) {
const { classes } = useStyles();
const { readOnly } = useContext(AIConfigContext);
const promptId = prompt._ui.id;
const onChangeInput = useCallback(
(newInput: AIConfigPromptInput) => onChangePromptInput(promptId, newInput),
Expand Down Expand Up @@ -104,6 +111,16 @@ export default memo(function PromptContainer({
[promptId, onRunPrompt]
);

const deletePrompt = useCallback(
async () => await onDeletePrompt(promptId),
[promptId, onDeletePrompt]
);

const deleteOutput = useCallback(
async () => await onDeleteOutput(promptId),
[promptId, onDeleteOutput]
);

const onCancelRun = useCallback(async () => {
if (!cancel) {
return;
Expand Down Expand Up @@ -158,54 +175,65 @@ export default memo(function PromptContainer({
const inputSchema = promptSchema?.input;

return (
<Flex justify="space-between" w="100%">
<Card
withBorder
className={`${classes.cellStyle} cellStyle`}
ref={cellInputOutputRef}
>
<Flex direction="column">
<Flex justify="space-between" mb="0.5em">
<PromptName
promptId={promptId}
name={prompt.name}
onUpdate={onChangeName}
/>
<ModelSelector
getModels={getModels}
prompt={prompt}
onSetModel={updateModel}
defaultConfigModelName={defaultConfigModelName}
/>
</Flex>
<PromptInputRenderer
input={prompt.input}
schema={inputSchema}
onChangeInput={onChangeInput}
onCancelRun={onCancelRun}
onRunPrompt={runPrompt}
isRunning={prompt._ui.isRunning}
isRunButtonDisabled={isRunButtonDisabled}
/>

{prompt.outputs && prompt.outputs.length > 0 && (
<>
<PromptOutputBar />
<PromptOutputsRenderer outputs={prompt.outputs} />
</>
)}
</Flex>
</Card>
<div className={`${classes.sidePanel} sidePanel`}>
<PromptActionBar
defaultConfigModelName={defaultConfigModelName}
prompt={prompt}
promptSchema={promptSchema}
onUpdateModelSettings={updateModelSettings}
onUpdateParameters={updateParameters}
onUpdatePromptMetadata={updatePromptMetadata}
<>
{!readOnly && (
<PromptMenuButton
showDeleteOutput={!!prompt.outputs?.length}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: feel prompt.outputs?.length > 0 is easier to read and I don't really like JS implicitly "falsey-ness" but don't really care

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it actually complains if you use this syntax unelss you do a (prompt.outputs?.length || 0) > 0 it seems, so thats why i kept it with this way

onDeletePrompt={deletePrompt}
onDeleteOutput={deleteOutput}
/>
</div>
</Flex>
)}
<Flex justify="space-between" w="100%">
<Card
withBorder
className={`${classes.cellStyle} cellStyle`}
ref={cellInputOutputRef}
>
<Flex direction="column">
<Flex justify="space-between" mb="0.5em">
<PromptName
promptId={promptId}
name={prompt.name}
onUpdate={onChangeName}
/>
<ModelSelector
getModels={getModels}
prompt={prompt}
onSetModel={updateModel}
defaultConfigModelName={defaultConfigModelName}
/>
</Flex>
<PromptInputRenderer
input={prompt.input}
schema={inputSchema}
onChangeInput={onChangeInput}
onCancelRun={onCancelRun}
onRunPrompt={runPrompt}
isRunning={prompt._ui.isRunning}
isRunButtonDisabled={isRunButtonDisabled}
/>

{prompt.outputs && prompt.outputs.length > 0 && (
<>
<Flex justify="space-between" direction="column">
<PromptOutputBar />
</Flex>
<PromptOutputsRenderer outputs={prompt.outputs} />
</>
)}
</Flex>
</Card>
<div className={`${classes.sidePanel} sidePanel`}>
<PromptActionBar
defaultConfigModelName={defaultConfigModelName}
prompt={prompt}
promptSchema={promptSchema}
onUpdateModelSettings={updateModelSettings}
onUpdateParameters={updateParameters}
onUpdatePromptMetadata={updatePromptMetadata}
/>
</div>
</Flex>
</>
);
});
20 changes: 15 additions & 5 deletions python/src/aiconfig/editor/client/src/components/prompt/PromptMenuButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Button, Menu, createStyles } from "@mantine/core";
import { IconDotsVertical, IconTrash } from "@tabler/icons-react";
import { IconDotsVertical, IconEraser, IconTrash } from "@tabler/icons-react";
import { memo } from "react";

const useStyles = createStyles(() => ({
Expand All @@ -9,11 +9,13 @@ const useStyles = createStyles(() => ({
}));

export default memo(function PromptMenuButton({
promptId,
showDeleteOutput,
onDeleteOutput,
onDeletePrompt,
}: {
promptId: string;
onDeletePrompt: (id: string) => void;
showDeleteOutput: boolean;
onDeletePrompt: () => void;
onDeleteOutput: () => void;
}) {
const { classes } = useStyles();

Expand All @@ -31,10 +33,18 @@ export default memo(function PromptMenuButton({
</Menu.Target>

<Menu.Dropdown>
{showDeleteOutput ? (
<Menu.Item
icon={<IconEraser size={16} />}
onClick={onDeleteOutput}
>
Clear Output
</Menu.Item>
) : null}
<Menu.Item
icon={<IconTrash size={16} />}
color="red"
onClick={() => onDeletePrompt(promptId)}
onClick={onDeletePrompt}
>
Delete Prompt
</Menu.Item>
Expand Down
14 changes: 6 additions & 8 deletions python/src/aiconfig/editor/client/src/components/prompt/PromptsContainer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { memo, useContext } from "react";
import AIConfigContext from "../../contexts/AIConfigContext";
import AddPromptButton from "./AddPromptButton";
import { ClientPrompt } from "../../shared/types";
import PromptMenuButton from "./PromptMenuButton";
import PromptContainer from "./PromptContainer";
import { JSONObject, PromptInput } from "aiconfig";

Expand All @@ -18,6 +17,7 @@ type Props = {
) => Promise<void>;
onChangePromptName: (promptId: string, newName: string) => Promise<void>;
onDeletePrompt: (promptId: string) => Promise<void>;
onDeleteOutput: (promptId: string) => Promise<void>;
onRunPrompt: (promptId: string) => Promise<void>;
onUpdatePromptMetadata: (
promptId: string,
Expand Down Expand Up @@ -58,25 +58,23 @@ export default memo(function PromptsContainer(props: Props) {
/>
)}
{props.prompts.map((prompt: ClientPrompt, i: number) => {
const promptId = prompt._ui.id;
const isAnotherPromptRunning =
props.runningPromptId !== undefined &&
props.runningPromptId !== prompt._ui.id;
props.runningPromptId !== promptId;

return (
<Stack key={prompt._ui.id}>
<Flex mt="md">
{!readOnly && (
<PromptMenuButton
promptId={prompt._ui.id}
onDeletePrompt={() => props.onDeletePrompt(prompt._ui.id)}
/>
)}
Comment on lines -67 to -72
Copy link
Contributor

@rossdanlm rossdanlm Mar 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

omg this had me confused so hard wondering wtf was going on, but this is the PromptsContainer (with an s), not PromptContainer :S

Copy link
Contributor

Choose a reason for hiding this comment

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

I like this change, I feel it makes sense to put with the individual prompt component anyways

<PromptContainer
prompt={prompt}
getModels={props.getModels}
onChangePromptInput={props.onChangePromptInput}
onChangePromptName={props.onChangePromptName}
cancel={props.cancelRunPrompt}
onRunPrompt={props.onRunPrompt}
onDeletePrompt={props.onDeletePrompt}
onDeleteOutput={props.onDeleteOutput}
Comment on lines +76 to +77
Copy link
Contributor

Choose a reason for hiding this comment

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

nice

onUpdateModel={props.onUpdatePromptModel}
onUpdateModelSettings={props.onUpdatePromptModelSettings}
onUpdateParameters={props.onUpdatePromptParameters}
Expand Down
6 changes: 6 additions & 0 deletions python/src/aiconfig/editor/client/src/reducers/actions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export type AIConfigReducerAction =
export type MutateAIConfigAction =
| AddPromptAction
| ClearOutputsAction
| DeleteOutputAction
| DeleteGlobalModelSettingsAction
| DeletePromptAction
| ProvidedAIConfigUpdateAction
Expand Down Expand Up @@ -56,6 +57,11 @@ export type ClearOutputsAction = {
type: "CLEAR_OUTPUTS";
};

export type DeleteOutputAction = {
type: "DELETE_OUTPUT";
id: string;
}

export type DeleteGlobalModelSettingsAction = {
type: "DELETE_GLOBAL_MODEL_SETTINGS";
modelName: string;
Expand Down
16 changes: 16 additions & 0 deletions python/src/aiconfig/editor/client/src/reducers/aiconfigReducer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,22 @@ export default function aiconfigReducer(
prompts,
};
}

case "DELETE_OUTPUT": {
const clearOutput = (statePrompt: ClientPrompt) => {
return {
...statePrompt,
outputs: undefined,
};
}

return reduceReplacePrompt(
dirtyState,
action.id,
clearOutput
);
}

case "DELETE_GLOBAL_MODEL_SETTINGS": {
const newModels = { ...state.metadata.models };
delete newModels[action.modelName];
Expand Down
1 change: 1 addition & 0 deletions python/src/aiconfig/editor/client/src/shared/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ export function aiConfigToClientConfig(aiconfig: AIConfig): ClientAIConfig {
export type LogEvent =
| "ADD_PROMPT"
| "CLEAR_OUTPUTS"
| "DELETE_OUTPUT"
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Would like to use wording "CLEAR_OUTPUT" to match with our UI, but again not a blocker or a big deal

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think maybe as a bigger refactor of the api? i made it clear output at first but then found that all the apis reference delete output so wanted to stay consistent there.

| "DELETE_GLOBAL_MODEL_SETTINGS"
| "DELETE_PROMPT"
| "DOWNLOAD_BUTTON_CLICKED"
Expand Down
Loading
Loading