Merge Requests

More

Recent comments

🇩🇪Germany marcus_johansson

The code looks good, teardown function is safe from deleting random directories. I have tried to restart the test 5 times and its working every time, so it looks like a winner!

I'll wait for Scott to check and if he agrees, just merge it. Thanks @littlepixiez

🇩🇪Germany marcus_johansson

Long story short - you control it via the prompt, so you can in theory just write "Forward the user request exactly as it is to the search", but 9 times out of 10 you would get worse results then if you write a prompt on how the AI should think about what to search for.

For an explanation:

So that is agentic search, and is generally the prefered way of doing search. The thing where you just push the actual chat message is called naive RAG and usually does not work for longer threads.

In your above example, if you have a page called "How you use our chatbot", that would be linked to the "Do you have content on" start, because it explains how it searches, meaning that would always be ranked high and be given back for any question starting with this.

Or for a simpler example, take for instance the question "What is the difference in population growth between Austin, Pensacola, New York, Tokyo, Mumbai, Paris and Berlin?". If you don't happen to have an article on this specific topic and if you limit your search to five results, you will not be able to answer that. Having an agentic system that can do 7 different searches in that case, is just so much better.

Or for a more complex example, if you have a poem database with just poems and authors and someone asks "What is the difference in how the protagonists are described by Emily Dickinson and Sylvia Plath?". There is no world where naive rag would solve this, but you would rather need to do multiple searches to get an overview to answer such a question.

I think for naive rag we should make a solution outside of Assistants API, I have seen that people wants to have a solution for total anonymous "chat with my content" as well, and that would be better suited for it, since as soon as agents are involved its a security issue.

🇩🇪Germany marcus_johansson

Sorry, my brain is slow to process @jibran - what you suggest sounds very good. Configuration, reusable even when the agent gets updated and doesn't require a complex setup.

🇩🇪Germany marcus_johansson

@ronraney - could you try updating to either 1.1.4 or 1.2.x-dev and see if its fixed? If not, please open up this issue again.

🇩🇪Germany marcus_johansson

Thank you for the find - raising priority.

🇩🇪Germany marcus_johansson

One comment and then you can merge it right after.

🇩🇪Germany marcus_johansson

Thank you everyone - fixed Artems issue and verified it with Valery.

🇩🇪Germany marcus_johansson

Demoing

So for anyone that wants to test this with a RAG database in ddev, you can:

1. Add the following file in your .ddev directory: https://github.com/salsadigitalauorg/xb_metadata/blob/develop/.ddev/dock...
2. Add the following as a post-start hook in ddev: - exec: psql --user db --host pgvector -c "CREATE EXTENSION vector;"
3. Apply the following recipe: https://github.com/salsadigitalauorg/xb_metadata/tree/develop/recipes/xb...
4. Login and resave every media file
5. Attach the media finder agent wherever you need it and ask it to find images for you.

The fourth last step will:
1. Have a vision model look at the image and describe it very verbosely.
2. Index it.

A real solution

My suggestion for a real solution that works on multiple systems would be a function call that has configurations attached to it, where it can do graceful fallback.

So in the settings page of it you can, choose which kind of solution you want, which will include:
1. Search API RAG Search - you link it to a media index, which will be the best solution. This has multiple dependencies (AI, VDB provider, Search API, Search API AI, AI Provider)
2. Search API DB Search - you link it to a media index. This requires Search API.
3. Free text search with LIKE "%something%" or something similar - you link it to the media type.

On installation it will fallback to #3 and any recipe has to setup #2 or #1.

🇩🇪Germany marcus_johansson

So for the actual Pexels integration there is a module here: https://www.drupal.org/project/pexels_ai

This means that if the issue 🌱 [Discuss] Create a way to extend an agent Active is solved, we can add the information that is needed for whatever agents that needs to use this as a middle step or as a new agent.

🇩🇪Germany marcus_johansson

Thanks, merging this

🇩🇪Germany marcus_johansson

