Pandore/homarr
Public Access
1
0 Fork 0
Code Actions Activity

fix: address assistant review feedback

Browse Source
This commit is contained in:
ajnart committed 2026-08-20 15:12:04 +02:00
1 parent d79c748c4d
commit f67f8c1c33
8 files changed
+178 -160
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

apps/nextjs/src/components/assistant/assistant-auto-approval.spec.ts
+40
View File
@@ -104,6 +104,46 @@ describe("assistant automatic approval tracker", () => {
await act(async () => root.unmount());
});
test("does not approve a ready action while switching conversations", async () => {
const container = document.createElement("div");
containers.push(container);
document.body.append(container);
const root = createRoot(container);
let confirmationCount = 0;
const renderConversation = async (conversationId: string, ready: boolean) => {
await act(async () => {
root.render(
createElement(
AssistantAutoApprovalProvider,
{ conversationId },
createElement(
Fragment,
null,
createElement(AutoApprovalProbe),
createElement(AutomaticActionProbe, {
ready,
completed: false,
confirm: () => {
confirmationCount += 1;
},
}),
),
),
);
});
};
await renderConversation("local-thread-1", false);
act(() => currentAutoApproval?.setEnabled(true));
await renderConversation("local-thread-2", true);
expect(confirmationCount).toBe(0);
expect(currentAutoApproval?.enabled).toBe(false);
expect(automaticActionInProgress).toBe(false);
await act(async () => root.unmount());
});
test("clears claimed calls when automatic approvals are turned off", async () => {
const container = document.createElement("div");
containers.push(container);
apps/nextjs/src/components/assistant/assistant-auto-approval.tsx
+10 -7
View File
@@ -59,6 +59,9 @@ export const AssistantAutoApprovalProvider = ({
const trackerRef = useRef(createAssistantAutoApprovalTracker(AUTO_APPROVAL_MAX_ATTEMPTS));
const retryTimersRef = useRef(new Map<string, { timer: number; mode: AssistantAutomaticActionMode }>());
const previousConversationIdRef = useRef(conversationId);
const conversationMatches = previousConversationIdRef.current === conversationId;
const effectivePreparationEnabled = conversationMatches && preparationEnabled;
const effectiveApprovalEnabled = conversationMatches && approvalEnabled;
const clearRetryTimer = useCallback((toolCallId: string) => {
const retry = retryTimersRef.current.get(toolCallId);
@@ -115,7 +118,7 @@ export const AssistantAutoApprovalProvider = ({
const requestAction = useCallback(
(toolCallId: string, confirm: () => void, mode: AssistantAutomaticActionMode = "preparation") => {
const modeEnabled = mode === "approval" ? approvalEnabled : preparationEnabled;
const modeEnabled = mode === "approval" ? effectiveApprovalEnabled : effectivePreparationEnabled;
if (!modeEnabled || !trackerRef.current.claim(toolCallId)) return false;
try {
@@ -135,7 +138,7 @@ export const AssistantAutoApprovalProvider = ({
return false;
}
},
[approvalEnabled, clearRetryTimer, preparationEnabled],
[clearRetryTimer, effectiveApprovalEnabled, effectivePreparationEnabled],
);
const completeAction = useCallback(
@@ -147,12 +150,12 @@ export const AssistantAutoApprovalProvider = ({
);
const value = useMemo(() => {
const enabled = preparationEnabled || approvalEnabled;
const enabled = effectivePreparationEnabled || effectiveApprovalEnabled;
return {
enabled,
setEnabled,
preparationEnabled,
approvalEnabled,
preparationEnabled: effectivePreparationEnabled,
approvalEnabled: effectiveApprovalEnabled,
setPreparationEnabled,
setApprovalEnabled,
requestAction,
@@ -160,9 +163,9 @@ export const AssistantAutoApprovalProvider = ({
retryRevision,
};
}, [
approvalEnabled,
completeAction,
preparationEnabled,
effectiveApprovalEnabled,
effectivePreparationEnabled,
requestAction,
retryRevision,
setApprovalEnabled,
apps/nextjs/src/components/assistant/assistant-panel.module.css
+8 -29
View File
@@ -181,23 +181,6 @@
flex: 0 0 auto;
}
.windowControls {
flex: 0 0 auto;
margin: 0;
padding: 0;
border: 0;
}
.windowControl {
width: 2.25rem;
height: 2.25rem;
}
.windowControl:focus-visible {
outline: 2px solid var(--mantine-primary-color-filled);
outline-offset: 2px;
}
.panelAction {
width: 2.625rem;
height: 2.625rem;
@@ -209,10 +192,6 @@
padding: 0.45rem 0.5rem 0.45rem 0.65rem;
}
.composerHints {
display: none;
}
.historyButton {
width: 2.625rem;
max-width: 2.625rem;
@@ -447,6 +426,9 @@
.message {
margin-block: 0.9rem;
}
.message[data-static] {
contain-intrinsic-size: auto 12rem;
content-visibility: auto;
}
@@ -1224,6 +1206,11 @@
background: var(--mantine-color-default-hover);
}
.messageMoreItem:focus-visible {
outline: 2px solid var(--mantine-primary-color-filled);
outline-offset: -2px;
}
.messageMoreSeparator {
height: 1px;
margin-block: 0.2rem;
@@ -1554,10 +1541,6 @@
pointer-events: none;
}
.composerHints {
padding-inline: 0.45rem;
}
.composerFooter {
min-width: 0;
max-width: 100%;
@@ -1826,10 +1809,6 @@
padding-inline: 0.1rem;
}
.composerHints {
display: none;
}
.runtimeSelectorTrigger {
max-width: 100%;
}
apps/nextjs/src/components/assistant/assistant-panel.tsx
+93 -81
View File
@@ -99,7 +99,6 @@ import {
IconFileExport,
IconLink,
IconMessage,
IconMicrophone,
IconMinus,
IconPaperclip,
IconPalette,
@@ -1259,7 +1258,7 @@ const UserMessage = () => {
const isEditing = useAuiState((state) => state.composer.isEditing);
if (isEditing) return <EditComposer />;
return (
<MessagePrimitive.Root className={`${classes.message} ${classes.userMessageWrap}`}>
<MessagePrimitive.Root className={`${classes.message} ${classes.userMessageWrap}`} data-static>
<Box className={classes.userMessage}>
<MessagePrimitive.Quote>
{(quote) => (
@@ -1941,51 +1940,58 @@ const WebSearchActivity = () => {
);
};
const AssistantMessage = () => (
<MessagePrimitive.Root className={`${classes.message} ${classes.assistantMessage}`}>
<MessagePrimitive.GroupedParts groupBy={assistantMessageGroupBy} indicator="empty">
{({ part, children }) => {
switch (part.type) {
case "group-agent-trace": {
const startIndex = part.indices[0];
const endIndex = part.indices.at(-1);
if (startIndex === undefined || endIndex === undefined) return null;
return (
<ChainOfThoughtByIndicesProvider startIndex={startIndex} endIndex={endIndex}>
<AssistantChainOfThought>{children}</AssistantChainOfThought>
</ChainOfThoughtByIndicesProvider>
);
const AssistantMessage = () => {
const isComplete = useAuiState((state) => state.message.status?.type === "complete");
return (
<MessagePrimitive.Root
className={`${classes.message} ${classes.assistantMessage}`}
data-static={isComplete || undefined}
>
<MessagePrimitive.GroupedParts groupBy={assistantMessageGroupBy} indicator="empty">
{({ part, children }) => {
switch (part.type) {
case "group-agent-trace": {
const startIndex = part.indices[0];
const endIndex = part.indices.at(-1);
if (startIndex === undefined || endIndex === undefined) return null;
return (
<ChainOfThoughtByIndicesProvider startIndex={startIndex} endIndex={endIndex}>
<AssistantChainOfThought>{children}</AssistantChainOfThought>
</ChainOfThoughtByIndicesProvider>
);
}
case "group-reasoning":
return <>{children}</>;
case "group-tool":
return <AgentTraceToolGroup>{children}</AgentTraceToolGroup>;
case "indicator":
return <AssistantMessagePending status={{ type: "running" }} />;
case "text":
return <AssistantTextPart />;
case "reasoning":
return <ReasoningPart {...part} />;
case "tool-call":
return part.toolUI ?? <ToolPart {...part} />;
case "source":
return <SourcePart {...part} />;
case "file":
return <FilePart {...part} />;
case "image":
return <ImagePart {...part} />;
case "data":
return part.dataRendererUI;
default:
return null;
}
case "group-reasoning":
return <>{children}</>;
case "group-tool":
return <AgentTraceToolGroup>{children}</AgentTraceToolGroup>;
case "indicator":
return <AssistantMessagePending status={{ type: "running" }} />;
case "text":
return <AssistantTextPart />;
case "reasoning":
return <ReasoningPart {...part} />;
case "tool-call":
return part.toolUI ?? <ToolPart {...part} />;
case "source":
return <SourcePart {...part} />;
case "file":
return <FilePart {...part} />;
case "image":
return <ImagePart {...part} />;
case "data":
return part.dataRendererUI;
default:
return null;
}
}}
</MessagePrimitive.GroupedParts>
<RuntimeError />
<WebSearchActivity />
<AssistantMessageActions />
</MessagePrimitive.Root>
);
}}
</MessagePrimitive.GroupedParts>
<RuntimeError />
<WebSearchActivity />
<AssistantMessageActions />
</MessagePrimitive.Root>
);
};
const assistantThreadMessageComponents = { UserMessage, AssistantMessage };
@@ -2001,7 +2007,13 @@ const ThreadListItem = () => {
const [exporting, setExporting] = useState(false);
const [actionsOpened, setActionsOpened] = useState(false);
const [renameOpened, setRenameOpened] = useState(false);
const [renaming, setRenaming] = useState(false);
const [nextTitle, setNextTitle] = useState(title ?? "");
const renameInputRef = useRef<HTMLInputElement>(null);
useEffect(() => {
if (renameOpened) renameInputRef.current?.focus();
}, [renameOpened]);
const exportConversation = async () => {
if (!remoteId || exporting) return;
@@ -2023,12 +2035,22 @@ const ThreadListItem = () => {
}
};
const renameConversation = () => {
const renameConversation = async () => {
const trimmedTitle = nextTitle.trim();
if (!trimmedTitle) return;
aui.threadListItem().rename(trimmedTitle);
setRenameOpened(false);
setActionsOpened(false);
if (!trimmedTitle || renaming) return;
setRenaming(true);
try {
await aui.threadListItem().rename(trimmedTitle);
setRenameOpened(false);
setActionsOpened(false);
} catch {
showErrorNotification({
title: t("renameConversation.failedTitle"),
message: t("renameConversation.failedDescription"),
});
} finally {
setRenaming(false);
}
};
const deleteConversation = () => {
@@ -2037,7 +2059,16 @@ const ThreadListItem = () => {
children: t("deleteConversation.description", { title: title ?? t("newConversation") }),
confirmProps: { color: "red" },
labels: { confirm: t("delete"), cancel: t("cancel") },
onConfirm: () => aui.threadListItem().delete(),
onConfirm: async () => {
try {
await aui.threadListItem().delete();
} catch {
showErrorNotification({
title: t("deleteConversation.failedTitle"),
message: t("deleteConversation.failedDescription"),
});
}
},
});
};
@@ -2045,16 +2076,16 @@ const ThreadListItem = () => {
return (
<ThreadListItemPrimitive.Root className={classes.historyItem}>
<TextInput
ref={renameInputRef}
className={classes.historyRenameInput}
value={nextTitle}
onChange={(event) => setNextTitle(event.currentTarget.value)}
aria-label={t("renameConversation.label")}
maxLength={72}
size="xs"
data-autofocus
onKeyDown={(event) => {
if (event.key === "Enter") renameConversation();
if (event.key === "Escape") setRenameOpened(false);
if (event.key === "Enter") void renameConversation();
if (event.key === "Escape" && !renaming) setRenameOpened(false);
}}
/>
<ActionIcon
@@ -2063,7 +2094,9 @@ const ThreadListItem = () => {
size="sm"
aria-label={t("renameConversation.save")}
disabled={!nextTitle.trim()}
onClick={renameConversation}
loading={renaming}
loaderProps={{ type: "bars" }}
onClick={() => void renameConversation()}
>
<IconCheck size={15} />
</ActionIcon>
@@ -2072,6 +2105,7 @@ const ThreadListItem = () => {
color="gray"
size="sm"
aria-label={t("cancel")}
disabled={renaming}
onClick={() => setRenameOpened(false)}
>
<IconX size={15} />
@@ -2902,26 +2936,6 @@ const Composer = (props: ComposerProps) => {
</ActionIcon>
</ComposerPrimitive.AddAttachment>
</Tooltip>
<AuiIf
condition={(state) => state.thread.capabilities.dictation && state.composer.dictation === undefined}
>
<Tooltip label={t("startVoiceInput")}>
<ComposerPrimitive.Dictate asChild>
<ActionIcon variant="subtle" color="gray" size="lg" aria-label={t("startVoiceInput")}>
<IconMicrophone size={17} />
</ActionIcon>
</ComposerPrimitive.Dictate>
</Tooltip>
</AuiIf>
<AuiIf condition={(state) => state.composer.dictation !== undefined}>
<Tooltip label={t("stopVoiceInput")}>
<ComposerPrimitive.StopDictation asChild>
<ActionIcon variant="light" color="red" size="lg" aria-label={t("stopVoiceInput")}>
<IconPlayerStop size={16} />
</ActionIcon>
</ComposerPrimitive.StopDictation>
</Tooltip>
</AuiIf>
</Group>
<LexicalComposerInput
ref={composerInputRef}
@@ -3334,15 +3348,13 @@ export const AssistantPanel = ({
const aui = useAui();
const previousFocusRef = useRef<HTMLElement | null>(null);
const panelRef = useRef<HTMLDialogElement>(null);
const stopVoiceActivityWithoutSurface = () => {
const stopSpeechWithoutSurface = () => {
if (hasVisibleWidget) return;
const composer = aui.composer();
if (composer.getState().dictation !== undefined) composer.stopDictation();
const thread = aui.thread();
if (thread.getState().speech !== undefined) thread.stopSpeaking();
};
const minimize = () => {
stopVoiceActivityWithoutSurface();
stopSpeechWithoutSurface();
onClose();
};
@@ -3408,7 +3420,7 @@ export const AssistantPanel = ({
autoFocusComposer
onMinimize={minimize}
onDismiss={() => {
stopVoiceActivityWithoutSurface();
stopSpeechWithoutSurface();
onDismissActivity();
onClose();
}}
apps/nextjs/src/components/assistant/assistant-provider.tsx
-5
View File
@@ -5,7 +5,6 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react";
import { useRouter } from "next/navigation";
import type {
AttachmentAdapter,
DictationAdapter,
MessageFormatAdapter,
RemoteThreadListAdapter,
SpeechSynthesisAdapter,
@@ -19,7 +18,6 @@ import {
useAuiEvent,
useAuiState,
useRemoteThreadListRuntime,
WebSpeechDictationAdapter,
WebSpeechSynthesisAdapter,
} from "@assistant-ui/react";
import { useChat } from "@ai-sdk/react";
@@ -486,10 +484,8 @@ const AssistantThreadRuntime = () => {
),
[selectedModel],
);
const [dictation, setDictation] = useState<DictationAdapter>();
const [speech, setSpeech] = useState<SpeechSynthesisAdapter>();
useEffect(() => {
if (WebSpeechDictationAdapter.isSupported()) setDictation(new WebSpeechDictationAdapter());
if ("speechSynthesis" in window && typeof SpeechSynthesisUtterance !== "undefined") {
setSpeech(new WebSpeechSynthesisAdapter());
}
@@ -530,7 +526,6 @@ const AssistantThreadRuntime = () => {
adapters: {
history,
attachments,
dictation,
feedback,
speech,
},
apps/nextjs/src/components/assistant/assistant-runtime-provider.spec.ts
+16 -7
View File
@@ -26,7 +26,8 @@ import { z } from "zod/v4";
import { showErrorNotification } from "@homarr/notifications";
import {
AssistantComposerSurfaceProvider,
AssistantComposerRuntimeProvider,
AssistantComposerSurfaceBoundary,
AssistantRunFocusPreserver,
AssistantRuntimeProviderWithTools,
assistantSurfaceComposerCacheLimit,
@@ -148,14 +149,22 @@ const SurfaceTestAssistant = ({
{ runtime, toolkit },
createElement(AssistantRunFocusPreserver),
createElement(
AssistantComposerSurfaceProvider,
{ surfaceId: "surface-a" },
createElement(SurfaceProbe, { id: "a", onReady }),
AssistantComposerRuntimeProvider,
null,
createElement(
AssistantComposerSurfaceBoundary,
{ surfaceId: "surface-a" },
createElement(SurfaceProbe, { id: "a", onReady }),
),
),
createElement(
AssistantComposerSurfaceProvider,
{ surfaceId: "surface-b" },
createElement(SurfaceProbe, { id: "b", onReady }),
AssistantComposerRuntimeProvider,
null,
createElement(
AssistantComposerSurfaceBoundary,
{ surfaceId: "surface-b" },
createElement(SurfaceProbe, { id: "b", onReady }),
),
),
simulateRunFocus ? createElement(LexicalRunFocusProbe) : null,
);
apps/nextjs/src/components/assistant/assistant-runtime-provider.tsx
+5 -21
View File
@@ -42,7 +42,6 @@ class AssistantSurfaceComposerRuntimeCore extends INTERNAL.DefaultThreadComposer
}
public dispose() {
if (this.dictation !== undefined) this.stopDictation();
composerConnectionDisposers.get(this)?.();
composerConnectionDisposers.delete(this);
}
@@ -158,9 +157,6 @@ const createSurfaceComposerHost = (
get attachments() {
return getThreadAdapters(threadCore)?.attachments ? attachments : undefined;
},
get dictation() {
return getThreadAdapters(threadCore)?.dictation;
},
};
return new Proxy(threadCore as ThreadCoreWithAdapters, {
@@ -302,7 +298,6 @@ export const createAssistantSurfaceRuntime = (runtime: AssistantRuntime): Assist
const nextCore = sharedBinding.getState();
const nextConversationId = getConversationId();
if (nextCore !== activeCore || nextConversationId !== activeConversationId) {
if (activeComposer.dictation !== undefined) activeComposer.stopDictation();
if (activeCore.speech !== undefined) activeCore.stopSpeaking();
unsubscribeComposer();
releaseConversation(activeConversationId);
@@ -365,7 +360,10 @@ export const createAssistantSurfaceRuntime = (runtime: AssistantRuntime): Assist
registerModelContextProvider: () => () => {},
dispose() {
activeConversationCounts.clear();
const threadCores = new Set(Array.from(surfaceCoreByConversation.values(), (entry) => entry.source));
const threadCores = new Set([
...Array.from(surfaceCoreByConversation.values(), (entry) => entry.source),
...Array.from(composerByConversation.values(), (entry) => entry.source),
]);
for (const threadCore of threadCores) {
if (threadCore.speech !== undefined) threadCore.stopSpeaking();
}
@@ -382,7 +380,7 @@ const AssistantComposerSurfaceContext = createContext<AssistantComposerSurfaceVa
export const useAssistantComposerSurface = () => {
const value = useContext(AssistantComposerSurfaceContext);
if (!value) throw new Error("useAssistantComposerSurface must be used within AssistantComposerSurfaceProvider");
if (!value) throw new Error("useAssistantComposerSurface must be used within AssistantComposerSurfaceBoundary");
return value;
};
@@ -576,10 +574,6 @@ export const AssistantRunFocusPreserver = () => {
return null;
};
interface AssistantComposerSurfaceProviderProps extends PropsWithChildren {
surfaceId: string;
}
/**
* Gives every rendered assistant surface one shared local composer while preserving the shared
* conversation. Individual surfaces should add AssistantComposerSurfaceBoundary around their UI.
@@ -601,13 +595,3 @@ export const AssistantComposerRuntimeProvider = ({ children }: PropsWithChildren
</AssistantRuntimeProvider>
);
};
/**
* Convenience boundary for a standalone surface. Use AssistantComposerRuntimeProvider with
* localized boundaries when multiple surfaces need to share one draft and attachment queue.
*/
export const AssistantComposerSurfaceProvider = ({ children, surfaceId }: AssistantComposerSurfaceProviderProps) => (
<AssistantComposerRuntimeProvider>
<AssistantComposerSurfaceBoundary surfaceId={surfaceId}>{children}</AssistantComposerSurfaceBoundary>
</AssistantComposerRuntimeProvider>
);
packages/translation/src/lang/en.json
+6 -10
View File
@@ -2309,7 +2309,6 @@
"open": "Open Homarr Assistant",
"close": "Close assistant",
"minimize": "Minimize assistant",
"windowControls": "Conversation window controls",
"spotlight": "Ask Homarr Assistant",
"spotlightDescription": "Type and send a prompt without leaving the command menu.",
"spotlightPrompt": {
@@ -2334,13 +2333,16 @@
"moreActions": "More response actions",
"renameConversation": {
"action": "Rename",
"title": "Rename conversation",
"label": "Conversation name",
"save": "Save name"
"save": "Save name",
"failedTitle": "Conversation could not be renamed",
"failedDescription": "Homarr could not rename this conversation. Try again."
},
"deleteConversation": {
"title": "Delete conversation?",
"description": "“{title}” and its messages will be permanently deleted."
"description": "“{title}” and its messages will be permanently deleted.",
"failedTitle": "Conversation could not be deleted",
"failedDescription": "Homarr could not delete this conversation. Try again."
},
"closeHistory": "Close conversation history",
"openHistory": "Open conversation history",
@@ -2416,13 +2418,10 @@
"notHelpful": "Not helpful",
"readAloud": "Read response aloud",
"stopReading": "Stop reading response",
"startVoiceInput": "Start voice input",
"stopVoiceInput": "Stop voice input",
"feedbackError": "Your feedback could not be saved. Try again.",
"exportMarkdown": "Export as Markdown",
"exportConversation": {
"action": "Export conversation as Markdown",
"working": "Exporting conversation…",
"failedTitle": "Conversation could not be exported",
"failedDescription": "Homarr could not load this conversation. Try again."
},
@@ -2447,7 +2446,6 @@
},
"mentions": {
"menu": "Add Homarr context",
"hint": "Type @ to add context",
"context": "Context",
"app": "Apps",
"integration": "Integrations",
@@ -2462,7 +2460,6 @@
},
"commands": {
"menu": "Assistant commands",
"hint": "Type / for shortcuts",
"health": "Prepare a service-health check",
"explore": "Prepare an overview of this Homarr instance",
"media": "Prepare a media search",
@@ -2575,7 +2572,6 @@
"otherPlaceholder": "Type another answer…",
"submit": "Send answer",
"cancelOther": "Cancel other answer",
"automaticAffirmative": "Choosing the affirmative answer automatically",
"optionKind": {
"affirmative": "Confirm",
"negative": "Decline",