-
Notifications
You must be signed in to change notification settings - Fork 52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a Term Cleanup Feature #815
base: develop
Are you sure you want to change the base?
Conversation
…ve enqueues that aren't being used yet. Fix test assertion
…atements for remaining methods that are needed because of that switch
@iamdharmesh I believe I have this feature fully moved over to ClassifAI but would appreciate if you could take a look since you're most familiar with how this functions. I moved the background processing to use Action Scheduler since we are already using that in ClassifAI but I believe everything else has been left the same. Note @jeffpaul mentioned there are some UI changes we're hoping to make so we can either leave this PR until those are ready or can merge this in (once it looks good) and open a new PR for those changes, but leaving this in draft for now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dkotter Thanks for working on this. The PR looks great. I’ve fixed a few issues related to the in-progress job status, canceling the running process, and similarity score for database-based comparison. Please take a quick look at those additional changes when you have time. Otherwise, everything looks good.
I would prefer to get a quick UX review to see if there are some quick iterations to improve here, but if we're otherwise near ready to ship a major release then I'm fine merging this in to ride along there |
@jeffpaul @dkotter I worked on UX items from https://github.com/10up/classifai-cleanup/issues/7, https://github.com/10up/classifai-cleanup/issues/20, https://github.com/10up/classifai-cleanup/issues/19 and https://github.com/10up/classifai-cleanup/issues/18 and listed status on each below. Some of these need discussion.
Thanks |
Things look cleaner to me from the last updates made by @iamdharmesh but I agree could use a more formal review, so I'm fine holding this until then |
…ng third parties to hook in and do things like logging
@dkotter thanks for the PR! Could you please rebase your PR on top of the latest changes in the base branch? |
Description of the Change
This PR adds in a new Feature, Term Cleanup, that uses the power of embeddings, to compare all terms against each other and then recommending similar terms that could be merged together if desired. This comparison process can be run entirely in WordPress but once you reach around 500 terms, performance really starts to slow down. Similar to the Smart 404 Feature, support for Elasticsearch (using ElasticPress) has been added to significantly speed things up.
This Feature requires two services in order to function:
Once the Feature is setup, you can start the comparison process which will send all terms off to the embeddings Provider to generate vector data. If configured, this data is then sent to Elasticsearch and vector queries are run there. If not configured, this data remains in WordPress only and comparisons are done there using that data.
Once the process is complete, you'll be given the opportunity to see all the results and decide which terms you want to merge together (or which terms you want to skip). This process can be re-run whenever needed (i.e. as new terms are added).
Closes #795
How to test the Change
Changelog Entry
Credits
Props @dkotter, @iamdharmesh, @berkod
Checklist: