microsoft / BCApps

Repository for collaboration on Microsoft Dynamics 365 Business Central applications.
https://microsoft.github.io/BCApps/
MIT License
308 stars 147 forks source link

[BC Idea]: Number Series Copilot #659

Closed DmitryKatson closed 4 days ago

DmitryKatson commented 7 months ago

BC Idea Link

https://experience.dynamics.com/ideas/idea/?ideaid=ae4b0c3a-30d6-ee11-92bd-000d3adb9365

Description

Introducing the concept of a "Number Series Copilot" for Business Central, an innovative feature designed to simplify the creation of number series through natural language commands. This idea allows users to effortlessly set up number series for specific entities like customers, vendors, or even across all entities, by simply describing their needs or specifying a format example.

Idea Highlights:

This concept envisions a more interactive and intelligent Business Central, where routine tasks are automated and simplified through AI, enhancing the user experience and operational efficiency.

Linked ideas - Document numeration series templates

Number of Votes - 47

I will provide the implementation for this BC Idea

Fixes AB#503148

Internal work item: AB#503148

ikoletic commented 7 months ago

@DmitryKatson , @JesperSchulz approving, but let's have a short discussion on this before we dive in. Want to understand if we'll be able ask for generation for master data documents posted documents (that may need year in no series and or not allow gaps) separately and apply no series in various setup pages, updating existing number series, generating new no series lines on top of your existing ones to make this an E2E story.

JesperSchulz commented 7 months ago

Setting up meeting to discuss.

DmitryKatson commented 7 months ago

@ikoletic great questions, basically defining a possible set of questions that LLM should cover in the AI implementation is the right way to start

JesperSchulz commented 7 months ago

PG Review conducted today. Discussed the need to follow Microsoft's responsible AI practices and agreed upon feature set. @DmitryKatson, you're good to go!

As agreed, I tried to run .\build\scripts\DevEnv\NewDevEnv.ps1 -containerName 'BC-BusinessFoundation' -userName admin -password 'somePW' -projectPaths '.\src\BusinessFoundation\*'

Unfortunately it deletes the Business Foundation and System Application in the end, which leaves an environment without any apps, but @mazhelez has agreed to look into this right away and you should soon be good to take your first steps into the world of code contributions πŸ₯³

Again, if you have any questions, don't ever hesitate to reach out to me! We'll be in touch 😊

mazhelez commented 7 months ago

@JesperSchulz @DmitryKatson, earlier I tried running:

.\build\scripts\DevEnv\NewDevEnv.ps1 -userName admin -password '<pass>' -projectPaths '.\src\BusinessFoundation\*'

And it worked as expected.

Unfortunately, after some recent changes, it is painfully slow, it took a bit more than 30 minutes, but once set up, the container is ready with all the apps installed.

If something fails on your side, can you check if apps are created in an .artifactsCache folder? Or check the logs, maybe something failed silently.

Let me know if there is more I can help out with!

DmitryKatson commented 7 months ago

Great. I will write down here steps I did to proceed with the development (so to make a full blog about the process at the end).

  1. Submit the idea at https://experience.dynamics.com/ideas/
  2. Get enough votes
  3. Submit [BC Idea] issue at https://github.com/microsoft/BCApps/issues , link to the idea and assign to myself
  4. Wait "Approved" status
  5. Make fork from https://github.com/microsoft/BCApps
  6. Clone locally and create feature branch
  7. Locally: run Powershell (as Administrador) on the project folder path
  8. Run command .\build\scripts\DevEnv\NewDevEnv.ps1 -userName admin -password '<pass>' -projectPaths '.\src\Business Foundation\*'
DmitryKatson commented 7 months ago

@mazhelez I have weird error

image

Seems that container was created, but i can't open it. This is the full log full log.rtf.zip

mazhelez commented 7 months ago

Which BCContainerHelper version are you using?

You'd need the latest one.

DmitryKatson commented 7 months ago

Seems the fix for this error is this commit and it's been deployed to the version 6.0.7-preview1150 of BCContainerHelper.

To get this i had to run Install-Module -Name BcContainerHelper -AllowPrerelease -Force in Powershell (as Administrator).

DmitryKatson commented 7 months ago

@JesperSchulz how should I create app.json file for "Number Series Copilot" ? Should it be something like this? What Id range to use?

