I think you can probably use this one https://www.drupal.org/project/ai_tmgmt/issues/3481001 ✨ Fallback to ai_translate configuration Active but you'll also need the patch in AI Core module. Then I guess as you CRUD your taxonomy you could programmatically update those prompts from AI Core.
If that helps let us know and you can mark as fixed.
Looks like composer failure is related to 📌 Automated Drupal 11 compatibility fixes for facets Active which is the case because this module has a dependency on facets.
Composer failure is unrelated to this
On the deprecation, it seems you can only deprecate within hook_theme but search_api does not implement hook theme as its theming list_item which is a hook_theme coming from elsewhere, but as noted in #18 its probably never touched and should not be harmful since now we use elements that are heavily covered by admin themes. A release note is probably all we can do.
Thanks for the feedback!
- count($nested_properties) > 1: Yes I can see this issue. I updated it to revert back to the status quo. I raised 🐛 Expanding some field types in the add index form result in nothing getting expanded Active as a follow-up as its a bug unrelated to the UI change as its also existing in search_api dev branch
- Sorting: yes I can see this too, completely right! The old method does not work for table rows though since the nested properties are rows just like any other. So I updated it to sort the properties before creating the rows instead.
- Added this for SOLR 📌 Prepare for updated add fields to index UI Active as per slack thread.
- Updated test coverage to change for expand buttons (x2 levels) and visits those expanded levels.
scott_euser → created an issue.
Probably better as 'Postponed' but setting to needs review as it would be good to get this reviewed and ready to go prior to ✨ Improve the Search API admin UI for adding/editing fields Active merge.
scott_euser → created an issue.
I don't know much about ECA myself but sounds sensible. If there is anything this module needs to change to be ECA compatible (that isn't TMGMT generic, otherwise should be raised there) I think we should keep it as a separate issue.
So I believe that means all bits you have raised are either resolved or not related to this particular module and this is now ready to merge, correct?
In any case thank you for all the time you put in to testing this. It's very valuable and much appreciated contribution!
scott_euser → changed the visibility of the branch 3480321-second-level-menu to hidden.
Before:
After:
Not sure if this is the right approach, but this splits the menu and and expand so you can use both.
Screen recording:
scott_euser → made their first commit to this issue’s fork.
Was trying to see if there is a way to add something into ViewTestData::createTestViews() to essentially:
- Replicate the import & resave via Views UI
- Assert that the config in test file matches the config after resave
Got stuck on (1) of course... maybe need to sleep on it a bit. Would welcome any tips/pointers in case anyone has bright ideas to help explore that direction. Maybe its going to be too heavily requiring mimicking form/form state that happens via UI, in which case perhaps not feasible...
Idea being, if we could have a check that config is as expected, we could catch any new additions that cause config to become outated.
I checked Search API SOLR and the method it extends is untouched (just buildForm) but good spot with the method name! I'll create a follow-up to make it easier for SOLR to avoid having to do that in the future
Actually follow-up not needed here, SOLR can be refactored to not need to extend the IndexAddFieldsForm because its in control of the entity->getDataSources() itself which is the only thing it changes in the one method it overrides as far as I can see. Anyways unaffected either way now that you have reverted the method name change.
Thanks!
- I ran through the process again after your changes, all works fine still, attempted nested fields, rendered items, custom values, base fields all fine.
- I checked Search API SOLR and the method it extends is untouched (just buildForm) but good spot with the method name! I'll create a follow-up to make it easier for SOLR to avoid having to do that in the future
- I also checked the two elasticsearch modules + the typesense one; only typesense does a form alter, but only to add a new separate warning element and so is unaffected
- Then in AI Search (submodule of AI) we extend a fair bit but just calling parent buildForm, and not changing the add field process + anyways I maintain that so if there is anything eventually I can adapt
So looks good to me, thank you! Also thank you @rajdip_755 and @borisson_ for reviewing.
Ah whoops, cross posted on that last comment, was meant for ✨ Improve the Search API admin UI for adding/editing fields Active sorry!
Thanks for the quick review & merge!
Re the comment on raising an issue in Core, I raised that here now and added test coverage to demonstrate: 🐛 Ajax cannot be triggered from within a table row Active . Should I create a new MR here to update that comment?
I added test coverage to demonstrate the issue. Needs an actual fix to the issue still.
scott_euser → created an issue.
Probably related to this I think: https://www.webomelette.com/ajax-elements-drupal-form-tables
Yep tests passing now after rerun
Sorry for the delay here, was focused on a bunch of contrib projects for a bit! Okay test only should fail now again, I had typed the alter hook name wrong, whoops!
There seem to be some unrelated test failures that have nothing to do with Views after merging latest from 11.x. They pass locally when I rerun... maybe @smustgrave you've seen similar on your travels through the issue queue lately? For now triggered rerun again.
Okay got it, here is how you can see the results:
$text = '我是一名退休教师,退休前我很难抽出时间去运动和锻炼,那时感觉自己处在一种亚健康的状态。退休后,我的时间充裕了,经过几年的坚持锻炼,我现在的身体素质比退休前还要好。单位每年组织体检,我的各项指标基本可以达标。在这里,我呼吁老年朋友多运动,多锻炼。立冬时气温降低,老年人外出运动时一定要穿着保暖透气的衣服,运动前做好准备活动,先热身、后运动。选择运动方式时,一定要适合自己。在运动中释放自己,保持好心情。';
/** @var \Drupal\ai\Utility\TextChunker $chunker */
$chunker = \Drupal::service('ai.text_chunker');
$chunker->setModel('openai__gpt-3.5-turbo');
$chunks = $chunker->chunkText($text, 100, 10);
dvm('overlap 10:');
dvm($chunks);
$chunks = $chunker->chunkText($text, 100, 0);
dvm('overlap 0:');
dvm($chunks);
to compare with overlap to without overlap.
Side note to anyone following along: I think this necessarily has a minor shift in the overlap but reviewing the changes to the test coverage its still sensible overlap and I don't think it needs any triggering of reindexing as its minor and vectors will be roughly the same (and since of course dimensions, etc don't change a query against it will continue to work fine).
Actually I see how to reproduce this, if I take your same example and add any overlap I get gibberish in later chunks rather than proper chunks reflecting the original text.
Thanks for retesting!
The continuous job is still triggered however, so after manual translation is finished, a new job is still created for each translation.
I suspect that that is also TMGMT core; if you have a continuous job set in Deepl and you manually create a job, do you have both? I think yes, because if I understand the logic in TMGMT right, the Jobs are independent of each other:
- You have a continuous Job that the content item matches the criteria for: it gets Job Items created
- You created a new independent Job, so it gets Job Items attached as you selected them
Possibly helpful:
- #2866969: Content changes tracking on our own instead of continuous job mechanism → in extension suite
- #2679949: Never allow duplicate submission of an item while in process → in TMGMT core
But I don't think its something we can address here; the Job and Job Item creation happens before ai_tmgmt module kicks in, we only respond it once TMGMT core has done its thing.
Cool glad that helped! It's really tough to know where a problem is when we've effectively got 2 ecosystems of modules in play and this bridging them together!
Excellent great to hear!
scott_euser → created an issue.
Thanks!
Wonder if we should pass the whole database settings instead of just the collection to avoid another future interface change.
Some phpcs issues flagged as well.
We'll need to do a coordinated release in all implementations of the interface.
Updating issue summary to cover 🐛 Translating more than one source mixes up translations Active as there is too much code overlap to do that in two separate MRs.
Screencast of this solved now in this MR:
Interesting one! I'll have to solve it over at 🐛 Reduce redundant batch runner Active as its heavily overlapping code.
Had a quick look; sounds like you're using cron to make AI Automators run, and you want the cron of TMGMT to run after that. You'd need some way to make AI Automators add to the TMGMT queue only once its done its job. Ie, some sort of post completion action there you'd have to code (or hire someone to code).
Probably not from this module, probably from TMGMT core, e.g. something like this 🐛 Infinite loop for entity reference fields translation Postponed: needs info could be the cause. To confirm, if you try with https://www.drupal.org/project/tmgmt_deepl → do you have the same problem?
Not sure I can help here as I don't use Automators but I think you are probably in the wrong issue queue for this one. TMGMT handles job submission and AI Automators handles filling in fields. If it's filling in fields too late eg post save TMGMT will always have problems.
Yeah it's simple cron not ultimate cron that has the ability to manually trigger the queue worker. Hope that helps!
Thanks! And then I just need 4. The expected chunks from comment #6 because I need to understand what's wrong with the chunking (ie, to me what you show looks fine, you asked for chunks of 100 tokens with no overlap and it gave you three chunks).
Okay this works for LibreOffice, but am thinking maybe https://github.com/ckeditor/ckeditor5/issues/17309 is stopping it from working from Word itself. Will see, maybe some kind soul at ckeditor side will nudge me in the right direction (or maybe that itself is the underlying issue)....
Follow-up added to remove lock to twig <= 3.14.0 here: 📌 Remove lock to twig <=3.14.0 once #3485956 in core is resolved Active
Yeah we had this issue in the footnotes module ( 🐛 Fix deprecated spaceless issue Active ) because spaceless is quite important to avoid inline citations having spaces around them. Perfectly happy though to have a mini footnotes_spaceless twig filter to cover that. If it turns out its needed by more modules, we can always switch to depending on something in contrib.
scott_euser → created an issue.
scott_euser → created an issue.
scott_euser → created an issue.
Okay here is a working sample code (replace MYMODULE with your module name that you drop this in):
function MYMODULE_page_attachments_alter() {
$text = 'Your text here that is failing';
/** @var \Drupal\ai\Utility\TextChunker $chunker */
$chunker = \Drupal::service('ai.text_chunker');
$chunker->setModel('openai__gpt-3.5-turbo');
$chunks = $chunker->chunkText($text, 100, 0);
dvm($chunks);
}
Really you can use any hook or drop it in an existing hook. I just randomly chose this one because it is triggered once on every page load.
Hmmm actually let me look further, could be that I need to update the code snippet
You'll need to run it within something (e.g. a existing function or method) and then eg dvm() the result. Thanks!
scott_euser → made their first commit to this issue’s fork.
Nice thanks!
Sorry I missed this. It is meant to show the admin account BUT you cannot remove the administrator role (unless you grant that permission via Role Delegation). So for example if you give 'Content Editor' permission to delegate 'Content Editor' role, you can change roles of admin to add 'Content Editor'. You cannot remove roles you do not have permission for, so the 'Administrator' role is not possible to remove/protected.
scott_euser → changed the visibility of the branch project-update-bot-only to hidden.
Thanks for the patch!
Questions (sorry for my lack of understanding of date recur)
- So no modification needed in UnifiedDateManager::getNodeDateFields() to add a new field type?
- The presave is to have the date change when the recurrence of a date happens right?
Next steps I believe would be converting to a merge request and adding test coverage in a test sub-module (I believe it needs to be a test submodule to avoid a dependency (kinda of like how we have view_ajax_history referenced from https://git.drupalcode.org/project/viewsreference for tests only, but not a hard dependency).
Awesome thank you!
Keen to get this in: if someone can do a final review would be much appreciated!
Okay its working now with 📌 Discuss: Unify translate operations over ai Active - probably some coding standards to clean up. But first we need that issue there resolved and there are some critical issues that are blockers still for it, for example it returns escaped html, so we cannot use it here yet.
I added some feedback as well after testing this in
✨
Fallback to ai_translate configuration
Active
There are also some comments from Marcus to be addressed still
In general though it seems like the right direction, and is really helpful to have, thank you for the efforts here!
Thank you!
scott_euser → made their first commit to this issue’s fork.
Thanks for raising. I guess you're trying to get the LLM to return the URI? It'll depend on prompt and model how reliably it'd do that. Within the process the vector database results give you the actual entity which would be a more reliable way to get the URI.
Since you've got it working though, seems like we should close and if someone has a similar issue maybe they can reopen with further steps on latest codebase.
Ready for review:
- Dependency optional (suggest area of composer)
- Notice message shown in chunk preview render if dependency is not there
scott_euser → made their first commit to this issue’s fork.
Can someone provide clear steps and example content that is failing in order to be able to reproduce this. When providing example content, please:
- Provide just enough e.g. to demonstrate what should be e.g. 2 chunks but isn't get broken into chunks (if I understand the issue right)
- The chunk size to test with
- The current chunked results (if any)
- The expected chunks
From @seogow's description, it sounds like the problem is at the chunker level, which means here is a quick sample code you could start from to demonstrate the problem:
$text = 'Your text here that is failing';
/** @var \Drupal\ai\Utility\TextChunker $chunker */
$chunker = \Drupal::service('ai.text_chunker');
$chunks = $chunker->chunkText($text, 100, 0);
Do you have a theme in mind that is particularly problematic (or a way I can fake it within a core theme)? I tested with stark and olivero as front-end theme and tried fiddly with the background on the body element within the iframe but couldn't get it to look bad.
Thanks! Retested and works well still in D11
Hmmm if that's the case that its the views_data causing the issue, I would expect that 🐛 Views handler loading should respect configuration Needs review in core should solve the issue then.
Referencing issues this affects around Search API & AI modules (which has AI Search using this) becoming part of Drupal CMS.
I have also been working on ✨ Improve the Search API admin UI for adding/editing fields Active which needs review. It should significantly improve the user interface for adding fields.
This is now ready for review:
- Screencast below
- Explanations that aren't suitable as comments in the code are added as comments in the MR
This should bring it much more in line with Drupal and automatically inherit theming from Gin/Claro/etc given it uses nothing bespoke to search api any more.
Is Seogow working on the Qdrant side of things still? Might make it more clear
Combine lexical + Vector search in one query (qdrant does this) (Note: BM25 is not the same as solr, etc). This is for when we provide context to an LLM, not just the boost with with AI functionality
Can you detail this one a bit more? Sorry I'm so used to the drupal terminology I don't know what this means in a practical sense. Also can't see anything in Qdrant docs that shows how its different from filtering in Milvus for example, so would be good to get pointed in direction there.
I think we also need to get someone who uses e.g. Mandarin or Japanese for example to work on 🐛 Broken Byte-Pair Encoding (BPE) Active in the embedding strategies.
Adding to issue summary that prompts should accept arguments or wouldn't really be very usable e.g. for RAG type things where multiple variables can get replaced. See also earlier comment in #6:
Recently I implemented arguments UI, which enables to define variables the prompt accepts in GUI ✨ Allow to pass variables to aiprompt programatically Active
Note last commit fixes https://www.drupal.org/project/search_api/issues/3022881#comment-15842781 🐛 Clicking on + icon makes the page scroll to the top Fixed
Yep as per #15 I could still reproduce it, but I am fixing it in ✨ Improve the Search API admin UI for adding/editing fields Active . The problem was drupal ajax attempts to refocus on the expand button, but expand is replaced by the collapse button. We need to maintain a 'data-drupal-selector' element that is consistent between the two so drupal ajax knows its meant to be the same focus target.
This appears to be fixed because of 🐛 Dialog focus is returned to dialog instead of first tabbable element on dialog AJAX update Fixed which changed exactly these lines of code. If you can still reproduce please reopen.
Thanks! Just gave your changes a quick run adding a new index again and all looks good. Code perspective, no concerns with the changes. And thanks for the quick review!
Just WIP, haven't started nesting... will I am sure need test updates. But in case anyone has any early feedback:
Before:
After:
Updated issue summary to reflect non-install route.
Suggesting there as well that the user selects which fields to run bulk action on.
Note that ✨ Ability to enable entity reference purger settings to every reference field that exists Active has a way to select all fields.
Interesting one! It's a big assumption though that the same title would be an intentional replacement. The bundle check probably partly mitigiates that (e.g. an event and article with same name being a common thing we see) but still I would imagine that:
- Maintainers guidance should be seeked out first before putting more effort to see if it's something they'd be interested in.
- At least it probably needs to be opt in via module settings rather than default as I expect it'd otherwise be a hard to track down unexpected side effect.
Updating issue summary to match the code which gets all revisions rather than just 'forward' revisions. Since the MR already covers the scenario of reverting to an older revision as well.
I think this would be handled by 📌 Don't add reference to purge to queue if already in queue Needs work . I know it's not exactly the same cause but since the resulting problem and symptom are the same I would expect it gets solved there.
Just had a look, yeah duplicate job exception thrown if you try to add the same job twice. Would it be worth considering requiring advanced queue in order to use the queue feature here?
Follow-up created here: ✨ Improve the Search API admin UI for adding/editing fields Active
scott_euser → created an issue.
Okay I reviewed the options here. I think the Add/Edit fields screen needs more significant thought and rework to bring it more in line with drupal core forms to allow it to more easily feel at home in Claro/Gin so I am suggesting I create a separate issue for that (and work on it) and keep this limited to the original issue summary.
I have crossed out items that are no longer a problem (ie, over the last 7 years must have been fixed elsewhere) and I completed the ones that do exist.
Great thanks for confirming and thanks for the detailed walk through earlier.
scott_euser → created an issue.
Glad you got it working as well. Let's see what Marcus thinks before closing as I don't know much about Automators myself.