Account created on 17 August 2022, about 3 years ago
  • Software Engineer at Acquia 
#

Merge Requests

More

Recent comments

🇮🇳India utkarsh_33

Tested this manually and it works fine. Also code changes looks good to me. Marking it RTBC'ed.
I used Using Page Builder, add a two-column layout with 3 Drupal icons in the second column. Exclude column_one slot from response as i was not able to reproduce the issue with Using Page Builder, add a two-column layout with 3 Drupal icons in the second column prompt.

🇮🇳India utkarsh_33

This issues is already fixed as a part of 🐛 Canvas AI: Agent should pass values to image props correctly for JS components Active where we added an exception which will cause a validation error and eventually AI will fix it. This is intermitent so just an exception will help.

🇮🇳India utkarsh_33

The changes are fairly simple and makes the $agent->setTokenContexts() more readable and maintainable. Marking it Rtbc.

🇮🇳India utkarsh_33

utkarsh_33 made their first commit to this issue’s fork.

🇮🇳India utkarsh_33

I was testing this with the prompt Create a component to render the admin menu as a mega menu with linkset disabled. For the first time it gave the correct message that the user should enable the linkset config to create a component. I prompted it that I have enabled it without actually doing that. For an instance it again gave the message that the linkset is not enabled but eventually it created a component using some different approach which i think is not correct as it does not use what we wanted it to use and also the component does not render anything.

Making it NW.

🇮🇳India utkarsh_33

utkarsh_33 made their first commit to this issue’s fork.

🇮🇳India utkarsh_33

The fix in the issue ensures that we only edit the component when we are on /code-editor/component route. If i already have a component named test1 and i give another prompt to create another component named test1 then it will create a component but with a different name using it's own intelligence.

Steps for testing that i used were:-

  1. Create a component named test1
  2. Try creating another component using Can you please create a CTA component named test1 that allows me to use images with a bit of text and to include a CTA to link to other pages and some background colour..
🇮🇳India utkarsh_33

utkarsh_33 made their first commit to this issue’s fork.

🇮🇳India utkarsh_33

utkarsh_33 created an issue.

🇮🇳India utkarsh_33

I have enforced a more robust instruction on how it should map the select list for the tailwind classes.
Now it generates the list which contains human readable options to select from instead of tailwind classes which are mapped in the enum.
Attaching the SS.

🇮🇳India utkarsh_33

utkarsh_33 changed the visibility of the branch 3547666-ai-should-not-props-for-tailwind-classes to hidden.

🇮🇳India utkarsh_33

utkarsh_33 created an issue.

🇮🇳India utkarsh_33

utkarsh_33 created an issue.

🇮🇳India utkarsh_33

This is duplicate of 📌 "Thinking" status persists after a process terminates with an error Active . Closing it as Duplicate.

🇮🇳India utkarsh_33

Tested it manually and confirmed that all the feedbacks are addressed. Also code changes looks good to me so marking it RTBC.

🇮🇳India utkarsh_33

Just a small push back. Marking it NW.

🇮🇳India utkarsh_33

I tested this manually and can confirm that it works as expected. I also tested all the other component generation tasks and everything seems to be working as expected.

🇮🇳India utkarsh_33

So 🐛 AI may generate Tailwind classes only compatible with older Tailwind versions Needs review issue has made the need of this issue more prominent. According to our discussion with Jamie, Markus, Tim, Narendra and Aanand we agreed with not proceeding the hardcoded approach in this issue.

Here's what i suggest until we get a better and a consolidated approach for this, We can find a way of reading the files that are present in the
/canvas/docs/user/src/content/docs/code-components and create temp file/set of instructions(generate a guide for LLM) which can be passed as a context to the LLM and we can avoid the usage of the libraries that are not present in Canvas.

Temporary advantages of this approach are:-

  1. This will temporary solve the problem of previews being broken.
  2. This approach is also better than the current approach as it removes the duplication of the instructions and it will also get the latest changes in the documentation.
  3. This might also be helpful in handling issues such as 🐛 AI may generate Tailwind classes only compatible with older Tailwind versions Needs review which might otherwise need separate instructions in the prompt thus increasing the instructions.
🇮🇳India utkarsh_33

