Drupal CMS

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

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.

svendecabooter created an issue. See original summary.

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?

This module provides some different layouts/functionality to the excellent Site Settings module.

Mainly, a complete overhaul of the layout is added.

There are two options:

• Single form
• Core config pages

Single form

The single form layout adds all options in vertical tabs, this can however become quite a big form and is usually only recommended for smaller collections of settings.

Core config pages

The core config pages layout uses a page with links like a core config overview page. You can add more information by setting up the "menu_description" view mode, this will be rendered as the description (useful for "multiple" items, for example social links: "facebook", "instagram", ... labels could be rendered as the description). Then you can choose to either open settings in modals or open them in new pages.

When opening settings forms in modals, you should definitely make use of the patch in the issue linked below, otherwise media library modals will wreck functionality. #2741877: Nested modals don't work

+1 - this fix is needed for #3519302: Abstract token usage support to work.

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

Added related issues in AI module, that need to be fixed first, before both token usage for both regular chat and streamed chat can be captured.

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

MR now triggers the event, when the StreamedChatMessageIterator object gets destructed.