India
Account created on 8 January 2018, over 7 years ago
  • Drupal and Mautic Engineer at Dropsolid 
#

Merge Requests

More

Recent comments

🇮🇳India abhisekmazumdar India

Thank You for clearing my confusion. I tested it again and it looks good to me.

🇮🇳India abhisekmazumdar India

I merged these changes on top of 📌 Use a different model for LLM evaluation Active

Here what I see when running the drush command:

Command: ddev drush agetes --group_id=1 --uid=1 --eval_provider=openai --eval_model=o3 --detailed

 [notice] Running tests as user: admin
 [notice] Running test group: Basic Pages Test Group
 [notice] Overriding LLM evaluation model: openai/o3
 [notice] Running test 1 of 3: Test ID 1 (nomic-embed-text:latest)
 [error]  Error invoking model response: "nomic-embed-text:latest" does not support chat 
 [notice] Running test 2 of 3: Test ID 2 (nomic-embed-text:latest)
 [error]  Error invoking model response: "nomic-embed-text:latest" does not support chat 
 [notice] Running test 3 of 3: Test ID 3 (nomic-embed-text:latest)
 [error]  Error invoking model response: "nomic-embed-text:latest" does not support chat 
 [notice] Test group completed. Results available at: /admin/content/ai-agents-test/group/result/9
 ------------- --------- ---------------------- -------------------------------------------------------------------------- 
  Id            Result    Label                  Message                                                                   
 ------------- --------- ---------------------- -------------------------------------------------------------------------- 
  test_1        Error     Is Basic Page          Error: Error invoking model response: "nomic-embed-text:latest" does not  
                          content type sticky    support chat, Line: 309, File:                                            
                          by default             /var/www/html/web/modules/contrib/ai/src/Plugin/ProviderProxy.php         
  test_2        Error     Is Basic Page          Error: Error invoking model response: "nomic-embed-text:latest" does not  
                          content type           support chat, Line: 309, File:                                            
                          promoted by default    /var/www/html/web/modules/contrib/ai/src/Plugin/ProviderProxy.php         
  test_3        Error     Is Basic Page          Error: Error invoking model response: "nomic-embed-text:latest" does not  
                          content type           support chat, Line: 309, File:                                            
                          published by default   /var/www/html/web/modules/contrib/ai/src/Plugin/ProviderProxy.php         
  group_total   Failure   Group: Basic Pages     0/3 tests passed | Duration: 0s | Group Result ID: 9 | Success rate:      
                          Test Group             0.00%                                                                     
 ------------- --------- ---------------------- -------------------------------------------------------------------------- 
🇮🇳India abhisekmazumdar India

Forgot to update the status

🇮🇳India abhisekmazumdar India

I think I can move this at least with the initial work. Then later on, we can decide about the validator thing.

🇮🇳India abhisekmazumdar India

Added some inline comment over the MR.

🇮🇳India abhisekmazumdar India

I see the new Agent Evaluation section. It opens a section which has the checkbox, and when the checkbox is unchecked, it shows the available module list to select from. It runs the test with the selected model.
Financially, the changes look good to me.

🇮🇳India abhisekmazumdar India

I have made the final changes, added the document and test. I also made some changes to the CSS and cleaned up the other parts of the code. This is now up for review.

Example usage:

$form['conversation'] = [
      '#type' => 'chat_history',
      '#title' => $this->t('Conversation'),
      '#default_value' => [
        [
          'role' => 'user',
          'content' => 'What is the weather like?'
        ],
        [
          'role' => 'assistant',
          'content' => 'I can check that for you.',
          'tool_calls' => [
            [
              'tool_call_id' => 'call_1',
              'function_name' => 'get_weather',
              'function_input' => '{"location":"Oslo","unit":"celsius"}',
            ],
          ],
        ],
        [
          'role' => 'tool',
          'content' => '{"temp":25,"condition":"sunny"}',
          'tool_call_id_reference' => 'call_1'
        ],
      ],
    ];

🇮🇳India abhisekmazumdar India

I made the update to the Draft changes: https://git.drupalcode.org/project/ai/-/merge_requests/719/diffs?commit_...

This includes added Tool call section for the Role Assistant which has the above said fields.
And Role Tool has a tool called Reference ID

Example use of the element:

