🇬🇧United Kingdom @andrewbelcher

This also includes a fix for #3531134-31: Create Base Class for OpenAI based clients → , though I'm not sure if TRUE is the correct default...

Also run a straight Fibers vs non-Fibers for 5 LLM calls each generating 15 haiku:

i$ ddev drush scr scripts/fibers.php

Without fibers
--------------

 5/5 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
                                                                                                                        
 [OK] Without fibers: 56.567 seconds                                                                                    
                                                                                                                        

With fibers
-----------

 5/5 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
                                                                                                                        
 [OK] With fibers: 14.601 seconds                                                                                       
                                                                                                                        

Comparison
----------

                                                                                                                        
 [OK] Performance improvement: 3.9                                                                                      
                                                                                                                        

Needs a provider extending OpenAiBasedProviderClientBase that supports streaming to test. Can be tested with:

phpunit --configuration phpunit.ai.xml --filter FiberTest

Where phpunit.ai.xml has a correctly configured OpenAI key.

I think there is an issue with the implementation of \Drupal\ai\Base\OpenAiBasedProviderClientBase::canChatStream.

Currently it simply returns $this->streamed. That property is whether or not we want a streamed response, not whether it's supported (which is what both the method name and documentation the method should do).

In 📌 Support Fibers for collaborative multitasking on LLM io waiting Active we are using streaming as a proxy for async requests, which means we need to be able to know whether it's supported separately to whether it's requested for the API call.

Not sure whether we want the default to be TRUE or FALSE? Specific providers can then override.

andrewbelcher → created an issue.

andrewbelcher → created an issue.

marcus_johansson → credited andrewbelcher → .

Firstly to say that I don't mean to be putting down the work you've done! I agree that the community, and in particular the maintainers who will end up responsible for it, are the ones to decide. I think both options have good merit and I'm introducing this only to help us find the best overall solution. It may be that we're best to bring in some of the good bits from ddev-drupal-contrib into our own 'custom rolled' stuff.

I've opened https://github.com/ddev/ddev-drupal-contrib/issues/135 to discuss where I feel the limitations of ddev-drupal-contrib lie. If I've missed anything, do say!

Your first two cons are the first two points of consideration on there.

ddev-drupal-contrib works better if you commit all the DDEV artifacts it creates. The AI ecosystem is huge, here we're thinking about the AI module, but then we should configure every other module in the same way

I'm leaning the other way - we only commit the .ddev configuration for the AI module, and everything else gets worked from in that project.

If we commit DDEV artifacts created by ddev-drupal-contrib, then we have to maintain them (update them every time they change on the source add-on)

We only need to update if there is something we want to update for. Once added to a project, the add-ons can be used "as is" without updates/changes.

An update is just ddev add-on get ddev/ddev-drupal-contrib and then review/commit any changes. Where as the alternative is the AI maintainers taking on not just the maintenance of 3 additional projects (plus documentation), but also the development to provide the functionality we'd want.

1. Production dependencies - these go into require, and we can use installer-paths if we want them in web/modules/custom so we can develop them.
2. Non-production dependencies - these go into require-dev and likewise can use installer-paths if we want.
3. Things we will always want when developing AI module - these could go in require-dev, but that would mean they also get installed in Drupal CI. This may be an acceptable trade-off, or we may want to find an alternative approach (git submodules could even be an acceptable approach).
4. An easy way to add ad-hoc dependencies in a specific instance - this could be solved with require-dev and no committing the changes, but that doesn't feel ideal. You could just check them out (and it'd be easy to provide some commands to make that easier).

I think looking at how preferred-install could be used to make it easier to get working repos would be worthwhile.

I do think a lot of those could be solved by having a single AI development project, which I think wouldn't need to be more than .ddev and composer.json (and probably a README.md).

andrewbelcher → created an issue.

Can I recommend https://github.com/ddev/ddev-drupal-contrib as a worthwhile consideration. It does a lot of the effort it setting up an environment that can be flexibly used across different core versions and maintain relative parity with Drupal's CI testing.

It's slightly geared towards having a single primary module, though I think there are ways that could be handled by making use of require-dev and installer paths to put all the "active" projects into web/modules/custom so the commands like ddev phpunit behave as expected.

I think that would allow us to do all the config in the main AI module, rather than needing additional projects etc.

andrewbelcher → changed the visibility of the branch 3535810-fix-phpunit-tests-andrew to hidden.

andrewbelcher → created an issue.

andrewbelcher → created an issue.

andrewbelcher → created an issue.

After discussion with Marcus I've merged some changes that should make this a bit easier to maintain as we don't have to maintain versions etc. Also moved into the .phpunit before_script to simplify and follow best practices.

Tested on D10.5 and can confirm that the issue also existed there, and that this fix resolves it.

andrewbelcher → created an issue.

Worth noting that this may only work on D11 (need to test D10).

andrewbelcher → created an issue.

andrewbelcher → created an issue. See original summary → .

Thanks for the really informative response! I appreciate why it's behaving the way it is now. The next minor test means that there is still coverage for 11.2, even though it's technically 11.2.x-dev.

It's not obvious that opting into previous minor is actually opting into the latest security release of the previous minor. Though I think that most of the time that would be the same as the previous minor generally only receives security releases.

I guess in the case of holding up the change for 11.2, it might have been worth back porting 10.5.1 to default-ref. But that's pretty minor in the grand scheme!

So, all in all, it really seems to me that you just want what “main” offers, which is latest versions.

I think there is a valid case for stability of the framework, but latest releases of core. But the issues with core 11.2 make sense why it wasn't done in this case.

andrewbelcher → created an issue.

LGTM

andrewbelcher → created an issue.

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

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

Rebased, fixed typo in line with 🐛 Fix typo in "Attachments" field Active and merged, thanks!

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

Thanks both!

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

Thanks both!

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

andrewbelcher → created an issue.

andrewbelcher → created an issue.

Tested, and it's working great!

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

Also tested and looks good! Am merging now!

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

PR on top of 📌 Add function calls for epics & tasks Active - https://git.drupalcode.org/project/minikanban_agent/-/merge_requests/9/d... are the changes for this issue.

andrewbelcher → created an issue.

PR on top of 📌 Refactor hooks into OO classes Active - https://git.drupalcode.org/project/minikanban_agent/-/merge_requests/8/d... is just the changes for this issue

andrewbelcher → created an issue.

andrewbelcher → created an issue.

andrewbelcher → created an issue.

andrewbelcher → created an issue.

andrewbelcher → created an issue.

andrewbelcher → created an issue.

Looks good!

Looks good!

andrewbelcher → created an issue.

andrewbelcher → created an issue.

andrewbelcher → created an issue.

andrewbelcher → created an issue.

marcus_johansson → credited andrewbelcher → .

andrewbelcher → created an issue.

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

Add token replacement on property restrictions and expose tokens on explorer. Well tested locally and working really nicely!

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

andrewbelcher → created an issue.