Skip to content

Conversation

stephentoub
Copy link
Member

@stephentoub stephentoub commented Apr 17, 2025

Add AIFunctionFactoryOptions.Services, and use it when examining function parameters to determine whether they should be resolved by default from DI.

Closes #6312, whether or not we decide to merge this.

Microsoft Reviewers: Open in CodeFlow

Add AIFunctionFactoryOptions.Services, and use it when examining function parameters to determine whether they should be resolved by default from DI.
@stephentoub stephentoub requested a review from a team as a code owner April 17, 2025 14:30
@github-actions github-actions bot added the area-ai Microsoft.Extensions.AI libraries label Apr 17, 2025
Copy link
Member

@jeffhandley jeffhandley left a comment

Choose a reason for hiding this comment

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

I'm in favor of supporting this.

Do I understand correctly though that the potential drawback is that we take the liberty of excluding parameters that will be resolved from the service provider from the schema exchanged with the model, therefore preventing the model from being able to include a value for the parameter itself, and there might be scenarios where that would be unexpected/undesired?

After typing that, I asked Copilot what the potential drawbacks were, and it included that reasoning and also included other drawbacks, pasted below.

I'm still in favor myself though, and the implementation plugs in very surgically, so I'm comfortable with the risk at this stage too, but this could also be serviced in later as a behavioral breaking change that I think would be acceptable.

I ultimately defer to you two though, @stephentoub and @SteveSandersonMS.

The potential drawback in integrating the feature outlined in this pull request includes:

  1. Increased Complexity:

    • The addition of IServiceProvider-specific logic introduces more complexity to the AIFunctionFactory and its associated classes. This could make the code harder to understand, maintain, and debug, especially for developers unfamiliar with the dependency injection flow.
  2. Dependency on IServiceProvider:

    • Tight coupling with IServiceProvider and IServiceProviderIsService may make the feature less flexible. This dependency assumes that the IServiceProvider provided at construction matches the one used during invocation, which could lead to issues if mismatched.
  3. Behavioral Changes:

    • Parameters that can be resolved via IServiceProvider are excluded from the JSON schema. This change in behavior may surprise users who expect all parameters to be explicitly listed in the schema. It may also create discrepancies between the schema and runtime behavior.
  4. Error Handling Overhead:

    • The feature introduces additional error handling for cases where IServiceProvider is missing or fails to resolve a parameter. This might lead to more runtime exceptions, adding to the risk of failure if users do not carefully configure the Services.
  5. Testing Complexity:

    • Unit testing becomes more challenging because the behavior of the functions now depends on external services and their availability in the provided IServiceProvider. Mocking or setting up these services for tests could increase the burden on test authors.
  6. Potential for Misconfiguration:

    • If the Services provided during construction and invocation are not aligned or do not satisfy the required dependencies, the feature could fail at runtime. This may make it harder for developers to debug and resolve configuration issues.
  7. Backward Compatibility:

    • Existing users of the library who rely on the explicit schema for all parameters or who do not use IServiceProvider may need to adjust their code to accommodate the new behavior.
  8. Performance Overhead:

    • Resolving parameters dynamically from IServiceProvider may introduce slight performance overhead compared to binding all parameters from the arguments dictionary.

These drawbacks should be weighed against the benefits of making the library more dependency-injection-friendly and flexible for users relying on service resolution.

@stephentoub stephentoub merged commit efcd303 into dotnet:main Apr 18, 2025
7 checks passed
@stephentoub stephentoub deleted the aifuncfacsp branch April 18, 2025 20:55
jeffhandley pushed a commit that referenced this pull request Apr 30, 2025
Add AIFunctionFactoryOptions.Services, and use it when examining function parameters to determine whether they should be resolved by default from DI.
jeffhandley pushed a commit that referenced this pull request Apr 30, 2025
Add AIFunctionFactoryOptions.Services, and use it when examining function parameters to determine whether they should be resolved by default from DI.
jeffhandley added a commit that referenced this pull request Apr 30, 2025
* Merged PR 49569: Cherry pick Evaluation changes for 9.4.3 release

* Update package-lock.json for Evaluations

