Een hart voor de Drupal Community

svendecabooter created an issue. See original summary.

Added patch file for the current state of the MR, for composer based patching workflows.

I added a MR to add this functionality. Whether or not the Provider Configuration fieldset is open by default or not, is managed via config. The default setting for this config is to remain open, to keep backwards compatibility.

svendecabooter made their first commit to this issue’s fork.

EDIT: seems to be unrelated to this patch, because I'm getting the same errors when patch is not applied.

Tested with OpenAI, and getting the following errors:

Warning: Undefined array key "fids" in Drupal\media_library\Form\FileUploadForm->validateUploadElement() (line 212 of /var/www/html/web/core/modules/media_library/src/Form/FileUploadForm.php)

ypeError: count(): Argument #1 ($value) must be of type Countable|array, null given in count() (line 212 of /var/www/html/web/core/modules/media_library/src/Form/FileUploadForm.php).

Haven't investigated the root cause yet...

All code review issues have been fixed, and extra explanation has been provided. Setting this to Needs Review.

Thanks for the bug report, MR and review! Merging now.

Sven Decabooter added a comment on drupal.org: sourceEditing plugin no longer gets enabled automatically in 1.1.x

Added a patch file, to be used in Composer-based patching workflows.

Sven Decabooter added a comment on drupal.org: sourceEditing plugin no longer gets enabled automatically in 1.1.x

svendecabooter created an issue. See original summary.

Problem/Motivation

Issue #3499702: AI CKEditor requires the Source CKEditor button added logic to automatically enable the CKEditor5 'sourceEditing' plugin, in the response text textarea, since this is needed for the Completion plugin to work.

However, in branch 1.1.x most of the logic is still there, except the flag to enable this functionality on the response_text text_format form element. This seems to be missing, probably because this is now handled in the generic AiCKEditorPluginBase class.

Steps to reproduce

- Use AI module 1.1.x - Enable the AI CKeditor module - Enable the Completion ("Generate with AI") plugin - Do NOT add the source code button to your CKEditor5 toolbar by default - Try using the Completion plugin in your CKEditor5-powered text area - this will fail (as described in #3499702: AI CKEditor requires the Source CKEditor button).

Proposed resolution

- Re-add the logic from #3499702: AI CKEditor requires the Source CKEditor button to active this for the Completion plugin.

Rebased the MR on current 11.x to resolve merge conflict.

svendecabooter made their first commit to this issue’s fork.

This looks like a duplicate of #3489583: Provide config action to set editor plugin configuration?

This module provides domain context to site setting entities. (Via the domain-module)

You can add a "fallback" (no domain context), meaning that if no site setting with a more specific domain context exists, there's always some value to show. You can also choose to not add a fallback, then only the site setting specific to that domain will be returned via the site setting loaders with domain context.

On that note, it's important to choose the correct site setting loader (one with domain context enabled), and also use the newly created site setting blocks with domain context instead of the default site setting loaders and blocks.

If the domain access module is enabled, then editor access is also checked.

Important note This module is still in alpha, not all functionality has been thoroughly tested and bugs certainly will come up, feel free to let me know about them and I'll try to fix it all ASAP.

Thanks for the fix, and sorry for the inconvenience :-(.

I'll release a new tag.

svendecabooter changed the visibility of the branch 3526978-add-providerid-context-1.1.x to hidden.

Created 2nd branch with same commit - branched from 1.1.x - since I needed that for my testing purposes.

Ah yes I did add the CSS like in #9 - forgot to document here.

Sven Decabooter added a comment on drupal.org: Abstract token usage support

@anjaliprasannan shouldn't the MR be against 1.1.x branch? Not sure what the branching strategy for AI is exactly, but seems to me active dev is against 1.1.x now.

@sarvjeetsingh:

I'm not sure if that would work - I'm still tipping my toes into how all of this works...

It would be something like this, in the ChatOutput constructor?:

    if ($normalized instanceof StreamedChatMessageIteratorInterface) {
      $normalized->triggerEvent();
    }

In my initial testing, that doesn't seem to work. I then get the same error as in the tests: TypeError: Drupal\ai\Event\PostStreamingResponseEvent::__construct(): Argument #1 ($request_thread_id) must be of type string, null given

Not sure if that's something that also needs to be fixed, or if this means this approach is not workable...

Sven Decabooter added a comment on drupal.org: Add provider_id context to PostStreamingResponseEvent

Problem/Motivation

Currently, the PostStreamingResponseEvent only contains some basic data regarding output and metadata. It does not store information about which provider ID the post streaming response is coming from, in contrary to \Drupal\ai\Event\PostGenerateResponseEvent, which does provide a getProviderId method.

For the AI Usage Limits module, and probably others reacting to this event as well, it would be essential information to know which AI provider has triggered this event exactly.

Proposed resolution

- Add optional setProviderId() and getProviderId() methods to the event, and make sure implementing logic is able to provide a value for it.

svendecabooter created an issue. See original summary.

Basic integration logic has been committed in 1.0.x branch, but still needs some work.

Sven Decabooter added a comment on drupal.org: Add support for streaming chat token usage

Test PromptJsonDecoderTest seems to fail now, because \Drupal\ai\Event\PostStreamingResponseEvent::__construct doesn't receive a request_thread_id string in its constructor. Not sure if this value is NULL in this test by design, or by accident. If by design, then the PostStreamingResponseEvent $request_thread_id parameter should be allowed to be nullable?