Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 94 additions & 0 deletions .changeset/brave-pagination-feature.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
---
'deepsource-mcp-server': minor
---

feat: implement true pagination for all list queries

Adds comprehensive cursor-based pagination support across all list endpoints to handle large datasets efficiently and provide deterministic, complete results.

## New Features

- **Multi-page fetching**: Automatically fetch multiple pages with `max_pages` parameter
- **Page size control**: Use convenient `page_size` parameter (alias for `first`)
- **Enhanced metadata**: User-friendly pagination metadata in responses
- **Backward compatibility**: Existing queries work without changes

## Supported Endpoints

All list endpoints now support full pagination:

- `projects`
- `project_issues`
- `runs`
- `recent_run_issues`
- `dependency_vulnerabilities`
- `quality_metrics`

## Usage

```javascript
// Fetch up to 5 pages of 50 items each
{
projectKey: "my-project",
page_size: 50,
max_pages: 5
}

// Traditional cursor-based pagination still works
{
projectKey: "my-project",
first: 20,
after: "cursor123"
}
```

## Response Format

Responses now include both standard `pageInfo` and enhanced `pagination` metadata:

```javascript
{
items: [...],
pageInfo: { /* Relay-style pagination */ },
pagination: {
has_more_pages: true,
next_cursor: "...",
page_size: 50,
pages_fetched: 3,
total_count: 250
}
}
```

## Technical Implementation

- **PaginationManager**: New orchestrator class for handling complex pagination scenarios
- **AsyncIterator support**: Modern async iteration patterns for paginated results
- **Bounded loops**: Replaced while loops with bounded for-loops to prevent infinite iterations
- **Error resilience**: Graceful handling of null/undefined data in API responses

## Test Coverage Improvements

- Added 200+ new tests for pagination functionality
- Achieved 100% line coverage for critical components (`issues-client.ts`)
- Comprehensive edge case testing (null data, missing fields, network errors)
- Integration tests for multi-page fetching scenarios

## Performance Considerations

- Single-page fetches remain unchanged for backward compatibility
- Multi-page fetching is opt-in via `max_pages` parameter
- Efficient cursor management reduces API calls
- Response merging optimized for large datasets

## Migration Notes

No breaking changes - existing code will continue to work without modifications. To leverage new pagination features:

1. Add `page_size` parameter for clearer intent (replaces `first`)
2. Add `max_pages` parameter to fetch multiple pages automatically
3. Use the enhanced `pagination` metadata in responses for better UX

This implementation ensures complete data retrieval for large DeepSource accounts without silent truncation or memory issues.

Closes #152
190 changes: 189 additions & 1 deletion src/__tests__/client/base-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@
* @vitest-environment node
*/

import { describe, it, expect, beforeEach, afterAll } from 'vitest';
import { describe, it, expect, beforeEach, afterAll, vi } from 'vitest';
import nock from 'nock';
import { BaseDeepSourceClient } from '../../client/base-client.js';
import { GraphQLResponse } from '../../types/graphql-responses.js';
import { PaginationParams, PaginatedResponse } from '../../utils/pagination/types.js';

// Extend the BaseDeepSourceClient to expose the protected methods
class TestableBaseClient extends BaseDeepSourceClient {
Expand All @@ -28,6 +29,13 @@ class TestableBaseClient extends BaseDeepSourceClient {
return this.findProjectByKey(projectKey);
}

async testFetchWithPagination<T>(
fetcher: (params: PaginationParams) => Promise<PaginatedResponse<T>>,
params: PaginationParams
): Promise<PaginatedResponse<T>> {
return this.fetchWithPagination<T>(fetcher, params);
}

// skipcq: JS-0105 - Test helper method calling static method
testNormalizePaginationParams(params: Record<string, unknown>) {
return BaseDeepSourceClient.normalizePaginationParams(params);
Expand Down Expand Up @@ -378,4 +386,184 @@ describe('BaseDeepSourceClient', () => {
expect(result).toBe('');
});
});

