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

Merge Requests

More

Recent comments

🇧🇬Bulgaria valthebald Sofia

Merged to 1.2.x, thank you @svendecabooter!

🇧🇬Bulgaria valthebald Sofia

I checked fresh install, as well as update from 1.2.x-dev, translations work as expected.
I have some thoughts about optimizing prompt selection UI, but this is definitely not a release blocker.

🇧🇬Bulgaria valthebald Sofia

I tried a fresh reinstall of ai from the latest 1.2, and then switched to the feature branch.
This revealed another issue - prompt type should be installed (if not exists) in hook_update as well. Otherwise there is
"No AI Prompt Types exist. These are normally provided by modules within the AI Ecosystem" error when trying to edit a prompt

🇧🇬Bulgaria valthebald Sofia

Update hook worked like a charm, thank you @svendecabooter!
No strong opinion here, but do we need to keep both default and modified prompts? In the end, we have the default one in module's install folder for the reference

🇧🇬Bulgaria valthebald Sofia

That greatly depends on the target release date for 2.0 and planned features that haven't found their way into 1.2.0.
There is sense in 1.3.x if all the following is true:

  1. We can't do all the refactoring/basing on symfony components in ~3-4 months
  2. Within the same 3-4 months period, there features that are an easy and/or big win, but break BC
🇧🇬Bulgaria valthebald Sofia

LGTM

🇧🇬Bulgaria valthebald Sofia

valthebald created an issue.

🇧🇬Bulgaria valthebald Sofia

Merged to 1.2.x, thanks @nikro!

🇧🇬Bulgaria valthebald Sofia

@marcus_johansson: Core has https://www.drupal.org/about/core/policies/roles-and-responsibilities/pa... exactly for that - pay respects to people who were maintainers, but have stepped down

🇧🇬Bulgaria valthebald Sofia

Took a bit longer than 30 minutes, but PoC is fully working (see the MR). Summarized changes:

  1. Remove thread_id from page' drupalSettings
  2. Removed thread setting logic from DeepChatApi::setAssistant() (good that there is already DeepChatApi::setThreadsId()
  3. Move historicalMessages() method from DeepChatFormBlock to DeepChatApi (because history is AJAX-loaded)
  4. Create JS class deepChatSession that handles all the logic of initialization/retrieving the history/resetting etc.
  5. Every instance of deepChatElement in deepchat-init.js gets an instance of deepChatSession
  6. Changed input and output parameters of DeepChatApi and ResetSession controllers

Open issues/questions

  1. Current approach (before or after switch to local storage) assumes there is only one chat session on the page. In theory, it is possible to pass unique storage ids to deepChatSession instances, thus working with different assistants/thread. Is this needed?
  2. Is there a need in anonymous session protection other than random 20-character string? Maybe store also user ID, to protect authenticated sessions from hijacking? Can currently unused deepChatSession.secret variable be of any value?
  3. Should the history be stored in local storage (all of it or limited number of recent messages)?
🇧🇬Bulgaria valthebald Sofia

valthebald created an issue.

🇧🇬Bulgaria valthebald Sofia

@svendecabooter: thank you, merged!

🇧🇬Bulgaria valthebald Sofia

valthebald created an issue.

🇧🇬Bulgaria valthebald Sofia

Here's the concept (in MR 907):

1. Bind thread_id to a random secret string, stored on client side
2. Do not store thread_id in drupalSettings (cache killer!). Instead, use chatSettings JS object, serialized to browser's storage
3. When there is no thread_id in chatSettings, call getSession() to generate fresh one.

The code in its current condition works and doesn't start a session.
What's missing is security check (need to prevent session hijacking)

🇧🇬Bulgaria valthebald Sofia

Changing back to Needs review for wider visibility

🇧🇬Bulgaria valthebald Sofia

LGTM as well

🇧🇬Bulgaria valthebald Sofia

There are some merge conflicts, I wonder if it'd be easier to open a new MR or fix them?

🇧🇬Bulgaria valthebald Sofia

@lpeabody:

Just to be clear, a session cannot exist at all if the page cache is to be used.

Totally, that's why session/local storage accompanied by expirable key/value storage

I will push to the issue branch later today

🇧🇬Bulgaria valthebald Sofia

@svendecabooter: I was wondering the same question!
While I don't know what is the "desired" behavior, the reason I decided to go with the OR logic is that it seems more flexible, i.e. you may want to give certain roles permission to only create translations using the core flow, and some other roles to only create translations using AI.

🇧🇬Bulgaria valthebald Sofia

As an alternative to browser storage (local or session), here's another trick that was used in a semi-decoupled Drupal project: the whole conversation is stored on server side (as it is today), and there's conversation id that is stored in session/local storage of the browser. It is also possible to protect the conversation with randomly generated secret, stored by KeyValueExpirableFactory service.
That way:

  • Nothing is stored in the session, so page cache is not broken
  • Conversation is protected with a mechanism similar to session generation in Drupal
  • Conversation itself is not stored on client side, so lower chance to face clientStorage limitations
🇧🇬Bulgaria valthebald Sofia

Closing as outdated, no evidence this bug exists in 1.2.x, and no feedback from original reporter

🇧🇬Bulgaria valthebald Sofia

This fix accompanies the one from AI Translate Module Leaves Fields Empty When No Translation is Found Active .
I have search the code base and don't see any calls to addTranslation() that don't get values from original entity.
Thanks everyone, merged to 1.2.x!

🇧🇬Bulgaria valthebald Sofia

Starting review

🇧🇬Bulgaria valthebald Sofia

@mkalkbrenner incubator module is a good mode, reminds me of solrlog that found its way into searchapi_solr :)