So this might fix the issue for now but we need a better and a concrete way of instructing/providing LLM with the libraries present in the Canvas ecosystem. 📌 Canvas AI: Give AI more context about the libraries present in Canvas Active Is the issue which i opened up in which we tried to instruct the LLM related to the libraries present in the Canvas.

🇮🇳India utkarsh_33

utkarsh_33 made their first commit to this issue’s fork.

🇮🇳India utkarsh_33

I tried the prompt mentioned in #3 but without creating a content type for tours. My expectation was it would fail and maybe give user a message that the content type does not exist, but it created a code component which eventually shows unable to load data.
Attaching the SS for it:-


I think this can be handled in the tool which checks for the fields.Marking it NW again.

🇮🇳India utkarsh_33

As per discussion with @tim.plunkett, @NarendraR and @balintbrews for now we can go with the hardcoded approach but we need to figure out a better way to dynamically parse the docs and pass the required libraries as a context to the LLM.
Added the descriptions for each libraries as per the request. This is ready for review again.

I have created a follow-up 📌 Canvas AI: Find a better approach of passing the libraries supported by canvas as a context to LLM Active .

🇮🇳India utkarsh_33

I tested this but this is preserving the chat history even if i explicitly close the AI panel. I think that's not a required behaviour so marking it NW again.

🇮🇳India utkarsh_33

The prompts that i used for testing are:-

  1. Create a component that has a formatted text area and gives me an option to change the background colour using a list of colour options
  2. Can you please create a CTA component named test1 that allows me to use images with a bit of text and to include a CTA to link to other pages and some background colour.

The output must contain the imports for cn and FormattedText.

🇮🇳India utkarsh_33

utkarsh_33 made their first commit to this issue’s fork.

🇮🇳India utkarsh_33

Discussed with @tim.plunkett and @NarendraR and we agreed that all the above mentioned issues can be fixed in a follow-up, so marking this one as RTBC.

🇮🇳India utkarsh_33

This is working as expected and also the code changes are looking good. Marking it RTBC.
I tested it with 3-4 prompts one of which is like:-
Create a homepage for a spring sales website which contains 3 card components placed adjacent to each other and an image component showing a poster of LIVE SALE and a catchy heading for the website.

🇮🇳India utkarsh_33

I tested it manually and here are my few findings:-

  1. When we do any operation that calls a subagent(for instance page builder) then we only get the AiAgentFinishedExecution for the orchestrator agent not for the sub agents, which causes the different sub-steps (in this case page building(step1) > Title generation(step2) > Metadata generation(step3)) not getting completed status or the green check mark before them. All the subtasks have to wait until the orchestrator is completed executing its final steps.Attaching the SS for reference:-

  2. Currently we also get the same messages from the sub-agent and the orchestrator agent twice. For example if we create a component then the component agents returns a message of success which is same as what we get as a final message from the orchestrator. We might need to figure out a way so that there is only 1 final message from either of the agents.Attaching the video demonstrating the scenario.
  3. Also a small but not so important thing that i noticed is that when we submit the prompt then for a moment the loader loads which is then replaced by the Thinking block. This is not a blocker but just pointing it out.

Apart from this all the functionalities are working as expected.

🇮🇳India utkarsh_33

For testing this the prompt that i used was :-

Can you please create a CTA component named test1 that allows me to use images with a bit of text and to include a CTA to link to other pages and some background colour.
🇮🇳India utkarsh_33

I have pushed the initial work for this. The problem with this is that when i try to select a different value from the dropdown present in the props it does not update the values/color. I will try to fix that issue.

🇮🇳India utkarsh_33

utkarsh_33 made their first commit to this issue’s fork.

🇮🇳India utkarsh_33

I tested it manually and it does not give the unnecessarily data fetching related messages and also verified that the 📌 Enable AI agents to generate components that fetch menus Active and 📌 Enable AI agents to generate components that fetch data from JSON:API Active functionality are not impacted due to the above changes.

🇮🇳India utkarsh_33

I agree with @akhil babu. We should wait until 🐛 XB AI: User selection should be taken into account while building page Active gets in.

🇮🇳India utkarsh_33

@tim.plunkett I have successfully rebased it. Marking it RTBC again.

🇮🇳India utkarsh_33

utkarsh_33 made their first commit to this issue’s fork.

🇮🇳India utkarsh_33

A simple yet effective change.

