refactor(workflow): add Jinja2 renderer abstraction for template transform#128
Open
tomerqodo wants to merge 6 commits into
Open
refactor(workflow): add Jinja2 renderer abstraction for template transform#128tomerqodo wants to merge 6 commits into
tomerqodo wants to merge 6 commits into
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>
Code Review by Qodo
1. render_template() may return None
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
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 |
There was a problem hiding this comment.
1. 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/None.
• TemplateTransformNode._run() then calls len(rendered), which will raise a TypeError and
bypass the intended NodeRunResult failure path.
• This creates an unhandled edge case and breaks graceful degradation for template rendering
failures.
Agent prompt
## Issue description
`CodeExecutorJinja2TemplateRenderer.render_template()` can return `None` even though it is annotated as returning `str`. This leads to a `TypeError` in `TemplateTransformNode._run()` when calling `len(rendered)`.
## Issue Context
`result.get("result")` may be missing or `None`. The node assumes a string and does not guard against `None`.
## 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-76]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+34
to
+36
| except CodeExecutionError as exc: | ||
| raise TemplateRenderError(str(exc)) from exc | ||
|
|
There was a problem hiding this comment.
2. templaterendererror leaks internals 📘 Rule violation ⛨ Security
• The code converts CodeExecutionError into TemplateRenderError(str(exc)), which can propagate internal implementation details. • TemplateTransformNode._run() returns error=str(e) in NodeRunResult, which may become user-visible depending on how node failures are surfaced. • This risks exposing sensitive/internal debugging information through error messages instead of keeping details in internal logs.
Agent prompt
## Issue description
`TemplateRenderError(str(exc))` may leak internal error details (from `CodeExecutionError`) and is returned as `NodeRunResult.error`.
## Issue Context
The compliance requirement is to keep user-facing errors generic, while retaining detailed context only in internal logs.
## Fix Focus Areas
- api/core/workflow/nodes/template_transform/template_renderer.py[30-36]
- api/core/workflow/nodes/template_transform/template_transform_node.py[75-76]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+793
to
+802
| @patch( | ||
| "core.workflow.nodes.template_transform.template_transform_node.CodeExecutorJinja2TemplateRenderer.render_template" | ||
| ) | ||
| def test_run_with_special_characters(self, mock_execute, mock_graph, mock_graph_runtime_state, graph_init_params): | ||
| """Test _run with special characters in variables.""" | ||
| node_data = { | ||
| "title": "Special Chars Template", | ||
| "variables": [{"variable": "text", "value_selector": ["sys", "input_text"]}], | ||
| "template": "{{ text }}", | ||
| } |
There was a problem hiding this comment.
3. Test file exceeds 800 📘 Rule violation ⛯ Reliability
• api/tests/unit_tests/core/workflow/nodes/template_transform/template_transform_node_spec.py now extends well beyond 800 lines (e.g., content exists at line 1151). • This violates the repository constraint to keep Python files under api/ below 800 lines to maintain readability and modularity. • The test suite should be split or consolidated (e.g., parameterization) to reduce file size.
Agent prompt
## Issue description
A Python test file under `api/` exceeds the 800-line limit.
## Issue Context
The test file contains many repetitive test cases that can likely be consolidated via parameterization or moved into multiple smaller spec modules.
## Fix Focus Areas
- api/tests/unit_tests/core/workflow/nodes/template_transform/template_transform_node_spec.py[430-1151]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Benchmark PR from qodo-benchmark#433