It looks good - the thing that I think should be added (but it might be implicit):

  • In the entity we should store the purpose as well and we should normalize this. So we know if a file is meant for finetuning or using with chat etc. If a third party module is built for finetuning, it should be able to filter out those files as an example.

Re traits: that is a pattern we have already for Chat, Embeddings etc, so all good.

Also how do we cover this scenario:

  • The user changes api key to another account?
🇩🇪Germany marcus_johansson

So the AI assistant was built with stuff like Claude Desktop, Claude Cli, ChatGPT, Claude, Loveable, Builder.io etc. in mind. So they store data that could be to large or sensitive to store in localstorage.

I'm wondering what the main use cases are for anonymous chatbots is? Is it mainly for a RAG chatbots, so like chat with your content? Is it read-only tool calling?

To get to the point where we have this, there are a couple of things that we have that I can think will reach the solution:

1. 🌱 Add ChatbackendInterface Active . That would be the big one, but its quite an undertaking.
2. Local storage as has been mentioned here.
3. For CSRF token, I'm wondering if we can store it via double submit cookie pattern?

The problem is that if you do not have any CSRF or any WAF, you open up yourself to be bombarded with requests that could make you lose money and become an AI proxy etc. Even if that is not the main purpose of CSRF tokens, that simple friction stops a lot of scripting. But I might be completely wrong here.

For the assistants part, we can do the changes so the PrivateTempStorage should never be in use.

🇩🇪Germany marcus_johansson

I think I know what happens here - the problem is that the return directly is mainly meant for a called agent to send back context to a callee without having to write out expensive and slow tokens if the tool response already suffices.

The problem when this is being used in the assistant context, then the history has the tool response in the history so when the new message is sent, the agent assumes that it already has the answer.

I've moved this to AI Agents, and I think we will have to look at if a user message exists after the tool result, then we should not return that tool result.

Good catch.

🇩🇪Germany marcus_johansson

Pushed directly to 1.0.x

🇩🇪Germany marcus_johansson

Thanks for the update, getting merged.

🇩🇪Germany marcus_johansson

Thank you, getting merged.

🇩🇪Germany marcus_johansson

Thank your Narendra for the issue and fix

🇩🇪Germany marcus_johansson

Follow up for next minor: https://www.drupal.org/project/ai/issues/3547139 🐛 Cache flush on VDB installation does not work Active

🇩🇪Germany marcus_johansson

Getting merged

🇩🇪Germany marcus_johansson

I've rebased this, however I get the following error when trying to upload a mp3 to AudioToAudio for instance.

🇩🇪Germany marcus_johansson

That's I'll merge this - I've also tested and verified that all methods are accounted for in the main classes or one of the extended base classes.

🇩🇪Germany marcus_johansson

hmm, I have rebased it and it still won't merge for me. Anyone with any clue?

🇩🇪Germany marcus_johansson

Sorry, nothing wrong, getting closed.

🇩🇪Germany marcus_johansson

Tested in 1.2.x. Both MRs are getting merged. Thank you everyone.

🇩🇪Germany marcus_johansson

Getting merged!

🇩🇪Germany marcus_johansson

Thank you Narendrar - I've added all those fixes.

🇩🇪Germany marcus_johansson

Great find and now I understand the issue. This is being merged and will also be backported.

🇩🇪Germany marcus_johansson

Do you think that it should still be possible to set style, width and height. Because if they are not auto, 100% and toolbar it looks really funny?

The toolbar is per definition for administrators/editors, so I think its ok that it has some more fixed styles and options?

I'm not an UX person, so it might be that I'm wrong, but I would opt for that happening when you choose the toolbar. If you do not agree with this, you can return it to RTBC and I'll merge it.

🇩🇪Germany marcus_johansson

The current implementation of summary is okish, but fails if you do not provider it with more context. I will anyway set this to in review and we can work on bettering that and also adding more assistant specific plugins in the future.

🇩🇪Germany marcus_johansson

I've done a code sweep and it looks good. Its only NITs or questions. Please have a look, but from code perspective it looks RTBC. I haven't had time to test it yet.

