Skip to content

Commit 0b8e3ca

Browse files
committed
fix(index): correct doc-index alignment for Fuse<string> with blank docs
Fuse<string> collections containing blank strings produced records whose .i values drifted from _docs indices through add, removeAt, and remove/removeAll. Search results were silently misaligned with the source array — e.g. removeAt(1) on ["", "apple", "banana"] could return "banana" as a match for "apple". Root cause: FuseIndex.add / removeAt / removeAll conflated records-array offset with source-array doc-index. Blank docs produce no record (per _createStringRecord), so records.length < _docs.length and the two index spaces diverge. - FuseIndex.add(doc, docIndex): IndexRecord | null — explicit doc-index parameter; returns the appended record or null when doc is blank. - FuseIndex.removeAt(idx) — operates on doc-index, order-independent. - FuseIndex.removeAll(indices) — filter by doc-index, shift surviving .i by count of removed indices strictly less than it. - Fuse.add now only feeds the inverted index when a record was appended; previously could re-ingest the prior doc on add("") under useTokenSearch, or crash on an all-blank collection. - Fuse.removeAt(idx) validates idx (integer, in-range) atomically before any mutation. - FuseIndex.add / removeAt / removeAll gain cheap input guards for direct callers. API note: FuseIndex.add gained a required docIndex parameter. The public Fuse.add(doc) signature is unchanged; only direct callers of FuseIndex.add are affected and will throw INVALID_DOC_INDEX if called without it. The alternative — optional docIndex with this.docs.length fallback — silently produces wrong indices when the FuseIndex is shared with a Fuse instance that ran .remove (the docs reference detaches), which is exactly the bug class this fix eliminates. Migration: if you serialized a Fuse<string> index via getIndex().toJSON() from a pre-fix instance whose collection contained blanks and ran any add/remove/removeAt/removeAll before serialization, those records may misrepresent the source array — rebuild via new Fuse(docs, options). Indexes from clean string collections are unaffected. Surfaced during fuse-swift port planning; fuse-swift v1 inherits the doc-index-canonical model from this fix.
1 parent 8e55cae commit 0b8e3ca

16 files changed

Lines changed: 785 additions & 176 deletions

bench/search.mjs

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,3 +117,47 @@ console.log('\n=== remove() — 10k docs, remove 1k ===')
117117
})
118118
}, 2, 5)
119119
}
120+
121+
// --- String mutation paths (Plan 013) ---
122+
// These exercise blank-heavy string collections through the fixed mutation
123+
// paths — add() per the new docIndex-parameter signature, removeAt with the
124+
// order-independent algorithm, and removeAll with the binary-search shift
125+
// (vs the pre-fix O(N) sequential-rewrite). Roughly 1/3 of docs are blank to
126+
// stress the records-shorter-than-docs case.
127+
console.log('\n=== String mutations (blank-heavy) — 10k docs ===')
128+
{
129+
const buildDocs = (n) => {
130+
const out = new Array(n)
131+
for (let i = 0; i < n; i++) {
132+
out[i] = i % 3 === 0 ? '' : randomWords(1, 4)
133+
}
134+
return out
135+
}
136+
137+
bench('build Fuse from 10k (1/3 blank)', () => {
138+
new Fuse(buildDocs(10_000))
139+
}, 2, 5)
140+
141+
bench('add 100 strings to 10k (1/3 blank)', () => {
142+
const fuse = new Fuse(buildDocs(10_000))
143+
for (let i = 0; i < 100; i++) {
144+
fuse.add(i % 4 === 0 ? '' : 'extra ' + i)
145+
}
146+
}, 2, 5)
147+
148+
bench('removeAt(0) ×100 on 10k (1/3 blank)', () => {
149+
const fuse = new Fuse(buildDocs(10_000))
150+
for (let i = 0; i < 100; i++) {
151+
fuse.removeAt(0)
152+
}
153+
}, 2, 5)
154+
155+
bench('remove 1k from 10k (1/3 blank)', () => {
156+
const fuse = new Fuse(buildDocs(10_000))
157+
let removed = 0
158+
fuse.remove(() => {
159+
if (removed < 1000) { removed++; return true }
160+
return false
161+
})
162+
}, 2, 5)
163+
}

dist/fuse.basic.cjs

