🇧🇬Bulgaria @valthebald

@efpapado: oh, I didn't realize that.
Still, I'd prefer the approach in https://git.drupalcode.org/project/ai/-/merge_requests/696 - I don't think it's the site builder who decides what is the translation flow (i.e. is the AI-translated page gets published), but the site administrators.

valthebald → created an issue.

kristen pol → credited valthebald → .

@efpapado: I'm afraid your approach creates an option to permission bypass, i.e. by manually adding 1 to the link, user will be able to publish an entity even if they don't have access to

I have the same behavior as @anjaliprasannan

@svendecabooter definitely related! Thanks for pointing out

valthebald → created an issue.

Merged to 1.1.x and 1.2.x, thank you all!

valthebald → created an issue.

To summarize the discussion in Slack, it would be good to not hard code the skip list of possible column names to (delta, parent, etc.)
Possible solution to check special key of "_translatable_columns" (name discussable, could be "_translatable_fields" or simply "_columns")
in TextExtractor::extractTextMetadata():

        foreach ($fieldMeta as $meta) {
          $fieldParents = [$field_name];
          if (isset($meta['delta'])) {
            $fieldParents[] = $meta['delta'];
            unset($meta['delta']);
          }
          if (isset($meta['parents'])) {
            $fieldParents = array_merge($fieldParents, $meta['parents']);
          }
          $meta['parents'] = $fieldParents;
          if (!isset($meta['_columns'])) {
            $meta['_columns'] = ['value'];
          }

and then loop over "_columns" in AiTranslateController::translateSingleField()

Merged to 1.2.x and 1.1.x, thank you all!

valthebald → created an issue.

LGTM

@all: does it mean we're good to go? This seems to be the last blocker to release 1.1.0

LGTM
I have searched the codebase for "\Hook" and found only one hook class other than ai_content_suggestions - ai_test.

Should there be a separate issue for it?

valthebald → created an issue.

Committed to 1.x, thanks everyone!

I believe this is solved by https://git.drupalcode.org/project/ai_provider_google_vertex/-/commit/97..., can we close the issue?

valthebald → made their first commit to this issue’s fork.

valthebald → created an issue.

Actually, it is possible to perform the check earlier and avoid the ReferenceFieldExtractor::extract() call

@lakhal: thanks for reporting this. The issue is caused by ReferenceFieldExtractor assuming that in the entity reference field, referenced entities are (fieldable) content entities, which is not the case for config entity Domain.

Possible solution is to check that $subEntity is instance of ContentEntityInterface before calling extractTextMetadata($subEntity)`:

    foreach ($entity->get($fieldName)
      ->referencedEntities() as $delta => $subEntity) {
      // Intentionally check each sub entity because of https://www.drupal.org/project/drupal/issues/2407587
      if (!($subEntity instanceof ContentEntityInterface)) {
        continue;
      }
      foreach ($this->textExtractor->extractTextMetadata($subEntity) as $subMeta) {
        $textMeta[] = ['delta' => $delta] + $subMeta;
      }
    }

Also, if you switch from 1.0.5 to 1.1.0-beta, you can disable domain translation and avoid this issue (go to Administration/Content types//fields/:

Version 1.1 is not BC with 1.0.5, but there will be an upgrade path.
If you start building a WIP, better start with 1.1-beta

@anjaliprasannan I've added some code style comments

valthebald → made their first commit to this issue’s fork.

Frankly, if this is the only issue blocking the release, happens only under certain conditions and only with VDB/RAG enabled, I would move it to 1.2.x and have the release without the fix.

In AiAssistantForm.php around line 239:

      $form['agents_enabled']['agents_agent'] = [
        '#type' => 'checkboxes',
        '#title' => $this->t('Agents to use'),
        '#options' => $agent_options,
        '#default_value' => $tools,
        '#description' => $this->t('Select which agents to use for this plugin.'),
      ];

      // Only show if AI search is enabled.
      if ($this->moduleHandler->moduleExists('ai_search')) {

there should be similar check if ai_agents module is enabled (same check as for ai_search module in the code fragment above).
Otherwise, there is an empty "Agents to use" form element.
I'm reviewing the MR and will add comments in gitlab if I see something else.

valthebald → created an issue.

I was thinking... can we combine the best of 2 worlds?
How hard would it be to create a form for dynamically creating a tool as a Drupal plugin?
Conceptually, this could be similar to what XB does with the option to create a component on the fly, only much-much simpler.
The form in question could contain a dynamic list of parameters (name/type/description) + the body field
The body could include PHP code (simple but potentially dangerous option) or some form of pseudo code - twig/ECA model

valthebald → created an issue.

Adding to merge train, thank you!

Hi @aolivera since you're still working on this (I see the status is "Needs work"), can you check some more things please:

1. I see that Google consistently uses the term "data store" but not "datastore"
2. In the new form element #description, is it possible to add a link to documentation what "data store" is? Maybe https://cloud.google.com/generative-ai-app-builder/docs/create-datastore... but if you have a better link that's fine
3. OK if I close MR#8?

valthebald → created an issue.

This was handled by 📌 Discuss: Unify translate operations over ai Active , so I dare mark this closed

Awesome! I think we're good to go

@prabha1997:

I feel that the safest would be to discard the changes except the one in AiTranslateController.php

My experience is the same as of @ronttizz
After some debugging, the issue is caused by the way translation is created in AiTranslateController::insertTranslation():

  public function insertTranslation(
    ContentEntityInterface $entity,
    string $lang_to,
    array &$context,
  ) {
    $translation = $entity->addTranslation($lang_to);

compared to how it is done in core's ContentTranslationController::prepareTranslation():

    $target_translation = $entity->addTranslation($target->getId(), $source_translation->toArray());

i.e. in AI, translation is created with empty defaults, while in core it is prepopulated by the source entity values.

Adding the source values in AI

  public function insertTranslation(
    ContentEntityInterface $entity,
    string $lang_to,
    array &$context,
  ) {
    $translation = $entity->addTranslation($lang_to, $entity->toArray());

solves the issue for me. Can someone confirm if this works?

Heads up: https://github.com/google-gemini-php/client/pull/92 is merged into main

jurgenhaas → credited valthebald → .

@marcus_johansson I see that MR is already against 1.1.x, changing issue version

@ishani patel Thanks!

Does this mean we can close the issue?

Can this be reapplied to 1.1.x please?

Can this be reapplied to 1.1.x please?

That change would make sense. Merge request is welcome!

Looks like the only remaining issue is ✨ Add the actual agents as configs Active ?

PHP port of python MCP SDK https://packagist.org/packages/logiscape/mcp-sdk-php allows something similar:

https://github.com/logiscape/mcp-sdk-php/blob/5885e4cad09eae9662f9c8e3b7...

 // Create schema with properties and required fields
    $inputSchema = new ToolInputSchema(
        properties: $properties,
        required: ['num1', 'num2']
    );

    // Create calculator tool
    $calculator = new Tool(
        name: 'add-numbers',
        description: 'Adds two numbers together',
        inputSchema: $inputSchema
    );

$server->registerHandler('tools/call', function($params) {
    $name = $params->name;
    switch ($name) {
        case 'add-numbers':
...

@marcus_johansson: I'd vote for 1.2.x as a target then

I wonder if that's too late to include this feature to 1.1.0
I agree this is a great-to-have, yet having 1.1.0 release is overdue now, can this be postponed to 1.2.x/2.0.x?

Linked 📌 Use Drupal plugin system with OperationTypes Active as a reference, marking RTBC

valthebald → created an issue.

No feedback for several days, merging

Looks good to me code-wise.
I miss some explanation on why this changes is needed in the first place, but this can be added in the follow-up issue when it's created.
Pending creation of 2.0.x/2.x branch to do this

Actually, my bad, translations work. @prashant.c can you review the CI bot comments?

After applying the patch, I see full list of operations in the settings form, but have no translate with ai links in node//translations

Having a simple autoloading class for the sake of autoloading sounds fair

valthebald → created an issue.

Alright, so this worked. It is now possible to create a Translation model in Vertex provider settings form /admin/config/ai/providers/google_vertex:

(on a side note, Vertex provider configuration is relatively complex and would win of a better documentation/examples)

Created first working version. Vertex native translation supports 2 modes: Translation LLM (highest quality, limited to 19 languages) and NMT (fastest, works with larger set of languages), see https://cloud.google.com/vertex-ai/generative-ai/docs/translate/translat... for details.
MR in its current state supports only Translation LLM.
Results are promising (as in translation is really fast and has the quality expected from Google translation), but I'd like to support NMT as well before marking MR as ready for review

I'll go on and merge this

I added hook_update_N() just in case

In order to provide native translation support, VertexProvider needs to implement Drupal\ai\OperationType\TranslateText\TranslateTextInterface (which has only one translateText() method)
Implementation can use its own endpoint, different from chat()

What if, to avoid confusion, we limit selection of keys in provider form to only file-based? Suggested MR does just that, and changes a little description of the key select to

Choose an available key. If the desired key is not listed, create a new key. Important: Key must use "File" provider and point to the JSON credentials file. Read more at https://cloud.google.com/docs/authentication/application-default-credent....

One thing to decide is whether we need an update hook to switch from config/environment key to file based for existing installations.

I see 2 parts here:
1. Although Vertex authentication should be done via JSON file, keys of type "File" do not work. This is a bug that is easily fixable in VertexProvider::getAccessToken() - which should try to unserialize the key before creating ServiceAccountCredentials
2. (working) keys of type environment/configuration should be documented better. This part specifically?

Could explain that the key should be in fact the path to the JSON file (which is a bit controversial)

borisson_ → credited valthebald → .

@aolivera: thank you, fixed!
Just a small note for the future: it's better to use branch that is named after the issue number (i.s. 3506263-something) instead of 1.0.x on a separate git origin - that makes local distinction easier

Adding contribution

@jibla: it turns out that function calling is not yet supported by google-gemini-php. There is a PR https://github.com/google-gemini-php/client/pull/67 against beta branch, but not merged yet.

Should we park the issue until PR is merged, or switch to vanilla Guzzle requests as suggested in related Google Vertex issue https://www.drupal.org/project/ai_provider_google_vertex/issues/3506263 🐛 Vertex provider uses PredictionServiceClient and slows down response time. Active ?