Add notebook with workaround for lengthy tool descriptions #13701
Conversation
dosubot
bot
added
the
size:XL
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
| @@ -70,6 +70,11 @@ def to_openai_function(self) -> Dict[str, Any]: | |||
|
|
|||
| def to_openai_tool(self) -> Dict[str, Any]: | |||
| """To OpenAI tool.""" | |||
| if len(self.description) > 1024: | |||
There was a problem hiding this comment.
my one comment is that (sadly) this function is used in a few things outside the scope of openai (namely, inside the huggingface llm integration, and the mistral llm integration.
Should we have an optional flag to disable this check for these cases?
There was a problem hiding this comment.
Ohh interesting, I was wondering why it appears in core. I can definitely add a flag, should we default to skipping the check?
There was a problem hiding this comment.
Since its specific to openai (or supposed to be lol), we should probably default to checking, and use the flag to disable
There was a problem hiding this comment.
Okay cool, should I update any of these other calls to set this flag? Will any break if we don't?
There was a problem hiding this comment.
yea lets update them to set the flag. They won't break per-say without it, but I'm not 100% sure if they have this limit
There was a problem hiding this comment.
Added the flag, let me know if I missed anything!
There was a problem hiding this comment.
Great write-up! And a decent workaround.
Do you think its worthing doing this automatically in the query plan tool constructor?
There was a problem hiding this comment.
Thanks! Yep we could easily modify the tool definition automatically, but if the user isn't aware that we've omitted the engine tool definitions, I suppose they won't know to include them in their agent query call. We could add an optional constructor param to omit the engine tool definitions, but this would be less automatic. Let me know what you think, I'm indifferent
There was a problem hiding this comment.
Ah right right -- the tool itself is still disconnected from the overall prompt
Ok, this is fine for now 👍🏻
Description
OpenAI tools have a limit on the length of the description provided, which is currently 1024 characters. In
QueryPlanTool, we ultimately concatenate a number of constituentQueryEngineToolsinto a single OpenAI tool. Obviously this character limit will be hit after adding enough engine tools, and probably pretty quickly. I've made some small changes and added a notebook to demonstrate an alternative solution: moving the engine tools to the prompt.I made a small opinionated change to allow manual overriding of the
QueryPlanToolmetadata, which will enable the separation of these tool components. Feel free to suggest another approach if it aligns better (e.g., we could have added an optional parameter toQueryToolPlanthat allows users to omit the tool descriptions in the metadata getter).Fixes #13159
New Package?
Did I fill in the
tool.llamahubsection in thepyproject.tomland provide a detailed README.md for my new integration or package?Version Bump?
Did I bump the version in the
pyproject.tomlfile of the package I am updating? (Except for thellama-index-corepackage)Type of Change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Suggested Checklist:
make format; make lintto appease the lint gods