{
    "id":  "7efce506-def2-4195-9d1e-bc538e729242",
    "name":  "No. Series Copilot",
    "publisher":  "Microsoft",
    "brief":  "",
    "description":  "",
    "version":  "25.0.0.0",
    "privacyStatement":  "https://go.microsoft.com/fwlink/?linkid=724009",
    "EULA":  "https://go.microsoft.com/fwlink/?linkid=2009120",
    "help":  "https://go.microsoft.com/fwlink/?linkid=2103698",
    "url":  "https://go.microsoft.com/fwlink/?linkid=724011",
    "logo":  "",
    "dependencies":  [
                         {
                             "id":  "63ca2fa4-4f03-4f2b-a480-172fef340d3f",
                             "name":  "System Application",
                             "publisher":  "Microsoft",
                             "version":  "25.0.0.0"
                         }
                     ],
    "internalsVisibleTo":  [
                               {
                               }
                           ],
    "screenshots":  [

                    ],
    "platform":  "24.0.0.0",
    "target":  "OnPrem",
    "idRanges":  [
                     {
                         "from":  1,
                         "to":  9999
                     },
                 ],
    "features":  [
                     "TranslationFile",
                     "GenerateCaptions",
                     "NoImplicitWith",
                     "NoPromotedActionProperties"
                 ],
    "resourceExposurePolicy":  {
                                   "allowDebugging":  true,
                                   "allowDownloadingSource":  true,
                                   "includeSourceInSymbolFile":  true
                               },
    "contextSensitiveHelpUrl":  "https://learn.microsoft.com/dynamics365/business-central/"
}
DmitryKatson commented 7 months ago

@mazhelez with the latest (preview) bc container helper the script was executed successfully. However for some reason I can't access container from the browser.

This is the status

 BcContainerHelper version 6.0.7-preview1151
BC.HelperFunctions emits usage statistics telemetry to Microsoft
PS C:\Users\vmadmin> Restart-BcContainer BC-BusinessFoundation
Removing Session BC-BusinessFoundation
BC-BusinessFoundation
Waiting for container BC-BusinessFoundation to be ready

Initializing...
Setting host.containerhelper.internal to 172.19.176.1 in container hosts file
Restarting Container
PublicDnsName unchanged
Hostname is BC-BusinessFoundation
PublicDnsName is BC-BusinessFoundation
Using NavUserPassword Authentication
Starting Local SQL Server
Starting Internet Information Server
Starting Service Tier
Container IP Address: 172.19.185.225
Container Hostname  : BC-BusinessFoundation
Container Dns Name  : BC-BusinessFoundation
Web Client          : http://BC-BusinessFoundation/BC/
Dev. Server         : http://BC-BusinessFoundation
Dev. ServerInstance : BC

Files:
http://BC-BusinessFoundation:8080/ALLanguage.vsix

Container Total Physical Memory is 32.0Gb
Container Free Physical Memory is 26.7Gb

Initialization took 24 seconds
Ready for connections! 

but when i open http://BC-BusinessFoundation/BC/ i see

image

When I run download symbols i get

[2024-03-04 12:55:20.37] Using reference symbols cache paths: [c:\Users\vmadmin\Documents\AL\BCApps-Number-Series-Copilot\BCApps\src\Business Foundation\App\.alpackages]
[2024-03-04 12:55:20.41] Targeting server 'http://BC-BusinessFoundation', server instance 'BC' and tenant 'default'.
[2024-03-04 12:55:20.62] Please authenticate in the Visual Studio Code...
[2024-03-04 12:55:28.28] Using reference symbols cache paths: [c:\Users\vmadmin\Documents\AL\BCApps-Number-Series-Copilot\BCApps\src\Business Foundation\App\.alpackages]
[2024-03-04 12:55:28.28] Targeting server 'http://BC-BusinessFoundation', server instance 'BC' and tenant 'default'.
[2024-03-04 12:55:28.30] Using user name and password authentication. User name used is: 'admin'.
[2024-03-04 12:55:28.32] Sending request to http://bc-businessfoundation:7049/BC/dev/metadata?tenant=default
[2024-03-04 12:55:29.41] Error: No such host is known. (bc-businessfoundation:7049)
No such host is known.

[2024-03-04 12:55:29.41] Error: An error occured while processing the request.
Request ID: 6a3a9330-c44c-433d-a5a1-45dae9c2eee0
Session ID: e222fbd6-cee4-4eba-88a0-ab4c2efce434

If you are targeting a cloud instance, supply these IDs if contacting Microsoft support.
[2024-03-04 12:55:29.42] Sending request to http://bc-businessfoundation:7049/BC/dev/metadata?tenant=default
[2024-03-04 12:55:30.42] Error: No such host is known. (bc-businessfoundation:7049)
No such host is known.

[2024-03-04 12:55:30.42] Error: An error occured while processing the request.
Request ID: a8a69d2d-9b04-464f-8423-e880216aca15
Session ID: e222fbd6-cee4-4eba-88a0-ab4c2efce434

