The MR patch does not apply, so I had to do it manually.
I think we're making progress. Now when I save at admin/structure/types/manage/page/automator_chain I'm only seeing this:
Warning message
Schema errors for ai_automators.ai_automator.node.page.field_extract_general_sentiment.default with the following errors: ai_automators.ai_automator.node.page.field_extract_general_sentiment.default:disabled missing schema. These errors mean there is configuration that does not comply with its schema. This is not a fatal error, but it is recommended to fix these issues. For more information on configuration schemas, check out <a href="https://www.drupal.org/docs/drupal-apis/configuration-api/configuration-schemametadata">the documentation</a>.
Schema errors for ai_automators.ai_automator.node.page.field_plain_text_long.default with the following errors: ai_automators.ai_automator.node.page.field_plain_text_long.default:disabled missing schema. These errors mean there is configuration that does not comply with its schema. This is not a fatal error, but it is recommended to fix these issues. For more information on configuration schemas, check out <a href="https://www.drupal.org/docs/drupal-apis/configuration-api/configuration-schemametadata">the documentation</a>.
Schema errors for ai_automators.ai_automator.node.page.field_sentiment_score.default with the following errors: ai_automators.ai_automator.node.page.field_sentiment_score.default:disabled missing schema. These errors mean there is configuration that does not comply with its schema. This is not a fatal error, but it is recommended to fix these issues. For more information on configuration schemas, check out <a href="https://www.drupal.org/docs/drupal-apis/configuration-api/configuration-schemametadata">the documentation</a>.
No schema for ai_automators.settings. These errors mean there is configuration that does not comply with its schema. This is not a fatal error, but it is recommended to fix these issues. For more information on configuration schemas, check out <a href="https://www.drupal.org/docs/drupal-apis/configuration-api/configuration-schemametadata">the documentation</a>.
Also, in 'Manage form display' settings, the following fields do not display the FWA field-set:
- Widget settings: Autocomplete // this one use to work fine I think
- Widget settings: Datetime Timestamp
- Widget settings: URL alias
- Widget settings: Text area (multiple rows) // this one use to work fine
- Widget settings: Comment
More feedback is required about what I wrote in #31, that there should only be a Save button i.e. the UX should exactly replicate what we see in Manage form display and Manage display tabs.
Setting to RTBC…
bisonbleu → created an issue.
I agree, makes sense.
Just tested the latest MR with an Article node where 2 fields are setup with Automators » Automator Worker: Direct
- When the automators are disabled in the UI at admin/structure/types/manage/page/automator_chain, saving the node does NOT trigger them i.e. no call to the LLM are made;
- When the automators are enabled in the UI, saving the node triggers both automators i.e. 2 calls are made to the LLM;
NOTES:
@anjaliprasannan, in #5 you write:
Split the form submission into two actions: "Resort" (current behavior) and "Save" (to persist disabled states)
I don't understand why I would want to «Resort» after just sorting the automators ? Resort suggests to sort again, which does not make sense in the context.
Unless you mean to say «Reset»? Reset | Undo here would suggest resetting the current form to its original saved states, in other words «Cancelling the changes.» If this is your intentions here, then «Cancel» might be a better word. But… «Cancel» also suggests to «Go back».
IMHO, there should be just one Save button i.e. the UX should exactly replicate what we see in Manage form display and Manage display tabs.
So yes and «great work!» this works fine with Automator Worker: Direct.
But unfortunately with the addition of Automator Worker: Field Widget disabling the Automators does not hide the field widget action buttons in the edit form and clicking the buttons will trigger calls to the LLM.
bisonbleu → created an issue.
valthebald → credited bisonbleu → .
@anjaliprasannan I'd like to test/review the MR but it has fallen quite a bit behind in 3 months…
212 commits behind, 7 commits ahead of the upstream repository.
Can you update the fork in GitLab ?
Oups… my bad… patching deleted the list_string plugin. After re-adding the plugin all is well!
Hum… tested the MR in composer.json. Now, in the node edit UI, the blue action button is gone? And in the Manage form display settings, the WFA config is gone…
Yep, sloppy and lazy… sorry for wasting your time with that. A useful experiment though.
So here's a complete rewrite, high level & low tech, based on my recent experience. Let me know if it goes in the right direction.
Recently spent quite a bit of time figuring and working on a few of those Automator plugins (boolean, email, list_float, list_integer, list_string). And in this process, the code duplication has become obvious.
Now I'm thinking «there’s got to be a better way…?» Maybe a deriver?
Here’s what Claude.ai had to say. And from what I gathered working on these plugins, it kind of makes sense - especially if you accept to trade code duplication for a bit more complexity.
Tier 1: Base Deriver for Simple Fields
Create a deriver that generates plugins for field types that follow Pattern A:
- string, text, email, integer, float, list_integer, list_float, boolean
- Configuration-driven: widget types, field types, form element property
- Handles 80% of use cases with zero code duplication
Tier 2: Manual Plugins for Complex Fields
Keep hand-coded plugins for Pattern B/C:
- link, image, entity_reference, specialized widgets
- Cases requiring custom saveFormValues(), traits, or complex logic
Tier 3: Override Mechanism
Allow manual plugins to override deriver-generated ones when needed.
But is the cognitive complexity of a deriver system worth eliminating the maintenance complexity of duplicated code? I’m leaning towards «not worth it». Code duplication is easier to work with, I can deal with that extra entropy.
Another option (?):
- Move
aiAutomatorsAjax()
to Base Class; - Create helper methods in base Class e.g.
getFieldValue()
,setStandardFormValue()
to simplify plugin structure;
This might make it possible to reduce code duplication / plugin size by more than 50% ?
It's possible that my vision might be too localized - i.e. the depth of filed is limited and I'm not getting the greater picture of the universe…
I realize the parent issue has status «Postpone», but I'm there so just in case.
No video for this one because:
The 'Float' and 'List (float)' field types can no longer be added via field UI. →
I realize the parent issue has status «Postpone», but I'm there so just in case.
Also attaching a video of the working FWA automator plugin.
@marcus_johansson Since I was in knee deep and could not extract myself, I gave it another go - blame it on obsession.
The latest commit fixes the issue when the options_select widget is used, see list-integer-sentiment-score--v2.mp4.
There may be a better place to fix this upstream, e.g. in FieldWidgetActionBase.php. Let me know.
Note - works fine with the Check boxes/radio buttons widget.
Attaching video…
With the latest commit, multiple emails are extracted from a Basic Page Body field. The only issue is I can't quite get/figure the ajax to update the multi-item field. In the video, I simply refreshed the page to see all 3 items.
The other issue, is that another call to the LLM occurs on Save, and that should not happen. See 🐛 When Field Widget worker is selected, Direct also fires (?) Active
Strangely, in my local ddev env. both phpcs and phpstan run without finding issues, but in GitLab phpcs fails !? I'm running these commands.
% ddev exec vendor/bin/phpcs --standard=Drupal,DrupalPractice web/modules/contrib/ai/modules/ai_automators/src/Plugin/FieldWidgetAction/Email.php
% ddev exec vendor/bin/phpstan analyze web/modules/contrib/ai/modules/ai_automators/src/Plugin/FieldWidgetAction/Email.php
Note: Using configuration file /var/www/html/phpstan.neon.
1/1 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
[OK] No errors
bisonbleu → created an issue.
bisonbleu → created an issue.
bisonbleu → created an issue.
bisonbleu → created an issue.
bisonbleu → created an issue.
Tested the patch. The View site statistics page now loads. But the charts for Days of month, Days of week (Averages), Hours (Averages) are back to displaying only 3 columns - i.e. broken.
bisonbleu → created an issue.
bisonbleu → created an issue.
bisonbleu → created an issue.
bisonbleu → created an issue.
NP, c'est la vie. Thank you @Marcus for taking the time to explain. You expose intricate & implicit details that I could only partially fathom. Now I better understand. Moving on… : )
bisonbleu → created an issue.
+1 @jurgenhaas
The other way round, when Drupal is crawling remote resources, this is what we are controlling, and here we should provide measures that respect guardrails set by the remote side.
If devices/agents created with Drupal AI can scrape external content, then we need to educate, inform and possibly contain the appetite of such devices/agents.
bisonbleu → created an issue.
I've removed the original description enhancing code from PropertyFormBuilder, it now lives upstream in the ActionPluginDeriver.
Thanks for your comment @adamps. I understand and, like I said, not judging. Synchronicity I guess ; )
The latest fix (which also resolved a circular dependency) re-introduces a phpstan \Drupal
call issue in SpoolStorage.php. Looks like creating a separate service that handles the immediate sending logic to break the circular dependency is the way to go… I'll come back to this later.
UPDATE:
The original patch in #33 changes these files:
/simplenews/config/install/simplenews.settings.yml
/simplenews/config/schema/simplenews.schema.yml
/simplenews/simplenews.install
/simplenews/src/Form/MailSettingsForm.php
/simplenews/src/Mail/MailEntity.php
/simplenews/src/Mail/Mailer.php
/simplenews/src/Spool/SpoolList.php
/simplenews/tests/src/Functional/SimplenewsSendTest.php
So I ran phpstan & phpcs on this list and fixed all the reported issues.
On the issue fork on GitLab, there are still many errors but it looks like most of them now relate to other files that I did not change?
This is starting to feel like a Pandora's box… Maybe I'm wasting my time here?
I'm not judging, but if I cannot get a bit of help here then I'll have to assume I'm in a dead end.
Turning back and moving on to something else is an acceptable outcome.
Thanks
Strange, I think the Needs work was set automagically…
Thanks for all the feedback! Much appreciated.
Ah… yes, smart @Marcus, now I'm starting to see how these description also play as prompts.
Promptscription ;^)
Seriously, when that is the case, maybe we could tag the description label with a small ai-icon to suggest this property?
Ok, I'll give that a try and get back shortly…
Update issue summary.
What it looks like:
Latest patch based on the current MR.
Thoughts? Does this go in the right direction?
Here is the patch. Let me know if you think…
bisonbleu → created an issue.
This is just a raw dump of some of the commands I used to create my Drupal AI Ddev sandbox.
- DDEV
% mkdir dai-sandbox && cd dai-sandbox
% ddev config --project-type=drupal11 --docroot=web
% ddev composer create “drupal/recommended-project:^11.0.0” --no-install
% ddev composer require drush/drush:^13.6
% ddev composer install
% mkdir -p config/sync
% cp web/sites/example.settings.local.php web/sites/default/settings.local.php
- configure settings.php: config_sync_directory, hash_salt, trusted_host_patterns
% ddev start
% ddev drush site:install --locale=en --account-name=admin --account-pass=admin -y
% git init
- update .gitignore as required e.g. *.sql, *.zip, *.mysql.gz, etc.
- add useful modules; customize for your requirements
% ddev composer require drupal/admin_toolbar drupal/pathauto drupal/redirect drupal/token drupal/ctools
- add ai related modules; customize for your requirements
% ddev composer require drupal/ai:1.1.x-dev@dev drupal/ai_agents:1.1.x-dev@dev drupal/ai_provider_anthropic:1.1.x-dev@dev drupal/ai_provider_openai:1.1.x-dev@dev
- to test patches in composer.json
% ddev composer require cweagans/composer-patches
- DDEV EXTRAS:
- Adminer (like PhpMyAdmin but smaller/nicer)
% ddev get ddev/ddev-adminer
- Mailpit: captures outgoing emails; it’s bundled in Ddev, you can open it with this command;
% ddev mailpit
- PHPUNIT (setup)
% ddev composer require --dev drupal/core-dev
% ddev composer update --with-all-dependencies
% cp web/core/phpunit.xml.dist web/core/phpunit.xml
- configure as follows
<env name=“SIMPLETEST_BASE_URL” value=“https://dai-sandbox.ddev.site”/>
<env name=“SIMPLETEST_DB” value=“mysql://db:db@db/db”/>
<parameter name=“outputDirectory” value=“sites/simpletest/browser_output”/>
- then you should be able to run tests, for example:
% ddev exec “cd web/core && ../../vendor/bin/phpunit tests/Drupal/Tests/Core/Routing/UrlGeneratorTest.php”
% ddev exec “cd web/core && ../../vendor/bin/phpunit ../modules/contrib/pathauto/tests/src/Unit/VerboseMessengerTest.php”
- PHP_CodeSniffer / phpcs (setup)
% ddev composer require --dev drupal/coder
- register the Drupal coding standards:
% ddev exec vendor/bin/phpcs --config-set installed_paths vendor/drupal/coder/coder_sniffer
- check what standards are available (should include “Drupal”, “DrupalPractice”)
% ddev exec vendor/bin/phpcs -i
- check a specific file or directory:
% ddev exec vendor/bin/phpcs --standard=Drupal web/modules/custom/your_module
% ddev exec vendor/bin/phpcs --standard=Drupal,DrupalPractice web/modules/custom/your_module
- add a phpcs.xml file in your project root
<?xml version=“1.0"?>
<ruleset name=“Custom Drupal Standards”>
<description>Custom coding standards for this project</description>
<rule ref=“Drupal”/>
<rule ref=“DrupalPractice”/>
<!-- Paths to check -->
<file>web/modules/custom</file>
<!-- Exclude certain files -->
<exclude-pattern>*.min.js</exclude-pattern>
<exclude-pattern>*.min.css</exclude-pattern>
</ruleset>
- then you can run this command
% ddev exec vendor/bin/phpcs
Phpcs: 3 x 'Expected newline after closing brace'.
Obsolete. This issue is also fixed in 🐛 Deprecated function: Implicit conversion from float 19.5 to int loses precision Active
Oh… looks like you found the Holy Grail… the patch that patches them all!
Tested just this patch (removed days-of-week-and-hours-charts-3532354.patch) and now all displays render perfectly. Even Days of Month is rolling!
I spent time in render_htmlchart.inc.php but could not quite untangle its intricacies. And so my patches are «localized» i.e. Zork grade, at flashlight point, deep down in a dark cave. ;^)
So I guess you can mark as -3532354.patch : )
Added the patch in composer.json and it applies without issue. Tested various translation scenarios and see no errors. Interestingly, when translating a paragraph to Navajo for which limited training data exists, it still manage to do pretty well and also provided context as can be seen in the captures.
Here is the patch. Now testing…
bisonbleu → created an issue.
I've been running the latest dev version for a few days and I'm not seeing any regressions and/or errors. But I'm not exactly sure how one would validate this… ?
Setting to RTBC. Feel free to change status if required.
After applying patch, the 2 charts appear to work as designed.
Days of Week chart for May (31 days)
Hours chart for May (31 days)
Here is the patch.
bisonbleu → created an issue.
Here's the patch…
A patch will follow shortly.
Now having a look at the capture labelled «After patch:» which shows not the Monthly History (which displays fine with the latest dev version) but rather Days of month. Ok, I see what's wrong. Let's see if I can figure why that is…
Hm… I'm running the dev version of the module from a few days ago with only my 2 patches (not yours yet):
- implicit-conversion-from-float-to-int-3531406.patch
- timezone-must-be-explicitly-set-to-utc-3531531-2.patch
What I see:
- whether I have $top_x++
or $top_x = is_numeric($top_x) ? $top_x + 1 : 1;
, Monthly History displays correctly (I don't see what you see in capture 2);
- When I use $top_x++
, I get «Warning: Increment on type bool has no effect», but not so with $top_x = is_numeric($top_x) ? $top_x + 1 : 1;
.
Did you try flushing all caches? Could it be because of fixes you introduced ?
Note - Maybe unrelated, but in render_htmlchart.inc.php there are quite a few occurrences of $some_var_x++
where $var might be set to false. So these will eventually need to be fix as well I guess…
Ran into this issue 🐛 There is an off-by-one error in Month select in Statistics selection Active in bawstats.module. After a long debugging session with claude.ai, $timezone = NULL; was identified as the culprit. The fix was: $timezone = 'UTC'; Here is the logic:
- The DrupalDateTime object is created in UTC timezone;
- We know the $timestamp is timezone-agnostic;
- When passing NULL for the timezone parameter to date_formatter->format(), the date.formatter service has to guess what value it should use for the timezone, so it applies the site's default timezone or the current user's timezone;
- Hence the UTC timezone of the DrupalDateTime object is overridden.
- That's not what we want.
Here a new patch where I've only removed the last change in bawstats.data.inc. Does that work for you?
I'm ok with providing a new patch. Can it excludes just this ?
@@ -250,8 +250,7 @@ function bawstats_get_site_files_yearmonth($regenerate_cache=FALSE, $site=NULL)
* If TRUE, clear the cache and regenerate it
*/
function bawstats_get_sites($regenerate_cache=FALSE) {
- $site_files_year_month = bawstats_get_site_files_year_month($regenerate_cache, $site);
+ // Pass NULL instead of undefined $site variable.
+ $site_files_year_month = bawstats_get_site_files_year_month($regenerate_cache, NULL);
return(array_unique(array_keys($site_files_year_month['files'])));
}
-
-?>
Hey @egfrith, thanks for your quick reply. I'll submit an update to the docs page and see what happens. On that page, anything in particular stands out as «misleading» ?
Ha! this is actually a pretty cool «24 month Rolling History» feature :-)
So works as designed.
Here's the patch…
bisonbleu → created an issue.
With the attached MR I think this issue should be set to 'Needs Review'…
Here's the promised patch
bisonbleu → created an issue.
bisonbleu → created an issue.
Made «If there already is a .htaccess
present, delete it…» into a new paragraph as a simple way to highlight this step which I feel is key for fixing this error.
Just a quick update & some questions.
I was able to rebase the issue’s stale fork. The good news is MR !88 is mergeable.
But in Gitlab, on the MR’s page, I see:
- Test summary: 144 failed, 242 total tests
- 78 out of 144 failed tests have failed more than once in the last 14 days
Are all these fails directly related to the MR ? Or is there a backlog of fails that are carried over from other issues ?
If someone can mentor me through this, I'll find more time to fix what needs fixing.
Thanks!
Setting to Needs review…
Here is the patch…