Feat: Function Tool decorator and Workflow as code part 1#216
Merged
srijanpatel merged 17 commits intomainfrom Mar 14, 2025
Merged
Feat: Function Tool decorator and Workflow as code part 1#216srijanpatel merged 17 commits intomainfrom
srijanpatel merged 17 commits intomainfrom
Conversation
… models from primitive types
…nd adding __init__.py files
…unctions from tools directory
… creation and code conversion
…ool_function decorator
… in BaseNode documentation
Contributor
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to 89ad246 in 3 minutes and 45 seconds
More details
- Looked at
3118lines of code in14files - Skipped
0files when reviewing. - Skipped posting
14drafted comments based on config settings.
1. backend/pyspur/execution/workflow_executor.py:540
- Draft comment:
Review: Changing the key from the dependency node ID to the node’s name could lead to collisions if node names aren’t unique. Consider using the node ID instead. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. backend/pyspur/utils/pydantic_utils.py:10
- Draft comment:
Review: The get_nested_field function may exit too early when encountering a dict, preventing proper traversal of nested structures. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. backend/pyspur/nodes/decorator.py:230
- Draft comment:
Review: Using print() for template rendering debug info may be too verbose for production. Consider using a proper logger with an appropriate log level. - Reason this comment was not posted:
Marked as duplicate.
4. backend/pyspur/workflow_code_handler.py:162
- Draft comment:
Review: Using exec() for parsing workflow code is a potential security risk. Ensure the executed code is trusted or properly sandboxed. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
The use of exec() here appears to be intentionally restricted - it only has access to WorkflowBuilder in its globals. The code is part of a workflow system where users are expected to write Python code to define workflows. Some form of code execution is necessary for this functionality. The current implementation already includes basic sandboxing by restricting the available globals.
The comment raises a valid security concern, and code execution should always be treated carefully. However, this appears to be a core feature requirement rather than a security oversight.
While code execution is inherently risky, this implementation already includes reasonable restrictions and appears to be a necessary part of the workflow system's design. The current sandboxing approach is appropriate for the use case.
The comment should be removed as the code already implements appropriate restrictions for its use case, and the security consideration appears to be handled appropriately for the requirements.
5. frontend/src/components/canvas/EditorCanvas.tsx:349
- Draft comment:
Review: Repeated re-computation of layouted nodes and grouping may affect performance. Consider using memoization to optimize these calculations. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
6. backend/pyspur/utils/pydantic_utils.py:10
- Draft comment:
The get_nested_field function returns immediately when encountering a dict, which prevents proper nested iteration. Update the loop to update 'value' rather than returning on the first key. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
7. backend/pyspur/execution/workflow_executor.py:546
- Draft comment:
Storing node outputs in the node input using the node's name (instead of its ID) may risk collisions if multiple nodes share the same name. Verify that node names are unique or consider preserving IDs for input keys. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
8. frontend/src/components/canvas/EditorCanvas.tsx:351
- Draft comment:
The useCopyPaste hook is invoked twice—this redundant call should be removed to avoid unnecessary re-renders or side‐effects. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
9. backend/pyspur/cli/main.py:106
- Draft comment:
Typo found: 'protoype' should be corrected to 'prototype' in the quick prototype instruction text. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
10. backend/pyspur/examples/tool_function_example.py:238
- Draft comment:
There's a duplicated assignment in the math tool node test: 'config = config = config.model_validate({...})'. This is a minor typographical error and should be corrected to a single assignment: 'config = config.model_validate({...})'. - Reason this comment was not posted:
Marked as duplicate.
11. backend/pyspur/examples/workflow_as_code_example.py:41
- Draft comment:
The model name 'openai/gpt-4o' appears in multiple nodes. Please verify if this is intentional or if it should be 'openai/gpt-4'. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50%
This is a new file showing example workflows. The model name "gpt-4o" does look unusual and could be a typo of "gpt-4". However, without knowing the internal model naming conventions of this codebase, I can't be certain that "gpt-4o" isn't a valid model name. The consistent use across multiple nodes suggests it might be intentional.
I might be too hesitant - if "gpt-4o" is not a real model name, this could cause the examples to fail. Also, the fact that they use "anthropic/claude-3-haiku" in one place but "openai/gpt-4o" elsewhere is suspicious.
While the concern is valid, this is an example file and we don't have enough context to be certain that "gpt-4o" is incorrect. Making assumptions about model names without full knowledge of the codebase could lead to incorrect changes.
The comment should be deleted as it asks for verification without being certain of an issue, and we don't have enough context to know if the model name is actually incorrect.
12. backend/pyspur/nodes/base.py:187
- Draft comment:
The call method docstring for the 'input' parameter has a typographical error: it shows 'Dict[str, NodeOutputModel]'. It would be clearer to use a comma (e.g., 'Dict[str, NodeOutputModel]') or an appropriate placeholder, to accurately represent the type. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
The comment is about a real change in the diff - the docstring was modified. The suggestion is technically correct - the type annotation format is incorrect. However, this is a very minor documentation issue that doesn't affect functionality. The meaning is still clear to readers. The comment feels overly pedantic.
The incorrect type annotation could confuse new developers who are learning Python type hints. Bad documentation practices, even minor ones, can propagate if not corrected.
While technically correct, this is an extremely minor documentation issue. The meaning is still clear, and we should focus review comments on more substantive issues.
Delete this comment as it's too minor of an issue to warrant a review comment. The docstring is understandable as-is.
13. backend/pyspur/nodes/registry.py:1
- Draft comment:
The header comment on line 1 currently reads "# backend/app/nodes/registry.py", which doesn't match the file's actual path (backend/pyspur/nodes/registry.py). Please update the header comment for clarity. - Reason this comment was not posted:
Comment was on unchanged code.
14. backend/pyspur/workflow_builder.py:37
- Draft comment:
Consider checking the model name 'openai/gpt-4o' in the llm_info configuration. If this is a typo and should be 'openai/gpt-4', please update it to avoid confusion. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50%
Since this is in example code, it's important that it be correct to avoid confusing users. However, we don't actually know if "openai/gpt-4o" is a typo - it could be a valid model name in their system. Without access to the full codebase and knowledge of valid model names, we can't be certain this is an error. The comment is speculative.
I might be too hesitant - if it looks like a typo and the model name is clearly wrong, maybe we should flag it. Example code should be held to a high standard.
While good example code is important, making assumptions about valid model names without full context could lead to incorrect suggestions. The comment itself uses speculative language ("If this is a typo...") which indicates uncertainty.
The comment should be deleted because it makes assumptions about valid model names without sufficient context and is speculative in nature.
Workflow ID: wflow_Quhnl1DUdVq25I7Y
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
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
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.
This pull request introduces a new feature for workflow code conversion, enhances the project initialization process, and adds an example script for custom tools. The most important changes include adding a new router for workflow code conversion, implementing the workflow code conversion feature, updating the project initialization to create additional directories and files, and providing an example script for creating custom tools.
New Feature: Function Tool Decorator
/pyspur/backend/pyspur/nodes/decorator.py: Added this decorator to convert functions with serializable I/O to PySpur nodes/toolsNew Feature: Workflow Code Conversion
backend/pyspur/api/api_app.py: Added a new router for workflow code conversion and included it in the API application. [1] [2]backend/pyspur/api/workflow_code_convert.py: Implemented the workflow code conversion feature with endpoints for generating code from workflows, creating workflows from code, updating workflows from code, and converting between workflow definitions and code.Project Initialization Enhancements
backend/pyspur/cli/main.py: Updated the project initialization process to create additional directories (tools,spurs) and files (__init__.py,.gitignore).Example Script for Custom Tools
backend/pyspur/examples/tool_function_example.py: Added an example script demonstrating how to create custom tools using the@tool_functiondecorator, including various tool functions and their usage.Important
Introduces workflow code conversion, enhances project initialization, and adds examples for custom tools and workflows.
@tool_functiondecorator indecorator.pyto convert functions to PySpur nodes/tools.workflow_code_convert.pywith endpoints for generating code from workflows, creating workflows from code, and updating workflows.main.pyto create additional directories (tools,spurs) and files (__init__.py,.gitignore) during project initialization.tool_function_example.pydemonstrating custom tool creation using@tool_function.workflow_as_code_example.pyshowcasing workflow creation usingWorkflowBuilder.workflow_executor.pyandbase.pyto support new features.This description was created by
for 89ad246. It will automatically update as commits are pushed.