If you are targeting a cloud instance, supply these IDs if contacting Microsoft support.
JesperSchulz commented 7 months ago

Your app.json looks about right! You should indeed just use the ID range between 1 - 9.999. Try to find available IDs close to the IDs already in the module. E.g. codeunit 326 and up seem to be free. We want to use ID Ninja going forward, but not everyone is using it yet, which sometimes leads to ID conflicts when using it.

DmitryKatson commented 7 months ago

@mazhelez there was probably something with my VM. I made a new one and everything worked as expected. Thanks.

mazhelez commented 7 months ago

@mazhelez there was probably something with my VM. I made a new one and everything worked as expected. Thanks.

Happy to hear everything works for you now!

DmitryKatson commented 7 months ago

@JesperSchulz sorry for stupid questions :)

My business central is empty

image

So, there are no base app, system app, foundation app etc. It's a bit uncomfortable to see such picture ;) What are the next steps?

DmitryKatson commented 7 months ago

@JesperSchulz also if i need to add, let's say, action to the Number Series page, should I change the Number Series page directly, or make pageextention ?

Drakonian commented 7 months ago

@JesperSchulz sorry for stupid questions :)

  • I made a local branch
  • I created new folder src\Business Foundation\App\NoSeriesCopilot

My business central is empty image

So, there are no base app, system app, foundation app etc. It's a bit uncomfortable to see such picture ;) What are the next steps?

This is something what should be improved :) https://github.com/microsoft/BCApps/pull/417#discussion_r1422164087

JesperSchulz commented 7 months ago

@JesperSchulz also if i need to add, let's say, action to the Number Series page, should I change the Number Series page directly, or make pageextention ?

Since you're now working directly on the application platform, you should just change the No. Series module to your liking. As a matter of fact... why are you even creating a separate module? Would it maybe make sense that you just add your capabilities to the "No. Series" module? πŸ€” I actually think it would! Or would one maybe want to have No. Series without AI wizards? I'm a little torn here - but this can be adjusted as we go along.

With regards to the environment, I just discussed the same matter with @mazhelez last week. I was expecting a container with at least the System App and Business Foundation pre-installed, but she told me I have to compile and publish those myself.

@freddydk, I seem to recall "in the old days" (say half a year ago), my containers came with everything pre-installed. Have we changed that?

DmitryKatson commented 7 months ago

@JesperSchulz also if i need to add, let's say, action to the Number Series page, should I change the Number Series page directly, or make pageextention ?

Since you're now working directly on the application platform, you should just change the No. Series module to your liking. As a matter of fact... why are you even creating a separate module? Would it maybe make sense that you just add your capabilities to the "No. Series" module? πŸ€” I actually think it would! Or would one maybe want to have No. Series without AI wizards? I'm a little torn here - but this can be adjusted as we go along.

With regards to the environment, I just discussed the same matter with @mazhelez last week. I was expecting a container with at least the System App and Business Foundation pre-installed, but she told me I have to compile and publish those myself.

@freddydk, I seem to recall "in the old days" (say half a year ago), my containers came with everything pre-installed. Have we changed that?

I try to follow the other copilots pattern, which are separate apps. Also, having in mind that some localizations (like Canada), and, probably India, according o the latest news, have separate AI regulations and rules, I think it make sense to make it as separate app, that can be uninstalled.

JesperSchulz commented 7 months ago

If you're putting the module in the Business Foundation, it will get compiled into one app (just as it happens in the System Application) and you will not have a possibility to uninstall the app. If you want it to be a separate app, you would have to create it as a first party app. In that case, we need to move this issue to the ALAppExtensions repository though. I would very much be in favor of keeping the code in the Business Foundation. @grobyns / @darjoo, what's your take on this?

darjoo commented 7 months ago

I'd be in favor of keeping it within Business Foundation as well. I understand Dmitry's concern about the localization and regulations, however in the long run, the view is that all these copilots will be part of BC by default irregardless of your the country and also can't be turned deactivated. As No. Series is a Business Foundation feature, its copilot should also be part of it. The registration etc can be limited within the Install/Upgrade codeunits where we specifically disallow the registration of the copilot in CA region. I'm not sure what other regions will have this limitation off the top of my head, but we can always update that when we're going through the responsible AI reviews.

DmitryKatson commented 7 months ago

Ok, got it. @JesperSchulz would that be correct structure then?

image
JesperSchulz commented 7 months ago

Getting there. We usually put our code into a src folder (see NoSeries module). Also, even though everything gets compiled into a single app, the modules must be able to compile individually. You will hence need a dependency to the No. Series module.

DmitryKatson commented 7 months ago

