Skip to content

Add optional client-side limit to FetchNotesRequest - #84

Draft
Dominik1999 wants to merge 2 commits into
mainfrom
ev/fetch-limit
Draft

Add optional client-side limit to FetchNotesRequest#84
Dominik1999 wants to merge 2 commits into
mainfrom
ev/fetch-limit

Conversation

@Dominik1999

@Dominik1999 Dominik1999 commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Added optional uint32 limit field to FetchNotesRequest proto
  • Client can request smaller pages (e.g. limit: 10 for a mobile wallet)
  • Server clamps to FETCH_NOTES_BATCH_SIZE (500) - client can tighten, never exceed
  • limit=0 treated as unset (uses server default)
  • When unset, existing behavior unchanged (500 max)

Supersedes the original #56 with a rewrite on current main, incorporating the seq-cursor migration, multi-tag snapshot queries, and structured tracing.

Client-side follow-up: 0xMiden/miden-client#2107

Test plan

  • make lint passes
  • make test passes (13/13)
  • Test with limit: Some(N) to verify clamping behavior

🤖 Generated with Claude Code

Closes #46
Closes #4

claude added 2 commits April 23, 2026 18:51
- Add `optional uint32 limit` field to FetchNotesRequest proto
- Server clamps client limit to FETCH_NOTES_BATCH_SIZE (500)
- When unset, server default (500) applies
- Thread limit parameter through Database trait, facade, and SQLite impl
- Regenerate proto bindings

Closes #46
Closes #4
Supersedes #56

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Treat limit=0 as None (use server default) instead of returning empty
- Record notes_returned in the tracing span

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

@mmagician mmagician left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Left a comment in the client-counterpart issue: 0xMiden/rust-sdk#2107

@mmagician
mmagician changed the base branch from next to main June 8, 2026 09:38

@WiktorStarczewski WiktorStarczewski 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.

LGTM

@Dominik1999
Dominik1999 marked this pull request as draft June 17, 2026 13:40
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.

4 participants