Add tests for best-match search interactions

Covers saved-search ranking and quick-search precedence, mixed
top-K/collection selections, rank-only behavior with an unavailable
index, reranking on indexer refresh events, scoped 'Match any' saves
keeping the marker at the root, numeric-operator round trips, and
concurrent query embeds sharing one worker call.

test/tests/advancedSearchTest.js

@@ -575,7 +575,67 @@ describe("Advanced Search", function () {
 		await collection.eraseTx();
 		await selectLibrary(win);
 	});
-	
+
+	it("should step the best-match cutoff from 'all' to 1 and back", async function () {
+		await zp.toggleAdvancedSearchState('open');
+		let pane = deck.pane;
+		let input = pane.querySelector('.best-match-topk-input');
+		assert.equal(input.value, '');
+
+		// The browser floors a step at min=0 in both directions; the direction
+		// is inferred from the previous value
+		input.value = '0';
+		input.dispatchEvent(new win.Event('input', { bubbles: true }));
+		assert.equal(input.value, '1');
+
+		input.value = '0';
+		input.dispatchEvent(new win.Event('input', { bubbles: true }));
+		assert.equal(input.value, '');
+
+		await zp.setAdvancedSearchState('closed');
+	});
+
+	it("should keep the bestMatch marker at the root when scoping a 'Match any' search", async function () {
+		var collection = await createDataObject('collection');
+		await selectCollection(win, collection.id);
+		await zp.toggleAdvancedSearchState('open');
+		var pane = deck.pane;
+
+		var s = new Zotero.Search();
+		s.libraryID = Zotero.Libraries.userLibraryID;
+		s.addCondition('resultLevel', 'item');
+		s.addCondition('joinMode', 'any');
+		s.addCondition('title', 'contains', 'flagfoo');
+		s.addCondition('creator', 'contains', 'flagbar');
+		s.addCondition('bestMatch', '5', 'some query');
+		pane.search = s;
+
+		var promptService = Services.prompt;
+		Services.prompt = {
+			prompt: (parent, title, message, nameObj) => {
+				nameObj.value = 'Scoped Semantic';
+				return true;
+			}
+		};
+		try {
+			await pane.save();
+		}
+		finally {
+			Services.prompt = promptService;
+		}
+
+		var saved = (await Zotero.Searches.getAll(Zotero.Libraries.userLibraryID))
+			.find(x => x.name == 'Scoped Semantic');
+		assert.ok(saved);
+		// The 'any' conditions were wrapped in a group, but the bestMatch marker
+		// stayed at the root, where getBestMatchQuery() finds it
+		assert.deepEqual(saved.getBestMatchQuery(), { query: 'some query', topK: 5 });
+
+		await saved.eraseTx();
+		await collection.eraseTx();
+		await selectLibrary(win);
+	});
+
 	it("should group the scope and the existing conditions when saving an 'any' search with a collection and a saved search selected", async function () {
 		var scopeSearch = new Zotero.Search();
 		scopeSearch.libraryID = Zotero.Libraries.userLibraryID;

test/tests/collectionViewItemTreeTest.js

@@ -212,10 +212,13 @@ describe("CollectionViewItemTree", function () {
 			});
 
 			afterEach(async function () {
-				stubs.forEach(stub => stub.restore());
-				stubs = [];
 				Zotero.Prefs.set('search.quicksearch-mode', 'fields');
 				await zp.itemsView.setFilter('search', '');
+				// Deselect any semantic saved search created by the test before
+				// its scoring stubs are restored
+				await selectLibrary(win);
+				stubs.forEach(stub => stub.restore());
+				stubs = [];
 			});
 
 			it("should show scored items ordered by a forced Relevance sort and restore the sort when cleared", async function () {
@@ -243,6 +246,8 @@ describe("CollectionViewItemTree", function () {
 				// Only the scored items, most similar first, despite title order
 				assert.deepEqual(itemsView._rows.map(row => row.id), [itemB.id, itemA.id]);
 				assert.equal(itemsView.getSortField(), 'relevance');
+				// Descending, so the fullest bars read as first
+				assert.equal(itemsView.getSortDirection(), -1);
 				// The Relevance cells show the ranks
 				assert.equal(itemsView.getCellText(0, 'relevance'), 1);
 				assert.equal(itemsView.getCellText(1, 'relevance'), 2);
@@ -250,17 +255,81 @@ describe("CollectionViewItemTree", function () {
 				assert.equal(itemsView.rowProvider.getBestMatchBarFractions().get(itemB.id), 0.9);
 				assert.equal(itemsView.rowProvider.getBestMatchBarFractions().get(itemA.id), 0.5);
 				assert.isFalse(itemsView._getColumns().find(c => c.dataKey == 'relevance').hidden);
+				// The rendered header shows the column
+				assert.ok(win.document.querySelector('.virtualized-table-header .cell.relevance'));
+				// The rows' bars are styled (a selector regression would leave
+				// collapsed inline spans) and filled. Row painting is async, so poll
+				// (the test times out on failure).
+				let bar;
+				for (let i = 0; i < 50 && !bar; i++) {
+					bar = itemsView.tree._jsWindow.getElementByIndex(0)
+						?.querySelector('.cell.relevance .relevance-bar');
+					if (!bar) {
+						await Zotero.Promise.delay(10);
+					}
+				}
+				if (!bar) {
+					let row0 = itemsView.tree._jsWindow.getElementByIndex(0);
+					dump('\nDIAG-BAR cells=' + [...row0.querySelectorAll('.cell')].map(c => c.className.split(' ')[1]).join(',')
+						+ ' visibleCols=' + itemsView.tree._columns.getAsArray().filter(c => !c.hidden).map(c => c.dataKey).join(',')
+						+ '\n');
+				}
+				assert.equal(win.getComputedStyle(bar).height, '6px');
+				assert.notEqual(bar.firstChild.style.width, '0%');
 
 				// Clearing the search restores the previous sort and columns
 				await itemsView.setFilter('search', '');
 				assert.equal(itemsView.getSortField(), defaultSortField);
 				assert.isTrue(itemsView._getColumns().find(c => c.dataKey == 'relevance').hidden);
+				assert.notOk(win.document.querySelector('.virtualized-table-header .cell.relevance'));
 				assert.deepEqual(
 					itemsView._rows.map(row => row.id),
 					[itemA.id, itemB.id, itemC.id]
 				);
 			});
 
+			it("should override a persisted column sort while a best-match search is active", async function () {
+				let col = await createDataObject('collection');
+				let itemA = await createDataObject('item', { title: "persistsort A", collections: [col.id] });
+				let itemB = await createDataObject('item', { title: "persistsort B", collections: [col.id] });
+				stubs.push(sinon.stub(Zotero.Embeddings, 'scoreItemIDs').callsFake(
+					async (query, itemIDs) => new Map(itemIDs.map(id => [id, id == itemB.id ? 0.9 : 0.5]))
+				));
+
+				await select(win, col);
+				itemsView = zp.itemsView;
+				// Sort by Date Modified via the header, as a real profile would have
+				let cols = itemsView._getColumns();
+				let dmIndex = cols.findIndex(c => c.dataKey == 'dateModified');
+				if (cols[dmIndex].hidden) {
+					itemsView.tree._columns.toggleHidden(dmIndex);
+				}
+				itemsView.tree._columns.toggleSort(dmIndex);
+				await itemsView.waitForLoad();
+				assert.equal(itemsView.getSortField(), 'dateModified');
+
+				try {
+					await itemsView.setFilter('search', 'some query');
+
+					assert.equal(itemsView.getSortField(), 'relevance');
+					assert.deepEqual(itemsView._rows.map(row => row.id), [itemB.id, itemA.id]);
+					assert.ok(win.document.querySelector('.virtualized-table-header .cell.relevance'));
+					// The replaced sort's column doesn't keep its indicator
+					assert.notOk(win.document.querySelector('.virtualized-table-header .cell.dateModified .sort-indicator'));
+
+					// Clearing restores the persisted sort
+					await itemsView.setFilter('search', '');
+					assert.equal(itemsView.getSortField(), 'dateModified');
+					assert.notOk(win.document.querySelector('.virtualized-table-header .cell.relevance'));
+				}
+				finally {
+					// Restore the profile's column state
+					delete itemsView._columnPrefs.dateModified;
+					itemsView._columnsId = null;
+					itemsView._sortedColumn = null;
+				}
+			});
+
 			it("should score once across a multi-collection selection", async function () {
 				let col1 = await createDataObject('collection');
 				let col2 = await createDataObject('collection');
@@ -288,6 +357,119 @@ describe("CollectionViewItemTree", function () {
 					[shared.id, other.id]
 				);
 			});
+
+			it("should rank a saved search with a bestMatch condition and let a quick search override it", async function () {
+				let itemA = await createDataObject('item', { title: "savedsimtest A" });
+				let itemB = await createDataObject('item', { title: "savedsimtest B" });
+				// Install the stub first: creating the saved search auto-selects
+				// it, which already runs a best-match refresh
+				let stub = sinon.stub(Zotero.Embeddings, 'scoreItemIDs').callsFake(
+					async (query, itemIDs) => new Map(itemIDs.map((id) => {
+						let best = query == 'saved query' ? itemA.id : itemB.id;
+						return [id, id == best ? 0.9 : 0.5];
+					}))
+				);
+				stubs.push(stub);
+				let search = new Zotero.Search();
+				search.name = "Saved best-match test";
+				search.libraryID = itemA.libraryID;
+				search.addCondition('resultLevel', 'item');
+				search.addCondition('title', 'contains', 'savedsimtest');
+				search.addCondition('bestMatch', 'contains', 'saved query');
+				await search.saveTx();
+
+				// Selecting the saved search activates ranking from its own marker
+				await select(win, search);
+				itemsView = zp.itemsView;
+				assert.equal(itemsView.getSortField(), 'relevance');
+				assert.deepEqual(itemsView._rows.map(row => row.id), [itemA.id, itemB.id]);
+
+				// An active best-match quick search overrides the saved marker
+				await itemsView.setFilter('search', 'typed query');
+				assert.deepEqual(itemsView._rows.map(row => row.id), [itemB.id, itemA.id]);
+			});
+
+			it("shouldn't let a top-K saved search's cutoff trim other selected rows", async function () {
+				let col = await createDataObject('collection');
+				// No embeddings for colItem1, so it can only survive via keepUnscored
+				let colItem1 = await createDataObject('item', { title: "mixedsel C1", collections: [col.id] });
+				let colItem2 = await createDataObject('item', { title: "mixedsel C2", collections: [col.id] });
+				let kItem1 = await createDataObject('item', { title: "mixedselk K1" });
+				let kItem2 = await createDataObject('item', { title: "mixedselk K2" });
+				// Install the stub first: creating the saved search auto-selects
+				// it, which already runs its top-K search
+				let scores = new Map([[kItem1.id, 0.9], [kItem2.id, 0.5], [colItem2.id, 0.7]]);
+				stubs.push(sinon.stub(Zotero.Embeddings, 'scoreItemIDs').callsFake(
+					async (query, itemIDs) => new Map(
+						itemIDs.filter(id => scores.has(id)).map(id => [id, scores.get(id)])
+					)
+				));
+				let search = new Zotero.Search();
+				search.name = "Top-K best-match test";
+				search.libraryID = col.libraryID;
+				search.addCondition('resultLevel', 'item');
+				search.addCondition('title', 'contains', 'mixedselk');
+				search.addCondition('bestMatch', '1', 'some query');
+				await search.saveTx();
+
+				await cv.selectByID("S" + search.id);
+				await waitForItemsLoad(win);
+				cv.selection.toggleSelect(cv.getRowIndexByID("C" + col.id));
+				await zp.onCollectionSelected();
+				await zp.itemsView.waitForLoad();
+				itemsView = zp.itemsView;
+
+				// The saved search returns its own top 1; the collection keeps both of
+				// its items, including the unscoreable one
+				assert.sameMembers(
+					itemsView._rows.filter(row => row.type == 'item').map(row => row.id),
+					[kItem1.id, colItem1.id, colItem2.id]
+				);
+			});
+
+			it("should keep a rank-only advanced search's results when the index isn't ready", async function () {
+				let col = await createDataObject('collection');
+				let itemA = await createDataObject('item', { title: "notready A", collections: [col.id] });
+				let itemB = await createDataObject('item', { title: "notready B", collections: [col.id] });
+				stubs.push(sinon.stub(Zotero.Embeddings, 'scoreItemIDs').callsFake(async () => {
+					throw new Zotero.Embeddings.IndexNotReadyError('test');
+				}));
+				let s = new Zotero.Search();
+				s.libraryID = col.libraryID;
+				s.addCondition('resultLevel', 'item');
+				s.addCondition('title', 'contains', 'notready');
+				s.addCondition('bestMatch', 'contains', 'some query');
+
+				await select(win, col);
+				itemsView = zp.itemsView;
+				await itemsView.setFilter('advanced-search', s);
+
+				// Membership is untouched; the rows just aren't ranked
+				assert.sameMembers(itemsView._rows.map(row => row.id), [itemA.id, itemB.id]);
+				assert.equal(itemsView.getCellText(0, 'relevance'), '');
+				await itemsView.setFilter('advanced-search', null);
+			});
+
+			it("should rerank when the indexer announces changed embeddings", async function () {
+				let col = await createDataObject('collection');
+				let itemA = await createDataObject('item', { title: "rerank A", collections: [col.id] });
+				let itemB = await createDataObject('item', { title: "rerank 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 indexer's coalesced notification after new/removed vectors
+				best = itemB.id;
+				await Zotero.Notifier.trigger('refresh', 'item', [itemA.id, itemB.id]);
+				await itemsView._refreshPromise;
+				assert.deepEqual(itemsView._rows.map(row => row.id), [itemB.id, itemA.id]);
+			});
 		});
 
 		it("should expand parent item and attachment for an annotation match", async function () {

test/tests/embeddingsTest.js

@@ -60,7 +60,95 @@ describe("Zotero.Embeddings", function () {
 		});
 	});
 
+	describe("#embedQuery()", function () {
+		it("should retry after a failed embed rather than caching the rejection", async function () {
+			let embedStub = sinon.stub(Zotero.Embeddings, 'embed');
+			embedStub.onFirstCall().rejects(new Error('embed failed'));
+			embedStub.onSecondCall().resolves(new Float32Array([1]));
+			let stubs = [
+				sinon.stub(Zotero.Embeddings.Indexing, 'startIndexing').resolves(),
+				sinon.stub(Zotero.Embeddings, 'pruneModels').resolves(),
+				embedStub
+			];
+			Zotero.Prefs.set('embeddings.model', 'bge-small-en-v1.5');
+			try {
+				await Zotero.Embeddings.Indexing.waitForPendingModelSwitch();
+				assert.ok(await getPromiseError(Zotero.Embeddings.embedQuery('retry query')));
+				// The eviction runs from a rejection handler
+				await Zotero.Promise.delay(0);
+				await Zotero.Embeddings.embedQuery('retry query');
+				assert.equal(embedStub.callCount, 2);
+			}
+			finally {
+				Zotero.Prefs.set('embeddings.model', '');
+				await Zotero.Embeddings.Indexing.waitForPendingModelSwitch();
+				Zotero.Prefs.clear('embeddings.indexingPaused');
+				stubs.forEach(stub => stub.restore());
+			}
+		});
+
+		it("should share one in-flight embed across concurrent calls", async function () {
+			let deferred = Zotero.Promise.defer();
+			let stubs = [
+				sinon.stub(Zotero.Embeddings.Indexing, 'startIndexing').resolves(),
+				sinon.stub(Zotero.Embeddings, 'pruneModels').resolves(),
+				sinon.stub(Zotero.Embeddings, 'embed').callsFake(() => deferred.promise)
+			];
+			// Select a model so the query prefix and model version resolve; the
+			// switch's indexing side effects are stubbed out above
+			Zotero.Prefs.set('embeddings.model', 'bge-small-en-v1.5');
+			try {
+				await Zotero.Embeddings.Indexing.waitForPendingModelSwitch();
+				let promise1 = Zotero.Embeddings.embedQuery('concurrent query');
+				let promise2 = Zotero.Embeddings.embedQuery('concurrent query');
+				deferred.resolve(new Float32Array([1]));
+				assert.equal(await promise1, await promise2);
+				assert.equal(Zotero.Embeddings.embed.callCount, 1);
+			}
+			finally {
+				Zotero.Prefs.set('embeddings.model', '');
+				await Zotero.Embeddings.Indexing.waitForPendingModelSwitch();
+				Zotero.Prefs.clear('embeddings.indexingPaused');
+				stubs.forEach(stub => stub.restore());
+			}
+		});
+	});
+
 	describe("Indexing", function () {
+		it("should announce cleared embeddings when the model changes", async function () {
+			let stubs = [
+				sinon.stub(Zotero.Embeddings.Indexing, 'startIndexing').resolves(),
+				sinon.stub(Zotero.Embeddings, 'pruneModels').resolves()
+			];
+			let item = await createDataObject('item');
+			try {
+				await Zotero.Embeddings.initDB();
+				await Zotero.DB.queryAsync(
+					"REPLACE INTO embeddings.itemEmbeddings VALUES (?, ?, ?)",
+					[item.id, new Uint8Array([0, 0, 0, 0]), 'hash']
+				);
+				// The model switch clears the old vectors and announces the
+				// removals (after the coalescing delay), so active semantic
+				// views refresh
+				let promise = waitForNotifierEvent('refresh', 'item');
+				Zotero.Prefs.set('embeddings.model', 'bge-small-en-v1.5');
+				let event = await promise;
+				assert.include(event.ids, item.id);
+				assert.equal(
+					await Zotero.DB.valueQueryAsync(
+						"SELECT COUNT(*) FROM embeddings.itemEmbeddings"
+					),
+					0
+				);
+			}
+			finally {
+				Zotero.Prefs.set('embeddings.model', '');
+				await Zotero.Embeddings.Indexing.waitForPendingModelSwitch();
+				Zotero.Prefs.clear('embeddings.indexingPaused');
+				stubs.forEach(stub => stub.restore());
+			}
+		});
+
 		it("should remove a deleted item's embedding", async function () {
 			await Zotero.Embeddings.initDB();
 			let stub = sinon.stub(Zotero.Embeddings, 'isEnabled').returns(true);

test/tests/searchTest.js

@@ -251,6 +251,28 @@ describe("Zotero.Search", function () {
 				stub.restore();
 			}
 		});
+
+		it("should round-trip a numeric operator through JSON and the database", async function () {
+			let s = new Zotero.Search();
+			s.name = "bestMatch operator round trip";
+			s.libraryID = Zotero.Libraries.userLibraryID;
+			s.addCondition('resultLevel', 'item');
+			s.addCondition('title', 'contains', 'roundtriptest');
+			s.addCondition('bestMatch', '25', 'some query');
+
+			// The strict JSON path used for synced data
+			let s2 = new Zotero.Search();
+			s2.libraryID = s.libraryID;
+			s2.fromJSON(s.toJSON(), { strict: true });
+			assert.deepEqual(s2.getBestMatchQuery(), { query: 'some query', topK: 25 });
+
+			// Database persistence
+			let id = await s.saveTx();
+			await Zotero.Searches.getAsync(id);
+			let loaded = Zotero.Searches.get(id);
+			assert.deepEqual(loaded.getBestMatchQuery(), { query: 'some query', topK: 25 });
+			await loaded.eraseTx();
+		});
 	});
 
 	describe("#search()", function () {