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 ?
valthebald → created an issue.
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
Going to work on this during DDD25
valthebald → created an issue.
Hello :)
kristen pol → credited valthebald → .
kristen pol → credited valthebald → .
ericgsmith → credited valthebald → .
Bumping the target version
valthebald → created an issue. See original summary → .
Changed the version
Merged, thank you!
valthebald → created an issue.
Can this be merged to 1.1.x as well?
Bumping the version
Bumping the version + MR is not mergable at the moment
valthebald → created an issue. See original summary → .
@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
valthebald → created an issue.
@marcus_johansson can this be merged to 1.1.x as well please?
@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:
- 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
- line 137: "The actual task, will support files later." should be "The actual task will support files later." (unnecessary comma)
- Drupal\ai_agents\Attribute\AiAgent.php line 11 "The ai provider attribute" AI should be capitalized.
- 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?
@mrdalesmith: simple and effective! Nice catch, marking RTBC
Bumping the version + suggest more generic approach
Changed the documentation of translate text call
@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.
valthebald → created an issue.
Closing since meetup was cancelled
Passes now