🇧🇬Bulgaria valthebald Sofia

After a quick check of https://git.drupalcode.org/project/permissions_by_term/-/tree/3.1.x-dev/..., there's no file AutomatorPluginDeriver.php
Which version of permission_by_term do you use? Any chance there was local override?

🇧🇬Bulgaria valthebald Sofia

Setting the component

🇧🇬Bulgaria valthebald Sofia

@svendecabooter: it's always a tricky question for contrib-contrib plugins :)
I personally don't have a strong opinion here

🇧🇬Bulgaria valthebald Sofia

PHPCS reports some code style issues, please check https://git.drupalcode.org/issue/ai-3541883/-/jobs/6569528

🇧🇬Bulgaria valthebald Sofia

LGTM and the test bot is happy

🇧🇬Bulgaria valthebald Sofia

valthebald created an issue.

🇧🇬Bulgaria valthebald Sofia

valthebald created an issue.

🇧🇬Bulgaria valthebald Sofia

Option to set the default state for the new translation was already added

🇧🇬Bulgaria valthebald Sofia

@anmolgoyal74: that's a good proposal! Do you want to create a merge request, or someone else can?

🇧🇬Bulgaria valthebald Sofia

I've reworked original idea, and added the logic to hide "AI translations" column when user doesn't have permissions or all languages are already translated

🇧🇬Bulgaria valthebald Sofia

I have added "beta blocker" tag to all the mentioned issues, feel free to add tag/untag/add more issues

🇧🇬Bulgaria valthebald Sofia

@yuri.p: common flow is to target the current default branch (1.2.x), and then, if needed, backport to previous one (1.1.x)
There's almost no chance new commits will be added to 1.0.x

🇧🇬Bulgaria valthebald Sofia

@IliaNoz: any reason you changed the target version from 1.2.x to 1.1.x?

🇧🇬Bulgaria valthebald Sofia

@IliaNoz: any reason you changed the target version from 1.2.x to 1.1.x?

🇧🇬Bulgaria valthebald Sofia

Lowering down the version, since we already have enough issues in the work, and it would be good for people to understand which issues go where

🇧🇬Bulgaria valthebald Sofia

This is fixed in Content moderation options in entity translations Active . If you want this feature to be 1.1.x, please provide a backport

🇧🇬Bulgaria valthebald Sofia

@dmundra: thanks for the update! Checking soon

🇧🇬Bulgaria valthebald Sofia

I've made several commits to the repo with the following results:

  • Added cache tags to the calendar view. That fixed stale cache when issues are added/updated
  • Added d.o. userid field to contributor content type. This reduced the number of calls to the API
  • Added static caching of user and node data when processing a project
  • Added drush commands to process single module and all modules
  • Process single module configuration using Drupal queues. First thing that is done by drush command is populate the queue, so even if the drush command fails for any reason, the queue can be processed later
  • Handle "429 Too many requests" responses from API (up to 3 retries with 30 seconds delay before finally giving up)
  • Added contributor now shows in the dashboard

Regarding timeouts, the proper way to overcome this would be to run the imports in scheduled cron jobs, but unfortunately, DF does not support that yet.

🇧🇬Bulgaria valthebald Sofia

@anjaliprasannan: I could reproduce the issue locally with the latest 1.2.x, and your patch fixes it.

Merged to 1.2.x, thank you!

Production build 0.71.5 2024