The work started https://github.com/DmitryKatson/BCApps/commit/44ac6087bd024dbfc203a1099726f200ac28d189 The progress can be seen here https://github.com/DmitryKatson/BCApps/tree/Number-Series-Copilot

JesperSchulz commented 7 months ago

The work started DmitryKatson@44ac608 The progress can be seen here https://github.com/DmitryKatson/BCApps/tree/Number-Series-Copilot

Now that is exciting! I'll tag along πŸ’ͺ

DmitryKatson commented 7 months ago

@JesperSchulz also if i need to add, let's say, action to the Number Series page, should I change the Number Series page directly, or make pageextention ?

Since you're now working directly on the application platform, you should just change the No. Series module to your liking. As a matter of fact... why are you even creating a separate module? Would it maybe make sense that you just add your capabilities to the "No. Series" module? πŸ€” I actually think it would! Or would one maybe want to have No. Series without AI wizards? I'm a little torn here - but this can be adjusted as we go along.

With regards to the environment, I just discussed the same matter with @mazhelez last week. I was expecting a container with at least the System App and Business Foundation pre-installed, but she told me I have to compile and publish those myself.

@freddydk, I seem to recall "in the old days" (say half a year ago), my containers came with everything pre-installed. Have we changed that?

Ok. I compiled and published System Application and Business Foundation app.

PS C:\Windows\System32> (Get-BcContainerAppInfo -containerName "BC-BusinessFoundation").Name
Business Foundation
Business Foundation Test Libraries
Business Foundation Tests
Library Assert
Any
Permissions Mock
System Application

The Business Cdntral UI didn't change (still empty) How should i publish base app?

DmitryKatson commented 7 months ago

@EvgenijKorovin any updates on "Support for calling AI functions and defining function hierarchies." from Wave 1 plans ?

Probably I'll need this here

DmitryKatson commented 7 months ago

Definition of covered / not covered scenarios

This classification helps in understanding the nature of user requests and designing AI responses that can accurately address them within the "Number Series Copilot" app.

Covered Scenarios

Question Example Description Intent
Set up numbers series for the new company Automatically generate number series for all relevant tables using a pattern similar to the Cronus demo company. Create New Number Series
Set up numbers series for the new company using next patterns: {specify patterns} Generate number series for all relevant tables using user-specified patterns. Create New Number Series
Set up a number series for purchase orders Create a new number series specifically for purchase orders. Create New Number Series
Set up a number series for purchase orders starting from 5000 Create a new number series for purchase orders, starting with 5000. Create New Number Series
Set up number series for the next year Generate new series for the next year based on existing series with a yearly pattern. Create New Number Series
Apply No Series to setup Find the setup table and field for each number series and add the number series code to the field. Configuration and Setup
Change the starting number of the sales order series to 1001 Modify the starting number of the sales order series to 1001. Modify Existing Number Series
Change the starting number of the sales order series to 1001 starting from next week Modify the starting number of the sales order series to 1001, effective from the next week. Modify Existing Number Series
Update the prefix for customer number series to 'CUS-'. Change the prefix of the customer number series to "CUS-". Modify Existing Number Series
Assign the 'INV-2024' series to invoices. Link the 'INV-2024' number series to both sales and purchase invoices. Modify Existing Number Series
Reset the purchase invoices number series for the new year. Reset the number series for purchase invoices for the upcoming new year. Modify Existing Number Series

Not Covered Scenarios:

Question Example Description Intent
What's the next number for the vendor series? Inquire about the next available number in the vendor series. Usage and Management
Generate the next project number. Request to generate and retrieve the next number in the project series. Usage and Management
How to reset number series annually? Query on the instruction to reset number series on an annual basis. Assistance
How many numbers are left in the 'INV-' series? Check the remaining numbers available in the 'INV-' series. Usage and Management
Create a different number series for west coast sales orders. Set up a unique number series for sales orders originating from the west coast. Advanced Features
Link the item number series with inventory management. Integrate the item number series with the inventory management module. Advanced Features
Find gaps in Sales Invoices numbers Identify any missing numbers in the sequence of Sales Invoice numbers. Advanced Features
Which number series is used for sales orders? Query to identify the number series assigned to sales orders. Usage and Management
How do I create a new invoice number series? Instructions on creating a new number series for invoices. Assistance
Check if my number series looks correct Request to validate the correctness and configuration of an existing number series. Usage and Management
DmitryKatson commented 7 months ago

@JesperSchulz also if i need to add, let's say, action to the Number Series page, should I change the Number Series page directly, or make pageextention ?

