Skip to content

refactor(workflow): add Jinja2 renderer abstraction for template transform#88

Open
tomerqodo wants to merge 6 commits intogreptile_combined_20260121_qodo_grep_cursor_copilot_1_base_refactorworkflow_add_jinja2_renderer_abstraction_for_template_transform_pr433from
greptile_combined_20260121_qodo_grep_cursor_copilot_1_head_refactorworkflow_add_jinja2_renderer_abstraction_for_template_transform_pr433
Open

refactor(workflow): add Jinja2 renderer abstraction for template transform#88
tomerqodo wants to merge 6 commits intogreptile_combined_20260121_qodo_grep_cursor_copilot_1_base_refactorworkflow_add_jinja2_renderer_abstraction_for_template_transform_pr433from
greptile_combined_20260121_qodo_grep_cursor_copilot_1_head_refactorworkflow_add_jinja2_renderer_abstraction_for_template_transform_pr433

Conversation

@tomerqodo
Copy link

Benchmark PR from qodo-benchmark#433

laipz8200 and others added 6 commits January 21, 2026 15:54
…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>
@greptile-apps
Copy link

greptile-apps bot commented Jan 21, 2026

Greptile Summary

Introduces a Jinja2 renderer abstraction layer for template transform nodes, replacing direct CodeExecutor calls with dependency-injected renderer pattern.

Key Changes:

  • Created Jinja2TemplateRenderer Protocol and CodeExecutorJinja2TemplateRenderer adapter in new template_renderer.py
  • Refactored TemplateTransformNode to accept renderer via constructor injection
  • Updated DifyNodeFactory to instantiate and wire the renderer
  • Updated all test mocks to use new renderer abstraction
  • Added 700+ lines of comprehensive edge-case tests

Issues Found:

  • template_renderer.py:37-40 - render_template() can return None violating its str return type annotation
  • node_factory.py:63 - passes code_executor parameter instead of self._code_executor to renderer initialization

Confidence Score: 3/5

  • This PR has two logic bugs that could cause runtime errors but is otherwise well-structured
  • The abstraction is well-designed with proper Protocol usage and dependency injection. However, two critical logic bugs were found: (1) render_template() can return None violating type annotations, and (2) incorrect parameter usage in factory initialization. The extensive test coverage is excellent, but these bugs need fixing before merge.
  • Pay close attention to template_renderer.py and node_factory.py - both contain logic bugs that must be fixed

Important Files Changed

Filename Overview
api/core/workflow/nodes/template_transform/template_renderer.py New file introducing Jinja2 renderer abstraction with Protocol and adapter pattern. Found critical issue: render_template can return None but type annotation declares str.
api/core/workflow/nodes/template_transform/template_transform_node.py Refactored to use dependency-injected template renderer instead of direct CodeExecutor calls. Clean implementation with proper error handling.
api/core/workflow/nodes/node_factory.py Wired template renderer into factory. Found issue: passes code_executor parameter directly instead of self._code_executor when initializing renderer.
api/tests/unit_tests/core/workflow/nodes/template_transform/template_transform_node_spec.py Updated all test mocks to match new renderer abstraction. Added extensive new test cases covering edge cases. Tests properly structured.

Sequence Diagram

sequenceDiagram
    participant Factory as DifyNodeFactory
    participant Node as TemplateTransformNode
    participant Renderer as Jinja2TemplateRenderer
    participant Adapter as CodeExecutorJinja2TemplateRenderer
    participant Executor as CodeExecutor
    
    Factory->>Factory: __init__(code_executor)
    Factory->>Adapter: create CodeExecutorJinja2TemplateRenderer(code_executor)
    Factory->>Node: create_node(config)
    Factory->>Node: TemplateTransformNode(template_renderer=adapter)
    
    Note over Node: Node execution starts
    Node->>Node: _run()
    Node->>Node: collect variables from variable_pool
    Node->>Renderer: render_template(template, variables)
    Renderer->>Adapter: render_template()
    Adapter->>Executor: execute_workflow_code_template(JINJA2, code, inputs)
    Executor-->>Adapter: {"result": rendered_string}
    Adapter->>Adapter: validate result is string
    Adapter-->>Node: rendered_string
    Node->>Node: check length <= MAX_TEMPLATE_TRANSFORM_OUTPUT_LENGTH
    Node-->>Factory: NodeRunResult(outputs={"output": rendered})
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

Comment on lines +37 to +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 rendered
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: render_template can return None if result.get("result") returns None, but the return type annotation is str (not str | None).

Suggested change
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
rendered = result.get("result")
if rendered is None:
raise TemplateRenderError("Template render result is missing.")
if not isinstance(rendered, str):
raise TemplateRenderError("Template render result must be a string.")
return rendered
Prompt To Fix With AI
This is a comment left during a code review.
Path: api/core/workflow/nodes/template_transform/template_renderer.py
Line: 37:40

Comment:
**logic:** `render_template` can return `None` if `result.get("result")` returns `None`, but the return type annotation is `str` (not `str | None`).

```suggestion
        rendered = result.get("result")
        if rendered is None:
            raise TemplateRenderError("Template render result is missing.")
        if not isinstance(rendered, str):
            raise TemplateRenderError("Template render result must be a string.")
        return rendered
```

How can I resolve this? If you propose a fix, please make it concise.

max_string_array_length=dify_config.CODE_MAX_STRING_ARRAY_LENGTH,
max_object_array_length=dify_config.CODE_MAX_OBJECT_ARRAY_LENGTH,
)
self._template_renderer = template_renderer or CodeExecutorJinja2TemplateRenderer(code_executor)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: passing code_executor parameter directly, but it could be None (before line 49 sets self._code_executor). Should use self._code_executor instead.

Suggested change
self._template_renderer = template_renderer or CodeExecutorJinja2TemplateRenderer(code_executor)
self._template_renderer = template_renderer or CodeExecutorJinja2TemplateRenderer(self._code_executor)
Prompt To Fix With AI
This is a comment left during a code review.
Path: api/core/workflow/nodes/node_factory.py
Line: 63:63

Comment:
**logic:** passing `code_executor` parameter directly, but it could be `None` (before line 49 sets `self._code_executor`). Should use `self._code_executor` instead.

```suggestion
        self._template_renderer = template_renderer or CodeExecutorJinja2TemplateRenderer(self._code_executor)
```

How can I resolve this? If you propose a fix, please make it concise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants