lazy rendering of preview rows

BestMatch session extracts only a small portion of
previews during scoring to show immediately. The rest
is derived when the browser is idle to not freeze the UI
and sent to the itemTree via onPreviewsFilled callback.
Similar approach to earlier placeholder rows - but
better performing.

chrome/content/zotero/collectionViewItemTree.jsx

@@ -245,12 +245,12 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider {
 		if (newQuery) {
 			session?.dispose();
 			session = Zotero.BestMatch.createSession(query);
+			session.onPreviewsFilled = itemIDs => this._showFilledPreviews(session, itemIDs);
 			this._bestMatchSession = session;
 		}
 		try {
-			// Scoring also derives the matched items' previews before it
-			// resolves, so the rows the refresh builds draw finished match
-			// rows
+			// 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
@@ -298,6 +298,43 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider {
 		return kept;
 	}
 
+	/**
+	 * Show the match rows of previews derived after the search resolved (see
+	 * Zotero.BestMatch.Session#score()), by reopening each item -- the same
+	 * path that builds children for an expansion the user asks for.
+	 *
+	 * @param {Zotero.BestMatch.Session} session - Ignored once it isn't the
+	 *     session the tree is showing
+	 * @param {Number[]} itemIDs
+	 */
+	_showFilledPreviews(session, itemIDs) {
+		if (this._bestMatchSession !== session) {
+			return;
+		}
+		let shown = [];
+		for (let itemID of itemIDs) {
+			// Looked up per item, since reopening one shifts the rows below it
+			let item = Zotero.Items.get(itemID);
+			let index = item ? this._rowMap[item.treeViewID] : undefined;
+			if (index === undefined || !this.isContainer(index)) {
+				continue;
+			}
+			if (this.isContainerOpen(index)) {
+				this._toggleOpenState(index);
+			}
+			this._toggleOpenState(index);
+			shown.push(itemID);
+		}
+		// Redrawing is the expensive part, so a batch with no rows in the
+		// tree (under a collapsed parent, say) costs nothing
+		if (!shown.length) {
+			return;
+		}
+		// The twisty appears with the preview, so the rows redraw too
+		this.itemTree.invalidateRowCache(shown);
+		this.runListeners('update', true, { restoreSelection: true, restoreScroll: true });
+	}
+
 	/**
 	 * The session holding the passages of the active best-match search, or
 	 * null when no such search is running
@@ -814,10 +851,11 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider {
 			let attachments = item.isRegularItem() ? item.getAttachments() : [];
 			// expand item row if it is a parent of a match
 			// OR if it has a child that is a parent of a match
-			// OR if it has best-match preview rows to show
+			// OR if it has best-match preview rows to show -- one still
+			// deriving has none, and opens in _showFilledPreviews() instead
 			let shouldBeOpened = searchParentIDs.has(item.id)
 				|| attachments.some(id => searchParentIDs.has(id))
-				|| !!this._bestMatchSession?.getPreviews(item.id);
+				|| this._bestMatchSession?.getPreviews(item.id)?.state == 'filled';
 			if (shouldBeOpened) {
 				this._toggleOpenState(i, true);
 			}

chrome/content/zotero/xpcom/bestMatch.js

@@ -62,8 +62,25 @@ Zotero.BestMatch = new function () {
 	// 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;
+	// Previews derived before scoring resolves, best-scored first: enough to
+	// cover the top of the results. The rest follow in the background, since
+	// reading every matched item's text takes far longer than the ranking.
+	const PRELOADED_MATCH_PREVIEWS = 10;
+	// How long background-derived previews accumulate before the consumer
+	// hears about them, so a pass redraws the view about twice a second
+	const PREVIEW_BATCH_INTERVAL = 500;
+	// How long a background derivation waits for an idle main thread before
+	// running anyway
+	const PREVIEW_IDLE_TIMEOUT = 1000;
+	// How long the pass rests after deriving a preview, as a multiple of what
+	// that preview cost. Deriving reads the item's text; run flat out, that
+	// work lands inside the frames of whatever the user is doing and
+	// scrolling stutters.
+	const PREVIEW_PAUSE_RATIO = 3;
+	const PREVIEW_MAX_PAUSE = 250;
 
 	this.MAX_QUOTED_PASSAGES = MAX_QUOTED_PASSAGES;
+	this.PRELOADED_MATCH_PREVIEWS = PRELOADED_MATCH_PREVIEWS;
 
 	//
 	// Errors
@@ -90,6 +107,13 @@ Zotero.BestMatch = new function () {
 		return !!Zotero.Items.get(itemID)?.isFileAttachment?.();
 	}
 
+	// Resolves the next time the main thread is idle, or after
+	// PREVIEW_IDLE_TIMEOUT regardless
+	function _idle() {
+		return new Promise(resolve => requestIdleCallback(
+			resolve, { timeout: PREVIEW_IDLE_TIMEOUT }));
+	}
+
 	/**
 	 * Whether a query has anything for best-match search to rank by. The
 	 * lexical engine needs at least one scoring unit; failing that, the
@@ -275,19 +299,28 @@ Zotero.BestMatch = new function () {
 	 * A best-match search session: one query's scoring pass plus the
 	 * previews explaining its matches.
 	 *
-	 * 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.
+	 * score() ranks candidates and derives the best-scored few previews
+	 * (PRELOADED_MATCH_PREVIEWS) before it resolves. The rest derive in the
+	 * background, in score order and paced to stay out of the user's way
+	 * (see PREVIEW_PAUSE_RATIO), reported through onPreviewsFilled and
+	 * awaitable through previewsSettled. 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) {
 			this._queryText = queryText;
 			this._previews = new Map();
 			this._disposed = false;
+			// Bumped per background pass, so a re-score abandons the last one
+			this._derivation = 0;
+			this._previewsSettled = Promise.resolve();
+			// Set by a consumer showing previews as they arrive: called with
+			// the itemIDs filled since the last call (see
+			// PREVIEW_BATCH_INTERVAL), never for what score() derived itself
+			this.onPreviewsFilled = null;
 		}
 
 		get queryText() {
@@ -298,9 +331,14 @@ Zotero.BestMatch = new function () {
 		 * Score candidates for this session's query (see
 		 * 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.
+		 * the best-scored pending previews before resolving. Items still
+		 * matched keep their settled previews -- a re-score doesn't re-derive
+		 * kept text -- and items no longer matched lose theirs.
+		 *
+		 * The previews past PRELOADED_MATCH_PREVIEWS derive after this
+		 * resolves (see onPreviewsFilled, previewsSettled). Ranks and
+		 * barFractions are complete either way -- neither depends on a
+		 * preview.
 		 *
 		 * @param {Number[]} itemIDs - Candidate item IDs to score
 		 * @param {Object} [options] - Passed through to scoreItemIDs()
@@ -345,10 +383,11 @@ Zotero.BestMatch = new function () {
 			}
 			this._previews = previews;
 			this._rank(scores);
-			for (let itemID of [...scores.entries()]
+			let pending = [...scores.entries()]
 				.sort((a, b) => b[1] - a[1])
 				.map(([id]) => id)
-				.filter(id => previews.get(id)?.state == 'pending')) {
+				.filter(id => previews.get(id)?.state == 'pending');
+			for (let itemID of pending.slice(0, PRELOADED_MATCH_PREVIEWS)) {
 				if (this._disposed) {
 					return scores;
 				}
@@ -357,9 +396,65 @@ Zotero.BestMatch = new function () {
 				}
 				await this._derive(itemID);
 			}
+			this._previewsSettled = this._deriveRest(
+				pending.slice(PRELOADED_MATCH_PREVIEWS));
 			return scores;
 		}
 
+		/**
+		 * Resolves once the previews score() didn't wait for have derived, or
+		 * once the session is disposed
+		 *
+		 * @return {Promise}
+		 */
+		get previewsSettled() {
+			return this._previewsSettled;
+		}
+
+		// Derive the previews score() left behind, best-scored first,
+		// reporting them in batches (see onPreviewsFilled). Left unawaited by
+		// score(), so a consumer draws the ranking while the explanations
+		// behind it fill in. A newer pass -- another score() -- or dispose()
+		// abandons this one.
+		async _deriveRest(itemIDs) {
+			let derivation = ++this._derivation;
+			let filled = [];
+			let reportedAt = Date.now();
+			let report = () => {
+				if (!filled.length) {
+					return;
+				}
+				let reported = filled;
+				filled = [];
+				reportedAt = Date.now();
+				// A consumer that throws shouldn't strand the rest
+				try {
+					this.onPreviewsFilled?.(reported);
+				}
+				catch (e) {
+					Zotero.logError(e);
+				}
+			};
+			for (let itemID of itemIDs) {
+				await _idle();
+				if (this._disposed || derivation !== this._derivation) {
+					return;
+				}
+				let started = Date.now();
+				await this._derive(itemID);
+				await Zotero.Promise.delay(Math.min(PREVIEW_MAX_PAUSE,
+					(Date.now() - started) * PREVIEW_PAUSE_RATIO));
+				// A preview that derived nothing has no rows to redraw
+				if (this._previews.get(itemID)?.state == 'filled') {
+					filled.push(itemID);
+				}
+				if (Date.now() - reportedAt >= PREVIEW_BATCH_INTERVAL) {
+					report();
+				}
+			}
+			report();
+		}
+
 		/**
 		 * Ranks from this session's last scoring pass: 1-based, tied
 		 * effective scores share a rank, and every row with a match anywhere

test/tests/bestMatchTest.js

@@ -747,6 +747,110 @@ describe("Zotero.BestMatch", function () {
 
 	});
 
+	describe("Session background previews", function () {
+		// More matches than score() derives before resolving, so the rest go
+		// to the background pass
+		const PRELOADED = Zotero.BestMatch.PRELOADED_MATCH_PREVIEWS;
+		var atts;
+
+		before(async function () {
+			this.timeout(60000);
+			atts = [];
+			for (let i = 0; i < PRELOADED + 3; i++) {
+				atts.push(await importFileAttachment('test.pdf'));
+			}
+		});
+
+		// Scores descending in creation order, so the background pass covers
+		// the last three
+		function stubScore() {
+			let scores = new Map(atts.map((att, i) => [att.id, 1 - i / 100]));
+			stubs.push(sinon.stub(Zotero.BestMatch, 'scoreItemIDs').resolves({
+				scores,
+				matches: { lexical: new Set(scores.keys()), semantic: new Set() }
+			}));
+			return scores;
+		}
+
+		// Derives one entry per item, holding each derivation until it's let
+		// through, so a test can stop the pass at a known point
+		function stubDerive() {
+			let waiting = [];
+			let stub = sinon.stub(Zotero.BestMatch.Session.prototype, 'getMatchingExcerpts')
+				.callsFake(async () => {
+					await new Promise(resolve => waiting.push(resolve));
+					return [{ source: 'title', text: 'owl', ranges: [], strength: 1 }];
+				});
+			stubs.push(stub);
+			let release = () => waiting.splice(0).forEach(resolve => resolve());
+			return {
+				stub,
+				release,
+				// Let derivations through until `count` have started. They run
+				// one at a time, so the last is left waiting, not released.
+				advanceTo: async (count) => {
+					while (stub.callCount < count) {
+						release();
+						await Zotero.Promise.delay(0);
+					}
+				}
+			};
+		}
+
+		it("should derive the best-scored previews before resolving and the rest after", async function () {
+			stubScore();
+			let { stub, release, advanceTo } = stubDerive();
+			let reported = [];
+			let session = Zotero.BestMatch.createSession('owl');
+			session.onPreviewsFilled = itemIDs => reported.push(...itemIDs);
+			let scored = session.score(atts.map(att => att.id));
+			// Only the previews score() waits for are let through, so its
+			// resolution can't have depended on the one left in flight
+			await advanceTo(PRELOADED + 1);
+			await scored;
+
+			// Settled by the time score() resolves
+			for (let att of atts.slice(0, PRELOADED)) {
+				assert.equal(session.getPreviews(att.id).state, 'filled');
+			}
+			for (let att of atts.slice(PRELOADED)) {
+				assert.equal(session.getPreviews(att.id).state, 'pending');
+			}
+			// Nothing reported for what score() derived itself
+			assert.isEmpty(reported);
+
+			let settled = false;
+			session.previewsSettled.then(() => settled = true);
+			while (!settled) {
+				release();
+				await Zotero.Promise.delay(0);
+			}
+			await session.previewsSettled;
+			assert.equal(stub.callCount, atts.length);
+			for (let att of atts.slice(PRELOADED)) {
+				assert.equal(session.getPreviews(att.id).state, 'filled');
+			}
+			assert.sameMembers(reported, atts.slice(PRELOADED).map(att => att.id));
+		});
+
+		it("should stop the background pass when the session is disposed", async function () {
+			stubScore();
+			let { stub, release, advanceTo } = stubDerive();
+			let session = Zotero.BestMatch.createSession('owl');
+			let scored = session.score(atts.map(att => att.id));
+			await advanceTo(PRELOADED + 1);
+			await scored;
+
+			session.dispose();
+			release();
+			await session.previewsSettled;
+			// The in-flight derivation settles nothing, and the ones behind
+			// it are never asked for
+			assert.equal(stub.callCount, PRELOADED + 1);
+			assert.equal(session.getPreviews(atts[PRELOADED].id).state, 'pending');
+		});
+	});
+
 	describe("#isSearchableQuery()", function () {
 		it("should accept any query the lexical engine can parse", function () {
 			stubs.push(sinon.stub(Zotero.Embeddings, 'isEnabled').returns(false));

test/tests/collectionViewItemTreeTest.js

@@ -813,6 +813,54 @@ describe("CollectionViewItemTree", function () {
 					itemsView.bestMatchSession.getPreviews(attachment.id).entries, 5);
 			});
 
+			it("should show match rows for previews derived after the search resolves", async function () {
+				this.timeout(60000);
+				// One more attachment than score() derives before resolving,
+				// so the last preview arrives after the results are on screen
+				let preloaded = Zotero.BestMatch.PRELOADED_MATCH_PREVIEWS;
+				let col = await createDataObject('collection');
+				let item = await createDataObject('item', { title: "backfill A", collections: [col.id] });
+				let attachments = [];
+				for (let i = 0; i <= preloaded; i++) {
+					attachments.push(await importFileAttachment('test.pdf', { parentID: item.id }));
+				}
+				// Descending, so the extra attachment is the one left over
+				let ids = attachments.map(att => att.id);
+				Zotero.Lexical.scoreItemIDs.callsFake(async (query, itemIDs) => new Map(
+					ids.filter(id => itemIDs.includes(id)).map((id, i) => [id, 0.9 - i / 100])));
+				stubs.push(sinon.stub(Zotero.Embeddings, 'scoreItemIDs')
+					.callsFake(scoreEnvelope(new Map())));
+				// Held open, so the search has to resolve without it
+				let release;
+				let held = new Promise(resolve => release = resolve);
+				let derived = 0;
+				stubs.push(sinon.stub(Zotero.BestMatch.Session.prototype, 'getMatchingExcerpts')
+					.callsFake(async () => {
+						if (++derived > preloaded) {
+							await held;
+						}
+						return [{ source: 'content', text: 'backfill owls', ranges: [], strength: 1 }];
+					}));
+
+				await select(win, col);
+				itemsView = zp.itemsView;
+				await itemsView.setFilter('search', 'some query');
+
+				let last = attachments[attachments.length - 1];
+				// The preloaded ones are on screen with the results...
+				assert.notStrictEqual(itemsView.getRowIndexByID('SM' + attachments[0].id + '-0'), false);
+				// ...while the leftover is still deriving, so it has no rows
+				assert.isFalse(itemsView.getRowIndexByID('SM' + last.id + '-0'));
+
+				release();
+				await itemsView.bestMatchSession.previewsSettled;
+				// Filling expanded the attachment and added the match row
+				let matchRow = itemsView.getRowIndexByID('SM' + last.id + '-0');
+				assert.notStrictEqual(matchRow, false);
+				assert.equal(itemsView.getParentIndex(matchRow),
+					itemsView.getRowIndexByID(last.id));
+			});
+
 			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] });