diff --git a/chrome/content/zotero/collectionViewItemTree.jsx b/chrome/content/zotero/collectionViewItemTree.jsx index 6b8ff4b44c..484b0ae859 100644 --- a/chrome/content/zotero/collectionViewItemTree.jsx +++ b/chrome/content/zotero/collectionViewItemTree.jsx @@ -48,8 +48,6 @@ const { LibraryHeaderItemTreeRow, SpacerItemTreeRow, SearchMatch } = require('zo const { OS } = ChromeUtils.importESModule("chrome://zotero/content/osfile.mjs"); const { ZOTERO_CONFIG } = ChromeUtils.importESModule('resource://zotero/config.mjs'); -const PRELOADED_MATCH_PREVIEWS = 10; - const COLORED_TAGS_RE = new RegExp("^(?:Numpad|Digit)([0-" + Zotero.Tags.MAX_COLORED_TAGS + "]{1})$"); // Minimal CollectionTreeRow-like object for callers that pass plain objects to @@ -154,24 +152,24 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider { } /** - * Best-match ranks for the Relevance column, computed over the merged - * result set in _refresh() while a best-match search is active + * Best-match ranks for the Relevance column while a best-match search is + * active, from the session's last scoring pass * * @returns {Map} - treeViewID -> 1-based rank (1 = most similar) */ getBestMatchRanks() { - return this._bestMatchRanks || new Map(); + return this._bestMatchSession?.ranks ?? new Map(); } /** * Score fractions for the Relevance column's bars, computed alongside the - * ranks: each row's own score (see Zotero.BestMatch.scoreItemIDs()), so - * the bars always agree with the ranking + * ranks (see Zotero.BestMatch.Session#barFractions), so the bars always + * agree with the ranking * * @returns {Map} - treeViewID -> 0-1 fraction for the bar */ getBestMatchBarFractions() { - return this._bestMatchBarFractions || new Map(); + return this._bestMatchSession?.barFractions ?? new Map(); } /** @@ -186,66 +184,27 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider { } /** - * Compute the current index coverage across the selected rows' libraries. - * Never throws -- the banner is informational and shouldn't break a - * refresh. + * Compute the current index coverage across the selected rows' libraries + * (see Zotero.BestMatch.getIndexState()) * * @return {Promise} */ async _getBestMatchIndexState() { - try { - let status = Zotero.Embeddings.Indexing.getStatus(); - if (!status.enabled) { - return null; - } - // Counts aren't populated until the indexer runs in this session - if (!status.libraries.length) { - status = await Zotero.Embeddings.Indexing.refreshStatus(); - } - let libraryIDs = new Set( - this.collectionTreeRows - .map(row => row.ref?.libraryID) - .filter(id => id !== undefined) - ); - let libraries = status.libraries - .filter(lib => !libraryIDs.size || libraryIDs.has(lib.libraryID)); - // Coverage is coverage: attachment fulltext is reported separately - // in the preferences, but an incomplete index is incomplete - // whichever part of it is still filling in - let indexed = libraries.reduce( - (sum, lib) => sum + lib.indexed + lib.indexedAttachments, 0); - let total = libraries.reduce( - (sum, lib) => sum + lib.eligible + lib.eligibleAttachments, 0); - if (indexed >= total) { - return null; - } - // Only an explicit pause reports as paused. Anything else -- - // between runs (startup, the pre-run debounce) or after an error - // (detailed in the preferences) -- reports as indexing, since the - // banner explains the incomplete coverage, not the indexer state - return { - type: status.paused ? 'paused' : 'indexing', - indexed, - total - }; - } - catch (e) { - Zotero.logError(e); - return null; - } + return Zotero.BestMatch.getIndexState( + this.collectionTreeRows + .map(row => row.ref?.libraryID) + .filter(id => id !== undefined) + ); } /** * The ranking stage of a best-match search: score the merged, * deduplicated results from all selected rows against the query in a - * single call, and keep the matching items ranked globally across the - * selection. Every item is scored on its own text, - * an item's rank reflects the best match anywhere beneath it, so a - * strongly matching annotation lifts its attachment and its paper to the - * top. The relevance bar reports only the row's own score, so a row - * ranked by a descendant shows its rank over an empty bar. Equal scores - * get equal ranks, so tied rows (including a child and its parent) order - * deterministically via the secondary sort fields. + * single session call, which also derives the match previews and + * computes the ranks and bar fractions the Relevance column reads (see + * Zotero.BestMatch.Session#score()). An item is kept when it or + * anything beneath it matched, so a strongly matching annotation keeps + * its attachment and its paper in the results. * * @param {Zotero.Item[]} items - Merged results from all selected rows * @return {Promise} - The matching items @@ -256,10 +215,13 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider { let queryRow = this.collectionTreeRows.find(rowIsBestMatchSearch); let query = queryRow.getBestMatchQuery(); let source = queryRow.getBestMatchSource(); - // A top-K cutoff is reapplied to the merged candidates below 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. + // 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; @@ -268,26 +230,29 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider { // searches, so keep unscoreable items -- they sort after the ranked // ones. (A uniform top-K set contains no unscoreable items anyway.) let keepUnscored = !!source; - let candidates = items.filter(item => item instanceof Zotero.Item); - let itemsByID = new Map(candidates.map(item => [item.id, item])); - let scores; + let candidateIDs = items + .filter(item => item instanceof Zotero.Item) + .map(item => item.id); let generation = this._bestMatchGeneration; // The session scores the query and owns the match previews the tree // shows as child rows. A new query gets a fresh session -- the old - // one's fills must never touch rows again -- while a re-score of the - // same query (an item edit, an index update) keeps it, so - // already-derived previews survive; the previews of the items that - // actually changed are invalidated in notify(). + // one must derive nothing more -- while a re-score of the same query + // (an item edit, an index update) keeps it, so already-derived + // previews survive; the previews of the items that actually changed + // are invalidated in notify(). let session = this._bestMatchSession; let newQuery = !session || session.queryText !== query; if (newQuery) { session?.dispose(); session = Zotero.BestMatch.createSession(query); - session.onUpdate = itemIDs => this._onMatchPreviewsUpdate(session, itemIDs); this._bestMatchSession = session; } try { - scores = await session.score([...itemsByID.keys()], { + // Scoring also derives the matched items' previews before it + // resolves, so the rows the refresh builds draw finished match + // rows + 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 @@ -302,78 +267,26 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider { if (this._bestMatchSession == session) { this._bestMatchSession = null; } - this._bestMatchRanks = new Map(); this._bestMatchIndexState = await this._getBestMatchIndexState(); // A rank-only search's membership doesn't depend on scoring, so // show its results unranked; anything else shows no results rather // than an unranked scope return keepUnscored ? items : []; } - // Each selected row's search applies a top-K cutoff to its own scope, - // so trim the merged candidates to K again here, with the same - // deterministic order as search(), so a multi-row selection returns K - // members total rather than K per row - if (topK) { - scores = new Map( - [...scores.entries()] - .sort((a, b) => (b[1] - a[1]) || (a[0] - b[0])) - .slice(0, topK) - ); + // A cancellation that lands after the last derivation resolves + // score() normally, so check once more before building the view state + if (generation !== this._bestMatchGeneration) { + throw new Zotero.BestMatch.ScoringCancelledError(); } - // Lift each scored item's score onto its ancestors (annotation -> - // attachment -> top-level item), so an item's effective score -- and - // so its rank -- is the best match anywhere beneath it - let effectiveScores = new Map(scores); - for (let [itemID, score] of scores) { - let item = itemsByID.get(itemID) || Zotero.Items.get(itemID); - let parentItemID = item && item.parentItemID; - while (parentItemID) { - let current = effectiveScores.get(parentItemID); - if (current === undefined || score > current) { - effectiveScores.set(parentItemID, score); - } - parentItemID = Zotero.Items.get(parentItemID)?.parentItemID; - } - } - let rankOfScore = new Map( - [...new Set(effectiveScores.values())].sort((a, b) => b - a) - .map((score, i) => [score, i + 1]) - ); - // Ranks cover every row with a match beneath it, including ancestors - // the selected rows' results didn't return themselves; the bar - // fraction is the row's own score alone, with an empty bar for a row - // that only inherited its rank - let ranks = new Map(); - let fractions = new Map(); - for (let [itemID, score] of effectiveScores) { - let item = itemsByID.get(itemID) || Zotero.Items.get(itemID); - if (!item) { - continue; - } - ranks.set(item.treeViewID, rankOfScore.get(score)); - fractions.set(item.treeViewID, scores.get(itemID) || 0); - } - // A new query's results are shown from the top (see _refresh()), so - // the previews the reader lands on are the best-ranked ones. Deriving - // them before the rows appear is what keeps those rows from visibly - // growing into their matches a moment after they're drawn; the rest - // fill in on demand as they're scrolled to. + // A new query's results are shown from the top (see _refresh()) if (newQuery) { this._scrollToTopOnUpdate = true; - await session.preload( - [...scores.entries()] - .sort((a, b) => b[1] - a[1]) - .map(([itemID]) => itemID) - .filter(itemID => session.getPreviews(itemID)) - .slice(0, PRELOADED_MATCH_PREVIEWS) - ); - if (generation !== this._bestMatchGeneration) { - throw new Zotero.BestMatch.ScoringCancelledError(); - } } + // The session's ranks cover every row with a match anywhere beneath + // it, so they say which items stay in the results let kept = []; for (let item of items) { - if (!(item instanceof Zotero.Item) || !effectiveScores.has(item.id)) { + if (!(item instanceof Zotero.Item) || !session.ranks.has(item.treeViewID)) { if (keepUnscored) { kept.push(item); } @@ -381,8 +294,6 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider { } kept.push(item); } - this._bestMatchRanks = ranks; - this._bestMatchBarFractions = fractions; this._bestMatchIndexState = await this._getBestMatchIndexState(); return kept; } @@ -397,126 +308,6 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider { return this._bestMatchSession ?? null; } - /** - * Called for every pending search-match row the tree draws: rendering - * is the demand signal for deriving previews. Reports are collected - * across the render pass and flushed as one request on a microtask -- a - * request per row would re-enter from the render that answers the first - * one. Each flush replaces the session's previous request, so scrolling - * past unfilled rows discards their work; rows still pending on screen - * are restated by the re-render that follows each fill. - * - * @param {Number} itemID - The item whose pending preview was drawn - */ - onSearchMatchRendered(itemID) { - if (!this._bestMatchSession) { - return; - } - if (!this._renderedMatchItemIDs) { - this._renderedMatchItemIDs = new Set(); - Promise.resolve().then(() => { - let painted = this._renderedMatchItemIDs; - this._renderedMatchItemIDs = null; - // What's on screen when the request goes out, plus whatever - // asked for it: the visible range is read a turn after the - // rows were drawn, and a row drawn as the view was still - // moving can fall outside it by then. It would never ask - // again -- it has already been painted -- so it says so here. - this._bestMatchSession?.request( - [...new Set([...this._pendingMatchItemIDs(), ...painted])]); - }); - } - this._renderedMatchItemIDs.add(itemID); - } - - /** - * The items whose placeholders are on screen now, topmost first. - * - * A row signals demand when it's painted, but a pass only paints rows - * newly in view -- so what was just painted is never the whole of what's - * waiting. Each request replaces the last, which is what discards work - * for rows scrolled past, and that only holds if the request says - * everything still wanted rather than only what changed. - * - * @return {Number[]} - */ - _pendingMatchItemIDs() { - let treebox = this.itemTree._treebox; - if (!treebox) { - return []; - } - let first = treebox.getFirstVisibleRow(); - let last = treebox.getLastVisibleRow(); - if (first === undefined || first === null) { - return []; - } - let itemIDs = []; - for (let i = first; i <= last; i++) { - let row = this.getRow(i); - if (row?.type == 'search-match-placeholder') { - itemIDs.push(row.ref.itemID); - } - } - return itemIDs; - } - - /** - * A session reported previews that settled: replace each affected - * container's placeholder row with the derived match rows -- or with - * nothing, when derivation found nothing to show. Runs after any - * in-flight refresh, and only while the session is still the view's; - * a superseded session's fills never touch rows. A selected placeholder - * hands its selection to the first derived row. - * - * @param {Zotero.BestMatch.Session} session - * @param {Number[]} itemIDs - */ - async _onMatchPreviewsUpdate(session, itemIDs) { - try { - // A refresh in flight materializes the settled previews itself - await this.itemTree._refreshPromise; - if (session !== this._bestMatchSession) { - return; - } - this.itemTree._cacheState(); - let handoffID = null; - let changed = false; - for (let itemID of itemIDs) { - let index = this._rowMap[itemID]; - // A collapsed container materializes its rows on reopen - if (index === undefined || !this.isContainerOpen(index)) { - continue; - } - let placeholderIndex = this._rowMap['SM' + itemID + '-pending']; - if (placeholderIndex !== undefined - && this.itemTree.selection.isSelected(placeholderIndex)) { - let preview = session.getPreviews(itemID); - // The first derived row, or the container itself when - // nothing derived - handoffID = preview?.state == 'filled' - ? 'SM' + itemID + '-' + preview.entries[0].key - : itemID; - } - this._refreshContainer(index, true); - changed = true; - } - if (!changed) { - return; - } - this.refreshRowMap(); - if (handoffID !== null && this._rowMap[handoffID] !== undefined) { - this.itemTree.selection.select(this._rowMap[handoffID]); - } - this.runListeners('update', true, { - restoreSelection: handoffID === null, - restoreScroll: true - }); - } - catch (e) { - Zotero.logError(e); - } - } - /** * When showing multiple libraries, group rows by library in collections-list * order -- independent of the active sort direction @@ -754,8 +545,6 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider { if (!options.reuseSearchResults) { this.collectionTreeRows.forEach(row => row.clearCache()); } - this._bestMatchRanks = null; - this._bestMatchBarFractions = null; this._bestMatchIndexState = null; // Get the full set of items we want to show, merged across all selected rows let newSearchItemSet = new Set(); @@ -1106,9 +895,9 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider { return; } - // A changed item's derived match previews are stale: back to - // placeholders, re-derived on their next render. The re-score the - // change triggers below keeps every other item's derived text. + // A changed item's derived match previews are stale: back to pending, + // re-derived by the refresh the change triggers below, which keeps + // every other item's derived text. if (type == 'item' && ['modify', 'refresh'].includes(action) && this._bestMatchSession) { this._bestMatchSession.invalidate(ids.map(id => parseInt(id))); } @@ -1182,6 +971,14 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider { refresh = true; madeChanges = true; } + else if (action == 'refresh' && type == 'item' && this._bestMatchSession + && ids.some(id => this._bestMatchSession.getPreviews(parseInt(id)))) { + // The invalidation above reset these items' previews, and only a + // scoring pass derives previews, so re-run the search + this.itemTree.invalidateRowCache(ids); + refresh = true; + madeChanges = true; + } else if (action == 'refresh') { // Clear row display cache and invalidate rows for refreshed items let rowsToInvalidate = []; diff --git a/chrome/content/zotero/elements/searchResultsBox.js b/chrome/content/zotero/elements/searchResultsBox.js index e3c1db7be4..d4703dce43 100644 --- a/chrome/content/zotero/elements/searchResultsBox.js +++ b/chrome/content/zotero/elements/searchResultsBox.js @@ -117,9 +117,10 @@ let preview = session.getPreviews(item.id); if (preview?.state == 'pending') { - // Selecting an item asks for its passages outright, rather - // than waiting for its rows to be scrolled to - await session.preload([item.id]); + // Previews are derived before the tree's rows appear, but a + // selection can still land mid-re-score, so settle this one + // outright + await session.fill([item.id]); // The selection, or the search, may have moved on while // deriving if (this.item !== item || this._session !== session) { diff --git a/chrome/content/zotero/itemTree.jsx b/chrome/content/zotero/itemTree.jsx index 4766055c80..4745f0535f 100644 --- a/chrome/content/zotero/itemTree.jsx +++ b/chrome/content/zotero/itemTree.jsx @@ -1322,7 +1322,7 @@ var ItemTree = class ItemTree extends LibraryTree { let indexes = []; for (let i = 0, count = this.getRowCount(); i < count; i++) { let type = this.getRow(i)?.type; - if (type == 'search-match' || type == 'search-match-placeholder') { + if (type == 'search-match') { height ??= this._getSearchMatchRowHeight(); heights.push([i, height]); indexes.push(i); @@ -1990,9 +1990,6 @@ var ItemTree = class ItemTree extends LibraryTree { * holds. Empty for any selection with something else in it, so a caller * can tell "these are passages" from "these are items". * - * A pending row stands in for passages that don't exist yet and names - * none. - * * @return {Object[]} - { itemID, entry } per selected passage */ getSelectedSearchMatches() { @@ -2000,9 +1997,7 @@ var ItemTree = class ItemTree extends LibraryTree { if (!selected.length || !selected.every(ref => ref instanceof SearchMatch)) { return []; } - return selected - .filter(ref => ref.entry) - .map(ref => ({ itemID: ref.itemID, entry: ref.entry })); + return selected.map(ref => ({ itemID: ref.itemID, entry: ref.entry })); } /** @@ -2439,12 +2434,7 @@ var ItemTree = class ItemTree extends LibraryTree { div.classList.toggle('first-highlighted', this._highlightedRows.has(rowData.id) && !this._highlightedRows.has(prevRowID)); div.classList.toggle('last-highlighted', this._highlightedRows.has(rowData.id) && !this._highlightedRows.has(nextRowID)); div.classList.toggle('annotation-row', row.type === 'annotation'); - // Both kinds of match row: one stands in for the other, and they lay - // out the same. Toggled here rather than set while rendering, since - // the tree recycles a row's div for whatever row next needs one. - div.classList.toggle( - 'search-match-row', - row.type === 'search-match' || row.type === 'search-match-placeholder'); + div.classList.toggle('search-match-row', row.type === 'search-match'); div.classList.toggle('library-header-row', row.type === 'library-header'); div.classList.toggle('spacer-row', row.type === 'spacer'); if (row.type !== 'annotation') { @@ -2474,13 +2464,6 @@ var ItemTree = class ItemTree extends LibraryTree { row.renderRow(div, index, columns, rowData, this._renderCtx); - // A pending search-match row on screen is the demand signal for - // deriving its item's previews: the virtualized list only renders - // what's visible, so rendering names exactly what's worth deriving - if (row.type == 'search-match-placeholder') { - this.rowProvider.onSearchMatchRendered?.(row.ref.itemID); - } - if (!oldDiv) { if (this.props.dragAndDrop && row.isDraggable) { div.setAttribute('draggable', true); diff --git a/chrome/content/zotero/itemTreeRow.js b/chrome/content/zotero/itemTreeRow.js index 9f1ece42ad..71ca57412d 100644 --- a/chrome/content/zotero/itemTreeRow.js +++ b/chrome/content/zotero/itemTreeRow.js @@ -479,7 +479,7 @@ class FileItemTreeRow extends ZoteroItemTreeRow { isContainerEmpty({ getMatchPreviews } = {}) { // An attachment with search matches to show can be expanded even // with no annotations of its own - if (getMatchPreviews?.(this.ref.id)) { + if (getMatchPreviews?.(this.ref.id)?.state == 'filled') { return false; } return this.ref.numAnnotations() == 0; @@ -598,31 +598,24 @@ class AnnotationItemTreeRow extends ZoteroItemTreeRow { /** * The reference a search-match row wraps: one place a best-match search - * matched inside an item, or -- with no entry yet -- a stand-in for that - * item's matches while its preview is still being derived. + * matched inside an item. * * Item tree rows normally wrap data objects. A preview isn't a stored * object, so this stands in as the tree's reference to one. */ class SearchMatch { - constructor(itemID, entry = null) { + constructor(itemID, entry) { this.itemID = itemID; - // A preview entry (see Zotero.BestMatch.Session#getPreviews()), or - // null while the item's previews are still pending + // A preview entry (see Zotero.BestMatch.Session#getPreviews()) this.entry = entry; - this.treeViewID = 'SM' + itemID + (entry ? '-' + entry.key : '-pending'); + this.treeViewID = 'SM' + itemID + '-' + entry.key; this.id = this.treeViewID; } - get isPending() { - return !this.entry; - } - /** * The search-match refs to materialize under an item, from its - * best-match preview: one pending ref while the preview is being - * derived, one ref per quoted entry once it's filled, and nothing when - * the item has no preview or its preview derived nothing. + * best-match preview: one ref per quoted entry, and nothing when the + * item has no filled preview or its preview derived nothing. * * A preview holds every passage the item matched in; the tree shows the * strongest few, which are the ones with a line quoted. The rest are @@ -636,12 +629,9 @@ class SearchMatch { */ static forItem(item, getMatchPreviews) { let preview = getMatchPreviews?.(item.id); - if (!preview) { + if (preview?.state != 'filled') { return []; } - if (preview.state == 'pending') { - return [new SearchMatch(item.id)]; - } return preview.entries .slice(0, Zotero.BestMatch.MAX_QUOTED_PASSAGES) .map(entry => new SearchMatch(item.id, entry)); @@ -808,36 +798,6 @@ class SearchMatchItemTreeRow extends ItemTreeRow { } } -/** - * Row standing in for an item's search-match rows while its preview is - * still pending: a single row showing that matches are on their way, which - * the fill replaces with the item's SearchMatchItemTreeRows. The ref is a - * SearchMatch with no entry yet. - */ -class SearchMatchPlaceholderItemTreeRow extends SearchMatchItemTreeRow { - get type() { - return 'search-match-placeholder'; - } - - getDisplayTitle() { - return ''; - } - - getLocationLabel() { - return ''; - } - - renderPrimaryCell(index, data, column) { - let span = document.createElement('span'); - span.className = `cell ${column.className} primary`; - let textSpan = document.createElement('span'); - textSpan.className = 'cell-text search-match-pending'; - textSpan.textContent = Zotero.ftl.formatValueSync('items-search-match-pending'); - span.append(this._renderLines(textSpan)); - return span; - } -} - /** * Row wrapping a Zotero.Collection (shown in trash view). */ @@ -1015,11 +975,7 @@ class SpacerItemTreeRow extends ItemTreeRow { ItemTreeRow.create = function (ref, level, isOpen) { if (ref instanceof Zotero.Collection) return new CollectionItemTreeRow(ref, level, isOpen); if (ref instanceof Zotero.Search) return new SearchItemTreeRow(ref, level, isOpen); - if (ref instanceof SearchMatch) { - return ref.isPending - ? new SearchMatchPlaceholderItemTreeRow(ref, level, isOpen) - : new SearchMatchItemTreeRow(ref, level, isOpen); - } + if (ref instanceof SearchMatch) return new SearchMatchItemTreeRow(ref, level, isOpen); if (ref.isAnnotation?.()) return new AnnotationItemTreeRow(ref, level, isOpen); if (ref.isFileAttachment?.()) return new FileItemTreeRow(ref, level, isOpen); return new ZoteroItemTreeRow(ref, level, isOpen); @@ -1031,7 +987,6 @@ module.exports.ZoteroItemTreeRow = ZoteroItemTreeRow; module.exports.FileItemTreeRow = FileItemTreeRow; module.exports.AnnotationItemTreeRow = AnnotationItemTreeRow; module.exports.SearchMatchItemTreeRow = SearchMatchItemTreeRow; -module.exports.SearchMatchPlaceholderItemTreeRow = SearchMatchPlaceholderItemTreeRow; module.exports.SearchMatch = SearchMatch; module.exports.CollectionItemTreeRow = CollectionItemTreeRow; module.exports.SearchItemTreeRow = SearchItemTreeRow; diff --git a/chrome/content/zotero/xpcom/bestMatch.js b/chrome/content/zotero/xpcom/bestMatch.js index bf0c64a02f..c2e0031080 100644 --- a/chrome/content/zotero/xpcom/bestMatch.js +++ b/chrome/content/zotero/xpcom/bestMatch.js @@ -46,10 +46,9 @@ Zotero.BestMatch = new function () { const LEXICAL_WEIGHT = 0.3; // About a line: what a passage is quoted down to for a one-line preview const SNIPPET_CHARS = 150; - // Most passages quoted for one item. Quoting one costs work -- sometimes - // the model's -- and the strongest few already say what the item has to - // offer at a glance. The rest are still derived: they're read whole - // rather than quoted, which needs no line chosen. + // Most passages quoted for one item. The strongest few already say what + // the item has to offer at a glance; the rest are still derived -- + // they're read whole rather than quoted, which needs no line chosen. const MAX_QUOTED_PASSAGES = 3; this.MAX_QUOTED_PASSAGES = MAX_QUOTED_PASSAGES; @@ -96,6 +95,56 @@ Zotero.BestMatch = new function () { return _useSemantic() && !!Zotero.Embeddings.normalizeQuery(queryText || ''); }; + /** + * Embedding-index coverage over the given libraries, for banners + * explaining incomplete best-match results. Null when the semantic + * engine is disabled or every eligible item is indexed. Never throws -- + * the state is informational and shouldn't break a search. + * + * @param {Number[]} [libraryIDs] - Limit coverage to these libraries; + * all libraries when empty + * @return {Promise} - { type: 'indexing'|'paused', indexed, + * total } + */ + this.getIndexState = async function (libraryIDs = []) { + try { + let status = Zotero.Embeddings.Indexing.getStatus(); + if (!status.enabled) { + return null; + } + // Counts aren't populated until the indexer runs in this session + if (!status.libraries.length) { + status = await Zotero.Embeddings.Indexing.refreshStatus(); + } + let ids = new Set(libraryIDs); + let libraries = status.libraries + .filter(lib => !ids.size || ids.has(lib.libraryID)); + // Coverage is coverage: attachment fulltext is reported separately + // in the preferences, but an incomplete index is incomplete + // whichever part of it is still filling in + let indexed = libraries.reduce( + (sum, lib) => sum + lib.indexed + lib.indexedAttachments, 0); + let total = libraries.reduce( + (sum, lib) => sum + lib.eligible + lib.eligibleAttachments, 0); + if (indexed >= total) { + return null; + } + // Only an explicit pause reports as paused. Anything else -- + // between runs (startup, the pre-run debounce) or after an error + // (detailed in the preferences) -- reports as indexing, since the + // state explains the incomplete coverage, not the indexer. + return { + type: status.paused ? 'paused' : 'indexing', + indexed, + total + }; + } + catch (e) { + Zotero.logError(e); + return null; + } + }; + /** * Score a given set of items by relevance to a query. Items that aren't * matches by any active engine's standards aren't returned. Scores are @@ -191,32 +240,21 @@ Zotero.BestMatch = new function () { /** * A best-match search session: one query's scoring pass plus the - * previews explaining its matches, derived on demand. + * previews explaining its matches. * - * score() ranks candidates and synchronously builds a pending preview - * per matched item that has anything to show, with no I/O. request() - * names the items whose previews are wanted next; each call replaces - * the last, so only what is still wanted gets derived, and repeating a - * request is free. Derivation runs one item at a time, each waiting - * first for a moment when the main thread has nothing else to do. An - * item's entries arrive all at once -- both engines' evidence, merged, - * deduplicated and ordered by strength (see getMatchingExcerpts()) -- - * and onUpdate reports each item whose preview settled; consumers read - * them back with getPreviews(). A disposed session derives nothing and - * never calls onUpdate. + * score() ranks candidates and, before it resolves, derives a preview + * for every matched item that has anything to show, so consumers read + * settled previews (getPreviews()) the moment scoring ends. An item's + * entries hold both engines' evidence, merged, deduplicated and ordered + * by strength (see getMatchingExcerpts()). A re-score keeps previews + * already derived; fill() re-derives ones invalidate() dropped back to + * pending. A disposed session derives nothing. */ this.Session = class { constructor(queryText) { - // Called with the itemIDs whose previews settled since the last - // call, from filling or from a failed derivation - this.onUpdate = null; this._queryText = queryText; this._previews = new Map(); - this._queue = []; - this._inFlight = new Set(); - this._pumping = false; this._disposed = false; - this._scoreGeneration = 0; } get queryText() { @@ -225,30 +263,39 @@ Zotero.BestMatch = new function () { /** * Score candidates for this session's query (see - * Zotero.BestMatch.scoreItemIDs()) and rebuild the preview set from - * the engines' match sets, synchronously and with no I/O once - * scoring resolves: a pending preview for every item a preview is - * shown for that some engine can show match excerpts in. Items still - * matched keep their settled previews -- a re-score doesn't drop - * derived text -- and items no longer matched lose theirs. A scoring - * pass superseded by a newer one on the same session leaves the - * previews to the newer pass. + * Zotero.BestMatch.scoreItemIDs()), rebuild the preview set from the + * engines' match sets, recompute ranks and barFractions, and derive + * every pending preview before resolving, best-scored first. Items + * still matched keep their settled previews -- a re-score doesn't + * re-derive kept text -- and items no longer matched lose theirs. * * @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} - itemID -> score, as scoreItemIDs() returns * @throws {Zotero.BestMatch.ScoringCancelledError} */ async score(itemIDs, options = {}) { - let generation = ++this._scoreGeneration; let { scores, matches } = await Zotero.BestMatch.scoreItemIDs( this._queryText, itemIDs, options); - if (this._disposed || generation != this._scoreGeneration) { + 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 (!_hasPreviews(itemID)) { + if (!scores.has(itemID) || !_hasPreviews(itemID)) { continue; } let existing = this._previews.get(itemID); @@ -264,9 +311,80 @@ Zotero.BestMatch = new function () { }); } this._previews = previews; + this._rank(scores); + for (let itemID of [...scores.entries()] + .sort((a, b) => b[1] - a[1]) + .map(([id]) => id) + .filter(id => previews.get(id)?.state == 'pending')) { + if (this._disposed) { + return scores; + } + if (options.shouldCancel?.()) { + throw new Zotero.BestMatch.ScoringCancelledError(); + } + await this._derive(itemID); + } return scores; } + /** + * Ranks from this session's last scoring pass: 1-based, tied + * effective scores share a rank, and every row with a match anywhere + * beneath it is covered (see _rank()). Empty before the first pass. + * + * @return {Map} - treeViewID -> rank (1 = most relevant) + */ + get ranks() { + return this._ranks ?? new Map(); + } + + /** + * Score fractions for the relevance bars, keyed like ranks: each + * row's own score alone, so a row that only inherited its rank from + * a descendant shows its rank over an empty bar + * + * @return {Map} - treeViewID -> 0-1 fraction + */ + get barFractions() { + return this._barFractions ?? new Map(); + } + + // Rank the scored items, lifting each item's score onto its ancestors + // (annotation -> attachment -> top-level item) first, so an item's + // effective score -- and so its rank -- is the best match anywhere + // beneath it. Equal effective scores get equal ranks, so tied rows + // (including a child and its parent) order deterministically via the + // consumer's secondary sort fields. + _rank(scores) { + let effectiveScores = new Map(scores); + for (let [itemID, score] of scores) { + let parentItemID = Zotero.Items.get(itemID)?.parentItemID; + while (parentItemID) { + let current = effectiveScores.get(parentItemID); + if (current === undefined || score > current) { + effectiveScores.set(parentItemID, score); + } + parentItemID = Zotero.Items.get(parentItemID)?.parentItemID; + } + } + let rankOfScore = new Map( + [...new Set(effectiveScores.values())].sort((a, b) => b - a) + .map((score, i) => [score, i + 1]) + ); + let ranks = new Map(); + let fractions = new Map(); + for (let [itemID, score] of effectiveScores) { + let item = Zotero.Items.get(itemID); + if (!item) { + continue; + } + ranks.set(item.treeViewID, rankOfScore.get(score)); + fractions.set(item.treeViewID, scores.get(itemID) || 0); + } + this._ranks = ranks; + this._barFractions = fractions; + } + /** * The preview to show for an item, or null when there's nothing to * show: no preview for it (see _hasPreviews()), or one that derived @@ -275,7 +393,7 @@ Zotero.BestMatch = new function () { * * @param {Number} itemID * @return {Object|null} - { state, entries }: state is 'pending' - * (placeholder) or 'filled'; entries are the derived entries + * (not yet derived) or 'filled'; entries are the derived entries * (see getMatchingExcerpts()), each with a `key` unique within * the preview and stable for as long as the preview stays filled */ @@ -285,41 +403,24 @@ Zotero.BestMatch = new function () { }; /** - * Derive previews for a small batch of items immediately. + * Derive the given items' previews, in order, for previews put back + * to pending after scoring -- see invalidate(). Items already settled + * are skipped, so filling again is free. * * @param {Number[]} itemIDs */ - async preload(itemIDs) { + async fill(itemIDs) { for (let itemID of itemIDs) { if (this._disposed) { return; } - await this._settle(itemID); + await this._derive(itemID); } } /** - * Ask for the given items' previews to be derived next. Each call - * replaces the previous request -- items no longer asked for aren't - * derived -- and items already settled or mid-derivation are - * skipped, so repeating a request is free. - * - * @param {Number[]} itemIDs - */ - request(itemIDs) { - if (this._disposed) { - return; - } - this._queue = itemIDs.filter((itemID) => { - return this._previews.get(itemID)?.state == 'pending' - && !this._inFlight.has(itemID); - }); - this._pump(); - } - - /** - * Drop the given items' previews back to placeholders, for items - * whose content changed and made derived text stale + * Drop the given items' previews back to pending, for items whose + * content changed and made derived text stale * * @param {Number[]} itemIDs */ @@ -329,8 +430,8 @@ Zotero.BestMatch = new function () { if (!preview) { continue; } - // A fresh object, so a fill of the old one that's still in - // flight can't settle it (see _fill()) + // A fresh object, so a derivation of the old one that's still + // in flight can't settle it (see _derive()) this._previews.set(itemID, { ...preview, state: 'pending', @@ -340,71 +441,36 @@ Zotero.BestMatch = new function () { } /** - * End the session: abandon queued and in-flight derivation. A - * disposed session derives nothing and never calls onUpdate. + * End the session: abandon in-flight derivation. A disposed session + * derives nothing. */ dispose() { this._disposed = true; - this._queue = []; - this.onUpdate = null; } - // Derive queued previews one at a time, each first waiting for a - // moment when the main thread has nothing else to do. The queue is - // read one item per turn, so a request() arriving mid-derivation - // takes effect at the very next item. - async _pump() { - if (this._pumping) { + // Derive one pending item's preview and settle it with the result -- + // its entries, all at once, each keyed for row identity. An item + // already settled is left alone, and a preview replaced while + // deriving (see invalidate()) is left to its next derivation. A + // derivation that failed would fail again, so it settles for showing + // nothing rather than being retried. + async _derive(itemID) { + let preview = this._previews.get(itemID); + if (!preview || preview.state != 'pending') { return; } - this._pumping = true; try { - while (!this._disposed && this._queue.length) { - await new Promise( - resolve => Services.tm.idleDispatchToMainThread(resolve)); - if (this._disposed) { - return; - } - let itemID = this._queue.shift(); - if (!await this._settle(itemID)) { - continue; - } - if (!this._disposed && this.onUpdate) { - try { - this.onUpdate([itemID]); - } - catch (e) { - Zotero.logError(e); - } - } + let entries = await this.getMatchingExcerpts(itemID); + if (this._disposed || this._previews.get(itemID) != preview) { + return; } - } - finally { - this._pumping = false; - } - } - - // Derive one pending item's preview, reporting whether it settled - // here: an item already settled or mid-derivation elsewhere is left - // alone. A derivation that failed would fail again, so it settles for - // showing nothing rather than being retried. - async _settle(itemID) { - let preview = this._previews.get(itemID); - if (!preview || preview.state != 'pending' || this._inFlight.has(itemID)) { - return false; - } - this._inFlight.add(itemID); - try { - await this._fill(itemID, preview); + preview.entries = entries.map((entry, i) => ({ key: i, ...entry })); + preview.state = entries.length ? 'filled' : 'empty'; } catch (e) { Zotero.logError(e); preview.state = 'empty'; } - finally { - this._inFlight.delete(itemID); - } - return true; } /** @@ -479,10 +545,9 @@ Zotero.BestMatch = new function () { }); } entries.sort((a, b) => b.strength - a.strength); - // Quoting is the expensive half -- a passage the query's words - // aren't in has to be read by the model -- so only the passages - // that will be quoted pay for it - await this._pickSnippets(entries.slice(0, MAX_QUOTED_PASSAGES), itemID); + // Only the strongest few passages are shown as rows in the tree, + // so only they get a line chosen to quote + await this._pickSnippets(entries.slice(0, MAX_QUOTED_PASSAGES)); return entries; } @@ -561,73 +626,36 @@ Zotero.BestMatch = new function () { } /** - * Choose where in each passage to quote from: the line best showing - * the query. + * Choose where in each passage to quote from. * - * Where a passage says the query outright, that's the window covering - * the most of it. Where it only means it -- the model matched what no - * word of the query says -- the passage is cut into sentences and the - * model picks the one it finds nearest, which is the only thing that - * knows where the resemblance lives. What's quoted from there is - * whole sentences (see _quoteFrom()). - * - * The passages needing the model are asked about together, in one - * call: embedding costs far more per call than per text, so asking - * once for an item's sentences is several times cheaper than asking - * per passage. Every passage gets its opening first, so a model that - * can't answer leaves a usable quote rather than none. - * - * Both halves answer to the same gates the passages did: a session - * pinned to one engine quotes the way that engine would, rather than - * ranking with it and then quoting with the other. + * Where a passage says the query outright, the quote is the window + * covering the most of it. Where it only means it -- the model + * matched what no word of the query says -- the passage is quoted + * from its opening: whole sentences until the line is filled (see + * _quoteFrom()). The model isn't asked to choose a line -- embedding + * every quoted passage's sentences would put model calls on the + * scoring pass that derives all previews at once. * * @param {Object[]} entries - Set in place - * @param {Number} itemID */ - async _pickSnippets(entries, itemID) { + async _pickSnippets(entries) { let chunking = Zotero.Utilities.Internal.Chunking; - let useModel = this._semanticApplies(itemID); - let pending = []; for (let entry of entries) { let sentences = chunking.splitSentences( entry.text, chunking.getCharacterMetrics(entry.text)); - // The passage's opening, for a passage nothing chooses within + // The passage's opening, for a passage with no words to quote + // around entry.snippet = sentences.length - ? _quoteFrom(sentences, 0) + ? _quoteFrom(sentences) : { start: 0, end: Math.min(entry.text.length, SNIPPET_CHARS) }; - if (entry.ranges.length) { - let window = await Zotero.Lexical.pickSnippetWindow( - this._queryText, entry.text, { width: SNIPPET_CHARS }); - if (window) { - entry.snippet = window; - continue; - } - } - // A passage that is a single sentence has nothing to choose - if (!useModel || sentences.length < 2) { + if (!entry.ranges.length) { continue; } - pending.push({ entry, sentences }); - } - if (!pending.length) { - return; - } - let scores; - try { - scores = await Zotero.Embeddings.scoreTexts( - this._queryText, - pending.flatMap(({ sentences }) => sentences.map(s => s.text)) - ); - } - catch (e) { - Zotero.logError(e); - return; - } - let offset = 0; - for (let { entry, sentences } of pending) { - let mine = scores.slice(offset, offset + sentences.length); - offset += sentences.length; - entry.snippet = _quoteFrom(sentences, mine.indexOf(Math.max(...mine))); + let window = await Zotero.Lexical.pickSnippetWindow( + this._queryText, entry.text, { width: SNIPPET_CHARS }); + if (window) { + entry.snippet = window; + } } } @@ -657,18 +685,6 @@ Zotero.BestMatch = new function () { _lexicalEnabled() { return Zotero.Prefs.get('search.bestMatchEngine') != 'semantic'; } - - // Derive one item's entries, all at once. A preview replaced while - // deriving (a re-score, an invalidate) keeps the newer object - // untouched. - async _fill(itemID, preview) { - let entries = await this.getMatchingExcerpts(itemID); - if (this._disposed || this._previews.get(itemID) != preview) { - return; - } - preview.entries = entries.map((entry, i) => ({ key: i, ...entry })); - preview.state = entries.length ? 'filled' : 'empty'; - } }; /** @@ -681,19 +697,14 @@ Zotero.BestMatch = new function () { return new this.Session(queryText); }; - // The extent to quote starting at one of a passage's sentences: that - // sentence, plus the ones after it that still fit SNIPPET_CHARS. - // - // The chosen sentence is taken whole however long it is -- half a sentence - // reads as a truncation rather than as a passage, and the row clips what - // doesn't fit anyway. A short one alone reads as a fragment, so the rest - // of the line goes to what follows it. - function _quoteFrom(sentences, index) { - let { start, end } = sentences[index]; - for (let i = index + 1; i < sentences.length; i++) { - if (sentences[i].end - start > SNIPPET_CHARS) { - break; - } + // The extent to quote from a passage's opening: sentences are added until + // the quote passes SNIPPET_CHARS, so the sentence that crosses the limit + // is taken whole. Half a sentence reads as a truncation rather than as a + // passage, and the row clips what doesn't fit anyway -- while a quote cut + // short of the limit would waste the line on a fragment. + function _quoteFrom(sentences) { + let { start, end } = sentences[0]; + for (let i = 1; i < sentences.length && end - start < SNIPPET_CHARS; i++) { end = sentences[i].end; } return { start, end }; diff --git a/chrome/locale/en-US/zotero/zotero.ftl b/chrome/locale/en-US/zotero/zotero.ftl index 395b3e09b7..ae9f1db405 100644 --- a/chrome/locale/en-US/zotero/zotero.ftl +++ b/chrome/locale/en-US/zotero/zotero.ftl @@ -460,7 +460,6 @@ items-column-relevance-rank = Rank { $rank } items-best-match-indexing = Indexing in progress — { $indexed } of { $total } items indexed items-best-match-indexing-paused = Indexing is paused — { $indexed } of { $total } items indexed -items-search-match-pending = Loading matches… # $page (String) - a page label, e.g. "12" or "ix" items-search-match-page = p. { $page } diff --git a/scss/components/_item-tree.scss b/scss/components/_item-tree.scss index a7dd010017..114312928b 100644 --- a/scss/components/_item-tree.scss +++ b/scss/components/_item-tree.scss @@ -401,8 +401,7 @@ text-overflow: ellipsis; white-space: nowrap; } - .search-match-location, - .search-match-pending { + .search-match-location { color: var(--fill-secondary); } &.selected { diff --git a/test/tests/bestMatchTest.js b/test/tests/bestMatchTest.js index bb47c13637..70561bc8bf 100644 --- a/test/tests/bestMatchTest.js +++ b/test/tests/bestMatchTest.js @@ -324,7 +324,7 @@ describe("Zotero.BestMatch", function () { assert.isUndefined(excerpts[0].score); }); - it("should quote the way a pinned semantic engine would", async function () { + it("should keep the lexical engine out of a pinned semantic session's quotes", async function () { stubs.push(sinon.stub(Zotero.Embeddings, 'isEnabled').returns(true)); stubs.push(sinon.stub(Zotero.Embeddings, 'getScoreFraction').callsFake(score => score)); let head = 'A first paragraph that never mentions the bird at all. '.repeat(5); @@ -336,9 +336,6 @@ describe("Zotero.BestMatch", function () { stubs.push(rangesStub); let windowStub = sinon.stub(Zotero.Lexical, 'pickSnippetWindow'); stubs.push(windowStub); - // The model reads the lines and prefers the last - stubs.push(sinon.stub(Zotero.Embeddings, 'scoreTexts') - .callsFake(async (query, texts) => texts.map((text, i) => i / texts.length))); pinEngine('semantic'); // The item matched lexically too, so only the pin can be keeping @@ -349,8 +346,9 @@ describe("Zotero.BestMatch", function () { assert.isFalse(rangesStub.called); assert.isFalse(windowStub.called); assert.isEmpty(excerpt.ranges); - // The model's own choice of line, not the one saying 'owl' - assert.isAbove(excerpt.snippet.start, 0); + // With no ranges to quote around, the passage's opening -- not + // the line saying 'owl' + assert.equal(excerpt.snippet.start, 0); // Nothing but the model weighed it, so its strength is the // model's fraction rather than a share of a blend assert.closeTo(excerpt.strength, 0.6, 1e-9); @@ -382,21 +380,20 @@ describe("Zotero.BestMatch", function () { } }); - it("should fill out a short chosen sentence with what follows it", async function () { + it("should quote whole opening sentences until the line is filled", async function () { stubs.push(sinon.stub(Zotero.Embeddings, 'isEnabled').returns(true)); stubs.push(sinon.stub(Zotero.Embeddings, 'getScoreFraction').callsFake(score => score)); + // The opening sentence is far too short to stand as a quote on + // its own let first = 'The owl is here.'; let second = 'A modest follow-up sentence that adds a little context.'; - let third = 'A third sentence long enough that adding it would overrun the ' - + 'budget for a quoted line, so it has to be left out of one that ' - + 'already holds two sentences before it, whatever else is true.'; + let third = 'A third sentence long enough to carry the quote past the ' + + 'budget for a quoted line, taken whole anyway, since half a ' + + 'sentence would read as a truncation rather than as a passage.'; + let fourth = 'A fourth sentence that lies beyond the filled line.'; stubs.push(sinon.stub(Zotero.Embeddings, 'getMatchingChunks').resolves([ - { text: [first, second, third].join(' '), score: 0.6 } + { text: [first, second, third, fourth].join(' '), score: 0.6 } ])); - // The model likes the first sentence best, and it is far too - // short to stand as a quote on its own - stubs.push(sinon.stub(Zotero.Embeddings, 'scoreTexts') - .callsFake(async (query, texts) => texts.map((text, i) => 1 - i))); pinEngine('semantic'); let session = await sessionFor(); @@ -405,26 +402,27 @@ describe("Zotero.BestMatch", function () { assert.equal(excerpt.snippet.start, 0); assert.include(quoted, first); - // The next sentence fits alongside it... + // The next sentence doesn't fill the line either... assert.include(quoted, second); - // ...and the one after that doesn't - assert.notInclude(quoted, third); + // ...so the one that crosses the limit is taken, whole... + assert.include(quoted, third); + // ...and nothing after it + assert.notInclude(quoted, fourth); }); - it("should not ask the model to quote an item it never ranked", async function () { + it("should never ask the model to choose a quote", async function () { stubs.push(sinon.stub(Zotero.Embeddings, 'isEnabled').returns(true)); stubs.push(sinon.stub(Zotero.Embeddings, 'getChunks').resolves([ { text: 'A passage of owlish things. '.repeat(20), chunkIndex: 0 } ])); // The lexical engine scored the passage on a term it then can't - // point at -- the one way a passage arrives with no ranges to - // quote around + // point at -- so the passage arrives with no ranges to quote + // around, which used to be the model's cue to pick a line stubs.push(sinon.stub(Zotero.Lexical, 'scoreTexts').resolves([0.8])); stubs.push(sinon.stub(Zotero.Lexical, 'findMatchRanges').resolves([[]])); let scoreTextsStub = sinon.stub(Zotero.Embeddings, 'scoreTexts'); stubs.push(scoreTextsStub); - // Hybrid, but scoring recorded no semantic match for this item let session = await sessionFor({ semantic: false }); let [excerpt] = await session.getMatchingExcerpts(attachment.id); @@ -496,103 +494,88 @@ describe("Zotero.BestMatch", function () { return stub; } - function settledOnce(session) { - return new Promise((resolve) => { - session.onUpdate = resolve; - }); - } - - it("should build placeholder previews from the engines' match sets", async function () { + it("should score and derive previews for the engines' matches", async function () { stubScore(new Map([[att1.id, 0.9], [att2.id, 0.8], [att3.id, 0.7]]), [att1.id], [att2.id]); + stubDerive(new Map([ + [att1.id, [ + { source: 'title', text: 'owl atlas', ranges: [[0, 3]], strength: 1 }, + { source: 'abstract', text: 'about owls', ranges: [[6, 10]], strength: 0.5 } + ]], + [att2.id, [{ source: 'content', text: 'strigiformes', ranges: [], strength: 1 }]] + ])); let session = Zotero.BestMatch.createSession('owl'); let scores = await session.score([att1.id, att2.id, att3.id]); assert.equal(scores.get(att1.id), 0.9); - assert.equal(session.getPreviews(att1.id).state, 'pending'); - assert.equal(session.getPreviews(att2.id).state, 'pending'); - // A scored item neither engine can show matches in -- a semantic - // match that is its own preview -- gets no placeholder - assert.isNull(session.getPreviews(att3.id)); - }); - - it("should fill requested previews all at once and report them", async function () { - stubScore(new Map([[att1.id, 0.9]]), [att1.id]); - let derive = stubDerive(new Map([[att1.id, [ - { source: 'title', text: 'owl atlas', ranges: [[0, 3]], strength: 1 }, - { source: 'abstract', text: 'about owls', ranges: [[6, 10]], strength: 0.5 } - ]]])); - let session = Zotero.BestMatch.createSession('owl'); - await session.score([att1.id]); - let settled = settledOnce(session); - session.request([att1.id]); - assert.deepEqual(await settled, [att1.id]); + // Settled by the time score() resolves let preview = session.getPreviews(att1.id); assert.equal(preview.state, 'filled'); assert.deepEqual(preview.entries.map(entry => entry.key), [0, 1]); assert.equal(preview.entries[0].text, 'owl atlas'); - assert.deepEqual(derive.firstCall.args, [att1.id]); + assert.equal(session.getPreviews(att2.id).state, 'filled'); + // A scored item neither engine can show matches in -- a semantic + // match that is its own preview -- gets no preview + assert.isNull(session.getPreviews(att3.id)); }); - it("should not derive again for a repeated or settled request", async function () { - stubScore(new Map([[att1.id, 0.9]]), [att1.id]); - let derive = stubDerive(new Map([[att1.id, [ - { source: 'title', text: 'owl', ranges: [], strength: 1 } - ]]])); - let session = Zotero.BestMatch.createSession('owl'); - await session.score([att1.id]); - let settled = settledOnce(session); - session.request([att1.id]); - await settled; - session.request([att1.id]); - await Zotero.Promise.delay(50); - assert.equal(derive.callCount, 1); - }); - - it("should derive preloaded previews without waiting to be requested", 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 }]] - ])); - let session = Zotero.BestMatch.createSession('owl'); - await session.score([att1.id, att2.id]); - - await session.preload([att1.id]); - // Settled by the time preload() resolves, with no request() and no - // wait for an idle main thread - assert.equal(session.getPreviews(att1.id).state, 'filled'); - assert.equal(session.getPreviews(att2.id).state, 'pending'); - - // A preview already in hand costs nothing to preload again - await session.preload([att1.id]); - assert.equal(derive.callCount, 1); - }); - - it("should not preload after dispose", async function () { - stubScore(new Map([[att1.id, 0.9]]), [att1.id]); - let derive = stubDerive(new Map([[att1.id, [ - { source: 'title', text: 'owl', ranges: [], strength: 1 } - ]]])); - let session = Zotero.BestMatch.createSession('owl'); - await session.score([att1.id]); - session.dispose(); - await session.preload([att1.id]); - assert.equal(derive.callCount, 0); - }); - - it("should let a newer request supersede an older one", async function () { + 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'); - await session.score([att1.id, att2.id]); - let settled = settledOnce(session); - // The second request lands before the first's idle batch runs - session.request([att1.id]); - session.request([att2.id]); - assert.deepEqual(await settled, [att2.id]); + 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); - assert.equal(session.getPreviews(att1.id).state, 'pending'); + }); + + 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 }); + stubScore(new Map([[child.id, 0.9], [att1.id, 0.5]]), [child.id, att1.id]); + stubDerive(new Map()); + let session = Zotero.BestMatch.createSession('owl'); + await session.score([child.id, att1.id]); + // The child's score lifts onto its parent, which shares its rank + assert.equal(session.ranks.get(child.treeViewID), 1); + assert.equal(session.ranks.get(parent.treeViewID), 1); + assert.equal(session.ranks.get(att1.treeViewID), 2); + // The bar reports only the row's own score: an empty bar for a + // row that only inherited its rank + assert.equal(session.barFractions.get(child.treeViewID), 0.9); + assert.equal(session.barFractions.get(parent.treeViewID), 0); + assert.equal(session.barFractions.get(att1.treeViewID), 0.5); + }); + + it("should not derive after dispose", async function () { + stubScore(new Map([[att1.id, 0.9]]), [att1.id]); + let derive = stubDerive(new Map([[att1.id, [ + { source: 'title', text: 'owl', ranges: [], strength: 1 } + ]]])); + let session = Zotero.BestMatch.createSession('owl'); + session.dispose(); + await session.score([att1.id]); + await session.fill([att1.id]); + assert.equal(derive.callCount, 0); + assert.isNull(session.getPreviews(att1.id)); + }); + + it("should abandon scoring between derivations when shouldCancel says to", 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'); + // Cancels between the first item and the second + let e = await getPromiseError(session.score([att1.id, att2.id], { + shouldCancel: () => session.getPreviews(att1.id)?.state == 'filled' + })); + assert.instanceOf(e, Zotero.BestMatch.ScoringCancelledError); + assert.equal(derive.callCount, 1); + assert.equal(session.getPreviews(att2.id).state, 'pending'); }); it("should show nothing for a preview that derives nothing, and not retry it", async function () { @@ -600,12 +583,8 @@ describe("Zotero.BestMatch", function () { let derive = stubDerive(new Map()); let session = Zotero.BestMatch.createSession('owl'); await session.score([att1.id]); - let settled = settledOnce(session); - session.request([att1.id]); - assert.deepEqual(await settled, [att1.id]); assert.isNull(session.getPreviews(att1.id)); - session.request([att1.id]); - await Zotero.Promise.delay(50); + await session.fill([att1.id]); assert.equal(derive.callCount, 1); }); @@ -615,49 +594,27 @@ describe("Zotero.BestMatch", function () { .rejects(new Error('cache file missing'))); let session = Zotero.BestMatch.createSession('owl'); await session.score([att1.id]); - let settled = settledOnce(session); - session.request([att1.id]); - assert.deepEqual(await settled, [att1.id]); assert.isNull(session.getPreviews(att1.id)); }); - it("should rederive an invalidated preview on the next request", async function () { + it("should rederive an invalidated preview on the next fill", async function () { stubScore(new Map([[att1.id, 0.9]]), [att1.id]); let derive = stubDerive(new Map([[att1.id, [ { source: 'title', text: 'owl', ranges: [], strength: 1 } ]]])); let session = Zotero.BestMatch.createSession('owl'); await session.score([att1.id]); - let settled = settledOnce(session); - session.request([att1.id]); - await settled; session.invalidate([att1.id]); assert.equal(session.getPreviews(att1.id).state, 'pending'); - let settledAgain = settledOnce(session); - session.request([att1.id]); - await settledAgain; + await session.fill([att1.id]); assert.equal(derive.callCount, 2); assert.equal(session.getPreviews(att1.id).state, 'filled'); - }); - it("should derive nothing and never report after dispose", async function () { - stubScore(new Map([[att1.id, 0.9]]), [att1.id]); - let derive = stubDerive(new Map([[att1.id, [ - { source: 'title', text: 'owl', ranges: [], strength: 1 } - ]]])); - let session = Zotero.BestMatch.createSession('owl'); - await session.score([att1.id]); - let updated = false; - session.onUpdate = () => { - updated = true; - }; - session.request([att1.id]); - session.dispose(); - await Zotero.Promise.delay(50); - assert.isFalse(updated); - assert.equal(derive.callCount, 0); + // A preview already in hand costs nothing to fill again + await session.fill([att1.id]); + assert.equal(derive.callCount, 2); }); it("should keep settled previews across a re-score and drop unmatched items", async function () { @@ -671,20 +628,20 @@ describe("Zotero.BestMatch", function () { scores: new Map([[att1.id, 0.9]]), matches: { lexical: new Set([att1.id]), semantic: new Set() } }); - stubDerive(new Map([[att1.id, [ - { source: 'title', text: 'owl', ranges: [], strength: 1 } - ]]])); + let derive = stubDerive(new Map([ + [att1.id, [{ source: 'title', text: 'owl', ranges: [], strength: 1 }]], + [att2.id, [{ source: 'title', text: 'two', ranges: [], strength: 1 }]] + ])); let session = Zotero.BestMatch.createSession('owl'); await session.score([att1.id, att2.id]); - let settled = settledOnce(session); - session.request([att1.id]); - await settled; + assert.equal(derive.callCount, 2); await session.score([att1.id, att2.id]); - // The derived text survives the re-score... + // The derived text survives the re-score without re-deriving... let preview = session.getPreviews(att1.id); assert.equal(preview.state, 'filled'); assert.equal(preview.entries[0].text, 'owl'); + assert.equal(derive.callCount, 2); // ...and an item no longer matched loses its preview assert.isNull(session.getPreviews(att2.id)); }); diff --git a/test/tests/collectionViewItemTreeTest.js b/test/tests/collectionViewItemTreeTest.js index a066b15e77..852cbfbb68 100644 --- a/test/tests/collectionViewItemTreeTest.js +++ b/test/tests/collectionViewItemTreeTest.js @@ -259,25 +259,6 @@ describe("CollectionViewItemTree", function () { }); } - // The demand path is what most of these tests exercise, so skip the - // preload that would otherwise settle top-ranked previews before - // their rows are ever drawn (see _applyBestMatch()) - function skipPreload() { - stubs.push(sinon.stub(Zotero.BestMatch.Session.prototype, 'preload').resolves()); - } - - // Wait for a row to appear (or, with present = false, disappear): - // preview derivation is demand-driven and asynchronous, so the - // 1->n replacement lands some time after the placeholder renders - async function waitForMatchRow(view, id, present = true) { - let deadline = Date.now() + 5000; - while ((view.getRowIndexByID(id) === false) == present - && Date.now() < deadline) { - await Zotero.Promise.delay(10); - } - return view.getRowIndexByID(id); - } - beforeEach(function () { stubs.push(sinon.stub(Zotero.Embeddings, 'isEnabled').returns(true)); stubs.push(sinon.stub(Zotero.Embeddings, 'getScoreFraction').callsFake(score => score)); @@ -524,7 +505,7 @@ describe("CollectionViewItemTree", function () { } }); - it("should show placeholder match rows under matched attachments", async function () { + it("should show match rows under matched attachments, derived before the rows appear", async function () { let col = await createDataObject('collection'); let item = await createDataObject('item', { title: "matchrow A", collections: [col.id] }); let attachment = await importFileAttachment('test.pdf', { parentID: item.id }); @@ -532,17 +513,20 @@ describe("CollectionViewItemTree", function () { itemIDs.includes(attachment.id) ? [[attachment.id, 0.8]] : [])); stubs.push(sinon.stub(Zotero.Embeddings, 'scoreItemIDs') .callsFake(scoreEnvelope(new Map()))); - - // Hold derivation so the placeholder stays put for the test - skipPreload(); - stubs.push(sinon.stub(Zotero.BestMatch.Session.prototype, 'getMatchingExcerpts') - .returns(new Promise(() => {}))); + stubs.push(sinon.stub(Zotero.BestMatch.Session.prototype, 'getMatchingExcerpts').resolves([ + { source: 'title', text: 'matchrow owls', ranges: [[9, 13]], strength: 1 }, + { source: 'abstract', text: 'about owls', ranges: [[6, 10]], strength: 0.5 } + ])); await select(win, col); itemsView = zp.itemsView; await itemsView.setFilter('search', 'some query'); - let matchRow = itemsView.getRowIndexByID('SM' + attachment.id + '-pending'); + // One row per derived entry, already in place when the search + // resolves + let matchRow = itemsView.getRowIndexByID('SM' + attachment.id + '-0'); assert.notStrictEqual(matchRow, false); + assert.notStrictEqual(itemsView.getRowIndexByID('SM' + attachment.id + '-1'), false); + assert.equal(itemsView.getRow(matchRow).ref.entry.text, 'matchrow owls'); // The parent and the matched attachment both auto-expanded assert.isTrue(itemsView.isContainerOpen(itemsView.getRowIndexByID(item.id))); assert.isTrue(itemsView.isContainerOpen(itemsView.getRowIndexByID(attachment.id))); @@ -550,10 +534,10 @@ describe("CollectionViewItemTree", function () { // Clearing the search removes the match rows await itemsView.setFilter('search', ''); - assert.isFalse(itemsView.getRowIndexByID('SM' + attachment.id + '-pending')); + assert.isFalse(itemsView.getRowIndexByID('SM' + attachment.id + '-0')); }); - it("should place a semantic match's placeholder under its attachment", async function () { + it("should place a semantic match's rows under its attachment", async function () { let col = await createDataObject('collection'); let item = await createDataObject('item', { title: "chunkcount A", collections: [col.id] }); let attachment = await importFileAttachment('test.pdf', { parentID: item.id }); @@ -565,16 +549,14 @@ describe("CollectionViewItemTree", function () { ? [attachment.id] : []) }) )); - - // Hold derivation so the placeholder stays put for the test - skipPreload(); - stubs.push(sinon.stub(Zotero.BestMatch.Session.prototype, 'getMatchingExcerpts') - .returns(new Promise(() => {}))); + stubs.push(sinon.stub(Zotero.BestMatch.Session.prototype, 'getMatchingExcerpts').resolves([ + { source: 'content', text: 'chunkcount owls', ranges: [], strength: 1 } + ])); await select(win, col); itemsView = zp.itemsView; await itemsView.setFilter('search', 'some query'); - let matchRow = itemsView.getRowIndexByID('SM' + attachment.id + '-pending'); + let matchRow = itemsView.getRowIndexByID('SM' + attachment.id + '-0'); assert.notStrictEqual(matchRow, false); // Under the attachment, which auto-expanded to show it let attachmentRow = itemsView.getRowIndexByID(attachment.id); @@ -633,11 +615,6 @@ describe("CollectionViewItemTree", function () { itemIDs.includes(note.id) ? [[note.id, 0.8]] : [])); stubs.push(sinon.stub(Zotero.Embeddings, 'scoreItemIDs') .callsFake(scoreEnvelope(new Map()))); - - // Hold derivation so the placeholder stays put for the test - skipPreload(); - stubs.push(sinon.stub(Zotero.BestMatch.Session.prototype, 'getMatchingExcerpts') - .returns(new Promise(() => {}))); await select(win, col); itemsView = zp.itemsView; await itemsView.setFilter('search', 'some query'); @@ -647,7 +624,7 @@ describe("CollectionViewItemTree", function () { let noteRow = itemsView.getRowIndexByID(note.id); assert.notStrictEqual(noteRow, false); assert.isFalse(itemsView.isContainer(noteRow)); - assert.isFalse(itemsView.getRowIndexByID('SM' + note.id + '-pending')); + assert.isFalse(itemsView.getRowIndexByID('SM' + note.id + '-0')); }); it("should show selected match rows as passages in the item pane", async function () { @@ -658,7 +635,6 @@ describe("CollectionViewItemTree", function () { itemIDs.includes(attachment.id) ? [[attachment.id, 0.8]] : [])); stubs.push(sinon.stub(Zotero.Embeddings, 'scoreItemIDs') .callsFake(scoreEnvelope(new Map()))); - skipPreload(); stubs.push(sinon.stub(Zotero.BestMatch.Session.prototype, 'getMatchingExcerpts').resolves([ { key: 0, text: 'matchselect owls', ranges: [], strength: 1, snippet: { start: 0, end: 16 } }, @@ -669,7 +645,7 @@ describe("CollectionViewItemTree", function () { itemsView = zp.itemsView; await itemsView.setFilter('search', 'some query'); - let first = await waitForMatchRow(itemsView, 'SM' + attachment.id + '-0'); + let first = itemsView.getRowIndexByID('SM' + attachment.id + '-0'); let second = itemsView.getRowIndexByID('SM' + attachment.id + '-1'); itemsView.selection.select(second); // A passage isn't an item, so no item is selected @@ -701,58 +677,6 @@ describe("CollectionViewItemTree", function () { assert.isEmpty(itemsView.getSelectedSearchMatches()); }); - it("should show the top-ranked matches already derived on a new search", async function () { - let col = await createDataObject('collection'); - let item = await createDataObject('item', { title: "preload A", collections: [col.id] }); - let attachment = await importFileAttachment('test.pdf', { parentID: item.id }); - Zotero.Lexical.scoreItemIDs.callsFake(async (query, itemIDs) => new Map( - itemIDs.includes(attachment.id) ? [[attachment.id, 0.8]] : [])); - stubs.push(sinon.stub(Zotero.Embeddings, 'scoreItemIDs') - .callsFake(scoreEnvelope(new Map()))); - stubs.push(sinon.stub(Zotero.BestMatch.Session.prototype, 'getMatchingExcerpts') - .resolves([ - { source: 'content', text: 'preload owls', ranges: [], strength: 1 } - ])); - - await select(win, col); - itemsView = zp.itemsView; - await itemsView.setFilter('search', 'some query'); - - // The rows are drawn with their matches already in place: no - // placeholder was left for a later fill to replace - assert.notStrictEqual( - itemsView.getRowIndexByID('SM' + attachment.id + '-0'), false); - assert.isFalse( - itemsView.getRowIndexByID('SM' + attachment.id + '-pending')); - }); - - it("should replace a rendered placeholder with the derived match rows", async function () { - let col = await createDataObject('collection'); - let item = await createDataObject('item', { title: "fillrow A", collections: [col.id] }); - let attachment = await importFileAttachment('test.pdf', { parentID: item.id }); - Zotero.Lexical.scoreItemIDs.callsFake(async (query, itemIDs) => new Map( - itemIDs.includes(attachment.id) ? [[attachment.id, 0.8]] : [])); - stubs.push(sinon.stub(Zotero.Embeddings, 'scoreItemIDs') - .callsFake(scoreEnvelope(new Map()))); - skipPreload(); - stubs.push(sinon.stub(Zotero.BestMatch.Session.prototype, 'getMatchingExcerpts').resolves([ - { source: 'title', text: 'fillrow owls', ranges: [[8, 12]], strength: 1 }, - { source: 'abstract', text: 'about owls', ranges: [[6, 10]], strength: 0.5 } - ])); - - await select(win, col); - itemsView = zp.itemsView; - await itemsView.setFilter('search', 'some query'); - - // Rendering the placeholder requests the derivation; the fill - // replaces it with one row per derived entry - let first = await waitForMatchRow(itemsView, 'SM' + attachment.id + '-0'); - assert.notStrictEqual(first, false); - assert.notStrictEqual(itemsView.getRowIndexByID('SM' + attachment.id + '-1'), false); - assert.isFalse(itemsView.getRowIndexByID('SM' + attachment.id + '-pending')); - assert.equal(itemsView.getRow(first).ref.entry.text, 'fillrow owls'); - }); - it("should hide non-matching annotations when the attachment has match rows", async function () { let col = await createDataObject('collection'); let item = await createDataObject('item', { title: "annhide A", collections: [col.id] }); @@ -763,7 +687,6 @@ describe("CollectionViewItemTree", function () { itemIDs.includes(attachment.id) ? [[attachment.id, 0.8]] : [])); stubs.push(sinon.stub(Zotero.Embeddings, 'scoreItemIDs') .callsFake(scoreEnvelope(new Map()))); - skipPreload(); stubs.push(sinon.stub(Zotero.BestMatch.Session.prototype, 'getMatchingExcerpts').resolves([ { key: 0, text: 'annhide owls', ranges: [], strength: 1, snippet: { start: 0, end: 12 } } @@ -772,7 +695,6 @@ describe("CollectionViewItemTree", function () { await select(win, col); itemsView = zp.itemsView; await itemsView.setFilter('search', 'some query'); - await waitForMatchRow(itemsView, 'SM' + attachment.id + '-0'); let attachmentRow = itemsView.getRowIndexByID(attachment.id); let childrenFor = () => itemsView.getRow(attachmentRow).getChildItems({ @@ -804,7 +726,6 @@ describe("CollectionViewItemTree", function () { itemIDs.includes(attachment.id) ? [[attachment.id, 0.8]] : [])); stubs.push(sinon.stub(Zotero.Embeddings, 'scoreItemIDs') .callsFake(scoreEnvelope(new Map()))); - skipPreload(); let position = { pageIndex: 3, rects: [[1, 2, 3, 4]] }; stubs.push(sinon.stub(Zotero.BestMatch.Session.prototype, 'getMatchingExcerpts').resolves([ { key: 0, text: 'openmatch owls', ranges: [], strength: 1, @@ -819,7 +740,7 @@ describe("CollectionViewItemTree", function () { await select(win, col); itemsView = zp.itemsView; await itemsView.setFilter('search', 'some query'); - let withPosition = await waitForMatchRow(itemsView, 'SM' + attachment.id + '-0'); + let withPosition = itemsView.getRowIndexByID('SM' + attachment.id + '-0'); // From the tree: the attachment, at the passage's geometry await itemsView.handleActivate({}, [withPosition]); @@ -857,7 +778,6 @@ describe("CollectionViewItemTree", function () { itemIDs.includes(attachment.id) ? [[attachment.id, 0.8]] : [])); stubs.push(sinon.stub(Zotero.Embeddings, 'scoreItemIDs') .callsFake(scoreEnvelope(new Map()))); - skipPreload(); let entries = [0, 1, 2, 3, 4].map(i => ({ key: i, text: `quotedrows passage ${i}`, @@ -874,7 +794,6 @@ describe("CollectionViewItemTree", function () { // The tree shows the quoted passages; the rest are read in // the item pane - await waitForMatchRow(itemsView, 'SM' + attachment.id + '-0'); for (let i = 0; i < Zotero.BestMatch.MAX_QUOTED_PASSAGES; i++) { assert.notStrictEqual( itemsView.getRowIndexByID('SM' + attachment.id + '-' + i), false, @@ -887,41 +806,7 @@ describe("CollectionViewItemTree", function () { itemsView.bestMatchSession.getPreviews(attachment.id).entries, 5); }); - it("should hand a selected placeholder's selection to the first derived row", async function () { - let col = await createDataObject('collection'); - let item = await createDataObject('item', { title: "handoff A", collections: [col.id] }); - let attachment = await importFileAttachment('test.pdf', { parentID: item.id }); - Zotero.Lexical.scoreItemIDs.callsFake(async (query, itemIDs) => new Map( - itemIDs.includes(attachment.id) ? [[attachment.id, 0.8]] : [])); - stubs.push(sinon.stub(Zotero.Embeddings, 'scoreItemIDs') - .callsFake(scoreEnvelope(new Map()))); - // Hold derivation open until the placeholder is selected - skipPreload(); - let resolveDerive; - stubs.push(sinon.stub(Zotero.BestMatch.Session.prototype, 'getMatchingExcerpts') - .returns(new Promise((resolve) => { - resolveDerive = resolve; - }))); - - await select(win, col); - itemsView = zp.itemsView; - await itemsView.setFilter('search', 'some query'); - - let placeholderIndex = itemsView.getRowIndexByID('SM' + attachment.id + '-pending'); - itemsView.selection.select(placeholderIndex); - // Wait for the rendered placeholder's derivation to start, - // then let it finish - let deadline = Date.now() + 5000; - while (!resolveDerive && Date.now() < deadline) { - await Zotero.Promise.delay(10); - } - resolveDerive([{ source: 'title', text: 'handoff owls', ranges: [], strength: 1 }]); - - let first = await waitForMatchRow(itemsView, 'SM' + attachment.id + '-0'); - assert.isTrue(itemsView.selection.isSelected(first)); - }); - - it("should remove the placeholder when derivation finds nothing to show", async function () { + it("should show no match rows when derivation finds nothing to show", async function () { let col = await createDataObject('collection'); let item = await createDataObject('item', { title: "emptyfill A", collections: [col.id] }); let attachment = await importFileAttachment('test.pdf', { parentID: item.id }); @@ -929,15 +814,12 @@ describe("CollectionViewItemTree", function () { itemIDs.includes(attachment.id) ? [[attachment.id, 0.8]] : [])); stubs.push(sinon.stub(Zotero.Embeddings, 'scoreItemIDs') .callsFake(scoreEnvelope(new Map()))); - skipPreload(); stubs.push(sinon.stub(Zotero.BestMatch.Session.prototype, 'getMatchingExcerpts').resolves([])); await select(win, col); itemsView = zp.itemsView; await itemsView.setFilter('search', 'some query'); - await waitForMatchRow(itemsView, 'SM' + attachment.id + '-pending', false); - assert.isFalse(itemsView.getRowIndexByID('SM' + attachment.id + '-pending')); assert.isFalse(itemsView.getRowIndexByID('SM' + attachment.id + '-0')); // The item stays -- it's still a scored result assert.notStrictEqual(itemsView.getRowIndexByID(item.id), false);