Skip to content

Conversation

@nbarbettini
Copy link
Contributor

Work in progress: Updates and cleanup in draft 1.0

@nbarbettini nbarbettini requested a review from Spartee March 5, 2025 21:14
@nbarbettini nbarbettini mentioned this pull request Mar 5, 2025
},
"additionalProperties": true
},
"user_id": {
Copy link

Choose a reason for hiding this comment

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

How is the validity of the user-id established? How is it used downstream?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Specifying a valid user ID is up to the client. The downstream tool may or may not need it.
I think this would be a good thing to call out in the requirements section, actually -- the difference between a tool that doesn't care who the user is (Calculator.Add) and one that does (Gmail.SearchEmails)

@nbarbettini
Copy link
Contributor Author

@eyurtsev My latest commit addresses most (not all) of your comments above. I will continue iterating tomorrow.

Copy link

@eyurtsev eyurtsev left a comment

Choose a reason for hiding this comment

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

forgot to submit the comments yesterday

"items": {
"type": "object",
"properties": {
"id": {

Choose a reason for hiding this comment

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

Do the secrets correspond to things like API keys that users will be passing directly as part of the request?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but passed by the client not by the user.
This gives the client the (optional) capability to load a secret like an API key when a tool is called, but keep that invisible from the caller/user.

Choose a reason for hiding this comment

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

Should we add an optional description field?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

description would be useful for humans, but not sure if it's useful for machines (the tool's code).

Choose a reason for hiding this comment

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

One workflow I was imagining is that a developer looks needs to add a specific API key as part of the context for a given tool.

e.g., the tool needs open_api_key, but the developer isn't familiar with what the key is or how to get access to it. So the description could include instructions on how to acquire the key.

And then the developer would client to pick up the key (e.g., as an env variable).


We can drop if this workflow doesn't make sense

Copy link
Contributor

@Spartee Spartee left a comment

Choose a reason for hiding this comment

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

One comment that can be addressed in further reviews and revisions.

},
"description": {
"type": "string",
"description": "A human-readable explanation of the tool's purpose. This field can be used by both humans and AI models."
Copy link
Contributor

Choose a reason for hiding this comment

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

I disagree that the emphasis should be on human readable. The main purpose of the description is to communicate to the predictor when and under what scenarios to use that particular tool.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I will clarify this

@nbarbettini nbarbettini merged commit a35d61f into main Mar 12, 2025
1 check passed
@nbarbettini nbarbettini deleted the 1.0-draft-updates branch March 12, 2025 18:45
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.

4 participants