Agreed. OK, I will aim to release this modules 1.0 with this functionality along with 1.1 version of AI Module on which it depends.
Flexibility of having block independent to View is important for design purposes, hence usage of View area (and thus View's own caching) would not be good for UX builders.
But the point is valid.
I suggest to add fingerprinting of content and caching the LLM response. That way, should the content fingerprint - the View's result - remains the same, the LLM call is bypassed and output of a cache is used instead.
Would that be satisfactory?
@scott_euser - you are 100% correct. The problem we have now is that it works for most scenarios (under 10-12 chunks), but fails for long nodes (and slow, local embedding LLMs) completely.
The solution I coded is actually imperfect either - it bypasses batch-chunking of long Entity on CRUD when immediate indexing is allowed. That potentially not only creates orphaned chunks, but actually fails to chunk and index long Entities.
I suggest we implement a robust solution:
- Now, an Entity is marked as indexed when child chunks are added to query. It should be the opposite - it should be marked as indexed, only when all the child chunks are processed.
- For AI Search index we replace Index status page with page reflecting chunking, not Entities. The user then will see non-processes Entities and - if any - an unfinished Entity, including number of unembedded chunks.
- We set robust handling of indexing for Entity CRUD, where we set message on failure where we offer to index manually on index page, or add to a Cron query. If user doesn't react, it will be added to the query (that way the re-indexing request is actually recorded and it will be processed eventually).
I have uploaded a patch which uses a suggested fix in a AI module.
Each AI provider supporting stream should be able to iterate any way it needs to, so I suggest this change: https://git.drupalcode.org/issue/ai-3493277/-/blob/3493277-extend-loggin...
This way the getIterator() works as a wrapper template, ensuring compatibility, which at the end of stream triggers the event. If a provider requires different implementation if iterator, it can override overrideGetIterator().
Merge request is ready.
Facade search for Milvus IDs from Drupal IDs was limited to 10, hence when the number of chunks per node was larger, some of these remained in the VDB index, because only the first 10 were found and deleted before saving new chunks.
Cool, I will check Milvus for this.
I went through all the tickets and have done/learned the following
Tickets mentioned in summary
- [Meta] Bring AI modules Tests, Error Handling and Documentation 🌱 [Meta] Bring AI modules Tests, Error Handling and Documentation Active has one unfinished child Add search api integration test 📌 Add search api integration test Active , which is reported by @scott_euser. I have asked for either implementing or assigning to me.
- SOLR 'boost' of results should find results that are not found by traditional SOLR search 📌 Check that SOLR 'boost' of results finds results that are not found by traditional SOLR search Active is reported by @scott_euser. I have asked for either implementing or assigning to me.
- Pass metric type to vector search function ✨ Pass metric type to vector search function Active is reported by @joshhytr. I have asked for either finishing implementation or allowing maintainers to finish it.
- Rename 'Boost' plugins to 'Boost and combine' plugins 📌 Rename 'Boost' plugins to 'Combine' plugins Active is reported and possibly also finished by @scott_euser. Asked for opening MR.
- Improve AI Search Module Indexing to Handle Long-Running Chunk Embedding Processes 📌 Improve AI Search Module Indexing to Handle Long-Running Chunk Embedding Processes Needs work is reported by and assigned to me, with PR comments by @scott_euser. I am going to implement these.
- [Meta] Features to focus on for v1.1 → - I have added a comment mentioning things which were done. Unless @yautja_cetanu wants to create child tickets, I suggest the remaining Features of Search module mentioned in the ticket as not required for moving out of experimental.
Other open bug reports
- Failure to insert into collection in Milvus 2.4+ with float32 instead of floatvector data passed 🐛 Failure to insert into collection in Milvus 2.4+ with float32 instead of floatvector data passed Postponed: needs info - asked to close or follow-up.
- Incorrect view mode configuration in RagAction::fullEntityCheck and hardcoded default value in Form 🐛 Incorrect View Mode Configuration in RagAction::fullEntityCheck and hardcoded default value in Form Active - merge request exists.
- Milvus-based server silently failing to insert to collection when indexing 🐛 Milvus-based server silently failing to insert to collection when indexing Active - stale ticket, warning issued for it to be closed on Tuesday, 4 February 2025.
- Error while indexing 🐛 Error while indexing Active - stale ticket, warning issued for it to be closed on Tuesday, 4 February 2025.
- Flush index if Embedding Strategy is set 🐛 Flush index if Embedding Strategy is set Needs review - possibly stale ticket, warning issued for it to be closed on Tuesday, 4 February 2025.
- Non-UTF8 encoding results in the exception "Failed to insert into collection" 🐛 Non-UTF8 encoding results in the exception "Failed to insert into collection" Active - possibly stale ticket, warning issued for it to be closed on Tuesday, 4 February 2025.
- VDB Provider only visible in Search API create Index form after cache clear 🐛 Pinecone not visible in seach_api only after cache clear. Active - asked @mrdalesmith & @scott_euser for heads-up and offered to investigate on Wednesday, 22 January 2025.
Suggested way ahead
I shall either create the fixes for PR of MRs, or close the tickets due to inactivity on dates provided above.
Hence
Assuming the work on stale tickets (where reporter or assigned developer do not communicate, they do not follow the recommendations, or ask us for help with finishing the ticket) start on Tuesday, 4 February 2025, it can be ready for PR on Friday 7 Ferbuary 2025.
@mrdalesmith & @scott_euser will you follow or should I investigate further on Wednesday, 22 January 2025?
@fishfree are you happy to close ticket as being fixed in Broken Byte-Pair Encoding (BPE) 🐛 Broken Byte-Pair Encoding (BPE) Active ? Without reactkons, I shall close this on Tuesday, 4 February 2025.
I believe we should not allow changing the embedding strategy for an index. The reason is that some of the strategies intentionally create a single chunk (so the search backend doesn't need to deal with multiple chunks per Entity ID), which can be expected and further processed as is. Changing the strategy would have unwanted consequences.
I suggest this had fixed the issue: https://git.drupalcode.org/project/ai/-/merge_requests/62
Proposal: close this as Done not Doing. I shall close this ticket accordingly on Tuesday, 4 February 2025 if there are no further objections?
If no new input is provided, this ticket will be closed on Tuesday, 4 February 2025 due to inactivity.
If no new input is provided, this ticket will be closed on Tuesday, 4 February 2025 due to inactivity.
@fishfree - I also cannot replicate. I plan to close the ticket on Tuesday, 4 February if nothing new is found/reported.
Crossed out feature requests were implemented/present in 1.0.x, commit 7809b7fd392ab3eef2c361437091aa91913a334e.
- - this is possible with existing module. It depends on VDB implementation (and capabilities).
- - the existing code does this.
- Views filters - working with RAG - Does work, but not good yet. - Probably need a specific Vector Exposed filter - this has to be done case by case, but we should/could probably start with disabling Views filters which actually do not work with VDB Search API index.
- Have the search respect user permissions. (permissions in Drupal is so huge computationally - So Drupal even does recalculation by itself). - This can be actually done using the same method as Drupal does, @yautja_cetanu it might be worth opening a new ticket.
@scott_euser is this ready to merge?
I agree, it is a bug. In order to move with [Meta] Bring AI Search out of Experimental 🌱 [Meta] Bring AI Search, Assistants and Chatbot out of Experimental Active , are you happy for me to do the code and assign PR to you afterwards or will you have time to do it in in ~2 weeks?
@joshhytr this issue was flagged is important for [Meta] Bring AI Search out of Experimental 🌱 [Meta] Bring AI Search, Assistants and Chatbot out of Experimental Active . We would like to have it fixed until 4th February 2025. Will you manage or are you happy for maintainers to help?
@scott_euser - are you going to implement or should I assign this to me and ask for PR when tode?
The code doesn't introduce any security issue, good.
However:
As pointed out by @MrDaleSmith, there is duplicity in:
public function runChatMessage(string $prompt, array $automatorConfig, $instance, ?ContentEntityInterface $entity = NULL) {
$text = $this->runRawChatMessage($prompt, $automatorConfig, $instance, $entity);
// Normalize the response.
$json = $this->promptJsonDecoder->decode($text);
if (!is_array($json)) {
throw new AiAutomatorResponseErrorException('The response was not a valid JSON response. The response was: ' . $text->getText());
}
return $this->promptJsonDecoder->decode($text);
}
Which should probably be fixed by:
$json = $this->promptJsonDecoder->decode($text);
if (!is_array($json)) {
throw new AiAutomatorResponseErrorException('Invalid JSON: ' . $text->getText());
}
return $json;
There also are repeated class_exists() checks:
$addressFieldOverrideClass = '\CommerceGuys\Addressing\AddressFormat\FieldOverride';
if (!class_exists($addressFieldOverrideClass)) {
return [];
}
Again as pointed by @MrDaleSmith, these should be in a single method, e.g. addressClassesArePresent(), that checks for all needed \CommerceGuys\Addressing\... classes at once.
Lastly, there is potential confusion with array_merge_recursive() in generate() - please check if it does what is expected: if LLM returns the same keys, array_merge_recursive() can cause unexpected nesting. For example, if $values has keys that already exist in $total, it may end up generating nested arrays instead of appending in a simple manner.
I have tested the '3480861-translate-with-ai-retake' branch with the following setup:
- Drupal 11.1.1
- drupal/language_access:^2.0
- drupal/ai:dev-3480861-translate-with-ai-retake as 1.0.x-dev (corresponds to commit 7fd2b16335c78caba729ade3454f04cd963460ab, above ai:1.0.2)
- Modules enabled: content_translation, ai, ai_provider_openai, ai_translate, key, language_access
Result:
- No Watchdog issues.
- Language access prevents access to language versions user has no access to.
- AI translations column contains translation link at '/node/{nid}/translations'.
- A translation is made when link is clicked.
@arwillame can you provide description how to replicate the error you observed?
seogow → made their first commit to this issue’s fork.
LLM will inevitably hallucinate links sometimes. Prompt engineering can get you reliable copy/paste result (on some models, including Gemini/Gemma, Claude 3.5 Sonnet and OpenAI 4o and later), but reliable rewrite is almost impossible to guarantee.
However, because we know the path constraints on Drupal and correct source URLs (in case of the search as a source), we can let LLM just choose as you suggested. That would work without hallucinations.
Don't you need to add prompts into ai_ckeditor.schema.yml?
The changes make the code more resilient to misconfiguration by:
- Properly handling the case when no model is configured.
- Properly handling the case when the default model string is empty.
- Only attempting to create the translation link when we have a valid model name.
This aligns with the issue's "Proposed resolution" of making the module more resilient to misconfiguration.
Tested - the missing permision was restored.
I have added an uninstall code, which makes sure the modules won't be enabled anymore.
Because there is no clear/safe way how to uninstall missing module, the removal should be done in 2.x, with clear statement at module's page, that the module must be updated to the latest 1.x version before upgrade into 2.x.
I have added suggested initial updates to the module.
Now an LLM process the user/agent communication and provides a summary of what was the chat about, what is the result and what could be done better:
When we are at it, this is worth including: https://github.com/cognesy/instructor-php
seogow → created an issue.
Good catch, I shall update the JSON statement.
The infinite loop is prevented here:
// Nothing to do, if we do not have content field or it is empty.
if (!isset($data['content']) || (strlen($data['content']) == 0)) {
throw new \Exception("Failed to record vector.");
}
If the calculation becames wrong for some reason (possibly a change in the API of Milvus), we shorten the content by 5% during each loop:
$difference = -max(1, (int) (mb_strlen($data['content'], 'UTF-8') * 0.05));
And when it reaches 0, exception is thrown.
marcus_johansson → credited seogow → .
TikToken doesn't fail, our implementation of it does. The easiest temporary fix is to disable overlap (put it to 0) and use averaging strategy. We need to better extend TikToken where this is accounted for.
marcus_johansson → credited seogow → .
This has been fixed in Alpha 2.
This has been fixed in Alpha 2.
Added AJAX, which doesn't slow View loading and provided Drush command for summarizing the source fields in bulk.
The difference between VDB and Solr (when only Keyword search is allowed in Solr) is that Solr will still work the same, if you add/remove a field. The field will just be returned as NULL if not populated. on the other hand, the vector might not just have different dimensions number (hard fail), but also different handling of returned items (soft fail in postprocessing in strategy).
However, I see your point with being able to choose different strategy for the same setup (e.g. during development for testing strategies' outcome). For that case, the index can be emptied (deleted/recreated) in VDB backend, whilst the index configuration stay (the deleting/recreating happens during saving changed index).
TYhis is a destructive operation and we must issue a pop-up warning before allowing saving the changed index.
Adding/removing fields in that case doesn't cause any issues - they will be taken care of during new indexing.
I believe index should not allow strategy to be changed when it was created. Index is a data structure defined by both strategy and embedding vectors. If something needs to change, a new index should be created.
The initial dev version was pushed to 'ai_search' branch.
The initial dev version was pushed to 'ai_search' branch.
Cosine is fine.
Qdrant.
Initial dev version was pushed into 'ai_search' branch of AI module.
Initial dev version was pushed into 'ai_search' branch of AI module.
Initial dev version was pushed into 'ai_search' branch of AI module.
The initial dev version was pushed to 'ai_search' branch.
The scope is to conduct a Vector search at indexed embeddedings and provide it as a Search API plugin, so the developer can:
- Create a Vector Search Index.
- Conduct a search on the index via Views.
If there are limitations as to what Vector Index can do, similarly to Search API Solr module, these should be reflected upon rendered View.
Reasoning: This module's aim is to work out of the box and be plug-and-play replacement for any other Search API index technology. However, there is no limit on implementation of Vector search:
- It can be combined with keyword search to provide even more accurate results.
- It can work as pre-selection/ordering tool of entities and then be fed to reranking.
- ...
But that are advanced implementations, which might (or not) be implemented here, after when 1.0 is out.
I hope the above makes sense.
Combination of indexes should be IMHO separate module. There is a Drupal 7 module Search API Multi-Index Searches → and it calls for Drupal 10 for sure.
However, I believe combining search indexes is out of scope of the AI module?
I am starting this right now. The idea is to be in line with the rest of the LLM APIs supported by the AI module. Each Embedding API must either support a default, set by the AI module abstraction, or provide its own defaults, which the module gracefully uses if that API is selected.
In the development version of the Search API AI for the AI module, I do not plan to have any GUI settings for chunking. I plan for it to work out of the box as a plug-and-play replacement for the Solr and DB search. For an index, this means that once it is created, it cannot be changed, though re-indexing may be triggered as usual.
I am happy to take more feature requests as the Alpha is out, but I do not want to chase too many birds at the same time.
I hope the above makes sense.