🇫🇮Finland @jhuhta

Maybe we should also bump the version of this module to 1.2.x-dev.

jhuhta → changed the visibility of the branch 3552969-refactor-azureprovider-to to active.

jhuhta → changed the visibility of the branch 3552969-refactor-azureprovider-to to hidden.

jhuhta → changed the visibility of the branch 3552969-refactor-azureprovider-to to hidden.

jhuhta → created an issue.

Let's say that when the currently open issues are closed, it's time for a stable release. So these:

🐛 Improve error handling for failed translation API requests Active
✨ Use glossary feature Active
🐛 Plugin fails to translate when using regional variants in source language (e.g., zh-hans) Active

Thanks for the report and patch. I added the missing use statement and created the MR which I immediately merged following the passing tests.

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

Looks good, thanks @ilianoz! Would you @mach3.zone be able to test and RTBC this?

Bumped into the same problem running the AI module's explorers, and #2 fixes it for me too.

jhuhta → created an issue.

Got an okay from @marcus_johansson on this in DrupalCon Vienna, so merged. Thanks @cmd87!

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

This was fixed in ✨ Azure provider missing tool/function support available in OpenAI provider Active . Thanks anyway!

Good call, thanks! Merged to 1.1.x.

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

Do I get it right: you use Ocp-Apim-Subscription-Key header for the API key, or is it an extra header for another key besides the API key?

Asking because there is a field in settings for the custom API key ("Type of model", a bit poorly named, perhaps). Choose Other/Custom to be able to name your API key header.

This seems to be fixed in both 1.0.x and 1.1.x branches. Please reopen if still relevant.

+1 for the fix

Confirmed the problem and the fix, and merged. Thank you! I'm not sure of the default of 1536 though, for me default was the double of it, 3072.

However, if users have configured something other than the default of their provider, this will change the behavior for them. A note is probably needed in the release notes.

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

Thank you for the contribution!

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

Is this still relevant for 2.0.0-alpha2?

Still, I prefer not adding direct support for other contrib modules and various field types they provide. I'd suggest to have another approach and add a hook mechanism for this module to provide the support with custom code.

Another approach could be to use the new Field Widget Actions module, with which you might be able to achieve the similar functionality.

The proposed approach is otherwise nice, but it would create a dependency to the - not necessarily otherwise enabled - ai_translate module, which is not even listed in the dependencies.

What I suggest is to use an exception that is available - the AI core module is listed as a dependency already.

Maybe this needs just some more feedback that it actually works, and works the right and sensible way. Once we release a stable version, changing the API becomes way more difficult.

We have been using this in production for a while, and I can almost promise that if there will be breaking changes, there will be an upgrade path.

As discussed in DrupalCon Vienna with @mach3.zone, this would need more work, as there needs to be support for glossaries for all enabled languages.

Thanks! Merged and credited.

Any change to make it a merge request?

Thanks for the report and patch! @ralkeon could you test if @ilianoz's patch fixes the bug for you, and mark this issue RTBC? It might speed things up.

I can't really investigate this further with no more info as I can't reproduce it myself. Hence, closing.

To your question, DeepL API doesn't provide any chat-like operation, so in a sense it is somewhat different a thing from the most LLM providers. It just translates, text in, text out. For this purpose we created the text translation operation type to the AI core module. So no, to my understanding the chat (or chat with complex json for that matter) operation type can't be provided, and AI Automators, which relies heavily on the chat operation type, can't be supported.

However, please correct me if I'm wrong. And obviously I'd gladly include new, contributed features to provide support for wider use cases. But that's a matter for another issue.

Can't reproduce, but the module is successfully uninstalled when I try it. Can you provide more information?

Thanks, great! Adding credits & merging.

#9: I think we should inject the language manager instead of calling \Drupal::languageManager() if this change is necessary. And continue from the previous merge request too.

I had to do an identical change to get the update process to work, before noticing there is a pending MR already. So, RTBC'ing this one. The bug breaks the drush process similarly.

Seems like my name/nick in the readme is triggering the spelling error. Could you fix those still so that everything is green?

Rebased. This works and helps getting rid of double loaded js-cookie library by removing the core asset. I can't RTBC this now as it's my rebase but still.

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

Rebased.

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

Thanks for the feedback! Would you be so kind and provide the fix as a MR? I don't have much time to work for this right now, so that would make it easier for me and faster for you to get a working solution (+ credit).

Just wondering if the code should try the default chat provider too, if none is configured.

The instructions for configuring the module are stated on the module front page.

How about this one? ✨ Support chat operation type AI providers Active

Just install the latest dev version. I can make a new release if someone could tell that it seems to work. (It works on our project pretty well though.)

I haven't had time to actually test it and check it properly. What I'd like to avoid though, is hardcoding contrib provided field types there - maybe either have it as an extendable structure or then get rid of of the thing completely and instead try to recognize if this or that field could have the feature enabled. The form mode config form shouldn't add the third party settings to every single existing field though.

Interesting. I don't think we should add direct support for various field types names provided by other contrib modules, but instead do something more generic, such as provide a hook or plugin type of approach that could add support for such so that module users could plug this into new field types.

The problem might be to know beforehand what kind of field processing would be needed in processElement to enable this kind of pluggable solution. But do you get my idea? Would it be something that you could work on? The plugin approach could be better in a sense that this module could already include plugins for various field types. Or, if it is possible to write the code so that it already supports many kind of fields automatically, then a hook to alter the list of supported field types would suffice. Allowing to add the button for just type of field widget would probably be a bit risky?

