Skip to content

CXP-47 fix inf loop#46

Merged
aldevv merged 2 commits intomainfrom
fix_pagination_inf_loop
Feb 3, 2026
Merged

CXP-47 fix inf loop#46
aldevv merged 2 commits intomainfrom
fix_pagination_inf_loop

Conversation

@aldevv
Copy link
Contributor

@aldevv aldevv commented Feb 3, 2026

Summary by CodeRabbit

  • Bug Fixes
    • Pagination now correctly stops at the last page and no longer returns tokens for non-existent pages, preventing extra/failed fetches.
  • Tests
    • Pagination tests updated to stop iterating when no further pages exist, improving test reliability.

@aldevv aldevv requested a review from a team February 3, 2026 00:10
@linear
Copy link

linear bot commented Feb 3, 2026

@coderabbitai
Copy link

coderabbitai bot commented Feb 3, 2026

Walkthrough

Introduces a guard in GetNextToken to compute nextPage and nextOffset and return an empty next-token when nextOffset >= total; test loop in users_test.go now breaks when nextToken is empty to avoid unnecessary unmarshalling and further iterations.

Changes

Cohort / File(s) Summary
Pagination logic
pkg/connector/client/pagination.go
Adds calculation of nextPage and nextOffset in GetNextToken and returns an empty string when nextOffset >= total, preventing generation of a token for non-existent pages.
Pagination test loop
pkg/connector/users_test.go
Adds early exit in pagination test: after asserting no rate-limit annotations, the loop breaks when nextToken is empty to avoid further unmarshalling and iterations.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I counted pages with careful hop,
A little guard said "this is the stop."
No more tokens floating off the ledge,
We tidy paging at the edge. 🌿

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'CXP-47 fix inf loop' is vague and abbreviated, using unclear shorthand like 'inf loop' that doesn't clearly convey the actual change. Use a more descriptive title like 'Fix infinite loop in pagination by adding bounds check' to clearly communicate the specific change.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix_pagination_inf_loop

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

Copy link

@FeliLucero1 FeliLucero1 left a comment

Choose a reason for hiding this comment

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

I think you should add this to the test to fix it:
if nextToken == "" { break }

@aldevv aldevv merged commit 67c1361 into main Feb 3, 2026
5 checks passed
@aldevv aldevv deleted the fix_pagination_inf_loop branch February 3, 2026 17:16
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