All input parameters in PromptBuilder are optional
Describe the bug All input parameters in PromptBuilder have a default empty-string value and are thus marked as optional. I guess this is not the intended behavior according to the API documentations (https://docs.haystack.deepset.ai/reference/builders-api#promptbuilder). This does not have a big impact in terms of execution, but it does mess up the auto-generated pipeline graphs so I decided to report and fix it.
Error message N/A
Expected behavior {'is_mandatory': True} for all PromptBuilder inputs
Additional context An one-line fix should be enough: https://github.com/deepset-ai/haystack/blob/main/haystack/components/builders/prompt_builder.py#L33
To Reproduce Follow the 2.0 RAG pipeline recipe from https://docs.haystack.deepset.ai/docs/creating-pipelines
FAQ Check
- [x] Have you had a look at our new FAQ page?
System:
- OS: not relevent
- GPU/CPU: not relevent
- Haystack version (commit or version number): haystack-ai==2.0.0
- DocumentStore: not relevent
- Reader: not relevent
- Retriever: not relevent
This is actually intended.
We made the decision to make all PromptBuilder inputs as optional to support some use cases like self correcting loops in a Pipeline. If the inputs are not optional you would not be able to create loops in the Pipeline using a PromptBuilder that easily.
Also this is a pretty big breaking change, it fundamentally changes how you can connect a PromptBuilder in a Pipeline.
In the end I wouldn't do this, though I would like to know more about your use case if you need all inputs to be mandatory. If it's a valid use case we can find a way to make it work if we're missing something.
This is actually intended.
We made the decision to make all
PromptBuilderinputs as optional to support some use cases like self correcting loops in a Pipeline. If the inputs are not optional you would not be able to create loops in the Pipeline using aPromptBuilderthat easily.Also this is a pretty big breaking change, it fundamentally changes how you can connect a
PromptBuilderin a Pipeline.In the end I wouldn't do this, though I would like to know more about your use case if you need all inputs to be mandatory. If it's a valid use case we can find a way to make it work if we're missing something.
Thanks for the reply and sharing the context. I think it does make sense either way, but the code behavior does not seem to match its documentation so I got a bit confused.
I would prefer that we can have an optional parameter during PromptBuilder initialization to tell whether to make (some of) the jinja inputs mandatory though (this also avoids breaking change since you could make the default behavior optional). As for my use case, I was trying out a customized RAG pipeline as a showcase to my colleagues, and I would prefer to do a sanity check to see if some key inputs are not provided, say, if "user question" is somehow missing when I use a LLM to generate a search query. It does feel weird if I would need to put this check elsewhere, and I would like to make this statement clear by showing pipeline.inputs().
Moreover, the graph does look a bit off when I run pipeline.draw(). For example, when I run 2.0 RAG Pipeline and here is what I get for pipeline.draw():
On the contrary, this is the graph shown on the documentation page, which I think looks much better (ignore the
AnswerBuilder part for now):
I think in this case prompt_builder expects a question input, and this information is much more clearly illustrated in the second graph.
I would prefer that we can have an optional parameter during
PromptBuilderinitialization to tell whether to make (some of) the jinja inputs mandatory though (this also avoids breaking change since you could make the default behavior optional).
This is a valid requests I'd say, I would keep the current behaviour and add a flag that makes the inputs non optional. Feel free to update the PR you already opened or create a new one from scratch if you want to take care of this.
The documentation is indeed wrong.
We heavily refactored the Pipeline.run() method before the 2.0.0 release and we had to change the PromptBuilder accordingly to support certain use cases. It was also one of the first Components we created when work on Haystack 2 started, the documentation was written at the time too. Same thing for the Pipeline graph.
We probably forgot to update the docs after those changes, I'll open a PR to update the docstring and update the documentation.
Thanks for the insights, really helpful. 🙏
This is a valid requests I'd say, I would keep the current behaviour and add a flag that makes the inputs non optional. Feel free to update the PR you already opened or create a new one from scratch if you want to take care of this.
Thank you for acknowledging the validity of the request. Sure, I can update the PR with the necessary modifications. You may expect these updates by early next week.
Thanks for the quick docstring fix and all the insightful discussions too! I'm glad to contribute to the improvement of the project, it's my fav framework for building LLM apps right now :)
Awesome! I'll be on the lookout for the updated PR, feel free to ping me directly if you want. 💪
I decided to open a new PR since it is no longer a fix: https://github.com/deepset-ai/haystack/pull/7553
Closing as fixed by #7553.