describe('fetchWithPagination', () => {
let client: TestableBaseClient;

beforeEach(() => {
client = new TestableBaseClient(API_KEY);
});

it('should fetch single page when max_pages is not provided', async () => {
const mockFetcher = vi.fn().mockResolvedValue({
items: ['item1', 'item2'],
pageInfo: {
hasNextPage: true,
hasPreviousPage: false,
endCursor: 'cursor1',
},
totalCount: 10,
});

const result = await client.testFetchWithPagination(mockFetcher, {
first: 2,
});

expect(result.items).toEqual(['item1', 'item2']);
expect(mockFetcher).toHaveBeenCalledOnce();
expect(mockFetcher).toHaveBeenCalledWith({ first: 2 });
});

it('should fetch multiple pages when max_pages is provided', async () => {
const page1 = {
items: ['item1', 'item2'],
pageInfo: {
hasNextPage: true,
hasPreviousPage: false,
endCursor: 'cursor1',
},
totalCount: 5,
};

const page2 = {
items: ['item3', 'item4'],
pageInfo: {
hasNextPage: true,
hasPreviousPage: true,
startCursor: 'cursor1',
endCursor: 'cursor2',
},
totalCount: 5,
};

const page3 = {
items: ['item5'],
pageInfo: {
hasNextPage: false,
hasPreviousPage: true,
startCursor: 'cursor2',
},
totalCount: 5,
};

const mockFetcher = vi
.fn()
.mockResolvedValueOnce(page1)
.mockResolvedValueOnce(page2)
.mockResolvedValueOnce(page3);

const result = await client.testFetchWithPagination(mockFetcher, {
first: 2,
max_pages: 5,
});

expect(result.items).toEqual(['item1', 'item2', 'item3', 'item4', 'item5']);
expect(result.pageInfo.hasNextPage).toBe(false);
expect(mockFetcher).toHaveBeenCalledTimes(3);
});

it('should handle page_size alias', async () => {
const mockFetcher = vi.fn().mockResolvedValue({
items: ['item1'],
pageInfo: {
hasNextPage: false,
hasPreviousPage: false,
},
totalCount: 1,
});

await client.testFetchWithPagination(mockFetcher, {
page_size: 10,
});

expect(mockFetcher).toHaveBeenCalledWith({ first: 10 });
});

it('should return correct structure when max_pages is 1', async () => {
const mockFetcher = vi.fn().mockResolvedValue({
items: ['item1', 'item2'],
pageInfo: {
hasNextPage: true,
hasPreviousPage: false,
endCursor: 'cursor1',
},
totalCount: 10,
});

const result = await client.testFetchWithPagination(mockFetcher, {
first: 2,
max_pages: 1,
});

expect(result.items).toEqual(['item1', 'item2']);
expect(result.pageInfo.hasNextPage).toBe(true);
expect(result.pageInfo.endCursor).toBe('cursor1');
expect(result.totalCount).toBe(10);
});

it('should handle errors during multi-page fetching', async () => {
const mockFetcher = vi
.fn()
.mockResolvedValueOnce({
items: ['item1'],
pageInfo: {
hasNextPage: true,
hasPreviousPage: false,
endCursor: 'cursor1',
},
totalCount: 3,
})
.mockRejectedValueOnce(new Error('Network error'));

await expect(
client.testFetchWithPagination(mockFetcher, {
first: 1,
max_pages: 3,
})
).rejects.toThrow('Network error');

expect(mockFetcher).toHaveBeenCalledTimes(2);
});

it('should include endCursor in pageInfo when multi-page fetch has lastCursor', async () => {
// Mock the fetchMultiplePages to return a result with lastCursor
const mockFetcher = vi.fn();

// We need to test the actual implementation, so let's mock fetchMultiplePages directly
// First, let's test with real multi-page fetching behavior
const page1 = {
items: ['item1', 'item2'],
pageInfo: {
hasNextPage: true,
hasPreviousPage: false,
endCursor: 'cursor1',
},
totalCount: 4,
};

const page2 = {
items: ['item3', 'item4'],
pageInfo: {
hasNextPage: false,
hasPreviousPage: true,
startCursor: 'cursor1',
endCursor: 'cursor2',
},
totalCount: 4,
};

mockFetcher.mockResolvedValueOnce(page1).mockResolvedValueOnce(page2);

const result = await client.testFetchWithPagination(mockFetcher, {
first: 2,
max_pages: 3,
});

// The result should have the endCursor from the last page
expect(result.pageInfo.endCursor).toBe('cursor2');
expect(result.items).toEqual(['item1', 'item2', 'item3', 'item4']);
expect(result.pageInfo.hasNextPage).toBe(false);
expect(mockFetcher).toHaveBeenCalledTimes(2);
});
});
});
Loading
Loading