feat: implemente node configuration registry, execution engine, and d… - #83
Conversation
…ynamic config modal for Gmail, Google Sheets, and FILTER nodes
📝 WalkthroughWalkthroughChangesFilter node
Variable-driven workflow UI
Node metadata and presentation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant WorkflowUser
participant ConfigModal
participant RichVariableInput
participant FilterExecutor
participant NodeRegistry
WorkflowUser->>ConfigModal: configure filter node
ConfigModal->>RichVariableInput: edit variable-backed fields
ConfigModal->>NodeRegistry: resolve registered filter node
NodeRegistry->>FilterExecutor: create filter executor
FilterExecutor-->>ConfigModal: return filtered and discarded data
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/common/src/index.tsOops! Something went wrong! :( ESLint: 9.32.0 Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './library.js' is not defined by "exports" in /node_modules/@workspace/eslint-config/package.json packages/common/src/interpolation.tsOops! Something went wrong! :( ESLint: 9.32.0 Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './library.js' is not defined by "exports" in /node_modules/@workspace/eslint-config/package.json packages/nodes/package.jsonOops! Something went wrong! :( ESLint: 9.32.0 Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './library.js' is not defined by "exports" in /node_modules/@workspace/eslint-config/package.json
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. Comment |
There was a problem hiding this comment.
Pull request overview
This PR expands BuildFlow’s node ecosystem by introducing a new Filter node (schema + executor + UI config), and improves the workflow configuration UX with richer variable insertion and more dynamic config behavior (including schema-aware dropdowns).
Changes:
- Register and execute a new Filter action node (backend registry + execution registry + Zod input schema).
- Improve interpolation/config resolution so exact
{{...}}values can resolve to non-strings (arrays/objects) for downstream nodes. - Update the web config modal to support richer variable inputs and dynamic schema-driven dropdown options; add node icons/SVG assets.
Reviewed changes
Copilot reviewed 17 out of 24 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/nodes/src/registry/node-registry.ts | Registers Filter node alongside existing nodes. |
| packages/nodes/src/registry/execution.registory.ts | Registers Filter executor for runtime execution. |
| packages/nodes/src/google-sheets/google-sheets.node.ts | Updates node description and adds icon path. |
| packages/nodes/src/gmail/gmail.node.ts | Updates node description and adds icon path. |
| packages/nodes/src/filter/filter.node.ts | Adds Filter node definition + registration. |
| packages/nodes/src/filter/filter.executor.ts | Implements Filter node execution logic + outputs. |
| packages/nodes/package.json | Adds workspace dependency on @repo/common. |
| packages/common/src/interpolation.ts | Improves config variable resolution for exact-match variables. |
| packages/common/src/index.ts | Adds FilterNodeInput Zod schema/type export. |
| apps/web/public/webhook.svg | Adds webhook icon asset. |
| apps/web/public/google_sheet.svg | Adds Google Sheets icon asset. |
| apps/web/public/gmail.svg | Adds Gmail icon asset. |
| apps/web/app/workflows/[id]/components/ConfigModal.tsx | Adds RichVariableInput + dynamic schema dropdown field rendering. |
| apps/web/app/lib/types/node.types.ts | Adds new config field type: dynamic_schema_dropdown. |
| apps/web/app/lib/nodeConfigs/index.ts | Registers Filter node config in UI registry. |
| apps/web/app/lib/nodeConfigs/filter.action.ts | Adds UI config schema for Filter node. |
| apps/web/app/components/ui/variable-panel.tsx | Switches variable insertion syntax to use nodeId-based keys. |
| apps/web/app/components/ui/TestPanel.tsx | Improves rendering for nested arrays/objects. |
| apps/web/app/components/ui/RichVariableInput.tsx | Adds contentEditable-based rich variable input component. |
| apps/web/app/components/nodes/BaseNode.tsx | Adjusts node icon rendering/styling. |
| metadata: { | ||
| operation_used: operation, | ||
| items_kept: filteredData.length, | ||
| items_discard: discardedData.length | ||
| } |
| const formattedNodeName = node.nodeId | ||
|
|
| import { config } from "dotenv"; | ||
| import NodeRegistry from "../registry/node-registry.js"; | ||
| import { FilterExecutor } from "./filter.executor.js"; | ||
|
|
| {icon ? | ||
| <img src={icon ? icon : "⚡"} className="w-16 h-16 object-obtain" | ||
| /> : ("⚡")} |
| export function parseValueToHtml(rawValue: string, availableNodes: AvailableNode[]): string { | ||
| if (!rawValue) return ""; | ||
|
|
||
| return rawValue.replace(/\{\{([^.]+)\.([^}]+)\}\}/g, (match, nodeId, path) => { | ||
| const node = availableNodes.find(n => n.id === nodeId); | ||
| const displayName = node ? node.name : "Unknown Node"; | ||
| const colorClass = getNodeColorClass(nodeId); | ||
|
|
||
| return `<span contenteditable="false" class="inline-flex items-center px-1.5 py-0.5 mx-1 rounded text-[10px] border align-middle font-mono select-all cursor-default ${colorClass}" data-id="${nodeId}" data-path="${path}">${displayName} > ${path}</span>`; | ||
| }); | ||
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (2)
packages/nodes/src/filter/filter.executor.ts (1)
136-176: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winWrap
casedeclarations in blocks.Biome's
noSwitchDeclarationsflags theconstdeclarations at Line 138 (unique_results), Line 152 (newResult), and Line 168 (existing_data). These declarations are not block-scoped to theircase, so they leak into the shared switch scope. The currentbreak/returnstatements prevent a live bug today, but this pattern is fragile: a future edit that adds a case referencing one of these names before its own case runs would hit a temporal-dead-zone error. Wrap each case body in{ }.🔧 Proposed fix
switch (operation) { - case 'unique_rows': + case 'unique_rows': { const unique_results = this.handleUniqueRowsFromList(normalizedSource, sourceKey); filteredData = unique_results.filteredData; discardedData = unique_results.discardedData; break; + } - case 'new_data_only': + case 'new_data_only': { if (!sourceKey || !referenceKey) return { success: false, error: "sourceKey and referenceKey are required to compare datasets" } if (normalizedRef === undefined) return { success: false, error: "reference data is required to compare datasets" } const newResult = this.compareDataSet(normalizedSource, normalizedRef, sourceKey, referenceKey); filteredData = newResult.uniqueData; discardedData = [...newResult.existingData, ...newResult.discardedData] break; + } - case 'existing_data_only': + case 'existing_data_only': { if (!sourceKey || !referenceKey) return { success: false, error: "sourceKey and referenceKey are required to compare datasets" } if (normalizedRef === undefined) return { success: false, error: "reference data is required to compare datasets" } const existing_data = this.compareDataSet(normalizedSource, normalizedRef, sourceKey, referenceKey); filteredData = existing_data.existingData; discardedData = [...existing_data.discardedData, ...existing_data.uniqueData]; break; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nodes/src/filter/filter.executor.ts` around lines 136 - 176, Wrap each switch case body declaring a const in its own block: unique_rows around unique_results, new_data_only around newResult, and existing_data_only around existing_data. Preserve the existing validation, assignments, breaks, and return behavior while ensuring each declaration is scoped only to its case.Source: Linters/SAST tools
packages/common/src/index.ts (1)
196-210: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider enforcing
sourceKey/referenceKeyco-requirement in the schema.
FilterNodeInputleavessourceKeyandreferenceKeyindependently optional.FilterExecutor.executeinpackages/nodes/src/filter/filter.executor.ts(Lines 144-147, 159-162) duplicates the sameif (!sourceKey || !referenceKey)check in two switch cases. Move this rule into the schema with.superRefine()so the constraint lives in one place and both callers benefit automatically.🔧 Proposed refactor
export const FilterNodeInput = z.object({ operation: z.enum([ "unique_rows", "new_data_only", "existing_data_only" ]), sourceData: z.array(z.any()), referenceData: z.array(z.any()).optional(), sourceKey: z.string().optional(), referenceKey: z.string().optional() -}) +}).superRefine((data, ctx) => { + if (data.operation !== "unique_rows" && (!data.sourceKey || !data.referenceKey || !data.referenceData)) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: "sourceKey, referenceKey, and referenceData are required for comparison operations", + }); + } +});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/common/src/index.ts` around lines 196 - 210, Update the FilterNodeInput schema with superRefine so sourceKey and referenceKey must either both be provided or both be absent, and report a validation issue when only one is present. Then remove the duplicated co-requirement checks from FilterExecutor.execute’s affected switch cases, relying on schema validation while preserving existing behavior for valid inputs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/app/components/nodes/BaseNode.tsx`:
- Line 98: Update the img element in BaseNode to replace the invalid
object-obtain Tailwind class with object-contain, preserving the existing sizing
and other classes.
In `@apps/web/app/components/ui/RichVariableInput.tsx`:
- Around line 39-45: Update the RichVariableInput rendering logic around the
rawValue replacement so rawValue, nodeId, path, and displayName are never
interpolated into innerHTML. Build the variable pill using DOM APIs or React
elements, assigning untrusted values through textContent or equivalent escaped
rendering while preserving the existing styling, data attributes, and display
behavior.
- Around line 44-45: Add the "pill" class to the serialized variable span
generated in the handleInput serialization logic, preserving all existing
classes and attributes so handleInput can recognize it and restore the
{{nodeId.path}} representation.
- Around line 75-84: Update the serialization logic in the RichVariableInput
element-processing branch to append "\n" for br elements and block-element
boundaries instead of relying solely on textContent. Preserve pill serialization
and text extraction for inline elements, ensuring multiline values retain their
line breaks when edited.
In `@apps/web/app/components/ui/TestPanel.tsx`:
- Around line 28-39: In the nested rendering logic, define and reuse a predicate
that identifies only non-null, non-array objects, replacing the current typeof
value[0] checks for both the padding class and renderArrayOfObjectsTable
selection. Keep arrays such as nested matrices in the inline branch, and
serialize object elements there so mixed arrays display their contents instead
of “[object Object]”.
In `@apps/web/app/components/ui/variable-panel.tsx`:
- Around line 155-164: The spreadsheet insertion logic in renderSpreadsheetTable
must handle direct 2D-array data when data.rows is absent: use the node path
itself for the full table and one-based row indexing with zero-based column
indexing for individual cells. Preserve the existing .rows-based paths when rows
is available, and update the “Select Entire Table” handler in the surrounding
spreadsheet output UI consistently.
In `@apps/web/app/lib/nodeConfigs/filter.action.ts`:
- Around line 32-57: Update the `referenceData` and `referenceKey` field
definitions in the filter action configuration to use `required: true` instead
of `required: false`. Keep their existing `showForOperation` conditions
unchanged so they remain required only when displayed for `new_data_only` and
`existing_data_only`, without affecting `unique_rows`.
In `@packages/common/src/interpolation.ts`:
- Around line 86-90: Update the bare-key branch in resolveVariable to return the
original interpolation expression when context[trimmed] is undefined, while
preserving all existing native values including 0, false, null, objects, and
arrays. Add regression tests covering missing bare variables and each of those
native value types, including the exact-match path used by ConfigModal.
In `@packages/nodes/src/filter/filter.executor.ts`:
- Around line 183-187: Update the metadata object returned by
FilterExecutor.execute so the items_discard key is renamed to items_discarded,
matching the declared output schema. Keep the existing discardedData.length
value and all other metadata fields unchanged.
---
Nitpick comments:
In `@packages/common/src/index.ts`:
- Around line 196-210: Update the FilterNodeInput schema with superRefine so
sourceKey and referenceKey must either both be provided or both be absent, and
report a validation issue when only one is present. Then remove the duplicated
co-requirement checks from FilterExecutor.execute’s affected switch cases,
relying on schema validation while preserving existing behavior for valid
inputs.
In `@packages/nodes/src/filter/filter.executor.ts`:
- Around line 136-176: Wrap each switch case body declaring a const in its own
block: unique_rows around unique_results, new_data_only around newResult, and
existing_data_only around existing_data. Preserve the existing validation,
assignments, breaks, and return behavior while ensuring each declaration is
scoped only to its case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: df09b8f4-dbdd-4537-b089-21888b181cdc
⛔ Files ignored due to path filters (7)
apps/web/public/filtering.pngis excluded by!**/*.pngapps/web/public/gmail.pngis excluded by!**/*.pngapps/web/public/gmail.svgis excluded by!**/*.svgapps/web/public/google_sheet.pngis excluded by!**/*.pngapps/web/public/google_sheet.svgis excluded by!**/*.svgapps/web/public/webhook.pngis excluded by!**/*.pngapps/web/public/webhook.svgis excluded by!**/*.svg
📒 Files selected for processing (17)
apps/web/app/components/nodes/BaseNode.tsxapps/web/app/components/ui/RichVariableInput.tsxapps/web/app/components/ui/TestPanel.tsxapps/web/app/components/ui/variable-panel.tsxapps/web/app/lib/nodeConfigs/filter.action.tsapps/web/app/lib/nodeConfigs/index.tsapps/web/app/lib/types/node.types.tsapps/web/app/workflows/[id]/components/ConfigModal.tsxpackages/common/src/index.tspackages/common/src/interpolation.tspackages/nodes/package.jsonpackages/nodes/src/filter/filter.executor.tspackages/nodes/src/filter/filter.node.tspackages/nodes/src/gmail/gmail.node.tspackages/nodes/src/google-sheets/google-sheets.node.tspackages/nodes/src/registry/execution.registory.tspackages/nodes/src/registry/node-registry.ts
| <img src={icon ? icon : "⚡"} className="w-16 h-16 object-cover" | ||
| /> : ("⚡")} | ||
| {icon ? | ||
| <img src={icon ? icon : "⚡"} className="w-16 h-16 object-obtain" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n --fixed-strings 'object-obtain' apps/web/app/components/nodes/BaseNode.tsx && exit 1
rg -n --fixed-strings 'object-contain' apps/web/app/components/nodes/BaseNode.tsxRepository: Dev-Pross/BuildFlow
Length of output: 239
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== relevant file excerpt =="
sed -n '85,105p' apps/web/app/components/nodes/BaseNode.tsx
echo
echo "== package/dependency references =="
rg -n '"tailwindcss"|tailwindcss|w-16|h-16|object-contain|object-obtain|object-fill' package.json node_modules apps web -g '!node_modules' -g '!dist' -g '!build' 2>/dev/null | head -120
echo
echo "== tailwind css if vendored =="
fd -a 'tailwindcss' node_modules . 2>/dev/null | head -5
echo
echo "== deterministic CSS behavior probe for the exact class token =="
python3 - <<'PY'
import re, pathlib
src = pathlib.Path('apps/web/app/components/nodes/BaseNode.tsx').read_text()
class_val = re.search(r'className="([^"]*)"', src).group(1)
print("className", class_val)
print("contains object-obtain", "object-obtain" in class_val)
print("contains object-contain", "object-contain" in class_val)
PYRepository: Dev-Pross/BuildFlow
Length of output: 1511
🌐 Web query:
Tailwind CSS object-center utility object-fit class names
💡 Result:
In Tailwind CSS, object-position and object-fit are separate sets of utility classes used to control replaced elements like images and videos [1][2]. Object Position Utilities These classes control the object-position CSS property, which determines how an element is positioned within its container [1][3]. Class | CSS Property --- | --- object-bottom | object-position: bottom; [1] object-bottom-left | object-position: bottom left; [1] object-bottom-right | object-position: bottom right; [1] object-center | object-position: center; [1] object-left | object-position: left; [1] object-right | object-position: right; [1] object-top | object-position: top; [1] object-top-left | object-position: top left; [1] object-top-right | object-position: top right; [1] Object Fit Utilities These classes control the object-fit CSS property, which determines how an element is resized to fit its container [2][4]. Class | CSS Property --- | --- object-contain | object-fit: contain; [2] object-cover | object-fit: cover; [2] object-fill | object-fit: fill; [2] object-none | object-fit: none; [2] object-scale-down | object-fit: scale-down; [2] Both types of utilities support responsive design (using prefixes like md:), arbitrary values (using square brackets like object-[center_bottom]), and state modifiers (like hover:) [1][3][2][5][6].
Citations:
- 1: https://tailwindcss.com/docs/object-position
- 2: https://tailwindcss.com/docs/object-fit
- 3: https://v3.tailwindcss.com/docs/object-position
- 4: https://www.geeksforgeeks.org/css/tailwind-css-object-fit/
- 5: https://windframe.dev/tailwind/classes/tailwind-object-fit
- 6: https://kombai.com/tailwind/object-fit/
Use the valid Tailwind object-fit utility.
object-obtain is not a Tailwind object-fit class, so the icon image is not constrained. Replace it with object-contain.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/app/components/nodes/BaseNode.tsx` at line 98, Update the img
element in BaseNode to replace the invalid object-obtain Tailwind class with
object-contain, preserving the existing sizing and other classes.
| return rawValue.replace(/\{\{([^.]+)\.([^}]+)\}\}/g, (match, nodeId, path) => { | ||
| const node = availableNodes.find(n => n.id === nodeId); | ||
| const displayName = node ? node.name : "Unknown Node"; | ||
| const colorClass = getNodeColorClass(nodeId); | ||
|
|
||
| return `<span contenteditable="false" class="inline-flex items-center px-1.5 py-0.5 mx-1 rounded text-[10px] border align-middle font-mono select-all cursor-default ${colorClass}" data-id="${nodeId}" data-path="${path}">${displayName} > ${path}</span>`; | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Do not inject configuration values into innerHTML.
rawValue, nodeId, path, and displayName are inserted as HTML without encoding. A saved value that contains HTML can create active DOM when the effect synchronizes the editor. Construct text and pill nodes with DOM APIs or React elements. Do not use editable configuration text as HTML.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/app/components/ui/RichVariableInput.tsx` around lines 39 - 45,
Update the RichVariableInput rendering logic around the rawValue replacement so
rawValue, nodeId, path, and displayName are never interpolated into innerHTML.
Build the variable pill using DOM APIs or React elements, assigning untrusted
values through textContent or equivalent escaped rendering while preserving the
existing styling, data attributes, and display behavior.
| return `<span contenteditable="false" class="inline-flex items-center px-1.5 py-0.5 mx-1 rounded text-[10px] border align-middle font-mono select-all cursor-default ${colorClass}" data-id="${nodeId}" data-path="${path}">${displayName} > ${path}</span>`; | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add the pill class to serialized variable spans.
handleInput recognizes a variable only when el.classList.contains("pill") is true. The span created on Line 44 has no pill class. After the user edits a value with a variable, the serializer stores Node Name > path instead of {{nodeId.path}}.
Proposed fix
- return `<span contenteditable="false" class="inline-flex items-center px-1.5 py-0.5 mx-1 rounded text-[10px] border align-middle font-mono select-all cursor-default ${colorClass}" data-id="${nodeId}" data-path="${path}">${displayName} > ${path}</span>`;
+ return `<span contenteditable="false" class="pill inline-flex items-center px-1.5 py-0.5 mx-1 rounded text-[10px] border align-middle font-mono select-all cursor-default ${colorClass}" data-id="${nodeId}" data-path="${path}">${displayName} > ${path}</span>`;📝 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.
| return `<span contenteditable="false" class="inline-flex items-center px-1.5 py-0.5 mx-1 rounded text-[10px] border align-middle font-mono select-all cursor-default ${colorClass}" data-id="${nodeId}" data-path="${path}">${displayName} > ${path}</span>`; | |
| }); | |
| return `<span contenteditable="false" class="pill inline-flex items-center px-1.5 py-0.5 mx-1 rounded text-[10px] border align-middle font-mono select-all cursor-default ${colorClass}" data-id="${nodeId}" data-path="${path}">${displayName} > ${path}</span>`; | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/app/components/ui/RichVariableInput.tsx` around lines 44 - 45, Add
the "pill" class to the serialized variable span generated in the handleInput
serialization logic, preserving all existing classes and attributes so
handleInput can recognize it and restore the {{nodeId.path}} representation.
| if (el.classList.contains("pill")) { | ||
| const nodeId = el.getAttribute("data-id"); | ||
| const path = el.getAttribute("data-path"); | ||
| if (nodeId && path) { | ||
| rawString += `{{${nodeId}.${path}}}`; | ||
| } | ||
| } else { | ||
| // For any pasted elements (br, divs), just extract text | ||
| rawString += el.textContent || ""; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve line breaks during serialization.
A <br> has empty textContent. Block elements also concatenate their text without a separator. This control now replaces textarea fields in apps/web/app/workflows/[id]/components/ConfigModal.tsx, so multiline values lose their newlines after editing. Serialize <br> and block boundaries as \n.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/app/components/ui/RichVariableInput.tsx` around lines 75 - 84,
Update the serialization logic in the RichVariableInput element-processing
branch to append "\n" for br elements and block-element boundaries instead of
relying solely on textContent. Preserve pill serialization and text extraction
for inline elements, ensuring multiline values retain their line breaks when
edited.
| <div className={`flex-[3] min-w-0 text-xs text-gray-300 border-l border-[#1a1f2e]/50 font-mono break-all ${isNested && Array.isArray(value) && value.length > 0 && typeof value[0] === 'object' ? 'p-0' : 'px-4 py-2.5'}`}> | ||
| {isNested ? ( | ||
| Array.isArray(value) ? ( | ||
| <span className="text-purple-400">[{value.map(v => typeof v === 'string' ? `"${v}"` : String(v)).join(', ')}]</span> | ||
| value.length > 0 && typeof value[0] === 'object' ? ( | ||
| <div className=" w-full">{renderArrayOfObjectsTable(value, true)}</div> | ||
| ) : ( | ||
| <span className="text-purple-400">[{value.map(v => typeof v === 'string' ? `"${v}"` : String(v)).join(', ')}]</span> | ||
| ) | ||
| ) : ( | ||
| <span className="text-gray-500 italic">Object</span> | ||
| <div className="mt-2 mb-2 w-full">{renderObjectTable(value)}</div> | ||
| ) | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use a strict predicate for nested object arrays.
typeof value[0] === 'object' also matches null and arrays. A nested matrix such as [[1, 2], [3, 4]] is therefore rendered as an object table with index-based columns. A mixed array such as [1, { id: 2 }] renders the object as [object Object].
Define one predicate for non-null, non-array objects and reuse it for the padding class and renderer selection. Serialize object values in the inline branch when mixed arrays are supported.
Proposed fix
{entries.map(([key, value]) => {
const isNested = typeof value === 'object' && value !== null;
+ const isArrayOfObjects =
+ Array.isArray(value) &&
+ value.length > 0 &&
+ value.every(item =>
+ item !== null &&
+ typeof item === 'object' &&
+ !Array.isArray(item)
+ );
return (
...
- ${isNested && Array.isArray(value) && value.length > 0 && typeof value[0] === 'object' ? 'p-0' : 'px-4 py-2.5'}`}>
+ ${isArrayOfObjects ? 'p-0' : 'px-4 py-2.5'}`}>
...
- value.length > 0 && typeof value[0] === 'object' ? (
+ isArrayOfObjects ? (
...
- value.map(v => typeof v === 'string' ? `"${v}"` : String(v)).join(', ')
+ value.map(v =>
+ typeof v === 'string'
+ ? `"${v}"`
+ : typeof v === 'object' && v !== null
+ ? JSON.stringify(v)
+ : String(v)
+ ).join(', ')📝 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.
| <div className={`flex-[3] min-w-0 text-xs text-gray-300 border-l border-[#1a1f2e]/50 font-mono break-all ${isNested && Array.isArray(value) && value.length > 0 && typeof value[0] === 'object' ? 'p-0' : 'px-4 py-2.5'}`}> | |
| {isNested ? ( | |
| Array.isArray(value) ? ( | |
| <span className="text-purple-400">[{value.map(v => typeof v === 'string' ? `"${v}"` : String(v)).join(', ')}]</span> | |
| value.length > 0 && typeof value[0] === 'object' ? ( | |
| <div className=" w-full">{renderArrayOfObjectsTable(value, true)}</div> | |
| ) : ( | |
| <span className="text-purple-400">[{value.map(v => typeof v === 'string' ? `"${v}"` : String(v)).join(', ')}]</span> | |
| ) | |
| ) : ( | |
| <span className="text-gray-500 italic">Object</span> | |
| <div className="mt-2 mb-2 w-full">{renderObjectTable(value)}</div> | |
| ) | |
| {entries.map(([key, value]) => { | |
| const isNested = typeof value === 'object' && value !== null; | |
| const isArrayOfObjects = | |
| Array.isArray(value) && | |
| value.length > 0 && | |
| value.every(item => | |
| item !== null && | |
| typeof item === 'object' && | |
| !Array.isArray(item) | |
| ); | |
| return ( | |
| <div className={`flex-[3] min-w-0 text-xs text-gray-300 border-l border-[`#1a1f2e`]/50 font-mono break-all ${isArrayOfObjects ? 'p-0' : 'px-4 py-2.5'}`}> | |
| {isNested ? ( | |
| Array.isArray(value) ? ( | |
| isArrayOfObjects ? ( | |
| <div className=" w-full">{renderArrayOfObjectsTable(value, true)}</div> | |
| ) : ( | |
| <span className="text-purple-400">[{value.map(v => | |
| typeof v === 'string' | |
| ? `"${v}"` | |
| : typeof v === 'object' && v !== null | |
| ? JSON.stringify(v) | |
| : String(v) | |
| ).join(', ')}]</span> | |
| ) | |
| ) : ( | |
| <div className="mt-2 mb-2 w-full">{renderObjectTable(value)}</div> | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/app/components/ui/TestPanel.tsx` around lines 28 - 39, In the nested
rendering logic, define and reuse a predicate that identifies only non-null,
non-array objects, replacing the current typeof value[0] checks for both the
padding class and renderArrayOfObjectsTable selection. Keep arrays such as
nested matrices in the inline branch, and serialize object elements there so
mixed arrays display their contents instead of “[object Object]”.
| <div className="flex items-center gap-2"> | ||
| <span>Spreadsheet Data</span> | ||
| <button | ||
| onClick={() => handleInsert(`{{${formattedNodeName}.rows}}`)} | ||
| className="px-2 py-0.5 bg-blue-500/20 text-blue-400 hover:bg-blue-500/40 rounded transition-colors" | ||
| title="Insert the entire array of data" | ||
| > | ||
| Select Entire Table | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect how interpolation resolves node paths and whether it aliases direct arrays as `rows`.
rg -n -C 6 'resolveVariable|rows' \
packages/common/src/interpolation.ts \
apps/web/app/components/ui/variable-panel.tsx \
'apps/web/app/workflows/[id]/components/ConfigModal.tsx'Repository: Dev-Pross/BuildFlow
Length of output: 29949
Insert direct-array paths for spreadsheet outputs.
When testOutput.data is a direct 2D array, renderSpreadsheetTable builds table insertions using rows, but InterpolationContext stores the node data under context[nodeId] without aliasing .rows. Use {{${formattedNodeName}}} for the full table and {{${formattedNodeName}[${rowIndex + 1}][${colIndex}]}} for cells when data.rows is missing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/app/components/ui/variable-panel.tsx` around lines 155 - 164, The
spreadsheet insertion logic in renderSpreadsheetTable must handle direct
2D-array data when data.rows is absent: use the node path itself for the full
table and one-based row indexing with zero-based column indexing for individual
cells. Preserve the existing .rows-based paths when rows is available, and
update the “Select Entire Table” handler in the surrounding spreadsheet output
UI consistently.
| { | ||
| name: "referenceData", | ||
| label: "Reference Data (JSON Array)", | ||
| type: "textarea", | ||
| placeholder: '{{ Database.rows }}', | ||
| required: false, | ||
| dependsOn: "operation", | ||
| showForOperation: ["new_data_only", "existing_data_only"], // Hides this field if unique_rows is selected! | ||
| description: "The master list to compare against" | ||
| }, | ||
| { | ||
| name: "sourceKey", | ||
| label: "Source Column / Key", | ||
| type: "dynamic_schema_dropdown", | ||
| required: false, | ||
| description: "The specific column to use for comparison. (Optional for deduplication, required for comparisons)" | ||
| }, | ||
| { | ||
| name: "referenceKey", | ||
| label: "Reference Column / Key", | ||
| type: "dynamic_schema_dropdown", | ||
| required: false, | ||
| dependsOn: "operation", | ||
| showForOperation: ["new_data_only", "existing_data_only"], | ||
| description: "The column in the Reference Data to match against." | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Mark referenceData and referenceKey as required when shown.
referenceData (Line 37) and referenceKey (Line 53) are required: false, but FilterExecutor.execute in packages/nodes/src/filter/filter.executor.ts returns a validation error when sourceKey/referenceKey are missing for the new_data_only and existing_data_only operations. Both fields already use showForOperation to hide themselves for unique_rows, so setting required: true on them does not affect the unique_rows flow and lets the UI catch the missing-field case before the user runs the node.
🔧 Proposed fix
{
name: "referenceData",
label: "Reference Data (JSON Array)",
type: "textarea",
placeholder: '{{ Database.rows }}',
- required: false,
+ required: true,
dependsOn: "operation",
showForOperation: ["new_data_only", "existing_data_only"], // Hides this field if unique_rows is selected!
description: "The master list to compare against"
}, {
name: "referenceKey",
label: "Reference Column / Key",
type: "dynamic_schema_dropdown",
- required: false,
+ required: true,
dependsOn: "operation",
showForOperation: ["new_data_only", "existing_data_only"],
description: "The column in the Reference Data to match against."
}📝 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.
| { | |
| name: "referenceData", | |
| label: "Reference Data (JSON Array)", | |
| type: "textarea", | |
| placeholder: '{{ Database.rows }}', | |
| required: false, | |
| dependsOn: "operation", | |
| showForOperation: ["new_data_only", "existing_data_only"], // Hides this field if unique_rows is selected! | |
| description: "The master list to compare against" | |
| }, | |
| { | |
| name: "sourceKey", | |
| label: "Source Column / Key", | |
| type: "dynamic_schema_dropdown", | |
| required: false, | |
| description: "The specific column to use for comparison. (Optional for deduplication, required for comparisons)" | |
| }, | |
| { | |
| name: "referenceKey", | |
| label: "Reference Column / Key", | |
| type: "dynamic_schema_dropdown", | |
| required: false, | |
| dependsOn: "operation", | |
| showForOperation: ["new_data_only", "existing_data_only"], | |
| description: "The column in the Reference Data to match against." | |
| } | |
| { | |
| name: "referenceData", | |
| label: "Reference Data (JSON Array)", | |
| type: "textarea", | |
| placeholder: '{{ Database.rows }}', | |
| required: true, | |
| dependsOn: "operation", | |
| showForOperation: ["new_data_only", "existing_data_only"], // Hides this field if unique_rows is selected! | |
| description: "The master list to compare against" | |
| }, | |
| { | |
| name: "sourceKey", | |
| label: "Source Column / Key", | |
| type: "dynamic_schema_dropdown", | |
| required: false, | |
| description: "The specific column to use for comparison. (Optional for deduplication, required for comparisons)" | |
| }, | |
| { | |
| name: "referenceKey", | |
| label: "Reference Column / Key", | |
| type: "dynamic_schema_dropdown", | |
| required: true, | |
| dependsOn: "operation", | |
| showForOperation: ["new_data_only", "existing_data_only"], | |
| description: "The column in the Reference Data to match against." | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/app/lib/nodeConfigs/filter.action.ts` around lines 32 - 57, Update
the `referenceData` and `referenceKey` field definitions in the filter action
configuration to use `required: true` instead of `required: false`. Keep their
existing `showForOperation` conditions unchanged so they remain required only
when displayed for `new_data_only` and `existing_data_only`, without affecting
`unique_rows`.
|
|
||
| const dotIndex = trimmed.indexOf('.'); | ||
| if (dotIndex === -1) { | ||
| return context[trimmed]; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve unresolved bare variables.
For {{missing}}, resolveVariable returns undefined because the bare-key branch returns context[trimmed]. The new exact-match branch stores that value directly. ConfigModal only detects unresolved values that are strings containing {{, so the missing field can reach api.execute.node without a warning.
Return the original interpolation expression when a bare lookup is missing. Add regression tests for missing variables and native values such as 0, false, null, objects, and arrays.
Proposed fix
if (dotIndex === -1) {
- return context[trimmed];
+ const value = context[trimmed];
+ return value !== undefined ? value : `{{${variable}}}`;
}Also applies to: 176-179
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/common/src/interpolation.ts` around lines 86 - 90, Update the
bare-key branch in resolveVariable to return the original interpolation
expression when context[trimmed] is undefined, while preserving all existing
native values including 0, false, null, objects, and arrays. Add regression
tests covering missing bare variables and each of those native value types,
including the exact-match path used by ConfigModal.
| metadata: { | ||
| operation_used: operation, | ||
| items_kept: filteredData.length, | ||
| items_discard: discardedData.length | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Fix metadata key mismatch: items_discard vs. items_discarded.
FilterExecutor.execute emits metadata.items_discard (Line 186), but apps/web/app/lib/nodeConfigs/filter.action.ts declares the output field at metadata.items_discarded (Line 68). Any variable reference or dynamic-schema lookup built against the declared output path resolves to undefined at runtime because the actual key differs. Rename the executor's key to match the declared output schema.
🐛 Proposed fix
metadata: {
operation_used: operation,
items_kept: filteredData.length,
- items_discard: discardedData.length
+ items_discarded: discardedData.length
}📝 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.
| metadata: { | |
| operation_used: operation, | |
| items_kept: filteredData.length, | |
| items_discard: discardedData.length | |
| } | |
| metadata: { | |
| operation_used: operation, | |
| items_kept: filteredData.length, | |
| items_discarded: discardedData.length | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/nodes/src/filter/filter.executor.ts` around lines 183 - 187, Update
the metadata object returned by FilterExecutor.execute so the items_discard key
is renamed to items_discarded, matching the declared output schema. Keep the
existing discardedData.length value and all other metadata fields unchanged.
…ynamic config modal for Gmail, Google Sheets, and FILTER nodes
Summary by CodeRabbit
New Features
Bug Fixes
Documentation