🇬🇧United Kingdom @MrDaleSmith

Ensured that messages thrown during the AIProvider's running are passed up the tree to the error handling within the core AI module.

I've tried to fix the failing tests on this, but cspell is an abomination and will not ignore the word "blacklist" which isn't in the project but the word "git_blacklist" does aoppear in the grumphp.yml file: presumably there is someway of getting cspell to ignore this file?

There was an additional issue in the Provider codebase that caused an error when I tried to use tool calling with the AI Provider: all tool parameters were being added as required fields, which meant any optional fields caused an error if they were missing from a valid AI response. Fixing that, and I have managed to get the AI Assistant to create a new content typed called "Tester" from the prompt "Create a content type called Tester" without erroring.

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

Can now be tested against https://www.drupal.org/project/patreon/releases/4.2.0-alpha1 → so I will close this ticket.

The work has been done and will soon be merged into a 4.2.x release from the PR at https://git.drupalcode.org/project/patreon/-/merge_requests/4.

Changes

PatreonServiceInterface

• The getOauth() method now returns a \HansPeterOrding\OAuth2\Client\Provider\Patreon object. This is configured to handle obtaining Oauth tokens from the Patreon API and should be used instead of the previous methods. This object does not allow for a return path to be set in the API call, so if you have custom code that relies on this you will need to find an alternative method.
• The getToken() method now automatically checks whether the League\OAuth2\Client\Token\AccessTokenInterface has expired and if so will attempt to obtain refreshed tokens from the Patreon API. It still returns the API token as a string.
• The getScopes() and setScopes() methods have been deprecated and will be removed in future versions of the module: the Oauth client handles to setting of scopes for Oauth access, so these methods no longer have any functionality. If you require setting custom scope, you will have to create your own League Oauth Client and override the one used by the module.
• The getRefreshToken() method has been deprecated as the refresh token is stored within the AccessTokenInterface and used automatically by the underlying code when required. If you require access to the refresh token, you will need to use the getStoredToken() method to access the current Access Token.
• The getAuthoriseUrl() method has been deprecated and no longer returns a value. The authorisation URL is obtained from the Patreon Oauth Client returned by getOauth().
• The decodeState() method has been deprecated as the URL the Patreon API returns users no no longer contains any encoded values. This is related to the removal of functionality around allowing the API to return a final url to redirect users to.
• The retry() method has been deprecated and no longer carries out any functions. If you require retry handling, will will need to implement custom code to provide this.
• The default service uses the global AuthToken by default. If you require using different token to access the API, the PatreonUserService should be used as this allows for a custom AuthToken to be set with its setToken() method.

General changes

• The constants set in the patreon.module and patreon_user.module have been moved into the PatreonServiceInterface and PatreonUserService respectively.
• The code providing Token-support in the Patreon Extras module has been moved to a separate tokens.inc file as required by that module.
• The module has been updated to only support Drupal 10 and 11 sites: this is so the code can rely on the features of PHP 8.3. Usage with lower versions is not supported, although I have not restricted this in the composer.json file.

I'm knocking this back to Needs review as no approach has been agreed, or any confirmation that either patch fixes the issue. I think the test fails are unrelated but it might be worth looking into them as well.

Hit this same issue and would agree that whilst Drupal 10 and Drush 12 are still supported, this is a breaking change and either needs reflecting as such in a new D11 only release or reverting in the 6.3 releases.

Update to Drush 13 does fix the issue, if that's something you can do.

mrdalesmith → created an issue.

Not an expert, but more detailed reading of the issue queue makes me think this might be a duplicate of https://www.drupal.org/project/gemini_provider/issues/3519469 ✨ Support tools calling Active - can anyone confirm, at least?

mrdalesmith → created an issue.

Works good for me :)

MR is in draft and has failing tests, so knocking back to needs work.

You may be hitting the issue described in https://www.drupal.org/project/ai/issues/3513409 🐛 Warning: Undefined array key "#entity" in Drupal\ai_translate\Form\AiTranslateForm Active - can you use the patch there and see if it is resolved for you?

