.Net: Revisit namespaces and type placement

[stephentoub]

Important

Labeled Urgent because it may require a breaking change.

(Metapoint: Try rewriting all the examples without using var and seeing how many namespaces need to be imported to successfully compile.)

• (1)
  To do anything meaningful with memory now I need both:

using Microsoft.SemanticKernel.Memory; // for ISemanticTextMemory, unless I rely exclusively on var
using Microsoft.SemanticKernel.Plugins.Memory; // for MemoryBuilder and friends

plus a using for whatever store I'm using it if it's not the VolatileMemoryStore, e.g.

using Microsoft.SemanticKernel.Connectors.Memory.AzureCognitiveSearch;

That's three usings just to create an ISemanticTextMemory. Can these be consolidated?

• (2)
  To do anything meaningful with planning, I now need both:

using Microsoft.SemanticKernel.Planners; // for the actual planner
using Microsoft.SemanticKernel.Planning; // for Plan

The difference between "Planners" and "Planning" is also very subtle. Can these be consolidated?

• (3)
  It looks like Microsoft.SemanticKernel.Diagnostics exists now just for SKException, HttpOperationException, and Telemetry. Do we need this namespace at all? Can we move these types somewhere else, like moving SKException to the root Microsoft.SemanticKernel namespace?

• (4)
  Similarly, can we consolidate Microsoft.SemanticKernel.Events with something else? "Events" is pretty broad; the types here are specific to pre/post hooks with plan execution, right?

• (5)
  Why do we need Microsoft.SemanticKernel.Plugins.Core? Can't those just move down into Plugins?

• (6)
  We should also move "extensions" types to the namespace with the types they extend. For example, it looks like ImageGenerationServiceExtensions, TextEmbeddingServiceExtensions, TextCompletionServiceExtensions, only have extension methods and those only apply to IAIService in Microsoft.SemanticKernel.Services... can they move there? And can they be consolidated into an AIServicesExtensions class? And OpenAIModelResultExtensions has methods that operate only on Microsoft.SemanticKernel.Orchestration.ModelResult... can it move there?

• (7)
  What is MemoryConfiguration and why does it live in the root namespace?

• (8)
  The main thing I do with an IKernel (which lives in the root namespace) is InvokeAsync on it, and to get a useful result from it, I need to access its KernelResult, which now lives in Microsoft.SemanticKernel.Orchestration. I suggest at least KernelResult and FunctionResult move to be next to IKernel and ISKFunction in the root namespace. And at that point, the Orchestration namespace contains very little and should likely be consolidated with something else.

• (9)
  Microsoft.SemanticKernel.TemplateEngine contains just a single type (IPromptTemplateEngine) and then a sub Basic namespace. Can everything in Basic just move up?

• (10)
  Microsoft.SemanticKernel.Reliability appears to solely be about HTTP and retries. Why is this separate from Microsoft.SemanticKernel.Http? Can we consolidate them?

• (11)
  .Net: PromptTemplateConfig and IPromptTemplate namesapce refactor #3173

[dmytrostruk]

@stephentoub Thanks for very useful feedback!

(1) Microsoft.SemanticKernel.Memory and Microsoft.SemanticKernel.Plugins.Memory - this is just transient phase, while we marked Memory related methods and properties in Kernel as Obsolete. In next versions, all memory-related functionality will be placed in Plugins.Memory package and use one root namespace Microsoft.SemanticKernel.Plugins.Memory.

Regarding Memory Stores (Microsoft.SemanticKernel.Connectors.Memory.AzureCognitiveSearch) - they are placed in their own packages, so namespace is based on Microsoft.SemanticKernel + folder path of concrete Memory store. We either have to place all of them under one package or use the same namespace for all of them (does it respect namespace naming conventions?). Which way is better here?

(2-5) I think these can be fixed.

(6)

We should also move "extensions" types to the namespace with the types they extend.

In case of OpenAIModelResultExtensions - it contains extension methods for Microsoft.SemanticKernel.Orchestration.ModelResult, but the logic of these methods is applicable only to OpenAI-related functionality. In this case shouldn't it be placed under Microsoft.SemanticKernel.Connectors.AI.OpenAI namespace? (to align namespace with actual class location).

(7) MemoryConfiguration will be removed in future versions when we remove Memory-related functionality from Kernel. I marked its methods as Obsolete, but I should probably also mark as Obsolete the class itself.

(8-11) I think these can be fixed.

[matthewbolanos]

Will need some additional feedback from Stephen so we can split this into multiple issues and provide a better estimate.

[stephentoub]

Regarding Memory Stores (Microsoft.SemanticKernel.Connectors.Memory.AzureCognitiveSearch) - they are placed in their own packages, so namespace is based on Microsoft.SemanticKernel + folder path of concrete Memory store. We either have to place all of them under one package or use the same namespace for all of them (does it respect namespace naming conventions?). Which way is better here?

I certainly wouldn't say they all need to be in the same package; that's both limiting and infeasible for something that's meant to have external plug-ins all playing in the same design. If we believe conflicts in naming of types between plug-ins is going to happen, then they should be in their own namespaces, though my expectation was that it would be rare to have such conflicts and we'd actively encourage folks to use a prefix on their type names, e.g. it'd be Microsoft.SemanticKernel.Connectors.Memory.AzureCognitiveSearchMemoryStore rather than Microsoft.SemanticKernel.Connectors.Memory.AzureCognitiveSearch.MemoryStore, at which point there shouldn't be conflicts.

