Skip to content

Conversation

Weves
Copy link
Contributor

@Weves Weves commented Sep 7, 2025

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.

  • This PR should be backported (make sure to check that the backport attempt succeeds)
  • [Optional] Override Linear Check

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

    • One default assistant (ID 0) with combined search, web, and image capabilities.
    • Admin page to enable/disable Search, Internet Search, and Image Generation tools at /admin/configuration/default-assistant.
    • Chat welcome component with short random greetings; hides the assistant name for ID 0.
    • Prebuilt personas updated: Assistant replaces Search; Paraphrase kept but not default.
    • Integration tests for the unified assistant and configuration.
  • Migration

    • Alembic migration updates persona 0 and marks General (ID 1) and Art (ID 3) as hidden/deleted; Paraphrase (ID 2) no longer default.
    • Assigns built-in tools to persona 0 (SearchTool always; InternetSearchTool/ImageGenerationTool if present).
    • Migrates user preference arrays to replace IDs 1/3 with 0 where relevant and hides 1/3.
    • Downgrade restores original persona settings but does not revert user preferences.

Copy link

vercel bot commented Sep 7, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
internal-search Ready Ready Preview Comment Sep 15, 2025 2:53am

Copy link
Contributor

@greptile-apps greptile-apps bot left a 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

Edit Code Review Bot Settings | Greptile

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a 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."];
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Sep 8, 2025

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 = [&quot;How can I help?&quot;, &quot;Let&#39;s get started.&quot;];
+
+export function getRandomGreeting(): string {
</file context>
Fix with Cubic

export function getRandomGreeting(): string {
return GREETING_MESSAGES[
Math.floor(Math.random() * GREETING_MESSAGES.length)
] as string;
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Sep 8, 2025

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>
Fix with Cubic



# Valid built-in tool IDs that can be toggled for the default assistant
VALID_BUILTIN_TOOL_IDS = ["SearchTool", "WebSearchTool", "ImageGenerationTool"]
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Sep 8, 2025

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 = [&quot;SearchTool&quot;, &quot;WebSearchTool&quot;, &quot;ImageGenerationTool&quot;]
+
+
</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",
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Sep 8, 2025

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=&quot;List of tool IDs to enable. If provided, must be from VALID_BUILTIN_TOOL_IDS&quot;,
+    )
+    system_prompt: str | None = Field(
</file context>
Suggested change
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.",
Fix with Cubic

@@ -0,0 +1,66 @@
import { AssistantIcon } from "@/components/assistants/AssistantIcon";
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Sep 8, 2025

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 &quot;use client&quot; directive for Next.js Client Components.</comment>

<file context>
@@ -0,0 +1,66 @@
+import { AssistantIcon } from &quot;@/components/assistants/AssistantIcon&quot;;
+import { Logo } from &quot;@/components/logo/Logo&quot;;
+import { MinimalPersonaSnapshot } from &quot;@/app/admin/assistants/interfaces&quot;;
</file context>
Fix with Cubic

console.log("Auth check:");
console.log(" authDisabled =", authDisabled);
console.log(" user =", !!user);
console.log(" referrer =", referrer);
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Sep 8, 2025

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(&quot;Auth check:&quot;);
+    console.log(&quot;  authDisabled =&quot;, authDisabled);
+    console.log(&quot;  user =&quot;, !!user);
+    console.log(&quot;  referrer =&quot;, referrer);
+    console.log(&quot;  fromLogin =&quot;, isRedirectedFromLogin);
 
</file context>
Suggested change
console.log(" referrer =", referrer);
console.log(" referrer_has_value =", !!referrer);
Fix with Cubic


# Verify no starter messages
starter_messages = unified_assistant.starter_messages or []
assert len(starter_messages) == 0, "Starter messages found"
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Sep 8, 2025

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, &quot;Starter messages found&quot;
</file context>

assert (
"ImageGenerationTool" in tool_names
), "ImageGenerationTool not found in unified assistant"
assert "WebSearchTool" in tool_names, "WebSearchTool not found in unified assistant"
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Sep 8, 2025

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 (
+        &quot;ImageGenerationTool&quot; in tool_names
+    ), &quot;ImageGenerationTool not found in unified assistant&quot;
+    assert &quot;WebSearchTool&quot; in tool_names, &quot;WebSearchTool not found in unified assistant&quot;
+
+    # 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[]>(
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Sep 8, 2025

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&lt;Set&lt;number&gt;&gt;(new Set());
+  const [systemPrompt, setSystemPrompt] = useState&lt;string&gt;(&quot;&quot;);
+  const { data: availableTools } = useSWR&lt;AvailableTool[]&gt;(
+    &quot;/api/admin/default-assistant/available-tools&quot;,
+    errorHandlingFetcher
</file context>
Fix with Cubic

Copy link
Contributor

@evan-onyx evan-onyx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nits

@blacksmith-sh blacksmith-sh bot deleted a comment from Weves Sep 15, 2025
@blacksmith-sh blacksmith-sh bot deleted a comment from Weves Sep 15, 2025
Copy link

blacksmith-sh bot commented Sep 15, 2025

5 Jobs Failed:

Run Integration Tests v2 / build-backend-image

Step "Build and push Backend Docker image" from job "build-backend-image" is failing. The last 20 log lines are:

[...]
google.golang.org/grpc.(*ClientConn).Invoke
	google.golang.org/grpc@v1.69.4/call.go:35
github.com/moby/buildkit/api/services/control.(*controlClient).Solve
	github.com/moby/buildkit@v0.21.0/api/services/control/control_grpc.pb.go:88
github.com/moby/buildkit/client.(*Client).solve.func2
	github.com/moby/buildkit@v0.21.0/client/solve.go:268
golang.org/x/sync/errgroup.(*Group).Go.func1
	golang.org/x/sync@v0.13.0/errgroup/errgroup.go:79
runtime.goexit
	runtime/asm_arm64.s:1223

3912 v0.23.0 /home/runner/.docker/cli-plugins/docker-buildx buildx build --file ./backend/Dockerfile --iidfile /home/runner/_work/_temp/docker-actions-toolkit-Gfx0iw/build-iidfile-7fd8f675a7.txt --platform linux/arm64 --attest type=provenance,mode=max,builder-id=https://github.com/***/onyx/actions/runs/17720702082/attempts/1 --tag experimental-registry.blacksmith.sh:5000/integration-test-onyx-backend:test-17720702082 --metadata-file /home/runner/_work/_temp/docker-actions-toolkit-Gfx0iw/build-metadata-4e047d6f2a.json --push ./backend --debug
github.com/moby/buildkit/client.(*Client).solve.func2
	github.com/moby/buildkit@v0.21.0/client/solve.go:285
golang.org/x/sync/errgroup.(*Group).Go.func1
	golang.org/x/sync@v0.13.0/errgroup/errgroup.go:79

  > Reporting build completion
Error: buildx failed with: golang.org/x/sync@v0.13.0/errgroup/errgroup.go:79
Run Integration Tests v2 / build-integration-image

Step "Build and push integration test Docker image" from job "build-integration-image" is failing. The last 20 log lines are:

[...]
google.golang.org/grpc.(*ClientConn).Invoke
	google.golang.org/grpc@v1.69.4/call.go:35
github.com/moby/buildkit/api/services/control.(*controlClient).Solve
	github.com/moby/buildkit@v0.21.0/api/services/control/control_grpc.pb.go:88
github.com/moby/buildkit/client.(*Client).solve.func2
	github.com/moby/buildkit@v0.21.0/client/solve.go:268
golang.org/x/sync/errgroup.(*Group).Go.func1
	golang.org/x/sync@v0.13.0/errgroup/errgroup.go:79
runtime.goexit
	runtime/asm_arm64.s:1223

3948 v0.23.0 /home/runner/.docker/cli-plugins/docker-buildx buildx build --file ./backend/tests/integration/Dockerfile --iidfile /home/runner/_work/_temp/docker-actions-toolkit-6WV2Y5/build-iidfile-f9966364a0.txt --platform linux/arm64 --attest type=provenance,mode=max,builder-id=https://github.com/***/onyx/actions/runs/17720702082/attempts/1 --tag experimental-registry.blacksmith.sh:5000/integration-test-onyx-integration:test-17720702082 --metadata-file /home/runner/_work/_temp/docker-actions-toolkit-6WV2Y5/build-metadata-84c4c3c11d.json --push ./backend --debug
github.com/moby/buildkit/client.(*Client).solve.func2
	github.com/moby/buildkit@v0.21.0/client/solve.go:285
golang.org/x/sync/errgroup.(*Group).Go.func1
	golang.org/x/sync@v0.13.0/errgroup/errgroup.go:79

  > Reporting build completion
Error: buildx failed with: golang.org/x/sync@v0.13.0/errgroup/errgroup.go:79
Run Integration Tests v2 / build-model-server-image

Step "Build and push Model Server Docker image" from job "build-model-server-image" is failing. The last 20 log lines are:

[...]
google.golang.org/grpc.(*ClientConn).Invoke
	google.golang.org/grpc@v1.69.4/call.go:35
github.com/moby/buildkit/api/services/control.(*controlClient).Solve
	github.com/moby/buildkit@v0.21.0/api/services/control/control_grpc.pb.go:88
github.com/moby/buildkit/client.(*Client).solve.func2
	github.com/moby/buildkit@v0.21.0/client/solve.go:268
golang.org/x/sync/errgroup.(*Group).Go.func1
	golang.org/x/sync@v0.13.0/errgroup/errgroup.go:79
runtime.goexit
	runtime/asm_arm64.s:1223

3898 v0.23.0 /home/runner/.docker/cli-plugins/docker-buildx buildx build --file ./backend/Dockerfile.model_server --iidfile /home/runner/_work/_temp/docker-actions-toolkit-7BogK0/build-iidfile-9c17bbcde5.txt --output type=registry --platform linux/arm64 --attest type=provenance,disabled=true --tag experimental-registry.blacksmith.sh:5000/integration-test-onyx-model-server:test-17720702082 --metadata-file /home/runner/_work/_temp/docker-actions-toolkit-7BogK0/build-metadata-6de6e7427c.json --push ./backend --debug
github.com/moby/buildkit/client.(*Client).solve.func2
	github.com/moby/buildkit@v0.21.0/client/solve.go:285
golang.org/x/sync/errgroup.(*Group).Go.func1
	golang.org/x/sync@v0.13.0/errgroup/errgroup.go:79

  > Reporting build completion
Error: buildx failed with: golang.org/x/sync@v0.13.0/errgroup/errgroup.go:79
Run Integration Tests v2 / required

Step "Run actions/github-script@v7" from job "required" is failing. The last 20 log lines are:

[...]
  retry-exempt-status-codes: 400,401,403,404,422
env:
  PRIVATE_REGISTRY: experimental-registry.blacksmith.sh:5000
  PRIVATE_REGISTRY_USERNAME: ***
  PRIVATE_REGISTRY_PASSWORD: ***
  OPENAI_API_KEY: ***
  SLACK_BOT_TOKEN: ***
  CONFLUENCE_TEST_SPACE_URL: ***
  CONFLUENCE_USER_NAME: ***
  CONFLUENCE_ACCESS_TOKEN: ***
  JIRA_BASE_URL: ***
  JIRA_USER_EMAIL: ***
  JIRA_API_TOKEN: ***
  PERM_SYNC_SHAREPOINT_CLIENT_ID: ***
  PERM_SYNC_SHAREPOINT_PRIVATE_KEY: ***
  PERM_SYNC_SHAREPOINT_CERTIFICATE_PASSWORD: ***
  PERM_SYNC_SHAREPOINT_DIRECTORY_ID: ***
  GITHUB_REPO_NAME: ***/onyx
Error: One or more upstream jobs failed or were cancelled.

1 job failed running on non-Blacksmith runners.


Summary: 7 successful workflows, 2 failed workflows

Last updated: 2025-09-15 03:00:47 UTC

@Weves Weves merged commit bb239d5 into main Sep 15, 2025
20 of 26 checks passed
@Weves Weves deleted the single-assistant-basic branch September 15, 2025 03:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants