-
Notifications
You must be signed in to change notification settings - Fork 25
WIP - API iteration #69
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR appears to be a work-in-progress API iteration that introduces significant debugging output and a new decorator-based API for defining workflows and nodes. The changes include extensive print statements for debugging event flow and message processing, along with a comprehensive new decorator system for workflow definition.
- Addition of extensive debugging print statements throughout the workflow execution pipeline
- Implementation of a new decorator-based API system for defining workflows, nodes, and their relationships
- Addition of traceback printing for better error debugging in workflow execution
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| grafi/workflows/impl/event_driven_workflow.py | Adds debug print statements and traceback output for workflow event processing |
| grafi/nodes/node.py | Adds debug print statement in node invocation |
| grafi/common/models/command.py | Adds debug print statement for command message yielding |
| grafi/common/decorators/record_workflow_a_invoke.py | Adds debug print statements for workflow invocation decorator |
| grafi/common/decorators/record_node_a_invoke.py | Adds debug print statements for node invocation decorator |
| grafi/api/decorators.py | Introduces new comprehensive decorator-based API for workflow and node definition |
| ) -> MsgsAGen: | ||
| if self._a_invoke_impl is None: | ||
| raise NotImplementedError("Must provide `_a__invoke_impl` Callable.") | ||
| yield self._a_invoke_impl(self, invoke_context, input_data) |
Copilot
AI
Aug 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method _a_invoke_impl is being called with self as the first parameter, but it's defined as a Callable that expects (InvokeContext, List[Message]). This will cause a TypeError as the callable doesn't expect a self parameter.
| yield self._a_invoke_impl(self, invoke_context, input_data) | |
| yield self._a_invoke_impl(invoke_context, input_data) |
|
|
||
| all_subscibed_topics = {topic.name: False for topic in self.subscribed_topics} | ||
| available_topics = all_subscibed_topics | {topic.name: True for topic in topic_with_messahes} | ||
| return eval(self._node_condiiton, __builtins__ ={}, globals=None, locals=available_topics) |
Copilot
AI
Aug 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a typo in the variable name _node_condiiton - it should be _node_condition to match the attribute defined in the constructor.
| return eval(self._node_condiiton, __builtins__ ={}, globals=None, locals=available_topics) | |
| return eval(self._node_condition, __builtins__ ={}, globals=None, locals=available_topics) |
|
|
||
| all_subscibed_topics = {topic.name: False for topic in self.subscribed_topics} | ||
| available_topics = all_subscibed_topics | {topic.name: True for topic in topic_with_messahes} | ||
| return eval(self._node_condiiton, __builtins__ ={}, globals=None, locals=available_topics) |
Copilot
AI
Aug 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using eval() with user-provided input is a security risk. Consider using a safer expression evaluation library like ast.literal_eval() or a dedicated expression parser to prevent code injection attacks.
| consumed_events=consumed_events, | ||
| ) | ||
| ) | ||
| print("NDOE OUTPUT") |
Copilot
AI
Aug 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in debug message: 'NDOE' should be 'NODE'.
| print("NDOE OUTPUT") | |
| print("NODE OUTPUT") |
| _a_invoke_impl: Callable[[InvokeContext, List[Message]], MsgsAGen] = None | ||
|
|
||
| def __init__(self, tool_invoke: Callable[[InvokeContext, Messages], MsgsAGen], **kwargs): | ||
| super().__init__(name = "CallableTool", oi_span_type=OpenInferenceSpanKindValues.TOOL, type="CallableTool", **kwargs) |
Copilot
AI
Aug 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Inconsistent spacing around the = operator. Should be consistent with Python PEP 8 style guide (no spaces around = in keyword arguments).
| super().__init__(name = "CallableTool", oi_span_type=OpenInferenceSpanKindValues.TOOL, type="CallableTool", **kwargs) | |
| super().__init__(name="CallableTool", oi_span_type=OpenInferenceSpanKindValues.TOOL, type="CallableTool", **kwargs) |
No description provided.