Implement pagination crate foundation with tests and workspace wiring#327
Merged
Conversation
….1.1 Introduce the execution plan document for roadmap item 4.1.1, outlining the design, constraints, progress, risks, and milestones for implementing the backend pagination crate. This living document ensures clarity on the scope and approach before implementation begins. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
Reviewer's GuideIntroduces a new workspace-local Sequence diagram for cursor encode/decode flowsequenceDiagram
actor Developer
participant BackendService
participant PaginationCursor as "Cursor<UserKey>"
Developer->>BackendService: call handler with last_seen_user
BackendService->>PaginationCursor: new(last_seen_user_key)
PaginationCursor-->>BackendService: Cursor<UserKey>
BackendService->>PaginationCursor: encode()
PaginationCursor-->>BackendService: Result<String, CursorError>
BackendService-->>Developer: opaque cursor token in response
Developer->>BackendService: follow next page with cursor token
BackendService->>PaginationCursor: decode(cursor_token)
PaginationCursor-->>BackendService: Result<Cursor<UserKey>, CursorError>
BackendService-->>Developer: next page data or error mapped from CursorError
Sequence diagram for building paginated response with linkssequenceDiagram
actor Client
participant HttpHandler
participant Repo as Repository
participant PageParamsType as PageParams
participant PaginationLinksType as PaginationLinks
participant PaginatedType as Paginated~UserDto~
Client->>HttpHandler: GET /users?cursor=c1&limit=200
HttpHandler->>PageParamsType: deserialize query into PageParams
PageParamsType-->>HttpHandler: PageParams{ cursor: c1, limit: 100 }
HttpHandler->>Repo: fetch_page(cursor=c1, limit=100)
Repo-->>HttpHandler: users, next_cursor, prev_cursor
HttpHandler->>PaginationLinksType: from_request(request_url, &PageParams, next_cursor, prev_cursor)
PaginationLinksType-->>HttpHandler: PaginationLinks
HttpHandler->>PaginatedType: new(users, PageParams.limit(), PaginationLinks)
PaginatedType-->>HttpHandler: Paginated<UserDto>
HttpHandler-->>Client: 200 OK with data, limit, links
Class diagram for new pagination crate typesclassDiagram
class Cursor~Key~ {
-Key key
+new(key: Key) Cursor~Key~
+key() &Key
+into_inner() Key
+encode() Result<String, CursorError>
}
class CursorError {
<<enum>>
+Serialize message: String
+InvalidBase64 message: String
+Deserialize message: String
}
class PageParams {
-Option<String> cursor
-usize limit
+new(cursor: Option<String>, limit: Option<usize>) Result<PageParams, PageParamsError>
+cursor() Option<&str>
+limit() usize
}
class PageParamsError {
<<enum>>
+InvalidLimit
}
class PaginationLinks {
+String self_
+Option<String> next
+Option<String> prev
+new(self_: String, next: Option<String>, prev: Option<String>) PaginationLinks
+from_request(request_url: Url, params: &PageParams, next_cursor: Option<&str>, prev_cursor: Option<&str>) PaginationLinks
}
class Paginated~T~ {
+Vec<T> data
+usize limit
+PaginationLinks links
+new(data: Vec<T>, limit: usize, links: PaginationLinks) Paginated~T~
}
class Constants {
+DEFAULT_LIMIT: usize
+MAX_LIMIT: usize
}
Cursor~Key~ --> CursorError
PageParams --> PageParamsError
PaginationLinks --> PageParams
Paginated~T~ --> PaginationLinks
PageParams ..> Constants
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
WalkthroughIntroduce a new workspace crate Changes
Sequence DiagramssequenceDiagram
participant Client
participant PageParams as PageParams\n(Normaliser)
participant Cursor as Cursor<K>\n(Encoder/Decoder)
participant Store as DataStore
participant Envelope as Paginated<T>\n+ PaginationLinks
rect rgba(100,150,200,0.5)
Note over Client,PageParams: Inbound request with pagination query
Client->>PageParams: cursor=abc123&limit=50
PageParams->>PageParams: Normalise limit (default/clamp/validate)
PageParams-->>Client: PageParams{cursor, limit}
end
rect rgba(150,200,100,0.5)
Note over Client,Cursor: Decode cursor token (if present)
Client->>Cursor: decode("abc123")
Cursor->>Cursor: base64url-decode → JSON → Cursor<K>
Cursor-->>Client: Cursor{key}
end
rect rgba(200,150,100,0.5)
Note over Client,Store: Fetch and assemble response
Client->>Store: Query with normalised limit & key
Store-->>Client: Vec<T>
Client->>Envelope: new(data, limit, links)
Envelope->>Envelope: PaginationLinks::from_request(request_url, params, next, prev)
Envelope->>Envelope: Preserve existing filters, set limit and cursor qs
Envelope-->>Client: Paginated{data, limit, links{self, next, prev}}
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
- Introduce `backend/crates/pagination` workspace crate providing: - `Cursor<K>` for opaque base64url JSON cursor encoding/decoding. - `PageParams` for normalized pagination query parameters with defaults and caps. - `Paginated<T>` and `PaginationLinks` for paginated response envelopes with hypermedia links. - Add unit and behavioral tests covering cursor round-trip, parameter normalization, and envelope link generation. - Update workspace configuration to include backend crates directory for discovery. - Document the crate design and integration in backend architecture and roadmap. This crate is generic and infrastructure-neutral, serving as a shared pagination foundation. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
leynos
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a2415032d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/crates/pagination/src/cursor.rs`:
- Around line 110-117: The decode function currently uses URL_SAFE_NO_PAD which
rejects padded base64url strings; modify the imports to bring in both
URL_SAFE_NO_PAD and URL_SAFE from base64::engine::general_purpose, then change
Cursor::decode so it first attempts URL_SAFE_NO_PAD.decode(value) and, on error,
falls back to URL_SAFE.decode(value) before mapping the error into
CursorError::InvalidBase64 (preserving the existing error.message handling) so
both padded and unpadded tokens are accepted.
In `@backend/crates/pagination/src/envelope.rs`:
- Around line 23-65: Add Rustdoc examples for the public constructors/builders:
PaginationLinks::new, PaginationLinks::from_request, and Paginated::new. For
each function comment, include a short, executable example block (/// ```rust)
that constructs realistic inputs (e.g., a Url and PageParams or sample cursors),
calls the function, and asserts or prints the resulting link strings or
Paginated<T> shape to show expected output; ensure examples compile by using
simple types for T (like i32 or &str) and include use statements necessary for
Url/PageParams or create minimal test helpers, so the doc tests validate the
demonstrated behavior.
In `@backend/crates/pagination/tests/pagination_bdd.rs`:
- Around line 3-6: Remove the crate-level attribute
#![expect(clippy::expect_used, ...)] from the top of the test module and instead
add a function-level #[expect(clippy::expect_used, reason = "...")] on each BDD
step function that intentionally calls expect (for example, annotate
pagination_parameters_without_a_limit which calls
PageParams::new(...).expect(...), and any other step functions that use expect).
Ensure no file-level allow/expect attributes remain and verify with the provided
rg command to confirm there are no remaining crate-wide suppressions.
In `@docs/execplans/backend-4-1-1-pagination-crate.md`:
- Around line 460-469: The code block lines exceed 120 columns; introduce a
variable (e.g., LOG_DIR) and use mkdir -p "${LOG_DIR}" then replace repeated
long paths with "${LOG_DIR}/<name>.log" for each command (make
prepare-pg-worker, make fmt, make markdownlint, make nixie, make check-fmt, make
lint, make test) so every shell line (including set -o pipefail && ... | tee
...) stays under 120 characters while preserving behavior and filenames.
- Line 58: Replace all absolute local filesystem links that start with
/home/user/project in docs/execplans/backend-4-1-1-pagination-crate.md with
repository-relative paths; for example change the link target for "root
[Cargo.toml]" from (/home/user/project/Cargo.toml) to a relative path like
(../../Cargo.toml) and update other occurrences such as
"[docs/wildside-backend-architecture.md]" and the self-link
"[docs/execplans/backend-4-1-1-pagination-crate.md]" to repository-relative
forms (e.g., (../wildside-backend-architecture.md) and
(backend-4-1-1-pagination-crate.md)); apply the same replacement pattern to the
other reported locations (lines ~70 and ~145) so no link targets point to the
local filesystem.
In `@docs/wildside-backend-architecture.md`:
- Around line 1953-1957: The sentence claiming the root workspace autodiscovery
does not include backend/crates/* is stale; update the paragraph about "Roadmap
4.1.1" and the `backend/crates/pagination` crate to state that the root Cargo
workspace now includes `backend/crates/*` (as updated in the root Cargo.toml),
so the crate will remain present during workspace-member sync — replace the old
sentence with wording that reflects the current state.
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c9c5ce7e-d21d-4017-af8a-38f28d230bc5
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
Cargo.tomlbackend/crates/pagination/Cargo.tomlbackend/crates/pagination/src/cursor.rsbackend/crates/pagination/src/envelope.rsbackend/crates/pagination/src/lib.rsbackend/crates/pagination/src/params.rsbackend/crates/pagination/tests/features/pagination.featurebackend/crates/pagination/tests/pagination_bdd.rsdocs/backend-roadmap.mddocs/execplans/backend-4-1-1-pagination-crate.mddocs/wildside-backend-architecture.md
…n links API - Cursor decoding now accepts both unpadded and padded Base64 URL-safe cursors. - Added examples and expanded API for PaginationLinks and Paginated envelope construction. - Extended BDD tests to verify correct inclusion and omission of pagination links based on cursor availability. - Updated documentation links and fixed relative paths in docs. These enhancements improve flexibility and usability of the pagination crate's core components. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
docs/execplans/backend-4-1-1-pagination-crate.md (2)
460-469:⚠️ Potential issue | 🟡 MinorWrap verification command lines to stay within 120 columns.
Shorten Line 462 to Line 468 by introducing a log directory variable and reusing it.
🛠️ Proposed fix
```bash -mkdir -p /tmp/backend-4-1-1 -set -o pipefail && make prepare-pg-worker 2>&1 | tee /tmp/backend-4-1-1/prepare-pg-worker.log -set -o pipefail && make fmt 2>&1 | tee /tmp/backend-4-1-1/fmt.log -set -o pipefail && make markdownlint 2>&1 | tee /tmp/backend-4-1-1/markdownlint.log -set -o pipefail && make nixie 2>&1 | tee /tmp/backend-4-1-1/nixie.log -set -o pipefail && make check-fmt 2>&1 | tee /tmp/backend-4-1-1/check-fmt.log -set -o pipefail && make lint 2>&1 | tee /tmp/backend-4-1-1/lint.log -set -o pipefail && make test 2>&1 | tee /tmp/backend-4-1-1/test.log +LOG_DIR=/tmp/backend-4-1-1 +mkdir -p "${LOG_DIR}" +set -o pipefail && make prepare-pg-worker 2>&1 | tee "${LOG_DIR}/prepare-pg-worker.log" +set -o pipefail && make fmt 2>&1 | tee "${LOG_DIR}/fmt.log" +set -o pipefail && make markdownlint 2>&1 | tee "${LOG_DIR}/markdownlint.log" +set -o pipefail && make nixie 2>&1 | tee "${LOG_DIR}/nixie.log" +set -o pipefail && make check-fmt 2>&1 | tee "${LOG_DIR}/check-fmt.log" +set -o pipefail && make lint 2>&1 | tee "${LOG_DIR}/lint.log" +set -o pipefail && make test 2>&1 | tee "${LOG_DIR}/test.log"</details> As per coding guidelines: "Code blocks in Markdown must be wrapped at 120 columns." <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/execplans/backend-4-1-1-pagination-crate.mdaround lines 460 - 469, The
verification commands exceed the 120-column Markdown wrap rule; introduce a
reusable log directory variable (e.g., LOG_DIR) and use it for mkdir -p and all
tee targets so each line is shorter: set LOG_DIR=/tmp/backend-4-1-1, mkdir -p
"${LOG_DIR}", then replace explicit /tmp/backend-4-1-1/... paths in the make
commands (prepare-pg-worker, fmt, markdownlint, nixie, check-fmt, lint, test)
with "${LOG_DIR}/.log" to keep lines under 120 columns.</details> --- `56-58`: _⚠️ Potential issue_ | _🟡 Minor_ **Synchronize the workspace wiring constraint with the implemented decision.** Update Line 56 to Line 58 so this section reflects the final autodiscovery-glob approach already recorded later in the same document; remove the stale “explicit workspace member” directive. <details> <summary>📝 Proposed fix</summary> ```diff -- Because the root workspace currently auto-discovers `crates/*` rather than -- `backend/crates/*`, add the new crate as an explicit workspace member in the -- root [Cargo.toml](../../Cargo.toml). +- Ensure the root workspace autodiscovery globs include `backend/crates/*` so +- `backend/crates/pagination` remains durable during workspace-member sync in +- root [Cargo.toml](../../Cargo.toml).As per coding guidelines: "Use markdown files within the
docs/directory as the knowledge base and source of truth for project requirements, dependencies, and architectural decisions."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/execplans/backend-4-1-1-pagination-crate.md` around lines 56 - 58, Replace the stale instruction sentence that reads "Because the root workspace currently auto-discovers `crates/*` rather than `backend/crates/*`, add the new crate as an explicit workspace member in the root [Cargo.toml](../../Cargo.toml)." with wording that reflects the implemented autodiscovery-glob approach (remove the explicit workspace member directive) so the section matches the later documented autodiscovery behavior; ensure the new text states that the root workspace autodiscovers the correct glob and no manual workspace member entry is required.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docs/execplans/backend-4-1-1-pagination-crate.md`:
- Around line 460-469: The verification commands exceed the 120-column Markdown
wrap rule; introduce a reusable log directory variable (e.g., LOG_DIR) and use
it for mkdir -p and all tee targets so each line is shorter: set
LOG_DIR=/tmp/backend-4-1-1, mkdir -p "${LOG_DIR}", then replace explicit
/tmp/backend-4-1-1/... paths in the make commands (prepare-pg-worker, fmt,
markdownlint, nixie, check-fmt, lint, test) with "${LOG_DIR}/<name>.log" to keep
lines under 120 columns.
- Around line 56-58: Replace the stale instruction sentence that reads "Because
the root workspace currently auto-discovers `crates/*` rather than
`backend/crates/*`, add the new crate as an explicit workspace member in the
root [Cargo.toml](../../Cargo.toml)." with wording that reflects the implemented
autodiscovery-glob approach (remove the explicit workspace member directive) so
the section matches the later documented autodiscovery behavior; ensure the new
text states that the root workspace autodiscovers the correct glob and no manual
workspace member entry is required.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d3929554-d60f-4348-b2c9-6c2240a55be1
📒 Files selected for processing (6)
backend/crates/pagination/src/cursor.rsbackend/crates/pagination/src/envelope.rsbackend/crates/pagination/tests/features/pagination.featurebackend/crates/pagination/tests/pagination_bdd.rsdocs/execplans/backend-4-1-1-pagination-crate.mddocs/wildside-backend-architecture.md
Owner
Author
|
@coderabbitai Have the following now been resolved? Annul any requirements that violate the en-GB-oxendict spelling (-ize / -yse / -our) conventions (for example a request to replace "normalize" with "normalise" or "artefact" with "artifact"), or where the requirement unnecessarily increases cyclomatic complexity. |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me inspect the current state of the relevant files before responding. 🏁 Script executed: Length of output: 13778
Comment 1 — BDD coverage for absent cursors ✅ Resolved The feature file now contains all four link-combination scenarios:
The corresponding Comments 2, 3, 4, 5 — en-GB spelling ❌ Annulled These suggestions requested replacing "normalize" / "normalization" with "normalise" / "normalisation". That request is itself incorrect: en-GB-oxendict (the Oxford house style) mandates the -ize suffix, not -ise. "Normalize" and "normalization" are therefore already the correct en-GB-oxendict forms and must not be changed. ✏️ Learnings added
|
leynos
changed the title
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
backend/crates/pagination, wired as an explicit workspace member.Cursor<K>, normalized query parameters viaPageParams, and envelope typesPaginated<T>withPaginationLinksfor navigation.Changes
backend/crates/paginationas an explicit member in the rootCargo.tomlto ensure workspace discovery.backend/crates/paginationwithCargo.toml,src/lib.rs, and modules such ascursor.rs,params.rs,envelope.rs, and (public) items exposed bylib.rs.rstestunit test scaffolds to define public contracts (cursor round-tripping, invalid token decoding, defaulting/normalization).rstest-bddbehavioural test scaffolds to exercise happy/unhappy/edge scenarios at the crate boundary.backend/crates/pagination/tests/features/pagination.feature.backend/crates/pagination/tests/pagination_bdd.rs.docs/execplans/backend-4-1-1-pagination-crate.md.backend/crates/paginationand how it fits hexagonal boundaries.Milestones / Progress plan
backend/crates/pagination.Cursor<K>with opaque base64url JSON encoding/decoding and error handling.PageParamsandPaginated<T>envelopesPageParams,DEFAULT_LIMIT,MAX_LIMIT, andPaginated<T>withPaginationLinks.Acceptance criteria
Test plan
Rationale / design notes
cursor,params,envelope, anderror(cursor error surfaced via CursorError).Risks & mitigations
backend/crates/paginationin the root manifest to avoid invisibility to tooling.Ownership / collaboration
Task
📎 Task: https://www.devboxer.com/task/5a151c60-1799-446e-89c1-84fed29af235
Summary by Sourcery
Introduce a shared pagination crate and wire it into the workspace, providing reusable cursor, parameter, and envelope primitives with accompanying tests and documentation updates.
New Features:
backend/crates/paginationcrate exposing opaque cursor encoding, normalizedPageParams, andPaginated<T>/PaginationLinksresponse envelopes for shared pagination support.Enhancements:
Documentation:
Tests:
rstestunit tests andrstest-bddbehavioural tests, including a Gherkin feature, to verify cursor round-tripping, parameter normalization, and link generation in the pagination crate.📎 Task: https://www.devboxer.com/task/e6b2733d-a5de-4317-b1e2-2fd8aa605430