That said, my comment was more concerned about consolidating from 3 down to 2 rather than from 3 down to 1, and from your comments it sounds like that's already in the works, which is good.

In case of OpenAIModelResultExtensions - it contains extension methods for Microsoft.SemanticKernel.Orchestration.ModelResult, but the logic of these methods is applicable only to OpenAI-related functionality. In this case shouldn't it be placed under Microsoft.SemanticKernel.Connectors.AI.OpenAI namespace? (to align namespace with actual class location).

My main objection was having them in the root namespace as they currently are. If we want to place them in Microsoft.SemanticKernel.Connectors.AI.OpenAI, that's ok, but that will also mean that if you haven't otherwise imported that namespace, you may not see them in IntelliSense in VS while exploring and seeing what you can add. I'd need to double-check, but I think ASP.NET typically includes its methods for adding into IServiceCollection in the same namespace that defines IServiceCollection, for example: that way, when you add the relevant package, and you already have the namespace that brings in IServiceCollection, you also have the namespace that brings in the extensions on it.

On that note, it's worth noting that ASP.NET generally also calls such methods "Add" not "With". SK should consider aligning with that as well, simply for familiarity for devs used to using ASP.NET.

[dmytrostruk]

My main objection was having them in the root namespace as they currently are.

Yes, this will definitely be fixed. My main concern was about putting it to the namespace where original type is located (Microsoft.SemanticKernel.Orchestration.ModelResult) instead of Microsoft.SemanticKernel.Connectors.AI.OpenAI, where it's more applicable based on extension method logic and the fact that it lives in Connectors.AI.OpenAI package.

I think ASP.NET typically includes its methods for adding into IServiceCollection in the same namespace that defines IServiceCollection, for example: that way, when you add the relevant package, and you already have the namespace that brings in IServiceCollection, you also have the namespace that brings in the extensions on it

I think it's a little bit different case, and here it makes sense, because usually you work with IServiceCollection to register services from different libraries, and if each IServiceCollection extension method will live in its library namespace, your Startup.cs file will be massively overloaded with different namespaces. So, in this pattern it makes sense to keep namespace of original type we extend, and we can actually apply the same strategy to our KernelBuilder and all its extensions.

But if we extend some base type (like ModelResult) with additional functionality, which is specific to concrete integration (e.g. OpenAI), I'm not sure if having the namespace of original type will provide some benefits. It could be even confusing, that this method is available by default, although I didn't include any OpenAI specific namespaces in my file.

On that note, it's worth noting that ASP.NET generally also calls such methods "Add" not "With". SK should consider aligning with that as well, simply for familiarity for devs used to using ASP.NET.

I agree. I think we had discussions about that previously. Although, this change should definitely include Obsolete, and for some period of time we will have both .With and .Add methods in place for smooth migration.

[dmytrostruk]

that will also mean that if you haven't otherwise imported that namespace, you may not see them in IntelliSense in VS while exploring and seeing what you can add

Regarding this, looks like IntelliSense will show you which extension methods from different namespaces are available:

[stephentoub]

that will also mean that if you haven't otherwise imported that namespace, you may not see them in IntelliSense in VS while exploring and seeing what you can add

Regarding this, looks like IntelliSense will show you which extension methods from different namespaces are available

If you have this enabled:

Note, though, that the option also has some issues: note that some of those extensions aren't actually applicable to the thing you're dot'ing off of, which it will realize the moment you select the extension and it imports the namespace, at which point you'll get an error because the extension isn't actually valid. So some folks turn this off.

I'm also not sure of the experience here in VS Code.

[dmytrostruk]

Yeah, that's interesting issue :)
I'm fine with both options, since they are better than keeping extension methods for ModelResult type in root namespace (Microsoft.SemanticKernel).

[SergeyMenshykh]

On that note, it's worth noting that ASP.NET generally also calls such methods "Add" not "With". SK should consider aligning with that as well, simply for familiarity for devs used to using ASP.NET.

The "Add" and "With" method names make perfect sense in context of classes they belong to and not necessary the other way around.

The "Add" extension methods, just like the "Remove", "Contains", etc.. , make sense in the context of the "ServiceCollection" class because the class is a collection. Therefore, an ASP.NET developer registers instances of classes by adding (calling the "Add" method) them to the service collection.

The "KernelBuilder" class is a builder, not a collection. Adding the "Add" methods to it would imply that it has collection capabilities, allowing it to register or have many instances of the same SK component types. This is the case for SK connectors but might not be the case for the other dependencies/components such as the logger factory, retry policy, and prompt template engine.

[stephentoub]

The "KernelBuilder" class is a builder, not a collection. Adding the "Add" methods to it would imply that it has collection capabilities, allowing it to register or have many instances of the same SK component types. This is the case for SK connectors but might not be the case for the other dependencies/components such as the logger factory, retry policy, and prompt template engine.

I think this just adds to the idea that the Kernel should not itself be a DI container. We should either:

a) Have Kernel take a dependency on Microsoft.Extensions.DependencyInjection and have it simply wrap a ServiceCollection. Developers can populate a ServiceCollection however they like, including using AddXx extension we provide on ServiceCollection, which then let's them work both with or without Kernel, e.g. a developer can use AddAzureChatCompletion in their ASP.NET application to register the IChatCompletion they want to use throughout the app, and they don't need to think or know anything about Kernel.

Or

b) Remove the concept of services from Kernel entirely. Anything in SK that needs an IWhatever is just passed the IWhatever.