chore: implement recommended claude code fixes

[layertwo]

No description provided.

[claude]

Bug: Missing pagination handling

This query doesn't handle pagination. DynamoDB query operations can return a maximum of 1 MB of data per call. If a user has many collections, the response may be paginated and include a LastEvaluatedKey.

Suggested change

	)
	# Query GSI for all collections for this user
	response = self.table.query(
	IndexName="UserCollectionsIndex",
	KeyConditionExpression="user_id = :user_id",
	ExpressionAttributeValues={":user_id": user_id},
	)
	for item in response.get("Items", []):
	collection = CollectionData.from_dict(item)
	collections.append(collection)
	# Handle pagination
	while "LastEvaluatedKey" in response:
	response = self.table.query(
	IndexName="UserCollectionsIndex",
	KeyConditionExpression="user_id = :user_id",
	ExpressionAttributeValues={":user_id": user_id},
	ExclusiveStartKey=response["LastEvaluatedKey"],
	)
	for item in response.get("Items", []):
	collection = CollectionData.from_dict(item)
	collections.append(collection)

[claude]

Performance: ProjectionType.ALL may be expensive

Using ProjectionType.ALL projects all attributes into the GSI, which doubles the storage cost for collection metadata items. Since list_collections only needs name, modified, count, and total_bytes, consider using ProjectionType.INCLUDE with specific attributes:

Suggested change

	projectionType: ProjectionType.ALL,
	projectionType: ProjectionType.INCLUDE,
	nonKeyAttributes: ["modified", "count", "total_bytes"],

This reduces GSI storage costs and improves query performance.

[claude]

PR Review Summary

This PR implements several configuration improvements and a significant performance optimization. Overall the changes look good with strong test coverage, but there are a few issues that need attention.

✅ Strengths

1. Excellent test coverage: All changes include comprehensive unit tests and integration test updates
2. Good documentation: Added environment variable documentation to AGENTS.md
3. Performance optimization: Replacing scan with GSI query in list_collections is a significant improvement
4. Backward compatibility: Default values preserve existing behavior
5. Proper pagination handling: GSI query correctly handles pagination with LastEvaluatedKey

⚠️ Critical Issues

1. Missing GSI Sort Key in Collection Metadata (High Priority)

The CDK stack defines the GSI with both user_id (partition key) and name (sort key), but the Python code only adds user_id to collection metadata items. The name attribute already exists but may not be at the top level where DynamoDB expects it for the GSI.

2. Incomplete DynamoDB Query Filtering (High Priority)

The list_collections query only filters by user_id, which could return ALL items for that user (BSOs, metadata, etc.), not just collection metadata. The old scan implementation filtered for SK = METADATA which was crucial.

3. Missing Sort Key in GSI Query Parameters (Medium Priority)

The CDK defines the GSI with a sort key (name), but the query doesn't include it in the KeyConditionExpression. While not required, this impacts query efficiency and may not match the intended design.

📋 Other Observations

• Configuration parameters properly injected through CDK stack
• HAWK and OIDC services correctly use configurable timeouts
• Test stubs properly updated for GSI queries
• Removed TODO section from README (good cleanup)

🔍 Security & Performance

• Security: No security issues identified. Proper validation maintained.
• Performance: GSI query is much more efficient than table scan ✅
• Costs: GSI adds storage cost but provides significant read performance benefit

See inline comments for specific recommendations.

[claude]

Critical Bug: This query will return ALL items for the user, not just collection metadata!

The GSI only filters by user_id, so it will return:

• Collection metadata (SK=METADATA) ✅
• All BSO objects (SK=OBJECT#...) ❌
• Any other user data ❌

Solution: Add a filter expression:

response = self.table.query(
    IndexName="UserCollectionsIndex",
    KeyConditionExpression="user_id = :user_id",
    FilterExpression="SK = :metadata",
    ExpressionAttributeValues={
        ":user_id": user_id,
        ":metadata": self._metadata_sk()
    },
)

Note: FilterExpression is applied after the query, so you still get the GSI performance benefit but filter out non-metadata items.

[claude]

Same FilterExpression needed here for pagination queries.

[claude]

Design Question: The GSI defines name as the sort key, but the Python query doesn't use it in the KeyConditionExpression.

This has implications:

1. Storage: DynamoDB will store the sort key value even if not queried
2. Query efficiency: Not leveraging the sort key for ordering
3. Potential waste: If name isn't needed for queries, consider removing it from the GSI to save on storage costs

Options:

• Use the sort key in queries: KeyConditionExpression="user_id = :user_id AND begins_with(#name, :prefix)"
• Or remove sort key from GSI if not needed: only define partition key user_id

What's the intended use case?

[claude]

Cost Consideration: ProjectionType.ALL projects all table attributes into the GSI, which doubles your storage costs for collection metadata items.

If queries only need name, modified, count, and usage, consider using ProjectionType.INCLUDE with specific attributes:

projectionType: ProjectionType.INCLUDE,
nonKeyAttributes: ["modified", "count", "usage"],

This reduces storage costs while maintaining query performance. Only use ALL if you truly need every attribute.

[claude]

Test Update Needed: Once you add the FilterExpression to fix the critical bug, these test stubs will need to be updated to include the filter parameters:

{
    "TableName": storage_table_name,
    "IndexName": "UserCollectionsIndex",
    "KeyConditionExpression": "user_id = :user_id",
    "FilterExpression": "SK = :metadata",
    "ExpressionAttributeValues": {
        ":user_id": "test-user-123",
        ":metadata": "METADATA"
    },
}

[github-actions]

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
lambda/src/environment
service_provider.py
lambda/src/services
hawk_service.py
oidc_validator.py
storage_manager.py
Project Total

This report was generated by python-coverage-comment-action

[claude]

Action Items

Before merging, please address:

🔴 Must Fix (Critical)

1. Add FilterExpression to list_collections query - The GSI query will return all user items (BSOs + metadata). Need to filter for SK = METADATA after the query.
2. Verify name attribute structure - Confirm CollectionData.to_dict() outputs name at root level for GSI sort key
3. Search for other collection metadata writes - Ensure all locations that create/update collection metadata include the user_id attribute

🟡 Should Consider

1. GSI design decision - If name sort key is not used in queries, consider removing it to reduce storage costs
2. GSI projection type - Consider ProjectionType.INCLUDE instead of ALL to reduce storage costs if you do not need all attributes

✅ What Works Well

• Comprehensive test coverage with proper pagination tests
• Backward-compatible configuration with sensible defaults
• Clear separation of HAWK vs OIDC clock skew tolerance
• Good documentation in AGENTS.md

Let me know if you need help with any of these issues!