Since you're now working directly on the application platform, you should just change the No. Series module to your liking. As a matter of fact... why are you even creating a separate module? Would it maybe make sense that you just add your capabilities to the "No. Series" module? πŸ€” I actually think it would! Or would one maybe want to have No. Series without AI wizards? I'm a little torn here - but this can be adjusted as we go along. With regards to the environment, I just discussed the same matter with @mazhelez last week. I was expecting a container with at least the System App and Business Foundation pre-installed, but she told me I have to compile and publish those myself. @freddydk, I seem to recall "in the old days" (say half a year ago), my containers came with everything pre-installed. Have we changed that?

Ok. I compiled and published System Application and Business Foundation app.

PS C:\Windows\System32> (Get-BcContainerAppInfo -containerName "BC-BusinessFoundation").Name
Business Foundation
Business Foundation Test Libraries
Business Foundation Tests
Library Assert
Any
Permissions Mock
System Application

The Business Cdntral UI didn't change (still empty) How should i publish base app?

Ok, seems I figure out. Not sure it's correct way, but Base App was published (took very long time) and now i see familiar BC look and feel.

 $containerName = "BC-BusinessFoundation"
 $baseApp = "C:\bcartifacts.cache\sandbox\25.0.16618.0\platform\Applications\BaseApp\Source\Microsoft_Base Application.app"

# Create a credential object non-interactively
$username = "admin"
$password = ConvertTo-SecureString "my password" -AsPlainText -Force
$credential = New-Object System.Management.Automation.PSCredential ($username, $password)

# Publish the app using the specified credentials
Publish-BcContainerApp -containerName $containerName -appFile $baseApp -skipVerification -ignoreIfAppExists -syncMode Development -sync -install -useDevEndpoint -credential $credential
image
DmitryKatson commented 7 months ago

Made a flow diagram. Not sure if we need the Apply Existing Number Series to Setup branch. I think for the beginning we need to focus on Generate New Number Series and Modify Existing Number Series.

diagram-export-16-03-2024-11_53_25

DmitryKatson commented 7 months ago

@darjoo @EvgenijKorovin I'm going to use Azure OpenAI functions calling support here and use new codeunit 7778 "AOAI Tools Impl" to add tools to the aoai call. Can i store functions definition in AL? Or you suppose to keep them secret (same as system prompt) and retrieve from Azure Key Vault?

darjoo commented 7 months ago

They have to also be kept in KV. Not everything needs to be in KV for example SetToolChoice that does not need to be kept in KV. And to comment on your question to Ievgenii about Tools being available. We want to make them available for all asap, but we're held back due to the preview specifications from AOAI. Once there's a GA version, we'll update and release it for all.

What tool did you use to create your flow diagram? It's really nice.

DmitryKatson commented 7 months ago

@darjoo I'm looking at codeunit 132686 "Azure OpenAI Tools Test" and this test particulary

    [Test]
    procedure TestToolCoiceInChatMessages()
    var
        AOAIChatMessages: Codeunit "AOAI Chat Messages";
        Function1Tool: JsonObject;
        ToolChoice: Text;
    begin
        Function1Tool := GetTestFunction1Tool();
        AOAIChatMessages.AddTool(GetTestFunction1Tool());
        LibraryAssert.AreEqual('auto', AOAIChatMessages.GetToolChoice(), 'Tool choice should be auto by default.');

        ToolChoice := GetToolChoice();
        AOAIChatMessages.SetToolChoice(ToolChoice);
        LibraryAssert.AreEqual(ToolChoice, AOAIChatMessages.GetToolChoice(), 'Tool choice should be equal to what was set.');
    end;

btw, there is a typo in the name TestToolCoiceInChatMessages ...

But does it mean that if i want to add tools to the AOAIChatMessages with AOAIChatMessages.AddTool the parameter, in this case GetTestFunction1Tool() should come from Azure Vault?

Also, when I retrieve the response, would be nice to have some procedure in AOAIChatMessages or AOAIOperationResponse indicating that function should be called. In codeunit 7772 "Azure OpenAI Impl".ProcessChatCompletionResponse() in case function calling answer you just save it as normal answer ChatMessages.AddAssistantMessage(ToolsCall);. Would be nice to somehow mark it that function should be called then.

So in my/others code we can do something like

        AzureOpenAI.GenerateChatCompletion(AOAIChatMessages, AOAIChatCompletionParams, AOAIOperationResponse);
        if AOAIOperationResponse.IsSuccess() then
            CompletionAnswerTxt := AOAIChatMessages.GetLastMessage()
        else
            Error(AOAIOperationResponse.GetError());

        if AOAIOperationResponse.ToolsCallRequired() then
            RunMyFunctions(AzureOpenAI.GetTools());

