Skip to content

fix: prompt-builder - jinja2 template set vars still shows required #9932

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+30 −2
Open

fix: prompt-builder - jinja2 template set vars still shows required #9932

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
136b7e0
fix: prompt-builder - jinja2 template set vars still shows required
srini047 Oct 24, 2025
f58094e
Merge branch 'main' into jinja2-set-vars-incorrect
srini047 Oct 24, 2025
492073e
Merge branch 'main' into jinja2-set-vars-incorrect
srini047 Oct 24, 2025
c78de27
Merge branch 'main' into jinja2-set-vars-incorrect
srini047 Oct 25, 2025
b102f58
Merge branch 'main' into jinja2-set-vars-incorrect
srini047 Oct 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions haystack/components/builders/prompt_builder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

from typing import Any, Literal, Optional, Union

from jinja2 import meta
from jinja2 import meta, nodes
from jinja2.sandbox import SandboxedEnvironment

from haystack import component, default_to_dict, logging
Expand Down Expand Up @@ -178,7 +178,13 @@ def __init__(
# infer variables from template
ast = self._env.parse(template)
template_variables = meta.find_undeclared_variables(ast)
variables = list(template_variables)

assigned_variables = set()
for node in ast.find_all((nodes.Assign, nodes.For)):
if hasattr(node.target, "name"):
assigned_variables.add(node.target.name)

variables = list(template_variables - assigned_variables)
Comment on lines +182 to +187
Copy link
Contributor

@sjrl sjrl Oct 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the addition, but this doesn't quite cover all the cases we would like. Let's use the following instead which also handles tuple and list unpacking when using set.

Also the nodes.For case doesn't seem to be needed. We don't pick up for-loop variables like doc in your test.

Suggested change
assigned_variables = set()
for node in ast.find_all((nodes.Assign, nodes.For)):
if hasattr(node.target, "name"):
assigned_variables.add(node.target.name)
variables = list(template_variables - assigned_variables)
assigned_variables = set()
for node in ast.find_all((nodes.Assign,)):
if isinstance(node.target, nodes.Name):
assigned_variables.add(node.target.name)
elif isinstance(node.target, (nodes.List, nodes.Tuple)):
for name_node in node.target.items:
if isinstance(name_node, nodes.Name):
assigned_variables.add(name_node.name)
variables = list(template_variables - assigned_variables)

variables = variables or []
self.variables = variables

Expand Down
22 changes: 22 additions & 0 deletions test/components/builders/test_prompt_builder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,3 +337,25 @@ def test_warning_no_required_variables(self, caplog):
with caplog.at_level(logging.WARNING):
_ = PromptBuilder(template="This is a {{ variable }}")
assert "but `required_variables` is not set." in caplog.text

def test_template_assigned_variables_from_required_inputs(self) -> None:
template = """{% if existing_documents is not none %}
{% set existing_doc_len = existing_documents|length %}
{% else %}
{% set existing_doc_len = 0 %}
{% endif %}
{% for doc in docs %}
<document reference="{{loop.index + existing_doc_len}}">
{{ doc.content }}
</document>
{% endfor %}
"""

builder = PromptBuilder(template=template, required_variables="*")
docs = [Document(content="Doc 1"), Document(content="Doc 2")]

res = builder.run(docs=docs, existing_documents=None)

Comment on lines +357 to +358
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's also explicitly test the found variables

Suggested change
res = builder.run(docs=docs, existing_documents=None)
builder = PromptBuilder(template=template, required_variables="*")
assert set(builder.variables) == {"docs", "existing_documents"}

assert "<document reference=" in res["prompt"]
assert "Doc 1" in res["prompt"]
assert "Doc 2" in res["prompt"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's also add a few more tests to test tuple and list unpacking

    def test_variables_correct_with_tuple_assignment(self):
        template = """{% if existing_documents is not none %}
{% set x, y = (existing_documents|length, 1) %}
{% else %}
{% set x, y = (0, 1) %}
{% endif %}
x={{ x }}, y={{ y }}
"""
        builder = PromptBuilder(template=template, required_variables="*")
        assert set(builder.variables) == {"existing_documents"}
        res = builder.run(existing_documents=None)
        assert "x=0, y=1" in res["prompt"]

    def test_variables_correct_with_list_assignment(self):
            template = """{% if existing_documents is not none %}
{% set x, y = [existing_documents|length, 1] %}
{% else %}
{% set x, y = [0, 1] %}
{% endif %}
x={{ x }}, y={{ y }}
"""
            builder = PromptBuilder(template=template, required_variables="*")
            assert set(builder.variables) == {"existing_documents"}
            res = builder.run(existing_documents=None)
            assert "x=0, y=1" in res["prompt"]

Loading