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

Some project structure suggestions #100

Open
dorthl opened this issue Aug 14, 2023 · 7 comments
Open

Some project structure suggestions #100

dorthl opened this issue Aug 14, 2023 · 7 comments
Assignees

Comments

@dorthl
Copy link

dorthl commented Aug 14, 2023

This project seems to be overly dependent reflecting the low reusability of the BotSharp.Core library. The key point is that it relies on the EntityFrameworkCore.BootKit package. BotSharp.Core seems to have assumed that the library can only run on the asp.net host and EntityFrameworkCore cannot be executed. persistent migration

some suggested changes

BotSharp.Abstraction => BotSharp Implement the abstract logic and interface of the user agent session
BotSharp.Core split into => BotSharp.Storage BotSharp.Web. Among them, BotSharp.Storage implements EntityFrameworkCore storage logic, generates data migration, and BotSharp.Web provides user agent session and other related interfaces. There is no dependency between these two projects. The user data provider interface is defined in the BotSharp project and then used in the web and then defined by Storage
WebStarter => BotSharp.Example Dependency injection should be used in this project to instantiate the data provider interface of user agent session, where plug-in loading and so on can use IServiceCollection loading parameters to set whether the configuration is enabled or not.

Reflection seems to be overused in this project

Oceania2018 added a commit that referenced this issue Aug 14, 2023
@Oceania2018 Oceania2018 self-assigned this Aug 14, 2023
@Oceania2018
Copy link
Member

Thanks for taking deep into this project. Here is some explanation.

  1. BotSharp is mainly a framework function, with built-in integrated minimal function implementation, such as Agent management, Conversation and dialog state management, and plug-in management. Richer features are implemented through plug-in, and this design can maximize the use of BotSharp.Core.
  2. In the latest code submission, the database dependency has been removed, and all storage and query are managed in the form of pure files by default. In the form of a file, it is most convenient to use any editor to modify the prompt or sample file.
  3. About BotSharp.Core split into => BotSharp.Storage BotSharp.Web it's a good idea. Could be done in the future.
  4. Not sure about this item Reflection seems to be overused in this project. Can you give more specific example?

@dorthl
Copy link
Author

dorthl commented Aug 15, 2023

For Reflection seems to be overused in this project. mainly because the way IBotSharpPlugin loads plug-ins has a feeling of repeated nesting

In the project WebStarter, you have used the Microsoft.AspNetCore.Authentication.JwtBearer package. You can refer to this plug-in loading mode. jwtbearer is a plug-in for Microsoft.AspNetCore.Authentication AddAuthentication builds AuthenticationBuilder and then extends AuthenticationBuilder in jwtbearer to add AddJwtBearer method to resolve dependencies

In this project, the plugin is loaded by BotSharp.Core by calling PluginLoader.Load in the AddBotSharp method by scanning the assembly and comparing it with the PluginLoader item defined in appsettings.json.
Perhaps BotSharp.Core does not need to do these things. BotSharp.Core only needs to define a method to build a BotSharpBuilder plug-in and then extend BotSharpBuilder. Add AddKnowledgeBasePlugin, AddMemVecDbPlugin and other methods.

The root of the problem is that I want BotSharp.Core to be a class library that does not need to implement any storage logic. It can define an interface for me to implement it myself, or provide a default implementation through other class libraries. I need a class library for I can get IUserService IAgentService and other services in the dependency injector through AddBotSharp, and I can also inject the ai I need in the host through AddLLamaSharpPlugin, AddAzureOpenAiPlugin... and so on.

Similar api interfaces are provided by the BotSharp.Web project and data storage is provided by BotSharp.Storage so that you can publish nuget packages such as BotSharp, BotSharp.Web, BotSharp.Storage, BotSharp.LLamaSharpPlugin, and users can reference them in their own hosts The required package is used after resolving DI in the host

@dorthl
Copy link
Author

dorthl commented Aug 15, 2023

In this way, the assembly is loaded through reflection. It seems that you want the plug-in to be loaded through dynamic loading. But BotSharp.Core becomes an asp.net core host that depends on reflection, and WebStarter just puts a shell on it BotSharp.Core is not a library

@Oceania2018
Copy link
Member

You mentioned some important points. At the beginning, my original intention was to let users install the least number of packages while taking into account the scalability of the architecture, so some important functions were built in. Indeed, you are right, I also plan to move this function to BotSharp.Plugin.xxxx to make Core cleaner.

@dorthl
Copy link
Author

dorthl commented Aug 15, 2023

I hope this library can get better and better. I am using it to understand LLamaSharp integration, but I don’t know much about LLamaSharp. Call {{host}}/conversation/{{agentId}} The model has been iterating out of garbled characters and the chat can no longer be suitable Opportunity expires

I have studied the code of this project in detail, and I am good at asp.net core direction. I want to expand it from the web direction and add a chat interface to it to form a chatgpt-like dialogue form, which is implemented by LLamaSharp. Perhaps LLamaSharp can be implemented by using SignalR to deploy a distributed service. The server receives a request and sends it to the connected LLamaSharp implementation to obtain Answer and forward to the caller, of course this is some of my thoughts

If you want to modify the project structure, it seems that this project needs to be refactored. I see a large number of projects using the _services.GetRequiredService() interface to get services. This way of using dependency injection is very wrong, which is explained in detail in the Microsoft documentation.

Mainly in terms of the scalability of the project, the coupling degree of this project is too high. Although many services have defined interfaces, there is no place to rewrite it and use it as a library. This is a problem

@dorthl
Copy link
Author

dorthl commented Aug 15, 2023

Use dependency-injection for inversion of control to decouple projects

@Oceania2018
Copy link
Member

@dorthl BotSharp.OpenAPI is seperated out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants