scriptoriumadmin/zotero
1
0
Fork 0
mirror of https://github.com/zotero/zotero.git synced 2026-10-11 03:38:25 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

drop topK operator of best match

Browse source 
Replaced by keeping results within X% quality margin from top match.
It adapts to different search queries better giving "Most relevant"
results in the library VS top X results that show top results
regardless if there are more results that rank well or
if most of top matches are bad.
This commit is contained in:
Bogdan Abaev 2026-10-07 15:59:42 -07:00
parent fff76fcbfc
commit e1655c3ea5
12 changed files with 55 additions and 274 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

13
chrome/content/zotero/collectionViewItemTree.jsx
View file

@ -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

38
chrome/content/zotero/elements/zoteroSearch.js
View file

@ -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 @@
<hbox class="best-match-row" align="center" hidden="true">
<label class="best-match-prefix" data-l10n-id="advanced-search-best-match-prefix"/>
<html:input class="best-match-input" type="text" data-l10n-id="advanced-search-best-match-input"/>
<label class="best-match-topk-prefix" data-l10n-id="advanced-search-best-match-topk-prefix"/>
<html:input class="best-match-topk-input" type="number" min="0" data-l10n-id="advanced-search-best-match-topk-input"/>
</hbox>
<hbox class="level-warning" hidden="true">
<description/>
@ -514,17 +506,6 @@
this.levelWarning = this.querySelector('.level-warning');
this.bestMatchRow = this.querySelector('.best-match-row');
this.bestMatchInput = this.querySelector('.best-match-input');
this.bestMatchTopKInput = this.querySelector('.best-match-topk-input');
// The cutoff has no zero -- empty means "all" -- so route 0 by
// direction: stepping down from 1 goes back to empty, and stepping
// up from empty (which the browser floors at min) goes to 1
this._lastTopKValue = this.bestMatchTopKInput.value;
this.bestMatchTopKInput.addEventListener('input', () => {
if (this.bestMatchTopKInput.value === '0') {
this.bestMatchTopKInput.value = this._lastTopKValue === '' ? '1' : '';
}
this._lastTopKValue = this.bestMatchTopKInput.value;
});
// The result level is tracked here and reflected to whichever control is active: the root's
// result-level menu ("Find ..."), or a nested group's binding menu ("... in the
@ -625,20 +606,6 @@
this.updateBestMatchRow();
}
/**
* Optional top-K cutoff for the best-match query: with a value, only
* the K most similar results match; empty means rank-only
*/
get bestMatchTopK() {
let val = parseInt(this.bestMatchTopKInput.value);
return val > 0 ? val : false;
}
set bestMatchTopK(val) {
this.bestMatchTopKInput.value = val || '';
this._lastTopKValue = this.bestMatchTopKInput.value;
}
// The best-match field is a root-level modifier, offered for
// top-level item results
updateBestMatchRow() {
@ -866,7 +833,6 @@
this.joinMode = 'all';
this.resultLevel = 'any';
this.bestMatch = '';
this.bestMatchTopK = false;
while (this.conditionsContainer.firstChild) {
this.conditionsContainer.removeChild(this.conditionsContainer.firstChild);
}

11
chrome/content/zotero/xpcom/bestMatch.js
View file

@ -329,10 +329,6 @@ Zotero.BestMatch = new function () {
*
* @param {Number[]} itemIDs - Candidate item IDs to score
* @param {Object} [options] - Passed through to scoreItemIDs()
* @param {Number} [options.topK] - Keep only the K best-scored items,
* with a deterministic tiebreak, so equal scores keep a stable
* membership; previews are only built and derived for the kept
* items
* @param {Function} [options.shouldCancel] - Also checked between
* preview derivations
* @return {Promise<Map>} - 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)) {

25
chrome/content/zotero/xpcom/collectionTreeRow.js
View file

@ -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 () {

37
chrome/content/zotero/xpcom/data/search.js
View file

@ -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;

6
chrome/content/zotero/xpcom/data/searchConditions.js
View file

@ -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: {

5
chrome/locale/en-US/zotero/zotero.ftl
View file

@ -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 =

4
scss/elements/_zoteroSearch.scss
View file

@ -179,10 +179,6 @@ zoterosearch {
.best-match-input {
flex: 1;
}
.best-match-topk-input {
width: 60px;
}
}
#search-binding-hint {

91
test/tests/advancedSearchTest.js
View file

@ -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 () {

14
test/tests/bestMatchTest.js
View file

@ -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 });

38
test/tests/collectionViewItemTreeTest.js
View file

@ -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] });

47
test/tests/searchTest.js
View file

@ -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();
});
});