diff --git a/chrome/content/zotero/collectionViewItemTree.jsx b/chrome/content/zotero/collectionViewItemTree.jsx index a55779a622..2d8612522b 100644 --- a/chrome/content/zotero/collectionViewItemTree.jsx +++ b/chrome/content/zotero/collectionViewItemTree.jsx @@ -566,7 +566,12 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider { this.itemTree._refreshPromise = deferred.promise; try { - this.collectionTreeRows.forEach(row => row.clearCache()); + // A best-match rerank after an embeddings change reuses the cached + // search results -- only the ranking depends on the embeddings, so + // there's no need to re-run the underlying search + if (!options.reuseSearchResults) { + this.collectionTreeRows.forEach(row => row.clearCache()); + } this._bestMatchRanks = null; this._bestMatchBarFractions = null; this._bestMatchIndexState = null; @@ -880,6 +885,7 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider { var madeChanges = false; var refresh = false; + var reuseSearchResults = false; var sort = false; // Selection strategy @@ -933,7 +939,15 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider { if (action == 'refresh' && type == 'item' && extraData && extraData.embeddingsUpdate && collectionTreeRows.some(rowIsBestMatchSearch)) { // The background indexer committed new or changed embeddings, so - // rerun the active best-match search to update the scores and ranks + // rerun the active best-match search to update the scores and ranks. + // Only the ranking depends on the embeddings, so unless a selected + // row trims membership by score (a top-K cutoff), the underlying + // search results are unchanged and can be reused while just the + // ranking is recomputed. + reuseSearchResults = collectionTreeRows.every((row) => { + return typeof row.hasBestMatchCutoff == 'function' + && !row.hasBestMatchCutoff(); + }); this.itemTree.invalidateRowCache(ids); refresh = true; madeChanges = true; @@ -1213,7 +1227,7 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider { } if (refresh) { - await this._refresh(); + await this._refresh({ reuseSearchResults }); } if (sort) { await this.itemTree._ensureSortContextReady(); diff --git a/chrome/content/zotero/xpcom/collectionTreeRow.js b/chrome/content/zotero/xpcom/collectionTreeRow.js index e67ffe9b67..23bdebe08a 100644 --- a/chrome/content/zotero/xpcom/collectionTreeRow.js +++ b/chrome/content/zotero/xpcom/collectionTreeRow.js @@ -759,6 +759,29 @@ Zotero.CollectionTreeRow.prototype.getBestMatchQuery = function () { return source ? source.getBestMatchQuery().query : this.searchText; }; +/** + * Whether an active best-match search has a top-K cutoff, which trims + * membership by score and so makes the underlying + * Zotero.Search.prototype.search() itself depend on the embeddings. Without + * one, the search results are stable across embedding changes, so they can be + * reused while only the ranking is recomputed. + * + * @return {Boolean} + */ +Zotero.CollectionTreeRow.prototype.hasBestMatchCutoff = function () { + let source = this.getBestMatchSource(); + if (source) { + return !!source.getBestMatchQuery().topK; + } + // A quick-search best-match doesn't trim membership itself, but a selected + // saved search may still carry its own top-K cutoff + if (this.isSearch() && this.ref instanceof Zotero.Search) { + let query = this.ref.getBestMatchQuery(); + return !!(query && query.topK); + } + return false; +}; + Zotero.CollectionTreeRow.prototype.isSortable = function () { return !this.isFeedsOrFeed() && !this.isRecentlyRead(); } diff --git a/test/tests/collectionViewItemTreeTest.js b/test/tests/collectionViewItemTreeTest.js index 2358e993c7..225c6798ec 100644 --- a/test/tests/collectionViewItemTreeTest.js +++ b/test/tests/collectionViewItemTreeTest.js @@ -555,6 +555,32 @@ describe("CollectionViewItemTree", function () { await itemsView._refreshPromise; assert.deepEqual(itemsView._rows.map(row => row.id), [itemA.id, itemB.id]); }); + + it("should rerank without re-running the search when embeddings change", async function () { + let col = await createDataObject('collection'); + let itemA = await createDataObject('item', { title: "reuse A", collections: [col.id] }); + let itemB = await createDataObject('item', { title: "reuse B", collections: [col.id] }); + let best = itemA.id; + stubs.push(sinon.stub(Zotero.Embeddings, 'scoreItemIDs').callsFake( + async (query, itemIDs) => new Map(itemIDs.map(id => [id, id == best ? 0.9 : 0.5])) + )); + + await select(win, col); + itemsView = zp.itemsView; + await itemsView.setFilter('search', 'some query'); + assert.deepEqual(itemsView._rows.map(row => row.id), [itemA.id, itemB.id]); + + // The underlying search is dropped and re-run by clearing the row cache + let clearCacheSpy = sinon.spy(Zotero.CollectionTreeRow.prototype, 'clearCache'); + stubs.push(clearCacheSpy); + + best = itemB.id; + await Zotero.Notifier.trigger('refresh', 'item', [itemA.id, itemB.id], { embeddingsUpdate: true }); + await itemsView._refreshPromise; + + assert.deepEqual(itemsView._rows.map(row => row.id), [itemB.id, itemA.id]); + assert.isFalse(clearCacheSpy.called); + }); }); it("should expand parent item and attachment for an annotation match", async function () {