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

Enable overriding FunctionInvokingChatClient function discovery #5809

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

stephentoub
Copy link
Member

@stephentoub stephentoub commented Jan 21, 2025

Allows a derived instance to more easily pick which AIFunction to use based on the call content being provided.

Also augment failure message for a function not found with the name of the function requested, to help mitigate hallucination.

Microsoft Reviewers: Open in CodeFlow

Allows a derived instance to more easily pick which `AIFunction` to use based on the call content being provided.

Also augment failure message for a function not found with the name of the function requested, to help mitigate hallucination.
Copy link
Member

@SteveSandersonMS SteveSandersonMS left a comment

Choose a reason for hiding this comment

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

This seems totally fine though I'm not sure what the use case is. A subclass would rarely be able to select anything other than the function that matches by name, since the supplied args won't match any other function in general.

I'd be interested if you could clarify the use case, and it would be good to have a test case showing this works. But no objection to the new API.

@stephentoub
Copy link
Member Author

It came up primarily in the context of hallucinations where the LLM would send back a name that wasn't quite right, e.g. using periods to separate a plugin and function name in SK vs using an underscore. Basically giving a simpler hook in case someone wants to tweak just that one part.

If you don't think it's worthwhile, I'm fine closing it.

@SteveSandersonMS
Copy link
Member

I haven't experienced the need for it and am not certain in what cases a developer would realistically be able to fix any incorrect function names they received. If you're confident there are use cases, it seems reasonably safe to allow it, but if you're not then I'd be fine with leaving this until a need becomes clearer.

@RussKie RussKie added the area-AI label Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants