- Issue created by @valthebald
- 🇩🇪Germany marcus_johansson
I think that if this service should exist it has to exist in the ai_translate module, because if we start adding combinations of operation types based on what they solve rather on how they solve it and want to be consistent, there will probably be 100s of combinations in the AI core module, which will be impossible to manage.
Having a dedicated third party module or third party service for that makes more sense in that case.
- 🇬🇧United Kingdom scott_euser
Adding related issue; depending on where this issue lands, we could leverage ai_translate more.
- 🇩🇪Germany marcus_johansson
@valthebald - code review is done from my side, the ai_translate.settings check and the setChatSystemRole method is important. Rest is optional if you think they are improvements, otherwise just skip.
I'll let Frederick do the feature review since he knows the module better, but from what I see in the code, it mostly works the same.
- 🇧🇬Bulgaria valthebald Sofia
Fixed most of the comments (2 left are for the future).
Also, brought back editing of default translation prompt (patch to have per-language left it hidden) and display of default prompt that has a condition - First commit to issue fork.
- 🇫🇷France nicolasgraph Strasbourg
Thanks @valthebald for this MR!
I pushed and revert a commit in favour of this comments:
- In ai_translate.install, is
ai_translate_requirements()
still required? I guess not. - In ai_translate.module,
_ai_translate_check_default_provider_and_model()
seems also outdated to me. - In AiTranslateForm.php, line 114,
$default_model = $this->providerManager->getSimpleDefaultProviderOptions('chat');
should not use the chat provider type anymore.
- In ai_translate.install, is
- 🇬🇧United Kingdom scott_euser
I added some feedback as well after testing this in ✨ Fallback to ai_translate configuration Active
There are also some comments from Marcus to be addressed stillIn general though it seems like the right direction, and is really helpful to have, thank you for the efforts here!