Skip to content

docs(storage): state the count cap cost as a decode per key past it - #1137

Open
Priyanshubhartistm wants to merge 4 commits into
cameri:rewrite/mainfrom
Priyanshubhartistm:docs/storage-count-cap-cost
Open

Priyanshubhartistm wants to merge 4 commits into
cameri:rewrite/mainfrom
Priyanshubhartistm:docs/storage-count-cap-cost

Conversation

@Priyanshubhartistm

Copy link
Copy Markdown
Collaborator

Description

ADR 0011 and the comments in crates/nostream-storage/src/lmdb/events.rs state the cost of the count's id-set cap wrongly: they say it is "a body decode per repeated key past it". The code reads the row's body for every key the set does not answer past the cap, unless the range is the family's first exact criterion, so the cost is not confined to repeated keys.

This PR corrects the prose in four places in the ADR and three comments in events.rs. It changes no code and no behaviour:

  • "The price of the cap is a body decode per repeated key past it" now says every key the set does not answer reads the row's body (a later criterion's key, or a #g prefix's key), not only a repeated one, and gives the measured cost (5.3× to 5.7× the uncapped walk at 20,000 rows), using the wording the issue proposes.
  • "Below the cap the walk reads no body" is qualified to a walk over a single tag family, since a second family decodes there too.
  • The same correction is applied at each copy: the ADR's other two statements, the tag field comment, the shortcut: marker and the test's doc comment.

What the issue says is right is left as it is: the set bound (270 352 B), the exactness of the count and the free cap below 5,000 rows.

Related Issue

Fixes #1081

Motivation and Context

The record is wrong in the direction a reader would trust: a reader who believes the cost is per repeated key would expect a filter with no repeated key to pay nothing past the cap, while {"#t":["zz","a"]} (one key a row, zero repeats) costs 5.7× its uncapped time at 20,000 rows.

How Has This Been Tested?

Prose and comment changes only. The Rust validation commands pass (cargo fmt --all --check, both clippy passes, cargo test --workspace --all-features, cargo build --workspace --all-features --release), and the diff touches no code line.

Screenshots (if appropriate):

Types of changes

  • Non-functional change (docs, style, minor refactor)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my code changes.
  • I added a changeset, or this is docs-only and I added an empty changeset.
  • All new and existing tests passed.

Signed-off-by: Priyanshubhartistm <bhartipriyanshustm@gmail.com>
@changeset-bot

changeset-bot Bot commented Nov 14, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: cb6a70a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 0 packages

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai

coderabbitai Bot commented Nov 14, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 16cdc962-599c-4163-b89e-614a13462582

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

@greptile-apps

greptile-apps Bot commented Nov 14, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Low impact] The PR appears safe to merge; it changes only documentation and comments.

Summary

Corrects the documented cost of the count’s saved-id cap without changing executable code.

  • Storage notes explain which capped tag keys read event bodies.

Reviews (3) · Last reviewed commit: "Merge remote-tracking branch 'upstream/r..." · Reviewed by Greptile

Signed-off-by: Priyanshubhartistm <bhartipriyanshustm@gmail.com>
greptile-apps[bot]
greptile-apps Bot previously approved these changes Nov 15, 2026
@greptile-apps
greptile-apps Bot dismissed their stale review November 15, 2026 08:35

Dismissed because a newer commit was pushed; Greptile will re-review the current head.

This branch has not been deployed

No deployments
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.

1 participant