feat: add search, filters, and plugin sync to Node Library#387
Conversation
- Add search input and category/plugin filter chips to NodePalette - Move I/O nodes (file_reader, file_writer, object_store_writer) from core to io top-level category for better discoverability - Fix plugin nodes not appearing by syncing schemas after plugin load (race condition: UI fetched schemas before background plugin loading completed, and never re-fetched) - Add syncPluginSchemas() helper in schemaStore - Add 14 unit tests for search, filter, and empty-state behavior Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
|
✅ Reviewed on |
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![staging-devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/870844?s=60&v=4){kind=small}
| .catch((err) => { | ||
| logger.error('Failed to load plugins', err); | ||
| toastRef.current.error('Could not load plugin list. Check logs for details.'); |
There was a problem hiding this comment.
🟡 Misleading error message when syncPluginSchemas fails after plugins loaded successfully
In the useEffect at ui/src/panes/ControlPane.tsx:307-317, the .catch handler covers both ensurePluginsLoaded() and syncPluginSchemas(). If ensurePluginsLoaded succeeds but syncPluginSchemas throws (e.g., reloadSchemas() gets a network error at ui/src/stores/schemaStore.ts:86-90), the user sees "Could not load plugin list" — which is wrong because the plugin list loaded fine; it was the schema reload that failed. This could mislead users and developers debugging the issue.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
There was a problem hiding this comment.
Fixed in 4a97002 — separated error handling so syncPluginSchemas failures are caught independently with their own log message (Failed to sync plugin schemas), while ensurePluginsLoaded failures continue to show the toast.
| serde_json::to_value(schema_for!(file_read::FileReadConfig)) | ||
| .expect("FileReadConfig schema should serialize to JSON"), | ||
| vec!["core".to_string(), "io".to_string()], | ||
| vec!["io".to_string(), "file".to_string()], |
There was a problem hiding this comment.
📝 Info: Category tag changes are a breaking change for any downstream consumers
The Rust-side changes remove "core" from the categories of core::file_reader, core::file_writer, and core::object_store_writer, replacing them with ["io", "file"], ["io", "file"], and ["io", "object_store"] respectively. This changes the top-level category from "core" to "io" for these nodes. Any external tool, automation, or saved UI state that relied on these nodes being in the "core" category would be affected. I searched for hardcoded references to "core" category filtering in the UI and found none — the UI derives categories dynamically from the API response. The test data at NodePalette.test.tsx:24 already uses the new ['io', 'file'] categories, confirming intentional alignment.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
There was a problem hiding this comment.
Correct — UI derives categories dynamically so no hardcoded references break. The kind string is unchanged (still core::file_reader etc.), only the display category hierarchy shifts from core > io to io > file.
- Separate error handling in ControlPane/MonitorView so plugin-load failures and schema-sync failures produce distinct log messages - Add single-flight guard to syncPluginSchemas to prevent redundant fetches when multiple views mount concurrently - Rename PLUGIN_FILTER from 'plugin' to '__plugin__' to avoid collision with a potential top-level category named 'plugin' - Memoize groups/topKeys computation in NodePalette Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
| export function syncPluginSchemas(pluginKinds: Iterable<string>): Promise<void> { | ||
| if (syncInFlight) return syncInFlight; | ||
|
|
||
| const schemaKinds = new Set(useSchemaStore.getState().nodeDefinitions.map((d) => d.kind)); | ||
| const missing = [...pluginKinds].some((kind) => !schemaKinds.has(kind)); | ||
| if (!missing) return Promise.resolve(); | ||
|
|
||
| syncInFlight = reloadSchemas().finally(() => { | ||
| syncInFlight = null; | ||
| }); | ||
| return syncInFlight; | ||
| } |
There was a problem hiding this comment.
📝 Info: syncPluginSchemas single-flight dedup won't retry if server hasn't finished loading plugins
The syncPluginSchemas function at ui/src/stores/schemaStore.ts:76-87 runs once per component mount (called from useEffect([], []) in both ControlPane and MonitorView). If the server hasn't finished loading a plugin by the time the reload completes, the plugin's schema will still be missing. Since the effect only runs once, there's no automatic retry mechanism. This is a deliberate design choice (best-effort sync), but means a page refresh may be needed if the server is slow to register plugins. The ControlPane's upload flow (handlePluginFilesSelected) does call reloadSchemas() directly after upload, so manually uploaded plugins are covered — this only affects plugins loaded at server startup that race with the initial schema fetch.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
There was a problem hiding this comment.
Correct — this is best-effort by design. The ensurePluginsLoaded call fetches /api/v1/plugins, so by the time syncPluginSchemas runs, the server has already reported those plugins as loaded. The race window is narrow (between initial schema fetch and plugin registration completing). A page refresh covers the edge case.
| React.useEffect(() => { | ||
| ensurePluginsLoaded() | ||
| .then(() => { | ||
| const kinds = usePluginStore.getState().plugins.map((p) => p.kind); | ||
| return syncPluginSchemas(kinds).catch((err) => { | ||
| viewsLogger.error('Failed to sync plugin schemas', err); | ||
| }); | ||
| }) | ||
| .catch((err) => { | ||
| viewsLogger.error('Failed to load plugins', err); | ||
| }); | ||
| }, []); |
There was a problem hiding this comment.
📝 Info: MonitorView plugin load failure is silently logged (no toast), unlike ControlPane
In ControlPane.tsx:315-318, ensurePluginsLoaded failure shows a toast via toastRef.current.error(...). In MonitorView.tsx:155-157, the same failure is only logged with viewsLogger.error(...) and no toast is shown. This is likely intentional to avoid duplicate toasts when both views are mounted, since ensurePluginsLoaded is single-flight and the ControlPane already handles user notification. Worth confirming this is the desired UX.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
There was a problem hiding this comment.
Intentional — MonitorView can be mounted independently without ControlPane, so a toast would be the only notification there. However, since ensurePluginsLoaded is single-flight, if ControlPane already succeeded, the MonitorView call resolves immediately. The log-only approach avoids duplicate toasts in the common case where both views are mounted.
|
|
||
| return true; | ||
| }); | ||
| }, [sortedDefs, searchQuery, activeFilters, pluginKinds, isSearchOrFilterActive]); |
There was a problem hiding this comment.
📝 Info: Redundant dependency in filteredDefs useMemo
isSearchOrFilterActive is included in the filteredDefs useMemo dependency array at line 249, but it's a local variable derived from searchQuery and activeFilters, both of which are already in the dependency array. The redundant dep is harmless (doesn't cause extra recomputation since the constituents already trigger it), but could be cleaned up for clarity.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
There was a problem hiding this comment.
Agreed it's redundant — included for readability since the early-return if (!isSearchOrFilterActive) return sortedDefs references it. Harmless since React deduplicates by referential equality on the actual state values.
| if (activeFilters.size > 0) { | ||
| const top = def.categories.length > 0 ? def.categories[0] : 'Uncategorized'; | ||
| const isPlugin = pluginKinds?.has(def.kind) ?? false; | ||
| const matchesFilter = | ||
| activeFilters.has(top) || (activeFilters.has(PLUGIN_FILTER) && isPlugin); | ||
| if (!matchesFilter) return false; |
There was a problem hiding this comment.
📝 Info: Filter chip OR semantics may surprise users expecting AND behavior
When multiple category filter chips are active, the filter uses OR logic: a node matches if it belongs to ANY active category. This is explicitly tested ('uses OR logic across multiple active filter chips' at NodePalette.test.tsx:157). However, combined with search text, the behavior is AND between search and chips (a node must match the text query AND match at least one active chip). This mixed AND/OR semantic is common in filter UIs but worth documenting — the test coverage here is good and confirms the intended behavior.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
There was a problem hiding this comment.
Intentional — OR logic is standard filter-chip UX (each chip widens the result set). The text search then narrows within the chip selection. AND across chips would be confusing since a node can only belong to one top-level category.
- Extract search/filter styled components to NodePalette.styles.ts to resolve max-lines lint violation (609 → 532 lines). - Add test for multi-chip OR filter logic. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
| let syncInFlight: Promise<void> | null = null; | ||
|
|
||
| /** | ||
| * Reloads schemas if any of the given plugin kinds are missing from the | ||
| * currently loaded node definitions. This handles the race where the UI | ||
| * fetches schemas before the server finishes loading plugins in the background. | ||
| * Uses single-flight deduplication to avoid redundant fetches when multiple | ||
| * views mount concurrently. | ||
| */ | ||
| export function syncPluginSchemas(pluginKinds: Iterable<string>): Promise<void> { | ||
| if (syncInFlight) return syncInFlight; | ||
|
|
||
| const schemaKinds = new Set(useSchemaStore.getState().nodeDefinitions.map((d) => d.kind)); | ||
| const missing = [...pluginKinds].some((kind) => !schemaKinds.has(kind)); | ||
| if (!missing) return Promise.resolve(); | ||
|
|
||
| syncInFlight = reloadSchemas().finally(() => { | ||
| syncInFlight = null; | ||
| }); | ||
| return syncInFlight; | ||
| } |
There was a problem hiding this comment.
📝 Info: Concurrent reloadSchemas calls from syncPluginSchemas and ensureSchemasLoaded
When the app starts, App.tsx:68 calls ensureSchemasLoaded() while ControlPane/MonitorView may concurrently call syncPluginSchemas → reloadSchemas(). Both fetch the same /api/v1/schema/nodes endpoint and update the same store. This results in redundant network requests but not data corruption, since both write the same data and zustand updates are synchronous. The ensureSchemasLoaded single-flight (inFlight) and syncPluginSchemas single-flight (syncInFlight) are independent sentinels, so they don't prevent cross-function duplication. This is acceptable for correctness but could be optimized in the future.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| export function syncPluginSchemas(pluginKinds: Iterable<string>): Promise<void> { | ||
| if (syncInFlight) return syncInFlight; | ||
|
|
||
| const schemaKinds = new Set(useSchemaStore.getState().nodeDefinitions.map((d) => d.kind)); | ||
| const missing = [...pluginKinds].some((kind) => !schemaKinds.has(kind)); | ||
| if (!missing) return Promise.resolve(); | ||
|
|
||
| syncInFlight = reloadSchemas().finally(() => { | ||
| syncInFlight = null; | ||
| }); | ||
| return syncInFlight; | ||
| } |
There was a problem hiding this comment.
📝 Info: syncPluginSchemas single-flight dedup is correct but has a subtle timing consideration
The syncPluginSchemas function uses syncInFlight for dedup, separate from ensureSchemasLoaded's own inFlight. Since App.tsx:64-71 calls ensureSchemasLoaded() and blocks rendering until appReady, by the time ControlPane or MonitorView mount and call syncPluginSchemas, schemas are already loaded. If ensureSchemasLoaded had failed (error is caught at App.tsx:68-70), syncPluginSchemas would find an empty nodeDefinitions array and treat all plugin kinds as missing — triggering reloadSchemas(), which effectively retries. This is actually a reasonable fallback, not a bug. The .finally() correctly clears syncInFlight on both success and failure, allowing future retries.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| React.useEffect(() => { | ||
| ensurePluginsLoaded() | ||
| .then(() => { | ||
| const kinds = usePluginStore.getState().plugins.map((p) => p.kind); | ||
| return syncPluginSchemas(kinds).catch((err) => { | ||
| viewsLogger.error('Failed to sync plugin schemas', err); | ||
| }); | ||
| }) | ||
| .catch((err) => { | ||
| viewsLogger.error('Failed to load plugins', err); | ||
| }); | ||
| }, []); |
There was a problem hiding this comment.
📝 Info: Duplicate ensurePluginsLoaded + syncPluginSchemas calls from ControlPane and MonitorView
Both ControlPane.tsx:307-319 and MonitorView.tsx:174-185 have identical useEffect blocks calling ensurePluginsLoaded().then(() => syncPluginSchemas(...)). Both use the same single-flight dedup in ensurePluginsLoaded and syncPluginSchemas, so they correctly share the same in-flight promises and don't cause duplicate network requests. However, this duplicated pattern across two components could be extracted into a shared hook (e.g., usePluginSchemaSync) to reduce duplication and ensure the pattern stays consistent if it needs to change.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
E2E Test Results — Node Library Search, Filters & Category ChangesTested locally against Vite dev server + backend (build hash Test Results (6/6 passed)
Key EvidenceCategory Reorganization
Search & Filters
No Plugin Chip6 category chips (Audio, Containers, Core, Io, Transport, Video). No Plugin chip. Notes
|
2 participants
Summary
Three improvements to the Nodes Library pane:
1. Search & Quick Filters
Adds a search input and category/plugin filter chips to
NodePalette. When search or filters are active, nodes are shown in a flat filtered list with category breadcrumbs. When inactive, the existing category drill-down behavior is preserved unchanged.kind,description, and category names2. Better Node Categorization
Moves I/O nodes out of the generic
coretop-level category into a dedicatediocategory:core::file_reader["core", "io"]["io", "file"]core::file_writer["core", "io"]["io", "file"]core::object_store_writer["core", "io", "object_store"]["io", "object_store"]Node
kindstrings are unchanged — only the display category hierarchy shifts. No impact on YAML pipelines or API consumers.3. Fix Plugin Nodes Not Appearing
Root cause:
ensureSchemasLoaded()inApp.tsxfetches/api/v1/schema/nodesat page load and caches the result (isLoaded=true). The backend'sUnifiedPluginManager::spawn_load_existing()loads plugins asynchronously in the background. If the UI fetches schemas before plugins finish registering, plugin node definitions are missing and never re-fetched.Fix: Added
syncPluginSchemas()helper inschemaStore.tsthat checks if any loaded plugin kinds are missing fromnodeDefinitionsand callsreloadSchemas()if needed. Uses single-flight deduplication to avoid redundant fetches. Called afterensurePluginsLoaded()in bothControlPaneandMonitorView, with separated error handling.Self-critique follow-up (bcf5dea)
NodePalette.styles.tsto resolve the max-lines lint violation introduced by this PR (609 → 532 lines). Follows the existingOutputPreviewPanel.styles.tspattern.Review & Testing Checklist for Human
iocategory appears withfile_reader,file_writer,object_store_writernodes, and these no longer appear undercoreRecommended test plan: Start the server with a plugin installed (e.g., whisper), load the UI, open Design view, and verify the plugin node appears in the Node Library. Use the search bar to find it by name. Toggle the "Plugin" filter chip.
Notes
NodePalettecovering search, filter chips, multi-chip OR logic, combined filtering, empty states, category breadcrumbs, and toggle behaviorNodePalette.styles.tsfollowingOutputPreviewPanel.styles.tspattern — eliminates the max-lines warning this PR introducedkindstrings (used in YAML pipelines) are unchangedsyncPluginSchemasis best-effort: if the server hasn't finished loading plugins by the time the reload completes, a page refresh covers the edge caseLink to Devin session: https://staging.itsdev.in/sessions/cbe13153507e4b96a2a37207f1b62f94
Requested by: @streamer45
Devin Review
bcf5dea