Skip to content
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

fix: set USE_DOCSTRING as default for ai_callable #1266

Merged
merged 4 commits into from
Dec 23, 2024
Merged

Conversation

longcw
Copy link
Collaborator

@longcw longcw commented Dec 21, 2024

@llm.ai_callable need a explicit description=llm.USE_DOCSTRING to use the docstring as the description.

Copy link

changeset-bot bot commented Dec 21, 2024

🦋 Changeset detected

Latest commit: 8d8b11b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
livekit-agents Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@longcw longcw requested a review from a team December 21, 2024 03:04
Copy link
Member

@davidzhao davidzhao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is rather annoying tho as a requirement.. wdyt about just making that the default behavior?

if no explicit description is provided, it'll use docstring. if docstring isn't set, it raises

@longcw
Copy link
Collaborator Author

longcw commented Dec 21, 2024

I think that makes sense, most people will explicitly set a description or write that in the docstring.

At least the current example is very misleading, I've always assumed that I just need to set the description at docstring, but it actually also needs description=llm.USE_DOCSTRING to take effect.

@davidzhao
Copy link
Member

I think that makes sense, most people will explicitly set a description or write that in the docstring.

At least the current example is very misleading, I've always assumed that I just need to set the description at docstring, but it actually also needs description=llm.USE_DOCSTRING to take effect.

honestly I did too.. 🤦 I think it should just "do the obvious thing" here..

@davidzhao
Copy link
Member

@longcw do you wanna take a stab at making it the default behavior?

@longcw
Copy link
Collaborator Author

longcw commented Dec 22, 2024

@davidzhao sure, updated.

@longcw longcw changed the title fix: set USE_DOCSTRING for ai callable example fix: set USE_DOCSTRING as default for ai_callable Dec 22, 2024
@longcw longcw merged commit f0175c4 into main Dec 23, 2024
14 checks passed
@longcw longcw deleted the longc/fix/fnc-example branch December 23, 2024 02:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants