Skip to content

Conversation

@marcodejongh
Copy link
Owner

Groups multiple attempts/ascents of the same climb on the same day into
a single card showing a summary (e.g., "3 attempts" or "Sent, 2 attempts").
The thumbnail still navigates to the climb info page when clicked.

Groups multiple attempts/ascents of the same climb on the same day into
a single card showing a summary (e.g., "3 attempts" or "Sent, 2 attempts").
The thumbnail still navigates to the climb info page when clicked.
…feed

- Add new GraphQL type GroupedAscentFeedItem and query userGroupedAscentsFeed
- Backend groups attempts by climbUuid + date with counts for flash/send/attempt
- Track best quality rating and latest comment per group
- Frontend displays grouped entries with combined status summary
- Thumbnail click navigates to climb info page (existing behavior)

Examples of grouped display:
- "3 attempts" - multiple failed attempts
- "6 sends" - multiple successful sends
- "Flashed, 2 attempts" - one flash with additional attempts
- "3 sends, 3 attempts" - mixed results
@vercel
Copy link

vercel bot commented Jan 12, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
boardsesh Building Building Preview, Comment Jan 12, 2026 8:45am

@claude
Copy link

claude bot commented Jan 12, 2026

Claude Review

⚠️ Needs attention - Critical performance issue: fetching all user ticks without limit before pagination.

Issues

  1. Performance: Fetches all rows before pagination - packages/backend/src/graphql/resolvers/ticks/queries.ts:228-255 - The query fetches ALL ticks for a user (no LIMIT), loads them into memory, groups them, then paginates. For users with thousands of logged climbs, this will cause memory issues and slow response times. Consider SQL-level grouping/pagination or at minimum add a reasonable fetch limit.

  2. Missing test coverage - No tests for the new userGroupedAscentsFeed query. Given the complex grouping logic, edge cases should be tested (empty results, single item groups, multiple statuses on same day/climb).

  3. Unused import - packages/web/app/components/activity-feed/ascents-feed.tsx:10 - Title is imported but no longer used after refactoring.

Prevents memory issues for users with thousands of logged climbs
by capping the fetch to 500 most recent entries before grouping.
@marcodejongh marcodejongh merged commit 6a0aa4d into main Jan 12, 2026
3 of 7 checks passed
@claude
Copy link

claude bot commented Jan 12, 2026

Claude Review

⚠️ Needs attention - Missing tests for new query, potential performance issue with in-memory grouping.

Issues

  1. No tests for userGroupedAscentsFeed resolver (packages/backend/src/graphql/resolvers/ticks/queries.ts:213-362) - The codebase has existing resolver tests in packages/backend/src/__tests__/graphql-resolvers.test.ts, but this new query lacks test coverage for grouping logic, edge cases (empty results, single items, pagination bounds), and error handling.

  2. In-memory grouping may cause performance issues (packages/backend/src/graphql/resolvers/ticks/queries.ts:254-336) - Fetches up to 500 records then groups in memory. For users with many climbs, this loads unnecessary data when only the first page is needed. Consider SQL-level grouping or a more efficient pagination strategy.

  3. Unused import (packages/web/app/components/activity-feed/ascents-feed.tsx:27) - Title is destructured from Typography but never used after refactoring.

  4. Missing userId validation (packages/backend/src/graphql/resolvers/ticks/queries.ts:217) - Unlike other queries that validate boardType, userId is used directly without validation. Consider validating the format to prevent potential issues.

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.

3 participants