if ($this->moduleHandler->moduleExists('ai_agents')) {
What's the plan with this and is it something I can help with now?
I'm wondering if the DataType up casting/converting should be independent of Contexts...I may be unnecessarily relating them.. I'm going to go back through this and see if that's the case.
I've implemented an improved version of this here:
📌
Experiment with improved Action plugin deriver for FunctionCalls
Active
Depending on the future of that task we can decide how to move forward here.
michaellander → created an issue.
I marked 'Needs review' but really looking for feedback before I go any further. I may have not used that status correctly!
michaellander → created an issue.
michaellander → changed the visibility of the branch 3512100-context-upcaster_applies-class to hidden.
michaellander → created an issue.
michaellander → created an issue.
With this branch, the workflow is:
- When
FunctionCall::setContextValue()
is called, loop throughAiContextConverter
plugins and checkapplies()
- If found(
TRUE
), stop looping and callconvert()
- If none apply, pass original value
Where this gets a bit tricky is to what extent applies()
should evaluate in regards to returning TRUE/FALSE. Should it just match the proper format? should it be able to successfully convert without doing the conversion?
Let's say we are given 'node:17', we can confirm the first part of the string contains a valid entity type, we can confirm the second part is a real entity. However, if entity type is correct, but the entity doesn't exist, do we return FALSE and let it continue checking for a match? One thing we may want to decide is if the applies
call should remain binary or provide more potential states(think \Drupal\Core\Access\AccessResult
). We'd create a AiConverterAppliesResult
class, with methods such as: applicable
, invalidApplicable
, notApplicable
. This would give us the ability to differentiate converters that definitely don't apply vs ones that may apply but theres an issue(i.e. InvalidApplicable('Node loaded but translation "fr" does not exist')
) vs converters that will apply and succeed.
This means the workflow could evolve to:
- When
FunctionCall::setContextValue()
is called, loop throughAiContextConverter
plugins and checkapplies()
- If
applicable
found, convert - If
invalidApplicable
, return these with description on why they were invalid - If
notApplicable
, fail, and decide what to do
I like this approach better because it's up to the implementer to decide how to treat success, partial success and failure. They could still interpret them as true/false, or they can potentially take the natural language description and send that back to a provider and even prompt the user for clarification on how to move forward.
I just want to maybe get some other eyes on it before I continue.
michaellander → created an issue.
michaellander → created an issue.
michaellander → created an issue.
michaellander → created an issue.
michaellander → created an issue.
michaellander → created an issue.
I pushed some fixes for the execute()
call and reworked how the action service is getting loaded so that the parent wouldn't be affected.
With this now merged into 1.1, I think we should close this issue and work off separate issues for bugs/features/etc.
michaellander → created an issue.
I believe some of this has made it into the repo already, but there are still some areas to sort out(marked as todos in this patch). We can use this thread here for records and to sort out remaining.
michaellander → created an issue.
Worth noting, we found an issue with eca_config_schema_info_alter()
not firing when the eca_config
module was enabled. I was able to work around this by using an OOP hook.
See 📌 Config validation for ECA models Active
Might not actually be anything configurable on this one..
@greggles, we are working on a new approach to image type derivatives so that we can support more complex workflows, for example:
Original image is .jpg
- Generated AVIF
- Generated WebP(fallback)
- Original JPG (removed from display)
Original image is .webp
- Generated AVIF
- Original WebP(fallback)
Original image is .avif
- Original AVIF
- Generated WebP(fallback)
This will also allow builders to mix in other standards, like if you have a photography site or maybe product shots and want to use Jpeg XL(with webp fallback) for those displays. We are nearing completion, and have a solution for this, although not the most elegant. Ultimately we will override the ImageStyle
entity class and have versions that support core and imageapi_optimize when enabled. Because we are trying to support both core and imageapi_optimize versions, I'm not sure a solution here really save us too much effort. Ultimately a solution might be a better fit for core.
michaellander → created an issue.
michaellander → created an issue. See original summary → .
michaellander → created an issue.
michaellander → created an issue.
michaellander → created an issue. See original summary → .
michaellander → created an issue. See original summary → .
michaellander → created an issue.
michaellander → created an issue.
michaellander → created an issue. See original summary → .
michaellander → created an issue.
michaellander → created an issue.
michaellander → created an issue.
michaellander → created an issue.
michaellander → created an issue.
michaellander → created an issue.
michaellander → created an issue.
michaellander → created an issue.
michaellander → created an issue.
michaellander → created an issue.
How embarrassing. This was a screw up with git on my part. A new alpha is released with the correct latest code.
If we wanted to cover that part in the ECA schema as well, we would have to have an aggregate of all the plugin schemas in the ECA schema. Or is there a way to refer to the plugin-specific schema by the plugin ID, including for all those unknown plugins that come from contrib modules?
You only need to define schema for the actions you are defining, and then you are correct, you can just have an argument for the plugin_id
to handle all actions, irregardless of if they are in your purview.
Here are a few Action schema snippets from core:
//from web/core/config/schema/core.entity.schema.yml
action.configuration.action_send_email_action:
type: mapping
label: 'Send email configuration'
mapping:
recipient:
type: string
label: 'Recipient'
subject:
type: label
label: 'Subject'
message:
type: text
label: 'Message'
and
//from web/core/modules/user/config/schema/user.schema.yml
action.configuration.user_add_role_action:
type: mapping
label: 'Configuration for the add role action'
mapping:
rid:
type: string
label: 'The ID of the role to add'
Which would be covered by:
type: action.configuration.[plugin_id]
michaellander → created an issue.
Looks like we might also want FunctionCall
Attribute to extend the Drupal\Component\Plugin\Attribute\Plugin
class, and then the FunctionProperty
might more closely reflect the changes
📌
Convert ContextDefinition plugin discovery to attributes
Closed: won't fix
in regards to using constraints and what have you.
It might be worth taking inspiration from how the core Context API works for this. In particular, I think it makes sense:
- Move the
FunctionProperty
definitions into theFunctionCall
definition. This would give you visibility into theFunctionCall
props prior to instantiating the plugin. An example context definition in another plugin:
context_definitions: [ 'entity' => new ContextDefinition( data_type: 'entity', label: new TranslatableMarkup("Entity"), required: FALSE, description: new TranslatableMarkup("The contextual entity that can be used for token replacements.") ), 'recipient' => new ContextDefinition( data_type: 'email', label: new TranslatableMarkup("Recipient"), description: new TranslatableMarkup("The email address to which the message should be sent.") ), 'subject' => new ContextDefinition( data_type: 'string', label: new TranslatableMarkup("Subject"), description: new TranslatableMarkup("The subject of the message.") ), 'message' => new ContextDefinition( data_type: 'string', label: new TranslatableMarkup("Message"), description: new TranslatableMarkup("The message that should be sent.") ), ]
- Additionally we could tap into cores DataType API and 'Constraints', similarly to Contexts. This would make it easier to describe and reuse existing property types, and set more advanced rules like 'string' with max length 84. For example:
constraints: [ "ComplexData" => [ "value" => [ "Length" => ["max" => 84], ], ], ]
- Consider interfacing/extending
Drupal\Core\Executable\ExecutablePluginBase
and/orDrupal\Core\Executable\ExecutableInterface
, thoughExecutablePluginBase
does have an implied config layer that might not really align with properties
michaellander → made their first commit to this issue’s fork.
I removed the remaining tasks for now, it was previously set to 'Review Patch', but that definitely was not sufficient.
In the fork I created, the things I was still sorting through:
execute
/executeMultiple
solution and infinite loops- Node actions extending entity agnostic
Drupal\Core\Field\FieldUpdateActionBase
, need means to know name of entity context - Backwards compatibility with Action plugins from contributed modules.
I appreciate the comments and I'll look at the Rules approach to it.
Looks like it still needs work around access calls.
Reviving this as I believe Action plugins may be a great pathway to unlock core and contrib en masse as AI tools. However, to do so, actions would need to be able to communicate required contexts.
Much has changed since the last patch, so I tried to work through and map changes accordingly.
I did change the approach for:
Drupal\Core\Action\ActionBase::executeMultiple
with the hopes of better backward compatibility.
Original:
/**
* {@inheritdoc}
*/
public function executeMultiple(array $entities) {
foreach ($entities as $entity) {
$this->execute($entity);
}
}
From most recent patch:
public function executeMultiple($context_name, array $context_values) {
foreach ($context_values as $context_value) {
$this->prepareExecutionContext([$context_name => $context_value]);
$this->execute();
}
New:
/**
* {@inheritdoc}
*/
public function executeMultiple(array $contexts) {
$context_names = array_keys($this->getContextDefinitions());
foreach ($contexts as $context_values) {
if (!empty($context_names) && $context_values instanceof EntityInterface) {
// Assume a single context of type entity exists.
$context_values = [$context_names[0] => EntityContext::fromEntity($context_values)];
}
// Assume the plugin hasn't been upgraded.
else {
$this->execute($context_values);
continue;
}
$this->prepareExecutionContext($context_values);
$this->execute();
}
}
Based on this change, I was looking for feedback of how:
Drupal\user\Plugin\Action\CancelUser
and Drupal\user_batch_action_test\Plugin\Action\BatchUserAction
used execute
and executeMultiple
I also wasn't sure if I needed to add constraints as well?
michaellander → made their first commit to this issue’s fork.
michaellander → created an issue.
Added as maintainer! Appreciate your interest!
michaellander → created an issue.
If you are on 10.1 and above, try the latest 2.0.x release. If you are on 10.3 and above, try the latest 2.1.x release or test the 3.0 alpha. Let me know if you are still seeing the issue!
Moved this into 3.x.. I'm not opposed to backporting it, but my concern is how significant of a change in approach it is from 2.x versions. The new 3.x alpha should be all the latest with this, and then some plans for a few additional improvements elsewhere.