fix: make from dict conditional router more resilient - Githubissues

deepset-ai / haystack

:mag: AI orchestration framework to build customizable, production-ready LLM applications. Connect components (models, vector DBs, file converters) to pipelines or agents that can interact with your data. With advanced retrieval methods, it's best suited for building RAG, question answering, semantic search or conversational agent chatbots.

https://haystack.deepset.ai

Apache License 2.0

16.93k stars 1.85k forks source link

fix: make from dict conditional router more resilient #8343

Closed ArzelaAscoIi closed 2 weeks ago

ArzelaAscoIi commented 2 weeks ago

Related Issues

fixes from_dict for conditional router to be resilient against explicit "None" mappings

Proposed Changes:

parse explicit None mappings for deserialization as empty dict

How did you test it?

unit test

Notes for the reviewer

although this can't happen if you call from_dict(to_dict) it can still happen if you change the yaml and (maybe by accident) set the key custom_filters to "None" based on the function signature.

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:.
I documented my code
I ran pre-commit hooks and fixed any issue

coveralls commented 2 weeks ago

Pull Request Test Coverage Report for Build 10772740336

Details

0 of 0 changed or added relevant lines in 0 files are covered.
2 unchanged lines in 1 file lost coverage.
Overall coverage increased (+0.002%) to 90.302%

Files with Coverage Reduction	New Missed Lines	%
components/routers/conditional_router.py	2	97.78%
<!--	Total:	2		-->

Totals
Change from base Build 10768832162:	0.002%
Covered Lines:	7086
Relevant Lines:	7847

💛 - Coveralls

vblagoje commented 2 weeks ago

@ArzelaAscoIi do we have the same issue in OutputAdapter?

ArzelaAscoIi commented 2 weeks ago

https://github.com/deepset-ai/haystack/blob/75955922b95351b8f16d501ec135d91bad78f80e/haystack/components/converters/output_adapter.py#L169

The output adapter should be fine! :)

vblagoje commented 2 weeks ago

Note: let's include this one in the first patch release 2.5.1