Open
Conversation
Rewrites tool_pipe as an inline canvas node that routes agent tool calls directly through connected output lanes (text/questions/documents/ table) using synchronous filter-chain execution, eliminating the need for an external RocketRide client, URI config, or separate .pipe file. Also fixes stack.cpp so that control-invoked nodes (e.g. tool_pipe) have their downstream data-flow connections included in pipeStack, enabling sub-pipeline nodes to receive binder bindings and participate in the filter chain. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
dylan-savage
requested review from
Rod-Christensen,
jmaionchi and
stepmikhaylov
as code owners
|
No description provided. |
github-actions
bot
added
module:server
Contributor
📝 WalkthroughWalkthroughAdds a new tool_pipe node exposing a sub-pipeline as an invokable tool: global config for description and return type, instance logic to open and route input through connected lanes and format results, a service descriptor, icon registration, and an engine stack traversal change to include downstream sub-pipeline nodes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Agent
participant ToolPipe as "tool_pipe Node"
participant SubPipeline as "Sub-Pipeline"
participant Resultizer as "Result Extractor"
Agent->>ToolPipe: invoke run_pipe(input)
ToolPipe->>ToolPipe: normalize input & choose lane
ToolPipe->>SubPipeline: open sub-instance and write to selected lane
SubPipeline->>SubPipeline: execute connected nodes
SubPipeline-->>ToolPipe: return entry.response
ToolPipe->>ToolPipe: convert response to dict and extract by return_type
ToolPipe->>Resultizer: serialize/extract final string
Resultizer-->>ToolPipe: formatted result
ToolPipe-->>Agent: return {'result': "<string>"}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nodes/src/nodes/tool_pipe/IInstance.py`:
- Around line 114-131: _send_to_connected_lane currently writes the same data to
every listener instead of honoring the “first connected output lane” contract
and also ignores the declared answers lane (_source). Change
_send_to_connected_lane to iterate over the output lanes in the node’s
advertised order and send the payload only to the first listener found (use
instance.hasListener checks in that order), mapping lane names to the correct
write calls (e.g., call writeQuestions for 'questions', writeDocuments for
'documents', writeTable for 'table', writeText for 'text') and add handling for
the 'answers' lane per services.json (_source) so the first matching lane
(including 'answers') receives the data and no other lanes are invoked.
- Around line 92-98: The current block can leave the pipeline instance open if
self.instance.open(entry) or _send_to_connected_lane(data) raises, so change the
flow to ensure self.instance.close() always runs when open succeeded: call
self.instance.open(entry), set a local flag (e.g., opened = True) immediately
after a successful open, perform _send_to_connected_lane(data) inside the try,
and in a finally block check the flag and call self.instance.close(); preserve
the existing exception/return behavior after closing. Reference: getObject(),
self.instance.open(...), _send_to_connected_lane(data), self.instance.close().
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 457d7f9c-8844-4b56-92fa-5590a833b3fb
📒 Files selected for processing (6)
nodes/src/nodes/tool_pipe/IGlobal.pynodes/src/nodes/tool_pipe/IInstance.pynodes/src/nodes/tool_pipe/__init__.pynodes/src/nodes/tool_pipe/requirements.txtnodes/src/nodes/tool_pipe/services.jsonpackages/server/engine-lib/engLib/store/stack.cpp
Collaborator
|
aside from CR comments - maybe ask Aaron for an SVG asset you can map for this node? |
- Ensure self.instance.close() always runs when open() succeeded by using an opened flag with a finally block - Add missing answers lane handler in _send_to_connected_lane to match services.json _source declaration - Add pipetool.svg icon and register it in get-icon-path.ts Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
nodes/src/nodes/tool_pipe/IInstance.py (1)
98-99:⚠️ Potential issue | 🟡 MinorSilent exception swallowing hides pipeline failures.
The bare
except Exceptioncatches and discards all errors without logging. This makes debugging sub-pipeline failures difficult. Consider logging the exception before returning the empty result.🛠️ Proposed fix
- except Exception: + except Exception as exc: + debug(f'tool_pipe: sub-pipeline failed: {exc}') return {'result': ''}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nodes/src/nodes/tool_pipe/IInstance.py` around lines 98 - 99, The except block in IInstance that currently swallows all exceptions (the "except Exception: return {'result': ''}" block) should capture the exception (e.g. "except Exception as e") and log it before returning so failures in sub-pipelines are visible; modify the handler inside the method in IInstance.py to log the exception with traceback (using the module logger or self.logger via logger.exception or logging.exception) and then return the same empty result, ensuring you reference the existing return {'result': ''} location.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nodes/src/nodes/tool_pipe/IInstance.py`:
- Around line 45-52: The code documents an 'image' return type in
_RETURN_TYPE_DESCRIPTIONS and services.json but there is no implementation in
routing or source lanes; either remove 'image' from the enum/dictionary or
implement full lane support. To fix, choose one: (A) remove 'image' from
_RETURN_TYPE_DESCRIPTIONS and from services.json return_type enum and any
references to image lanes; or (B) implement image support by adding an 'image'
branch to _send_to_connected_lane to route to the image lane, add 'image' to the
_source lanes in services.json, and ensure routing calls writeImage where other
lanes call writeText/writeJson (update any producers/consumers to handle base64
image payloads). Reference symbols: _RETURN_TYPE_DESCRIPTIONS,
_send_to_connected_lane, services.json _source lanes, and writeImage.
- Around line 138-139: The code currently calls
self.instance.writeAnswers([data]) passing a list of raw strings; instead create
an Answer object, set its text, and pass that Answer instance to writeAnswers.
Specifically, import/instantiate Answer, call answer.setAnswer(data) (or the
appropriate setter on the Answer class), and replace the list call with
self.instance.writeAnswers(answer) in the IInstance logic where
instance.hasListener('answers') is checked so writeAnswers receives a single
Answer object.
In `@nodes/src/nodes/tool_pipe/services.json`:
- Around line 13-14: The return_type enum includes "image" but the pipeline
lacks an image lane and handlers; either remove "image" from the return_type
enum to prevent unsupported configuration, or fully add image support by (a)
adding "image" to the "_source" lanes array in services.json, (b) implementing
image routing in IInstance._send_to_connected_lane to handle the "image" lane,
and (c) adding a writeImage method to the filter protocol (and any downstream
handlers) so the pipeline can accept and produce images; pick one of these two
options and apply it consistently across return_type, _source lanes,
IInstance._send_to_connected_lane, and the filter protocol.
---
Duplicate comments:
In `@nodes/src/nodes/tool_pipe/IInstance.py`:
- Around line 98-99: The except block in IInstance that currently swallows all
exceptions (the "except Exception: return {'result': ''}" block) should capture
the exception (e.g. "except Exception as e") and log it before returning so
failures in sub-pipelines are visible; modify the handler inside the method in
IInstance.py to log the exception with traceback (using the module logger or
self.logger via logger.exception or logging.exception) and then return the same
empty result, ensuring you reference the existing return {'result': ''}
location.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7ad7bc56-0353-471d-9e6a-5873c003789a
⛔ Files ignored due to path filters (1)
packages/shared-ui/src/assets/nodes/pipetool.svgis excluded by!**/*.svg
📒 Files selected for processing (3)
nodes/src/nodes/tool_pipe/IInstance.pynodes/src/nodes/tool_pipe/services.jsonpackages/shared-ui/src/modules/flow/util/get-icon-path.ts
…wers - Remove 'image' from return_type enum and VALID_RETURN_TYPES — no corresponding lane, write handler, or _source declaration exists - Fix writeAnswers call to pass an Answer object instead of a raw string list, matching the contract used by all other nodes Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nodes/src/nodes/tool_pipe/IGlobal.py`:
- Around line 54-65: validateConfig currently treats whitespace-only return_type
as valid while beginGlobal/runtime rejects it; change the normalization in
validateConfig to match runtime by doing return_type =
(str(cfg.get('return_type') or '').strip() or 'text') (or equivalent) before the
membership check so whitespace-only values default to 'text' and validation
behavior matches the constructor/runtime path (see validateConfig and the place
that sets self.return_type).
In `@nodes/src/nodes/tool_pipe/IInstance.py`:
- Around line 94-99: The current try/except in IInstance swallows all exceptions
from self.instance.open(entry) and _send_to_connected_lane(data) and returns
{'result': ''}, hiding real failures; change the except to catch Exception as e,
perform necessary cleanup (undo any partial state e.g., if you set opened = True
then call the instance cleanup method or close on self.instance, or reset any
flags), then either re-raise the original exception (raise) or return an
explicit failure payload such as {'error': str(e), 'result': None}; update the
except block around self.instance.open and _send_to_connected_lane to use the
exception variable e and perform cleanup before re-raising or returning the
explicit error payload.
In `@nodes/src/nodes/tool_pipe/services.json`:
- Around line 10-20: Update the tile description in
nodes/src/nodes/tool_pipe/services.json to reflect the actual fan-out behavior
(input is copied to every connected listener) instead of implying single-lane
selection; reference the runtime behavior implemented in
nodes/src/nodes/tool_pipe/IInstance.py (the code around the send-to-listeners
loop at lines ~118-143) and change phrases like “routed to whichever lane has a
connected node” to something like “sent to all connected lanes/listeners” and
clarify that multiple downstream branches will all receive the input.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4553ba49-415d-4e74-b1da-8a5fa8273b5c
📒 Files selected for processing (3)
nodes/src/nodes/tool_pipe/IGlobal.pynodes/src/nodes/tool_pipe/IInstance.pynodes/src/nodes/tool_pipe/services.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
output lanes to any downstream nodes on the same canvas, and the agent can invoke the full pipeline as a
named tool
pipeStack, enabling sub-pipeline nodes to bind correctly and participate in the filter chain
Type
Testing
./builder testpassesChecklist
Linked Issue
Fixes #646
Summary by CodeRabbit
New Features
Bug Fixes