Skip to content

feat(rest): total-count pagination via Prefer: count= (Content-Range) - #2147

Merged
kriszyp merged 21 commits into
mainfrom
feat/rest-pagination-total-count
Sep 1, 2026
Merged

feat(rest): total-count pagination via Prefer: count= (Content-Range)#2147
kriszyp merged 21 commits into
mainfrom
feat/rest-pagination-total-count

Conversation

@cb1kenobi

@cb1kenobi cb1kenobi commented Aug 11, 2026

Copy link
Copy Markdown
Member

Summary

Tracking issue: #2162

Adds opt-in total-record-count for REST collection queries so a client can paginate ("1-25 of 1,234") without a second round-trip or a custom resource.

A GET/HEAD on a collection with a Prefer: count= header gets the total in RFC 7233-style response headers:

GET /Product/?category=software&limit(0,25)
Prefer: count=exact
->
200 OK
Content-Range: items 0-24/1234
Range-Unit: items
Preference-Applied: count=exact
  • count=exactTable.search drains the full matched set once, windowing the requested page in the same pass (O(matched) filter evals, O(limit) memory).
  • count=estimated — returns just the page plus a cheap planner/table estimate, no full scan.
  • No default — without the header nothing is computed and no headers are emitted.
  • Status is always 200 (Content-Range is informational, not 206). HEAD returns the headers with no body — a cheap "how many match?" pre-flight. The three headers are added to Access-Control-Expose-Headers so browsers can read them cross-origin.

count=estimated rides the storage-level range estimator

count=estimated funnels through the query planner's estimateCondition. Since Use storage-level statistical range estimates in the query planner (#2163) merged to main, that planner now derives range-query estimates from rocksdb-js 2.8.0's statistical range estimator (estimateCount) rather than an arbitrary fraction of the table size — so after merging main, this PR's count=estimated reports a range-aware total for between / starts_with / greater-less / open-range queries for free, with no change needed here.

This branch adds only the small piece the count path needs on top of #2163: it exports estimatedEntryCount from search.ts (used by the estimated whole-collection path in Table.ts), and adds REST-count coverage that a range count=estimated produces a valid, range-aware total.

Guardrails and operator control

  • An exact count's tail (past the requested page) is bounded by a row cap and a wall-clock budget; on exhaustion the total is reported unavailable (items x-y/*) rather than truncating the page.
  • Per-mount config rest: { exactCount: false } serves count=exact as an estimate instead (default enabled) for sensitive/public mounts.

Review findings addressed

Cross-model (Codex) and Harper-domain review surfaced and fixed: read-transaction release on the count path (finally), guardrail no longer truncating the page (+ time budget), estimate corruption by the planner's synthetic sort condition, filter-aware estimates (unknown total instead of a misleading / cardinality-disclosing one), a spurious-header guard for single-record responses, valid-range clamping, and CORS append-not-overwrite. A flagged read-buffer-aliasing concern was verified a non-issue (records return owned copies) and is guarded by a regression test.

