-
Notifications
You must be signed in to change notification settings - Fork 962
Add low hanging ui features #1304
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -309,6 +309,68 @@ export class ChatSessionManager extends EventEmitter { | |
| await this.store.update(sessionId, patch); | ||
| } | ||
|
|
||
| async updateAgentConfig({ | ||
| sessionId, | ||
| maxThinkingTokens, | ||
| model, | ||
| }: { | ||
| sessionId: string; | ||
| maxThinkingTokens?: number | null; | ||
| model?: string | null; | ||
| }): Promise<void> { | ||
| const session = this.sessions.get(sessionId); | ||
| if (!session) { | ||
| console.warn( | ||
| `[chat/session] Session ${sessionId} not found for config update`, | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| const registration = this.provider.getAgentRegistration({ | ||
| sessionId, | ||
| cwd: session.cwd, | ||
| }); | ||
|
|
||
| if (maxThinkingTokens !== undefined) { | ||
| if (maxThinkingTokens === null) { | ||
| delete registration.bodyTemplate.maxThinkingTokens; | ||
| } else { | ||
| registration.bodyTemplate.maxThinkingTokens = maxThinkingTokens; | ||
| } | ||
| } | ||
|
|
||
| if (model !== undefined) { | ||
| if (model === null) { | ||
| delete registration.bodyTemplate.model; | ||
| } else { | ||
| registration.bodyTemplate.model = model; | ||
| } | ||
| } | ||
|
|
||
| const headers = buildProxyHeaders(); | ||
| const res = await fetch(`${PROXY_URL}/v1/sessions/${sessionId}/agents`, { | ||
| method: "POST", | ||
| headers, | ||
| body: JSON.stringify({ agents: [registration] }), | ||
| }); | ||
| if (!res.ok) { | ||
| throw new Error( | ||
| `POST /v1/sessions/${sessionId}/agents failed: ${res.status}`, | ||
| ); | ||
| } | ||
|
|
||
| console.log( | ||
| `[chat/session] Updated agent config for ${sessionId}`, | ||
| [ | ||
| maxThinkingTokens !== undefined && | ||
| `maxThinkingTokens=${maxThinkingTokens}`, | ||
| model !== undefined && `model=${model}`, | ||
| ] | ||
| .filter(Boolean) | ||
| .join(", "), | ||
| ); | ||
| } | ||
|
Comment on lines
+312
to
+372
Contributor
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. Missing error handling around fetch — inconsistent with other methods.
As per coding guidelines, "Never swallow errors silently; at minimum log errors with context before rethrowing or handling them explicitly." Proposed error handling async updateAgentConfig({
sessionId,
maxThinkingTokens,
}: {
sessionId: string;
maxThinkingTokens?: number | null;
}): Promise<void> {
const session = this.sessions.get(sessionId);
if (!session) {
console.warn(
`[chat/session] Session ${sessionId} not found for config update`,
);
return;
}
const registration = this.provider.getAgentRegistration({
sessionId,
cwd: session.cwd,
});
if (maxThinkingTokens !== undefined) {
if (maxThinkingTokens === null) {
delete registration.bodyTemplate.maxThinkingTokens;
} else {
registration.bodyTemplate.maxThinkingTokens = maxThinkingTokens;
}
}
- const headers = buildProxyHeaders();
- const res = await fetch(`${PROXY_URL}/v1/sessions/${sessionId}/agents`, {
- method: "POST",
- headers,
- body: JSON.stringify({ agents: [registration] }),
- });
- if (!res.ok) {
- throw new Error(
- `POST /v1/sessions/${sessionId}/agents failed: ${res.status}`,
- );
- }
-
- console.log(
- `[chat/session] Updated agent config for ${sessionId}`,
- maxThinkingTokens !== undefined
- ? `maxThinkingTokens=${maxThinkingTokens}`
- : "",
- );
+ try {
+ const headers = buildProxyHeaders();
+ const res = await fetch(`${PROXY_URL}/v1/sessions/${sessionId}/agents`, {
+ method: "POST",
+ headers,
+ body: JSON.stringify({ agents: [registration] }),
+ });
+ if (!res.ok) {
+ throw new Error(
+ `POST /v1/sessions/${sessionId}/agents failed: ${res.status}`,
+ );
+ }
+
+ console.log(
+ `[chat/session] Updated agent config for ${sessionId}`,
+ maxThinkingTokens !== undefined
+ ? `maxThinkingTokens=${maxThinkingTokens}`
+ : "",
+ );
+ } catch (error) {
+ const message = error instanceof Error ? error.message : String(error);
+ console.error(`[chat/session] Failed to update agent config:`, message);
+ throw error;
+ }
}🤖 Prompt for AI Agents |
||
|
|
||
| isSessionActive(sessionId: string): boolean { | ||
| return this.sessions.has(sessionId); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -16,6 +16,7 @@ import { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } from "@superset/ui/ai-elements/prompt-input"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { Shimmer } from "@superset/ui/ai-elements/shimmer"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { Suggestion, Suggestions } from "@superset/ui/ai-elements/suggestion"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { ThinkingToggle } from "@superset/ui/ai-elements/thinking-toggle"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useCallback, useEffect, useRef, useState } from "react"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| HiMiniAtSymbol, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -48,6 +49,9 @@ export function ChatInterface({ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }: ChatInterfaceProps) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [selectedModel, setSelectedModel] = useState<ModelOption>(MODELS[1]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [modelSelectorOpen, setModelSelectorOpen] = useState(false); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [thinkingEnabled, setThinkingEnabled] = useState(false); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const updateConfig = electronTrpc.aiChat.updateSessionConfig.useMutation(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+52
to
+54
Contributor
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.
If a session already has 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { data: config } = electronTrpc.aiChat.getConfig.useQuery(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -60,6 +64,7 @@ export function ChatInterface({ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| stop, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| addToolApprovalResponse, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| connect, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| collections, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } = useDurableChat({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sessionId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| proxyUrl: config?.proxyUrl ?? "http://localhost:8080", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -214,6 +219,28 @@ export function ChatInterface({ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| [addToolApprovalResponse], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const handleThinkingToggle = useCallback( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (enabled: boolean) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setThinkingEnabled(enabled); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| updateConfig.mutate({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sessionId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| maxThinkingTokens: enabled ? 10000 : null, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| [sessionId, updateConfig], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+222
to
+231
Contributor
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. Magic number Two issues in this handler:
Proposed fixIn const DEFAULT_MAX_THINKING_TOKENS = 10_000;Then in the handler: const handleThinkingToggle = useCallback(
(enabled: boolean) => {
setThinkingEnabled(enabled);
updateConfig.mutate(
{
sessionId,
- maxThinkingTokens: enabled ? 10000 : null,
+ maxThinkingTokens: enabled ? DEFAULT_MAX_THINKING_TOKENS : null,
+ },
+ {
+ onError: (err) => {
+ console.error("[chat/thinking] Failed to update config:", err);
+ setThinkingEnabled(!enabled);
+ },
},
);
},
[sessionId, updateConfig],
);🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const handleModelSelect = useCallback( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (model: ModelOption) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setSelectedModel(model); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| updateConfig.mutate({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sessionId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| model: model.id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| [sessionId, updateConfig], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+233
to
+242
Contributor
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.
Same pattern as Proposed fix const handleModelSelect = useCallback(
(model: ModelOption) => {
+ const previousModel = selectedModel;
setSelectedModel(model);
- updateConfig.mutate({
- sessionId,
- model: model.id,
- });
+ updateConfig.mutate(
+ {
+ sessionId,
+ model: model.id,
+ },
+ {
+ onError: (err) => {
+ console.error("[chat/model] Failed to update model:", err);
+ setSelectedModel(previousModel);
+ },
+ },
+ );
},
- [sessionId, updateConfig],
+ [sessionId, updateConfig, selectedModel],
);📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const handleStop = useCallback( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (e: React.MouseEvent) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| e.preventDefault(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -296,15 +323,22 @@ export function ChatInterface({ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <PromptInputButton> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <HiMiniAtSymbol className="size-4" /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </PromptInputButton> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <ThinkingToggle | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| enabled={thinkingEnabled} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onToggle={handleThinkingToggle} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <ModelPicker | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| selectedModel={selectedModel} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onSelectModel={setSelectedModel} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onSelectModel={handleModelSelect} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| open={modelSelectorOpen} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onOpenChange={setModelSelectorOpen} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </PromptInputTools> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div className="flex items-center gap-1"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <ContextIndicator /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <ContextIndicator | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| collections={collections} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| modelId={selectedModel.id} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <PromptInputSubmit | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| status={isLoading ? "streaming" : undefined} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onClick={isLoading ? handleStop : undefined} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| "use client"; | ||
|
|
||
| import { BrainIcon } from "lucide-react"; | ||
| import type { ComponentProps } from "react"; | ||
| import { cn } from "../../lib/utils"; | ||
| import { Button } from "../ui/button"; | ||
| import { | ||
| Tooltip, | ||
| TooltipContent, | ||
| TooltipProvider, | ||
| TooltipTrigger, | ||
| } from "../ui/tooltip"; | ||
|
|
||
| export type ThinkingToggleProps = Omit< | ||
| ComponentProps<typeof Button>, | ||
| "onClick" | "onToggle" | ||
| > & { | ||
| enabled: boolean; | ||
| onToggle: (enabled: boolean) => void; | ||
| }; | ||
|
|
||
| export const ThinkingToggle = ({ | ||
| enabled, | ||
| onToggle, | ||
| className, | ||
| ...props | ||
| }: ThinkingToggleProps) => ( | ||
| <TooltipProvider> | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <Button | ||
| type="button" | ||
| variant="ghost" | ||
| size="icon-sm" | ||
| className={cn( | ||
| enabled && "bg-accent text-accent-foreground", | ||
| className, | ||
| )} | ||
| onClick={() => onToggle(!enabled)} | ||
| {...props} | ||
| > | ||
| <BrainIcon className="size-4" /> | ||
| <span className="sr-only"> | ||
| {enabled ? "Extended thinking enabled" : "Enable extended thinking"} | ||
| </span> | ||
| </Button> | ||
| </TooltipTrigger> | ||
| <TooltipContent> | ||
| <p> | ||
| {enabled ? "Extended thinking enabled" : "Enable extended thinking"} | ||
| </p> | ||
| </TooltipContent> | ||
| </Tooltip> | ||
| </TooltipProvider> | ||
| ); |
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.
🧩 Analysis chain
🏁 Script executed:
Repository: superset-sh/superset
Length of output: 2982
🏁 Script executed:
Repository: superset-sh/superset
Length of output: 733
🏁 Script executed:
Repository: superset-sh/superset
Length of output: 617
🏁 Script executed:
Repository: superset-sh/superset
Length of output: 2236
🏁 Script executed:
Repository: superset-sh/superset
Length of output: 6883
updateAgentConfigrebuilds registration withoutpaneId/tabId— routing context will be lost.startSessionandrestoreSessionpasspaneIdandtabIdtogetAgentRegistration, butupdateAgentConfigomits them. SinceActiveSessiondoes not store these values, they cannot be retrieved when the registration is rebuilt and POSTed back, causing any pane/tab routing information to be lost.Store
paneIdandtabIdin theActiveSessioninterface so they're available when updating the agent config.🤖 Prompt for AI Agents