$form['conversation'] = [
      '#type' => 'chat_history',
      '#title' => $this->t('Conversation'),
      '#default_value' => [
        [
          'role' => 'user', 
          'content' => 'What is the weather like?'
        ],
        [
          'role' => 'assistant',
          'content' => 'I can check that for you.',
          'tool_calls' => [
            [
              'tool_call_id' => 'call_1',
              'function_name' => 'get_weather',
              'function_input' => '{"location":"Oslo","unit":"celsius"}',
            ],
          ],
        ],
        [
          'role' => 'tool', 
          'content' => '{"temp":25,"condition":"sunny"}', 
          'tool_call_id_reference' => 'call_1'
        ],
        [
          'role' => 'user', 
          'content' => 'Can you also check the time?'
        ],
      ],
      '#chat_history_add_more' => TRUE,
      '#rows' => 2,
    ];

preview:

Sharing my work in progress here to ensure I'm heading in the right direction.

🇮🇳India abhisekmazumdar India

I got the gist of it and will give it a try
My understanding is this chat_history FormElement will need the following changes next:

- A existing / new section of assistant is created it should have the following as string as of now

  • tool_call_id
  • function_name
  • function_input

- And for tool add a field tool_call_id_reference.

🇮🇳India abhisekmazumdar India

I feel there is a bug in the logic how the form handle the Automator Base Field value. Event if we select Advanced Mode (Token) the value for Automator Base Field get stored in the config. Which result it making the Placeholders context work in some cases. In theory Placeholders context shouldn't be need for the advance mode.

I feel the solution here will be to remove the placeholder for the advance mode and manage the value of Automator Base Field addConfigValues() to only save the value when the mode is Base Mode.

// Only persist base_field when Base mode is selected; otherwise clear.
$aiConfig->set('base_field', ($formState->getValue('automator_mode') === 'base') ? ($formState->getValue('automator_base_field') ?? '') : '');

Advanced Mode is intended to be used only with tokens, that's how I feel.
Changing this needs review, as it requires input from the maintainer of the module regarding the logical implementation standpoint for this functionality.

🇮🇳India abhisekmazumdar India

I attempted to clean up the direct calling of services in the RuleBase file and quickly realized that it would be a significant task. The RuleBase has been extended by many other classes, each with its own dependency injection for the services included in the RuleBase. I will create an issue for this cleanup since I have completed the current task.

🇮🇳India abhisekmazumdar India

Made the required remaining changes listed by @scott_euser. Thanks for that! That made things easy for me to quickly jump in and add them.
Update the test. It looks like it is working now. I see a few failed cases for AiSearchSetupMySqlTest, but they are mostly annotation deprecation warnings and a few others that are not related to the changes included in this MR.

🇮🇳India abhisekmazumdar India

I can try to add the missing things here.

🇮🇳India abhisekmazumdar India

I tested it again. It looks like it's working for me.

I made some improvements to the MR and added a commit to the MR: https://git.drupalcode.org/project/ai/-/merge_requests/612/diffs?commit_...

🇮🇳India abhisekmazumdar India

Thanks @b_sharpe and @admitriiev for the suggestion. I have update the MR and also added test coverage.

🇮🇳India abhisekmazumdar India

The failed CI job says something wrong with Drupal\Tests\ckeditor5\FunctionalJavascript\MediaLibrary Not sure if that related to my changes.

Other then that its open for review.

🇮🇳India abhisekmazumdar India

@himanshu5050

I tried the following steps

  1. Installed the module ai_ckeditor
  2. Config Basic HTML in Text formats and editors to have the "✨ AI CKEditor" button to the active window.
  3. In the same config form AI tools tab enable Fix spelling, Modify with a prompt, Tone and few others.
  4. Edit a basic page with has a Basic HTML format field.
  5. I tried using few of the AI tools as shown in the below.

As you can see, I can't reproduce the issue. Can you help me identify if I'm missing anything?

🇮🇳India abhisekmazumdar India

So there the code: https://gitlab.com/dropsolid/drupal_ai_cms_installer

This is a modified version of the drupal_cms_installer. It adds two install tasks: one to select an AI provider of choice (a form) and another to use the package manager to pull the said module into the codebase and set up the key config with the selected AI provider.

This was just a proof of concept, and we need to clarify how we manage different AI provider configs.
Let me know your thoughts on this POC if this can be the solution we want?