Testing

  • Resources unit (unitTests/resources/queryCount*.test.js): exact/estimated/window/filtered/default-streaming, plus a Bytes read-buffer-safety guard on both storage engines. Adds primary-key and secondary-index range count=estimated cases proving the total is range-aware (a narrow tail estimates fewer than the whole table/index) via the Use storage-level statistical range estimates in the query planner #2163 estimator.
  • REST integration (integrationTests/apiTests/rest.test.mjs): Content-Range/Range-Unit/Preference-Applied, offset window, filtered, estimated, unavailable-total (/*), opt-in, HEAD, and the exactCount gate (its own instance).
  • Full resources unit suite green (1882 passing) on the merge with main; tsc and lint clean.

Docs

REST reference docs (Pagination and Total Count, the exactCount option, the Prefer header): HarperFast/documentation#623.

🤖 Generated with Claude Code

cb1kenobi and others added 5 commits August 10, 2026 10:14
Adds opt-in total-record-count for REST collection queries so a client can paginate
("1–25 of 1,234") without a second round-trip or a custom resource.

- `Prefer: count=exact` — Table.search drains the full matched set once, windowing the
  requested page in the same pass (O(matched) filter evals, O(limit) memory), bounded by
  MAX_EXACT_COUNT_SCAN so a page fetch can't turn into an unbounded scan.
- `Prefer: count=estimated` — returns just the page plus a cheap planner/table estimate
  (estimateCondition / estimatedEntryCount, now exported), no full scan.
- No default: without the header nothing is computed and no header is emitted.
- REST emits `Content-Range: items <start>-<end>/<total>` (200, not 206), `Range-Unit:
  items`, and `Preference-Applied: count=exact|estimated|none`, and adds them to
  `Access-Control-Expose-Headers` so browser (CORS) clients can read them. HEAD returns
  the headers with no body — a cheap "how many match?" pre-flight.

Tests: resources-level unit (exact/estimated/window/filtered/default streaming) and REST
integration (Content-Range/Range-Unit/Preference-Applied/CORS/HEAD/opt-in).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Cross-model review (Codex) of the count feature surfaced several correctness,
resource, and disclosure issues, all fixed here:

- Read-txn leak: the count drain now releases the read transaction in a `finally`,
  so a throw mid-iteration (record load, rowFilter policy error) can't leak a pinned
  snapshot.
- Guardrail no longer truncates the page: the requested [offset, end) window is
  always collected in full; the row cap only abandons the running total. Added a
  wall-clock budget (MAX_EXACT_COUNT_MS) alongside the row cap so an exact count of a
  large match set can't run unbounded — on exhaustion the total is reported unknown
  (Content-Range .../*), never a short page.
- Estimated totals no longer corrupted by the planner's synthetic `sort`
  pseudo-condition: hasUserConditions now reads the raw request conditions, and the
  estimate drops `sort` pseudo-conditions. A clamp keeps a non-empty page's
  Content-Range valid when an estimate undershoots (exact totals stay authoritative).
- Estimated totals return unknown (null -> .../*) when an opaque rowFilter/vectorFilter
  participates, instead of a misleading estimate that could disclose hidden cardinality.
- Spurious headers: the REST gate now requires an array result, so a single-record GET
  whose record carries a `recordCount` attribute can't be mistaken for a count page.
- CORS: Access-Control-Expose-Headers is appended (not overwritten), preserving a
  resource's own exposed headers.

Adds regression tests for the sorted-estimate, filter-aware estimate, and
filtered-exact paths. Resources unit 8 passing; REST integration 21 passing.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds an operator control for the expensive exact-count scan: `rest: { exactCount:
false }` on a REST mount serves a `Prefer: count=exact` request as a cheap estimate
instead (signaled back via `Preference-Applied: count=estimated`), rather than
rejecting it. Default enabled. Read from httpOptions in the same per-mount way as the
existing `includeExpensiveRecordCountEstimates` option.

This is the operator-facing half of the DoS mitigation for exact counts: the in-code
guardrails (row cap + time budget) bound a single request, and this lets a deployment
turn exact counts off entirely on a sensitive/public mount. It is a per-REST-mount
policy — components exporting at the shared root path share one mount's options.

Integration: a dedicated suite (its own instance, since a gated component would
otherwise share the root mount with the main suite) verifies count=exact downgrades to
estimated while count=estimated is unchanged. 23 REST integration tests passing.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A code-review concern held that the count path releases its read transaction before
the page is serialized, so a Bytes/Blob field decoded as a zero-copy view of the read
buffer could be corrupted by later reads/writes. Verified it does NOT occur: the count
drain reads every record eagerly while the txn is open and returns owned copies
(Bytes come back as standalone Buffers, byteOffset 0), so releasing before serialize is
safe — unlike the streaming path, which reads lazily during serialization and must hold
the txn. This test churns writes/reads after an exact count and asserts the returned
Bytes are unchanged, on both storage engines.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…actCount gate

Addresses two review findings:

- #2: Preference-Applied now echoes the count mode the server applied (exact|estimated,
  after any per-mount downgrade) instead of `count=none` when the total is unavailable.
  A `Content-Range: items x-y/*` now reads as "that mode was applied but the total is
  unavailable" (guardrail hit, or an estimate suppressed by an opaque filter / Infinity
  estimate) rather than "no count was requested". Added an integration case: a `ne`
  condition (Infinity estimate) yields items 0-.../* with count=estimated.

- #4: the exactCount disable check also accepts the string "false", since not every
  config source coerces to a boolean.

24 REST integration tests passing.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for REST pagination total-count using the Prefer: count=exact|estimated header. It updates Table.search to return materialized pages with exact or estimated record counts, incorporating guardrails to prevent unbounded scans. The REST layer is updated to parse the preference, handle configuration-based downgrades, and emit RFC 7233-style headers (Content-Range, Range-Unit, and Preference-Applied). Additionally, new unit and integration tests are added to verify the functionality and ensure read-buffer safety. There are no review comments, and I have no feedback to provide.

Comment thread resources/Table.ts Outdated
@claude

claude Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Reviewed; no blockers found.

Comment thread resources/Table.ts Outdated
cb1kenobi and others added 3 commits August 11, 2026 18:26
Review (claude[bot] on #2147) found the exact-count guardrail (row cap + time
budget) and the estimated early-exit only applied when the request included a
limit(): both live inside `if (end !== undefined ...)`. A count=exact/estimated
request with no limit() therefore drained AND materialized the entire matched set
with no cap — the exact unbounded-scan/-memory DoS the guardrail was built to
prevent, on the most likely-hit path (a bare collection GET), and it bypassed the
exactCount gate too.

Counting is a pagination feature, so it now requires a limit(): a count request
without one falls through to the normal streaming path (no count emitted), which
keeps the guardrail always applied to a bounded page. Updated the unit test that
documented the no-limit drain as intentional, and added a test asserting a no-limit
count streams (does not materialize).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Base automatically changed from fix/sql-engine-top-limit-normalization to main August 12, 2026 22:47

@kriszyp kriszyp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a really cool idea, I like the interface.
However, I agree with the comments; I think we should probably have exactCount disabled by default, it could pretty expensive. I also think this really increases the need for HarperFast/rocksdb-js#311 or some better solution for estimating range counts (maybe should be a prerequisite for this?)
And do we have an issue for this? I'd like to get priorities associated with these (especially if this starts entraining dependency PRs).
🤖 Reviewed with Codex

Comment thread resources/Table.ts Outdated
Comment thread server/REST.ts Outdated
Comment thread resources/Table.ts
Comment thread server/REST.ts
Comment thread server/REST.ts Outdated
… only, Vary/CORS

Addresses kriszyp's review on #2147:

- Bound the count page: the count path now requires a finite, non-negative integer
  limit no larger than MAX_COUNT_PAGE (10k). limit(Infinity), limit(foo)->NaN, a
  negative, or an oversized limit fall through to streaming with no count, so a
  count request can't be coerced into materializing an unbounded page.
- Exact counting is now opt-in per mount (`rest: { exactCount: true }`, default
  off); count=exact is otherwise served as an estimate. Estimated stays the safe
  default, removing the default worker-saturation surface on public tables.
- Only honor Prefer: count on GET/HEAD. It was set for every method, so a
  collection DELETE carrying limit()+Prefer received a materialized array from
  search() (declared AsyncIterable) and threw instead of deleting.
- Emit `Vary: Prefer` on collection reads (after serialize, which resets Vary) so a
  shared cache can't serve count headers to a request that didn't ask, or a cached
  non-count response to one that did.
- Compare Access-Control-Expose-Headers as case-insensitive comma tokens, not
  substrings, so an unrelated existing token (e.g. X-Content-Range-Metadata) no
  longer suppresses the real Content-Range token.

Tests: unit 11 passing (added invalid/oversized-limit fall-through); integration 27
passing (oversized-limit fall-through, Vary: Prefer, DELETE-not-misrouted, and the
new opt-in default via exactCount: true / default-off suites).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@cb1kenobi

Copy link
Copy Markdown
Member Author

Thanks @kriszyp — really helpful review, all five points were spot on. Addressed in 5ec0d38:

Design

  • Exact counting is now opt-in (rest: { exactCount: true }, default off); count=exact is otherwise served as an estimate. Estimated stays the safe default, so a public table no longer exposes the exact-scan budget by default.
  • Tracking issue: REST pagination total count (Prefer: count=) #2162 — links rocksdb-js#311 as the range-estimation dependency, and flags there the "should better estimation be a prerequisite for broader rollout?" question for prioritization.

Fixes

  1. Unbounded/invalid limit — the count path now requires a finite, non-negative integer limit no larger than a max page size (10k); limit(Infinity), limit(foo)→NaN, negative, or oversized limits fall through to streaming with no count.
  2. search() contract / DELETEPrefer: count is now GET/HEAD-only, so a collection DELETE carrying limit()+Prefer keeps the AsyncIterable and deletes instead of receiving a Promise and throwing.
  3. Vary: Prefer — emitted on collection reads after serialization (which resets Vary), so shared caches key on Prefer.
  4. CORS token comparison — expose-header dedup now compares case-insensitive comma tokens instead of substrings, so X-Content-Range-Metadata no longer suppresses Content-Range.

Docs updated in HarperFast/documentation#623. Tests: unit 11, integration 27 green. Ready for another look when you have a moment.

— Claude Opus 4.8

@kriszyp

kriszyp commented Aug 13, 2026

Copy link
Copy Markdown
Member

Heads-up for the estimation side of this PR: rocksdb-js #778 — feat: statistical range key-count estimation adds storage-level range estimates that this PR should use once it lands:

  • db.estimateCount({ start, end }){ count, confidence } — a no-iteration statistical range count (memtable stats + range-local SST entry density), measured within ~±5% at 9–83µs where exact scans took 1–85ms. This can replace the planner-fraction path behind count=estimated (estimateCondition's hardcoded 5%/10%/30% guesses for range comparators) with a real per-range number, and confidence gives the REST layer a principled basis for deciding when an estimate is trustworthy enough to report.
  • db.createCountEstimator(range) — rides the iteration this PR already does: advance(lastKey, n) per page, estimate() = exact-so-far + calibrated remainder, finish() = exact. This composes nicely with the count=exact guardrail here: when the row-cap/time budget exhausts mid-drain, the partial drain can become a calibrated estimate (with confidence) instead of reporting the total unavailable (items x-y/*).

Harper-side integration is coming as a follow-up PR (planner estimateCondition + estimatedEntryCount, feature-detected so it degrades cleanly on older rocksdb-js). Nothing blocking here — this PR's shape is already right for it.

— Claude (Fable 5)

@kriszyp kriszyp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will post the queued inline comment, but more importantly this probably should switch to use iterator estimator in 778, as noted in the other comment.
🤖 Reviewed with Codex

Comment thread resources/Table.ts Outdated
cb1kenobi and others added 2 commits August 13, 2026 17:24
Follow-up to kriszyp's review on #2147: the bounded-page check validated the limit
but accepted any offset. A negative offset (limit(-5,10)) diverged from the normal
slice path, and an arbitrarily large offset postponed the exact-count guardrails
(which engage only past the page window) until that offset had been scanned.

The count path now also requires the offset to be a finite, non-negative integer and
the window (offset + limit) to be within MAX_EXACT_COUNT_SCAN; anything else (a
negative offset, or a deep-page window past the scan budget) falls through to
streaming with no count. Adds unit + integration coverage for negative and
oversized-window offsets.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
kriszyp added a commit that referenced this pull request Aug 15, 2026
Range comparators (starts_with/prefix, between and friends, lt/le/gt/ge)
have always estimated as fixed fractions of the table size (5%/10%/30%),
which makes condition ordering — and #2147 count=estimated totals —
wildly wrong for any real range. When the store provides rocksdb-js
estimateCount ({ count, confidence }), estimateCondition now estimates
the actual range the search would iterate (mirroring searchByIndex range
construction), blended with the old fraction heuristic by the estimate
confidence, so low-confidence estimates (block-granular tiny ranges,
open-ended complement subtraction) degrade gracefully to the previous
behavior. RocksIndexStore translates value-space bounds to its composite
[indexedValue, primaryKey] keys ([value, MAXIMUM_KEY]), mirroring its
getRange. estimatedEntryCount switches from an exact full-store
getKeysCount scan (every 10s per store) to the O(1) estimate-num-keys
read.

The capability is feature-detected (typeof store.estimateCount), so
behavior is unchanged until the rocksdb-js dependency ships
HarperFast/rocksdb-js#778; the end-to-end tests self-skip until then.

@kriszyp kriszyp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we just need to publish a new rocksdb-js version to unblock this so you can switch to the newer estimation APIs.
🤖 Reviewed with Codex

Comment thread resources/Table.ts
kriszyp added a commit that referenced this pull request Aug 26, 2026
Range comparators (starts_with/prefix, between and friends, lt/le/gt/ge)
have always estimated as fixed fractions of the table size (5%/10%/30%),
which makes condition ordering — and #2147 count=estimated totals —
wildly wrong for any real range. When the store provides rocksdb-js
estimateCount ({ count, confidence }), estimateCondition now estimates
the actual range the search would iterate (mirroring searchByIndex range
construction), blended with the old fraction heuristic by the estimate
confidence, so low-confidence estimates (block-granular tiny ranges,
open-ended complement subtraction) degrade gracefully to the previous
behavior. RocksIndexStore translates value-space bounds to its composite
[indexedValue, primaryKey] keys ([value, MAXIMUM_KEY]), mirroring its
getRange. estimatedEntryCount switches from an exact full-store
getKeysCount scan (every 10s per store) to the O(1) estimate-num-keys
read.

The capability is feature-detected (typeof store.estimateCount), so
behavior is unchanged until the rocksdb-js dependency ships
HarperFast/rocksdb-js#778; the end-to-end tests self-skip until then.
kriszyp added a commit that referenced this pull request Aug 27, 2026
Range comparators (starts_with/prefix, between and friends, lt/le/gt/ge)
have always estimated as fixed fractions of the table size (5%/10%/30%),
which makes condition ordering — and #2147 count=estimated totals —
wildly wrong for any real range. When the store provides rocksdb-js
estimateCount ({ count, confidence }), estimateCondition now estimates
the actual range the search would iterate (mirroring searchByIndex range
construction), blended with the old fraction heuristic by the estimate
confidence, so low-confidence estimates (block-granular tiny ranges,
open-ended complement subtraction) degrade gracefully to the previous
behavior. RocksIndexStore translates value-space bounds to its composite
[indexedValue, primaryKey] keys ([value, MAXIMUM_KEY]), mirroring its
getRange. estimatedEntryCount switches from an exact full-store
getKeysCount scan (every 10s per store) to the O(1) estimate-num-keys
read.

The capability is feature-detected (typeof store.estimateCount), so
behavior is unchanged until the rocksdb-js dependency ships
HarperFast/rocksdb-js#778; the end-to-end tests self-skip until then.
kriszyp added a commit that referenced this pull request Aug 27, 2026
Range comparators (starts_with/prefix, between and friends, lt/le/gt/ge)
have always estimated as fixed fractions of the table size (5%/10%/30%),
which makes condition ordering — and #2147 count=estimated totals —
wildly wrong for any real range. When the store provides rocksdb-js
estimateCount ({ count, confidence }), estimateCondition now estimates
the actual range the search would iterate (mirroring searchByIndex range
construction), blended with the old fraction heuristic by the estimate
confidence, so low-confidence estimates (block-granular tiny ranges,
open-ended complement subtraction) degrade gracefully to the previous
behavior. RocksIndexStore translates value-space bounds to its composite
[indexedValue, primaryKey] keys ([value, MAXIMUM_KEY]), mirroring its
getRange. estimatedEntryCount switches from an exact full-store
getKeysCount scan (every 10s per store) to the O(1) estimate-num-keys
read.

The capability is feature-detected (typeof store.estimateCount), so
behavior is unchanged until the rocksdb-js dependency ships
HarperFast/rocksdb-js#778; the end-to-end tests self-skip until then.
Comment thread resources/Table.ts

@kriszyp kriszyp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still not using the estimation functionality in rocksdb 2.8.
🤖 Reviewed with Codex

Comment thread resources/Table.ts Outdated
Comment thread resources/Table.ts Outdated
Bumps @harperfast/rocksdb-js to 2.8.0 and wires its new statistical range
key-count estimator (`estimateCount`) into the query planner's range estimates,
which the `Prefer: count=estimated` pagination path reads.

Range comparators (between, starts_with, greater/less, open ranges) previously
returned an arbitrary fixed fraction of the table size, because the storage
layer could not estimate a range's cardinality. `estimateCondition` now asks
the engine for a real range estimate on RocksDB, and only falls back to the old
heuristic when unavailable (LMDB engine, non-indexed / custom-indexed attribute,
an unbounded/degenerate range, or a zero-confidence — failed-statistics — read).
Equals still uses the exact per-value index count; this only replaces the range
guesses. Because the REST estimated-count path funnels through
`estimateCondition`, `count=estimated` now reports a range-aware total, and the
planner picks indexes for range queries from real selectivity rather than a
constant.

`RocksIndexStore.estimateCount` overrides the inherited estimator to apply the
same `[indexedValue, primaryKey]` composite-key rewrite as its `getRange`, so a
secondary-index range estimate covers exactly the keys the scan would visit
(otherwise an inclusive end / exclusive start would miss the value's bucket).

Tests: resources unit adds primary-key and secondary-index range-estimate cases
proving the total tracks range width (a narrow tail estimates fewer than the
whole table/index) where the old range-blind heuristic would tie. Existing
count, planner, and search suites green (1803 resources tests passing); tsc and
lint clean. Behavior on LMDB is unchanged (falls back to the prior heuristic).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@kriszyp kriszyp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you rebase/stack this onto #2163, which owns the implementation of using estimation for query planning, deleting rangeBoundsForEstimate / estimateRangeCount / the RocksIndexStore override here, along with the dep bump. #2147 then keeps what's genuinely its own — Prefer: count=, the guardrails, the Content-Range headers, the page materialization — and the estimateCondition() call inherits the corrected estimates for free.

rocksdb-js 2.8.0 also ships CountEstimator (db.createCountEstimator()), which progressively refines a range estimate from entries already iterated. The count=estimated path is a natural fit — it walks offset + limit entries of the range and then discards that knowledge for a pure statistical estimate. Feeding it advance(lastKey, n) per page makes the traversed portion exact and calibrates the remainder, and finish() on natural exhaustion returns confidence: 1 — an exact total for free whenever the match set fits inside the guardrail, which is most paginated queries. That would also subsume a good chunk of the count=exact machinery. We could potentially do this as a follow-up, but I think it makes more sense here.

🤖 Reviewed with Codex

Comment thread resources/Table.ts Outdated
Comment thread resources/search.ts Outdated
kriszyp added a commit that referenced this pull request Aug 31, 2026
…2163)

* Use storage-level statistical range estimates in the query planner

Range comparators (starts_with/prefix, between and friends, lt/le/gt/ge)
have always estimated as fixed fractions of the table size (5%/10%/30%),
which makes condition ordering — and #2147 count=estimated totals —
wildly wrong for any real range. When the store provides rocksdb-js
estimateCount ({ count, confidence }), estimateCondition now estimates
the actual range the search would iterate (mirroring searchByIndex range
construction), blended with the old fraction heuristic by the estimate
confidence, so low-confidence estimates (block-granular tiny ranges,
open-ended complement subtraction) degrade gracefully to the previous
behavior. RocksIndexStore translates value-space bounds to its composite
[indexedValue, primaryKey] keys ([value, MAXIMUM_KEY]), mirroring its
getRange. estimatedEntryCount switches from an exact full-store
getKeysCount scan (every 10s per store) to the O(1) estimate-num-keys
read.

The capability is feature-detected (typeof store.estimateCount), so
behavior is unchanged until the rocksdb-js dependency ships
HarperFast/rocksdb-js#778; the end-to-end tests self-skip until then.

* Address cross-model review: negation inversion, dependency-shape guards

- negated conditions now estimate the complement of their positive
  estimate (root fix in estimateConditionForTable — also covers the
  pre-existing negated-equals defect): a narrow negated range previously
  looked highly selective, won the condition ordering, and executed as a
  full scan
- the estimate path validates {count, confidence} shape and wraps the
  native call in try/catch, so a future dependency bump (or a
  concurrently closing store) degrades to the fraction heuristic instead
  of NaN-poisoning plan ordering or failing the request
- over-length string bounds fall back (execution truncates at
  MAX_SEARCH_KEY_LENGTH + filters, so the executed range is wider than
  the estimable one)
- intersectionEstimate divisor floored at 1
- comment trims per review

* Document query-plan range estimation invariants in DESIGN.md

* Negated conditions estimate Infinity, matching the full-scan convention

The complement inversion still let a wide positive range make its
negation look cheap while executing as a full scan. estimated_count
feeds driving-condition ordering, and the codebase already encodes
full-scan cost as Infinity for the filter-only comparators
(contains/ends_with) — negated conditions (which always force
needFullScan) now follow the same convention.

* Bump rocksdb-js to 2.8.0 and use the shipped estimateCount API

rocksdb-js 2.8.0 ships the statistical range estimation the query planner was
written against, so the RocksIndexStore override becomes a real typed method
mirroring getRange (reverse flip included) instead of a conditional prototype
assignment, and the real-store estimate tests run unconditionally.

The per-store capability probe and result-shape validation stay: LMDB-backed
and custom index stores don't implement estimateCount.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Gate the real-store estimate suite on the live store, not the Rocks prototype

test:unit:lmdb re-runs test:unit:resources with HARPER_STORAGE_ENGINE=lmdb,
where the index stores have no estimateCount at all. Keying the skip off
RocksDatabase.prototype was already wrong and became always-true once 2.8.0
was pinned, so gate on the table's actual index store instead.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Estimate lt/le over the range execution actually iterates

searchByIndex bounds lt/le at `start: true`, which sorts above `null`, so an
indexNulls index's `[null, primaryKey]` entries are outside the executed range.
estimateRangeCondition left the lower bound open and counted them: on an index
that is 99% nulls the estimate came back 21x high (19960 for a 200-row
condition), so a genuinely selective condition looked like a full scan and lost
the driving-condition ordering — worse than the flat heuristic it replaces.

getRange and estimateCount now share one translateIndexBounds helper so the
"estimate the range you execute" invariant is structural instead of two copies,
and a spy test pins that both forward identical bounds, reverse branch included.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Pin estimates against executed result counts, not just against each other

The real-store suite compared estimates to one another (narrow < wide), which
passes even if every estimate is an order of magnitude off. Three cases now run
the actual query and assert the estimate lands within 10x of what it returned,
covering the primary-key path as well — that path passes `start: true` through
to the primary store, and nothing pinned it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Align root msgpackr pin with rocksdb-js 2.8.0's dependency

rocksdb-js 2.8.0 depends on msgpackr@2.0.6; the root package.json still
pinned 2.0.5, so npm couldn't dedupe to a single msgpackr instance and
the shrinkwrap-pin smoke check failed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Fix DESIGN.md formatting after rebase conflict resolution

Prettier wants blank lines around the two independently-appended
sections merged during the rebase onto main.

* Short-circuit negated range conditions before the native estimate call

condition.negated forced estimated_count to Infinity at the end of
estimateConditionForTable, but every branch above it (including the
native estimateCount FFI call for range comparators) still ran first
and had its result discarded. Hoist the negated check to skip that
work entirely — same outcome, no wasted native call.

Found by the pre-push independent review (codex, graded leg) on this
rebase.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Reject negative counts in the estimateCount result-shape validation

estimateRangeCondition checked Number.isFinite(count) and the
confidence bound but not count's sign, so a store returning a
negative count (a malformed/buggy native response) would pass
validation and blend a negative number into the estimate.

Found by the pre-push independent review (codex, graded leg) on this
rebase.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Reject non-numeric confidence in the estimateCount result-shape validation

The malformed-shape guard rejected NaN/negative count but let a
non-numeric-but-comparable confidence (e.g. NaN via >= coercion quirks,
or an out-of-range non-finite value) slip through the >=/<= comparison.
Mirror the count check: require Number.isFinite before bounds-checking.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
# Conflicts:
#	resources/RocksIndexStore.ts
#	resources/search.ts
Comment thread resources/Table.ts Outdated
A `Prefer: count=exact` request sorted by a vector/HNSW attribute (or shaped by
a `vectorFilter`) drained `scanned` rows and reported that as the exact total.
An HNSW traversal returns a bounded, approximate candidate set whose size is
chosen from `minResults` (offset + limit), so `scanned` tracks the requested
page size, not the true match count — the same query at limit(5) vs limit(200)
could advertise two different `count=exact` totals.

The count path now detects an approximate (vector-sorted or vector-filtered)
result set and reports the total as unavailable (`recordCount` null,
`recordCountExact` false → `Content-Range: items x-y/*`) instead of a
page-size-dependent number, mirroring how the estimated branch already bails to
null for an opaque row/vector filter. Pages still materialize normally; only the
untrustworthy total is withheld.

Regression test (`queryCountVector.test.js`): the same cosine-sorted query at
limit(5) and limit(40) must report the total unavailable at both, not two
different exact numbers. Full resources suite green (1883).

Addresses the standing review blocker raised across rounds (2026-08-20..31).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@cb1kenobi

Copy link
Copy Markdown
Member Author

@kriszyp — done, and thanks for the detailed steer through this. Summary of where things landed:

Rebased onto #2163. This branch is merged onto main, which now carries #2163 (the query-planner estimation work). I deleted rangeBoundsForEstimate / estimateRangeCount / the RocksIndexStore.estimateCount override and the rocksdb-js dep bump from this PR, so #2147 keeps only what's genuinely its own — Prefer: count=, the guardrails, the Content-Range headers, and page materialization — and estimateCondition() inherits #2163's storage-level range estimates for count=estimated. Net diff vs main in search.ts is now a single line (re-exporting estimatedEntryCount, which the count path imports).

HNSW/vector count=exact blocker fixed (9e44500). An approximate result set — a vector/HNSW-driven sort or a vectorFilter — now reports its total as unavailable (items x-y/*) rather than advertising a page-size-dependent scanned as exact. Regression test runs the same cosine-sorted query at two page sizes and asserts the total is unavailable at both.

The remaining inline threads (GET/HEAD-only preference, Vary: Prefer, CORS token comparison, limit(Infinity)/offset guards, exact-count opt-in) were addressed in earlier commits — I replied inline on each with the specifics and resolved them.

Full resources unit suite is green (1883). Ready for another look when you have a moment!

🤖 Drafted with Claude Code (Opus 4.8)

Comment thread resources/Table.ts Outdated
cb1kenobi and others added 2 commits August 31, 2026 10:46
…e count

Broadens the approximate-result detection from the previous fix: an HNSW `lt`/`le`
threshold filter drives the same bounded, minResults-widened traversal as a vector
sort (HierarchicalNavigableSmallWorld handles `lt`/`le` in the same switch as
`sort`), and can be the driving condition with no `sort` clause at all — so
`count=exact` over it advertised a page-size-dependent `scanned` as authoritative,
the same defect the sort fix addressed, reached through a sibling comparator.

Detection now walks the executing `conditions` (recursively, through OR groups) for
any attribute backed by a custom index, plus the `vectorFilter` check. This is both
broader (catches the threshold-filter path) and more precise than the sort-chain
walk: a vector sort applied as in-memory post-ordering leaves no custom-index
condition in `conditions`, so it correctly stays exact rather than being over-flagged.

Regression test adds the `lt` threshold-filter case (no sort) at two page sizes;
verified it reports `scanned` as exact under the old sort-only detection and
unavailable under this one.

Addresses the follow-up review blocker on the HNSW fix.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@cb1kenobi
cb1kenobi requested a review from kriszyp August 31, 2026 16:18
…harden Prefer parse

Cross-model review (Gemini + Cursor-Grok) findings on the count path:

- The exact-count drain iterated the store's async iterator with no macrotask yield.
  On a store whose iterator settles synchronously (the common indexed-scan case)
  a large `count=exact` scan ran as one uninterrupted microtask burst, blocking the
  event loop for up to the whole MAX_EXACT_COUNT_MS budget and starving concurrent
  requests. Yield to the macrotask queue every COUNT_YIELD_INTERVAL rows so I/O and
  other requests keep progressing — covering the page-window scan too, not just the
  tail past it.

- For an approximate (vector/HNSW) result set, `count=exact` now stops at the page
  window instead of draining the tail: the total is reported unavailable anyway, so
  the tail work produced a number that was never published.

- REST Prefer parse hardened against malformed input: optional-chain `httpOptions`
  (a programmatic mount may pass none) and String()-coerce the preference value
  before lower-casing (a bare `Prefer: count` with no `=` yields a non-string).

Not changed — an unadjudicated Cursor-Grok "async allowRead breaks count" blocker
was investigated and did NOT reproduce: driving `get()` with an async allowRead and
`Prefer: count=` returns a proper page array (recordCount intact, iterates cleanly);
the Table-level async-authorization branch isn't reached for the count path (auth
resolves at the Resource layer first). No fix shipped for a non-issue.

The 8 pre-existing resources-suite failures (transaction-log/snapshot/audit/reload)
reproduce identically on a clean tree and are unrelated to this change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@cb1kenobi

Copy link
Copy Markdown
Member Author

Cross-model review (Gemini 3.1 Pro + Cursor-Grok)

Ran the pre-push cross-model review on d7d4356b6. The Codex graded leg timed out on a local model-manager error, which starved the domain adjudicator, so findings below are author-triaged, not machine-adjudicated. Independent outside-family coverage still came from Gemini and Cursor-Grok. Verified each against the code before acting.

Fixed (pushed in d7d4356b6)

  • Event-loop starvation on a large count=exact scan (Gemini major / Grok significant). The drain iterated the store's async iterator with no macrotask yield; on a synchronously-settling iterator (the common indexed-scan case) a big scan ran as one microtask burst, blocking the loop for up to the whole MAX_EXACT_COUNT_MS. It now yields to the macrotask queue every COUNT_YIELD_INTERVAL rows — covering the page-window scan too, not just the tail.
  • Approximate exact-count wasted the tail (Grok suggestion). For a vector/HNSW result set, count=exact now stops at the page window instead of draining a tail whose total is reported unavailable anyway.
  • Prefer-parse hardening (Gemini minors). Optional-chain httpOptions (a programmatic mount may pass none) and String()-coerce the preference value before lower-casing (a bare Prefer: count with no = can yield a non-string).

Investigated — not a bug

  • "async allowRead breaks the count path → 500" (Grok blocker). Did not reproduce: driving get() (what REST calls) with an async allowRead and Prefer: count= returns a proper page array — recordCount intact, iterates cleanly. The Table-level async-authorization branch isn't reached on the count path (authorization resolves at the Resource layer first). No fix shipped for a non-issue.

Surfaced for a maintainer call (not changed)

These are real but touch product/security defaults, so I'd rather you decide than change them unilaterally:

  • CORS request admission (Grok significant): the default Access-Control-Allow-Headers (Accept, Content-Type, Authorization) omits Prefer, so a cross-origin browser can't send Prefer — preflight blocks the request before the new expose-headers path matters. The feature's cross-origin story is only half-wired without it. Options: add Prefer to the default allow-list (small, safe — Prefer is a standard header), or document that operators must. Happy to implement either.
  • Conditional GET + count (Grok significant): a matching If-None-Match returns 304 and clears the body after the count scan already ran, so Content-Range/Preference-Applied are dropped and the scan was wasted. Edge (needs a prior ETag), but worth a decision: skip the scan on a 304, or emit the count headers on it.
  • Quoted preference token (Grok suggestion): Prefer: count="exact" (RFC-quoted) is silently ignored. Cheap to accept if we want it.
  • Minor (Gemini): setCountHeaders allocates a Set/array per count response (opt-in path, low impact); and the new comments run heavier than Harper's usual density — can trim on request.

Full resources unit suite: 1876 passing; the 8 failures (transaction-log / snapshot / audit / table-reload) reproduce identically on a clean tree — pre-existing and unrelated to this change.

🤖 Drafted with Claude Code (Opus 4.8)

@kriszyp kriszyp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, great work!
🤖 Reviewed with Codex

Comment thread resources/Table.ts Outdated
// authoritative total: the same query at limit(5) vs limit(200) would otherwise advertise two
// different `count=exact` totals. Report the total as unavailable instead (mirroring how the
// estimated branch below already bails to null for a vector/row filter).
let approximateResultSet = typeof target.vectorFilter === 'function';

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approximateResultSet only examines target.vectorFilter and the sort chain, but HNSW is also directly queryable through lt/le conditions. For example, conditions: [{ attribute: 'vector', comparator: 'lt', value: threshold, target }] has neither marker, so this remains false even though executeConditions() passes the page-derived minResults into the same bounded HNSW candidate search.

When that candidate iterator exhausts, exact remains true and line 3787 still publishes scanned as an authoritative total. Please include HNSW-backed planned conditions in the approximate-result classification and add a threshold-condition regression alongside the vector-sort test.

— KrAIs (GPT-5)

@kriszyp
kriszyp merged commit 2e65550 into main Sep 1, 2026
46 of 48 checks passed
@kriszyp
kriszyp deleted the feat/rest-pagination-total-count branch September 1, 2026 05:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants