Use storage-level statistical range estimates in the query planner

[kriszyp]

Summary

Query-planner integration of rocksdb-js #778 — feat: statistical range key-count estimation.

Range comparators (starts_with/prefix, between and the gele/gelt/gtlt/gtle family, lt/le/gt/ge) have always been estimated as fixed fractions of the table size (5%/10%/30%) — the code even said "just arbitrarily guess". That makes condition ordering, the adaptive filter→index switch, and #2147's count=estimated totals wildly wrong for any real range. When the store provides estimateCount ({ count, confidence }):

• estimateCondition now estimates the actual range the search would iterate (range construction mirrors searchByIndex's comparator switch, including the encoded prefix upper bound for starts_with and [value, null]→[value, MAXIMUM_KEY] for prefix), blended with the old fraction heuristic by the estimate's confidence — a low-confidence estimate (block-granular tiny range, open-ended complement subtraction, degraded statistics) leans on the previous behavior instead of replacing it. Estimates never go below 1; primary-key ranges estimate against the primary store.
• Negated conditions estimate Infinity at the root of estimateConditionForTable, following the existing filter-only convention (contains/ends_with): the negated flag always forces needFullScan, so a (possibly narrow) positive-range estimate must never win the driving-condition ordering. This also fixes a pre-existing defect where negated equals estimated its (possibly tiny) positive getValuesCount.
• RocksIndexStore gains an estimateCount override translating value-space bounds to its composite [indexedValue, primaryKey] keys ([value, MAXIMUM_KEY]). It and getRange share one translateIndexBounds helper, so "estimate the range you execute" is structural rather than two copies that can drift (reverse branch included).
• estimatedEntryCount switches from an exact getKeysCount() full-store iteration (re-run every 10 seconds per store) to the O(1) getEstimatedKeyCount() property read — this needs no dependency bump and pays off immediately on large tables. The intersectionEstimate divisor is floored at 1.

The dependency is now pinned at @harperfast/rocksdb-js@^2.8.0, which ships #778, so this path is live. The per-store capability probe (typeof store.estimateCount === 'function') stays because LMDB-backed and custom index stores do not implement it, and the estimate path validates the {count, confidence} shape and try/catches the native call, so a store that answers differently — or one closing concurrently — degrades to the fraction heuristic rather than NaN-poisoning plan ordering or failing the request. Bounds longer than MAX_SEARCH_KEY_LENGTH also fall back — execution truncates + filters there, so the executed range is wider than the estimable one.

For the human reviewer

• The confidence blend is the design decision: round(confidence × estimate + (1 − confidence) × fraction-heuristic). At confidence 1 the statistical estimate wins outright; at 0 the old behavior is preserved. A hard threshold was rejected as a cliff.
• estimated_count semantics: for negated conditions it is now execution-cost ordering (Infinity), not result cardinality — consistent with the other full-scan comparators. If feat(rest): total-count pagination via Prefer: count= (Content-Range) #2147 later wants a cardinality for negated queries, complement arithmetic can be added on its side.
• estimatedEntryCount semantics shift slightly and this is the one behavior change live before the dependency bump: estimate-num-keys skews high on overwrite/delete-heavy data until compaction, where the old exact scan did not. All consumers are relative-ordering or explicitly-estimated paths; the accuracy-per-cost trade is deliberate (review kept it as a noted minor).
• Per-condition native probe (~10µs, uncached beyond the existing per-condition memoization) replaces arithmetic on a 10s-cached integer in query planning. Accepted: it is bounded to one probe per range condition per planned query; a per-store range-estimate cache can be added if profiling ever shows it.
• sort, equals, in, ne branches untouched — equals selectivity still uses the exact per-value getValuesCount.
• lt/le must carry searchByIndex's start: true lower bound. Without it the estimate counts the [null, primaryKey] entries an indexNulls index holds and execution skips (true sorts above null) — measured at 21× inflation on an index that is 99% nulls, i.e. worse than the flat heuristic it replaces. Pinned by a real-store test.

Verification

• unitTests/resources/estimateRangeCondition.test.js — 26 passing on RocksDB, 19 passing / 7 pending under HARPER_STORAGE_ENGINE=lmdb (the real-store block gates on the table's actual index store, not the Rocks prototype). 13 stub-store tests — dispatch, per-comparator range construction (asserting the exact ranges passed to the store), the confidence blend (1 / 0 / 0.5), primary-key routing, capability-absent fallback, the ≥1 floor, negated→Infinity (range and equals), malformed-shape fallback (bare number, NaN, missing/out-of-range confidence, null), thrown-native-call fallback, and over-length-bound fallback. These run on the stock dependency. Plus 6 RocksIndexStore bound-translation tests asserting estimateCount and getRange forward identical bounds, and 7 end-to-end tests against real tables with secondary indexes: width-ordered between estimates, real prefix ranges for starts_with, ordered open-range tails, three cases pinned within 10× of the executed result count (secondary between, secondary lt, primary-key lt), and the sparse-index lt null-exclusion case.
• npm run test:unit:resources: 1581 passing. One unrelated pre-existing failure, requestPathRetryExhaustion.test.js — a background analytics timer calls stat(getLogFilePath()) with undefined during the test's ~20s backoff window and leaks an unhandled rejection; reproduced identically with this branch's source reverted to the pre-change tree.
• unitTests/resources/conditionsArrayMutation.test.js (planner-adjacent) passes. Full unit gates rely on CI (local runs hit shared-lock contention per prior experience).
• tsc build clean; oxlint clean; prettier applied to changed files.
• End-to-end route: the real-store block is the integration evidence; feat(rest): total-count pagination via Prefer: count= (Content-Range) #2147's count=estimated totals improve automatically once both PRs land. No user-facing API/config change → no documentation PR needed.

Review coverage

Generated by Claude (Fable 5). Cross-model pre-push review via prepush-review.mjs, six rounds:

• Round 1 (full, d05e194): Codex (graded) + Harper-domain adjudication — Gemini timed out, cursor-composer failed (output format), grok pruned. Two majors fixed in round 2: negated ranges receiving their positive-range estimate while execution full-scans, and blind trust in an unshipped dependency shape (caret-activation NaN risk). Minors fixed: no try/catch around the native call, MAX_SEARCH_KEY_LENGTH truncation divergence for starts_with, divisor floor.
• Round 2 (delta, 38052b9): Codex (Gemini failed again). Confirmed the guards; kept a major on the complement inversion (a wide positive range still made its negation look cheap) — fixed in round 3 by adopting the Infinity full-scan convention.
• Round 3 (delta, pre-rebase final head 01b8d693c): Codex on the negation-convention fix + DESIGN.md.
• Rounds 4–5 (post-rebase full, 30c51bb0f): Codex + Gemini, full re-review (the rebase moved the base commit, so the prior codex session wasn't a resumable ancestor). Fixed: negated range conditions were computing the native estimateCount before discarding it for the Infinity short-circuit (now short-circuits first); the malformed-shape guard didn't reject a negative count. Domain adjudication was pruned from these rounds — this environment's ~10-minute foreground command cap is smaller than codex+gemini's own concurrent runtime on this diff's risk/size (~460–555s), leaving no budget left for the 900s-budgeted adjudication pass; graded+gemini still ran to completion and supply independent outside coverage on their own.
• Round 6 (delta, final head 44f7fff7f): Codex + Gemini on the confidence-validation fix below; no regressions found.

Fixed: estimateRangeCondition's malformed-shape guard checked Number.isFinite(count) but only bounds-checked confidence with >=/<= (which coerce), letting a non-finite confidence slip through — now requires Number.isFinite(confidence) too.

Declined, pre-existing/mirrors execution (not introduced by this PR — the estimate path intentionally replicates searchByIndex's own range/bound construction so the estimated range never diverges from the executed one): prefix: [] (empty array) writing to the array's "-1" property instead of a real bound (mirrors an identical, older bug in searchByIndex's own prefix case, search.ts:357-365); the string-length truncation guard not checking array bounds (mirrors searchByIndex's own truncation guard, search.ts:417-425 — arrays are never truncated by execution either); lt/le's start: true allegedly excluding indexed false values (mirrors searchByIndex's own lt/le construction verbatim — pre-existing behavior, not new); RocksIndexStore's bound-translation object spread setting explicit undefined keys (this predates the PR — getRange did the same spread before this PR extracted it into the shared translateIndexBounds helper).

Declined, false positive: a Gemini finding that estimateRangeCondition is reachable (and dead) for searchType === 'sort' — sort has its own dedicated branch in estimateConditionForTable and never calls estimateRangeCondition.

Declined, judgment call / out of scope for this PR: implicit-primary-key and relationship-path range conditions fall back to the fraction heuristic instead of the statistical estimate (safe, just a missed optimization — extending coverage there is a feature addition, not this PR's scope); the broad try/catch around the native estimateCount call masking any thrown error as a closing-store degrade rather than distinguishing a real engine defect (deliberate: this is a planner-optimization path with a safe fallback, not a correctness path); no end-to-end planner-integration test proving a multi-condition plan's driving-condition ordering actually changes (test-coverage gap, not a defect); moving the fraction * estimatedEntryCount(...) + 1 fallback calculation inside estimateRangeCondition to dedup 3 call sites (legitimate DRY suggestion, but a deliberate function-contract choice carried through multiple review rounds).

Accepted, not changed (with rationale): the per-condition native probe on the planning path (bounded, ~10µs, memoized per condition); estimate-num-keys cardinality skew on overwrite-heavy tables (deliberate O(1) trade, floored divisor, relative-ordering consumers).

_{Review-Coverage: authored=claude; ran=codex,gemini; declined=cursor-grok,cursor-composer,domain; rounds=6 @ 44f7fff}

_{Human-Review-Need: 4 @ 44f7fff}

[gemini-code-assist]

Code Review

This pull request introduces statistical range estimation to the query planner, blending RocksDB's estimateCount with historical heuristics based on confidence. It also updates RocksDB's key count estimation to use the O(1) getEstimatedKeyCount method and adds comprehensive unit tests. The review feedback suggests guarding against nullish results from store.getStats() in estimatedEntryCount to prevent runtime errors, and preserving additional options (such as transactions or snapshots) when overriding estimateCount in RocksIndexStore.

[gemini-code-assist]

If store.getStats() returns nullish, accessing entryCount will throw a runtime error. Additionally, if store.getEstimatedKeyCount() or store.getStats()?.entryCount returns undefined, null, or NaN, store.estimatedEntryCount will be set to a non-finite value. We should guard against nullish results from getStats() and explicitly validate that the estimated count is a finite number, defaulting to 0 otherwise.

Suggested change

	store.estimatedEntryCount =
	store instanceof RocksDatabase ? store.getEstimatedKeyCount() : store.getStats().entryCount;
	const estimated = store instanceof RocksDatabase ? store.getEstimatedKeyCount() : store.getStats()?.entryCount;
	store.estimatedEntryCount = Number.isFinite(estimated) ? estimated : 0;

References

1. When accessing properties of a result from a method that might return nullish (e.g., backend.capabilities()), guard against nullish values to prevent runtime errors.

[gemini-code-assist]

When overriding estimateCount for RocksIndexStore, only start and end are passed to the base estimateCount call. Any other options (such as transaction, snapshot, or future parameters) are stripped. It is safer to preserve the other options while removing exclusiveStart and inclusiveEnd (since their effects have already been manually applied to the composite boundaries).

(RocksIndexStore.prototype as any).estimateCount = function estimateCount(options?: any) {
	let { start, end, exclusiveStart, inclusiveEnd } = options ?? {};
	if (exclusiveStart && start !== undefined) start = [start, MAXIMUM_KEY];
	if (inclusiveEnd && end !== undefined) end = [end, MAXIMUM_KEY];
	const baseOptions = { ...options, start, end };
	delete baseOptions.exclusiveStart;
	delete baseOptions.inclusiveEnd;
	return (RocksDatabase.prototype as any).estimateCount.call(this, baseOptions);
};

[claude]

What: store.getEstimatedKeyCount() is called unconditionally for every RocksDatabase instance, with no feature-detection, try/catch, or fallback — unlike the sibling estimateCount integration in this same PR (resources/search.ts:1120 guards with typeof store?.estimateCount !== 'function' and wraps the call in try/catch; resources/RocksIndexStore.ts:72 conditionally assigns the override so the capability probe doesn't lie).

Why it matters: estimatedEntryCount() is on the hot path for essentially every query-plan estimate against a RocksDB-backed table (not just the new range-estimation feature — it also feeds intersectionEstimate and every non-range branch's heuristic). If getEstimatedKeyCount is absent from the resolved @harperfast/rocksdb-js build (a platform-specific prebuild, a version skew, or simply if this method turns out not to exist on the pinned 2.7.1 the way estimateCount is claimed to need a future dependency bump), this throws a TypeError on effectively every RocksDB query, not a graceful degradation. That directly contradicts the PR's own stated design principle that "on the currently-pinned rocksdb-js the planner behaves exactly as before."

Suggested fix: Apply the same defensive pattern used for estimateCount elsewhere in this PR — feature-detect (typeof store.getEstimatedKeyCount === 'function') and fall back to the previous getKeysCount() when absent.

[claude]

Blocker: resources/search.ts:1726 still calls store.getEstimatedKeyCount() with no feature-detection or fallback, unlike every other rocksdb-js capability probe in this PR — see inline comment. Flagged by gemini and this bot across all prior pushes (2026-08-13, 2026-08-26, 2026-08-27 ×2) and remains unaddressed at the current head; the three commits since the last review don't touch this line.

[socket-security]

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	msgpackr@​2.0.5 ⏵ 2.0.6			+1	+4
	@​harperfast/​rocksdb-js@​2.7.1 ⏵ 2.8.0	+19

View full report

[claude]

Standing finding (unaddressed across 4 pushes): getEstimatedKeyCount() called with no feature-detection

What: store.getEstimatedKeyCount() is called unconditionally whenever store instanceof RocksDatabase, with no typeof/try-catch guard — unlike the sibling estimateCount integration added in this same PR (resources/search.ts:1124 guards with typeof store?.estimateCount !== 'function' and try/catches the native call).

Why it matters: estimatedEntryCount() runs on the hot path for essentially every RocksDB query-plan estimate (it also feeds intersectionEstimate). The store parameter here is untyped (implicit any — tsconfig.json has strict: false), so tsc provides no compile-time guarantee this method exists; a platform-specific prebuild or version skew that lacks it turns this into a TypeError on effectively every RocksDB query rather than a graceful degradation, unlike every other capability probe this PR adds for the same dependency.

This was flagged by gemini-code-assist (high) and in this bot's own review on 2026-08-13, and remains unresolved and unaddressed through the four commits pushed since (47e242a38, a76c45f2b, 641a1b725, c3620162d) — none of which touch this line.

Suggested fix: typeof store.getEstimatedKeyCount === 'function' ? store.getEstimatedKeyCount() : store.getKeysCount(), matching the defensive pattern already used for estimateCount in this PR.

[claude]

Standing finding (unaddressed across all pushes, now at final head): getEstimatedKeyCount() called with no feature-detection

What: store.getEstimatedKeyCount() is called unconditionally whenever store instanceof RocksDatabase, with no typeof/try-catch guard — unlike the sibling estimateCount integration added in this same PR (resources/search.ts:1120, guarded by typeof store?.estimateCount !== 'function' and try/caught) and unlike RocksIndexStore.estimateCount, which is only conditionally assigned so its own capability probe doesn't lie.

Why it matters: estimatedEntryCount() is on the hot path for essentially every RocksDB query-plan estimate — it also feeds intersectionEstimate and every non-range branch's heuristic, not just the new range-estimation feature. store is untyped here (implicit any), so tsc gives no compile-time guarantee this method exists on the resolved @harperfast/rocksdb-js build. A platform-specific prebuild or version skew lacking it turns this into a TypeError on effectively every RocksDB query — not a graceful degradation — contradicting the very design principle this PR's own DESIGN.md addition documents ("Capability is feature-detected per store … so a store that answers differently … degrades the plan … instead of NaN-poisoning condition ordering"), which is written one paragraph away from this exact ungated call.

This was flagged by gemini-code-assist (high) on 2026-08-13 and by this bot on 2026-08-13 and again on 2026-08-26. It remains unaddressed through every commit since, including the two most recent (e02d9bb7b, 65259335b, 9541770b6) and the branch rebase that produced the current head — none touch this line.

Suggested fix: typeof store.getEstimatedKeyCount === 'function' ? store.getEstimatedKeyCount() : store.getKeysCount(), matching the defensive pattern already used for estimateCount in this PR.

[claude]

Standing finding (unaddressed across 5+ pushes, still at current head): getEstimatedKeyCount() called with no feature-detection

What: store.getEstimatedKeyCount() is still called unconditionally whenever store instanceof RocksDatabase, with no typeof/try-catch guard — unlike the sibling estimateCount integration in this same PR (resources/search.ts:1126, guarded by typeof store?.estimateCount !== 'function' and try/caught) and unlike RocksIndexStore.estimateCount, which only overrides the base method so its own capability probe doesn't lie.

Why it matters: estimatedEntryCount() is on the hot path for essentially every RocksDB query-plan estimate — it also feeds intersectionEstimate and every non-range branch's heuristic, not just the new range-estimation feature. store is untyped here (implicit any), so tsc gives no compile-time guarantee this method exists on the resolved @harperfast/rocksdb-js build. A platform-specific prebuild or version skew lacking it turns this into a TypeError on effectively every RocksDB query — not a graceful degradation — contradicting this PR's own DESIGN.md addition ("Capability is feature-detected per store … so a store that answers differently … degrades the plan … instead of NaN-poisoning condition ordering"), written one paragraph away from this exact ungated call.

This was flagged by gemini-code-assist (high) on 2026-08-13, and by this bot on 2026-08-13, 2026-08-26, and 2026-08-27 (twice now). The two commits pushed since the last review (a824835d3, 30c51bb0f) both land above this line (in estimateRangeCondition/negation handling) and don't touch it.

Also still open from gemini's original comment on this line: the LMDB fallback branch, store.getStats().entryCount, has no nullish guard on getStats() either.

Suggested fix: typeof store.getEstimatedKeyCount === 'function' ? store.getEstimatedKeyCount() : store.getStats()?.entryCount ?? store.getKeysCount(), matching the defensive pattern already used for estimateCount in this PR.

[claude]

Standing finding (unaddressed across 6+ pushes, still at current head 44f7fff7f): getEstimatedKeyCount() called with no feature-detection

What: store.getEstimatedKeyCount() is still called unconditionally whenever store instanceof RocksDatabase, with no typeof/try-catch guard — unlike the sibling estimateCount integration in this same PR (guarded by typeof store?.estimateCount !== 'function' and try/caught) and unlike RocksIndexStore.estimateCount, which only overrides the base method so its own capability probe doesn't lie.

Why it matters: estimatedEntryCount() is on the hot path for essentially every RocksDB query-plan estimate — it also feeds intersectionEstimate and every non-range branch's heuristic, not just the new range-estimation feature. store is untyped here (implicit any), so tsc gives no compile-time guarantee this method exists on the resolved @harperfast/rocksdb-js build. A platform-specific prebuild or version skew lacking it turns this into a TypeError on effectively every RocksDB query — not a graceful degradation — contradicting this PR's own DESIGN.md addition ("Capability is feature-detected per store … so a store that answers differently … degrades the plan … instead of NaN-poisoning condition ordering"), written one paragraph away from this exact ungated call.

Flagged by gemini-code-assist (high) on 2026-08-13, and by this bot on 2026-08-13, 2026-08-26, and 2026-08-27 (three times now). The three commits pushed since the last review (a824835d3, 30c51bb0f, 44f7fff7f) all land above this line (negation short-circuit, count/confidence shape validation) and don't touch it.

Also still open from gemini's original comment on this line: the LMDB fallback branch, store.getStats().entryCount, has no nullish guard on getStats() either (pre-existing behavior, not introduced by this PR, but adjacent to the same line).

Suggested fix: typeof store.getEstimatedKeyCount === 'function' ? store.getEstimatedKeyCount() : store.getKeysCount(), matching the defensive pattern already used for estimateCount in this PR.