feat: `Tool` dataclass - unified abstraction to represent tools #8652

anakin87 · 2024-12-17T14:05:13Z

Related Issues

part of Port Tools from experimental #8631

Proposed Changes:

port the Tool dataclass from experimental (introduced in feat: Tool class haystack-experimental#53 + feat: convert function into Tool haystack-experimental#114)

How did you test it?

CI, several new tests

Notes for the reviewer

DO NOT MERGE
This PR is currently based on the new-chatmessage branch, to allow me to create other PRs for Chat Generators, which require both new ChatMessage and Tool.

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

coveralls · 2024-12-17T14:10:44Z

Pull Request Test Coverage Report for Build 12391961949

Details

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

Totals
Change from base Build 12376759419:	0.09%
Covered Lines:	8292
Relevant Lines:	9147

💛 - Coveralls

anakin87 · 2024-12-17T14:31:44Z

@dfokina take a look at this when possible

vblagoje

Only one comment - should we undermine UX/DX for agents and tools over 80kb binary? :-)

vblagoje · 2024-12-18T08:33:29Z

haystack/dataclasses/tool.py

+    function: Callable
+
+    def __post_init__(self):
+        jsonschema_import.check()


Knowing that Tool is central piece of the Agents push, should be we make this dependency default? It's 80Kb binary

I'm not totally sure. Let's involve also @julian-risch in the decision.

I remember that when we introduce new dependencies, we also have to pay attention to Anaconda.

We had some issues in the past with jsonschema: ci: Skip collection of test_json_schema.py to fix CI failures #7353

It seems to be not occurring on 3.9 and after, but ok, let's get this integrated and then we can experiment with including it as a default dependency. Or not.

dfokina

I capitalized all "Tools" in the docstrings so that it looks unified :)

* draft * del HF token in tests * adaptations * progress * fix type * import sorting * more control on deserialization * release note * improvements * support name field * fix chatpromptbuilder test * port Tool from experimental * release note * docs upd * Update tool.py --------- Co-authored-by: Daria Fokina <[email protected]>