P.S. The diagramm tool is called eraser.io . Very nice tool, allow to build diagrams with their code and also they have "text 2 diagramm with AI" option

DmitryKatson commented 7 months ago

@JesperSchulz dependency question. I would like to use codeunit 5459 "JSON Management", but it's a part of Base Application. I don't think I need to make dependency on Base App, it makes no sense for the Business Foundation Level. However, I believe, Json Management should be in the system app. What would you suggest?

JesperSchulz commented 7 months ago

@JesperSchulz dependency question. I would like to use codeunit 5459 "JSON Management", but it's a part of Base Application. I don't think I need to make dependency on Base App, it makes no sense for the Business Foundation Level. However, I believe, Json Management should be in the system app. What would you suggest?

You're absolutely right, that JSON Management should be a module in the System App. You are also right, that you may not take a dependency on the BaseApp from the Business Foundation, just as you may not use .NET in the Business Foundation. In reality that only leaves you with one option: Go the extra mile and create a JSON module (only implement the API you require). This is actually the way we do componentization these days internally as well. We refactor and componentize on a "per need" basis. It shouldn't be too hard to create such module. Let me know if you need my help doing so!

DmitryKatson commented 7 months ago

@JesperSchulz dependency question. I would like to use codeunit 5459 "JSON Management", but it's a part of Base Application. I don't think I need to make dependency on Base App, it makes no sense for the Business Foundation Level. However, I believe, Json Management should be in the system app. What would you suggest?

You're absolutely right, that JSON Management should be a module in the System App. You are also right, that you may not take a dependency on the BaseApp from the Business Foundation, just as you may not use .NET in the Business Foundation. In reality that only leaves you with one option: Go the extra mile and create a JSON module (only implement the API you require). This is actually the way we do componentization these days internally as well. We refactor and componentize on a "per need" basis. It shouldn't be too hard to create such module. Let me know if you need my help doing so!

If I would need some JSON Module APIs, should I run a separate idea > issue flow ?

DmitryKatson commented 7 months ago

@darjoo the Copilots issued by Microsoft run only in SaaS, so there is CopilotActionsVisible := EnvironmentInformation.IsSaaSInfrastructure() in many places. I will do the same, but how do you manually test/debug copilots in docker? Or do you add this at the very end when everything is tested?

JesperSchulz commented 7 months ago

@JesperSchulz dependency question. I would like to use codeunit 5459 "JSON Management", but it's a part of Base Application. I don't think I need to make dependency on Base App, it makes no sense for the Business Foundation Level. However, I believe, Json Management should be in the system app. What would you suggest?

You're absolutely right, that JSON Management should be a module in the System App. You are also right, that you may not take a dependency on the BaseApp from the Business Foundation, just as you may not use .NET in the Business Foundation. In reality that only leaves you with one option: Go the extra mile and create a JSON module (only implement the API you require). This is actually the way we do componentization these days internally as well. We refactor and componentize on a "per need" basis. It shouldn't be too hard to create such module. Let me know if you need my help doing so!

If I would need some JSON Module APIs, should I run a separate idea > issue flow ?