Looks good and works well.

Sounds like this may be a candidate for closing as works as designed?

Looks fine but this would be a breaking change, yes?

Good idea and resolves any issues.

Seems like a sensible change and causes no issues that I've found.

LGTM

LGTM

Ha ve you confirmed that this is an issue with ai_translate? You seem to be using a number of LB-specific modules that I would have thought were more likely to be the source of the problem? The core AI translate module doesn't specifically interact with LB.

First go at resolving this issue. One thing I can't work out a way of doing is stopping you having to put in a "restricted to vocabulary" prompt if you have no vocabularies that will let you use that option. Possibly not a big deal, but it's a required field you won't need.

Changes requested by maintainer in MR

Test fail is unrelated, but it would be good to know (away from this issue) what's happened to provoke it. Code looks good and a more robust version of the original fix. I cannot recreate the problem some users had with the first attempt, so we may need someone who can to confirm this change addresses the issue.

I still don't see why an example module that cannot be used has code in it that is commented out.

This seems to remove the code that ensures the latest revision of the entity is used if needed: was that intentional?

Code looks good to me: cannot be properly tested or merged until https://www.drupal.org/project/ai/issues/3517618 ✨ Abstract token usage Active has been approved and merged, so this will probably sit for a while.

Looks OK to me.

The maintainers are reluctant to enforce module-level dependencies that aren't required by every plugin used by their code, so I think we will need to implement a system like https://www.drupal.org/project/ai_agents/issues/3515453 📌 Agents have silent dependencies Active for the AiCKEditor plugins. This will take longer, but will allow dependencies to be controlled by the plugins that need them.

Nevertheless, the way to do it is through a provider, so you will need to raise a ticket that details the changes required to be able to support it: I'd suggest going step by step as any one change could easily break system for every other plugin. The basis of the ciore AI module is that everything is abstracted so that it can be used by any provider: any changes will need to respect this and work regardless of how the AI is ultimately accessed.

I'm leaving this issue as closed unless one of the maintainers want to reopen it, because this doesn't feel like something the current code can support in a single issue. It may well be that the AI family of modules cannot support an AI that works so differently to the other ones, in which case it may be easier to implement a separate contrib module for webGPU.

It looks like the explorers were using incorrect FAPI elements for the file upload that weren't correctly restricting file type uploads: the element should be restricted only to MP3 for compatibility across the widest range of AI Providers.

I've updated all the explorers and added the help text to the descriptions.

As the maintainer has detailed, the core AI module does not do anything directly with an AI and aklways interacts through a provider. AIUI, the appropriate way to support a WebGPU Ai would be through a new provider module that the other AI functionality can interact with, as per https://project.pages.drupalcode.org/ai/developers/writing_an_ai_provider/

As such, I'm going to close this as won't fix.

Code looks fine but we need details of the issue this change corrects, and steps to recreate so we can test properly.

There is a fork but no merge request: see https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... → which may assist.

There is no code here to be reviewed, so setting back to Active.

OK, it sounds to me that what we're saying is this:

1. It's complicated, because decisions made in the past mean there's no easy way for this module to know if functionCalls are available in a provider;
2. Really it should be fixed at the provider level, but we don't control every provider;
3. Anything we do do could introduce breaking changes into either the 1.0 or 1.1 versions of this module.

In lieu of a better solution, I think we should really look at 2 as a quick fix: composer has a mechanism for telling people that certain combinations of versions don't work together and its very easy to implement. I've added and done a MR for adding a composer conflict to the Open AI Provider at https://www.drupal.org/project/ai_provider_openai/issues/3519419 🐛 Version of modules can go out of alignment Active - if that's approved, we can start rolling out similar for the other providers the maintainers here control, and leave this ticket open for a discussion about a more generic solution and when it might be best to implement it.

Have added a composer conflict for the 1.0 branch so it cannot be used with the 1.1 versions of AI or AI Agents, which will resolve this issue for the Open AI provider.

mrdalesmith → created an issue.

