Skip to content

test(api-web): enumerate pagination boundaries#3973

Open
chet wants to merge 1 commit into
NVIDIA:mainfrom
chet:gh-issue-3970
Open

test(api-web): enumerate pagination boundaries#3973
chet wants to merge 1 commit into
NVIDIA:mainfrom
chet:gh-issue-3970

Conversation

@chet

@chet chet commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

paginate_vec is the in-memory pagination path used by the admin UI handlers, but its direct unit coverage stopped at one ordinary page while an unused paginate_ids copy owned six boundary tests.

So, this removes the dead copy and makes one labeled paginate_vec table the record for slicing and metadata behavior. It also enumerates PaginationInfo navigation, PageContext construction/delegation, and real query deserialization.

The conversion checkpoint retains all seven existing inputs. The expanded checkpoint takes pagination.rs production coverage from 67/135 to 114/114 lines, and the complete file from 147/216 to 403/403 lines.

Related issues

This supports #3970

Type of Change

  • Add - New feature or capability
  • Change - Changes in existing functionality
  • Fix - Bug fixes
  • Remove - Removed features or deprecated functionality
  • Internal - Internal changes (refactoring, tests, docs, etc.)

Breaking Changes

  • This PR contains breaking changes

Testing

  • Unit tests added/updated
  • Integration tests added/updated
  • Manual testing performed
  • No testing required (docs, internal refactor, etc.)

cargo test -p carbide-api-web pagination::tests --lib

cargo +1.96.0 llvm-cov test --locked --package carbide-api-web --lib --remap-path-prefix --no-fail-fast --no-report

cargo make format-nightly

cargo make clippy

cargo make carbide-lints

Additional Notes

paginate_ids had no workspace callers and lived in the private pagination module with #[allow(dead_code)]. Removing it does not change the active pagination path.

Remove the unused `paginate_ids` helper and move its boundary inputs into one table for the active `paginate_vec` path.

Add metadata, context, and query-deserialization tables so the module production code moves from 67/135 to 114/114 covered lines without repeating the helper contract.

This supports NVIDIA#3970

Signed-off-by: Chet Nichols III <chetn@nvidia.com>
@chet
chet requested a review from a team as a code owner July 23, 2026 11:23
@chet

chet commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai full_review, thanks!

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: a8391181-ea10-47c3-aee0-19ce46ba51ae

📥 Commits

Reviewing files that changed from the base of the PR and between 2de970b and 2d5ba6d.

📒 Files selected for processing (1)
  • crates/api-web/src/pagination.rs

Summary by CodeRabbit

  • New Features
    • Enhanced pagination for owned collections: returns only the items in the requested page.
    • Supports unbounded limits, clamps oversized limits, handles empty collections, and safely returns empty results for out-of-range page requests.
    • Improved pagination metadata and navigation/range calculations.
  • Bug Fixes
    • More robust pagination query handling for missing, empty, null, invalid, and negative values.
  • Tests
    • Reworked and expanded coverage for pagination behavior, navigation helpers, and query/JSON parsing scenarios.

Walkthrough

The pagination API replaces borrowed-slice pagination with an owned-vector helper and adds scenario coverage for page boundaries, metadata navigation, page contexts, and query parameter deserialization.

Changes

Pagination API

Layer / File(s) Summary
Vector pagination behavior
crates/api-web/src/pagination.rs
paginate_ids is replaced by consuming paginate_vec, which handles bounded, unbounded, empty, and out-of-range pages while returning pagination metadata.
Metadata and page context validation
crates/api-web/src/pagination.rs
Tests cover PaginationInfo navigation helpers and PageContext constructors, accessors, and delegated navigation.
Pagination parameter deserialization
crates/api-web/src/pagination.rs
Tests cover Axum query parsing, invalid numeric values, missing values, and JSON null deserialization.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

  • NVIDIA/infra-controller issue 3970 — The PR replaces the pagination helper requested by the issue and adds boundary coverage.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main change: expanded pagination boundary tests in api-web.
Description check ✅ Passed The description matches the PR changes, covering the paginator refactor and the added pagination coverage.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

@chet Understood — I’ll perform a fresh, comprehensive review of the full PR, including the pagination behavior, test coverage, and maintainability of the refactor.

🐇🔍

✅ Action performed

Full review finished.

@chet
chet enabled auto-merge (squash) July 23, 2026 14:29
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