refactor: Add create knowledge button#9520
Conversation
…ndleCreateKnowledge function into KnowledgeBasesTab - Refactored KnowledgeBaseEmptyState to accept handleCreateKnowledge as a prop, removing internal logic for flow creation. - Updated KnowledgeBasesTab to implement handleCreateKnowledge, enhancing the flow creation process and improving component interaction.
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughReplaces internal flow-creation logic in KnowledgeBaseEmptyState with a prop-driven handler. Moves creation/navigation/analytics logic into KnowledgeBasesTab, wiring a toolbar button and empty state to a new handleCreateKnowledge that creates a Knowledge Ingestion flow, updates IDs, navigates via folderId from URL or fallback, and tracks an event. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant K as KnowledgeBasesTab
participant FM as FlowsManager
participant ST as Stores (user/collections)
participant R as Router
participant A as Analytics
U->>K: Click "Create knowledge" (toolbar or empty state)
K->>ST: Resolve folderId (URL params or user collection)
K->>FM: Get "Knowledge Ingestion" example
FM-->>K: Example flow template
K->>FM: Create new flow with updated IDs
FM-->>K: New flow id
K->>R: Navigate to /.../folders/{folderId}/flows/{newFlowId}
K->>A: Track "New Flow Created"
R-->>U: Show new flow page
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
github-actions
Bot
added
refactor
| const KnowledgeBaseEmptyState = () => { | ||
| const examples = useFlowsManagerStore((state) => state.examples); | ||
| const addFlow = useAddFlow(); | ||
| const navigate = useCustomNavigate(); | ||
| const { folderId } = useParams(); | ||
| const myCollectionId = useFolderStore((state) => state.myCollectionId); | ||
|
|
||
| const folderIdUrl = folderId ?? myCollectionId; | ||
|
|
||
| const handleCreateKnowledge = async () => { | ||
| const knowledgeBasesExample = examples.find( | ||
| (example) => example.name === "Knowledge Ingestion", | ||
| ); | ||
|
|
||
| if (knowledgeBasesExample && knowledgeBasesExample.data) { | ||
| updateIds(knowledgeBasesExample.data); | ||
| addFlow({ flow: knowledgeBasesExample }).then((id) => { | ||
| navigate(`/flow/${id}/folder/${folderIdUrl}`); | ||
| }); | ||
| track("New Flow Created", { | ||
| template: `${knowledgeBasesExample.name} Template`, | ||
| }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
moved this to the parent component because both empty and header buttons do the same thing.
github-actions
Bot
added
refactor
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/frontend/src/pages/MainPage/pages/filesPage/components/KnowledgeBaseEmptyState.tsx (1)
18-31: Align label casing and add a stable test id for the button
- The toolbar uses “Create knowledge” while this button says “Create Knowledge.” Pick one style to avoid UI inconsistency.
- Add a data-testid to make E2E tests less brittle.
Apply this diff:
- <Button - onClick={handleCreateKnowledge} - className="!px-3 md:!px-4 md:!pl-3.5" - > + <Button + onClick={handleCreateKnowledge} + className="!px-3 md:!px-4 md:!pl-3.5" + data-testid="create-knowledge-btn" + > @@ - <span className="whitespace-nowrap font-semibold"> - Create Knowledge - </span> + <span className="whitespace-nowrap font-semibold">Create knowledge</span>src/frontend/src/pages/MainPage/pages/filesPage/components/KnowledgeBasesTab.tsx (1)
196-201: Harden toolbar CTA for testing and UX
- Add a data-testid to stabilize tests.
- Consider disabling the button while creation is in progress to prevent duplicate flows (can be done with a simple isCreating state).
Apply this diff:
- <Button - className="flex items-center gap-2 font-semibold" - onClick={handleCreateKnowledge} - > - <ForwardedIconComponent name="Plus" /> Create knowledge - </Button> + <Button + className="flex items-center gap-2 font-semibold" + onClick={handleCreateKnowledge} + data-testid="create-knowledge-toolbar-btn" + > + <ForwardedIconComponent name="Plus" /> Create knowledge + </Button>
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/frontend/src/pages/MainPage/pages/filesPage/components/KnowledgeBaseEmptyState.tsx(1 hunks)src/frontend/src/pages/MainPage/pages/filesPage/components/KnowledgeBasesTab.tsx(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/frontend/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend_development.mdc)
src/frontend/src/**/*.{ts,tsx,js,jsx}: All frontend TypeScript and JavaScript code should be located under src/frontend/src/ and organized into components, pages, icons, stores, types, utils, hooks, services, and assets directories as per the specified directory layout.
Use React 18 with TypeScript for all UI components in the frontend.
Format all TypeScript and JavaScript code using the make format_frontend command.
Lint all TypeScript and JavaScript code using the make lint command.
Files:
src/frontend/src/pages/MainPage/pages/filesPage/components/KnowledgeBaseEmptyState.tsxsrc/frontend/src/pages/MainPage/pages/filesPage/components/KnowledgeBasesTab.tsx
🧠 Learnings (1)
📚 Learning: 2025-06-23T12:46:42.048Z
Learnt from: CR
PR: langflow-ai/langflow#0
File: .cursor/rules/frontend_development.mdc:0-0
Timestamp: 2025-06-23T12:46:42.048Z
Learning: Custom React Flow node types should be implemented as memoized components, using Handle components for connection points and supporting optional icons and labels.
Applied to files:
src/frontend/src/pages/MainPage/pages/filesPage/components/KnowledgeBasesTab.tsx
🧬 Code graph analysis (1)
src/frontend/src/pages/MainPage/pages/filesPage/components/KnowledgeBasesTab.tsx (4)
src/frontend/src/stores/foldersStore.tsx (1)
useFolderStore(4-29)src/frontend/src/utils/reactflowUtils.ts (1)
updateIds(563-624)src/frontend/src/customization/utils/analytics.ts (1)
track(1-7)src/frontend/src/components/common/genericIconComponent/index.tsx (1)
ForwardedIconComponent(15-146)
🔇 Additional comments (4)
src/frontend/src/pages/MainPage/pages/filesPage/components/KnowledgeBaseEmptyState.tsx (2)
4-8: LGTM: Presentational empty-state is now prop-drivenGood separation of concerns. Moving creation/navigation logic out keeps this component clean and reusable.
22-26: No changes needed for ForwardedIconComponent import.
Verified thatsrc/frontend/src/components/common/genericIconComponent/index.tsxincludes bothexport const ForwardedIconComponent = memo(/*…*/) export default ForwardedIconComponent;ensuring the existing default import is valid.
src/frontend/src/pages/MainPage/pages/filesPage/components/KnowledgeBasesTab.tsx (2)
176-179: LGTM: Empty state correctly wired to creation handlerRendering KnowledgeBaseEmptyState with the new prop keeps the creation flow discoverable when the list is empty.
8-12: No changes needed: default export ofForwardedIconComponentis presentThe verification shows that
src/frontend/src/components/common/genericIconComponent/index.tsxincludes bothexport const ForwardedIconComponent = memo( forwardRef(/* … */) ); export default ForwardedIconComponent;Since the module explicitly provides a default export, importing it as the default is correct and will not result in an undefined component at runtime.
No modifications are required to the import in
KnowledgeBasesTab.tsx(or inKnowledgeBaseEmptyState.tsx).Likely an incorrect or invalid review comment.
| const examples = useFlowsManagerStore((state) => state.examples); | ||
| const addFlow = useAddFlow(); | ||
| const navigate = useCustomNavigate(); | ||
| const { folderId } = useParams(); | ||
| const myCollectionId = useFolderStore((state) => state.myCollectionId); | ||
| const folderIdUrl = folderId ?? myCollectionId; | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Guard against empty folderIdUrl to avoid navigating to an invalid route
If both folderId (from URL) and myCollectionId (from store) are empty, folderIdUrl becomes "", producing /flow/${id}/folder/ which might not match any route.
- Either make the
foldersegment optional in routing, or - Fallback to
/flow/${id}whenfolderIdUrlis falsy.
Run this to inspect route definitions and confirm whether the folder segment is optional:
🏁 Script executed:
#!/bin/bash
# Look for flow routes and whether /folder/:folderId is required
rg -nP -C3 'path\s*:\s*["\']\/flow\/:id(\/folder\/:folderId)?["\']' src/frontend/src || true
rg -nP -C3 'path\s*:\s*["\']\/flow\/:id\/folder\/:folderId["\']' src/frontend/src || trueLength of output: 275
🏁 Script executed:
#!/usr/bin/env bash
# Search for explicit route definitions for flow with folder segment
echo "=== /flow/:id/folder/:folderId routes ==="
rg -n -C2 "/flow/:id/folder/:folderId" -t tsx -t ts src/frontend/src || true
echo
# Search for routes matching /flow/:id (without folder)
echo "=== /flow/:id routes ==="
rg -n -C2 "/flow/:id" -t tsx -t ts src/frontend/src || trueLength of output: 350
🏁 Script executed:
#!/usr/bin/env bash
# Search for router definitions in the frontend source
echo "=== createBrowserRouter or RouterProvider usage ==="
rg -n 'createBrowserRouter' -C2 src/frontend/src || true
rg -n 'RouterProvider' -C2 src/frontend/src || true
echo
echo "=== Route path definitions ==="
rg -nP "path\\s*:" -C2 src/frontend/src || trueLength of output: 32171
🏁 Script executed:
#!/usr/bin/env bash
# Display the route definitions in routes.tsx to inspect /flow routes
head -n 100 src/frontend/src/routes.tsxLength of output: 4098
🏁 Script executed:
#!/usr/bin/env bash
# Display children routes under “flows/” to inspect folder segment definitions
sed -n '101,200p' src/frontend/src/routes.tsxLength of output: 3574
Guard against invalid folder routes before navigation
The router in src/frontend/src/routes.tsx defines both
• /flow/:id/ – renders <FlowPage /> via the empty-string child route
• /flow/:id/folder/:folderId/ – also renders <FlowPage /> when folderId is present
However, if folderIdUrl (from useParams() or the store) is empty, navigating to
/flow/${id}/folder/
won’t match either route. To fix this, conditionally include the /folder/:folderId segment only when folderIdUrl is truthy, falling back to /flow/${id} otherwise.
• File: src/pages/MainPage/pages/filesPage/components/KnowledgeBasesTab.tsx
• Lines: 59–65
Suggested diff:
- const { folderId } = useParams();
- const myCollectionId = useFolderStore((state) => state.myCollectionId);
- const folderIdUrl = folderId ?? myCollectionId;
+ const { folderId } = useParams();
+ const myCollectionId = useFolderStore((state) => state.myCollectionId);
+ const folderIdUrl = folderId ?? myCollectionId;
+ // Only include /folder/:folderId when we have an ID
+ const folderSegment = folderIdUrl ? `/folder/${folderIdUrl}` : "";
const handleNavigate = (flowId: string) => {
- navigate(`/flow/${flowId}/folder/${folderIdUrl}`);
+ navigate(`/flow/${flowId}${folderSegment}`);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const examples = useFlowsManagerStore((state) => state.examples); | |
| const addFlow = useAddFlow(); | |
| const navigate = useCustomNavigate(); | |
| const { folderId } = useParams(); | |
| const myCollectionId = useFolderStore((state) => state.myCollectionId); | |
| const folderIdUrl = folderId ?? myCollectionId; | |
| const examples = useFlowsManagerStore((state) => state.examples); | |
| const addFlow = useAddFlow(); | |
| const navigate = useCustomNavigate(); | |
| const { folderId } = useParams(); | |
| const myCollectionId = useFolderStore((state) => state.myCollectionId); | |
| const folderIdUrl = folderId ?? myCollectionId; | |
| // Only include /folder/:folderId when we have an ID | |
| const folderSegment = folderIdUrl ? `/folder/${folderIdUrl}` : ""; | |
| const handleNavigate = (flowId: string) => { | |
| navigate(`/flow/${flowId}${folderSegment}`); | |
| }; |
🤖 Prompt for AI Agents
In
src/frontend/src/pages/MainPage/pages/filesPage/components/KnowledgeBasesTab.tsx
around lines 59 to 65, the code builds a navigation path that always appends the
'/folder/:folderId' segment using folderIdUrl which can be empty, producing a
route like '/flow/${id}/folder/' that doesn't match any route; change the
navigation logic to conditionally include the folder segment only when
folderIdUrl is truthy (use '/flow/${id}/folder/${folderIdUrl}' when truthy,
otherwise use '/flow/${id}') so navigation only generates valid routes.
| const handleCreateKnowledge = async () => { | ||
| const knowledgeBasesExample = examples.find( | ||
| (example) => example.name === "Knowledge Ingestion", | ||
| ); | ||
|
|
||
| if (knowledgeBasesExample && knowledgeBasesExample.data) { | ||
| updateIds(knowledgeBasesExample.data); | ||
| addFlow({ flow: knowledgeBasesExample }).then((id) => { | ||
| navigate(`/flow/${id}/folder/${folderIdUrl}`); | ||
| }); | ||
| track("New Flow Created", { | ||
| template: `${knowledgeBasesExample.name} Template`, | ||
| }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Clone template before updateIds, add error handling, and include flow id in analytics
- updateIds mutates nodes/edges in place. Passing store-backed
exampleswill mutate shared state. Deep-clone first. - Handle addFlow failures to provide user feedback.
- Include the created flow id in analytics for traceability.
- Navigate to
/flow/${id}whenfolderIdUrlis missing.
Apply this diff:
- const handleCreateKnowledge = async () => {
- const knowledgeBasesExample = examples.find(
- (example) => example.name === "Knowledge Ingestion",
- );
-
- if (knowledgeBasesExample && knowledgeBasesExample.data) {
- updateIds(knowledgeBasesExample.data);
- addFlow({ flow: knowledgeBasesExample }).then((id) => {
- navigate(`/flow/${id}/folder/${folderIdUrl}`);
- });
- track("New Flow Created", {
- template: `${knowledgeBasesExample.name} Template`,
- });
- }
- };
+ const handleCreateKnowledge = async () => {
+ const tpl = examples.find((e) => e.name === "Knowledge Ingestion");
+ if (!tpl?.data) {
+ setErrorData({
+ title: "Template not found",
+ list: ['The "Knowledge Ingestion" template is missing.'],
+ });
+ return;
+ }
+ // Clone to avoid mutating examples store
+ const flowCopy = typeof structuredClone === "function" ? structuredClone(tpl) : JSON.parse(JSON.stringify(tpl));
+ updateIds(flowCopy.data);
+ try {
+ const id = await addFlow({ flow: flowCopy });
+ track(
+ "New Flow Created",
+ { template: `${tpl.name} Template` },
+ id,
+ );
+ const targetPath = folderIdUrl ? `/flow/${id}/folder/${folderIdUrl}` : `/flow/${id}`;
+ navigate(targetPath);
+ } catch (e: any) {
+ setErrorData({
+ title: "Failed to create knowledge flow",
+ list: [e?.response?.data?.detail || e?.message || "An unknown error occurred"],
+ });
+ }
+ };Optional follow-ups:
- Wrap
handleCreateKnowledgeinuseCallbackto keep a stable reference for children. - Replace the hard-coded template name with a constant or a templateId to avoid brittleness.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleCreateKnowledge = async () => { | |
| const knowledgeBasesExample = examples.find( | |
| (example) => example.name === "Knowledge Ingestion", | |
| ); | |
| if (knowledgeBasesExample && knowledgeBasesExample.data) { | |
| updateIds(knowledgeBasesExample.data); | |
| addFlow({ flow: knowledgeBasesExample }).then((id) => { | |
| navigate(`/flow/${id}/folder/${folderIdUrl}`); | |
| }); | |
| track("New Flow Created", { | |
| template: `${knowledgeBasesExample.name} Template`, | |
| }); | |
| } | |
| }; | |
| const handleCreateKnowledge = async () => { | |
| const tpl = examples.find((e) => e.name === "Knowledge Ingestion"); | |
| if (!tpl?.data) { | |
| setErrorData({ | |
| title: "Template not found", | |
| list: ['The "Knowledge Ingestion" template is missing.'], | |
| }); | |
| return; | |
| } | |
| // Clone to avoid mutating examples store | |
| const flowCopy = | |
| typeof structuredClone === "function" | |
| ? structuredClone(tpl) | |
| : JSON.parse(JSON.stringify(tpl)); | |
| updateIds(flowCopy.data); | |
| try { | |
| const id = await addFlow({ flow: flowCopy }); | |
| track( | |
| "New Flow Created", | |
| { template: `${tpl.name} Template` }, | |
| id, | |
| ); | |
| const targetPath = folderIdUrl | |
| ? `/flow/${id}/folder/${folderIdUrl}` | |
| : `/flow/${id}`; | |
| navigate(targetPath); | |
| } catch (e: any) { | |
| setErrorData({ | |
| title: "Failed to create knowledge flow", | |
| list: [ | |
| e?.response?.data?.detail || | |
| e?.message || | |
| "An unknown error occurred", | |
| ], | |
| }); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In
src/frontend/src/pages/MainPage/pages/filesPage/components/KnowledgeBasesTab.tsx
around lines 138-152, the handler mutates the shared examples object, lacks
error handling for addFlow, doesn't include the created flow id in analytics,
and doesn't handle missing folderIdUrl in navigation; fix by deep-cloning
knowledgeBasesExample.data (e.g., structuredClone or
JSON.parse(JSON.stringify(...))) before calling updateIds to avoid mutating
store-backed state, await addFlow inside try/catch and show user feedback on
failure, on success navigate to `/flow/${id}/folder/${folderIdUrl}` but fall
back to `/flow/${id}` when folderIdUrl is falsy, and include the returned id in
the track call payload (e.g., { template: `${name} Template`, flowId: id }) so
analytics contains the flow id.
|
Frontend Unit Test Coverage ReportCoverage Summary
Unit Test Results
|
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (40.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #9520 +/- ##
==========================================
- Coverage 33.82% 33.82% -0.01%
==========================================
Files 1196 1196
Lines 56389 56392 +3
Branches 5335 5335
==========================================
Hits 19072 19072
- Misses 37247 37250 +3
Partials 70 70
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
2 participants
Just adds a button to allow users to create knowledge using a template
Summary by CodeRabbit
New Features
Refactor