-
Notifications
You must be signed in to change notification settings - Fork 891
Open
Labels
Description
-
Microsoft.Agents.AI.Abstractions
- Look into how we handle AIAgent.Id for Workflow agents, e.g. what does it expose. Typically ids should be unique.
- How does AIAgent.Id work with foundry hosted agents, e.g. when hosting an AF agent in foundry, does the id need to have a specific value in order to have the telemetry show up correctly in foundry
- See if we can remove DisplayName by reviewing its usage locations, and see if the usage location should perhaps be a combination of id and name especially for logging cases, e.g. {id}:{name}. Id is unique, which is valuable, but name is also valuable since it is human readable. PR: .NET: [Breaking] Delete display name property #2758
- Update AIAgent.Id:
- to be non-vritual and add a virtual core property. Non-virtual defaults to guid: .NET: [BREAKING] Prevent nulls in AIAgent.Id property #2719
- Also review other properties to see if they can benefit from a similar change.
- Consider renaming GetNewThread methods. Naming alternatives: GetnewThreadObject, GetnewThreadInstance, CreateAgentContext, CreateAgentState, CreateAgentSession
- Consider renaming AgentThread. Naming alternatives: AgentContext, AgentState, AgentSession, AgentRunContext, AgentRunsContext
- AIAgent.GetNewThread & Agent.DeserializeThread
- Should these be async, for the case where a user wants to generate an id using a service in the factory method for the ChatMessageStore. Would also need a cancellation token in that case.
- DeserializeThread param name serializedThread alternatives: threadElement, stateElement, etc. Rename the
AIContextProviderFactoryContext.SerializedStateandChatMessageStoreFactoryContext.SerializedStateproperties as well if needed. Search codebase forserializedStateparams and rename them if necessary. - Consider sealing AIContextProviderFactoryContext and AIContextProviderFactoryContext
- AgentThread.Serialize & ChatMessageStore.Serialize
- We should consider separating thread state from behavior. I.e. the agent thread is just a state object model, and behaviors are bound to objects in the tree. The AgentThread may contain behavior instances, but they can be recreated at any time. Problem is that we don't know the types of the ChatMessageStore state. We need to do polymorphic deserialization for ChatMessageStore and AIContextProvider. Problem with current approach is we only support json, and users cannot serialize the thread using other means. Also, users expect to be able to just use JsonSerializer.
- AIAgentMetadata & AgentThreadMetadata
- Should these be sealed?
- We are not using these everywhere we should, see A2AAgent for AgentMetadata, and most threads for thread metadata.
- Why do these not have concrete properties in the abstraction and are instead only exposed via GetService?(This is because we followed the pattern from M.E.AI, where the IChatClient interface can't be extended. As a result, everything extra is returned via GetService for consistency.) We can consider adding well known metadata props to AIAgent class.
- Should it be modeled via the Features concept or via middleware? How to extend(add new props) the metadata and allow telemetry/logging decorators access it?
- AgentThreadMetadata is not used anywhere, can it be dropped?
- AgentRunResponse & AgentRunResponseUpdate
- Should AgentId be required? It's required in AIAgent class.
- Should ResponseId be required?
- Should CreatedAt be required?
- JSO should be optional in Deserialize/TryDeserialize.
- UserInputRequests properties might be misleading because they do not take into account FunctionCallContent that might come out of the agent unhandled. That should be relatively rare, but it is still possible if FICC is configured in a certain way or if AIFunctionDeclaration is put into the tools collection, and because there's nothing to invoke, FunctionCallContent will be returned to the consumer who might not be expecting it, relying on UserInputRequests to provide the request/content to handle. @stephentoub has more context/ideas about potential solutions.
Consider change type of AllowBackgroundResponses property form bool? to bool. "Typically we use null to mean "not set, use impl's default"... but that's not desirable here, as a consumer shouldn't have to be prepared to handle background responses unless they opt in. So just having bool rather than bool? here might be warranted." .NET: [Breaking] Change AgentRunOptions.AllowBackgroundResponses from bool? to bool #2819
- Seal InvokingContext & InvokedContext: .NET: Seal options and context classes #2633
- InMemoryAgentThread & InMemoryChatMessageStore & ServiceIdAgentThread
- Do InMemoryAgentThread & InMemoryChatMessageStore belong to Microsoft.Agents.AI.Abstractions project?
- Should InMemoryAgentThread and ServiceIdAgentThread be abstract? Consumers will have to inherit from them and keep the implementations empty in cases where the base class provides everything they need. On the other hand, inheriting from them and have agent specific thread may allow agents to verify thread type to reject foreign ones.
- Should we remove InMemoryAgentThread.MessageStore property to be aligned with the way AIAgent provides access to AgentMetada via the GetService?
- Consider renaming ServiceIdAgentThread to something that includes the word "hosted" to align with M.E.AI naming convention.
- AIContextProviders
- Consider modeling AIContextProviders as middleware.
- Can we create pipelenes of these providers?
- If we can, do we need two separate methods InvokingAsync & InvokedAsync rather than one method that will handle both like we have today in middleware?
- Explore the usage of AIContextProvider in the IChatClient pipeline to provide additional context, such as instructions and tools, by implementing the IChatClient decorator that accepts an instance of AIContextProvider. Consumers will then register the decorator in the chat client pipeline between, for example, OpenAIChatClient and FunctionInvokingChatClient, to intercept and call the provider for each interaction with the AI model. cc @dmytrostruk
- RunAsync & RunStreamingAsync
- Q: What is the purpose of the overloads that accept only a thread and not messages, given that there's no API to add to the thread?
- Consider making RunAsync and RunStreamingAsync non-abstract methods that call protected (abstract?) methods RunCoreAsync and RunCoreStreamingAsync. This would allow the base AIAgent class to intercept calls and perform tasks such as input validation and inspecting the output from the derived class. Otherwise, the base class has no ability to intercept the calls because the derived classes provide the full implementation of the methods. It might not be worth the added complexity if it's just argument validation. .NET: [Breaking] Introduce RunCoreAsync/RunCoreStreamingAsync delegation pattern in AIAgent #2749
- AgentRunOptions
- Consider making the copying constructor protected and have public virtual clone method. This pattern is implemented by M.E.AI ChatOptions class
- Structural output
- Reconsider having the AgentRunResponse.Deserialize and AgentRunResponse.TryDeserialize methods and revisit the current approach to structural output in AF because it's very easy to make mistakes when using those methods. POtential alternative options are:
- Agents that support structural output can implement a certain interface (IStructuralAIAgent?). However, how should we deal with ChatClientAgents that may be configured with chat clients that do not support structural output?
- Fallback mechanism that would first try to obtain structural output from the agent, if possible. If the agent cannot provide structural output, the mechanism would then pipe the agent's output as input to a chat client responsible for structural output. The chat client would then generate the structural output.
- etc...
- Reconsider having the AgentRunResponse.Deserialize and AgentRunResponse.TryDeserialize methods and revisit the current approach to structural output in AF because it's very easy to make mistakes when using those methods. POtential alternative options are:
- Consider adding AdditionalProperties to AIAgent class. The ask and the PR.
-
Microsoft.Agents.AI
- Either seal
ChatClientAgentOptionsor add copy constructor: .NET: Seal options and context classes #2633 - Double check if
TextSearchProviderandTextSearchProviderOptionsclasses should be in theMicrosoft.Agents.AI.Datanamespace. .NET: [Breaking] Move TextSearchProvider and TextSearchProviderOptions to Microsoft.Agents.AI namespace #2639 ake theTextSearchProviderOptionsproperties init-only because they are read in theTextSearchProviderconstructor, and changing them after the provider is created will not affect its behaviour. Alternatively, the provider can be changed to read the properties dynamically.- ChatClientAgent
- Consider adding an ID to the ChatClientAgent constructor that accepts agent properties as parameters. Propagate this change to all corresponding extension methods that create ChatClientAgent instances.
- Either seal
-
Microsoft.Agents.AI.OpenAI .NET: [BREAKING] Change namespaces of the Microsoft.Agents.AI.OpenAI classes #2627
- Should it be in the
Microsoft.Agents.AInamespace instead ofOpenAIone? - Should
AIAgentWithOpenAIExtensionsextensions be in theMicrosoft.Agents.AInamespace instead ofOpenAIone? - Should
OpenAIAssistantClientExtensionsextensions be in theOpenAI.Assistantsnamespace instead ofOpenAIone? - Should
OpenAIChatClientAgentclass be in theMicrosoft.Agents.AI.OpenAInamespace instead ofOpenAIone? - Should
OpenAIResponseClientAgentextensions be in theMicrosoft.Agents.AI.OpenAInamespace instead ofOpenAIone? - Should
OpenAIChatClientAgentandOpenAIResponseClientAgentbe sealed? No, we want them to be inheritable. - Should we keep
OpenAIAssistantClientExtensions? .NET: Annotate all public methods of OpenAIAssistantClientExtensions with Obsolete attribute #2640 - Mark AF API that uses the OpenAI experimental API experimental as well. Temporarily remove the warning suppression OPENAI001 to identify the AF API that needs to be marked as experimental.
- Should it be in the
-
Microsoft.Agents.AI.AzureAI
Done -
Other:
- E2E scenario showing usage of continuation token: received request rehydrates agent state (continuation token, thread, agent look-up), runs the agent, and persists the agent state (thread & continuation token).
- Needs API review:
- The ChatMessageStore & InMemoryChatMessageStore public API surface needs to be reviewed because they were skipped during the initial review since its implementation was about to change in this PR: .NET: [BREAKING] Refactor ChatMessageStore methods to be similar to AIContextProvider and add filtering support #2604.
-
Can be done later:
- Consider Adding GetRequiredService to AIContextContextProvider and other AF classes that have GetService methods
Metadata
Metadata
Assignees
Labels
Type
Projects
Status
In Progress