.NET: Single Agent API review

[SergeyMenshykh]

• 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. -- CreateSession and DeserializeSession may benefit from this, so we should also do it for those. .NET: Introduce Core implementation methods for session methods on AIAgent #3699
  • Agent thread related
    • Consider renaming GetNewThread methods. Naming alternatives: GetnewThreadObject, GetnewThreadInstance, CreateAgentContext, CreateAgentState, CreateAgentSession
    • Consider renaming AgentThread. Naming alternatives: AgentContext, AgentState, AgentSession, AgentRunContext, AgentRunsContext
    • Should AIAgent.GetNewThread & Agent.DeserializeThread 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. .NET: Consider making ChatMessageStore and AIContextProvider Factories plus GetNewThread and DesriailzeThread async #2520
    • DeserializeThread param name serializedThread alternatives: threadElement, stateElement, etc. Rename the AIContextProviderFactoryContext.SerializedState and ChatMessageStoreFactoryContext.SerializedState properties as well if needed. Search codebase for serializedState params and rename them if necessary. -- Discussed and will go with serializedState for all similar parameters.
    • [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.
  • ChatMessageStore
    • Consider a way to identify the source of AIContextProviderMessages passed to ChatMessageStore.InvokedAsync. Maybe via AuthorName. .NET: [BREAKING] Add ability to mark the source of Agent request messages and use that for filtering #3540
    • Consider whether ChatMessageStore and AIContextProvider can be modelled as a pipeline where each calls the next in the pipeline. This could help with context window sizing, as entries further down the pipeline have a view of the entire input and can choose to add less or truncate existing message lists.
    • Reconsider Invoking / Invoked naming. They are in the context of the caller rather than the ChatMessageStore. Should these be on an interface instead? Renamed to ChatHistoryProvider, which make Invoking/Invoked make more sense.
    • Reconsider ChatMessageStore naming, since it is not a full ChatMessageStore abstraction but more like a cursor. It provides a way to append messages and retrieve a view into the that history.
    • Also have InMemoryChatMessageStore implement IReadOnlyList .NET: Implement IReadOnlyList on InMemoryChatMessageStore #3205
  • AIAgentMetadata & AgentThreadMetadata
    • Should these be sealed? .NET: Expose metadata from A2AAgent and seal AIAgentMetadata #3417
    • We are not using these everywhere we should, see A2AAgent for AgentMetadata, and most threads for thread metadata. .NET: Expose metadata from A2AAgent and seal AIAgentMetadata #3417
    • 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 if there is an ask for it.
    • 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?
      Features is for data/behavior input, while metadata needs to be retrieved. Features cannot be used to retrieve metadata. GetService already satisfies the requiresments.
    • AgentThreadMetadata is not used anywhere, can it be dropped? [BREAKING] Remove unused AgentThreadMetadata #3067
  • ChatHistoryMemoryProvider
    • Consider making ChatHistoryMemoryProvider functionality part of a ChatMessageStore, since it'll make it easier for users to use it correctly.
  • AgentRunResponse & AgentRunResponseUpdate
    • Should AgentId be required? It's required in AIAgent class. --- Yes, no reason for it not to be required, let's explore changing this to see if there are any hiccups.
    • Should ResponseId be required?
    • Should CreatedAt be required?
      • ChatResponse.CreatedAt is also optional. We should check if there was a good reason for that.
    • JSO should be optional in Deserialize/TryDeserialize. .NET: Seal factory contexts and add non JSO deserialize overloads #3066
    • UserInputRequests properties might be misleading because they do not take into account FunctionCallContent that might come out of the agent unhandled. -- Decided to remove the property for now, we can add it again later if there is requests for it, and requirements are clearer. .NET: [BREAKING] Remove UserInputRequests property #3682
    • 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?
      Without them here, consumers who build their own agents on top of InMemoryAgentThread would need to depend on core. These are also lightweight with no dependencies, so keeping in abstractions.
    • 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.
      Having them as abstract base classes allows us to keep the constructors protected/internal, helping to enforce the pattern that users create these via the agent. If we make these non-abstract for direct usage, we'd have to make their constructors public.
    • Should we remove InMemoryAgentThread.MessageStore property to be aligned with the way AIAgent provides access to AgentMetada via the GetService?
      These are not mutually exclusive. If someone already has a typed reference to the thread, they should be able to access properties on it normally. This doesn't mean that if users have the abstract AgentThread they shouldn't be able to ask for the MessageStore via GetService.
    • Consider renaming ServiceIdAgentThread to something that includes the word "hosted" to align with M.E.AI naming convention. -- MEAI actually refers to this as ConversationId in ChatOptions, so I don't think hosted makes sense here.
  • AIContextProviders
    • Consider modeling AIContextProviders as middleware.
      • AIContextProviders are currently only supported with ChatClientAgent, since they may make use of function calling or instructions, but middleware is built over AIAgent, which does not support this.
    • Can we create pipelenes of these providers?
      • This is currently supported by nesting 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
    • Prototype adding a AIAgent decorator that calls an AIContextProvider
  • 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 removing it until we have a request for it.
      We need these to support scenarios like resuming a long running task, where subsequent requests do not take new input.
    • 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...
  • Consider sealing AIContextProviderFactoryContext and AIContextProviderFactoryContext .NET: Seal factory contexts and add non JSO deserialize overloads #3066

• Microsoft.Agents.AI

  • Either seal ChatClientAgentOptions or add copy constructor: .NET: Seal options and context classes #2633
  • Double check if TextSearchProvider and TextSearchProviderOptions classes should be in the Microsoft.Agents.AI.Data namespace. .NET: [Breaking] Move TextSearchProvider and TextSearchProviderOptions to Microsoft.Agents.AI namespace #2639
  • ake the TextSearchProviderOptions properties init-only because they are read in the TextSearchProvider constructor, 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. - ID is autogenerated whne not provided, so it has a default already. Supplying one is an edge case, and less used parameters are supplied via ChatClientAgentOptions, so not adding it here.

• Microsoft.Agents.AI.OpenAI .NET: [BREAKING] Change namespaces of the Microsoft.Agents.AI.OpenAI classes  #2627

  • Should it be in the Microsoft.Agents.AI namespace instead of OpenAI one?
  • Should AIAgentWithOpenAIExtensions extensions be in the Microsoft.Agents.AI namespace instead of OpenAI one?
  • Should OpenAIAssistantClientExtensions extensions be in the OpenAI.Assistants namespace instead of OpenAI one?
  • Should OpenAIChatClientAgent class be in the Microsoft.Agents.AI.OpenAI namespace instead of OpenAI one?
  • Should OpenAIResponseClientAgent extensions be in the Microsoft.Agents.AI.OpenAI namespace instead of OpenAI one?
  • Should OpenAIChatClientAgent and OpenAIResponseClientAgent be 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.
  • Remove the sync overloads for CreateAIAgent and GetAIAgent .NET: Remove sync versions of CreateAgent and GetAgent #3289
  • Mark the AIAgentWithOpenAIExtensions as experimental

• Microsoft.Agents.AI.AzureAI

  • Remove the sync overloads for CreateAIAgent and GetAIAgent .NET: Remove sync versions of CreateAgent and GetAgent #3289

• 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). -- Done in sample Agent_Step13_BackgroundResponsesWithToolsAndPersistence
  • 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
  • Consider adding AdditionalProperties to AIAgent class. The ask and the PR.