Skip to content

Fix backwards-scan off-by-one in ReadableStorage#258

Merged
albe merged 3 commits into
mainfrom
copilot/create-pr-from-commit-5d9626d
Mar 21, 2026
Merged

Fix backwards-scan off-by-one in ReadableStorage#258
albe merged 3 commits into
mainfrom
copilot/create-pr-from-commit-5d9626d

Conversation

Copilot AI commented Mar 21, 2026

Copy link
Copy Markdown
Contributor

The backwards-scan loop in ReadableStorage used a strict > boundary, causing the final batch to be skipped whenever the range length was an exact multiple of the batch size (10). This meant reads like readRange(12, 1) would silently drop item 1.

Changes

  • src/Storage/ReadableStorage.js — Change while (batchUntil > readUntil)while (batchUntil >= readUntil) so the terminal batch is always included
  • test/Storage.spec.js — Add regression test: write 12 items, read in reverse, assert all 12 are returned in order
  • bench/package.json — Add bench-read-scenarios.js to the bench script
  • bench/bench-read-scenarios.js — New benchmark covering four realistic read scenarios (forward full scan, backwards full scan, join stream, range scan) on a store with two interleaved streams of 10 000 events (~512 bytes each). Both stores are opened once in readOnly mode; setup waits for the ready event before the suite starts.
// Before: readRange(12, 1) on 12 items silently dropped item 1
// batchUntil = 2 after first batch (items 3–12), then 2 > 1 → true → batch [1,2] read
// batchUntil = 1 after second batch, then 1 > 1 → false → exits, skipping nothing? 
// Actually: batchFrom = max(1, 2-10)=1, batchUntil = 1-1 = 0 → next check 0 > 1 false → misses item at position 1

// After: >= ensures batchUntil == readUntil still triggers one last batch
while (batchUntil >= readUntil) { ... }

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] Create PR based on commit 5d9626d Fix backwards-scan off-by-one in ReadableStorage Mar 21, 2026
Copilot AI requested a review from albe March 21, 2026 14:52
Copilot finished work on behalf of albe March 21, 2026 14:52
Comment thread bench/package.json
"version": "0.0.1",
"scripts": {
"bench": "node bench-index.js && node bench-storage.js && node bench-eventstore.js"
"bench": "node bench-index.js && node bench-storage.js && node bench-eventstore.js && node bench-read-scenarios.js"

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot the file for this benchmark suite is missing in the PR. Can you reproduce the benchmark suite from the other PR? Otherwise remove it from here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added bench/bench-read-scenarios.js in commit f4f8305. It covers the four scenarios described in the PR: forward full scan, backwards full scan, join stream (stream-1 + stream-2), and range scan (middle third). Both stores are pre-populated with 10 000 interleaved events (~512 bytes each) and opened once in readOnly mode before the suite runs.

@coveralls

coveralls commented Mar 21, 2026

Copy link
Copy Markdown

Coverage Status

coverage: 97.883%. remained the same
when pulling f4f8305 on copilot/create-pr-from-commit-5d9626d
into 778e2fc on main.

Copilot AI requested a review from albe March 21, 2026 15:01
Copilot finished work on behalf of albe March 21, 2026 15:01
@albe
albe marked this pull request as ready for review March 21, 2026 15:25
@albe
albe merged commit eaa8ee1 into main Mar 21, 2026
10 checks passed
@albe
albe deleted the copilot/create-pr-from-commit-5d9626d branch March 21, 2026 15:25
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