-
Notifications
You must be signed in to change notification settings - Fork 460
fix: Strip argument sections out of inputSpec top-level description #1142
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
base: main
Are you sure you want to change the base?
fix: Strip argument sections out of inputSpec top-level description #1142
Conversation
Per strands-agents#1067 including the args in the description is redundant as it's already included in the parameter docs which can increase the token counts. Strip args from the description strings for inputSpecs
| spec = tool_with_full_docstring.tool_spec | ||
| assert ( | ||
| spec["description"] | ||
| == """This is the main description. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: So that you can present this more cleanly in code, you can use textwrap.dedent:
import textwrap
...
description = textwrap.dedent("""
This is the main description.
...
""")| stripped_line = line.strip() | ||
|
|
||
| # Check if we're starting the Args section | ||
| if stripped_line.lower().startswith(("args:", "arguments:", "parameters:", "param:", "params:")): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consideration: Not saying it needs to be done for this PR, but I'm wondering if there is a reliable third party library out there that we can use to parse the docstrings. From a cursory search, I came across https://pypi.org/project/docstring-parser/. It's in preview though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah - I also looked for built in ways to do this and didn't see any great ways. I think if we were starting today we'd do this differently so that we're not managing raw strings.
I'm a bit reluctant to add a library just this :(
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Description
Per #1067 including the args in the description is redundant as it's already included in the parameter docs which can increase the token counts. Strip args from the description string for inputSpecs
Note: This is changing the inputSpecs that we're generating which has the risk of changing agent behavior. However, I think the reduction in token counts is worth it given that argument descriptions already have this information.
Related Issues
#1067
Documentation PR
N/A - this more closely aligns our docs
Type of Change
Bug fix
Testing
How have you tested the change? Verify that the changes do not break functionality or introduce warnings in consuming repositories: agents-docs, agents-tools, agents-cli
hatch run prepareChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.