Allow AI to validate changes before publishing

Note that this should also remove the hardcoded workaround for URI that was included in 🐛 XB AI: uri-reference type links not getting generated Active

So here is what I tried. In the ‘set_component_structure’ tool where we get the component structure generated by the AI I tried to pre-validate the components that are created by the AI using ApiAutoSaveController::post where I edited the `field_xb_demo` that is a part of the $all_auto_saves’s data array. This is by default populated from the UI changes that we make(example if we add 2 components in the UI then those 2 components are a part of this array).
The array structure looks something like :-

[
   'uuid' => 'da3155db-cda8-4b1a-81e1-5cf501c2065b',
   'inputs' => '{"text":"Press","href":      {"uri":"https:\\/\\/www.drupal.org","options":[]}}',
   'component_id' => 'sdc.xb_test_sdc.my-cta',
   'component_version' => '9454c3bca9bbbf4b',
   'parent_uuid' => null,
   'slot' => null,
   'label' => null
]

Where I tried to validate the props for the elements that we create and randomly coding rest of the values as arbitrary values.
What I tried was add a list of arrays with there props set against the input value in the array structure defined above and then letting the auto save do the rest of the stuff.
The response for the `post` method that I always got was {"message":"Successfully published 0 items."} which should not be the case as I am populating wrong prop data as a part of 1 component in the $all_auto_saves’s data array.
Now the real question is am I going in the right direction or I need to try something else? I don’t have much idea about how the Autosave manager works so it would be helpful to get some insights for it.
Also we might need or use any existing api which can validate the component structure before returning the response to the client side as it would not be possible for us to re-iterate in case we get some error on the client side. If we can do that in the backend then it would be easier for use to re-iterate.
Just for the context we get the following structure from the AI tool in ‘set_component_structure’/SetAIGeneratedComponentStructure.php file:-

reference_nodepath: [1, 0]
placement: above
components:
 - js.text:
   props:
    text: "Subscribe to our newsletter and be the first to know about exclusive deals and new arrivals."
    textColor: "Dark"
    textSize: "Normal"
 - js.button:
   props:
    text: "Subscribe Now"
    variant: "Solid"
    link: "#"

@utkarsh_33 can you push up the local work you have for #3 to branch? It would really helpful for me and/or others to help. Thanks

@utkarsh_33 thanks, I will take a look

I pushed up a MR that takes a slightly different approach.

The title of the issue says "validate changes before publishing" which I think would imply that the validation would happen after components are added to the page

In my MR https://git.drupalcode.org/project/experience_builder/-/merge_requests/1381 it validates the structure of the component before they are added to the page. It will make some tests fail in `SetAIGeneratedComponentStructureTest` because they do not use valid components

I am just getting to know this module so it is hard to hard to test manually, the AI seems to just make valid components. But I locally I did hardcode some exceptions is like "The cta2 for component %s contains a URL in for cta2 that uses "Click". Please use "Pop"'for the header link I can see the AI being creating a first version using "Click" and then remaking the component using "Pop" , So it does appear that `SetAIGeneratedComponentStructure` is a place we can throw exceptions and it triggers the AI to take the feedback an fix the components, neat!

Needs review for my general approach

Thanks @tedbow for the work. I tested it manually and here is my insights after testing:-

1. I get the correct structure once the structure is validated and the feedback(in the form of exception) is given back to the AI related to the problem in the prop structure. The exception is processed by the AI and the required changes are made to generate a valid structure.
2. One thing that AI does great is, say if we have 3 heading components in a page layout and somehow there was a issue with props structure of heading component and we give AI the feedback that the problem exists then it refactors all the heading components at once thus reducing our looping through other heading components as well(This might be expected behaviour as well but just documenting it as a win)
3. Another scenario that i tested was with a combination of components namely :- heading and hero-component. I intentionally told the AI the wrong structure for both the component by adding
   

   *  **CRITICAL** : If the component is *sdc.xb_test_sdc.heading* component then never include element props in it.
           *  **CRITICAL** : If the component is *sdc.xb_test_sdc.my-hero* component then use then never add cta1href props in it.
   

   these 2 rules in my page builder agent which lead to the wrong generation of the component structure.
   I gave the prompt Add 3 heading component and 2 hero component and then i got the error for the heading component which fixed the issue for all the 3 headings and in another round of validation it fixed for the hero component by adding the required props back to the structure. For now i think we can go with validating 1 component at a time and maybe open a discussion with @laurii and @tim.plunkett related to whether we want to validate all components at once. Not sure if this might require some changes in prompting or will it be taken care off just like it is happening now.