🇮🇳India abhisekmazumdar India

Here the demo for what I have been working on: https://www.loom.com/share/5df4b8feae5f43f2858edcf660a84454?sid=0f739b10...

The code is very rough right now and needs to be cleaned up a little. I'll share the modified drupal_cms_installer from Drupal CMS in a repo. In brief, here is what I've done:

- Added two install tasks after the site installation.
- The first task is to show a form for users to select their required AI provider and then require the module using Package manager.
- The second install task sets up the key and the selected AI provider config.

Some limitations are:

We still need to write specific code for each module.
For example, the module ai_provider_google_vertex requires its key to be in the config file. Other AI provider modules accept the key in the config as well. Therefore, a custom solution will be needed to add a specific AI provider. Alternatively, maintainer of the AI provider module can follow a standard config structure.

🇮🇳India abhisekmazumdar India

Yes, the inspiration for adding an extra form in the installer came from the drupal-starforge repository. I have mostly copied the same form.

Regarding the BYO keys module, I too believe it would be beneficial to have, but at the same time, my intention is to resolve the issues with the existing AI provider modules. Therefore, the approach of using the package manager after the site can address that.

🇮🇳India abhisekmazumdar India

This is merged and new release will be creating soon.

🇮🇳India abhisekmazumdar India

Made the changes. Please review the two new actions for DNC.

🇮🇳India abhisekmazumdar India

Thank you. This is a merge to the development branch. A release will be created soon with a few other similar actions are merged.

🇮🇳India abhisekmazumdar India

Thank you for the improvements. It's great to see how everything fits together like Lego pieces.

🇮🇳India abhisekmazumdar India

Thank you for the feedback.
I have addressed all the inline comments, please review them.
I also moved the relevant helper API method to mautic_api: 📌 Add helper method loadConnection & checkApiResponse Active

🇮🇳India abhisekmazumdar India

I was not feeling happy with the structure. I made some adjustments to the overall structure to align it with the other actions.
A new commit: https://git.drupalcode.org/project/mautic_eca/-/merge_requests/1/diffs?c...

🇮🇳India abhisekmazumdar India

Thank you for the improvement. This is good to go.

🇮🇳India abhisekmazumdar India

abhisekmazumdar made their first commit to this issue’s fork.

🇮🇳India abhisekmazumdar India

Thank you, @mallezie, for the improvement.

This enhancement makes the code cleaner and more maintainable, significantly enhancing the developer experience.

I have made a small fix to the merge request and added a new commit. Everything else looks great, so I'll proceed to merge this and create a new minor release tag.

🇮🇳India abhisekmazumdar India

Thank you for the detailed review and for fixing the bugs. I have now merged the MR.

🇮🇳India abhisekmazumdar India

New changes done:

  • My bad, the check of the old and new secret wasn't behaving well. I break it down and added proper checks.
  • Also moved the config under web services where it made more sense.
  • Made improvements to the consistency for the "mautic api connection" to "Mautic API Connection".
🇮🇳India abhisekmazumdar India

In the overview it might make more sense in the status to show the result of the connection test.

I didn't add that because the status is not stored in the entity(database). To obtain the status of an API, we need to load the entire entity and make request for status all the connection entities. This might become problematic as we start to have more connection entities in the overview listing.

I just replaced the status with the site URL, which makes it easier to identify.

  • I made the error handling better.
  • Clear value if the auth method is toggled
  • Rename the oauth to the said public and secret keys
  • moved the status message in the top and just shows error msg doesn't show WSOD

All the other MR inline feedback has also been resolved now.

🇮🇳India abhisekmazumdar India

This task is based on this MR from mautic_api

🇮🇳India abhisekmazumdar India

This work is based on this related issue.

🇮🇳India abhisekmazumdar India

Thank you for the quick response.

🇮🇳India abhisekmazumdar India

abhisekmazumdar changed the visibility of the branch 3527905-multi-instance-mautic-connection to hidden.

🇮🇳India abhisekmazumdar India

abhisekmazumdar changed the visibility of the branch 2.0.x to hidden.

🇮🇳India abhisekmazumdar India

abhisekmazumdar changed the visibility of the branch 3527905-implement-multi-instance-mautic to hidden.

Production build 0.71.5 2024