diff --git a/chrome/content/zotero/collectionViewItemTree.jsx b/chrome/content/zotero/collectionViewItemTree.jsx
index bd9d148bf3..8ef83aa154 100644
--- a/chrome/content/zotero/collectionViewItemTree.jsx
+++ b/chrome/content/zotero/collectionViewItemTree.jsx
@@ -229,20 +229,10 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider {
let queryRow = this.collectionTreeRows.find(rowIsBestMatchSearch);
let query = queryRow.getBestMatchQuery();
let source = queryRow.getBestMatchSource();
- // Each selected row's search applies a top-K cutoff to its own scope,
- // so K is reapplied to the merged candidates (see Session#score()) --
- // a multi-row selection returns K members total rather than K per row.
- // Only when the source is the transient Advanced Search, which applies
- // uniformly to every selected row: a saved search's cutoff is part of
- // that row's own membership and must not trim other selected rows'
- // results.
- let topK = queryRow.advancedSearch && source
- ? source.getBestMatchQuery().topK
- : false;
// A best-match quick search shows only the items it can rank. With any
// search source, membership is defined by the selected rows' own
// searches, so keep unscoreable items -- they sort after the ranked
- // ones. (A uniform top-K set contains no unscoreable items anyway.)
+ // ones
let keepUnscored = !!source;
let candidateIDs = items
.filter(item => item instanceof Zotero.Item)
@@ -266,7 +256,6 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider {
// Scoring derives the best-scored items' previews before it
// resolves; the rest arrive through onPreviewsFilled above
await session.score(candidateIDs, {
- topK,
// A newer filter (e.g. more typed search text) makes this
// query obsolete -- stop scoring and let its refresh take over
shouldCancel: () => generation !== this._bestMatchGeneration
diff --git a/chrome/content/zotero/elements/zoteroSearch.js b/chrome/content/zotero/elements/zoteroSearch.js
index bc164603c4..ca815a38bf 100644
--- a/chrome/content/zotero/elements/zoteroSearch.js
+++ b/chrome/content/zotero/elements/zoteroSearch.js
@@ -153,7 +153,6 @@
case 'bestMatch':
stack[stack.length - 1].bestMatch = condition.value;
- stack[stack.length - 1].bestMatchTopK = parseInt(condition.operator) || false;
continue;
case 'groupStart': {
@@ -311,14 +310,9 @@
flat.push({ condition: 'resultLevel', operator: group.resultLevel, value: null });
}
// The best-match query is a root-level modifier, offered only for
- // top-level item results. The operator carries the optional top-K
- // cutoff; 'contains' means rank-only.
+ // top-level item results
if (isRoot && group.bestMatch && group.resultLevel == 'item') {
- flat.push({
- condition: 'bestMatch',
- operator: group.bestMatchTopK ? String(group.bestMatchTopK) : 'contains',
- value: group.bestMatch
- });
+ flat.push({ condition: 'bestMatch', operator: 'contains', value: group.bestMatch });
}
for (let child of group.conditionsContainer.children) {
if (child.localName == 'zoterosearchcondition') {
@@ -496,8 +490,6 @@
} - itemID -> score, as scoreItemIDs() returns
@@ -344,13 +340,6 @@ Zotero.BestMatch = new function () {
if (this._disposed) {
return scores;
}
- if (options.topK) {
- scores = new Map(
- [...scores.entries()]
- .sort((a, b) => (b[1] - a[1]) || (a[0] - b[0]))
- .slice(0, options.topK)
- );
- }
let previews = new Map();
for (let itemID of new Set([...matches.lexical, ...matches.semantic])) {
if (!scores.has(itemID) || !_hasPreviews(itemID)) {
diff --git a/chrome/content/zotero/xpcom/collectionTreeRow.js b/chrome/content/zotero/xpcom/collectionTreeRow.js
index 448343de60..640a7d6c5a 100644
--- a/chrome/content/zotero/xpcom/collectionTreeRow.js
+++ b/chrome/content/zotero/xpcom/collectionTreeRow.js
@@ -756,30 +756,7 @@ Zotero.CollectionTreeRow.prototype.getBestMatchQuery = function () {
return false;
}
let source = this.getBestMatchSource();
- 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;
+ return source ? source.getBestMatchQuery() : this.searchText;
};
Zotero.CollectionTreeRow.prototype.isSortable = function () {
diff --git a/chrome/content/zotero/xpcom/data/search.js b/chrome/content/zotero/xpcom/data/search.js
index a3866b0ff3..9fa29c43a9 100644
--- a/chrome/content/zotero/xpcom/data/search.js
+++ b/chrome/content/zotero/xpcom/data/search.js
@@ -557,11 +557,8 @@ Zotero.Search.prototype.hasPostSearchFilter = function () {
this._requireData('conditions');
for (let i of Object.values(this._conditions)) {
// Applied in search() after the SQL runs, so uses of this search as a
- // scope have to route through search() to include them. A rank-only
- // bestMatch condition (no cutoff) doesn't affect membership, so it
- // doesn't count.
- if (i.condition == 'fulltextContent'
- || (i.condition == 'bestMatch' && this.getBestMatchQuery()?.topK)) {
+ // scope have to route through search() to include them
+ if (i.condition == 'fulltextContent') {
return true;
}
}
@@ -830,21 +827,6 @@ Zotero.Search.prototype.search = async function (asTempTable) {
//Zotero.debug('Final result set');
//Zotero.debug(ids);
- // A root-level 'bestMatch' condition with a top-K cutoff makes membership
- // relevance-based: only the K results most relevant to the query match,
- // so the saved search returns the same set when used as a source (scopes,
- // counts, the API). Without a cutoff, best match is only a ranking in the
- // items list and membership is untouched.
- let bestMatch = this.getBestMatchQuery();
- if (ids && ids.length && bestMatch && bestMatch.topK) {
- let { scores } = await Zotero.BestMatch.scoreItemIDs(bestMatch.query, ids);
- ids = [...scores.entries()]
- // Deterministic order: by score, then by itemID for equal scores
- .sort((a, b) => (b[1] - a[1]) || (a[0] - b[0]))
- .slice(0, bestMatch.topK)
- .map(([itemID]) => itemID);
- }
-
if (!ids || !ids.length) {
return [];
}
@@ -857,10 +839,10 @@ Zotero.Search.prototype.search = async function (asTempTable) {
/**
- * The root-level 'bestMatch' condition, or false if none
+ * The query of the root-level 'bestMatch' condition, which the items list
+ * ranks the results by, or false if none
*
- * @return {Object|false} - { query, topK }, with topK false when the
- * condition is rank-only (operator 'contains') rather than a cutoff
+ * @return {String|false}
*/
Zotero.Search.prototype.getBestMatchQuery = function () {
let depth = 0;
@@ -873,10 +855,7 @@ Zotero.Search.prototype.getBestMatchQuery = function () {
}
else if (depth == 0 && condition.condition == 'bestMatch' && condition.value
&& Zotero.BestMatch.isSearchableQuery(condition.value)) {
- return {
- query: condition.value,
- topK: parseInt(condition.operator) || false
- };
+ return condition.value;
}
}
return false;
@@ -1247,8 +1226,8 @@ Zotero.Search.prototype._buildQuery = async function () {
lastCondition = null;
conditions.push({ name: 'resultLevel', operator: condition.operator });
continue;
- // Applied as a filter at the end of search() and as a ranking by the
- // items list, not as part of the condition tree
+ // Applied as a ranking by the items list, not as part of the
+ // condition tree
case 'bestMatch':
lastCondition = null;
continue;
diff --git a/chrome/content/zotero/xpcom/data/searchConditions.js b/chrome/content/zotero/xpcom/data/searchConditions.js
index d145bf3c18..c38bf1c620 100644
--- a/chrome/content/zotero/xpcom/data/searchConditions.js
+++ b/chrome/content/zotero/xpcom/data/searchConditions.js
@@ -266,10 +266,8 @@ Zotero.SearchConditions = new function () {
}
},
- // Root-level modifier rather than a regular condition: restricts the
- // results to items with a stored embedding, and the items list ranks
- // them by semantic similarity to the value (see
- // CollectionViewItemTreeRowProvider._applyBestMatch())
+ // Root-level modifier rather than a regular condition: the items
+ // list ranks the results by how well they match the value
{
name: 'bestMatch',
operators: {
diff --git a/chrome/locale/en-US/zotero/zotero.ftl b/chrome/locale/en-US/zotero/zotero.ftl
index 878f2c4052..da0ca2910a 100644
--- a/chrome/locale/en-US/zotero/zotero.ftl
+++ b/chrome/locale/en-US/zotero/zotero.ftl
@@ -989,11 +989,6 @@ advanced-search-best-match-prefix =
advanced-search-best-match-input =
.aria-label = Sort results by best match for
.placeholder = Enter a topic or phrase
-advanced-search-best-match-topk-prefix =
- .value = Keep top:
-advanced-search-best-match-topk-input =
- .aria-label = Number of results to keep
- .placeholder = all
advanced-search-level-warning-mixed = These conditions cannot all match the same item, so this search will never return results. Try matching “{ $matchAny }” of them, or set the result type to “{ $topLevelItems }”.
advanced-search-level-warning-unreachable = This search has a condition that cannot apply to the chosen result type. Set the result type to “{ $topLevelItems }” or remove the incompatible condition.
advanced-search-group-warning-unreachable =
diff --git a/scss/elements/_zoteroSearch.scss b/scss/elements/_zoteroSearch.scss
index de76b3f70f..8bcf869f63 100644
--- a/scss/elements/_zoteroSearch.scss
+++ b/scss/elements/_zoteroSearch.scss
@@ -179,10 +179,6 @@ zoterosearch {
.best-match-input {
flex: 1;
}
-
- .best-match-topk-input {
- width: 60px;
- }
}
#search-binding-hint {
diff --git a/test/tests/advancedSearchTest.js b/test/tests/advancedSearchTest.js
index e863d1c827..d9277fd924 100644
--- a/test/tests/advancedSearchTest.js
+++ b/test/tests/advancedSearchTest.js
@@ -640,64 +640,51 @@ describe("Advanced Search", function () {
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;
- }
- };
+ var saved;
try {
- await pane.save();
+ 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', 'contains', '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;
+ }
+
+ 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.equal(saved.getBestMatchQuery(), 'some query');
}
finally {
- Services.prompt = promptService;
+ if (saved) {
+ await saved.eraseTx();
+ }
+ await collection.eraseTx();
+ await selectLibrary(win);
}
-
- 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 () {
diff --git a/test/tests/bestMatchTest.js b/test/tests/bestMatchTest.js
index fcfe078a4e..742a361f6e 100644
--- a/test/tests/bestMatchTest.js
+++ b/test/tests/bestMatchTest.js
@@ -840,20 +840,6 @@ describe("Zotero.BestMatch", function () {
assert.isNull(session.getPreviews(att3.id));
});
- it("should only build and derive previews for items kept by topK", async function () {
- stubScore(new Map([[att1.id, 0.9], [att2.id, 0.8]]), [att1.id, att2.id]);
- let derive = stubDerive(new Map([
- [att1.id, [{ source: 'title', text: 'one', ranges: [], strength: 1 }]],
- [att2.id, [{ source: 'title', text: 'two', ranges: [], strength: 1 }]]
- ]));
- let session = Zotero.BestMatch.createSession('owl');
- let scores = await session.score([att1.id, att2.id], { topK: 1 });
- assert.deepEqual([...scores.keys()], [att1.id]);
- assert.equal(session.getPreviews(att1.id).state, 'filled');
- assert.isNull(session.getPreviews(att2.id));
- assert.equal(derive.callCount, 1);
- });
-
it("should rank rows by the best match beneath them, with bars reporting own scores", async function () {
let parent = await createDataObject('item');
let child = await importFileAttachment('test.pdf', { parentID: parent.id });
diff --git a/test/tests/collectionViewItemTreeTest.js b/test/tests/collectionViewItemTreeTest.js
index 415cf08634..ca49bc6d07 100644
--- a/test/tests/collectionViewItemTreeTest.js
+++ b/test/tests/collectionViewItemTreeTest.js
@@ -1060,44 +1060,6 @@ describe("CollectionViewItemTree", function () {
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(scoreEnvelope(
- 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] });
diff --git a/test/tests/searchTest.js b/test/tests/searchTest.js
index 76ef9c1b08..f04339c724 100644
--- a/test/tests/searchTest.js
+++ b/test/tests/searchTest.js
@@ -221,17 +221,16 @@ describe("Zotero.Search", function () {
});
describe("bestMatch condition", function () {
- it("shouldn't affect membership without a cutoff and should return the top K with one", async function () {
+ it("shouldn't affect membership", async function () {
let itemA = await createDataObject('item', { title: "bestmatchcondtest A" });
let itemB = await createDataObject('item', { title: "bestmatchcondtest B" });
- // Rank-only (no cutoff): membership is unchanged
let s = new Zotero.Search();
s.libraryID = itemA.libraryID;
s.addCondition('resultLevel', 'item');
s.addCondition('title', 'contains', 'bestmatchcondtest');
s.addCondition('bestMatch', 'contains', 'some query');
- assert.deepEqual(s.getBestMatchQuery(), { query: 'some query', topK: false });
+ assert.equal(s.getBestMatchQuery(), 'some query');
assert.sameMembers(await s.search(), [itemA.id, itemB.id]);
// A query that normalizes to nothing (e.g., just quotes) is no
@@ -241,48 +240,6 @@ describe("Zotero.Search", function () {
sEmpty.addCondition('resultLevel', 'item');
sEmpty.addCondition('bestMatch', 'contains', '""');
assert.isFalse(sEmpty.getBestMatchQuery());
-
- // With a cutoff, membership is the K most relevant
- let stub = sinon.stub(Zotero.BestMatch, 'scoreItemIDs').callsFake(
- async (query, itemIDs) => ({
- scores: new Map(itemIDs.map(id => [id, id == itemB.id ? 0.9 : 0.5])),
- matches: { lexical: new Set(itemIDs), semantic: new Set() }
- })
- );
- try {
- let s2 = new Zotero.Search();
- s2.libraryID = itemA.libraryID;
- s2.addCondition('resultLevel', 'item');
- s2.addCondition('title', 'contains', 'bestmatchcondtest');
- s2.addCondition('bestMatch', '1', 'some query');
- assert.deepEqual(s2.getBestMatchQuery(), { query: 'some query', topK: 1 });
- assert.sameMembers(await s2.search(), [itemB.id]);
- }
- finally {
- 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();
});
});