That's mostly up to you. To reduce the chance of code review fatigue, I would suggest that you split the PRs and first get the JSON module checked in - that should be a quick one! The larger PRs get, the less effective the code reviews are. Whether you want to track your work under a new issue, or whether you want to link your separate PR to this issue, is up to you. It'll take me 10 sec to approve the new issue, if you create one (no BC idea needed here - just refer to this issue in the new issue and mention it's a spin-off) 😊

DmitryKatson commented 7 months ago

@JesperSchulz seems i have a major issue. To make it all work as expected i need to have the info about Business Central tables. For this i use table 2000000136 "Table Metadata".

This is my function

    local procedure ListAllTablesWithNumberSeries(var NewNoSeriesPrompt: TextBuilder)
    var
        TableMetadata: Record "Table Metadata";
    begin
        // Looping trhough all Setup tables
        TableMetadata.SetFilter(Name, '* Setup');
        TableMetadata.SetRange(ObsoleteState, TableMetadata.ObsoleteState::No); //TODO: Check if 'Pending' should be included
        TableMetadata.SetRange(TableType, TableMetadata.TableType::Normal);
        if TableMetadata.FindSet() then
            repeat
                ListAllNoSeriesFields(NewNoSeriesPrompt, TableMetadata);
            until TableMetadata.Next() = 0;
    end;

The problem is that, seems, that from Business Foundation layer this table don't "see" all tables from the Base Application! Is it me? Or this is our stop factor?

DmitryKatson commented 7 months ago

@JesperSchulz seems i have a major issue. To make it all work as expected i need to have the info about Business Central tables. For this i use table 2000000136 "Table Metadata".

This is my function

    local procedure ListAllTablesWithNumberSeries(var NewNoSeriesPrompt: TextBuilder)
    var
        TableMetadata: Record "Table Metadata";
    begin
        // Looping trhough all Setup tables
        TableMetadata.SetFilter(Name, '* Setup');
        TableMetadata.SetRange(ObsoleteState, TableMetadata.ObsoleteState::No); //TODO: Check if 'Pending' should be included
        TableMetadata.SetRange(TableType, TableMetadata.TableType::Normal);
        if TableMetadata.FindSet() then
            repeat
                ListAllNoSeriesFields(NewNoSeriesPrompt, TableMetadata);
            until TableMetadata.Next() = 0;
    end;

The problem is that, seems, that from Business Foundation layer this table don't "see" all tables from the Base Application! Is it me? Or this is our stop factor?

Wait, seems it's me. When I publish Business Foundation app, my Base Application is been removed ... hm

JesperSchulz commented 7 months ago

Indeed you should have full access to the "Table Metadata" table!

DmitryKatson commented 7 months ago

@darjoo I've added AddToolMessage API to the codeunit 7763 "AOAI Chat Messages" as i need to construct message in this format

                    "tool_call_id": tool_call.id,
                    "role": "tool",
                    "name": function_name,
                    "content": function_response,

according to https://learn.microsoft.com/en-us/azure/ai-services/openai/how-to/function-calling?tabs=python-new

Take a look here

This is a mockup of the new chat history, that's been send to

{"role":"system","content":"Help with Number Series"}
{"role":"user","content":"Set up numbers series for the new company"}
{"role":"assistant","tool_calls":[{"id":"call_KzEtrcxiQNaXOOUJjOeyike1","type":"function","function":{"name":"generate_new_numbers_series","arguments":"{}"}}]}
{"role":"tool","content":"Bank Account Nos.\r\nPatterns: \r\n B00010","name":"generate_new_numbers_series","tool_call_id":"call_KzEtrcxiQNaXOOUJjOeyike1"}
darjoo commented 7 months ago

Thanks for pointing out the spelling in the test name. I will get that fixed. Yes, the tool should come from KV and you can mock the KV similar to the function below (Ripped from ImageAnalysis test). We haven't put out guidance on it, however, not everything of the function is required to be from KV. Fx name, parameters. The key is description which is passed and interpreted by the model for what the function does. We consider the description to be IP, just like metaprompts.

    local procedure InitializeMockKeyvault(ApiKey: Text; ApiEndpoint: Text; ImageAnalysisLimit: Text; ImageAnalysisPeriodType: Text)
    var
        AzureKeyVaultTestLibrary: Codeunit "Azure Key Vault Test Library";
    begin
        MockAzureKeyvaultSecretProvider := MockAzureKeyvaultSecretProvider.MockAzureKeyVaultSecretProvider();
        MockAzureKeyvaultSecretProvider.AddSecretMapping('AllowedApplicationSecrets', 'cognitive-vision-params');
        MockAzureKeyvaultSecretProvider.AddSecretMapping(
          'cognitive-vision-params', StrSubstNo(KeyvaultValueTxt, ApiKey, ApiEndpoint, ImageAnalysisPeriodType, ImageAnalysisLimit));

        AzureKeyVaultTestLibrary.SetAzureKeyVaultSecretProvider(MockAzureKeyvaultSecretProvider);
    end;

It is a very good suggestion to add the tools into the messages, we didn't directly add it yet because we weren't exactly sure how that should be used as GetLastMessage returns the parsed message (without tools) and if we add it, we know have a json in there. Or we track it separately. That's something that needs a bit more brainstorming. I definitely agree there are improvements that need to be done when working with Tools/Functions. One of the biggest annoyance I have with it right now is having to parse with JSONObjects & tokens when you get the result. Which is repetitive work devs should not be required to do and that should live in the module directly.

When it comes parts where we have CopilotActionsVisible := EnvironmentInformation.IsSaaSInfrastructure(), we add that at the very end once we have everything completed and tested locally. Then when going to test on SaaS we start to add it and ensure the SaaS & OnPrem are as expected.

Let me review that last message about AddToolMessage and get back to you again later today!

DmitryKatson commented 7 months ago

@darjoo I found little issue in this commit. I'll fix, rebase commit once again and let you know.

DmitryKatson commented 7 months ago

@darjoo fixed, you can check here

Now it's correctly accept this chat history and return back answer

{"role":"system","content":"Help with Number Series"}
{"role":"user","content":"Set up numbers series for the new company"}
{"role":"assistant","tool_calls":[{"id":"call_KzEtrcxiQNaXOOUJjOeyike1","type":"function","function":{"name":"generate_new_numbers_series","arguments":"{}"}}]}
{"role":"tool","content":"Bank Account Nos.\r\nPatterns: \r\n B00010","name":"generate_new_numbers_series","tool_call_id":"call_KzEtrcxiQNaXOOUJjOeyike1"}
DmitryKatson commented 6 months ago

@darjoo Introduce here "response_format":"{ "type": "json_object" }" to explicitly set JSON Mode output as described here . It works perfectly well.

However 1 thing is out of my control. Would be nice to get finish_reason from the responce, to understand if Json output is full by analysing values stop or length. I added a placeholder to this in

codeunit 7770 "AOAI Operation Response"
    var
...
    // FinishReason: Text; //TODO: Enable this when FinishReason is added to ALCopilotOperationResponse dotnet class

...

    /// <summary>
    /// Gets the finish reason of last response.
    /// </summary>
    /// <returns></returns>
    // TODO: Enable this when FinishReason is added to ALCopilotOperationResponse dotnet class
    // procedure GetFinishReason(): Text
    // begin
    //     exit(FinishReason);
    // end;

    internal procedure SetOperationResponse(var ALCopilotOperationResponse: DotNet ALCopilotOperationResponse)
    begin
        Success := ALCopilotOperationResponse.IsSuccess();
        StatusCode := ALCopilotOperationResponse.StatusCode;
        Result := ALCopilotOperationResponse.Result();
        Error := ALCopilotOperationResponse.ErrorText();
        // FinishReason := ALCopilotOperationResponse.FinishReason; //TODO: Add FinishReason to ALCopilotOperationResponse dotnet class

        if Error = '' then
            Error := GetLastErrorText();
    end;
DmitryKatson commented 6 months ago

@JesperSchulz dependency question. I would like to use codeunit 5459 "JSON Management", but it's a part of Base Application. I don't think I need to make dependency on Base App, it makes no sense for the Business Foundation Level. However, I believe, Json Management should be in the system app. What would you suggest?

You're absolutely right, that JSON Management should be a module in the System App. You are also right, that you may not take a dependency on the BaseApp from the Business Foundation, just as you may not use .NET in the Business Foundation. In reality that only leaves you with one option: Go the extra mile and create a JSON module (only implement the API you require). This is actually the way we do componentization these days internally as well. We refactor and componentize on a "per need" basis. It shouldn't be too hard to create such module. Let me know if you need my help doing so!

If I would need some JSON Module APIs, should I run a separate idea > issue flow ?

That's mostly up to you. To reduce the chance of code review fatigue, I would suggest that you split the PRs and first get the JSON module checked in - that should be a quick one! The larger PRs get, the less effective the code reviews are. Whether you want to track your work under a new issue, or whether you want to link your separate PR to this issue, is up to you. It'll take me 10 sec to approve the new issue, if you create one (no BC idea needed here - just refer to this issue in the new issue and mention it's a spin-off) 😊

Done Json module (only required for the copilot implementation apis) https://github.com/microsoft/BCApps/pull/716

DmitryKatson commented 6 months ago

@darjoo added PR with AI module changes required to Number Series Copilot https://github.com/microsoft/BCApps/pull/719

DmitryKatson commented 6 months ago

@darjoo weird behaviour. TokenCountImpl.GetGPT4TokenCount return me always 0. Am I doing something wrong?

procedure TestGetGPT4TokenCount()
        TestText: Text;
        TestTextTokens: Integer;
        TestSecretText: SecretText;
        TestSecretTextTokens: Integer;
    begin
        TestText := 'This is test text';
        TestSecretText := TestText;

        TestTextTokens := TokenCountImpl.GetGPT4TokenCount(TestText);
        TestSecretTextTokens := TokenCountImpl.GetGPT4TokenCount(TestSecretText);
end

results for TestTextTokens, TestSecretTextTokens = 0

darjoo commented 6 months ago

@darjoo weird behaviour. TokenCountImpl.GetGPT4TokenCount return me always 0. Am I doing something wrong?

procedure TestGetGPT4TokenCount()
        TestText: Text;
        TestTextTokens: Integer;
        TestSecretText: SecretText;
        TestSecretTextTokens: Integer;
    begin
        TestText := 'This is test text';
        TestSecretText := TestText;

        TestTextTokens := TokenCountImpl.GetGPT4TokenCount(TestText);
        TestSecretTextTokens := TokenCountImpl.GetGPT4TokenCount(TestSecretText);
end

results for TestTextTokens, TestSecretTextTokens = 0

Oh no.. I just tested and got the same issue. Let me investigate into that. It worked before :(