4. I had a question related to validation error @tedbow, are the errors descriptive enough to provide the AI with sufficient details about the reason for failure? In the above 2 cases it was able to tell AI that the 2 fields that i deliberately told not include were actually required. I will try to figure out scenarios related to wrong props structure and maybe test it against them, in the mean time if you have one in mind please let me know i can test that as well.

For now that's all i have in my mind.Thanks!

@utkarsh_33 thanks for the detailed response and testing!
re

are the errors descriptive enough to provide the AI with sufficient details about the reason for failure?

I do have idea to provide better information.

re #13 change the errors to show the actual paths for the yaml structure. you can see the changes made to the test expectations in this commit https://git.drupalcode.org/project/experience_builder/-/merge_requests/1...

@utkarsh_33 take another look and let me know what you think

Not sure why EntityFormControllerTest is failing. Since the MR only affects xb_ai I don't think it could be related

I am currently looking at refactoring it to use XB own validators. I hoping this will work so we don't have duplicate the logic as there is my current MR

tedbow → closed merge request !1381

tedbow → closed merge request !1357

@utkarsh_33 take another look. I change to new MR since you last reviewed.
Basically switched the approach to using XB's own validators rather than having to add specific validation in xb_ai class
for SetAIGeneratedComponentStructure this mean a conversion first to a `ComponentTreeItemList` and then just calling validate() on that. Basically the same for CreateComponent but there isn't conversion needed beyond what was already in the class

This has the advantage of running the actual validation that XB will run later on publish so less surprises later. Also we don't have to maintain duplication validation logic

@tedbow i tested the MR and here are my findings:-

1. The page builder works fine and the errors reported to the AI makes the required changes and there is no error reported on the UI side.For this i tested the same scenario which i mentioned in #12, with all the changes mentioned in #12.
2. There seems to be some problem related to the validation for the component builder. I made the following changes in PropsSchema.json :-
   

   {
       "name": "Image with example value",
       "type": "object",
       "$ref": "json-schema-definitions://experience_builder.module/image",
       "example": {
         "width": "1200",
         "height": "900",
         "alt": "Example image placeholder"
       },
       "id": "4ea2a14d-7828-467c-a505-063f006f1ebf",
       "derivedType": "image"
     },

   use this image prop instead of the existing image prop to feed the wrong example to the AI about the prop structure so that it produces wrong props.

   When we do the above change and ask the AI to create a hero component using Create a hero component prompt and then wait for it to do all the validation and return the valid prop structure it does return if we add a breakpoint in CreateComponent but proceeding further it gives an error in the UI An error occurred while processing your request. Please try again. and also you can see the errors in the reports which are basically the validation error. I didn't dig deep into it what is causing the issue but i have described the exact steps that took me to the error.

re #22 I think found what is going on. In \Drupal\xb_ai\Controller\XbBuilder::render It has special logic for these plugin classes

$map = [
        EditComponentJs::class => ['js_structure', 'props_metadata'],
        CreateComponent::class => ['component_structure'],
        CreateFieldContent:: class => ['created_content'],
        EditFieldContent:: class => ['refined_text'],
        AddMetadata::class => ['metadata'],
      ];
      $plugins = [
        'ai_agents::ai_agent::experience_builder_component_agent',
        'ai_agents::ai_agent::experience_builder_metadata_generation_agent',
        'ai_agents::ai_agent::experience_builder_title_generation_agent',
      ];