Failing tests so set back to needs work.

Added as a fgork to allow tests to run, and you have some test fails so this will need further work.

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

Seems OK to me

Might also be worth looking at the HTML structure of that response: it looks to me from the screenshot that the issue occurs outside of the codeblock this MR adds.

Possibly this would make more sense within the ai_logging sub-module: that already has custom AI-related logs, and moderation failures are something that can happen for any provider type (although I believe Open AI is the only one that moderates by default).

LGTM

Not a maintainer and this is just my 2p worth, but this seems like a lot of code to reinvent "log an error message" for a handful of people who need it. The exception throwing has been standardised as the AI module's method of telling the code that something has gone wrong and not to continue processing (which might involve more AI calls and so costs) and I'd be reluctant to introduce something non-standard for one area as that would become expensive to maintain and cause confusion for users.

I'm not sure why standard Drupal logging of the error with more relevant information included won't suit the desired purpose here?

There are multiple test fails in the code so this can't be merged, but I would maybe wait for the maintainers to chip in about the approach before spending any more time working on it?

Tests are failing because of a coding standards breach, so setting back to needs work.

I can only assume you didn't install through composer, or else had some kind of composer error whilst installing, but I cannot recreate thios issue and the required library is listed in the composer.json file for both versions, so I'm going to close this as works as designed.

Looks good to me.

Yeah I'd recommend forcing ECA to use the root account as an initial test: I'm not 100% familiar with ECA but I wouldn't be surprised if it runs its integrations as anonymous in most cases.

Works well, applies cleanly. i think this is a good addition to the module. RTBC.

The current user tokens should be automatically turned into their values from the logged in user as part of the token generation process: the most obvious thing to check is that there is a currently logged in user at the point that the AI request is being sent: if it's being trggered by the anonymous user (such as if you are running it via the queued process rather than directly) there will be no value. The other thing to check is that the user triggering the update has that value set on their account.

this is a duplicate of https://www.drupal.org/project/ai/issues/3516104 ✨ Add text to image support for OpenAI 4o Image Generation Active and that ticket's response still stands, I'm afraid.

Display inline and works nicely. The chatbot is displayed open by default and the minimise button doesn't seem to do anything: this is a change from the default behaviour when the modal display options are used, so I think it needs to work in a similar fashion. At the very least, the collapse button should collapse the chatbot.

Will need the maintainers to feed back on the styling questions, and also decide whether this change needs to be ported to the deprecated Chatbot block as well.

TBH I would advise against this: plugins are the Drupal way of doing this and allowing code to be discovered and used by any module that wants to. That you find the system complicated is more of an issue with Drupal than it is with the AI module, and anything you do to "improve" it will be non-standard and therefore confusing to people who are familiar with Drupal.

I think if you wanted to, a decent compromise would be to have an implementation of the existing plugins that could discover your simplified tools and run them. Otherwise you risk redesigning Drupal from inside contrib and causing unintended issues everywhere.

Hi @bbruno - not seeing any suggestions here: can you create a fork using the guide at https://www.drupal.org/community/contributor-guide → to add your changes to this issue please?

@bbruno feel free: https://www.drupal.org/community/contributor-guide →

Hi @bbruno - you don't need permission: feel free to create a fork through this ticket and add your suggested changes. If you need help with the process you can use https://www.drupal.org/community/contributor-guide →

Is the second screenshot cropped at all? It seems a strange ratio, and the header (which contains the element you'd use to minimise the block when you aren't using it) seems to be cropped out. I'm wondering of what you are actually experiencing here is the issue detailed in https://www.drupal.org/project/ai/issues/3512670 🐛 AI Chatbot interface height is not adjusted in homepage Active - the block is supposed to float above other elements on the page when it's open.

The branch was out of date, and appears to have originally been split from 1.0 but is being tested against 1.1. I can see you've updated it, but I wonder if that's what's causing the seemingly random test fail? I can't test because the patch version of the MR doesn't apply to the current 1.1 dev branch, but I don't want to break anything by attempting the rebase.

