-
Notifications
You must be signed in to change notification settings - Fork 3
Disables direct recursion on templates by default #466
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
Conversation
eb8680
left a comment
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.
Rather than adding a recursive parameter to Template.define, can we expose this behavior via a special effectful.ops.types.Annotation that is applied to a return type and could in principle desugar to a more verbose but equivalent effect type, even if it's not (yet?) implemented that way?
class _IsRecursiveAnnotation(effectful.ops.types.Annotation): ...
IsRecursive = _IsRecursiveAnnotation()
...
@Tool.define
def search_web(query: str) -> str: ...
@Template.define
def research(topic: str) -> typing.Annotated[str, IsRecursive]: ...|
Also needs to be updated following #424 |
gotcha, can do!
gotcha! rebasing. |
c90355d to
284fbc2
Compare
|
Done! |
bb8f9b1 to
b41910b
Compare
eb8680
left a comment
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.
Thanks, just a couple nits
Fixes #447
On the meeting on Tuesday we discussed a few solutions, I believe this was the one we agreed would be the minimal fix?
#447 (comment)
Adds optional parameter
recursive(default value isFalse) toTemplate.defineand fieldis_recursiveto Templates.When constructing
tools, if theTemplateis not recursive, then it removes itself from the tuple of tools.This restores previous functionality, and in cases where users require direct recursive calls, they can call
Template.define(recursive=True)