It is one of these plugins it seems to always assume getReadableOutput will be parsable yaml. So when \Drupal\xb_ai\Plugin\AiFunctionCall\CreateComponent::execute is called it first gets the validation error you mentioned "" Prop "backgroundImage" has invalid example value: [src] The property src is required" but then CreateComponent::execute is called again and now the component is created correctly and returns the yaml structure.

Problem is that in XbBuilder::render both the response from CreateComponent::execute are attempted to be parsed in
this loop

if ($sub_agent_tool instanceof $class) {
                  // @todo Refactor this after https://www.drupal.org/i/3529313 is fixed.
                  $output = $sub_agent_tool->getReadableOutput();
                  $data = Yaml::parse($output);
                  foreach ($keys as $key) {
                    if (!empty($data[$key])) {
                      $response[$key] = $data[$key];
                    }
                  }
                } 

The first one is non-yaml response which causes the error you are seeing.

I haven't had time to figure why the module needs the special logic for the these particular plugins and not for instance SetAIGeneratedComponentStructure which is able to output errors
like this
$this->setOutput(sprintf('Failed to process layout data: %s', $e->getMessage()));
and have the AI just call \Drupal\xb_ai\Plugin\AiFunctionCall\SetAIGeneratedComponentStructure::execute again to get the correct output.

So SetAIGeneratedComponentStructure does not have problem that failed attempted results in a plain string and successful attempt results in json string. But CreateComponent is set up not to work that way.

My suspicion is that if CreateComponent hit the existing error

$this->setOutput("The component with same name already exists.");

It would also fail to work correclty because it would try treat this as yaml in \Drupal\xb_ai\Controller\XbBuilder::render

There is test coverage in \Drupal\Tests\xb_ai\Kernel\Plugin\AiFunctionCall\CreateComponentTest::testCreateExistingComponentFails but this does not test the output with \Drupal\xb_ai\Controller\XbBuilder::render

Ok. I figured out what is happening

I ran this command 2x

create a code component with 1 text prop. Make sure the machine name of the component is text1

Of course the 2nd time it will cause an error because the machine name is in use

In

$output = $sub_agent_tool->getReadableOutput();
                  $data = Yaml::parse($output);
                  foreach ($keys as $key) {
                    if (!empty($data[$key])) {
                      $response[$key] = $data[$key];
                    }
                  }

It just happens that "The component with same name already exists." will parse not throw exception as $data = Yaml::parse($output); because it happens not to have a ":" in string or any other character that could cause a problem.
so Yaml::parse($output) === "The component with same name already exists." because it might work with simple strings

the never asserts that $data is any array so this unnoticed and

if (!empty($data[$key])) {
                      $response[$key] = $data[$key];
                    }

does not pass because it is not a string. so basically the output is ignore, I think by accident, not on purpose
but the validation string for the image src problem Yaml::parse($output) will throw an exception.

so we either need a try catch around Yaml::parse($output) or better yet figure why it can't behave like SetAIGeneratedComponentStructure and gracefully handle failed attempts.

or you could just make sure $output always starts with "error:" or if it is an error and check for that

For now to solve #26, I made sure that error in CreateComponent will still result in Yaml parsable string see https://git.drupalcode.org/project/experience_builder/-/merge_requests/1...

That is best solution I can come up with now without making larger changes

I am pretty sure at least CreateComponentTest will need to be updated for new error message format

I tested the scenario mentioned in #22 and it works fine now.

Ok. Ready for review again

@tedbow since we are validating the components and it's props while creating page/component, does it make sense to validate the component structure when we are editing any component? Say if i manually create a component using XB and then I intentionally feed the wrong info about the props structure as we did in #22 and then say Add an image prop to this component. then it will add the wrong prop structure to the component which might eventually lead to problems.
EditComponentJs is the tool which is used for editing the code components. If we validate the prop structure in this tool as well then this can well be avoided.
Marking it NW again for small changes and getting your thought on the above scenario.

Add Validation for AI generated component after editing task. 📌 Add Validation for AI generated component after editing task. Active Added a follow-up for adding validation after editing the component.

The changes looks good to me. I also tested the functionality and it does validate the components after creating them. Marking it RTBC.

Merged and backported!