I've pushed up a commit that automatically handles 'refining' definitions as input values are set. This means that validation should occur with a more well defined definition than the original generic definition.
This change currently is only reflected in the \Drupal\tool_content\Plugin\tool\Tool\FieldSetValue
tool. Basically the tool accepts 3 values 'entity', 'field_name' and 'value', with 'value' being typed as any
. After values are set for 'entity' and 'field_name', the 'value' property then becomes a map
, with a definition that matches the actual field definition(multiple, required, property definitions, etc).
We could additionally add a constraint to confirm the field actually exists on the entity, but that introduces the next challenge.
How best do we communicate prior to execution that a tool(and specific properties) are dynamic?
Using the same field_set_value
as an example, we could technically decide to only show 'entity' as the only starting input definition. Then after adding an entity we would refine the tool to append the 'field_name' input definition, with all available fields as 'options' for the field. Then after selecting a 'field_name' we would append the 'value' definition'. This would make the tool truly dynamic, but means multiple tool calls from AI to fully understand the tool, and generating a form for the tool(in the case of ECA) would be impossible when using tokens. We could alternatively present all top level inputs to start, and only allow existing inputs to be 'refined'. This helps with the form challenge as we can have some sort of form element to display, which could be refined as additional values are provided. This also helps with AI make it clear all inputs a tool is expecting, and may reduce total calls required. Though this does leave definitions in a some what ambiguous state where it's not clear to a form or AI if an input definition is complete or still waiting to be refined.
This part is still TBD.
I merged the part for MapContextDefinition, still need to determine what is necessary for the ListContextDefinition. It's probably similar.
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.
I'm going to leave that failing test until I can get another set of eyes that this needs to switch.
We need to test this with json_as_string
and yaml_as_string
data types... hmmmmm...
michaellander → created an issue.
michaellander → created an issue.
michaellander → created an issue.
My understanding is artifacts are generally something that AI creates. In our case we also want pointers to things that may already exist and that we are modifying. Like if we ask AI to create a node, to me it's an artifact, if we ask it to load a node, is it still an artifact? Even if in both cases we intend to modify and save them. I just want to make sure we are using the correct terminology and would love to find some precedent somewhere.
michaellander → created an issue.
jurgenhaas → credited michaellander → .
jurgenhaas → credited michaellander → .
jurgenhaas → credited michaellander → .
Think maybe there's an issue here still.
Is this supposed to be == instead of !==?
if ($data_type !== 'yaml_as_string') {
return AppliesResult::notApplicable('The data type is not YAML.');
}
and
if ($data_type !== 'json_as_string') {
return AppliesResult::notApplicable('The data type is not JSON.');
}
Since we are saying to not deserialize them if they are one of these two types.
michaellander → changed the visibility of the branch 3540064-rename-arguments-and-provides to hidden.
michaellander → created an issue.
michaellander → created an issue.
michaellander → created an issue.
michaellander → created an issue.
michaellander → created an issue.
michaellander → created an issue. See original summary → .
michaellander → created an issue.
michaellander → created an issue. See original summary → .
marcus_johansson → credited michaellander → .
kristen pol → credited michaellander → .
@marcus_johansson, i'm good with it being merged
I've got an initial greenfield version of this to put up, that includes parts I cherrypicked from the Rules module. My version does not work through all the BC issues yet, which is the elephant in the room. I do have ideas around how we can sort out many of the known BC issues, but it's the part I am still nervous about. The requirements are not set in stone yet either and we should confirm there's agreement before we go too far into the solution.
michaellander → created an issue.
Let me follow up with @gantal. I know locally he was testing them against 3.0.x but he was running into a number of automated test failures and was trying to locate the issue. Will get you an answer shortly.