Use an options object for searchByIndex and the custom-index search() contract

benchmarks/hnsw-search.js

@@ -223,7 +223,7 @@ for (const p of SELECTIVITIES) {
 
 		// predicate-aware traversal
 		const t0 = performance.now();
-		const pred = hnsw.search({ target: query, comparator: 'sort', descending: false }, {}, filter);
+		const pred = hnsw.search({ target: query, comparator: 'sort', descending: false }, {}, { filter });
 		predNs += (performance.now() - t0) * 1e6;
 		const predIdx = new Set(pred.slice(0, k).map((r) => idxOf(r.key)));
 		predRecall += overlap(gt, predIdx) / k;

resources/DESIGN.md

@@ -93,7 +93,7 @@ One giant `makeTable()` factory that returns a `TableResource extends Resource`
 | How does a query opt out of a read snapshot?                          | Pass `snapshot: false` on the search request (e.g. `get_analytics`). `Table.ts → search` calls `txn.useReadTxn(snapshot === false)`; on RocksDB `DatabaseTransaction.getReadTxn` then builds the read txn with `{ disableSnapshot: true }` so a long scan reads latest without pinning a snapshot. No-op on LMDB (`LMDBTransaction.useReadTxn`).                                                                                                                                                                                                                                                                                                                                                                                                        |
 | How does a URL path map to a Resource?                                | `Resources.ts → getMatch` (exact/prefix fast path) then `matchParamRoute` (parameterised routes); see "Path routing" below                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                              |
 | How does HNSW keep the graph connected on delete?                     | `indexes/HierarchicalNavigableSmallWorld.ts → index()` delete path: zero-degree orphans reindexed via `needsReindexing`; severed multi-node islands detected and reconnected by `repairSeveredNeighbors` (#1712)                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                        |
-| How is a filter applied _during_ a vector search?                     | Predicate-aware traversal (#1241): `search.ts → executeConditions` composes companion AND conditions with request `vectorFilter` / `rowFilter` predicates into one `(primaryKey) => boolean` (`composeRecordFilter`) and passes it to `HierarchicalNavigableSmallWorld.search(cond, ctx, filter)`. The filter gates result admission at layer 0 only (routing ignores it, ACORN-style); a visit budget (`filterExpansion`) bounds the under-filled/selective case. Very selective _condition_ filters are instead diverted to the exact brute-force path by the query planner's `estimateCountAsSort` ordering.                                                                                                                                         |
+| How is a filter applied _during_ a vector search?                     | Predicate-aware traversal (#1241): `search.ts → executeConditions` composes companion AND conditions with request `vectorFilter` / `rowFilter` predicates into one `(primaryKey) => boolean` (`composeRecordFilter`) and passes it to `HierarchicalNavigableSmallWorld.search(cond, ctx, { filter })`. The filter gates result admission at layer 0 only (routing ignores it, ACORN-style); a visit budget (`filterExpansion`) bounds the under-filled/selective case. Very selective _condition_ filters are instead diverted to the exact brute-force path by the query planner's `estimateCountAsSort` ordering.                                                                                                                                     |
 | How does post-ordering resolve vector distances safely?               | Each comparator owns its `Sort`, passes it directly to the custom-index resolver, and caches distances by that immutable per-query sort object.                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                         |
 | How is application row filtering applied?                             | Authorization admission happens in the resource operation before query work. The legacy `allow*` hook, when armed by the protocol, is evaluated once with its historical receiver semantics; overriding it never changes its scope. An operation override may add indexed conditions and/or attach the JavaScript-only synchronous `target.rowFilter(record, context)`. `Table.search` composes it with query filters and rechecks the final materialized cache/source record. `SubscriptionRequest.rowFilter` covers full-row events; `eventFilter(event, context)` explicitly handles tombstones/messages/raw events. Prefer indexed conditions because an opaque predicate may inspect every admitted candidate and `limit` applies after filtering. |
 

resources/Table.ts

@@ -4962,7 +4962,7 @@ export function makeTable(options) {
 											txnForContext(context).getReadTxn(),
 											false,
 											relatedTable,
-											false
+											{ allowFullScan: false }
 										) as any
 									).map((entry) => {
 										if (entry && entry.key !== undefined) return entry;

resources/indexes/HierarchicalNavigableSmallWorld.ts

@@ -961,12 +961,17 @@ export class HierarchicalNavigableSmallWorld {
 	 * This the main entry from Harper's query functionality, where we actually search for an ordered list of nearest
 	 * neighbors, using the provided sort/order definition object and performing the multi-layer skip-list search.
 	 * This returns an iterable of the nearest neighbors to the provided target vector, with nearest ordered first.
+	 *
+	 * This is also the contract an index implemented outside this repo has to satisfy, so everything
+	 * optional is named: a future capability (a paging cursor, a deadline, a recall target) is a new
+	 * field on `options` rather than a positional argument that breaks every existing implementation.
 	 * @param target
 	 * @param value
 	 * @param descending
 	 * @param distance
 	 * @param comparator
 	 * @param context
+	 * @param options
 	 */
 	search(
 		{
@@ -987,16 +992,21 @@ export class HierarchicalNavigableSmallWorld {
 			filterExpansion?: number;
 		},
 		context: any,
-		// Predicate-aware traversal (#1241). When provided, only nodes for which `filter(primaryKey)`
-		// returns true are admitted to the result list at layer 0; routing is unaffected. Composed by
-		// search.ts from companion AND conditions and caller-supplied vector/row filters. Must be
-		// synchronous and side-effect free. JS-API only (never from a REST query string).
-		filter?: (primaryKey: Id) => boolean,
-		// offset + limit for a bounded query. A layer-0 search returns at most `ef` candidates, so a
-		// query asking for more rows than that used to come back short with no error — capped at 512
-		// (AUTO_EF_MAX) however large the limit was. Raising ef to cover the request keeps `limit`
-		// meaningful; the caller pays for what it asked for.
-		minResults?: number
+		{
+			filter,
+			minResults,
+		}: {
+			// Predicate-aware traversal (#1241). When provided, only nodes for which `filter(primaryKey)`
+			// returns true are admitted to the result list at layer 0; routing is unaffected. Composed by
+			// search.ts from companion AND conditions and caller-supplied vector/row filters. Must be
+			// synchronous and side-effect free. JS-API only (never from a REST query string).
+			filter?: (primaryKey: Id) => boolean;
+			// offset + limit for a bounded query. A layer-0 search returns at most `ef` candidates, so a
+			// query asking for more rows than that used to come back short with no error — capped at 512
+			// (AUTO_EF_MAX) however large the limit was. Raising ef to cover the request keeps `limit`
+			// meaningful; the caller pays for what it asked for.
+			minResults?: number;
+		} = {}
 	) {
 		let limit: number | undefined; // only set for threshold comparators; 0 is a valid threshold (e.g. dotProduct)
 		let limitInclusive = false; // true for `le`, false for `lt`

resources/search.ts

@@ -140,16 +140,12 @@ export function executeConditions(
 				filtered
 				// recordAccess intentionally omitted: guards run once, at the top level (see above).
 			);
-		return searchByIndex(
-			condition,
-			txn,
-			condition.descending || request.reverse === true,
-			table,
-			request.allowFullScan,
+		return searchByIndex(condition, txn, condition.descending || request.reverse === true, table, {
+			allowFullScan: request.allowFullScan,
 			filtered,
 			context,
-			request.limit !== undefined ? (request.offset || 0) + request.limit : undefined
-		);
+			minResults: request.limit !== undefined ? (request.offset || 0) + request.limit : undefined,
+		});
 	}
 	function mapConditionsToFilters(conditions, intersection, estimatedIncomingCount) {
 		return conditions
@@ -233,26 +229,34 @@ function composeRecordFilter(recordFilters, table, context): (primaryKey: Id) =>
 }
 
 /**
- * Search for records or keys, based on the search condition, using an index if available
+ * Search for records or keys, based on the search condition, using an index if available. The four
+ * leading parameters are required at every call site; everything optional is named, so a new
+ * capability can be added without shifting positions at callers that don't use it.
  * @param searchCondition
  * @param transaction
  * @param reverse
  * @param Table
- * @param allowFullScan
- * @param filtered
+ * @param options
  */
 export function searchByIndex(
 	searchCondition: DirectCondition,
 	transaction: any,
 	reverse: boolean,
 	Table: any,
-	allowFullScan?: boolean,
-	filtered?: any,
-	context?: any,
-	// How many rows the query will ultimately consume (offset + limit), when it is bounded. An
-	// approximate index returns a fixed-size candidate list, so without this a query asking for more
-	// rows than that list holds silently gets a short result set. Only custom indexes read it.
-	minResults?: number
+	{
+		allowFullScan,
+		filtered,
+		context,
+		minResults,
+	}: {
+		allowFullScan?: boolean;
+		filtered?: any;
+		context?: any;
+		// How many rows the query will ultimately consume (offset + limit), when it is bounded. An
+		// approximate index returns a fixed-size candidate list, so without this a query asking for more
+		// rows than that list holds silently gets a short result set. Only custom indexes read it.
+		minResults?: number;
+	} = {}
 ): AsyncIterable<Id | { key: Id; value: any }> {
 	let attribute_name = searchCondition[0] ?? searchCondition.attribute;
 	let value = searchCondition[1] ?? searchCondition.value;
@@ -284,8 +288,7 @@ export function searchByIndex(
 				transaction,
 				reverse,
 				relatedTable,
-				allowFullScan,
-				joined
+				{ allowFullScan, filtered: joined }
 			);
 			if (attribute.relationship.to) {
 				// this is one-to-many or many-to-many, so we need to track the filtering of related entries that match
@@ -302,8 +305,7 @@ export function searchByIndex(
 						transaction,
 						reverse,
 						Table,
-						allowFullScan,
-						joined
+						{ allowFullScan, filtered: joined }
 					);
 				};
 				if (attribute.elements) {
@@ -510,21 +512,23 @@ export function searchByIndex(
 			// exploring until it has enough MATCHING results, rather than post-filtering an under-filled
 			// candidate set. Only indexes that opt in (filteredSearch) receive it; others post-filter as before.
 			const recordFilter = index.customIndex.filteredSearch ? searchCondition.recordFilter : undefined;
-			const loaded = index.customIndex.search(searchCondition, context, recordFilter, minResults).map((entry) => {
-				// if the custom index returns an entry with metadata, merge it with the loaded entry
-				if (typeof entry === 'object' && entry) {
-					const { key, ...otherProps } = entry;
-					if (key == null) return SKIP; // primaryKey missing from HNSW node — skip rather than crash
-					const loadedEntry = Table.primaryStore.getEntry(key, {
-						transaction: context && Table._readTxnForContext(context),
-					});
-					if (!loadedEntry) return SKIP; // record was deleted/expired or not yet visible
-					freezeRecord(loadedEntry?.value);
-					recordRead(loadedEntry);
-					return { ...otherProps, ...loadedEntry };
-				}
-				return entry;
-			});
+			const loaded = index.customIndex
+				.search(searchCondition, context, { filter: recordFilter, minResults })
+				.map((entry) => {
+					// if the custom index returns an entry with metadata, merge it with the loaded entry
+					if (typeof entry === 'object' && entry) {
+						const { key, ...otherProps } = entry;
+						if (key == null) return SKIP; // primaryKey missing from HNSW node — skip rather than crash
+						const loadedEntry = Table.primaryStore.getEntry(key, {
+							transaction: context && Table._readTxnForContext(context),
+						});
+						if (!loadedEntry) return SKIP; // record was deleted/expired or not yet visible
+						freezeRecord(loadedEntry?.value);
+						recordRead(loadedEntry);
+						return { ...otherProps, ...loadedEntry };
+					}
+					return entry;
+				});
 			if (index.customIndex.rescoreResults) {
 				const rescored = index.customIndex.rescoreResults(loaded, searchCondition, comparator, attribute_name);
 				if (rescored != null) return rescored as any;

unitTests/resources/searchByIndexOptions.test.js

@@ -0,0 +1,53 @@
+const assert = require('node:assert');
+const { setupTestDBPath } = require('../testUtils');
+const { table } = require('#src/resources/databases');
+const { searchByIndex } = require('#src/resources/search');
+const { setMainIsWorker } = require('#js/server/threads/manageThreads');
+
+// searchByIndex takes its optional tail as a named options object (#2165). These assert the options
+// are actually read off that object: a caller that reverts to positional arguments would hand
+// `false` in where the options belong, and destructuring it yields `allowFullScan === undefined` —
+// silently turning a rejected full scan into a permitted one.
+describe('searchByIndex options object (#2165)', () => {
+	if (process.env.HARPER_STORAGE_ENGINE === 'lmdb') return;
+	let T;
+
+	before(async () => {
+		setupTestDBPath();
+		setMainIsWorker(true);
+		T = table({
+			table: 'SearchByIndexOptionsTest',
+			database: 'test',
+			attributes: [
+				{ name: 'id', isPrimaryKey: true },
+				{ name: 'indexedName', indexed: true },
+				{ name: 'unindexedName' },
+			],
+		});
+		await T.put(1, { indexedName: 'a', unindexedName: 'x' });
+		await T.put(2, { indexedName: 'b', unindexedName: 'y' });
+	});
+
+	after(() => {
+		T.dropTable();
+	});
+
+	it('rejects an unindexed attribute when allowFullScan is false in the options object', () => {
+		assert.throws(
+			() =>
+				searchByIndex({ attribute: 'unindexedName', value: 'x' }, undefined, false, T, {
+					allowFullScan: false,
+				}),
+			/not indexed/,
+			'allowFullScan must be read from the named option'
+		);
+	});
+
+	it('keeps the permissive defaults when the options object is omitted entirely', async () => {
+		const ids = [];
+		for await (const entry of searchByIndex({ attribute: 'unindexedName', value: 'x' }, undefined, false, T)) {
+			ids.push(entry?.key ?? entry);
+		}
+		assert.deepStrictEqual(ids, [1], 'omitting options must full-scan, not inherit allowFullScan: false');
+	});
+});

unitTests/resources/vectorIndex.test.js

@@ -809,8 +809,7 @@ describe('HNSW limit above the resolved search ef', () => {
 			customIndex.search(
 				{ target: [1, 0, 0], comparator: 'sort' },
 				{ transaction: undefined },
-				() => true,
-				100_000 // a limit far above anything the index would choose for itself
+				{ filter: () => true, minResults: 100_000 } // a limit far above anything the index would choose for itself
 			);
 		} finally {
 			customIndex.searchLayer = originalSearchLayer;
@@ -1959,7 +1958,7 @@ describeUnlessLmdbFilter('HNSW predicate-aware traversal (#1241)', () => {
 		const results = hnsw.search(
 			{ target: [0], comparator: 'sort', descending: false },
 			{ transaction: undefined },
-			even
+			{ filter: even }
 		);
 		assert(results.length > 0, 'expected matching results');
 		assert(
@@ -1978,7 +1977,7 @@ describeUnlessLmdbFilter('HNSW predicate-aware traversal (#1241)', () => {
 		const results = hnsw.search(
 			{ target: [0], comparator: 'sort', descending: false },
 			{ transaction: undefined },
-			everyTenth
+			{ filter: everyTenth }
 		);
 		assert(
 			results.every((r) => Number(r.key) % 10 === 0),
@@ -1998,7 +1997,7 @@ describeUnlessLmdbFilter('HNSW predicate-aware traversal (#1241)', () => {
 		const results = hnsw.search(
 			{ target: [0], comparator: 'sort', descending: false, ef: 10, filterExpansion: 2 },
 			{ transaction: undefined },
-			matchesNothing
+			{ filter: matchesNothing }
 		);
 		assert.strictEqual(results.length, 0, 'no matches yields an empty result, not an error');
 		assert(results.nodesVisited > 0, 'some nodes were visited');
@@ -2011,7 +2010,7 @@ describeUnlessLmdbFilter('HNSW predicate-aware traversal (#1241)', () => {
 		const results = hnsw.search(
 			{ target: [0], comparator: 'sort', descending: false },
 			{ transaction: undefined },
-			even
+			{ filter: even }
 		);
 		assert.strictEqual(typeof results.nodesVisited, 'number');
 		assert.strictEqual(typeof results.filterEvaluations, 'number');
@@ -2027,7 +2026,7 @@ describeUnlessLmdbFilter('HNSW predicate-aware traversal (#1241)', () => {
 		const results = hnsw.search(
 			{ target: [0], comparator: 'sort', descending: false },
 			{ transaction: undefined },
-			even
+			{ filter: even }
 		);
 		assert(!results.some((r) => r.key === 10 || r.key === 20), 'deleted nodes must not be returned');
 		assert(
@@ -2042,7 +2041,7 @@ describeUnlessLmdbFilter('HNSW predicate-aware traversal (#1241)', () => {
 		const results = hnsw.search(
 			{ target: [0], comparator: 'sort', descending: false },
 			{ transaction: undefined },
-			even
+			{ filter: even }
 		);
 		assert(
 			results.every((r) => Number(r.key) % 2 === 0),