Skip to content

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

Open
tomerqodo wants to merge 6 commits intoqodo_action_req_1_base_refactorworkflow_add_jinja2_renderer_abstraction_for_template_transform_pr3from
qodo_action_req_1_head_refactorworkflow_add_jinja2_renderer_abstraction_for_template_transform_pr3
Open

refactor(workflow): add Jinja2 renderer abstraction for template transform#141
tomerqodo wants to merge 6 commits intoqodo_action_req_1_base_refactorworkflow_add_jinja2_renderer_abstraction_for_template_transform_pr3from
qodo_action_req_1_head_refactorworkflow_add_jinja2_renderer_abstraction_for_template_transform_pr3

Conversation

@tomerqodo
Copy link

Benchmark PR from agentic-review-benchmarks#3

laipz8200 and others added 6 commits January 25, 2026 12:04
…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>
@qodo-code-review
Copy link

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (2) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Test file exceeds 800 lines 📘 Rule violation ⛯ Reliability
Description
• The updated test module under api/ is now well over the 800-line limit, reducing readability and
  maintainability.
• This violates the repository constraint requiring backend Python files under api/ to remain
  below 800 lines.
Code

api/tests/unit_tests/core/workflow/nodes/template_transform/template_transform_node_spec.py[R1120-1151]

+        assert result.status == WorkflowNodeExecutionStatus.SUCCEEDED
+        assert result.outputs["output"] == "abc"
+
+    @patch(
+        "core.workflow.nodes.template_transform.template_transform_node.CodeExecutorJinja2TemplateRenderer.render_template"
+    )
+    def test_run_with_escape_filter(self, mock_execute, mock_graph, mock_graph_runtime_state, graph_init_params):
+        """Test _run with escape filter for HTML."""
+        node_data = {
+            "title": "Escape Filter Template",
+            "variables": [{"variable": "html", "value_selector": ["sys", "html_content"]}],
+            "template": "{{ html | escape }}",
+        }
+
+        mock_html = MagicMock()
+        mock_html.to_object.return_value = "<script>alert('xss')</script>"
+
+        mock_graph_runtime_state.variable_pool.get.return_value = mock_html
+        mock_execute.return_value = "&lt;script&gt;alert(&#39;xss&#39;)&lt;/script&gt;"
+
+        node = TemplateTransformNode(
+            id="test_node",
+            config=node_data,
+            graph_init_params=graph_init_params,
+            graph=mock_graph,
+            graph_runtime_state=mock_graph_runtime_state,
+        )
+
+        result = node._run()
+
+        assert result.status == WorkflowNodeExecutionStatus.SUCCEEDED
+        assert "&lt;script&gt;" in result.outputs["output"]
Evidence
The compliance rule requires Python files under api/ to be fewer than 800 lines. The test file
contains code at line 1151, proving the file is at least 1151 lines long.

AGENTS.md
api/tests/unit_tests/core/workflow/nodes/template_transform/template_transform_node_spec.py[1120-1151]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`api/tests/unit_tests/core/workflow/nodes/template_transform/template_transform_node_spec.py` exceeds the 800-line maximum for Python files under `api/`.

## Issue Context
The file now reaches line ~1151, which violates the repository compliance constraint and makes the tests harder to maintain.

## Fix Focus Areas
- api/tests/unit_tests/core/workflow/nodes/template_transform/template_transform_node_spec.py[412-1151]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. render_template() may return None 📘 Rule violation ⛯ Reliability
Description
CodeExecutorJinja2TemplateRenderer.render_template() is annotated to return str but can return
  None when result.get("result") is missing.
• TemplateTransformNode._run() immediately calls len(rendered), which will raise a TypeError
  if rendered is None, causing an unhandled crash instead of a controlled failure result.
Code

