-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Fix assistant tools loading #7207 #7232
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
base: main
Are you sure you want to change the base?
Fix assistant tools loading #7207 #7232
Conversation
Greptile SummaryFixed tools appearing as disabled when refreshing or directly loading the assistant edit page by enabling Formik reinitialization and adding loading states. Key changes:
The core issue was that Formik initialized with default values before tool data was fetched, and without Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant AgentEditorPage
participant Formik
participant useMcpServers
participant useOpenApiTools
participant useAvailableTools
participant API
User->>AgentEditorPage: Navigate to /edit page (or refresh)
Note over AgentEditorPage: Page loads, hooks start fetching
AgentEditorPage->>useMcpServers: Initialize hook
AgentEditorPage->>useOpenApiTools: Initialize hook
AgentEditorPage->>useAvailableTools: Initialize hook
useMcpServers->>API: GET /api/admin/mcp/servers
useOpenApiTools->>API: GET /api/tool/openapi
useAvailableTools->>API: GET /api/tool
Note over AgentEditorPage: isPageLoading = true<br/>Loading overlay shown
AgentEditorPage->>Formik: Initialize with default values<br/>(tools appear disabled)
API-->>useMcpServers: Return MCP data
API-->>useOpenApiTools: Return OpenAPI tools
API-->>useAvailableTools: Return available tools
useMcpServers-->>AgentEditorPage: mcpData, isLoading=false
useOpenApiTools-->>AgentEditorPage: openApiTools, isLoading=false
useAvailableTools-->>AgentEditorPage: availableTools, isLoading=false
Note over AgentEditorPage: All loading complete<br/>isPageLoading = false<br/>Loading overlay hidden
Note over AgentEditorPage: initialValues recalculated<br/>with actual tool data
AgentEditorPage->>Formik: Reinitialize form values<br/>(enableReinitialize=true)
Note over Formik: Form updates with correct<br/>tool states (enabled/disabled)
Formik-->>User: Display correctly initialized form
|
Greptile's behavior is changing!From now on, if a review finishes with no comments, we will not post an additional "statistics" comment to confirm that our review found nothing to comment on. However, you can confirm that we reviewed your changes in the status check section. This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR". |
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.
1 issue found across 3 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="web/src/app/chat/message/MemoizedTextComponents.tsx">
<violation number="1" location="web/src/app/chat/message/MemoizedTextComponents.tsx:81">
P0: This `useMemo` is called conditionally (inside nested `if` blocks) and after an early return statement, violating React's Rules of Hooks. Hooks must be called at the top level of the component in the same order on every render. This will cause React to crash with "Rendered more hooks than during the previous render" when the component takes different code paths.
Consider moving the memoization logic outside the conditionals, or simply removing the `useMemo` since the object creation here is inexpensive and the parent component is already wrapped with `memo`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
2d0819c to
cb2b8b5
Compare
|
@Weves can you review this PR and merge it !! |
- Switch citation wrapper from `<span>` to `<a>` to leverage native link behavior, which is more robust during DOM churn. - Memoize props in `MemoizedTextComponents` to stabilize the Virtual DOM and prevent unnecessary re-renders of citations as message chunks arrive. - Add `stopPropagation` and `preventDefault` to citation click handlers to ensure consistent behavior across a "
…efresh (onyx-dot-app#7207) - Enable in to allow form values to update once asynchronous tool data is available. - Add a loading overlay to [AgentEditorPage](cci:1://file:///Users/kumardhananjaya/Desktop/Projects/onyx/web/src/refresh-pages/AgentEditorPage.tsx:446:0-1440:1) to prevent interaction before tools are fully synchronized. - Update tool fetching hooks to expose status to the editor page.
cb2b8b5 to
e44e85e
Compare
Description
This PR fixes an issue where enabled tools appeared as disabled when directly loading or refreshing the assistant edit page.
When the page was refreshed, tool configuration data was loaded asynchronously after the form was initialized, causing Formik to retain stale default values. This resulted in an incorrect UI state where tools looked disabled despite being enabled in the backend.
Changes made:
Enabled enableReinitialize in Formik so form values correctly update once tool data is fetched.
Added a loading overlay to the AgentEditorPage to prevent user interaction before tool states are fully synchronized.
Updated tool-fetching hooks to expose an isLoading state, allowing the editor to render only after data is ready.
These changes ensure tool states are accurately reflected after page refreshes and direct navigation.
How Has This Been Tested?
Created a custom assistant with one or more tools enabled.
Navigated to the assistant edit page via Admin → Assistants → Edit and confirmed tools appeared enabled.
Refreshed the page directly and verified tool states remained enabled.
Tested on a self-hosted v2.7.0 instance to confirm consistent behavior.
Issue Tagged: #7207
Summary by cubic
Fixes incorrect tool states on the Assistant Editor after refresh by reinitializing form values when tool data loads and blocking interaction until tools finish loading. Also improves citation click reliability during streaming. Fixes #7207.
Written for commit 49c007c. Summary will update on new commits.