-
Notifications
You must be signed in to change notification settings - Fork 79
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] Add 'Metadata' Section Under Settings Tab #1205
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -244,19 +244,21 @@ function AIConfigEditorBase({ | |
async ( | ||
promptName: string, | ||
newPrompt: Prompt, | ||
onSuccess: (aiconfigRes: AIConfig) => void, | ||
onError: (err: unknown) => void | ||
callbacks: { | ||
onSuccess?: (aiconfigRes: AIConfig) => void; | ||
onError?: (err: unknown) => void; | ||
} | ||
) => { | ||
try { | ||
const serverConfigRes = await updatePromptCallback( | ||
promptName, | ||
newPrompt | ||
); | ||
if (serverConfigRes?.aiconfig) { | ||
onSuccess(serverConfigRes.aiconfig); | ||
callbacks?.onSuccess?.(serverConfigRes.aiconfig); | ||
} | ||
} catch (err: unknown) { | ||
onError(err); | ||
callbacks?.onError?.(err); | ||
} | ||
}, | ||
DEBOUNCE_MS | ||
|
@@ -302,13 +304,15 @@ function AIConfigEditorBase({ | |
...prompt, | ||
input: newPromptInput, | ||
}, | ||
(config) => | ||
dispatch({ | ||
type: "CONSOLIDATE_AICONFIG", | ||
action, | ||
config, | ||
}), | ||
onError | ||
{ | ||
onSuccess: (config) => | ||
dispatch({ | ||
type: "CONSOLIDATE_AICONFIG", | ||
action, | ||
config, | ||
}), | ||
onError, | ||
} | ||
); | ||
} catch (err: unknown) { | ||
onError(err); | ||
|
@@ -350,15 +354,17 @@ function AIConfigEditorBase({ | |
// PromptName component maintains local state for the name to show in the UI | ||
// We cannot update client config state until the name is successfully set server-side | ||
// or else we could end up referencing a prompt name that is not set server-side | ||
() => { | ||
dispatch({ | ||
type: "UPDATE_PROMPT_NAME", | ||
id: promptId, | ||
name: newName, | ||
}); | ||
logEventHandler?.("UPDATE_PROMPT_NAME"); | ||
}, | ||
onError | ||
{ | ||
onSuccess: () => { | ||
dispatch({ | ||
type: "UPDATE_PROMPT_NAME", | ||
id: promptId, | ||
name: newName, | ||
}); | ||
logEventHandler?.("UPDATE_PROMPT_NAME"); | ||
}, | ||
onError, | ||
} | ||
); | ||
} catch (err: unknown) { | ||
onError(err); | ||
|
@@ -392,6 +398,57 @@ function AIConfigEditorBase({ | |
); | ||
}, [updateModelCallback]); | ||
|
||
const onUpdatePromptMetadata = useCallback( | ||
async (promptId: string, newMetadata: JSONObject) => { | ||
if (!debouncedUpdatePrompt) { | ||
// Just no-op if no callback specified. We could technically perform | ||
// client-side updates but that might be confusing | ||
Comment on lines
+404
to
+405
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Nah this is a good call |
||
return; | ||
} | ||
|
||
dispatch({ | ||
type: "UPDATE_PROMPT_METADATA", | ||
id: promptId, | ||
metadata: newMetadata, | ||
}); | ||
|
||
const onError = (err: unknown) => { | ||
const message = (err as RequestCallbackError).message ?? null; | ||
showNotification({ | ||
title: "Error updating prompt metadata", | ||
message, | ||
type: "error", | ||
}); | ||
}; | ||
|
||
try { | ||
const statePrompt = getPrompt(stateRef.current, promptId); | ||
if (!statePrompt) { | ||
throw new Error(`Could not find prompt with id ${promptId}`); | ||
} | ||
const prompt = clientPromptToAIConfigPrompt(statePrompt); | ||
|
||
await debouncedUpdatePrompt( | ||
prompt.name, | ||
{ | ||
...prompt, | ||
metadata: { | ||
...newMetadata, | ||
model: prompt.metadata?.model, | ||
parameters: prompt.metadata?.parameters, | ||
}, | ||
}, | ||
{ | ||
onError, | ||
} | ||
); | ||
} catch (err: unknown) { | ||
onError(err); | ||
} | ||
}, | ||
[debouncedUpdatePrompt, showNotification] | ||
); | ||
|
||
const onUpdatePromptModelSettings = useCallback( | ||
async (promptId: string, newModelSettings: JSONObject) => { | ||
if (!debouncedUpdateModel) { | ||
|
@@ -1098,6 +1155,7 @@ function AIConfigEditorBase({ | |
onChangePromptName={onChangePromptName} | ||
onDeletePrompt={onDeletePrompt} | ||
onRunPrompt={onRunPrompt} | ||
onUpdatePromptMetadata={onUpdatePromptMetadata} | ||
onUpdatePromptModel={onUpdatePromptModel} | ||
onUpdatePromptModelSettings={onUpdatePromptModelSettings} | ||
onUpdatePromptParameters={onUpdatePromptParameters} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
105 changes: 92 additions & 13 deletions
105
...c/aiconfig/editor/client/src/components/prompt/prompt_metadata/PromptMetadataRenderer.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,24 +1,103 @@ | ||
import { ClientPrompt } from "../../../shared/types"; | ||
import { GenericPropertiesSchema } from "../../../utils/promptUtils"; | ||
import { memo } from "react"; | ||
import { memo, useState } from "react"; | ||
import JSONRenderer from "../../JSONRenderer"; | ||
import { JSONObject } from "aiconfig"; | ||
import { Flex, Text, createStyles } from "@mantine/core"; | ||
import JSONEditorToggleButton from "../../JSONEditorToggleButton"; | ||
import { ErrorBoundary, useErrorBoundary } from "react-error-boundary"; | ||
import PromptMetadataSchemaRenderer from "./PromptMetadataSchemaRenderer"; | ||
|
||
type Props = { | ||
prompt: ClientPrompt; | ||
metadata?: JSONObject; | ||
onUpdatePromptMetadata: (metadata: JSONObject) => void; | ||
schema?: GenericPropertiesSchema; | ||
}; | ||
|
||
function ModelMetadataConfigRenderer(_props: Props) { | ||
return null; // TODO: Implement | ||
} | ||
const useStyles = createStyles(() => ({ | ||
metadataContainer: { | ||
overflow: "auto", | ||
paddingTop: "0.5em", | ||
width: "100%", | ||
}, | ||
})); | ||
|
||
type ErrorFallbackProps = { | ||
metadata?: JSONObject; | ||
toggleJSONEditor: () => void; | ||
}; | ||
|
||
function ModelMetadataSchemaRenderer(_props: Props) { | ||
return null; // TODO: Implement | ||
function MetadataErrorFallback({ | ||
metadata, | ||
toggleJSONEditor, | ||
}: ErrorFallbackProps) { | ||
const { resetBoundary: clearRenderError } = useErrorBoundary(); | ||
return ( | ||
<Flex direction="column"> | ||
<Text color="red" size="sm"> | ||
<Flex justify="flex-end"> | ||
<JSONEditorToggleButton | ||
isRawJSON={false} | ||
setIsRawJSON={() => { | ||
clearRenderError(); | ||
toggleJSONEditor(); | ||
}} | ||
/> | ||
</Flex> | ||
Invalid metadata format for model. Toggle JSON editor to update. Set to | ||
{" {}"} in JSON editor and toggle back to reset. | ||
</Text> | ||
<JSONRenderer content={metadata} /> | ||
</Flex> | ||
); | ||
} | ||
|
||
export default memo(function PromptMetadataRenderer({ prompt, schema }: Props) { | ||
return schema ? ( | ||
<ModelMetadataSchemaRenderer prompt={prompt} schema={schema} /> | ||
) : ( | ||
<ModelMetadataConfigRenderer prompt={prompt} /> | ||
export default memo(function PromptMetadataRenderer({ | ||
metadata, | ||
onUpdatePromptMetadata, | ||
schema, | ||
}: Props) { | ||
const { classes } = useStyles(); | ||
const [isRawJSON, setIsRawJSON] = useState(schema == null); | ||
|
||
const rawJSONToggleButton = ( | ||
<Flex justify="flex-end"> | ||
<JSONEditorToggleButton | ||
isRawJSON={isRawJSON} | ||
setIsRawJSON={setIsRawJSON} | ||
/> | ||
</Flex> | ||
); | ||
|
||
return ( | ||
<Flex direction="column" className={classes.metadataContainer}> | ||
{isRawJSON || !schema ? ( | ||
<> | ||
{/* // Only show the toggle if there is a schema to toggle between JSON and custom schema renderer */} | ||
{schema && rawJSONToggleButton} | ||
<JSONRenderer | ||
content={metadata} | ||
onChange={(val) => | ||
onUpdatePromptMetadata(val as Record<string, unknown>) | ||
} | ||
/> | ||
</> | ||
) : ( | ||
<ErrorBoundary | ||
fallbackRender={() => ( | ||
<MetadataErrorFallback | ||
metadata={metadata} | ||
toggleJSONEditor={() => setIsRawJSON(true)} | ||
/> | ||
)} | ||
> | ||
{rawJSONToggleButton} | ||
<PromptMetadataSchemaRenderer | ||
metadata={metadata} | ||
schema={schema} | ||
onUpdatePromptMetadata={onUpdatePromptMetadata} | ||
/> | ||
</ErrorBoundary> | ||
)} | ||
</Flex> | ||
); | ||
}); |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the rationale to make this an object vs keep the callbacks defined?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using
onSuccess
andonError
is a very common frontend pattern. I think Ryan is just following conventions?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ya, for js usually it's common to use an object when props can be undefined rather than having literal 'undefined' prop before a defined one