The code itself doesn't look to have any major issues, and seems like a good way of handling a very complex idea.

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

The Basic method provides ONLY the selected field's data as {{ context }}. The tokens you are adding in your rubric do not appear to be valid Drupal tokens (they should be prefixed with the entity type, as in [node:field_model] so there is nothing that can turn these into values.

I would say that this is working as expected: the Basic mode is - as the name suggests - a basic method that does not support complex data structures: you SHOULD be using the token method for a set up that is this complicated, and if you have issues with the translations of your tokens that would be the issue you'd need to resolve.

The alternative would be a hook_requirements check in the 1.1 modules to check that any other AI-family modules were also on the 1.1 versions: @Marcus did you have an idea of which way you want to go?

@Marcus - you've put the ticket to needs review but the MR is in draft: is this ready for review? Can you also explain the reasoning behind this being added here rather than in the AI module's ai_validation sub-module? Given the dependency on another module that most sites won't be using, I would have thought this would have made more sense as its own separate contrib module if we're thinking of deprecating the ai_validations sub-module?

I've put a note on the ticket for the maintainer: I'll close this as a dupe

@marcus Can this be ported to 1.1 as well?

I believe this is a duplicate of https://www.drupal.org/project/ai_agents/issues/3499719 🐛 Use Interfaces instead of Class names in AgentHelper::construct Active , which has already been fixed?

Looks good and works without issue for me. RTBC.

OK, I've tested this as best I can, but I think this will need careful review: we are essentially logging in here as a user with elevated permissions but none of the other features of a user account. This could cause security issues if this user gets saved or otherwise sticks around longer than we think it will. At a more basic level, any code that checks $user->isAnonymous() whilst our user is logged in is going to either throw a fatal error or provide an incorrect answer.

Your video doesn't show you clicking "enable" on the Generate with AI section of the settings: it shows you unchecking the box. To recreate, use the latest dev 1.1 release, check the "enable" box but do not enter a prompt in the text area.

mrdalesmith → created an issue.

Change still works, but the new code changes make the code itself a lot neater. I think this is a better version than the original: setting back to RTBC

mrdalesmith → created an issue.

Selecting the tools when adding a new Agent fires an AJAX rebuild, but the "Detailed Tool Usage" section does not appear within the form unless you save the agent and edit it again: I can't see anything within this change that would cause that - is it a separate issue?

I'm not getting a testable response from the agent itself, but I'd assume that's me rather than the code: I think this needs testing by someone more familiar with setting up agents and when a direct response is suitable. The rest of the UI works, if that helps.

Set to "needs work" at Marcus' request so a test can be identified and built into the testing suite.

No, not really: why does it require an entity to be created? I was just suggesting changing the label so the reference widget you'd created appears in the "reference" list when setting up a new field, rather than as a separate field type.

Also, I'm not a maintainer on this project so I wouldn't completely retool your approach based on my suggestions without any feedback from the actual maintainers. My suggestions have no weight here.

Suspect this is outdated with https://www.drupal.org/project/ai/issues/3517883 📌 Module homepage does not list AI Vector Database (VDB) Providers like AI Providers are listed Active and https://www.drupal.org/project/ai/issues/3517884 📌 Documentation page for providers is out of date Active - would you agree @Marcus?

Looks good to me.

@Marcus can you suggest a good test to see if this interferes with anything? Code looks fine but I can't see what would use it when you approach through the UI.

@Marcus is this a duplicate of https://www.drupal.org/project/ai_agents/issues/3517373 📌 Add a plugin manager definition lister tool Active ?

Whilst a Block Content agent is a valid addition, it might be worth considering if it's possible to have a more generic Entity Agent that can handle any agent: at the minute we have a node agent, a taxonomy agent, a deprecated webform agent and now a block agent, and if we continue this way there will be a lot of agents all doing very similar things. If a generic agent is not possible, possibly a new base class or trait so that we don't duplicate some of the underlying methods.

Set to needs work because of Marcus' comments and the failing tests.