fix: pagination error when paginatedField is not _id

[jordins]

📚 Context/Description Behind The Change

Pagination breaks when using paginatedField !== '_id' with projections that exclude _id.

The Bug:

• sanitizeParams sets _id: 0 by default
• When paginatedField !== '_id', cursors must encode as [paginatedFieldValue, _id] tuples
• Without _id in results, cursor encodes as string instead of tuple
• Second page fails with: TypeError: op is not iterable

The Fix:

• Override _id: 0 to _id: 1 when secondary sort is needed
• Strip _id from final results if user didn't request it
• Maintains backward compatibility

Deeper explanation for reviewers

The bug occurs in two places in query.ts:

Place 1: Cursor Encoding (Line 79-81)

// src/utils/query.ts:79-81
response.next = shouldSecondarySortOnId && '_id' in response.next
  ? bsonUrlEncoding.encode([nextPaginatedField, response.next._id])  // ← Encodes as TUPLE
  : bsonUrlEncoding.encode(nextPaginatedField);                      // ← Encodes as STRING

Place 2: Cursor Destructuring (Line 174)

// src/utils/query.ts:174
const [paginatedFieldValue, idValue] = op;  // ← Expects tuple, gets string!

Bug: When op is a string like "my_id_2", JavaScript destructures it as an iterable of characters: ["m", "y"], causing:

• Incorrect query conditions (Id < "m" instead of Id < "my_id_2")
• Or the error: TypeError: op is not iterable (depending on string content)

🚨 Potential Risks & What To Monitor After Deployment

Low risk - fix restores expected behavior for broken edge case. Monitor for:

• Performance impact of additional _id field in projections
• Unexpected _id appearing in results (should not happen - test coverage added)

🧑‍🔬 How Has This Been Tested?

• Added test: "pagination works with custom paginatedField and projection without _id"
• Test demonstrates bug (fails without fix, passes with fix)
• All 110 existing tests pass with both mongoist and native drivers

🚚 Release Plan

Standard release - no special deployment steps needed.

[coderabbitai]

Walkthrough

Tracks whether _id was originally requested in the projection, ensures _id is internally included when using a non-_id paginated field to support cursor encoding, and then removes _id from returned rows if it was not part of the original projection. Tests were added exercising multi-page pagination with a custom paginatedField and a projection that excludes _id; one test block appears duplicated.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Find as find()
    participant Sanitize as sanitizeParams()
    participant DB as Database
    participant Response

    Client->>Find: Request (paginatedField != '_id', projection excludes _id)
    activate Find

    Note over Find: Record whether `_id` was requested originally
    Find->>Sanitize: params + projection

    activate Sanitize
    rect rgb(200,220,255)
        Note over Sanitize: If secondary sort on paginatedField, ensure `_id` is added for cursor (internal)
        Sanitize->>Sanitize: Add `_id` to projection if absent
    end
    Sanitize-->>Find: Modified params (with `_id`)
    deactivate Sanitize

    Find->>DB: Query including `_id`
    DB-->>Find: Rows with `_id`

    rect rgb(220,200,255)
        Note over Find: If `_id` wasn't originally requested and paginatedField != '_id'
        Find->>Find: Strip `_id` from response rows
    end

    Find-->>Response: Paginated results (without `_id` if originally excluded)
    deactivate Find
    Response-->>Client: Return page + next cursor

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–25 minutes

