⚡ perf(api): fetch user and collection in parallel in feed response#970
⚡ perf(api): fetch user and collection in parallel in feed response#970is0692vs wants to merge 1 commit into
Conversation
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Warning Review limit reached
More reviews will be available in 51 minutes and 37 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
このリポジトリでは staging 先行フローを採用しています。PR のターゲットを |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the buildUserCollectionFeedResponse function in apps/api/src/routes/feed.ts to fetch the user and collection concurrently using Promise.all instead of sequentially. The reviewer suggested utilizing Drizzle's native db.batch() instead of Promise.all to combine these queries into a single network roundtrip to Cloudflare D1, which would optimize performance and reduce latency.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const [user, collection] = await Promise.all([ | ||
| db.select().from(users).where(eq(users.id, id)).get(), | ||
| db | ||
| .select() | ||
| .from(collections) | ||
| .where( | ||
| and( | ||
| eq(collections.ownerType, "user"), | ||
| eq(collections.ownerId, id), | ||
| eq(collections.slug, collectionSlug), | ||
| ), | ||
| ) | ||
| .get(), | ||
| ]); |
There was a problem hiding this comment.
When using Drizzle ORM with Cloudflare D1, executing multiple queries in parallel using Promise.all results in multiple separate network roundtrips to the D1 database. To optimize performance and reduce latency, we should leverage D1's native batching capability via db.batch(). This executes both queries in a single roundtrip.
| const [user, collection] = await Promise.all([ | |
| db.select().from(users).where(eq(users.id, id)).get(), | |
| db | |
| .select() | |
| .from(collections) | |
| .where( | |
| and( | |
| eq(collections.ownerType, "user"), | |
| eq(collections.ownerId, id), | |
| eq(collections.slug, collectionSlug), | |
| ), | |
| ) | |
| .get(), | |
| ]); | |
| const [userRows, collectionRows] = await db.batch([ | |
| db.select().from(users).where(eq(users.id, id)), | |
| db | |
| .select() | |
| .from(collections) | |
| .where( | |
| and( | |
| eq(collections.ownerType, "user"), | |
| eq(collections.ownerId, id), | |
| eq(collections.slug, collectionSlug), | |
| ), | |
| ), | |
| ]); | |
| const user = userRows[0]; | |
| const collection = collectionRows[0]; |
References
- When using Drizzle ORM with Cloudflare D1,
db.batch()is preferred for executing multiple independent queries to minimize network roundtrips and maintain consistency with repository helper patterns.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const [user, collection] = await Promise.all([ | ||
| db.select().from(users).where(eq(users.id, id)).get(), | ||
| db | ||
| .select() | ||
| .from(collections) | ||
| .where( | ||
| and( | ||
| eq(collections.ownerType, "user"), | ||
| eq(collections.ownerId, id), | ||
| eq(collections.slug, collectionSlug), | ||
| ), | ||
| ) | ||
| .get(), | ||
| ]); | ||
|
|
||
| if (!user) return c.json({ error: "User not found" }, 404); | ||
| const authorName = (user.displayName ?? user.name ?? "").trim() || "OpenShelf"; | ||
|
|
||
| const collection = await db | ||
| .select() | ||
| .from(collections) | ||
| .where( | ||
| and( | ||
| eq(collections.ownerType, "user"), | ||
| eq(collections.ownerId, id), | ||
| eq(collections.slug, collectionSlug), | ||
| ), | ||
| ) | ||
| .get(); | ||
|
|
||
| if (!collection || collection.visibility !== "public") { | ||
| return c.json({ error: "Collection not found" }, 404); |
There was a problem hiding this comment.
並列化により、user が存在しない場合（404）でもコレクションクエリが常に D1 へ発行されます。変更前は user の存在確認後にコレクションクエリを発行するフェイルファースト構造でした。ハッピーパスの最適化として意図的なトレードオフではありますが、無効なユーザー ID への大量リクエスト時に D1 の読み取りコストが 2 倍になることに留意してください。
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/api/src/routes/feed.ts
Line: 485-504
Comment:
**存在しないユーザーへのリクエストで不要なDBクエリが実行される**
並列化により、`user` が存在しない場合（404）でもコレクションクエリが常に D1 へ発行されます。変更前は `user` の存在確認後にコレクションクエリを発行するフェイルファースト構造でした。ハッピーパスの最適化として意図的なトレードオフではありますが、無効なユーザー ID への大量リクエスト時に D1 の読み取りコストが 2 倍になることに留意してください。
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
💡 What: Replaced the sequential database queries for fetching a user and their collection with a parallel execution using
Promise.allinbuildUserCollectionFeedResponse.🎯 Why: To improve latency in the user collection RSS feed route (
/users/:id/collections/:cSlug/atom.xml). The collection query depends only on the useridprovided in the request parameters, not on theuserrecord fetched from the database, meaning both queries can be executed simultaneously.📊 Measured Improvement: We established a synthetic microbenchmark (measuring simulated asynchronous query resolution) demonstrating that parallel execution via
Promise.alltheoretically halves the elapsed time (from ~20ms sequential to ~10ms parallel for two concurrent ~10ms I/O operations). In the actual SQLite/D1 database query execution, latency will be improved proportional to the time taken to evaluate the individual database query in Cloudflare D1.PR created automatically by Jules for task 2570954789380940540 started by @is0692vs
Greptile Summary
buildUserCollectionFeedResponse内のユーザー取得とコレクション取得を逐次awaitからPromise.allによる並列実行へ変更し、/users/:id/collections/:cSlug/atom.xmlエンドポイントのレイテンシを削減します。コレクションクエリはリクエストパラメータのidを直接参照しており（ユーザーレコードに依存しない）、並列化の前提条件を正しく満たしています。Promise.allで並列実行し、D1 への往復レイテンシを理論上半減させます。userの存在確認（→ "User not found" 404）はコレクション確認より先に行われており、エラーハンドリングの順序は変更前と同一です。Confidence Score: 4/5
変更は小さく、ロジックの正確性は維持されています。マージ自体は安全です。
並列化の前提（コレクションクエリがユーザーレコードに依存しない）は正しく、エラーハンドリング順序も保たれています。一点、存在しないユーザー向けリクエストでも常にコレクションクエリが走るようになった点が動作の変化として残ります。
apps/api/src/routes/feed.ts — 並列実行後のエラーパス（ユーザー不在時）の挙動を確認してください。
Important Files Changed
buildUserCollectionFeedResponse内の2つの逐次DBクエリをPromise.allによる並列実行へリファクタリング。エラーハンドリングの順序（ユーザー存在確認→コレクション存在確認）は維持されているが、ユーザーが存在しない場合でもコレクションクエリが常に実行されるようになった。Sequence Diagram
sequenceDiagram participant Client participant FeedRoute participant D1 Note over FeedRoute,D1: 変更前（逐次実行） Client->>FeedRoute: GET /users/:id/collections/:cSlug/atom.xml FeedRoute->>D1: "SELECT users WHERE id = :id" D1-->>FeedRoute: user (or null) FeedRoute->>D1: "SELECT collections WHERE ownerId = :id AND slug = :slug" D1-->>FeedRoute: collection (or null) FeedRoute-->>Client: Feed Response Note over FeedRoute,D1: 変更後（並列実行） Client->>FeedRoute: GET /users/:id/collections/:cSlug/atom.xml par Promise.all FeedRoute->>D1: "SELECT users WHERE id = :id" and FeedRoute->>D1: "SELECT collections WHERE ownerId = :id AND slug = :slug" end D1-->>FeedRoute: user + collection FeedRoute-->>Client: Feed ResponsePrompt To Fix All With AI
Reviews (1): Last reviewed commit: "⚡ perf(api): fetch user and collection i..." | Re-trigger Greptile