-
Notifications
You must be signed in to change notification settings - Fork 1.9k
feat: single default assistant #5351
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
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
a5a8300
to
5dfcb4b
Compare
afc84b5
to
442ad26
Compare
105ec63
to
751a10c
Compare
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.
Greptile Summary
This PR implements a unified assistant architecture, consolidating multiple specialized assistants (Search, General, Art) into a single configurable "Assistant" with ID 0. The changes introduce a comprehensive system where one default assistant combines search, web browsing, and image generation capabilities, replacing the previous multi-assistant model.
The implementation includes several key components:
Backend Changes: A new default assistant API (backend/onyx/server/features/default_assistant/api.py
) provides admin endpoints for configuring the unified assistant's tools and system prompt. The persona seeding system has been completely refactored - prebuilt_personas.py
(322 lines) has been replaced with default_persona.py
, which uses a sophisticated field categorization approach that distinguishes between "always updated" system fields and "admin-controlled" customizable fields.
Tool System Refactoring: Throughout the codebase, InternetSearchTool
has been systematically renamed to WebSearchTool
for better naming consistency. The built-in tools system now supports dynamic tool assignment to the default assistant, with SearchTool and ImageGenerationTool as required tools, and WebSearchTool as optional.
Frontend UI Updates: The chat interface now provides special treatment for the unified assistant (ID 0), displaying the Onyx logo instead of an assistant icon, using randomized greeting messages instead of assistant names, and excluding it from pinning/unpinning behaviors. A new admin configuration page (/admin/configuration/default-assistant
) allows administrators to toggle tools on/off and customize the system prompt.
Data Flow Integration: The chat context has been enhanced to include availableTools
data, flowing from the backend through layout components to enable tool-aware UI components. This supports the dynamic configuration of the unified assistant's capabilities.
Database Architecture: The changes maintain backward compatibility through careful field management in the persona system, where core system fields are automatically updated while preserving admin customizations. The approach ensures that the unified assistant stays current with system updates while respecting administrative configurations.
Confidence score: 3/5
- This PR introduces significant architectural changes that could cause issues if the field categorization logic in
default_persona.py
fails or if the database migrations don't handle edge cases properly - Score reflects the complexity of the persona seeding refactor and potential runtime errors from undefined fields in the DEFAULT_PERSONA instance
- Pay close attention to
backend/onyx/seeding/default_persona.py
which contains undefined fields and mutating functions that could cause runtime failures
38 files reviewed, 9 comments
backend/onyx/tools/tool_implementations/internet_search/internet_search_tool.py
Outdated
Show resolved
Hide resolved
backend/onyx/tools/tool_implementations/internet_search/internet_search_tool.py
Outdated
Show resolved
Hide resolved
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.
15 issues found across 39 files
React with 👍 or 👎 to teach cubic. You can also tag @cubic-dev-ai
to give feedback, ask questions, or re-run the review.
@@ -0,0 +1,7 @@ | |||
export const GREETING_MESSAGES = ["How can I help?", "Let's get started."]; |
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.
Exported array is mutable; mark as readonly to prevent accidental modification.
Prompt for AI agents
Address the following comment on web/src/lib/chat/greetingMessages.ts at line 1:
<comment>Exported array is mutable; mark as readonly to prevent accidental modification.</comment>
<file context>
@@ -0,0 +1,7 @@
+export const GREETING_MESSAGES = ["How can I help?", "Let's get started."];
+
+export function getRandomGreeting(): string {
</file context>
export function getRandomGreeting(): string { | ||
return GREETING_MESSAGES[ | ||
Math.floor(Math.random() * GREETING_MESSAGES.length) | ||
] as string; |
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.
Type assertion hides potential undefined under noUncheckedIndexedAccess; provide a safe fallback instead of casting.
Prompt for AI agents
Address the following comment on web/src/lib/chat/greetingMessages.ts at line 6:
<comment>Type assertion hides potential undefined under noUncheckedIndexedAccess; provide a safe fallback instead of casting.</comment>
<file context>
@@ -0,0 +1,7 @@
+export function getRandomGreeting(): string {
+ return GREETING_MESSAGES[
+ Math.floor(Math.random() * GREETING_MESSAGES.length)
+ ] as string;
+}
</file context>
|
||
|
||
# Valid built-in tool IDs that can be toggled for the default assistant | ||
VALID_BUILTIN_TOOL_IDS = ["SearchTool", "WebSearchTool", "ImageGenerationTool"] |
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.
Unused constant duplicates API logic and risks drift; remove it or consolidate the single source of truth in the API layer.
Prompt for AI agents
Address the following comment on backend/onyx/server/features/default_assistant/models.py at line 8:
<comment>Unused constant duplicates API logic and risks drift; remove it or consolidate the single source of truth in the API layer.</comment>
<file context>
@@ -0,0 +1,43 @@
+
+
+# Valid built-in tool IDs that can be toggled for the default assistant
+VALID_BUILTIN_TOOL_IDS = ["SearchTool", "WebSearchTool", "ImageGenerationTool"]
+
+
</file context>
|
||
tool_ids: list[int] | None = Field( | ||
default=None, | ||
description="List of tool IDs to enable. If provided, must be from VALID_BUILTIN_TOOL_IDS", |
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.
Description says tool IDs must be from VALID_BUILTIN_TOOL_IDS, but that constant contains string names while the API expects database IDs; reword to reflect DB IDs and built-in requirement.
Prompt for AI agents
Address the following comment on backend/onyx/server/features/default_assistant/models.py at line 27:
<comment>Description says tool IDs must be from VALID_BUILTIN_TOOL_IDS, but that constant contains string names while the API expects database IDs; reword to reflect DB IDs and built-in requirement.</comment>
<file context>
@@ -0,0 +1,43 @@
+
+ tool_ids: list[int] | None = Field(
+ default=None,
+ description="List of tool IDs to enable. If provided, must be from VALID_BUILTIN_TOOL_IDS",
+ )
+ system_prompt: str | None = Field(
</file context>
description="List of tool IDs to enable. If provided, must be from VALID_BUILTIN_TOOL_IDS", | |
description="List of database tool IDs to enable; each must be a built-in tool.", |
@@ -0,0 +1,66 @@ | |||
import { AssistantIcon } from "@/components/assistants/AssistantIcon"; |
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.
This component uses React hooks but lacks the required "use client" directive for Next.js Client Components.
Prompt for AI agents
Address the following comment on web/src/app/chat/components/WelcomeMessage.tsx at line 1:
<comment>This component uses React hooks but lacks the required "use client" directive for Next.js Client Components.</comment>
<file context>
@@ -0,0 +1,66 @@
+import { AssistantIcon } from "@/components/assistants/AssistantIcon";
+import { Logo } from "@/components/logo/Logo";
+import { MinimalPersonaSnapshot } from "@/app/admin/assistants/interfaces";
</file context>
console.log("Auth check:"); | ||
console.log(" authDisabled =", authDisabled); | ||
console.log(" user =", !!user); | ||
console.log(" referrer =", referrer); |
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.
Logging the full referrer may capture tokens or sensitive query parameters; avoid printing the raw URL.
Prompt for AI agents
Address the following comment on web/src/lib/chat/fetchChatData.ts at line 132:
<comment>Logging the full referrer may capture tokens or sensitive query parameters; avoid printing the raw URL.</comment>
<file context>
@@ -119,9 +126,11 @@ export async function fetchChatData(searchParams: {
+ console.log("Auth check:");
+ console.log(" authDisabled =", authDisabled);
+ console.log(" user =", !!user);
+ console.log(" referrer =", referrer);
+ console.log(" fromLogin =", isRedirectedFromLogin);
</file context>
console.log(" referrer =", referrer); | |
console.log(" referrer_has_value =", !!referrer); |
|
||
# Verify no starter messages | ||
starter_messages = unified_assistant.starter_messages or [] | ||
assert len(starter_messages) == 0, "Starter messages found" |
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.
Strictly asserting zero starter messages risks flakiness; prefer validating type/shape or allowing empty-or-nonempty.
Prompt for AI agents
Address the following comment on backend/tests/integration/tests/personas/test_unified_assistant.py at line 49:
<comment>Strictly asserting zero starter messages risks flakiness; prefer validating type/shape or allowing empty-or-nonempty.</comment>
<file context>
@@ -0,0 +1,49 @@
+
+ # Verify no starter messages
+ starter_messages = unified_assistant.starter_messages or []
+ assert len(starter_messages) == 0, "Starter messages found"
</file context>
assert ( | ||
"ImageGenerationTool" in tool_names | ||
), "ImageGenerationTool not found in unified assistant" | ||
assert "WebSearchTool" in tool_names, "WebSearchTool not found in unified assistant" |
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.
Unconditionally requiring WebSearchTool makes the test depend on external configuration, increasing flakiness.
Prompt for AI agents
Address the following comment on backend/tests/integration/tests/personas/test_unified_assistant.py at line 45:
<comment>Unconditionally requiring WebSearchTool makes the test depend on external configuration, increasing flakiness.</comment>
<file context>
@@ -0,0 +1,49 @@
+ assert (
+ "ImageGenerationTool" in tool_names
+ ), "ImageGenerationTool not found in unified assistant"
+ assert "WebSearchTool" in tool_names, "WebSearchTool not found in unified assistant"
+
+ # Verify no starter messages
</file context>
const [isSaving, setIsSaving] = useState(false); | ||
const [enabledTools, setEnabledTools] = useState<Set<number>>(new Set()); | ||
const [systemPrompt, setSystemPrompt] = useState<string>(""); | ||
const { data: availableTools } = useSWR<AvailableTool[]>( |
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.
If fetching available tools fails, the page shows no actions and no error, leading to confusing UX; surface and handle the available-tools error similarly to configuration errors.
Prompt for AI agents
Address the following comment on web/src/app/admin/configuration/default-assistant/page.tsx at line 41:
<comment>If fetching available tools fails, the page shows no actions and no error, leading to confusing UX; surface and handle the available-tools error similarly to configuration errors.</comment>
<file context>
@@ -0,0 +1,234 @@
+ const [isSaving, setIsSaving] = useState(false);
+ const [enabledTools, setEnabledTools] = useState<Set<number>>(new Set());
+ const [systemPrompt, setSystemPrompt] = useState<string>("");
+ const { data: availableTools } = useSWR<AvailableTool[]>(
+ "/api/admin/default-assistant/available-tools",
+ errorHandlingFetcher
</file context>
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.
nits
c409d8a
to
a36e24a
Compare
5 Jobs Failed: Run Integration Tests v2 / build-backend-imageStep "Build and push Backend Docker image" from job "build-backend-image" is failing. The last 20 log lines are:
Run Integration Tests v2 / build-integration-imageStep "Build and push integration test Docker image" from job "build-integration-image" is failing. The last 20 log lines are:
Run Integration Tests v2 / build-model-server-imageStep "Build and push Model Server Docker image" from job "build-model-server-image" is failing. The last 20 log lines are:
Run Integration Tests v2 / requiredStep "Run actions/github-script@v7" from job "required" is failing. The last 20 log lines are:
1 job failed running on non-Blacksmith runners. Summary: 7 successful workflows, 2 failed workflows
Last updated: 2025-09-15 03:00:47 UTC |
Description
Fixes https://linear.app/danswer/issue/DAN-2420/create-new-assistant-deprecate-old-assistants
Fixes https://linear.app/danswer/issue/DAN-2421/admin-page-to-adjust-default-assistant
Fixes https://linear.app/danswer/issue/DAN-2422/prompt-tuning-to-make-assistant-use-all-tools-effectively
How Has This Been Tested?
[Describe the tests you ran to verify your changes]
Backporting (check the box to trigger backport action)
Note: You have to check that the action passes, otherwise resolve the conflicts manually and tag the patches.
Summary by cubic
Merge the default assistants into a single “Assistant” (persona 0) with search, web browsing, and image generation. Adds an admin page to configure its capabilities and updates data, tests, and UI for the single-assistant model.
New Features
Migration