Rebased both branches. For 11.x, 📌 [ignore] Convert everything everywhere all at once Active (I wasn't familiar with) forced me to move a change from comment.module to src/Hook/CommentHooks.php. Hopefully it's ok now.

I'm not sure if the patches above contain all the same changes.

Crediting @pontus.talvikarhu for testing & review.

jhuhta → created an issue.

jhuhta → created an issue.

Thanks! Crediting @mitechworld for idea and testing.

Please RTBC this if you are sure it works - and doesn't break any existing functionality. I can create a new release at some point so that you can get rid of the patches.

See if the code in the MR would work for you.

It would be fairly easy to add a configuration option with a multiple select like "Offer the translation feature for these languages only".

Not against adding such a feature - I'd be happy to add it if you provide the patch.

The MR fixes the issue for me. See if there's problems in the approach or code.

Included also two unrelated minor things:
1) The delete button copy ("delete: 'Delete Model'") got saved to the settings, which I removed.
2) The Edit button text was misleading: "Edit Model", when the user was already editing and was about to save it.

Does it need an update hook?

jhuhta → created an issue.

Crediting @wouters_f for the idea and support to make this happen.

It also required writing a new operation type for the AI module → and a provider module for DeepL → .

Fixed in 2.0.x branch.

Looks good, merging.

The code looks ok by reading. Do you have these changes running on your project already?

Ok, so my work here should be done.

Just FYI, I published a simple DeepL provider module for the AI ecosystem right now: https://www.drupal.org/project/ai_provider_deepl → .

My translation module ( https://www.drupal.org/project/ai_translate_textfield → ), which has different approach than the AI Translate module, will be using it soon.

Should be ready now.

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

I'll just change this to feature request, which category probably better fits the request.

Happy to include the change, and even happier if there's a contribution! So the exception needs to be passed on and caught on a later phase, to be able to pass the error message.

Let's have this in and make another release once it's there.

And now you have the alpha release too, thanks for requesting that!

Nevermind, I gave it a try and it seemed to do the trick. Merging.

Nice, thanks! Would you get someone (ie. from your project team) to test the feature and RTBC this issue? It would speed things up so that I can merge this. And you're really using this module, I'd be happy to make a release too.

Ok, case closed then, a full release won't be done. I'll deprecate this one when I have time to replace it myself from a live project.

For the entity select need if yours, I'm quite sure there's a module for that, too, and if not, it's not difficult for you to write one.

Hi @Anybody and thanks for asking! I'd gladly make this a real module and add you as a co-maintainer. But why the rename? This is not about a select element (such as ie. those term select elements) but just to show an entity on the form.

By the way this module looks a lot like https://www.drupal.org/project/webform_entity_view → (which didn't exist when I originally wrote this) by its description, but is this one more suitable for your needs?

The changes look good and the touched functionalities still work, given that ExportTermsForm is ignored as @sokru said.

Phpstan and the likes will be addressed separately I assume.

jhuhta → created an issue.

This patch fixes the issue for us. I can make an MR out of it if the maintainer thinks this makes any sense.

jhuhta → created an issue.

I think the first option is not that good as the Azure OpenAI API is somewhat different and may diverge even more. I noticed this when trying to use the openai-php/client library in a custom chat solution: the structure of a response message was slightly different and the library couldn't process it as it wrongly assumed an array key existing - I don't remember which one exactly though.

I asked if the library could support Azure OpenAI too but haven't gotten a reply yet: https://github.com/openai-php/client/issues/320

Now that I plan to start using this module too, I'd need a provider that would supports at least the chat operation to begin with. Currently that would require reinventing the wheel for the client library, if I'm not mistaken.

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

The suggestion sounds very reasonable. By the time of writing this module, the need was to make it as simple as possible and match the requirements from the client. Writing the LLM client appeared to be so simple that adding a dependency to the AI module ecosystem felt a bit overkill, but I'm ready to reconsider that. A working MR would make the change easier. An upgrade path would also help.

But now this current implementation provides also a possibility to include service provider specific processing, which I'm not sure would be possible.

Added credit to @akseli.miettinen for report, debugging, review and test.

Does adding a config option solve the problem with the code from https://git.drupalcode.org/project/ai_translate_textfield/-/merge_reques...?

    language_fallbacks:
      en: en-GB

It seems DeepL really doesn't support 'en' anymore(?) (https://developers.deepl.com/docs/api-reference/translate/openapi-spec-f...). I'll add a language fallback config option for the module for this case, as the language code from Drupal, which the module uses ('en'), doesn't necessarily include the full locale name.

xjm → credited jhuhta → .

Confirming the problem.
I have a list of path aliases like these:
/en/english-alias
/fi/finnish-alias
/sv/swedish-alias

And I'd like to create Url objects of them, removing those that point to other translations than the one of active language. But Drupal don't seem to consider those ones with non-active language prefix routable urls, even though the creation function has been explicitly provided with the correct language option.

Following the code with debugger, it seems getRouteCollectionCacheId() doesn't use the provided language anymore, but uses the current language instead. Which leads to null result.

It is not intended, we just didn't have that need and thus time to test it. I'd be happy to enable it if you'd like to have it. Would you send a patch?

Great, thanks for the contribution! It works and looks good so merged.