diff --git a/chrome/content/zotero/collectionViewItemTree.jsx b/chrome/content/zotero/collectionViewItemTree.jsx index 8710cb475a..a6095e7ea5 100644 --- a/chrome/content/zotero/collectionViewItemTree.jsx +++ b/chrome/content/zotero/collectionViewItemTree.jsx @@ -405,14 +405,51 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider { if (!this._renderedMatchItemIDs) { this._renderedMatchItemIDs = new Set(); Promise.resolve().then(() => { - let itemIDs = [...this._renderedMatchItemIDs]; + let painted = this._renderedMatchItemIDs; this._renderedMatchItemIDs = null; - this._bestMatchSession?.request(itemIDs); + // 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 diff --git a/chrome/content/zotero/itemTree.jsx b/chrome/content/zotero/itemTree.jsx index 444f0eae4e..c27c01be8e 100644 --- a/chrome/content/zotero/itemTree.jsx +++ b/chrome/content/zotero/itemTree.jsx @@ -2858,13 +2858,25 @@ var ItemTree = class ItemTree extends LibraryTree { if (row === undefined) { return; } - this._treebox.scrollToRow(Math.max(row - scrollPosition.offset, 0), true); + var topRow = Math.max(row - scrollPosition.offset, 0); + // scrollToRow() aligns a row's top with the viewport's, which throws + // away however far into that row the view had been scrolled. Rows are + // tall enough now for that to read as the list jumping backwards, so + // restore the exact pixel when we know it. + if (scrollPosition.pixelOffset !== undefined) { + this._treebox.scrollTo( + this._treebox._getItemPosition(topRow) + scrollPosition.pixelOffset); + return; + } + this._treebox.scrollToRow(topRow, true); } /** * Return an object describing the current scroll position to restore after changes * - * @return {Object|Boolean} - Object with .id (a treeViewID) and .offset, or false if no rows + * @return {Object|Boolean} - Object with .id (a treeViewID), .offset (rows between the + * anchor and the top of the view) and .pixelOffset (how far into the top row the + * view is scrolled), or false if no rows */ _saveScrollPosition() { if (!this._treebox) return false; @@ -2873,6 +2885,12 @@ var ItemTree = class ItemTree extends LibraryTree { if (first === undefined || first === null) { return false; } + // How far into the first visible row the view is scrolled. Measured + // against the same offset getFirstVisibleRow() reads, so the two + // always describe the same position. + var pixelOffset = typeof treebox._getItemPosition == 'function' + ? treebox.scrollOffset - treebox._getItemPosition(first) + : undefined; var last = treebox.getLastVisibleRow(); for (let i = first; i <= last; i++) { // If an object is selected, keep the first selected one in position @@ -2881,7 +2899,8 @@ var ItemTree = class ItemTree extends LibraryTree { if (!row) return false; return { id: row.ref.treeViewID, - offset: i - first + offset: i - first, + pixelOffset }; } } @@ -2899,7 +2918,8 @@ var ItemTree = class ItemTree extends LibraryTree { if (!row) return false; return { id: row.ref.treeViewID, - offset: 0 + offset: 0, + pixelOffset }; } diff --git a/chrome/content/zotero/itemTreeRow.js b/chrome/content/zotero/itemTreeRow.js index 9482d37371..165964d64b 100644 --- a/chrome/content/zotero/itemTreeRow.js +++ b/chrome/content/zotero/itemTreeRow.js @@ -647,8 +647,40 @@ class SearchMatchItemTreeRow extends ItemTreeRow { return 'search-match'; } + /** + * The line of the passage this row shows: the window best carrying the + * query (see Zotero.BestMatch.Session#getMatchingExcerpts()), cut out of + * the passage with ellipses where it cuts, and the query's matches + * located within it. + * + * The row quotes a line; the passage it came from stays whole on the + * entry, for anything that shows the match in full. + * + * @return {Object} - { text, ranges } + */ + getQuotedLine() { + let { text, ranges, snippet } = this.ref.entry; + let start = snippet ? snippet.start : 0; + let end = snippet ? snippet.end : text.length; + let prefix = start > 0 ? '…' : ''; + let quoted = prefix + text.slice(start, end) + (end < text.length ? '…' : ''); + let quotedRanges = []; + for (let [rangeStart, rangeEnd] of ranges || []) { + let from = Math.max(rangeStart, start); + let to = Math.min(rangeEnd, end); + if (from >= to) { + continue; + } + quotedRanges.push([ + from - start + prefix.length, + to - start + prefix.length + ]); + } + return { text: quoted, ranges: quotedRanges }; + } + getDisplayTitle() { - return this.ref.entry.text; + return this.getQuotedLine().text; } getField(field) { @@ -694,7 +726,7 @@ class SearchMatchItemTreeRow extends ItemTreeRow { span.className = `cell ${column.className} primary`; let textSpan = document.createElement('span'); textSpan.className = 'cell-text'; - let { text, ranges } = this.ref.entry; + let { text, ranges } = this.getQuotedLine(); let last = 0; for (let [start, end] of ranges || []) { if (start > last) { diff --git a/chrome/content/zotero/xpcom/bestMatch.js b/chrome/content/zotero/xpcom/bestMatch.js index 8146085bb5..45a7081052 100644 --- a/chrome/content/zotero/xpcom/bestMatch.js +++ b/chrome/content/zotero/xpcom/bestMatch.js @@ -38,6 +38,18 @@ Zotero.BestMatch = new function () { // rank): high enough that a handful of rank positions in one engine // can't drown out the other engine's opinion entirely const RRF_K = 60; + // How a passage's two kinds of evidence weigh against each other. The + // model's reading leads: it is the calibrated signal, and it chose which + // passages are worth showing. Saying the query's own words lifts a + // passage above an equally similar one that only paraphrases them. + const SEMANTIC_WEIGHT = 0.7; + const LEXICAL_WEIGHT = 0.3; + // About a line: what a passage is quoted down to for a one-line preview + const SNIPPET_CHARS = 200; + // Most passages shown for one item. The strongest few say what the item + // has to offer, and quoting a passage costs work -- sometimes the model's + // -- so passages past this are not worth deriving. + const MAX_PASSAGES = 3; // // Errors @@ -393,69 +405,241 @@ Zotero.BestMatch = new function () { } /** - * Every excerpt explaining why an item matched this session's query: - * the lexical engine's excerpts around the query's literal matches - * (see Zotero.Lexical.getMatchingExcerpts()) merged with the item's - * most similar indexed chunks (see - * Zotero.Embeddings.getMatchingChunks()), which carry document - * locations and have those literal matches highlighted within them - * too. A lexical fulltext excerpt a shown chunk already covers is - * dropped as redundant, and what's left is ordered by strength, each - * entry on its engine's 0-1 display scale. + * Every passage explaining why an item matched this session's query, + * strongest first. + * + * A passage is a chunk of the item's text: the chunks the semantic + * index already holds, or -- for an item it hasn't indexed -- chunks + * cut the same way from the item's own structure or flat text (see + * _getPassages()). One unit for both engines, so a match is always a + * piece of the document that knows where it sits, rather than a + * window cut around a word. + * + * At most MAX_PASSAGES come back: the strongest few say what the item + * has to offer, and quoting the rest costs more than it shows. + * + * Each passage carries the whole chunk's `text` and a `snippet` + * extent within it -- the one line that best shows the query (see + * _pickSnippets()) -- so a consumer can quote the line or read the + * passage from the same entry. `ranges` locate the query's literal + * matches in the full text. * * Only the engines scoring recorded a match in are asked (see * score()), so an item that matched one of them never pays the - * other's cost -- scanning the document's whole text, or embedding - * the query. A semantic index that isn't ready contributes nothing. + * other's cost. A semantic index that isn't ready contributes + * nothing. * * @param {Number} itemID - * @return {Promise} - Entries with `text`, `ranges`, and - * `strength`, plus chunk location fields or a lexical `source` + * @return {Promise} - Entries with `text`, `snippet`, + * `ranges` and `strength`, plus location fields where the + * passage knows them */ async getMatchingExcerpts(itemID) { let queryText = this._queryText; - let preview = this._previews.get(itemID); - // Temporary, for testing: the bestMatchEngine pref keeps the - // pinned engine's excerpts alone -- no lexical excerpts or - // highlights when pinned semantic, no chunks when pinned lexical - let engine = Zotero.Prefs.get('search.bestMatchEngine'); - // Uncapped: the tree shows every place an item matched - let options = { limit: Infinity }; - let excerpts = engine == 'semantic' || preview?.lexical === false - ? [] - : await Zotero.Lexical.getMatchingExcerpts(queryText, itemID, options); - if (preview?.semantic === false || engine == 'lexical' || !_useSemantic() - || !Zotero.Embeddings.normalizeQuery(queryText || '')) { - return excerpts; + let passages = await this._getPassages(itemID); + if (!passages.length) { + return []; } - let chunks = []; - try { - chunks = await Zotero.Embeddings.getMatchingChunks(queryText, itemID, options); - // Only fulltext chunks carry their own text; item-level - // matches have nothing to excerpt - chunks = chunks.filter(chunk => chunk.text); + let texts = passages.map(passage => passage.text); + // Chunks always highlight the query's literal matches: finding + // them in texts already in hand is cheap, unlike scanning a + // document, so it isn't gated on the item having matched + // lexically + let [ranges, lexical] = await Promise.all([ + Zotero.Lexical.findMatchRanges(queryText, texts), + this._lexicalApplies(itemID) ? Zotero.Lexical.scoreTexts(queryText, texts) : null + ]); + let entries = []; + for (let i = 0; i < passages.length; i++) { + let passage = passages[i]; + let share = lexical ? lexical[i] : 0; + // A passage the model never weighed has only its words to + // recommend it, so one that says nothing of the query isn't a + // match at all + if (passage.score === undefined && !share) { + continue; + } + entries.push({ + ...passage, + ranges: ranges[i], + strength: passage.score === undefined + ? share + : SEMANTIC_WEIGHT * Zotero.Embeddings.getScoreFraction(passage.score) + + LEXICAL_WEIGHT * share + }); } - catch (e) { - if (!(e instanceof Zotero.Embeddings.IndexNotReadyError)) { - throw e; + entries.sort((a, b) => b.strength - a.strength); + entries = entries.slice(0, MAX_PASSAGES); + // Quoting is the expensive half -- a passage the query's words + // aren't in has to be read by the model -- so it happens only for + // the passages that survived + await this._pickSnippets(entries); + return entries; + } + + /** + * The passages of an item to weigh against the query, from the best + * source the item has: the chunks the semantic index holds, the + * chunks its structured text divides into, or failing both, its flat + * text cut to the same size. Only the first two know where they sit + * in the document. + * + * @param {Number} itemID + * @return {Promise} - Passages, each with `text` and, from + * an indexed source, a semantic `score` and location fields + */ + async _getPassages(itemID) { + let queryText = this._queryText; + if (this._semanticApplies(itemID)) { + try { + let chunks = await Zotero.Embeddings.getMatchingChunks( + queryText, itemID, { limit: Infinity }); + // Only fulltext chunks carry their own text; item-level + // matches have nothing to excerpt + chunks = chunks.filter(chunk => chunk.text); + if (chunks.length) { + return chunks; + } + } + catch (e) { + if (!(e instanceof Zotero.Embeddings.IndexNotReadyError)) { + throw e; + } } } - if (!chunks.length) { - return excerpts; + // Indexed, but with no model weighing the chunks -- either it + // isn't ranking, or it found nothing here worth ranking. They + // still say how the item divides, for the words to be found in. + if (!this._lexicalApplies(itemID)) { + return []; } - let ranges = engine == 'semantic' - ? chunks.map(() => []) - : await Zotero.Lexical.findMatchRanges( - queryText, chunks.map(chunk => chunk.text)); - chunks = chunks.map((chunk, i) => ({ - ...chunk, - ranges: ranges[i], - strength: Zotero.Embeddings.getScoreFraction(chunk.score) - })); - excerpts = excerpts.filter( - excerpt => excerpt.source != 'content' || !_coveredByChunk(excerpt, chunks)); - return [...chunks, ...excerpts] - .sort((a, b) => (b.strength || 0) - (a.strength || 0)); + let indexed = await Zotero.Embeddings.getChunks(itemID); + indexed = indexed.filter(chunk => chunk.text); + if (indexed.length) { + return indexed; + } + return this._cutPassages(itemID); + } + + // Cut an unindexed item into passages the size an indexed one's are: + // along its outline where it has structured text, so each passage + // still knows its section and page, and along its flat text where it + // doesn't. + async _cutPassages(itemID) { + let item = await Zotero.Items.getAsync(itemID); + if (!item) { + return []; + } + let chunking = Zotero.Utilities.Internal.Chunking; + // Only structure already extracted: generating it costs seconds, + // which is not a price a preview may charge (see + // Zotero.SDT.getPack()). Without it the flat text still divides, + // just without knowing where its passages sit. + let structure = await Zotero.SDT.getSections(itemID, { cachedOnly: true }); + if (structure.ok && structure.sections.length) { + // The metrics only read the text for its script, so a sample + // of the opening sections says as much as all of them + let sample = structure.sections.slice(0, 5) + .map(section => section.text).join('\n\n'); + return chunking.chunkSections( + structure.sections, chunking.getCharacterMetrics(sample)); + } + let text = await item.attachmentText; + if (!text) { + return []; + } + return chunking.chunkText(text, chunking.getCharacterMetrics(text)); + } + + /** + * Choose where in each passage to quote from: the line best showing + * the query. + * + * 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 lines and the + * model picks the one it finds nearest, which is the only thing that + * knows where the resemblance lives. + * + * 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 lines 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. + * + * @param {Object[]} entries - Set in place + */ + async _pickSnippets(entries) { + let chunking = Zotero.Utilities.Internal.Chunking; + let pending = []; + for (let entry of entries) { + entry.snippet = { + 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; + } + } + if (!this._modelApplies()) { + continue; + } + let lines = chunking.chunkText(entry.text, { + ...chunking.getCharacterMetrics(entry.text), + budget: SNIPPET_CHARS, + minSize: Math.floor(SNIPPET_CHARS / 4), + overlap: 0 + }); + // A passage that is already one line has nothing to choose + if (lines.length > 1) { + pending.push({ entry, lines }); + } + } + if (!pending.length) { + return; + } + let scores; + try { + scores = await Zotero.Embeddings.scoreTexts( + this._queryText, + pending.flatMap(({ lines }) => lines.map(line => line.text)) + ); + } + catch (e) { + Zotero.logError(e); + return; + } + let offset = 0; + for (let { entry, lines } of pending) { + let mine = scores.slice(offset, offset + lines.length); + offset += lines.length; + let best = mine.indexOf(Math.max(...mine)); + entry.snippet = { start: lines[best].start, end: lines[best].end }; + } + } + + // Whether this session's query reaches each engine for the item being + // derived. The bestMatchEngine pref is temporary, for testing. + _semanticApplies(itemID) { + return this._previews.get(itemID)?.semantic !== false + && this._modelApplies(); + } + + _lexicalApplies(itemID) { + return this._previews.get(itemID)?.lexical !== false + && Zotero.Prefs.get('search.bestMatchEngine') != 'semantic'; + } + + // Whether the model can be asked about this query at all, apart from + // what it made of any one item + _modelApplies() { + return Zotero.Prefs.get('search.bestMatchEngine') != 'lexical' + && _useSemantic() + && !!Zotero.Embeddings.normalizeQuery(this._queryText || ''); } // Derive one item's entries, all at once. A preview replaced while @@ -481,26 +665,6 @@ Zotero.BestMatch = new function () { return new this.Session(queryText); }; - // Whether a lexical fulltext excerpt's matched evidence already appears - // inside one of the semantic chunks being shown, making a separate card - // for it redundant. Tested on the excerpt's first match with a little - // surrounding context -- enough to place the passage, not just the word - // -- with whitespace runs collapsed, since the two texts come from the - // same extraction but may break lines differently. - function _coveredByChunk(excerpt, chunks) { - if (!excerpt.ranges.length) { - return false; - } - let [start, end] = excerpt.ranges[0]; - let probe = excerpt.text - .slice(Math.max(0, start - 20), Math.min(excerpt.text.length, end + 20)) - // The context slice can reach the excerpt's own ellipsis marks, - // which the chunk's text doesn't contain - .replace(/…/g, '') - .replace(/\s+/g, ' '); - return chunks.some(chunk => chunk.text.replace(/\s+/g, ' ').includes(probe)); - } - // Fuse the two engines' scores with strength-weighted Reciprocal Rank // Fusion: an item's fused score sums fraction / (RRF_K + rank) over the // engines that matched it, where fraction is that engine's own 0-1 diff --git a/chrome/content/zotero/xpcom/embeddings.js b/chrome/content/zotero/xpcom/embeddings.js index c418a54f3f..cece29c216 100644 --- a/chrome/content/zotero/xpcom/embeddings.js +++ b/chrome/content/zotero/xpcom/embeddings.js @@ -1262,6 +1262,8 @@ Zotero.Embeddings = new function () { if (!this.isEnabled() || !texts.length) { return texts.map(() => 0); } + // _center() centers with the measured mean, which has to be in memory + await this.initDB(); await this.loadCalibration(); let query = _center(await this.embedQuery(queryText)); let vectors = await this.embedPassages(texts); diff --git a/chrome/content/zotero/xpcom/lexical.js b/chrome/content/zotero/xpcom/lexical.js index 2e867b1d12..994a18f37b 100644 --- a/chrome/content/zotero/xpcom/lexical.js +++ b/chrome/content/zotero/xpcom/lexical.js @@ -657,14 +657,32 @@ Zotero.Lexical = new function () { return ceiling * (FTS5_K1 + 1); } - // The terms BM25 can score with, of a query's terms. FTS5 floors the - // inverse document frequency of a term in more than about half a corpus - // (see FTS5_MIN_IDF), which is its way of saying the term separates - // nothing there -- so such a term moves no score, and pointing at it as a - // reason an item matched would be pointing at nothing. A term still - // telling documents apart in either index is kept, since that's the index - // its score came from. + // The scoring terms of the last query asked about. Deciding them costs a + // document-frequency count per term per index, and every matched item asks + // the same question about the same query. + let _scoringTermsCache = null; + + // The terms BM25 can score with, of a query's terms (see + // _computeScoringTerms()), kept for the query last asked about async function _getScoringTerms(terms) { + let key = terms.map( + term => term.type + '\u0000' + term.text + '\u0000' + !!term.prefix + ).join('\u0001'); + if (_scoringTermsCache && _scoringTermsCache.key === key) { + return _scoringTermsCache.scoring; + } + let scoring = await _computeScoringTerms(terms); + _scoringTermsCache = { key, scoring }; + return scoring; + } + + // FTS5 floors the inverse document frequency of a term in more than about + // half a corpus (see FTS5_MIN_IDF), which is its way of saying the term + // separates nothing there -- so such a term moves no score, and pointing + // at it as a reason an item matched would be pointing at nothing. A term + // still telling documents apart in either index is kept, since that's the + // index its score came from. + async function _computeScoringTerms(terms) { let scoring = []; for (let term of terms) { let tables = term.type == 'cjk' diff --git a/chrome/content/zotero/xpcom/sdt.js b/chrome/content/zotero/xpcom/sdt.js index 454c1214e3..145e5dcdf5 100644 --- a/chrome/content/zotero/xpcom/sdt.js +++ b/chrome/content/zotero/xpcom/sdt.js @@ -77,11 +77,13 @@ Zotero.SDT = new function () { * front of the worker queue (for user-initiated requests) * @param {Boolean} [options.allowStale=true] - Whether a cached pack from * an older processor version may be returned + * @param {Boolean} [options.cachedOnly] - Return 'not-cached' rather than + * extracting the document when no pack is cached * @param {Function} [options.onProgress] - Called with SDT generation * progress from 0 to 100 when generation is needed * @returns {Promise} { ok: true, bytes: ArrayBuffer, packVersion, * schemaMajorVersion }, or { ok: false, reason: 'unavailable' | - * 'password-required' | 'failed' } + * 'password-required' | 'not-cached' | 'failed' } */ this.getPack = async function (itemID, options = {}) { try { @@ -101,6 +103,11 @@ Zotero.SDT = new function () { } return _makeResult(cache); } + // Extracting a document costs seconds; a caller that only wants + // structure if it's already there says so rather than waiting + if (options.cachedOnly) { + return { ok: false, reason: 'not-cached' }; + } return await _generate(context, options); } catch (e) { diff --git a/test/tests/bestMatchTest.js b/test/tests/bestMatchTest.js index de6600ac31..0bbe8751df 100644 --- a/test/tests/bestMatchTest.js +++ b/test/tests/bestMatchTest.js @@ -206,74 +206,74 @@ describe("Zotero.BestMatch", function () { return session; } - it("should return lexical excerpts when no semantic model is enabled", async function () { - let lexicalExcerpts = [{ source: 'title', text: 'owl', ranges: [[0, 3]], strength: 1 }]; + it("should cut an unindexed item into passages of its own text", async function () { + stubs.push(sinon.stub(Zotero.Embeddings, 'isEnabled').returns(false)); let chunksStub = sinon.stub(Zotero.Embeddings, 'getMatchingChunks'); stubs.push(chunksStub); - stubs.push(sinon.stub(Zotero.Embeddings, 'isEnabled').returns(false)); - stubs.push(sinon.stub(Zotero.Lexical, 'getMatchingExcerpts') - .resolves(lexicalExcerpts)); + stubs.push(sinon.stub(Zotero.Embeddings, 'getChunks').resolves([])); + stubs.push(sinon.stub(Zotero.SDT, 'getSections').resolves({ ok: false, reason: 'none' })); + let text = 'A paragraph about owls.\n\n' + 'Filler about nothing. '.repeat(200) + + '\n\nAnother owl paragraph entirely.'; + stubs.push(sinon.stub(Zotero.Items, 'getAsync') + .resolves({ attachmentText: Promise.resolve(text) })); let session = await sessionFor(); let excerpts = await session.getMatchingExcerpts(attachment.id); + // The model was never asked about an item it hasn't indexed assert.isFalse(chunksStub.called); - assert.equal(excerpts, lexicalExcerpts); + // Only the passages that say the query come back, each a slice of + // the item's text with the query located in it + assert.isAbove(excerpts.length, 0); + for (let excerpt of excerpts) { + assert.include(text, excerpt.text); + assert.isAbove(excerpt.ranges.length, 0); + assert.isAbove(excerpt.strength, 0); + // The one line worth quoting, inside the passage it came from + assert.isAtLeast(excerpt.snippet.start, 0); + assert.isAtMost(excerpt.snippet.end, excerpt.text.length); + } }); - it("should overlay lexical highlights onto the semantic chunks that carry text", async function () { + it("should quote a passage where the query's words are", async function () { stubs.push(sinon.stub(Zotero.Embeddings, 'isEnabled').returns(true)); stubs.push(sinon.stub(Zotero.Embeddings, 'getScoreFraction').callsFake(score => score)); + let lead = 'Nothing of interest here. '.repeat(30); stubs.push(sinon.stub(Zotero.Embeddings, 'getMatchingChunks').resolves([ - { text: 'the owl chunk', score: 0.6, position: 1 }, + { text: lead + 'And here the owl appears at last.', score: 0.6, position: 1 }, + // A chunk whose source drifted has no text to quote { text: null, score: 0.9 } ])); - stubs.push(sinon.stub(Zotero.Lexical, 'getMatchingExcerpts').resolves([])); - stubs.push(sinon.stub(Zotero.Lexical, 'findMatchRanges') - .resolves([[[4, 7]]])); let session = await sessionFor(); let excerpts = await session.getMatchingExcerpts(attachment.id); assert.lengthOf(excerpts, 1); - assert.equal(excerpts[0].text, 'the owl chunk'); - assert.deepEqual(excerpts[0].ranges, [[4, 7]]); - assert.equal(excerpts[0].strength, 0.6); + let [excerpt] = excerpts; + // The whole passage is carried, for reading it in full... + assert.include(excerpt.text, 'Nothing of interest'); + // ...and the snippet is the line the query is on + assert.include(excerpt.text.slice(excerpt.snippet.start, excerpt.snippet.end), 'owl'); + // Ranges locate the query in the whole passage, not in the snippet + assert.deepEqual(excerpt.ranges, [[lead.length + 13, lead.length + 16]]); // Chunk fields pass through for the row's location line - assert.equal(excerpts[0].position, 1); + assert.equal(excerpt.position, 1); }); - it("should merge both engines' evidence, strongest first", async function () { + it("should weigh saying the query against merely resembling it", async function () { stubs.push(sinon.stub(Zotero.Embeddings, 'isEnabled').returns(true)); stubs.push(sinon.stub(Zotero.Embeddings, 'getScoreFraction').callsFake(score => score)); stubs.push(sinon.stub(Zotero.Embeddings, 'getMatchingChunks').resolves([ - { text: 'a section that mentions owl migration in passing', score: 0.5 } - ])); - stubs.push(sinon.stub(Zotero.Lexical, 'findMatchRanges') - .resolves([[[24, 37]]])); - stubs.push(sinon.stub(Zotero.Lexical, 'getMatchingExcerpts').resolves([ - { source: 'title', text: 'Owl migration atlas', ranges: [[0, 13]], strength: 1 }, - // The same passage the chunk shows: redundant - { - source: 'content', - text: '…that mentions owl migration in passing…', - ranges: [[15, 28]], - strength: 0.3 - }, - // A passage no chunk surfaced: stays - { - source: 'content', - text: '…a different owl migration passage entirely…', - ranges: [[13, 26]], - strength: 0.3 - } + { text: 'a passage the model likes but that never says the word', score: 0.9 }, + { text: 'a passage about the owl itself', score: 0.5 } ])); + stubs.push(sinon.stub(Zotero.Lexical, 'scoreTexts').resolves([0, 1])); let session = await sessionFor(); let excerpts = await session.getMatchingExcerpts(attachment.id); - assert.deepEqual( - excerpts.map(excerpt => excerpt.source || 'chunk'), - ['title', 'chunk', 'content'] - ); - assert.include(excerpts[2].text, 'different'); + assert.lengthOf(excerpts, 2); + // 0.7 * 0.5 + 0.3 * 1 beats 0.7 * 0.9 + 0.3 * 0 + assert.include(excerpts[0].text, 'the owl itself'); + assert.closeTo(excerpts[0].strength, 0.7 * 0.5 + 0.3, 1e-9); + assert.closeTo(excerpts[1].strength, 0.7 * 0.9, 1e-9); }); it("should skip the lexical engine for an item that didn't match it", async function () { @@ -296,42 +296,47 @@ describe("Zotero.BestMatch", function () { }); it("should skip the semantic engine for an item that didn't match it", async function () { - let lexicalExcerpts = [{ source: 'title', text: 'owl', ranges: [[0, 3]], strength: 1 }]; stubs.push(sinon.stub(Zotero.Embeddings, 'isEnabled').returns(true)); - stubs.push(sinon.stub(Zotero.Lexical, 'getMatchingExcerpts') - .resolves(lexicalExcerpts)); let chunksStub = sinon.stub(Zotero.Embeddings, 'getMatchingChunks'); stubs.push(chunksStub); + stubs.push(sinon.stub(Zotero.Embeddings, 'getChunks').resolves([ + { text: 'a passage naming the owl', chunkIndex: 0 } + ])); // Scoring recorded a lexical match only, so the query is never - // embedded for it + // embedded for it -- but the chunks still say how it divides let session = await sessionFor({ semantic: false }); let excerpts = await session.getMatchingExcerpts(attachment.id); assert.isFalse(chunksStub.called); - assert.equal(excerpts, lexicalExcerpts); + assert.lengthOf(excerpts, 1); + assert.equal(excerpts[0].text, 'a passage naming the owl'); + // Nothing weighed it but its words + assert.isUndefined(excerpts[0].score); }); - it("should keep the lexical excerpts alone when the model shows nothing", async function () { - let lexicalExcerpts = [{ source: 'title', text: 'owl', ranges: [[0, 3]], strength: 1 }]; + it("should read an indexed item's chunks when the model shows nothing", async function () { stubs.push(sinon.stub(Zotero.Embeddings, 'isEnabled').returns(true)); - stubs.push(sinon.stub(Zotero.Lexical, 'getMatchingExcerpts') - .resolves(lexicalExcerpts)); + stubs.push(sinon.stub(Zotero.Embeddings, 'getChunks').resolves([ + { text: 'a passage naming the owl', chunkIndex: 0 }, + { text: 'a passage naming nothing', chunkIndex: 1 } + ])); - // No chunks with text - let chunksStub = sinon.stub(Zotero.Embeddings, 'getMatchingChunks') - .resolves([{ text: null }]); + // No chunk cleared the model's floor + let chunksStub = sinon.stub(Zotero.Embeddings, 'getMatchingChunks').resolves([]); stubs.push(chunksStub); let session = await sessionFor(); - assert.equal(await session.getMatchingExcerpts(attachment.id), lexicalExcerpts); + let excerpts = await session.getMatchingExcerpts(attachment.id); + // Only the passage that says the query is a match + assert.lengthOf(excerpts, 1); + assert.equal(excerpts[0].text, 'a passage naming the owl'); - // The semantic index isn't ready + // The semantic index isn't ready: same fallback chunksStub.rejects(new Zotero.Embeddings.IndexNotReadyError('test')); - assert.equal(await session.getMatchingExcerpts(attachment.id), lexicalExcerpts); + assert.lengthOf(await session.getMatchingExcerpts(attachment.id), 1); }); it("should rethrow an unexpected semantic failure", async function () { stubs.push(sinon.stub(Zotero.Embeddings, 'isEnabled').returns(true)); - stubs.push(sinon.stub(Zotero.Lexical, 'getMatchingExcerpts').resolves([])); stubs.push(sinon.stub(Zotero.Embeddings, 'getMatchingChunks') .rejects(new Error('model exploded')));