Lines changed: 79 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ const EXTENDED_SEARCH_UNAVAILABLE = 'Extended search is not available';
6262
const LOGICAL_SEARCH_UNAVAILABLE = 'Logical search is not available';
6363
const TOKEN_SEARCH_UNAVAILABLE = 'Token search is not available';
6464
const INCORRECT_INDEX_TYPE = "Incorrect 'index' type";
65+
const INVALID_DOC_INDEX = 'Invalid doc index: must be a non-negative integer within the bounds of the docs array';
6566
const LOGICAL_SEARCH_INVALID_QUERY_FOR_KEY = key => `Invalid value for key ${key}`;
6667
const PATTERN_LENGTH_TOO_LARGE = max => `Pattern length exceeds max of ${max}.`;
6768
const MISSING_KEY_PROPERTY = name => `Missing ${name} property in key`;
@@ -303,30 +304,80 @@ class FuseIndex {
303304
this.records.length = recordCount;
304305
this.norm.clear();
305306
}
306-
// Adds a doc to the end of the index
307-
add(doc) {
308-
const idx = this.size();
307+
// Appends a record for `doc` at `docIndex` (the doc's position in the source
308+
// array). Returns the appended record, or null when `doc` is a blank string
309+
// (those are skipped at record creation; see `_createStringRecord`). Callers
310+
// use the return value to gate downstream bookkeeping like the inverted
311+
// index, which must not be touched when no record was produced.
312+
add(doc, docIndex) {
313+
if (!Number.isInteger(docIndex) || docIndex < 0) {
314+
throw new Error(INVALID_DOC_INDEX);
315+
}
309316
if (isString(doc)) {
310-
this._addString(doc, idx);
311-
} else {
312-
this._addObject(doc, idx);
317+
const record = this._createStringRecord(doc, docIndex);
318+
if (record) {
319+
this.records.push(record);
320+
}
321+
return record;
313322
}
323+
const record = this._createObjectRecord(doc, docIndex);
324+
this.records.push(record);
325+
return record;
314326
}
315-
// Removes the doc at the specified index of the index
327+
// Removes the record for the doc at the specified source-array (docs) index.
328+
// Blank string docs have no record; callers may pass such an index and the
329+
// splice is a no-op, but subsequent records still need their .i decremented
330+
// to track the docs array that the caller is splicing in parallel.
316331
removeAt(idx) {
317-
this.records.splice(idx, 1);
332+
if (!Number.isInteger(idx) || idx < 0) {
333+
throw new Error(INVALID_DOC_INDEX);
334+
}
318335

319-
// Change ref index of every subsquent doc
320-
for (let i = idx, len = this.size(); i < len; i += 1) {
321-
this.records[i].i -= 1;
336+
// Find and remove the record at this doc-index, if one exists. Records are
337+
// typically sorted by .i but the algorithm doesn't depend on it — parsed
338+
// indexes via setIndexRecords may arrive in arbitrary order.
339+
for (let i = 0, len = this.records.length; i < len; i += 1) {
340+
if (this.records[i].i === idx) {
341+
this.records.splice(i, 1);
342+
break;
343+
}
344+
}
345+
346+
// Decrement every record whose source-array index is now stale.
347+
for (let i = 0, len = this.records.length; i < len; i += 1) {
348+
if (this.records[i].i > idx) {
349+
this.records[i].i -= 1;
350+
}
322351
}
323352
}
324-
// Removes docs at the specified indices
353+
// Removes records for the docs at the specified source-array indices, then
354+
// shifts every surviving record's .i down by the count of removed indices
355+
// strictly less than it (mirrors removeAndShiftInvertedIndex's shift math).
356+
// Invalid entries (non-integer, negative) in `indices` are dropped silently
357+
// — removeAll's natural use case is "caller passed a list of matched doc
358+
// indices"; asymmetric throw-vs-no-op would be more surprising than a clean
359+
// filter.
325360
removeAll(indices) {
326-
const toRemove = new Set(indices);
327-
this.records = this.records.filter((_, i) => !toRemove.has(i));
328-
for (let i = 0, len = this.records.length; i < len; i += 1) {
329-
this.records[i].i = i;
361+
const toRemove = new Set();
362+
for (const v of indices) {
363+
if (Number.isInteger(v) && v >= 0) {
364+
toRemove.add(v);
365+
}
366+
}
367+
if (toRemove.size === 0) {
368+
return;
369+
}
370+
this.records = this.records.filter(r => !toRemove.has(r.i));
371+
const sorted = Array.from(toRemove).sort((a, b) => a - b);
372+
for (const record of this.records) {
373+
// shift = count of removed indices strictly less than record.i
374+
let lo = 0;
375+
let hi = sorted.length;
376+
while (lo < hi) {
377+
const mid = lo + hi >>> 1;
378+
if (sorted[mid] < record.i) lo = mid + 1;else hi = mid;
379+
}
380+
record.i -= lo;
330381
}
331382
}
332383
getValueForItemAtKeyId(item, keyId) {
@@ -335,15 +386,6 @@ class FuseIndex {
335386
size() {
336387
return this.records.length;
337388
}
338-
_addString(doc, docIndex) {
339-
const record = this._createStringRecord(doc, docIndex);
340-
if (record) {
341-
this.records.push(record);
342-
}
343-
}
344-
_addObject(doc, docIndex) {
345-
this.records.push(this._createObjectRecord(doc, docIndex));
346-
}
347389
_createStringRecord(doc, docIndex) {
348390
if (!isDefined(doc) || isBlank(doc)) {
349391
return null;
@@ -1256,9 +1298,12 @@ class Fuse {
12561298
return;
12571299
}
12581300
this._docs.push(doc);
1259-
this._myIndex.add(doc);
1260-
if (this._invertedIndex) {
1261-
const record = this._myIndex.records[this._myIndex.records.length - 1];
1301+
const record = this._myIndex.add(doc, this._docs.length - 1);
1302+
1303+
// Skip inverted-index bookkeeping when no record was appended (blank
1304+
// strings produce null). The previous code read `records[records.length-1]`
1305+
// unconditionally, which would re-ingest the previous doc on `add("")`.
1306+
if (this._invertedIndex && record) {
12621307
const analyzer = createAnalyzer({
12631308
isCaseSensitive: this.options.isCaseSensitive,
12641309
ignoreDiacritics: this.options.ignoreDiacritics,
@@ -1291,6 +1336,12 @@ class Fuse {
12911336
return results;
12921337
}
12931338
removeAt(idx) {
1339+
// Validate before any mutation. The previous code spliced `_docs` first
1340+
// and let FuseIndex.removeAt throw afterward — partial-state on invalid
1341+
// input. Atomic now.
1342+
if (!Number.isInteger(idx) || idx < 0 || idx >= this._docs.length) {
1343+
throw new Error(INVALID_DOC_INDEX);
1344+
}
12941345
if (this._invertedIndex) {
12951346
removeAndShiftInvertedIndex(this._invertedIndex, [idx]);
12961347
}

dist/fuse.basic.min.cjs

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

dist/fuse.basic.min.mjs

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

dist/fuse.basic.mjs

Lines changed: 79 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ const EXTENDED_SEARCH_UNAVAILABLE = 'Extended search is not available';
6060
const LOGICAL_SEARCH_UNAVAILABLE = 'Logical search is not available';
6161
const TOKEN_SEARCH_UNAVAILABLE = 'Token search is not available';
6262
const INCORRECT_INDEX_TYPE = "Incorrect 'index' type";
63+
const INVALID_DOC_INDEX = 'Invalid doc index: must be a non-negative integer within the bounds of the docs array';
6364
const LOGICAL_SEARCH_INVALID_QUERY_FOR_KEY = key => `Invalid value for key ${key}`;
6465
const PATTERN_LENGTH_TOO_LARGE = max => `Pattern length exceeds max of ${max}.`;
6566
const MISSING_KEY_PROPERTY = name => `Missing ${name} property in key`;
@@ -301,30 +302,80 @@ class FuseIndex {
301302
this.records.length = recordCount;
302303
this.norm.clear();
303304
}
304-
// Adds a doc to the end of the index
305-
add(doc) {
306-
const idx = this.size();
305+
// Appends a record for `doc` at `docIndex` (the doc's position in the source
306+
// array). Returns the appended record, or null when `doc` is a blank string
307+
// (those are skipped at record creation; see `_createStringRecord`). Callers
308+
// use the return value to gate downstream bookkeeping like the inverted
309+
// index, which must not be touched when no record was produced.
310+
add(doc, docIndex) {
311+
if (!Number.isInteger(docIndex) || docIndex < 0) {
312+
throw new Error(INVALID_DOC_INDEX);
313+
}
307314
if (isString(doc)) {
308-
this._addString(doc, idx);
309-
} else {
310-
this._addObject(doc, idx);
315+
const record = this._createStringRecord(doc, docIndex);
316+
if (record) {
317+
this.records.push(record);
318+
}
319+
return record;
311320
}
321+
const record = this._createObjectRecord(doc, docIndex);
322+
this.records.push(record);
323+
return record;
312324
}
313-
// Removes the doc at the specified index of the index
325+
// Removes the record for the doc at the specified source-array (docs) index.
326+
// Blank string docs have no record; callers may pass such an index and the
327+
// splice is a no-op, but subsequent records still need their .i decremented
328+
// to track the docs array that the caller is splicing in parallel.
314329
removeAt(idx) {
315-
this.records.splice(idx, 1);
330+
if (!Number.isInteger(idx) || idx < 0) {
331+
throw new Error(INVALID_DOC_INDEX);
332+
}
316333

317-
// Change ref index of every subsquent doc
318-
for (let i = idx, len = this.size(); i < len; i += 1) {
319-
this.records[i].i -= 1;
334+
// Find and remove the record at this doc-index, if one exists. Records are
335+
// typically sorted by .i but the algorithm doesn't depend on it — parsed
336+
// indexes via setIndexRecords may arrive in arbitrary order.
337+
for (let i = 0, len = this.records.length; i < len; i += 1) {
338+
if (this.records[i].i === idx) {
339+
this.records.splice(i, 1);
340+
break;
341+
}
342+
}
343+
344+
// Decrement every record whose source-array index is now stale.
345+
for (let i = 0, len = this.records.length; i < len; i += 1) {
346+
if (this.records[i].i > idx) {
347+
this.records[i].i -= 1;
348+
}
320349
}
321350
}
322-
// Removes docs at the specified indices
351+
// Removes records for the docs at the specified source-array indices, then
352+
// shifts every surviving record's .i down by the count of removed indices
353+
// strictly less than it (mirrors removeAndShiftInvertedIndex's shift math).
354+
// Invalid entries (non-integer, negative) in `indices` are dropped silently
355+
// — removeAll's natural use case is "caller passed a list of matched doc
356+
// indices"; asymmetric throw-vs-no-op would be more surprising than a clean
357+
// filter.
323358
removeAll(indices) {
324-
const toRemove = new Set(indices);
325-
this.records = this.records.filter((_, i) => !toRemove.has(i));
326-
for (let i = 0, len = this.records.length; i < len; i += 1) {
327-
this.records[i].i = i;
359+
const toRemove = new Set();
360+
for (const v of indices) {
361+
if (Number.isInteger(v) && v >= 0) {
362+
toRemove.add(v);
363+
}
364+
}
365+
if (toRemove.size === 0) {
366+
return;
367+
}
368+
this.records = this.records.filter(r => !toRemove.has(r.i));
369+
const sorted = Array.from(toRemove).sort((a, b) => a - b);
370+
for (const record of this.records) {
371+
// shift = count of removed indices strictly less than record.i
372+
let lo = 0;
373+
let hi = sorted.length;
374+
while (lo < hi) {
375+
const mid = lo + hi >>> 1;
376+
if (sorted[mid] < record.i) lo = mid + 1;else hi = mid;
377+
}
378+
record.i -= lo;
328379
}
329380
}
330381
getValueForItemAtKeyId(item, keyId) {
@@ -333,15 +384,6 @@ class FuseIndex {
333384
size() {
334385
return this.records.length;
335386
}
336-
_addString(doc, docIndex) {
337-
const record = this._createStringRecord(doc, docIndex);
338-
if (record) {
339-
this.records.push(record);
340-
}
341-
}
342-
_addObject(doc, docIndex) {
343-
this.records.push(this._createObjectRecord(doc, docIndex));
344-
}
345387
_createStringRecord(doc, docIndex) {
346388
if (!isDefined(doc) || isBlank(doc)) {
347389
return null;
@@ -1254,9 +1296,12 @@ class Fuse {
12541296
return;
12551297
}
12561298
this._docs.push(doc);
1257-
this._myIndex.add(doc);
1258-
if (this._invertedIndex) {
1259-
const record = this._myIndex.records[this._myIndex.records.length - 1];
1299+
const record = this._myIndex.add(doc, this._docs.length - 1);
1300+
1301+
// Skip inverted-index bookkeeping when no record was appended (blank
1302+
// strings produce null). The previous code read `records[records.length-1]`
1303+
// unconditionally, which would re-ingest the previous doc on `add("")`.
1304+
if (this._invertedIndex && record) {
12601305
const analyzer = createAnalyzer({
12611306
isCaseSensitive: this.options.isCaseSensitive,
12621307
ignoreDiacritics: this.options.ignoreDiacritics,
@@ -1289,6 +1334,12 @@ class Fuse {
12891334
return results;
12901335
}
12911336
removeAt(idx) {
1337+
// Validate before any mutation. The previous code spliced `_docs` first
1338+
// and let FuseIndex.removeAt throw afterward — partial-state on invalid
1339+
// input. Atomic now.
1340+
if (!Number.isInteger(idx) || idx < 0 || idx >= this._docs.length) {
1341+
throw new Error(INVALID_DOC_INDEX);
1342+
}
12921343
if (this._invertedIndex) {
12931344
removeAndShiftInvertedIndex(this._invertedIndex, [idx]);
12941345
}

0 commit comments

Comments
 (0)