From aea7b90fecf590b43de0619cf78c88442e73dd2d Mon Sep 17 00:00:00 2001 From: Debanjum Date: Wed, 9 Apr 2025 09:53:07 +0530 Subject: [PATCH] Track if agent modified in chatSidebar to simplify code, fix looping Previously the sidebar could recurse on opening chat page (from home?) due to child modelSelector component updating parent chatSidebar prop which was passed back down to it in a loop. The chatSidebar decides if agent has been modified in a single useEffect and enables the Save button accordingly. - Track agent modification wrt agent info received from server in chatSidebar instead. - Reduce modelSelector's mandate to just notify when the user changes the model. - Fix to infer, show & update agent state from chat sidebar on web app This logic is fragile and convoluted because: - the default agent chat model is dynamically determined. - need to disambiguate tools not set vs none set vs all set by user The default agent's tool selection is stored as undefined to show not set scenario, which allows for all tools to be dynamically used by agent. But the user can also set no tools or all tools for their agents. All 3 scenarios are handled differently. - Track tools to be displayed vs tools to be stored --- .../web/app/common/modelSelector.tsx | 31 ++------- .../components/chatSidebar/chatSidebar.tsx | 63 ++++++++++++------- 2 files changed, 47 insertions(+), 47 deletions(-) diff --git a/src/interface/web/app/common/modelSelector.tsx b/src/interface/web/app/common/modelSelector.tsx index beacdbdc..70c171b0 100644 --- a/src/interface/web/app/common/modelSelector.tsx +++ b/src/interface/web/app/common/modelSelector.tsx @@ -28,8 +28,7 @@ import { HoverCard, HoverCardContent, HoverCardTrigger } from "@/components/ui/h import { Skeleton } from "@/components/ui/skeleton"; interface ModelSelectorProps extends PopoverProps { - onSelect: (model: ModelOptions, userModification: boolean) => void; - selectedModel?: string; + onSelect: (model: ModelOptions) => void; disabled?: boolean; initialModel?: string; } @@ -49,9 +48,8 @@ export function ModelSelector({ ...props }: ModelSelectorProps) { setModels(userConfig.chat_model_options); if (!props.initialModel) { const selectedChatModelOption = userConfig.chat_model_options.find(model => model.id === userConfig.selected_chat_model_config); - if (!selectedChatModelOption) { + if (!selectedChatModelOption && userConfig.chat_model_options.length > 0) { setSelectedModel(userConfig.chat_model_options[0]); - return; } else { setSelectedModel(selectedChatModelOption); } @@ -63,29 +61,10 @@ export function ModelSelector({ ...props }: ModelSelectorProps) { }, [userConfig, props.initialModel, isLoadingUserConfig]); useEffect(() => { - if (props.selectedModel && selectedModel && props.selectedModel !== selectedModel.name) { - const model = models.find(model => model.name === props.selectedModel); - setSelectedModel(model); + if (selectedModel && userConfig) { + props.onSelect(selectedModel); } - else if (props.selectedModel === null && userConfig) { - const selectedChatModelOption = userConfig.chat_model_options.find(model => model.id === userConfig.selected_chat_model_config); - if (!selectedChatModelOption) { - props.onSelect(userConfig.chat_model_options[0], false); - return; - } else { - props.onSelect(selectedChatModelOption, false); - } - } - }, [props.selectedModel, models]); - - useEffect(() => { - if (selectedModel) { - const userModification = selectedModel.id !== userConfig?.selected_chat_model_config; - if (props.selectedModel !== selectedModel.name) { - props.onSelect(selectedModel, userModification); - } - } - }, [selectedModel]); + }, [selectedModel, userConfig, props.onSelect]); if (isLoadingUserConfig) { return ( diff --git a/src/interface/web/app/components/chatSidebar/chatSidebar.tsx b/src/interface/web/app/components/chatSidebar/chatSidebar.tsx index 52604ab0..e8711fa4 100644 --- a/src/interface/web/app/components/chatSidebar/chatSidebar.tsx +++ b/src/interface/web/app/components/chatSidebar/chatSidebar.tsx @@ -311,12 +311,14 @@ function ChatSidebarInternal({ ...props }: ChatSideBarProps) { isLoading: authenticationLoading, } = useAuthenticatedData(); - const [customPrompt, setCustomPrompt] = useState(""); + const [customPrompt, setCustomPrompt] = useState(); const [selectedModel, setSelectedModel] = useState(); const [inputTools, setInputTools] = useState(); const [outputModes, setOutputModes] = useState(); const [hasModified, setHasModified] = useState(false); - const [isDefaultAgent, setIsDefaultAgent] = useState(agentData?.slug.toLowerCase() === "khoj" ? true : false); + const [isDefaultAgent, setIsDefaultAgent] = useState(!agentData || agentData?.slug.toLowerCase() === "khoj"); + const [displayInputTools, setDisplayInputTools] = useState(); + const [displayOutputModes, setDisplayOutputModes] = useState(); const [isSaving, setIsSaving] = useState(false); @@ -325,12 +327,14 @@ function ChatSidebarInternal({ ...props }: ChatSideBarProps) { function setupAgentData() { if (agentData) { setInputTools(agentData.input_tools); - if (agentData.input_tools === undefined || agentData.input_tools.length === 0) { - setInputTools(agentConfigurationOptions?.input_tools ? Object.keys(agentConfigurationOptions.input_tools) : []); + setDisplayInputTools(agentData.input_tools); + if (agentData.input_tools === undefined) { + setDisplayInputTools(agentConfigurationOptions?.input_tools ? Object.keys(agentConfigurationOptions.input_tools) : []); } setOutputModes(agentData.output_modes); - if (agentData.output_modes === undefined || agentData.output_modes.length === 0) { - setOutputModes(agentConfigurationOptions?.output_modes ? Object.keys(agentConfigurationOptions.output_modes) : []); + setDisplayOutputModes(agentData.output_modes); + if (agentData.output_modes === undefined) { + setDisplayOutputModes(agentConfigurationOptions?.output_modes ? Object.keys(agentConfigurationOptions.output_modes) : []); } if (agentData.name.toLowerCase() === "khoj" || agentData.is_hidden === true) { @@ -351,16 +355,30 @@ function ChatSidebarInternal({ ...props }: ChatSideBarProps) { useEffect(() => { setupAgentData(); + setHasModified(false); }, [agentData]); + // Track changes to the model, prompt, input tools and output modes fields + useEffect(() => { + if (!agentData || agentDataLoading) return; // Don't compare until data is loaded + + const modelChanged = !!selectedModel && selectedModel !== agentData.chat_model; + const promptChanged = !!customPrompt && customPrompt !== agentData.persona; + + // Order independent check to ensure input tools or output modes haven't been changed. + const toolsChanged = JSON.stringify(inputTools?.sort() || []) !== JSON.stringify(agentData.input_tools?.sort()); + const modesChanged = JSON.stringify(outputModes?.sort() || []) !== JSON.stringify(agentData.output_modes?.sort()); + + setHasModified(modelChanged || promptChanged || toolsChanged || modesChanged); + + // Add agentDataLoading to dependencies to ensure it runs after loading finishes + }, [selectedModel, customPrompt, inputTools, outputModes, agentData, agentDataLoading]); function isValueChecked(value: string, existingSelections: string[]): boolean { return existingSelections.includes(value); } function handleCheckToggle(value: string, existingSelections: string[]): string[] { - setHasModified(true); - if (existingSelections.includes(value)) { return existingSelections.filter((v) => v !== value); } @@ -370,7 +388,6 @@ function ChatSidebarInternal({ ...props }: ChatSideBarProps) { function handleCustomPromptChange(value: string) { setCustomPrompt(value); - setHasModified(true); } function handleSave() { @@ -430,11 +447,8 @@ function ChatSidebarInternal({ ...props }: ChatSideBarProps) { setHasModified(false); } - function handleModelSelect(model: string, userModification: boolean = true) { + function handleModelSelect(model: string) { setSelectedModel(model); - if (userModification) { - setHasModified(true); - } } return ( @@ -514,9 +528,8 @@ function ChatSidebarInternal({ ...props }: ChatSideBarProps) { handleModelSelect(model.name, userModification)} + onSelect={(model) => handleModelSelect(model.name)} initialModel={isDefaultAgent ? undefined : agentData?.chat_model} - selectedModel={selectedModel} /> @@ -551,8 +564,12 @@ function ChatSidebarInternal({ ...props }: ChatSideBarProps) { setInputTools(handleCheckToggle(key, inputTools ?? []))} + checked={isValueChecked(key, displayInputTools ?? [])} + onCheckedChange={() => { + let updatedInputTools = handleCheckToggle(key, displayInputTools ?? []) + setInputTools(updatedInputTools); + setDisplayInputTools(updatedInputTools); + }} disabled={!isEditable} > {key} @@ -584,8 +601,12 @@ function ChatSidebarInternal({ ...props }: ChatSideBarProps) { setOutputModes(handleCheckToggle(key, outputModes ?? []))} + checked={isValueChecked(key, displayOutputModes ?? [])} + onCheckedChange={() => { + let updatedOutputModes = handleCheckToggle(key, displayOutputModes ?? []) + setOutputModes(updatedOutputModes); + setDisplayOutputModes(updatedOutputModes); + }} disabled={!isEditable} > {key} @@ -649,8 +670,8 @@ function ChatSidebarInternal({ ...props }: ChatSideBarProps) {