feat: add RichVariableInput component and implement dynamic variable … - #84
Conversation
…resolution in execution engine
📝 WalkthroughWalkthroughThe PR escapes raw variable input and node names before HTML rendering, adds the ChangesInput rendering and execution context
Estimated code review effort: 3 (Moderate) | ~20 minutes 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
apps/worker/src/engine/executor.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 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
Adds UI support for rich variable placeholders (pill-style) and updates the worker execution engine to resolve {{...}} variables dynamically during workflow execution (including per-row looping for spreadsheet-like inputs).
Changes:
- Introduces
RichVariableInputto render{{node.path}}references as non-editable “pills” in a contentEditable editor. - Resolves config variables at execution time using an interpolation context built from previously executed node outputs.
- Adds looped execution behavior for spreadsheet inputs (per-row execution with retries, skip rules, and aggregated results).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| apps/worker/src/engine/executor.ts | Builds interpolation context from prior node outputs and resolves {{...}} variables in node config; adds spreadsheet-driven loop execution path. |
| apps/web/app/components/ui/RichVariableInput.tsx | Adds a pill-based variable editor that converts {{node.path}} into styled spans and serializes them back into template strings. |
| // Store this node's output for variable resolution in subsequent nodes | ||
| executedNodeOutputs.push({ | ||
| nodeName: node.name, | ||
| nodeName: node.id, | ||
| outputData: execute.output | ||
| }); |
| 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>`; | ||
| // 3. IMPORTANT: Added the "pill" class at the start of the class list | ||
| 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>`; |
| import { | ||
| resolveConfigVariables, | ||
| buildInterpolationContext, | ||
| InterpolationContext | ||
| } from "@repo/common/zod"; |
![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
🤖 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/ui/RichVariableInput.tsx`:
- Line 55: Update the HTML generation around the rich variable input renderer to
escape displayName before interpolating it into the span returned by the
relevant function. Reuse an existing HTML-escaping utility if available, and
ensure node names containing markup render as text without changing rawValue
sanitization or the surrounding pill attributes.
In `@apps/worker/src/engine/executor.ts`:
- Around line 303-306: Update the output-context storage in the node execution
flow around executedNodeOutputs so the context key uses node.id exactly, without
lowercasing or replacing spaces. Ensure buildInterpolationContext and
resolveVariable can resolve RichVariableInput tokens in the {{nodeId.path}} form
while preserving existing output data.
🪄 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: d505de22-462d-46c1-9376-66a5ebc43f1f
📒 Files selected for processing (2)
apps/web/app/components/ui/RichVariableInput.tsxapps/worker/src/engine/executor.ts
…ation support to execution engine
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/worker/src/engine/executor.ts (1)
11-14: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMake
nodeIdrequired inNodeExecutionOutput.Every execution output comes from a node with a required ID. If a producer omits
nodeId, the guards at Line 106 and Line 176 silently skip ID-based context entries, so{{nodeId.path}}cannot resolve. ChangenodeId?: stringtonodeId: stringand remove the silent omission path after all producers providenode.id.Proposed type fix
interface NodeExecutionOutput { nodeName: string; - nodeId?: string; + nodeId: string; outputData: any; }🤖 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/worker/src/engine/executor.ts` around lines 11 - 14, Update the NodeExecutionOutput interface to make nodeId required, then update every producer of NodeExecutionOutput to always provide its node.id. Remove the optional-ID guards in the execution handling paths around the nodeId-based context entries so valid outputs are not silently skipped.
🤖 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/worker/src/engine/executor.ts`:
- Around line 105-107: Update buildInterpolationContext and its
executedNodeOutputs loop so raw node IDs cannot overwrite normalized node-name
keys; use a distinct ID namespace for ID-based entries, or detect and
reject/report collisions before insertion, while preserving name-based
interpolation values.
---
Nitpick comments:
In `@apps/worker/src/engine/executor.ts`:
- Around line 11-14: Update the NodeExecutionOutput interface to make nodeId
required, then update every producer of NodeExecutionOutput to always provide
its node.id. Remove the optional-ID guards in the execution handling paths
around the nodeId-based context entries so valid outputs are not silently
skipped.
🪄 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: 816e5a6b-55d5-4cb4-b192-98d36923ebff
📒 Files selected for processing (2)
apps/web/app/components/ui/RichVariableInput.tsxapps/worker/src/engine/executor.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/app/components/ui/RichVariableInput.tsx
| for (const out of executedNodeOutputs) { | ||
| if (out.nodeId) interpolationContext[out.nodeId] = out.outputData; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 8 \
'buildInterpolationContext|normalizedName|resolveVariable|resolveConfigVariables|nodeId|node\.name|model .*Node|`@default`\(cuid\)|`@default`\(uuid\)' \
--glob '*.ts' --glob '*.tsx' --glob '*.prisma' . || trueRepository: Dev-Pross/BuildFlow
Length of output: 50376
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== interpolation implementation =="
sed -n '50,115p' packages/common/src/interpolation.ts
echo "== prisma schema Node and relation fields =="
sed -n '67,95p' packages/db/prisma/schema.prisma
sed -n '113,127p' packages/db/prisma/schema.prisma
echo "== workflow/node creation snippets =="
rg -n -C 5 'workflow\.nodes|nodes:|AvailableNodeID|NodeId|nodeId|name.*\s*:|name\s*:' --glob '*.ts' --glob '*.tsx' --glob '*.prisma' . || true
echo "== focused UUID prefix checks via static patterns =="
rg -n 'uuid\\(\\)|node\\.id|nodeId|node.*name|AvailableNode#|AvailableNodeID|createMany\\(|create\\(' --glob '*.ts' --glob '*.tsx' --glob '*.prisma' . || trueRepository: Dev-Pross/BuildFlow
Length of output: 50376
Prevent node IDs from overwriting normalized node-name keys.
buildInterpolationContext maps sanitized node names, then the execution loop writes raw node.id keys into the same context object. Since node IDs are UUIDs and node names can normalize to any lowercase snake_case string, interpolationContext[out.nodeId] = out.outputData can silently replace a name-based variable source. Use a separate ID namespace or block/emit a collision before insertion.
🤖 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/worker/src/engine/executor.ts` around lines 105 - 107, Update
buildInterpolationContext and its executedNodeOutputs loop so raw node IDs
cannot overwrite normalized node-name keys; use a distinct ID namespace for
ID-based entries, or detect and reject/report collisions before insertion, while
preserving name-based interpolation values.
…resolution in execution engine
Summary by CodeRabbit