* Reset package-lock.json to same as main branch

* Merged PR 49585: [9.4.3] [cherry pick] A couple of minor fixes

A couple of minor fixes

1. Adds back tool tip for metrics which are useful in cases where the metric name is long and does not fit entirely in the card
2. Makes the size of the selection buttons on the left of cards consistent with the size of status icons on the right to make the alignment cleaner
3. Implement IEquatable for UrlCacheKey (per Copilot's suggestion)
4. Throw ArgumentException when the number of messages passed to ContentSafetyChatClient does not match expectations (also per Copilot's suggestion)

* Add test for optional parameters being required with RequireAllProperties (#6265)

* Add test for optional parameters being required with RequireAllProperties

* Test both requireAllProperties values

* Use AssertDeepEquals that logs the difference to other tests

* Adding reference to an unsupported built-in tool on OpenAI Chat API no longer throws (#6276)

* Update M.E.AI changelogs (#6269)

* Augment UseDistributedCache XML docs (#6256)

* Augment AIFunctionFactory.Create XML docs (#6255)

* Rename EmbeddingGeneratorExtensions.GenerateEmbedding extension methods (#6295)

To align with the the base method on IEmbeddingGenerator

* Augment FunctionInvokingChatClient's span with token counts (#6296)

* Rename ChatThreadId to ConversationId (#6300)

* In OpenAI responses client, use response ID as ChatThreadId

* Rename ChatThreadId -> ConversationId

* Related renames

* Restore deleted members as obsolete (#6304)

* Restore EmbeddingGeneratorExtensions members as obsolete

* Restore ChatThreadId as obsolete

* Restore ChatResponse.ChatThreadId and ChatResponseUpdate.ChatThreadId as obsolete.

* Remove the tests for obsolete members

* Remove the Embeddings tests for obsolete members

* Support [FromKeyedServices] in AIFunctionFactory (#6310)

* Utilize IServiceProviderIsService in AIFunctionFactory (#6317)

Add AIFunctionFactoryOptions.Services, and use it when examining function parameters to determine whether they should be resolved by default from DI.

* Remove AsChatClient/AsEmbeddingGenerator that were obsoleted in 9.4.0-preview.1.25207.5 (#6327)

* Add ChatOptions.AllowMultipleToolCalls (#6326)

* Add ChatOptions.AllowMultipleToolCalls

* Use it in OpenAI adapter

* Disable default required property schema generation and OpenAI strict mode. (#6285)

* Disable default required property schema generation and OpenAI strict mode.

* Default strictness to OpenAI client defaults.

* Fix a few failing tests.

* Undo a number of additional changes made by #6064

* Revert change to structured output defaults.

* Incorporate more test fixes.

* Address feedback.

* Tweak strict mode signature, fix failing tests.

* Roll back schemaIsStrict flag from ChatResponseFormat.

* Merged PR 49624: [9.4.3] [cherry-pick] Update readmes (#6345)

Includes name of new metric that was added to Quality package.

Also updates some doc comments.

* Bump version to 9.4.3

* Merged PR 49636: [9.4.3] [cherry-pick] Skip messages that have no text when rendering conversations as part of evaluation prompts (#6349)

* Add test for optional parameters being required with RequireAllProperties (#6265)

* Add test for optional parameters being required with RequireAllProperties

* Test both requireAllProperties values

* Use AssertDeepEquals that logs the difference to other tests

* Adding reference to an unsupported built-in tool on OpenAI Chat API no longer throws (#6276)

* Update M.E.AI changelogs (#6269)

* Augment UseDistributedCache XML docs (#6256)

* Augment AIFunctionFactory.Create XML docs (#6255)

* Rename EmbeddingGeneratorExtensions.GenerateEmbedding extension methods (#6295)

To align with the the base method on IEmbeddingGenerator

* Augment FunctionInvokingChatClient's span with token counts (#6296)

* Rename ChatThreadId to ConversationId (#6300)

* In OpenAI responses client, use response ID as ChatThreadId

* Rename ChatThreadId -> ConversationId

* Related renames

* Restore deleted members as obsolete (#6304)

* Restore EmbeddingGeneratorExtensions members as obsolete

* Restore ChatThreadId as obsolete

* Restore ChatResponse.ChatThreadId and ChatResponseUpdate.ChatThreadId as obsolete.

* Remove the tests for obsolete members

* Remove the Embeddings tests for obsolete members

* Support [FromKeyedServices] in AIFunctionFactory (#6310)

* Utilize IServiceProviderIsService in AIFunctionFactory (#6317)

Add AIFunctionFactoryOptions.Services, and use it when examining function parameters to determine whether they should be resolved by default from DI.

* Remove AsChatClient/AsEmbeddingGenerator that were obsoleted in 9.4.0-preview.1.25207.5 (#6327)

* Add ChatOptions.AllowMultipleToolCalls (#6326)

* Add ChatOptions.AllowMultipleToolCalls

* Use it in OpenAI adapter

* Disable default required property schema generation and OpenAI strict mode. (#6285)

* Disable default required property schema generation and OpenAI strict mode.

* Default strictness to OpenAI client defaults.

* Fix a few failing tests.

* Undo a number of additional changes made by #6064

* Revert change to structured output defaults.

* Incorporate more test fixes.

* Address feedback.

* Tweak strict mode signature, fix failing tests.

* Roll back schemaIsStrict flag from ChatResponseFormat.

* Bump version to 9.4.3

* Merged PR 49636: [9.4.3] [cherry-pick] Skip messages that have no text when rendering conversations as part of evaluation prompts (#6349)

* Update chat template dependencies, fix OpenAI/Aspire config, and address build warnings (#6280)

* Update chat template dependencies

* Update test snapshots

* Update Aspire version

* Revert Aspire + update CommunityToolkit.Aspire

* Open README in VS after project creation

* Add Known Issue to Aspire README for Qdrant

* Update survey template URL

* Set the Project Template package version to -preview.2

* Update template baseline

* Update template pinned versions

* Do not append template args to snapshot names

* Fix vector store index in the README. Add an AzureAISearch template test.

* Add a note to the Aspire README for trusting the development certificate

* Use AddOpenAIClient for OpenAI and AddAzureOpenAIClient for Azure OpenAI

* Remove duplicated using System.ClientModel

* Update Aspire README to specify the exception thrown for the known issue

* Augment the Aspire README for more Docker notes for Ollama and Qdrant

* Fix Microsoft.Extensions.Http.Resilience warnings with separate pinned versions

* Update test baseline for: Rename EmbeddingGeneratorExtensions.GenerateEmbedding extension methods (#6295)

* Add --managed-identity to the template developer README

* Apply the Ollama timeout recommendation in the template code

* Call http.RemoveAllResilienceHandlers before adding the handler

* Update template test baseline

---------

Co-authored-by: Jeff Handley <[email protected]>
Co-authored-by: David Cantu <[email protected]>

* Expose AIContent constructor (#6346)

* Add PDF support to OpenAI AsIChatClient (#6344)

* Add PDF support to OpenAI AsIChatClient

* Add missing reference

* Make CreateJsonSchema tolerate JSO inputs that don't have a resolver set. (#6348)

* Update MEAI.Templates to use the just-built version of the libraries

* Enhance Function Invocation Extensibility for Microsoft.Extensions.AI (#6325)

---------

Co-authored-by: Peter Waldschmidt <[email protected]>
Co-authored-by: Peter Waldschmidt <[email protected]>
Co-authored-by: Shyam Namboodiripad <[email protected]>
Co-authored-by: David Cantú <[email protected]>
Co-authored-by: Art Leonard <[email protected]>
Co-authored-by: Stephen Toub <[email protected]>
Co-authored-by: Shay Rojansky <[email protected]>
Co-authored-by: Steve Sanderson <[email protected]>
Co-authored-by: Eirik Tsarpalis <[email protected]>
Co-authored-by: Mackinnon Buck <[email protected]>
Co-authored-by: Roger Barreto <[email protected]>
@github-actions github-actions bot locked and limited conversation to collaborators May 19, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-ai Microsoft.Extensions.AI libraries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extend DI support for AIFunctionFactory
2 participants