feat: improve indexes - #20
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
✅ Deploy Preview for hyperdb ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Review limit reached
Next review available in: 30 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThe change validates index definitions, introduces shared persistent physical indexes, replaces textual SQLite sort keys with binary keys, and updates SQLite and IndexedDB storage, scans, migrations, tests, and documentation. ChangesPersistent index storage
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Schema
participant PhysicalIndexResolver
participant SQLiteDriver
participant SQLiteStorage
Schema->>PhysicalIndexResolver: validate and resolve index definitions
PhysicalIndexResolver-->>SQLiteDriver: persistent physical index metadata
SQLiteDriver->>SQLiteStorage: store or migrate binary BLOB sort keys
SQLiteDriver->>SQLiteStorage: execute range or primary-key IN query
SQLiteStorage-->>SQLiteDriver: matching records
sequenceDiagram
participant Schema
participant PhysicalIndexResolver
participant IndexedDBDriver
participant IndexedDBObjectStore
Schema->>PhysicalIndexResolver: validate and resolve index definitions
PhysicalIndexResolver-->>IndexedDBDriver: persistent physical index names
IndexedDBDriver->>IndexedDBObjectStore: scan native index or fetch primary-key matches
IndexedDBObjectStore-->>IndexedDBDriver: ordered records
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/hyperdb/src/hyperdb/drivers/idb/idb-driver.browser.test.ts (1)
162-162: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert the encoded bytes, not only the container type.
Line 162 passes for an empty or incorrect
ArrayBuffer. Compare the stored bytes with the expectedbyCountsort key, or decode the key and assert its value. This keeps the test covering the persisted binary-index contract.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/hyperdb/src/hyperdb/drivers/idb/idb-driver.browser.test.ts` at line 162, Strengthen the assertion in the relevant IndexedDB persistence test around stored.indexes.byCount so it verifies the encoded bytes or decodes and compares the key to the expected byCount sort key, rather than only checking ArrayBuffer type. Preserve the existing persistence assertions while ensuring an empty or incorrect buffer fails the test.packages/hyperdb/src/hyperdb/drivers/sqlite/sqlite-sort-key.ts (1)
134-154: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe variable-length integer encoding preserves order. Add the bounds to the comment.
The lead-byte ranges are disjoint and increasing across lengths:
0x01–0x7f,0xc2–0xdf,0xe0–0xef, and0xf0. Continuation bytes carry six bits each in big-endian order, so the encoding is bytewise monotone. The 4-byte branch carries 22 bits and therefore only supports values up to0x3fffff. Both callers stay inside that limit (charCodeAt(...) + 1reaches0x10000,byte + 1reaches0x100), but the function does not reject larger input. State the supported range in the comment so a future caller does not silently produce a wrong key.♻️ Proposed comment update
// Encodes a positive integer with the same bytewise order as its numeric -// value. Zero is reserved as a terminator, so callers pass values >= 1. +// value. Zero is reserved as a terminator, so callers pass values >= 1. +// The encoding supports values in the range 1..0x3fffff. function encodePositiveInteger(value: number): number[] {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/hyperdb/src/hyperdb/drivers/sqlite/sqlite-sort-key.ts` around lines 134 - 154, Update the comment above encodePositiveInteger to document that it supports positive values from 1 through 0x3fffff inclusive, with zero reserved as the terminator; note that callers must remain within this range.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/hyperdb/src/hyperdb/drivers/idb/idb-driver.ts`:
- Around line 895-924: Update the primary-key branch around
isPrimaryKeyBackedIndex to validate every equality condition like SQLite,
throwing “Primary-key index ${indexName} requires string IDs” when a condition
is not for id or its value is not a string; then collect unique ids before
issuing store.get requests so each record is returned once. Remove the discarded
createSortKeyRanges call, including any now-unused dependency, since this path
does not need to construct sort-key ranges.
In `@packages/hyperdb/src/hyperdb/drivers/sqlite/sql-driver.ts`:
- Around line 451-452: Update the generated-name parsing in
packages/hyperdb/src/hyperdb/drivers/sqlite/sql-driver.ts lines 451-452 and
packages/hyperdb/src/hyperdb/drivers/sqlite/async-sql-driver.ts lines 853-854 to
select and remove only one matching suffix, preferring the v2 suffix before the
legacy suffix, rather than chaining both replacements. Add regression tests in
both driver suites for logical index names ending in _sort_key, verifying
correct physical index lookup and scan ordering.
In `@packages/hyperdb/src/hyperdb/drivers/sqlite/sqlite-common.ts`:
- Around line 368-378: Update validateHashBounds to reject empty rawBounds and
any bound without gte before the primary-key handling in
buildSortKeyWhereClause. Perform this validation before the existing string-ID
check so no-condition hash/uniqhash selectors cannot produce an empty id list or
WHERE id IN ().
---
Nitpick comments:
In `@packages/hyperdb/src/hyperdb/drivers/idb/idb-driver.browser.test.ts`:
- Line 162: Strengthen the assertion in the relevant IndexedDB persistence test
around stored.indexes.byCount so it verifies the encoded bytes or decodes and
compares the key to the expected byCount sort key, rather than only checking
ArrayBuffer type. Preserve the existing persistence assertions while ensuring an
empty or incorrect buffer fails the test.
In `@packages/hyperdb/src/hyperdb/drivers/sqlite/sqlite-sort-key.ts`:
- Around line 134-154: Update the comment above encodePositiveInteger to
document that it supports positive values from 1 through 0x3fffff inclusive,
with zero reserved as the terminator; note that callers must remain within this
range.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3cfb6eeb-4c39-4337-a872-43aadf48f3d3
📒 Files selected for processing (16)
README.mdpackages/hyperdb-doc/src/content/docs/database/indexes.mdpackages/hyperdb-doc/src/content/docs/database/schemas.mdpackages/hyperdb-doc/src/content/docs/runtime/drivers.mdpackages/hyperdb-doc/src/content/docs/start/llm-cheat-sheet.mdpackages/hyperdb/src/hyperdb/drivers/idb/idb-driver.browser.test.tspackages/hyperdb/src/hyperdb/drivers/idb/idb-driver.tspackages/hyperdb/src/hyperdb/drivers/sqlite/async-sql-driver.tspackages/hyperdb/src/hyperdb/drivers/sqlite/driver-edge-cases.test.tspackages/hyperdb/src/hyperdb/drivers/sqlite/sql-driver.tspackages/hyperdb/src/hyperdb/drivers/sqlite/sqlite-common.tspackages/hyperdb/src/hyperdb/drivers/sqlite/sqlite-sort-key.test.tspackages/hyperdb/src/hyperdb/drivers/sqlite/sqlite-sort-key.tspackages/hyperdb/src/hyperdb/runtime/db.test.tspackages/hyperdb/src/hyperdb/schema/table.test.tspackages/hyperdb/src/hyperdb/schema/table.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/hyperdb-doc/src/content/docs/database/indexes.md (1)
139-142: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winDocument when
idis appended for non-unique indexes.
persistentPhysicalIndexesaddsidonly forbtreeindexes;uniqhashindexes include only the hash columns. The current wording makes it sound likeidis part of the non-unique key tuple foruniqhashtoo, which is not true.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/hyperdb-doc/src/content/docs/database/indexes.md` around lines 139 - 142, Update the index documentation to state that HyperDB appends id only to non-unique btree indexes that do not already end in id; clarify that uniqhash indexes contain only their hash columns. Preserve the explanation of deterministic ordering for btree indexes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/hyperdb-doc/src/content/docs/database/indexes.md`:
- Around line 139-142: Update the index documentation to state that HyperDB
appends id only to non-unique btree indexes that do not already end in id;
clarify that uniqhash indexes contain only their hash columns. Preserve the
explanation of deterministic ordering for btree indexes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: dbbd31ed-7723-4baf-8718-14b79b4d87ce
📒 Files selected for processing (12)
README.mdpackages/hyperdb-doc/src/content/docs/database/indexes.mdpackages/hyperdb-doc/src/content/docs/runtime/drivers.mdpackages/hyperdb-doc/src/content/docs/start/llm-cheat-sheet.mdpackages/hyperdb/src/hyperdb/drivers/idb/idb-driver.browser.test.tspackages/hyperdb/src/hyperdb/drivers/idb/idb-driver.tspackages/hyperdb/src/hyperdb/drivers/sqlite/async-sql-driver.test.tspackages/hyperdb/src/hyperdb/drivers/sqlite/async-sql-driver.tspackages/hyperdb/src/hyperdb/drivers/sqlite/driver-edge-cases.test.tspackages/hyperdb/src/hyperdb/drivers/sqlite/sql-driver.tspackages/hyperdb/src/hyperdb/drivers/sqlite/sqlite-common.tspackages/hyperdb/src/hyperdb/drivers/sqlite/sqlite-sort-key.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/hyperdb-doc/src/content/docs/start/llm-cheat-sheet.md
- packages/hyperdb/src/hyperdb/drivers/sqlite/sql-driver.ts
- README.md
- packages/hyperdb/src/hyperdb/drivers/sqlite/async-sql-driver.ts
- packages/hyperdb-doc/src/content/docs/runtime/drivers.md
- packages/hyperdb/src/hyperdb/drivers/idb/idb-driver.ts
- packages/hyperdb/src/hyperdb/drivers/sqlite/sqlite-common.ts
1 participant
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit
New Features
Bug Fixes
Documentation