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

[srini047]

Related Issues

• fixes PromptBuilder incorrectly marks internally set Jinja2 variables as required inputs when using `required_variables='*' #9916

Proposed Changes:

Previously we checked only the undeclared variables and left the set variables.
Now, we find all the variables assigned using set in the template and push it to the variables list.

This behaviour would be seen in multiple components namely:

• PromptBuilder
• ChatPromptBuilder
• LLMMetadataExtractor
• LLMDocumentContentExtractor

How did you test it?

Run the exisiting suite and added test case for the this scenario.

Notes for the reviewer

• Is this logic for addressing the problem is fine.
• Then, do let me know if there's any other component (other than ones mentioned above) as well that needs to be addressed with this logic.

Checklist

• I have read the contributors guidelines and the code of conduct
• I have updated the related issue with new insights and changes
• I added unit tests and updated the docstrings
• I've used one of the conventional commit types for my PR title: fix:, feat:, build:, chore:, ci:, docs:, style:, refactor:, perf:, test: and added ! in case the PR includes breaking changes.
• I documented my code
• I ran pre-commit hooks and fixed any issue

[vercel]

@srini047 is attempting to deploy a commit to the deepset Team on Vercel.

A member of the Team first needs to authorize it.

[coveralls]

Pull Request Test Coverage Report for Build 18847537461

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.002%) to 92.221%

---

Totals
Change from base Build 18846783538:	0.002%
Covered Lines:	13468
Relevant Lines:	14604

---

💛 - Coveralls

[sjrl]

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)

[sjrl]

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"}

[sjrl]

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"]

[sjrl]

With the above listed changes I think we could then extend this to the other components you mention. It may be wise to add this as a utility method in haystack/utils/ either in misc.py or a new file called jinja2.py