diff --git a/chrome/content/zotero/xpcom/collectionTreeRow.js b/chrome/content/zotero/xpcom/collectionTreeRow.js index 29d39090a3..e67ffe9b67 100644 --- a/chrome/content/zotero/xpcom/collectionTreeRow.js +++ b/chrome/content/zotero/xpcom/collectionTreeRow.js @@ -718,7 +718,7 @@ Zotero.CollectionTreeRow.prototype.getBestMatchSource = function () { } // An active best-match quick search overrides a selected saved search's // own marker, so the user's typed query wins - if (this.searchText + if (this.searchText && Zotero.Embeddings.normalizeQuery(this.searchText) && (this.searchMode || Zotero.Prefs.get('search.quicksearch-mode')) === 'bestMatch') { return false; } @@ -742,7 +742,7 @@ Zotero.CollectionTreeRow.prototype.isBestMatchSearch = function () { if (this.getBestMatchSource()) { return true; } - return !!this.searchText + return !!(this.searchText && Zotero.Embeddings.normalizeQuery(this.searchText)) && (this.searchMode || Zotero.Prefs.get('search.quicksearch-mode')) === 'bestMatch'; }; diff --git a/chrome/content/zotero/xpcom/data/search.js b/chrome/content/zotero/xpcom/data/search.js index 1334cecda3..d6df1ae1ad 100644 --- a/chrome/content/zotero/xpcom/data/search.js +++ b/chrome/content/zotero/xpcom/data/search.js @@ -881,7 +881,8 @@ Zotero.Search.prototype.getBestMatchQuery = function () { else if (condition.condition == 'groupEnd') { depth--; } - else if (depth == 0 && condition.condition == 'bestMatch' && condition.value) { + else if (depth == 0 && condition.condition == 'bestMatch' && condition.value + && Zotero.Embeddings.normalizeQuery(condition.value)) { return { query: condition.value, topK: parseInt(condition.operator) || false diff --git a/chrome/content/zotero/xpcom/embeddings.js b/chrome/content/zotero/xpcom/embeddings.js index 1369dff4e2..b83accf108 100644 --- a/chrome/content/zotero/xpcom/embeddings.js +++ b/chrome/content/zotero/xpcom/embeddings.js @@ -685,6 +685,19 @@ Zotero.Embeddings = new function () { // search triggers (per-row membership cutoffs plus the merged ranking) let _queryCache = null; + /** + * Normalize a best-match query: trim whitespace and strip a single pair + * of wrapping quotes, which carry no phrase semantics here -- the whole + * query embeds as one string. A query that normalizes to an empty string + * is no query at all, and callers treat it as no active search. + * + * @param {String} text + * @return {String} + */ + this.normalizeQuery = function (text) { + return text.trim().replace(/^"(.*)"$/s, '$1').trim(); + }; + /** * Embed a search query string, applying the active model's query prefix. * @@ -692,6 +705,13 @@ Zotero.Embeddings = new function () { * @return {Promise} */ this.embedQuery = function (text) { + text = this.normalizeQuery(text); + // Callers treat a query that normalizes to nothing as no search at + // all, so it should never get this far -- embedding just the model's + // query prefix would rank against noise + if (!text) { + throw new Error("Empty best-match query"); + } let modelVersion = this.getModelVersion(); if (_queryCache && _queryCache.modelVersion === modelVersion && _queryCache.text === text) { diff --git a/test/tests/embeddingsTest.js b/test/tests/embeddingsTest.js index f5f8dd1787..e81572a9a3 100644 --- a/test/tests/embeddingsTest.js +++ b/test/tests/embeddingsTest.js @@ -87,6 +87,31 @@ describe("Zotero.Embeddings", function () { } }); + it("should strip a single pair of wrapping quotes", async function () { + let embedStub = sinon.stub(Zotero.Embeddings, 'embed').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(); + // Whitespace around the quotes doesn't defeat the stripping + await Zotero.Embeddings.embedQuery(' "wrapped query" '); + assert.include(embedStub.firstCall.args[0], 'wrapped query'); + assert.notInclude(embedStub.firstCall.args[0], '"'); + // A query that normalizes to nothing is a caller bug + assert.throws(() => Zotero.Embeddings.embedQuery('""')); + } + 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 = [ diff --git a/test/tests/searchTest.js b/test/tests/searchTest.js index 703d045c0d..1b72252318 100644 --- a/test/tests/searchTest.js +++ b/test/tests/searchTest.js @@ -234,6 +234,14 @@ describe("Zotero.Search", function () { assert.deepEqual(s.getBestMatchQuery(), { query: 'some query', topK: false }); assert.sameMembers(await s.search(), [itemA.id, itemB.id]); + // A query that normalizes to nothing (e.g., just quotes) is no + // query at all + let sEmpty = new Zotero.Search(); + sEmpty.libraryID = itemA.libraryID; + sEmpty.addCondition('resultLevel', 'item'); + sEmpty.addCondition('bestMatch', 'contains', '""'); + assert.isFalse(sEmpty.getBestMatchQuery()); + // With a cutoff, membership is the K most similar let stub = sinon.stub(Zotero.Embeddings, 'scoreItemIDs').callsFake( async (query, itemIDs) => new Map(itemIDs.map(id => [id, id == itemB.id ? 0.9 : 0.5]))