refactor(workflow): add Jinja2 renderer abstraction for template transform#16
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>
| rendered = result.get("result") | ||
| if rendered is not None and not isinstance(rendered, str): | ||
| raise TemplateRenderError("Template render result must be a string.") | ||
| return rendered |
There was a problem hiding this comment.
🔴 render_template can return None instead of str, causing TypeError in caller
The render_template method in CodeExecutorJinja2TemplateRenderer can return None when the result dict doesn't contain a "result" key or when the value is None, despite declaring a return type of str.
Click to expand
Bug mechanism
In template_renderer.py:37-40:
rendered = result.get("result")
if rendered is not None and not isinstance(rendered, str):
raise TemplateRenderError("Template render result must be a string.")
return renderedThe condition rendered is not None and not isinstance(rendered, str) only raises an error when rendered is not None and not a string. When rendered is None, the condition evaluates to False, so no error is raised and None is returned.
Impact
The caller in template_transform_node.py:69 does:
if len(rendered) > MAX_TEMPLATE_TRANSFORM_OUTPUT_LENGTH:If rendered is None, this will raise TypeError: object of type 'NoneType' has no len().
Actual vs Expected
- Actual: Returns
Nonewhen result doesn't contain "result" key or value isNone - Expected: Should raise
TemplateRenderErrorwhen the result is not a valid string
Recommendation: Change the condition to also handle the None case:
rendered = result.get("result")
if not isinstance(rendered, str):
raise TemplateRenderError("Template render result must be a string.")
return renderedWas this helpful? React with 👍 or 👎 to provide feedback.
Benchmark PR from qodo-benchmark#433