refactor(workflow): add Jinja2 renderer abstraction for template transform#3
Conversation
…de and threaded it through DifyNodeFactory so TemplateTransform nodes receive the dependency by default, keeping behavior unchanged unless an override is provided. Changes are in `api/core/workflow/nodes/template_transform/template_transform_node.py` and `api/core/workflow/nodes/node_factory.py`. **Commits** - chore(workflow): identify TemplateTransform dependency on CodeExecutor - feat(workflow): add CodeExecutor constructor injection to TemplateTransformNode (defaulting to current behavior) - feat(workflow): inject CodeExecutor from DifyNodeFactory when creating TemplateTransform nodes **Tests** - Not run (not requested) Next step: run `make lint` and `make type-check` if you want to validate the backend checks.
…Transform to use it, keeping CodeExecutor as the default adapter while preserving current behavior. Updates are in `api/core/workflow/nodes/template_transform/template_renderer.py`, `api/core/workflow/nodes/template_transform/template_transform_node.py`, `api/core/workflow/nodes/node_factory.py`, and `api/tests/unit_tests/core/workflow/nodes/template_transform/template_transform_node_spec.py`. Commit-style summary: - feat(template-transform): add Jinja2 template renderer abstraction with CodeExecutor adapter - refactor(template-transform): use renderer in node/factory and update unit test patches Tests not run (not requested).
…ode creation to return TemplateTransformNode directly for template-transform nodes in `api/core/workflow/nodes/node_factory.py`. Commit-style summary: - refactor(template-transform): derive TemplateRenderError from ValueError - refactor(node-factory): instantiate TemplateTransformNode directly with injected renderer Tests not run (not requested).
…ts/core/workflow/nodes/template_transform/template_transform_node_spec.py`) chore(type-check): ran `make type-check` (basedpyright clean, 0 errors) No errors reported.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
📝 WalkthroughWalkthroughThe changes introduce a Jinja2-based template rendering abstraction layer for workflow nodes. A new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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 review |
1 similar comment
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@api/core/workflow/nodes/template_transform/template_renderer.py`:
- Around line 29-39: render_template currently assigns rendered =
result.get("result") which may be None; ensure the function always returns a
string by checking for None and raising TemplateRenderError when rendered is
None or not a str. Specifically, in render_template (which calls
_code_executor.execute_workflow_code_template with CodeLanguage.JINJA2), after
obtaining result use an explicit check like "if rendered is None or not
isinstance(rendered, str): raise TemplateRenderError(...)" so callers (e.g.,
TemplateTransformNode) never receive None.
| def render_template(self, template: str, variables: Mapping[str, Any]) -> str: | ||
| try: | ||
| result = self._code_executor.execute_workflow_code_template( | ||
| language=CodeLanguage.JINJA2, code=template, inputs=variables | ||
| ) | ||
| except CodeExecutionError as exc: | ||
| raise TemplateRenderError(str(exc)) from exc | ||
|
|
||
| rendered = result.get("result") | ||
| if rendered is not None and not isinstance(rendered, str): | ||
| raise TemplateRenderError("Template render result must be a string.") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the CodeExecutor implementation and Jinja2 template handling
rg -n "def execute_workflow_code_template" --type-add "py" -A 10Repository: agentic-review-benchmarks/dify-combined-coderabbit
Length of output: 153
🏁 Script executed:
# Find CodeExecutor class definition
rg -n "class.*CodeExecutor" --type-add "py" -B 2 -A 5Repository: agentic-review-benchmarks/dify-combined-coderabbit
Length of output: 153
🏁 Script executed:
# Look for Jinja2TemplateTransformer implementation
rg -n "class.*Jinja2" --type-add "py" -A 10Repository: agentic-review-benchmarks/dify-combined-coderabbit
Length of output: 153
🏁 Script executed:
# Check TemplateTransformNode and how it uses render_template
fd "template_transform_node.py" -x cat -n {}Repository: agentic-review-benchmarks/dify-combined-coderabbit
Length of output: 4335
🏁 Script executed:
# Check the actual template renderer file to see current implementation
cat -n api/core/workflow/nodes/template_transform/template_renderer.pyRepository: agentic-review-benchmarks/dify-combined-coderabbit
Length of output: 1855
🏁 Script executed:
# Find the CodeExecutor implementation
fd -e py -type f | xargs rg -l "def execute_workflow_code_template" | head -5Repository: agentic-review-benchmarks/dify-combined-coderabbit
Length of output: 328
🏁 Script executed:
# Search for CodeExecutor class
fd -e py -type f | xargs rg "class CodeExecutor" -B 2 -A 15Repository: agentic-review-benchmarks/dify-combined-coderabbit
Length of output: 5598
🏁 Script executed:
# Get the execute_workflow_code_template method implementation
cat -n api/core/helper/code_executor/code_executor.py | head -200Repository: agentic-review-benchmarks/dify-combined-coderabbit
Length of output: 6571
🏁 Script executed:
# Search for execute_workflow_code_template specifically
rg -n "def execute_workflow_code_template" api/core/helper/code_executor/code_executor.py -A 20Repository: agentic-review-benchmarks/dify-combined-coderabbit
Length of output: 810
🏁 Script executed:
# Find Jinja2TemplateTransformer implementation
cat -n api/core/helper/code_executor/jinja2/jinja2_transformer.pyRepository: agentic-review-benchmarks/dify-combined-coderabbit
Length of output: 3410
🏁 Script executed:
# Also check TemplateTransformer base class to understand the contract
rg -n "class TemplateTransformer" api/core/helper/code_executor/template_transformer.py -A 30Repository: agentic-review-benchmarks/dify-combined-coderabbit
Length of output: 1302
🏁 Script executed:
# Get the rest of TemplateTransformer to see extract_result_str_from_response fully
rg -n "def extract_result_str_from_response" api/core/helper/code_executor/template_transformer.py -A 10Repository: agentic-review-benchmarks/dify-combined-coderabbit
Length of output: 622
Ensure render_template always returns a string, not None.
The return type is str, but result.get("result") can return None if the result dict lacks a "result" key or if future template transformers return unexpected values. This violates the contract and causes len(rendered) to crash in TemplateTransformNode line 69. Raise TemplateRenderError if rendered is None instead of returning it.
🔧 Proposed fix
def render_template(self, template: str, variables: Mapping[str, Any]) -> str:
try:
result = self._code_executor.execute_workflow_code_template(
language=CodeLanguage.JINJA2, code=template, inputs=variables
)
except CodeExecutionError as exc:
raise TemplateRenderError(str(exc)) from exc
rendered = result.get("result")
- if rendered is not None and not isinstance(rendered, str):
+ if not isinstance(rendered, str):
raise TemplateRenderError("Template render result must be a string.")
return rendered📝 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.
| def render_template(self, template: str, variables: Mapping[str, Any]) -> str: | |
| try: | |
| result = self._code_executor.execute_workflow_code_template( | |
| language=CodeLanguage.JINJA2, code=template, inputs=variables | |
| ) | |
| except CodeExecutionError as exc: | |
| raise TemplateRenderError(str(exc)) from exc | |
| rendered = result.get("result") | |
| if rendered is not None and not isinstance(rendered, str): | |
| raise TemplateRenderError("Template render result must be a string.") | |
| def render_template(self, template: str, variables: Mapping[str, Any]) -> str: | |
| try: | |
| result = self._code_executor.execute_workflow_code_template( | |
| language=CodeLanguage.JINJA2, code=template, inputs=variables | |
| ) | |
| except CodeExecutionError as exc: | |
| raise TemplateRenderError(str(exc)) from exc | |
| rendered = result.get("result") | |
| if not isinstance(rendered, str): | |
| raise TemplateRenderError("Template render result must be a string.") | |
| return rendered |
🤖 Prompt for AI Agents
In `@api/core/workflow/nodes/template_transform/template_renderer.py` around lines
29 - 39, render_template currently assigns rendered = result.get("result") which
may be None; ensure the function always returns a string by checking for None
and raising TemplateRenderError when rendered is None or not a str.
Specifically, in render_template (which calls
_code_executor.execute_workflow_code_template with CodeLanguage.JINJA2), after
obtaining result use an explicit check like "if rendered is None or not
isinstance(rendered, str): raise TemplateRenderError(...)" so callers (e.g.,
TemplateTransformNode) never receive None.
3 participants
Benchmark PR from agentic-review-benchmarks#3
Summary by CodeRabbit
Release Notes
New Features
Refactor
Tests