Sofia
Account created on 11 February 2008, over 17 years ago
#

Merge Requests

More

Recent comments

🇧🇬Bulgaria valthebald Sofia

@prabha1997:

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

🇧🇬Bulgaria valthebald Sofia

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?

🇧🇬Bulgaria valthebald Sofia

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

🇧🇬Bulgaria valthebald Sofia

Does this mean we can close the issue?

🇧🇬Bulgaria valthebald Sofia

That change would make sense. Merge request is welcome!

🇧🇬Bulgaria valthebald Sofia

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

🇧🇬Bulgaria valthebald Sofia

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':
...
🇧🇬Bulgaria valthebald Sofia

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

🇧🇬Bulgaria valthebald Sofia

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?

🇧🇬Bulgaria valthebald Sofia

No feedback for several days, merging

🇧🇬Bulgaria valthebald Sofia

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

🇧🇬Bulgaria valthebald Sofia

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

🇧🇬Bulgaria valthebald Sofia

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

🇧🇬Bulgaria valthebald Sofia

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

🇧🇬Bulgaria valthebald Sofia

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)

🇧🇬Bulgaria valthebald Sofia

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

🇧🇬Bulgaria valthebald Sofia

I added hook_update_N() just in case

🇧🇬Bulgaria valthebald Sofia

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()

🇧🇬Bulgaria valthebald Sofia

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.

🇧🇬Bulgaria valthebald Sofia

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)

🇧🇬Bulgaria valthebald Sofia

@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

🇧🇬Bulgaria valthebald Sofia

@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 ?

🇧🇬Bulgaria valthebald Sofia

As @andypost mentioned in #7, the issue is about 2 variants of transcribing "щ" character that are found in the core.
From several sources I've checked, "shch" variant is more "favourable", so I'd suggest use it consistently in all places

🇧🇬Bulgaria valthebald Sofia

Going to work on this during DDD25

🇧🇬Bulgaria valthebald Sofia

Bumping the target version

🇧🇬Bulgaria valthebald Sofia

Bumping the version + MR is not mergable at the moment

🇧🇬Bulgaria valthebald Sofia

@vivek panicker: I tested with core 10.4 and ai 1.0.x, and it worked for me. Can you provide more details about your configuration? Are variation fields marked as translatable with AI? Another place to check is AI translate settings /admin/config/ai/ai-translate, especially entity reference section

🇧🇬Bulgaria valthebald Sofia

@marcus_johansson can this be merged to 1.1.x as well please?

🇧🇬Bulgaria valthebald Sofia

@vivek panicker: I have asked ChatGPT and Gemini to analyze and find possible grammatical mistakes in the source code of ai_agents, and here are some other places to fix/improve:

  1. AiAgentExplorerController.php line 84: "Check if its a json response." Should be "Check if it's a JSON response." (capitalize JSON and add an apostrophe
  2. line 137: "The actual task, will support files later." should be "The actual task will support files later." (unnecessary comma)
  3. Drupal\ai_agents\Attribute\AiAgent.php line 11 "The ai provider attribute" AI should be capitalized.
  4. Drupal\ai_agents\Form\AiAgentPromptChanger.php line 13 "Configure on AI Agent." sounds confusing and misleading

I wonder if those should be handled in the same or separate issue?

🇧🇬Bulgaria valthebald Sofia

@mrdalesmith: simple and effective! Nice catch, marking RTBC

🇧🇬Bulgaria valthebald Sofia

Bumping the version + suggest more generic approach

🇧🇬Bulgaria valthebald Sofia

Changed the documentation of translate text call

🇧🇬Bulgaria valthebald Sofia

@mrdalesmith: The issue this patch tries to solve: out of the box, with AI module alone or AI recipe in Drupal CMS), users have "Translate text" operation type on the settings screen, and the list of providers that implement this operation type, is empty.

Which is confusing, and the most frequent question I get from the new users trying to use AI translation capabilities, is why they cannot use OpenAI or Claude for translation.

By "any other you might be using", I am aware only about deepl implementing "Translate text" operation, and this is not mentioned on the AI settings screen.

🇧🇬Bulgaria valthebald Sofia

Closing since meetup was cancelled

Production build 0.71.5 2024