Fix/cursor identified performance issues #585
Conversation
Expand error detection to catch common crypto failures that were previously missed: - Node.js crypto: 'Unsupported state or unable to authenticate data' - Web Crypto API: OperationError exceptions This ensures users see the password retry prompt instead of a generic 'Download failed' message when entering an incorrect password for encrypted file downloads.
Replace client-side aggregation of all interactions with a PostgreSQL function that performs GROUP BY, SUM, ORDER BY, and LIMIT in the database. Now returns only 10 records per query instead of potentially millions, preventing memory issues and browser crashes for large date ranges.
…ror handling, remove unused queries
✅ Deploy Preview for auto-drive-storage ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
| $fromDate: timestamp! | ||
| $toDate: timestamp! | ||
| $fromDate: timestamptz! | ||
| $toDate: timestamptz! |
There was a problem hiding this comment.
Inconsistent timestamp types may cause timezone filtering differences
Medium Severity
The new GET_TOP_ACCOUNTS_BY_INTERACTION query uses timestamptz! types while the existing GET_ALL_STATS query uses timestamp! types for the same date range inputs (both receive toISOString() values). When the underlying interactions.created_at column is timestamp (without timezone), these differing types cause PostgreSQL to apply different timezone conversions. This could result in the aggregated statistics and top accounts rankings reflecting slightly different time ranges, leading to inconsistent data in the admin dashboard.
Additional Locations (1)
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
| uploadLimit: 0, | ||
| downloadLimit: 0, | ||
| model: AccountModel.Monthly, | ||
| })); |
There was a problem hiding this comment.
Unused type fields add unnecessary code complexity
Low Severity
The transformToRankingAccounts function sets uploadLimit: 0, downloadLimit: 0, and model: AccountModel.Monthly but these fields are never used in the component. Only id, organizationId, and totalSize are consumed in the rendering logic. The AccountModel import is also only used for this dead assignment.
…tats Both GET_ALL_STATS and GET_TOP_ACCOUNTS_BY_INTERACTION queries now use timestamptz! instead of mixing timestamp! and timestamptz! types. This ensures consistent timezone handling when comparing ISO date strings against the interactions table.
- pg.ts: Release PostgreSQL client after connection verification The pool.connect() call was acquiring a client but never releasing it, causing a connection leak on every application startup. - transactionManager.ts: Clean up Polkadot API error listeners Added api.off() call in cleanup() to remove the error listener when transactions complete, preventing listener accumulation on the shared API object. - readable.ts: Properly clean up stream event listeners Added centralized cleanup() function called from all exit paths (data complete, stream end, stream error) to prevent listener leaks on byte-range requests.
Removes the database function and migration for top accounts calculation. Keeps the timestamptz fix for consistent date handling in GraphQL queries. Top accounts are now calculated client-side as before, avoiding schema changes.
Added limit of 10,000 interactions ordered by size desc to the ranking query. This bounds memory usage while still capturing the largest interactions for accurate top accounts calculation.
|
@cursoragent review |
Review CompleteI've reviewed the PR and addressed the issues identified by the Cursor bot: ✅ Fixed: Unused Type Fields (Low Severity)Removed unnecessary code complexity from the
✅ Already Resolved: Timestamp Type Consistency (Medium Severity)The timestamp type inconsistency was already fixed in a previous commit. Both Changes Committed & PushedThe cleanup has been committed to the |
- Remove unused uploadLimit, downloadLimit, and model fields - Remove unused AccountModel import - Simplify aggregateByAccount function mapping Co-authored-by: Emil Fattakhov <EmilFattakhov@users.noreply.github.com>
jim-counter
left a comment
jim-counter
left a comment
There was a problem hiding this comment.
LGTM. How come you switched back to client-side top 10 aggregation?
3 participants
Summary
This PR includes performance optimizations for the admin dashboard and improved error handling for encrypted file downloads.
Changes
🚀 Performance: Server-side Top Accounts Aggregation
Replaced client-side aggregation with a PostgreSQL function that performs
GROUP BY,SUM,ORDER BY, andLIMITdirectly in the database.Before: Fetched all interaction records to the browser, then aggregated in JavaScript
After: Returns only the top 10 accounts per query from the database
This prevents memory issues and potential browser crashes when querying large date ranges with millions of interactions.
Files changed:
apps/hasura/migrations/.../up.sql- Newget_top_accounts_by_interactionfunctionapps/hasura/metadata/...- Hasura function configurationapps/frontend/.../AdminStats.tsx- Updated to use server-side aggregation🐛 Fix: Improved Decryption Error Detection
Expanded error detection to catch crypto failures that were previously missed, ensuring users see the password retry prompt instead of a generic "Download failed" message.
Now detects:
"Unsupported state or unable to authenticate data"OperationErrorexceptionsFiles changed:
apps/frontend/src/utils/file.ts🧹 Cleanup: Remove Unused GraphQL Queries
Deleted
query.graphqlwhich contained 3 unused queries (GetUploadStats,GetDownloadStats,GetAllStats). The component uses inlinegqltemplate literals instead.