• src/find.ts: verify tracking of originalFieldsIncludedId and correct conditional removal of _id from response rows.
• src/utils/sanitizeParams.ts: confirm _id is only added when needed for secondary-sort cursor encoding and not leaked to responses.
• test/find.test.ts: deduplicate the duplicated test block if unintentional and ensure assertions validate absence of _id and paginatedField where expected.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The pull request title "fix: pagination error when paginatedField is not _id" clearly and specifically summarizes the main issue being addressed in this changeset. It directly references the core problem (pagination error) and the specific condition that triggers it (when paginatedField is not _id), which aligns perfectly with the changes in the code. The title is concise, descriptive, and provides sufficient context for a teammate reviewing the commit history to understand the primary fix being applied without needing to open the PR details.
Description Check	✅ Passed	The pull request description includes all four required sections from the repository template: Context/Description Behind The Change (with detailed explanation of the bug, root cause, and fix), Potential Risks & What To Monitor After Deployment (addressing performance impact and unexpected _id appearance), How Has This Been Tested (describing the test added and existing test results), and Release Plan (indicating standard release with no special steps). The description is comprehensive, well-structured, and provides sufficient context and depth for reviewers to understand the issue, the solution, and the implications of the change without being vague or generic.

✨ 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 jordins/PG-686-fix-pagination-error

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 28ba215 and b9c6dca.

📒 Files selected for processing (3)

• src/find.ts (2 hunks)
• src/utils/sanitizeParams.ts (1 hunks)
• test/find.test.ts (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

• src/find.ts
• test/find.test.ts

🔇 Additional comments (1)

src/utils/sanitizeParams.ts (1)

106-111: LGTM! Fix correctly ensures _id is available for cursor encoding.

The logic properly handles the case where a secondary sort on _id is required but the user's projection excludes it. Key strengths:

• The falsy check !params.fields._id is defensive and handles both 0 and false values (common in MongoDB projections), addressing the past review feedback.
• Placement is correct: after the default _id: 0 is set and after ensuring paginatedField is included.
• The comment clearly explains the rationale and references where cursors are encoded.
• Consistent with the existing pattern at line 102: if (!params.fields[params.paginatedField]).

This ensures cursor encoding as [paginatedFieldValue, _id] tuples works correctly when paginatedField !== '_id', preventing the TypeError: op is not iterable on subsequent pages.

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)

• JIRA integration encountered authorization issues. Please disconnect and reconnect the integration in the CodeRabbit UI.

---

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

test/find.test.ts (1)

1393-1441: Good test coverage for the core bug scenario.

This test effectively validates the fix for pagination with custom paginatedField and projection excluding _id. It confirms that:

• Pagination works across multiple pages
• _id is correctly excluded from results when not requested
• hasNext logic remains correct

Consider adding test coverage for:

• Backward pagination using previous parameter
• Pagination using after/before parameters with the same scenario

This would ensure the fix works correctly in all pagination directions.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3e4da14 and 28ba215.

📒 Files selected for processing (3)

• src/find.ts (2 hunks)
• src/utils/sanitizeParams.ts (1 hunks)
• test/find.test.ts (1 hunks)

🔇 Additional comments (4)

src/utils/sanitizeParams.ts (1)

105-111: LGTM! Clear fix for cursor encoding.

The safeguard correctly ensures _id is included in projections when using a non-_id paginated field, which is necessary for proper cursor tuple encoding. The comment clearly explains the rationale and references the encoding location.

src/find.ts (3)

63-67: LGTM! Proper state tracking before modification.

The code correctly captures whether _id was originally requested before sanitizeParams modifies the projection. The timing and logic are sound.

---

120-127: LGTM! Correct cleanup of internally-added field.

The conditional removal properly strips _id from results when it was added internally for cursor encoding but not requested by the user. The logic correctly complements the safeguard in sanitizeParams.

---

69-76: The original review comment is incorrect - the fix already applies to the aggregate path.

The cleanup logic at lines 122-127 (lines that remove unwanted _id from results) is outside and after the if/else block, so it processes results from both the aggregate() call (line 69-76) and the find() path (line 77-119). The variables originalFieldsIncludedId and paginatedField are also set before the conditional, ensuring the logic works for both code paths.

The shouldRemoveIdFromResponse condition correctly evaluates for results returned from aggregate(), removing the injected _id when appropriate.

[ahmed-anas]

LGTM

[jordins]

🎉 This PR is included in version 9.1.1 🎉

The release is available on:

• GitHub release
• npm package (@latest dist-tag)

Your semantic-release bot 📦🚀