…8605) * initial import * adding initial version + tests * adding more tests * more tests * incorporating SentenceSplitter based on NLTK * adding more tests * adding release notes * adding LICENSE header * removing unused imports * fixing example docstring * addding docstrings * fixing tests and returning a dictionary * updating release notes * attending PR comments * Update haystack/components/preprocessors/recursive_splitter.py Co-authored-by: Sebastian Husch Lee <[email protected]> * wip: updating tests for split_idx_start and _split_overlap * adding tests for split_idx and split_start and overlaps * adjusting file for LICENSE checking * adding more tests * adding tests for page numbering * adding tests for min split lenghts and falling back to character-level chunking based on size * fixing linting issue * Update haystack/components/preprocessors/recursive_splitter.py Co-authored-by: Sebastian Husch Lee <[email protected]> * Update haystack/components/preprocessors/recursive_splitter.py Co-authored-by: Sebastian Husch Lee <[email protected]> * Update haystack/components/preprocessors/recursive_splitter.py Co-authored-by: Sebastian Husch Lee <[email protected]> * Update haystack/components/preprocessors/recursive_splitter.py Co-authored-by: Sebastian Husch Lee <[email protected]> * Update haystack/components/preprocessors/recursive_splitter.py Co-authored-by: Sebastian Husch Lee <[email protected]> * Update haystack/components/preprocessors/recursive_splitter.py Co-authored-by: Sebastian Husch Lee <[email protected]> * Update haystack/components/preprocessors/recursive_splitter.py Co-authored-by: Sebastian Husch Lee <[email protected]> * Update haystack/components/preprocessors/recursive_splitter.py Co-authored-by: Sebastian Husch Lee <[email protected]> * wip * wip * updating tests * wip: fixing all tests after changes * more tests * wip: debugging sentence overlap * wip: debugging page number * wip * wip; fixed bug with sentence tokenizer, needs to keep white spaces * adding tests for counting pages on different split approaches * NLTK checks done on SentenceSplitter * fixing types * adding detecting for full overlap with previous chunks * fixing types * improving docstring * improving docstring * adding custom lenght, 'character' use case * customising overlap function for word and adding a few tests * updating docstring * Update haystack/components/preprocessors/recursive_splitter.py Co-authored-by: Sebastian Husch Lee <[email protected]> * Update haystack/components/preprocessors/recursive_splitter.py Co-authored-by: Sebastian Husch Lee <[email protected]> * Update haystack/components/preprocessors/recursive_splitter.py Co-authored-by: Sebastian Husch Lee <[email protected]> * wip: adding more tests for word unit length * fix * feat: `Tool` dataclass - unified abstraction to represent tools (#8652) * draft * del HF token in tests * adaptations * progress * fix type * import sorting * more control on deserialization * release note * improvements * support name field * fix chatpromptbuilder test * port Tool from experimental * release note * docs upd * Update tool.py --------- Co-authored-by: Daria Fokina <[email protected]> * fix: fix deserialization issues in multi-threading environments (#8651) * adding 'word' as default length * fixing types * handing both default strategies * wip * \f was not being counted properly * updating tests * fixing the overlap bug * adding more tests * refactoring _apply_overlap * further refactoring * Update haystack/components/preprocessors/recursive_splitter.py Co-authored-by: Sebastian Husch Lee <[email protected]> * Update haystack/components/preprocessors/recursive_splitter.py Co-authored-by: Sebastian Husch Lee <[email protected]> * Update haystack/components/preprocessors/recursive_splitter.py Co-authored-by: Sebastian Husch Lee <[email protected]> * Update haystack/components/preprocessors/recursive_splitter.py Co-authored-by: Sebastian Husch Lee <[email protected]> * adding ticks to close code block * fixing comments * applying changes: split with space and force keep_white_spaces=True * fixing some tests and replacing count words approach in more places * keep_white_spaces = True only if not defined * cleaning docs * handling some more edge cases, when split is still too big and all separators ran * fixing fallback whitespaces count to fixed word/char split based on split size * cleaning --------- Co-authored-by: Sebastian Husch Lee <[email protected]> Co-authored-by: Stefano Fiorucci <[email protected]> Co-authored-by: Daria Fokina <[email protected]> Co-authored-by: Tobias Wochinger <[email protected]>

anakin87 added 15 commits December 13, 2024 08:46

draft

4b941b0

del HF token in tests

9ec7de7

Merge branch 'fix-hf-token-test' into new-chatmessage

4865ab0

adaptations

7b6e9d2

progress

c462ddc

Merge branch 'main' into new-chatmessage

94a103a

fix type

873ae4f

import sorting

fe6c4c8

more control on deserialization

1a5b46c

release note

e3f4c89

Merge branch 'main' into new-chatmessage

2370c2f

improvements

180d0f3

support name field

328cebd

fix chatpromptbuilder test

b88daae

port Tool from experimental

a83b10c

github-actions bot added topic:tests topic:build/distribution type:documentation Improvements on the docs labels Dec 17, 2024

release note

b3c9381

anakin87 changed the title ~~feat: Tool dataclass~~ feat: Tool dataclass Dec 17, 2024

anakin87 changed the title ~~feat: Tool dataclass~~ feat: Tool dataclass - unified abstraction to represent tools Dec 17, 2024

anakin87 marked this pull request as ready for review December 17, 2024 14:30

anakin87 requested review from a team as code owners December 17, 2024 14:30

anakin87 requested review from dfokina and vblagoje and removed request for a team December 17, 2024 14:30

anakin87 changed the base branch from main to new-chatmessage December 17, 2024 15:03

Merge branch 'new-chatmessage' into tool-dataclass

65e73fd

Base automatically changed from new-chatmessage to main December 17, 2024 16:02

Merge branch 'main' into tool-dataclass

8a87019

vblagoje reviewed Dec 18, 2024

View reviewed changes

vblagoje self-requested a review December 18, 2024 09:14

vblagoje approved these changes Dec 18, 2024

View reviewed changes

docs upd

c50a119

dfokina approved these changes Dec 18, 2024

View reviewed changes

Update tool.py

b35a568

anakin87 enabled auto-merge (squash) December 18, 2024 11:28

anakin87 merged commit 96b4a1d into main Dec 18, 2024
20 checks passed

anakin87 deleted the tool-dataclass branch December 18, 2024 11:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: `Tool` dataclass - unified abstraction to represent tools #8652

feat: `Tool` dataclass - unified abstraction to represent tools #8652

anakin87 commented Dec 17, 2024 •

edited

Loading

coveralls commented Dec 17, 2024 •

edited

Loading

anakin87 commented Dec 17, 2024

vblagoje left a comment

vblagoje Dec 18, 2024

anakin87 Dec 18, 2024

vblagoje Dec 18, 2024 •

edited

Loading

dfokina left a comment

feat: Tool dataclass - unified abstraction to represent tools #8652

feat: Tool dataclass - unified abstraction to represent tools #8652

Conversation

anakin87 commented Dec 17, 2024 • edited Loading

Related Issues

Proposed Changes:

How did you test it?

Notes for the reviewer

Checklist

coveralls commented Dec 17, 2024 • edited Loading

Pull Request Test Coverage Report for Build 12391961949

Details

💛 - Coveralls

anakin87 commented Dec 17, 2024

vblagoje left a comment

Choose a reason for hiding this comment

vblagoje Dec 18, 2024

Choose a reason for hiding this comment

anakin87 Dec 18, 2024

Choose a reason for hiding this comment

vblagoje Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

dfokina left a comment

Choose a reason for hiding this comment

feat: `Tool` dataclass - unified abstraction to represent tools #8652

feat: `Tool` dataclass - unified abstraction to represent tools #8652

anakin87 commented Dec 17, 2024 •

edited

Loading

coveralls commented Dec 17, 2024 •

edited

Loading

vblagoje Dec 18, 2024 •

edited

Loading