api/core/workflow/nodes/template_transform/template_renderer.py[R37-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
Evidence
The renderer returns rendered without handling the None case, and the node run path assumes
rendered is always a string and calls len(rendered). This is a missing null/edge-case handling
at a clear failure point.

Rule 3: Generic: Robust Error Handling and Edge Case Management
api/core/workflow/nodes/template_transform/template_renderer.py[37-40]
api/core/workflow/nodes/template_transform/template_transform_node.py[68-70]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`CodeExecutorJinja2TemplateRenderer.render_template()` can return `None` even though its return type is `str`. This leads to `TemplateTransformNode._run()` calling `len(rendered)` and crashing with `TypeError`.

## Issue Context
`result.get(&quot;result&quot;)` may return `None` if the executor returns an unexpected payload; this should be handled as a controlled failure with a helpful error.

## Fix Focus Areas
- api/core/workflow/nodes/template_transform/template_renderer.py[29-40]
- api/core/workflow/nodes/template_transform/template_transform_node.py[67-80]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Tests use wrong ctor 🐞 Bug ✓ Correctness
Description
• Newly added unit tests instantiate TemplateTransformNode with graph=..., but
  TemplateTransformNode.__init__ does not accept a graph keyword argument, so these tests will
  raise TypeError and fail CI.
• These tests also pass a raw node_data dict as config, but the base Node initializer requires
  a wrapped config containing id and data; this will raise ValueError("Node ID is required.")
  even if the graph kwarg is removed.
Code

api/tests/unit_tests/core/workflow/nodes/template_transform/template_transform_node_spec.py[R451-457]

+        node = TemplateTransformNode(
+            id="test_node",
+            config=node_data,
+            graph_init_params=graph_init_params,
+            graph=mock_graph,
+            graph_runtime_state=mock_graph_runtime_state,
+        )
Evidence
The new tests pass graph=mock_graph and a raw node_data dict into TemplateTransformNode. The
production constructor signature has no graph parameter, and the base Node constructor enforces
that config must include an id and data mapping; therefore the test code will error at
construction time.

api/tests/unit_tests/core/workflow/nodes/template_transform/template_transform_node_spec.py[434-457]
api/core/workflow/nodes/template_transform/template_transform_node.py[26-41]
api/core/workflow/nodes/base/node.py[223-235]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The unit tests in `template_transform_node_spec.py` instantiate `TemplateTransformNode` with an unsupported `graph` keyword argument and with a `config` dict that is not shaped like the production node config (missing `id`/`data`). This will raise during test execution.

### Issue Context
`TemplateTransformNode` subclasses `Node`, and `Node.__init__` requires `config[&quot;id&quot;]` and `config[&quot;data&quot;]`.

### Fix Focus Areas
- api/tests/unit_tests/core/workflow/nodes/template_transform/template_transform_node_spec.py[434-470]
- api/tests/unit_tests/core/workflow/nodes/template_transform/template_transform_node_spec.py[58-66]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



ⓘ The new review experience is currently in Beta. Learn more

Qodo Logo

Comment on lines +1120 to +1151
assert result.status == WorkflowNodeExecutionStatus.SUCCEEDED
assert result.outputs["output"] == "abc"

@patch(
"core.workflow.nodes.template_transform.template_transform_node.CodeExecutorJinja2TemplateRenderer.render_template"
)
def test_run_with_escape_filter(self, mock_execute, mock_graph, mock_graph_runtime_state, graph_init_params):
"""Test _run with escape filter for HTML."""
node_data = {
"title": "Escape Filter Template",
"variables": [{"variable": "html", "value_selector": ["sys", "html_content"]}],
"template": "{{ html | escape }}",
}

mock_html = MagicMock()
mock_html.to_object.return_value = "<script>alert('xss')</script>"

mock_graph_runtime_state.variable_pool.get.return_value = mock_html
mock_execute.return_value = "&lt;script&gt;alert(&#39;xss&#39;)&lt;/script&gt;"

node = TemplateTransformNode(
id="test_node",
config=node_data,
graph_init_params=graph_init_params,
graph=mock_graph,
graph_runtime_state=mock_graph_runtime_state,
)

result = node._run()

assert result.status == WorkflowNodeExecutionStatus.SUCCEEDED
assert "&lt;script&gt;" in result.outputs["output"]

Choose a reason for hiding this comment

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

Action required

1. Test file exceeds 800 lines 📘 Rule violation ⛯ Reliability

• The updated test module under api/ is now well over the 800-line limit, reducing readability and
  maintainability.
• This violates the repository constraint requiring backend Python files under api/ to remain
  below 800 lines.
Agent prompt
## Issue description
`api/tests/unit_tests/core/workflow/nodes/template_transform/template_transform_node_spec.py` exceeds the 800-line maximum for Python files under `api/`.

## Issue Context
The file now reaches line ~1151, which violates the repository compliance constraint and makes the tests harder to maintain.

## Fix Focus Areas
- api/tests/unit_tests/core/workflow/nodes/template_transform/template_transform_node_spec.py[412-1151]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

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

Choose a reason for hiding this comment

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

Action required

2. render_template() may return none 📘 Rule violation ⛯ Reliability

CodeExecutorJinja2TemplateRenderer.render_template() is annotated to return str but can return
  None when result.get("result") is missing.
• TemplateTransformNode._run() immediately calls len(rendered), which will raise a TypeError
  if rendered is None, causing an unhandled crash instead of a controlled failure result.
Agent prompt
## Issue description
`CodeExecutorJinja2TemplateRenderer.render_template()` can return `None` even though its return type is `str`. This leads to `TemplateTransformNode._run()` calling `len(rendered)` and crashing with `TypeError`.

## Issue Context
`result.get("result")` may return `None` if the executor returns an unexpected payload; this should be handled as a controlled failure with a helpful error.

## Fix Focus Areas
- api/core/workflow/nodes/template_transform/template_renderer.py[29-40]
- api/core/workflow/nodes/template_transform/template_transform_node.py[67-80]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +451 to +457
node = TemplateTransformNode(
id="test_node",
config=node_data,
graph_init_params=graph_init_params,
graph=mock_graph,
graph_runtime_state=mock_graph_runtime_state,
)

Choose a reason for hiding this comment

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

Action required

3. Tests use wrong ctor 🐞 Bug ✓ Correctness

• Newly added unit tests instantiate TemplateTransformNode with graph=..., but
  TemplateTransformNode.__init__ does not accept a graph keyword argument, so these tests will
  raise TypeError and fail CI.
• These tests also pass a raw node_data dict as config, but the base Node initializer requires
  a wrapped config containing id and data; this will raise ValueError("Node ID is required.")
  even if the graph kwarg is removed.
Agent prompt
### Issue description
The unit tests in `template_transform_node_spec.py` instantiate `TemplateTransformNode` with an unsupported `graph` keyword argument and with a `config` dict that is not shaped like the production node config (missing `id`/`data`). This will raise during test execution.

### Issue Context
`TemplateTransformNode` subclasses `Node`, and `Node.__init__` requires `config["id"]` and `config["data"]`.

### Fix Focus Areas
- api/tests/unit_tests/core/workflow/nodes/template_transform/template_transform_node_spec.py[434-470]
- api/tests/unit_tests/core/workflow/nodes/template_transform/template_transform_node_spec.py[58-66]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

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