-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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.
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
All input parameters in PromptBuilder are optional #7441
Comments
This is actually intended. We made the decision to make all Also this is a pretty big breaking change, it fundamentally changes how you can connect a 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 Moreover, the graph does look a bit off when I run |
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 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. 🙏 |
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: #7553 |
Closing as fixed by #7553. |
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
System:
The text was updated successfully, but these errors were encountered: