perf: parallelize get_card_embeds calls in display_cards (#98) #157

Merged
cal merged 1 commits from issue/98-perf-parallelize-get-card-embeds-calls-in-display into main 2026-04-08 10:26:06 +00:00
Collaborator

Closes #98

Summary

Replace sequential await-in-list-comprehension with asyncio.gather() in display_cards():

# Before — sequential (N round-trips one at a time)
card_embeds = [await get_card_embeds(x) for x in cards]

# After — concurrent (all N round-trips in parallel)
card_embeds = list(await asyncio.gather(*[get_card_embeds(x) for x in cards]))

Impact

Each get_card_embeds() makes 2 DB round-trips (refractor state + paperdex lookup). For a standard 5-pack open (25 cards), this eliminates 50 sequential round-trips and replaces them with ~2 concurrent batches. asyncio was already imported.

list() wraps the result since asyncio.gather() returns a tuple; downstream code indexes card_embeds like a list.

Files Changed

  • helpers/main.py — 1-line change at line 342
Closes #98 ## Summary Replace sequential `await`-in-list-comprehension with `asyncio.gather()` in `display_cards()`: ```python # Before — sequential (N round-trips one at a time) card_embeds = [await get_card_embeds(x) for x in cards] # After — concurrent (all N round-trips in parallel) card_embeds = list(await asyncio.gather(*[get_card_embeds(x) for x in cards])) ``` ## Impact Each `get_card_embeds()` makes 2 DB round-trips (refractor state + paperdex lookup). For a standard 5-pack open (25 cards), this eliminates 50 sequential round-trips and replaces them with ~2 concurrent batches. `asyncio` was already imported. `list()` wraps the result since `asyncio.gather()` returns a tuple; downstream code indexes `card_embeds` like a list. ## Files Changed - `helpers/main.py` — 1-line change at line 342
Claude added 1 commit 2026-04-08 07:33:14 +00:00
perf: parallelize get_card_embeds calls in display_cards (#98)
All checks were successful
Ruff Lint / lint (pull_request) Successful in 17s
2f22a11e17
Closes #98

Replace sequential await-in-list-comprehension with asyncio.gather() so
all card embed fetches run concurrently. Cuts 50 sequential DB round-trips
(5 packs × 5 cards × 2 calls each) down to ~2 concurrent batches.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude added the
ai-reviewing
label 2026-04-08 09:52:27 +00:00
Claude reviewed 2026-04-08 09:54:16 +00:00
Claude left a comment
Author
Collaborator

AI Code Review

Files Reviewed

  • helpers/main.py (modified — line 342)

Findings

Correctness

No issues found. The change is a correct, idiomatic asyncio parallelization:

  • asyncio is already imported at line 1.
  • asyncio.gather() accepts an unpacked list of coroutines — correct usage.
  • Empty cards input: asyncio.gather() with no arguments returns [], safe.
  • Error propagation is unchanged — any exception from a get_card_embeds() call still bubbles up through gather and is caught by the existing except Exception as e block at line 350, returning False.

One minor inaccuracy in the PR body: asyncio.gather() when awaited returns a list, not a tuple. The list() wrapper is therefore a no-op (a shallow copy), not a type conversion. This is harmless but the PR body comment could mislead future contributors.

Security

No issues found. No user input flows through the changed line.

Style & Conventions

No issues found. Parallelizing async calls with asyncio.gather() is the established pattern in this codebase.

Suggestions

  • The list() wrapper is unnecessary since await asyncio.gather(...) already returns a list. It can stay, but consider correcting the PR body comment to avoid the "returns a tuple" misconception.
  • For 25-card packs this fires ~75 concurrent requests to the database API (1 refractor + 1 paperdex + 1 cards-dupe per card). Worth smoke-testing with a full pack open on staging to confirm the API handles the burst without 429s or timeouts.

Verdict: APPROVED

Clean 1-line change. Correctly converts a sequential await-in-comprehension to a parallel asyncio.gather(). Error handling, empty-input, and downstream list indexing are all unaffected. Gitea blocks self-approval — posting as COMMENT.


Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `helpers/main.py` (modified — line 342) ### Findings #### Correctness No issues found. The change is a correct, idiomatic asyncio parallelization: - `asyncio` is already imported at line 1. - `asyncio.gather()` accepts an unpacked list of coroutines — correct usage. - Empty `cards` input: `asyncio.gather()` with no arguments returns `[]`, safe. - Error propagation is unchanged — any exception from a `get_card_embeds()` call still bubbles up through `gather` and is caught by the existing `except Exception as e` block at line 350, returning `False`. One minor inaccuracy in the PR body: `asyncio.gather()` when awaited returns a `list`, not a tuple. The `list()` wrapper is therefore a no-op (a shallow copy), not a type conversion. This is harmless but the PR body comment could mislead future contributors. #### Security No issues found. No user input flows through the changed line. #### Style & Conventions No issues found. Parallelizing async calls with `asyncio.gather()` is the established pattern in this codebase. #### Suggestions - The `list()` wrapper is unnecessary since `await asyncio.gather(...)` already returns a `list`. It can stay, but consider correcting the PR body comment to avoid the "returns a tuple" misconception. - For 25-card packs this fires ~75 concurrent requests to the database API (1 refractor + 1 paperdex + 1 cards-dupe per card). Worth smoke-testing with a full pack open on staging to confirm the API handles the burst without 429s or timeouts. ### Verdict: APPROVED Clean 1-line change. Correctly converts a sequential await-in-comprehension to a parallel `asyncio.gather()`. Error handling, empty-input, and downstream list indexing are all unaffected. Gitea blocks self-approval — posting as COMMENT. --- *Automated review by Claude PR Reviewer*
Claude added
ai-reviewed
and removed
ai-reviewing
labels 2026-04-08 09:54:45 +00:00
cal approved these changes 2026-04-08 10:25:58 +00:00
cal left a comment
Owner

Approved via pd-pr

Approved via pd-pr
cal merged commit ff57b8fea3 into main 2026-04-08 10:26:06 +00:00
cal deleted branch issue/98-perf-parallelize-get-card-embeds-calls-in-display 2026-04-08 10:26:06 +00:00
Sign in to join this conversation.
No reviewers
cal
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: cal/paper-dynasty-discord#157
No description provided.