🇮🇳India utkarsh_33

I tested this and the active_component_uuid is now getting passed as a request parameter. Also the changes looks good to me so marking it RTBC.

🇮🇳India utkarsh_33

I can confirm that the latest changes fixed the issue reported in #15. I also tested the prompts mentioned in #3 and all of them are working along with prompt for a custom field like Create a component to show testing field of all the test1 where test1 is custom entity type and testing field is the custom field.
Also the changes looks good to me overall so marking it RTBC.

🇮🇳India utkarsh_33

Re #5 🐛 Canvas AI: Request parameters are not passed correctly. Needs review :-

This was fixed in #3545378: Canvas AI: Agents fail due to missing entity type and entity ID parameters and it seems to be working fine when I tested it just now. Are you still experiencing the issue on your side?

This was only fixed for the /canvas/editor/canvas_page/1 url as this contains the required params and their values are correctly extracted from URL. But now the Canvas has a new default page /canvas/ where the above params are not present.

If we are restricting all operations to the Canvas Page entity type, wouldn’t it be better to handle this at the orchestrator level by adding a prompt like the following, rather than sub agent level

I intentionally removed it from the orchestrator as that's already present in the subagent(page builder, title and metadata) that require that context and not all the subagents require the entity context for example component creation and editing.
Removing it from orchestrator also reduces the context sent to all the subagent which don't need them.

🇮🇳India utkarsh_33

I tested the issue and it seems to work partially. The following are the commands that i used and the SS for their results:-

  • Create a component to show title of the latest page -> Always worked
  • Create a component to show title of all the articles -> Worked only once out of five tries
  • Create a component to show title and body of all the articles and pages -> Always worked

Prompt 1 SS:-

Prompt 2 failed SS:-

Prompt 2 passed SS:-

Prompt 3 SS:-

The issue related to the article list not loading lies in:

{data && data.length > 0 ? (
          data.map((article) => (
            <div key={article.id} className="article-item">
              <h3 className="article-title">{article.attributes.title}</h3>
            </div>
          ))
        )

Where it some times uses {article.attributes.title} which is wrong and when it uses {article.title} then the ;ist is correctly rendered.

Apart from the above I also tested the remaining functionality related to code component and everything seems to be working as expected.
Marking it NW to fix a small issue.

🇮🇳India utkarsh_33

utkarsh_33 changed the visibility of the branch 3544132-give-ai-more to hidden.

🇮🇳India utkarsh_33

utkarsh_33 changed the visibility of the branch 3544132-give-ai-more to hidden.

🇮🇳India utkarsh_33

utkarsh_33 changed the visibility of the branch 3543093-xb-ai-ai to hidden.

🇮🇳India utkarsh_33

Tested the issue and it was failing on 1.x see the SS:-

But adding the try catch block fixes the problem of processing further if 1 tool fails. I tried 7-8 time the same prompt as in the image and every time it successfully generated the page.

🇮🇳India utkarsh_33

utkarsh_33 changed the visibility of the branch 3544277-tailwind-directives to hidden.

🇮🇳India utkarsh_33

Successfully migrated to canvas project. This can be reviewed now.

🇮🇳India utkarsh_33

utkarsh_33 changed the visibility of the branch 3540725-xb-ai-fails-to to hidden.

🇮🇳India utkarsh_33

Re #7: Discussed it with @laurii and decided that we need to covert it to camelCase as per the latest changes in XB. So this does not need any actions.

🇮🇳India utkarsh_33

Tested it with the following props:-

  1. Create a promotional banner
  2. Create a hero banner
  3. Create a alert banner

And it creates human readable names for the component. Marking it NR.

🇮🇳India utkarsh_33

I tested this with the latest changes and it works 10/10 times for me now.
The prompt that i used was :-
Create an Alert component with a message and an optional type prop (info by default).
It does name the props sometimes as type and sometimes as alertType but also does use them with the same name across the code, which makes it more consistent. We can also change the alert type from the props and see the colour of the alert change according the value selected in the prop.

🇮🇳India utkarsh_33

I tested this and i can confirm that functionality wise it's working as expected. If we get an approval on the usage of indexedDB then happy to RTBC it.

🇮🇳India utkarsh_33

Modified the prompt and tested it manually. It works as expected. Marking it NR again.

Production build 0.71.5 2024