From bf410694428bda5762112f9785ff570614f402d2 Mon Sep 17 00:00:00 2001 From: Kyle Corbitt Date: Fri, 23 Jun 2023 13:23:35 -0700 Subject: [PATCH] more work on getting the editor to cache and update properly --- prisma/schema.prisma | 4 + .../OutputsTable/VariantConfigEditor.tsx | 74 ++++++++++++++----- src/components/OutputsTable/VariantHeader.tsx | 13 +--- src/components/OutputsTable/index.tsx | 9 +-- .../api/routers/promptVariants.router.ts | 20 +++-- 5 files changed, 79 insertions(+), 41 deletions(-) diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 2405709..0c8d8de 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -26,6 +26,8 @@ model PromptVariant { id String @id @default(uuid()) @db.Uuid label String + uiId String @default(uuid()) @db.Uuid + visible Boolean @default(true) sortIndex Int @default(0) @@ -37,6 +39,8 @@ model PromptVariant { createdAt DateTime @default(now()) updatedAt DateTime @updatedAt ModelOutput ModelOutput[] + + @@index([uiId]) } model TestScenario { diff --git a/src/components/OutputsTable/VariantConfigEditor.tsx b/src/components/OutputsTable/VariantConfigEditor.tsx index fb2716a..3a572e5 100644 --- a/src/components/OutputsTable/VariantConfigEditor.tsx +++ b/src/components/OutputsTable/VariantConfigEditor.tsx @@ -1,25 +1,33 @@ -import { Box, Button, Stack, Title } from "@mantine/core"; +import { Box, Button, Group, Stack, Title } from "@mantine/core"; import { useMonaco } from "@monaco-editor/react"; -import { useRef, useEffect, useState } from "react"; +import { useRef, useEffect, useState, useCallback } from "react"; import { set } from "zod"; import { useHandledAsyncCallback } from "~/utils/hooks"; let isThemeDefined = false; export default function VariantConfigEditor(props: { - initialConfig: string; + savedConfig: string; onSave: (currentConfig: string) => Promise; }) { const monaco = useMonaco(); const editorRef = useRef["editor"]["create"]> | null>(null); const [editorId] = useState(() => `editor_${Math.random().toString(36).substring(7)}`); const [isChanged, setIsChanged] = useState(false); + const savedConfigRef = useRef(props.savedConfig); + + const checkForChanges = useCallback(() => { + if (!editorRef.current) return; + const currentConfig = editorRef.current.getValue(); + setIsChanged(currentConfig !== savedConfigRef.current); + }, []); const [onSave] = useHandledAsyncCallback(async () => { const currentConfig = editorRef.current?.getValue(); if (!currentConfig) return; await props.onSave(currentConfig); - }, [props.onSave]); + checkForChanges(); + }, [props.onSave, checkForChanges]); useEffect(() => { if (monaco) { @@ -36,7 +44,7 @@ export default function VariantConfigEditor(props: { } editorRef.current = monaco.editor.create(document.getElementById(editorId) as HTMLElement, { - value: props.initialConfig, + value: props.savedConfig, language: "json", theme: "customTheme", lineNumbers: "off", @@ -50,28 +58,56 @@ export default function VariantConfigEditor(props: { wordWrapBreakBeforeCharacters: "", }); - editorRef.current.addCommand(monaco.KeyMod.CtrlCmd | monaco.KeyCode.Enter, onSave); - - editorRef.current.onDidChangeModelContent(() => { - const currentConfig = editorRef.current?.getValue(); - if (currentConfig !== props.initialConfig) { - setIsChanged(true); - } else { - setIsChanged(false); - } + editorRef.current.onDidFocusEditorText(() => { + // Workaround because otherwise the command only works on whatever + // editor was loaded on the page last. + // https://github.com/microsoft/monaco-editor/issues/2947#issuecomment-1422265201 + editorRef.current?.addCommand(monaco.KeyMod.CtrlCmd | monaco.KeyCode.Enter, onSave); }); - return () => editorRef.current?.dispose(); + editorRef.current.onDidChangeModelContent(checkForChanges); + + return () => { + editorRef.current?.dispose(); + }; } - }, [monaco, editorId, props.initialConfig, onSave]); + + // We intentionally skip the onSave and props.savedConfig dependencies here because + // we don't want to re-render the editor from scratch + /* eslint-disable-next-line react-hooks/exhaustive-deps */ + }, [monaco, editorId]); + + useEffect(() => { + const savedConfigChanged = savedConfigRef.current !== props.savedConfig; + + savedConfigRef.current = props.savedConfig; + + if (savedConfigChanged && editorRef.current?.getValue() !== props.savedConfig) { + editorRef.current?.setValue(props.savedConfig); + } + + checkForChanges(); + }, [props.savedConfig, checkForChanges]); return (
{isChanged && ( - + + + + )} ); diff --git a/src/components/OutputsTable/VariantHeader.tsx b/src/components/OutputsTable/VariantHeader.tsx index 1f19fbd..dbc37e9 100644 --- a/src/components/OutputsTable/VariantHeader.tsx +++ b/src/components/OutputsTable/VariantHeader.tsx @@ -1,9 +1,7 @@ -import { Button, Stack, Title } from "@mantine/core"; -import { useMonaco } from "@monaco-editor/react"; -import { useCallback, useEffect, useRef, useState } from "react"; +import { Stack, Title } from "@mantine/core"; +import { useCallback } from "react"; import type { PromptVariant } from "./types"; import { api } from "~/utils/api"; -import { useHandledAsyncCallback } from "~/utils/hooks"; import { notifications } from "@mantine/notifications"; import { type JSONSerializable } from "~/server/types"; import VariantConfigEditor from "./VariantConfigEditor"; @@ -44,16 +42,13 @@ export default function VariantHeader({ variant }: { variant: PromptVariant }) { // TODO: invalidate the variants query }, - [variant.id, replaceWithConfig] + [variant.id, replaceWithConfig, utils.promptVariants.list] ); return ( {variant.label} - + ); } diff --git a/src/components/OutputsTable/index.tsx b/src/components/OutputsTable/index.tsx index a7ffa05..64c37b7 100644 --- a/src/components/OutputsTable/index.tsx +++ b/src/components/OutputsTable/index.tsx @@ -31,10 +31,6 @@ export default function OutputsTable({ experimentId }: { experimentId: string | ); const columns = useMemo[]>(() => { - console.log( - "rebuilding cols", - variants.data?.map((variant) => variant.label) - ); return [ { id: "scenario", @@ -47,7 +43,7 @@ export default function OutputsTable({ experimentId }: { experimentId: string | }, ...(variants.data?.map( (variant): MRT_ColumnDef => ({ - id: variant.id, + id: variant.uiId, header: variant.label, Header: , size: 400, @@ -87,9 +83,6 @@ export default function OutputsTable({ experimentId }: { experimentId: string | enableHiding={false} enableColumnActions={false} enableColumnResizing - state={{ - columnOrder: ["scenario", ...variants.data.map((variant) => variant.id)], - }} mantineTableProps={{ sx: { th: { diff --git a/src/server/api/routers/promptVariants.router.ts b/src/server/api/routers/promptVariants.router.ts index b51f522..ceb4259 100644 --- a/src/server/api/routers/promptVariants.router.ts +++ b/src/server/api/routers/promptVariants.router.ts @@ -30,9 +30,6 @@ export const promptVariantsRouter = createTRPCRouter({ }, }); - console.log("got existing", existing); - console.log("config", input.config); - let parsedConfig; try { parsedConfig = JSON.parse(input.config) as OpenAIChatConfig; @@ -44,19 +41,32 @@ export const promptVariantsRouter = createTRPCRouter({ throw new Error(`Prompt Variant with id ${input.id} does not exist`); } + console.log("new config", { + experimentId: existing.experimentId, + label: existing.label, + sortIndex: existing.sortIndex, + uiId: existing.uiId, + config: parsedConfig, + }); + // Create a duplicate with only the config changed const newVariant = await prisma.promptVariant.create({ data: { experimentId: existing.experimentId, label: existing.label, sortIndex: existing.sortIndex, + uiId: existing.uiId, config: parsedConfig, }, }); - await prisma.promptVariant.update({ + // Hide anything with the same uiId besides the new one + await prisma.promptVariant.updateMany({ where: { - id: input.id, + uiId: existing.uiId, + id: { + not: newVariant.id, + }, }, data: { visible: false,