🇩🇪Germany marcus_johansson

We take this out, and aim for stable by 2.0.

🇩🇪Germany marcus_johansson

Thanks, merging this until there is a real solution in Drupal Core.

🇩🇪Germany marcus_johansson

In the sense that if you install the module via a recipe, and then boot up the site, you would get redirected (as admin) to the provider settings page to login/authenticate the provider.

I will try to test the MR with a modified recipe (of the ai_provider_amazeeio_recipe) to test out how it looks and feels.

Would this be outside of this feature/review. I'm asking mostly because this is one of the last three feature requests that needs to go in (or be skipped), before 1.2.0-beta1 release.

🇩🇪Germany marcus_johansson

I started doing review of this, but realized that per scotts recommendation, that this should not be a beta blocker.

🇩🇪Germany marcus_johansson

Getting merged.

🇩🇪Germany marcus_johansson

First in general:

Based on this information https://cookbook.openai.com/examples/how_to_count_tokens_with_tiktoken and asking ChatGPT, there is unfortunately not any base that is always bigger - because if so, we could always set to that to be sure. 3.5 and larger seems to at least be larger on western text.

It will also not be exact, just using TikToken. Gemini for instance, should use this API call, since it doesn't map to any base mapping in TikToken: https://cloud.google.com/vertex-ai/generative-ai/docs/multimodal/get-tok...

I'm wondering for 2.0.0 if we should make this a provider method?

Regarding this issue:

  • Anyway for now - we could just set it to the default chat model, since all models show up there - getting it from the embedding engine would always fail I think? The problem is that OpenAI has 3 embeddings engines, and TikToken has these, but since we produce the tokenizers based on the chat models, they will never match. The options is that the drop down should only show the embeddings engines?
  • But what I see is that there is a huge bug in general, that the Tokenize's getSupportedModels is using the readable name, instead of the key when trying to push it to getModelNameFromSimpleOption, which causes all models to show up, which is deceptive. Thus artems question.
🇩🇪Germany marcus_johansson

marcus_johansson changed the visibility of the branch 3531000-polling-status to hidden.

🇩🇪Germany marcus_johansson

marcus_johansson changed the visibility of the branch 3525540-error-call-to to hidden.

🇩🇪Germany marcus_johansson

Since no work is done on Add FFI PHP module for PHP 8.1 as well Active , we should install it manually for 8.1 (Previous Major)

🇩🇪Germany marcus_johansson

Thanks all - merging into 1.2.x-dev and backporting to 1.1.x-dev. You can expect a new 1.1.4 release on Wednesday if the manual review of it all works.

🇩🇪Germany marcus_johansson

Polling service has landed in 1.2.x of AI Agents, so work on this should be possible to start as per discussions: 📌 Make it possible to poll and see what the agent is doing. Active

🇩🇪Germany marcus_johansson

Could you verify from the logs that it does replace it with something/English?

🇩🇪Germany marcus_johansson

Currently its missing a plugin for both those kind of fields - could you make two feature issues for this? I will close this issue.

🇩🇪Germany marcus_johansson

I can still reproduce it in 1.2.x, by:

1. Install two providers with Embeddings.
2. Visit the embeddings form and switch the provider.

🇩🇪Germany marcus_johansson

The reference is gone, but this would still need addressing:

"Remove the config key in getFieldsForBundle in AutomatorsToolForm."

🇩🇪Germany marcus_johansson

Thank you, getting merged.

🇩🇪Germany marcus_johansson

I'll keep it as it is, we haven't had a test for this yet, and that detail is minor. Thanks

🇩🇪Germany marcus_johansson

This already exists now, but this issue was missed. Sorry about that - I will credit this, since the solution is similar to what exists in there.

🇩🇪Germany marcus_johansson

Seems to be failing

🇩🇪Germany marcus_johansson

Drupal CMS has integration with Klaro, I will close this issue.

🇩🇪Germany marcus_johansson

Since we don't use this anymore, I'll close this.

Production build 0.71.5 2024