Skip to content

Add paginated item listing#8

Merged
tukue merged 10 commits into
mainfrom
feature/platform-product-surface
May 15, 2026
Merged

Add paginated item listing#8
tukue merged 10 commits into
mainfrom
feature/platform-product-surface

Conversation

@tukue
Copy link
Copy Markdown
Owner

@tukue tukue commented May 13, 2026

Summary

  • Adds bounded pagination to GET /items with limit, cursor, and nextCursor support.
  • Fixes async handler error mapping so validation failures return JSON 400 responses.
  • Documents the pagination contract and adds focused Lambda handler tests.
  • Narrows TypeScript global types so the CDK project can build reliably.

Validation

  • npm run build
  • NODE_OPTIONS=--max-old-space-size=4096 npm test -- --runInBand

Notes

Unrelated local changes in lib/cdk-app-stack.ts, .aidevops/, and .codex were left out of the pushed commit.

Copy link
Copy Markdown

@amazon-q-developer amazon-q-developer Bot left a comment

Choose a reason for hiding this comment

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

Review Summary

This PR adds paginated item listing functionality, but there are 3 critical logic errors that must be fixed before merge:

Critical Issues Found

  1. Pagination Logic Broken: Sorting items in memory after DynamoDB pagination breaks cursor-based navigation, causing inconsistent results across pages
  2. Scan Without GSI: Using DynamoDB scan without a Global Secondary Index prevents proper ordered pagination and will cause data inconsistencies
  3. Overly Restrictive Cursor Validation: The cursor decoder rejects valid DynamoDB LastEvaluatedKey tokens that don't match the exact {id: string} structure

Recommended Fixes

  • Remove in-memory sorting and use DynamoDB's native ordering with ScanIndexForward
  • Create a GSI with createdAt as sort key for proper ordered pagination, or accept unordered results
  • Allow any valid key structure in cursor validation, not just {id: string}

The error handling improvements and test coverage are solid, but the core pagination implementation needs these fixes to work correctly in production.


You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.


⚠️ This PR contains more than 30 files. Amazon Q is better at reviewing smaller PRs, and may miss issues in larger changesets.

Comment thread lib/function.ts Outdated
Comment on lines +196 to +198
const items = ((result.Items ?? []) as ItemRecord[]).sort((left, right) =>
right.createdAt.localeCompare(left.createdAt),
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛑 Logic Error: Sorting after pagination breaks cursor-based navigation. The sort occurs after DynamoDB returns paginated results, causing inconsistent ordering across pages. When the next page is fetched using the cursor, items may appear out of order or be duplicated because sorting changes which items should have been included in each page.

Suggested change
const items = ((result.Items ?? []) as ItemRecord[]).sort((left, right) =>
right.createdAt.localeCompare(left.createdAt),
);
const result = await dynamodb
.scan({
TableName: requireTableName(),
Limit: limit,
ExclusiveStartKey: exclusiveStartKey,
ScanIndexForward: false,
})
.promise();
const items = (result.Items ?? []) as ItemRecord[];

Comment thread lib/function.ts
Comment on lines +188 to +194
const result = await dynamodb
.scan({
TableName: requireTableName(),
Limit: limit,
ExclusiveStartKey: exclusiveStartKey,
})
.promise();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛑 Logic Error: Using scan without a GSI prevents proper ordered pagination. The cursor-based approach expects consistent ordering, but scan operations don't guarantee order. This will cause items to be missed or duplicated across pages when new items are added during pagination. Create a GSI with createdAt as the sort key or use query with proper indexes.

Comment thread lib/function.ts Outdated
Comment on lines 161 to 170
try {
const decoded = JSON.parse(Buffer.from(rawCursor, 'base64url').toString('utf8')) as Partial<ItemsCursor>;

if (typeof decoded.id !== 'string' || decoded.id.trim() === '') {
throw new Error('Cursor does not contain a valid id');
}

return {
statusCode: 200,
headers: {
'Content-Type': 'application/json',
'Access-Control-Allow-Origin': '*'
},
body: JSON.stringify({
message: 'Testing from Lambda!',
event,
context
})
id: decoded.id,
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛑 Logic Error: The decoded cursor validation rejects valid cursors without the id property. DynamoDB's LastEvaluatedKey can contain any attributes from the table schema (not just id), and the cursor validation is overly restrictive. If the cursor contains additional attributes or different key structures, valid pagination tokens will be rejected.

Suggested change
try {
const decoded = JSON.parse(Buffer.from(rawCursor, 'base64url').toString('utf8')) as Partial<ItemsCursor>;
if (typeof decoded.id !== 'string' || decoded.id.trim() === '') {
throw new Error('Cursor does not contain a valid id');
}
return {
statusCode: 200,
headers: {
'Content-Type': 'application/json',
'Access-Control-Allow-Origin': '*'
},
body: JSON.stringify({
message: 'Testing from Lambda!',
event,
context
})
id: decoded.id,
};
try {
const decoded = JSON.parse(Buffer.from(rawCursor, 'base64url').toString('utf8')) as Record<string, unknown>;
if (!decoded || typeof decoded !== 'object') {
throw new Error('Invalid cursor format');
}
return decoded as AWS.DynamoDB.DocumentClient.Key;

@tukue tukue self-assigned this May 14, 2026
@tukue tukue added the enhancement New feature or request label May 14, 2026
@tukue tukue added this to the mvp milestone May 15, 2026
@tukue tukue marked this pull request as ready for review May 15, 2026 09:45
@tukue tukue merged commit eb167cd into main May 15, 2026
3 checks passed
@tukue tukue deleted the feature/platform-product-surface branch May 15, 2026 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants