perf: parallelize N+1 player/creator lookups with asyncio.gather (#89) #118

Claude · 2026-03-20T20:04:55Z

Claude commented

2026-03-20 20:04:55 +00:00

Closes #89

Summary

Replaces sequential per-item await loops with asyncio.gather() to fire all independent API lookups in parallel.

`services/decision_service.py` — `find_winning_losing_pitchers`

Before: wp, lp, sv, hold_ids, and bsv_ids were fetched one-at-a-time (5–10 sequential calls per game).

After: all IDs are gathered into a single ordered list [wp_id, lp_id, sv_id, *hold_ids, *bsv_ids] and fetched in one asyncio.gather(). None-IDs use asyncio.sleep(0, result=None) as an immediately-resolving no-op. Results are sliced back into wp, lp, sv, holders, blown_saves by position.

`services/custom_commands_service.py` — three methods

get_popular_commands, get_commands_needing_warning, get_commands_eligible_for_deletion each looped over a command list calling get_creator_by_id() sequentially.

After: all creator lookups are gathered in parallel with return_exceptions=True. The post-gather loop preserves original behavior:

BotException → log warning and skip (missing creator)
other BaseException → re-raise (unexpected error, same as the original try/except BotException propagating non-BotException errors)

Files Changed

services/decision_service.py
services/custom_commands_service.py

Test Results

977 passed, 2 skipped

Other observations

The creator_id field on the command model is typed Optional[int] but get_creator_by_id expects int — pre-existing Pyright issue, not introduced here.

Closes #89 ## Summary Replaces sequential per-item `await` loops with `asyncio.gather()` to fire all independent API lookups in parallel. ### `services/decision_service.py` — `find_winning_losing_pitchers` Before: wp, lp, sv, hold_ids, and bsv_ids were fetched one-at-a-time (5–10 sequential calls per game). After: all IDs are gathered into a single ordered list `[wp_id, lp_id, sv_id, *hold_ids, *bsv_ids]` and fetched in one `asyncio.gather()`. None-IDs use `asyncio.sleep(0, result=None)` as an immediately-resolving no-op. Results are sliced back into `wp`, `lp`, `sv`, `holders`, `blown_saves` by position. ### `services/custom_commands_service.py` — three methods `get_popular_commands`, `get_commands_needing_warning`, `get_commands_eligible_for_deletion` each looped over a command list calling `get_creator_by_id()` sequentially. After: all creator lookups are gathered in parallel with `return_exceptions=True`. The post-gather loop preserves original behavior: - `BotException` → log warning and skip (missing creator) - other `BaseException` → re-raise (unexpected error, same as the original `try/except BotException` propagating non-BotException errors) ## Files Changed - `services/decision_service.py` - `services/custom_commands_service.py` ## Test Results 977 passed, 2 skipped ## Other observations - The `creator_id` field on the command model is typed `Optional[int]` but `get_creator_by_id` expects `int` — pre-existing Pyright issue, not introduced here.

Claude added 1 commit 2026-03-20 20:04:57 +00:00

perf: parallelize N+1 player/creator lookups with asyncio.gather (#89 ) aa8330e4d3

Closes #89

Replace sequential per-item await loops with asyncio.gather() to fetch
all results in parallel:

- decision_service.find_winning_losing_pitchers: gather wp, lp, sv,
  hold_ids, and bsv_ids (5-10 calls) in a single parallel batch
- custom_commands_service: parallelize get_creator_by_id() in
  get_popular_commands, get_commands_needing_warning, and
  get_commands_eligible_for_deletion using return_exceptions=True to
  preserve the existing BotException-skip / re-raise-other behavior

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Claude referenced this pull request

2026-03-20 20:05:10 +00:00

perf: N+1 sequential player/creator lookups in loops #89

Claude added the

ai-reviewing

label 2026-03-20 20:15:48 +00:00

Claude reviewed 2026-03-20 20:17:07 +00:00

Claude left a comment

AI Code Review

Files Reviewed

services/decision_service.py (modified)
services/custom_commands_service.py (modified)

Findings

Correctness

No issues found.

decision_service.py — find_winning_losing_pitchers:

ordered_ids = [wp_id, lp_id, sv_id] + hold_ids + bsv_ids builds the gather input in documented order.
asyncio.gather preserves order, so results[0..2] → wp/lp/sv is correct.
results[3 : 3 + len(hold_ids)] and results[3 + len(hold_ids) :] slice correctly for holders and blown_saves.
asyncio.sleep(0, result=None) is the correct stdlib no-op for None IDs — immediately resolves to None, matching the original if pid else None semantics.
if p filter on holders/blown_saves preserves original behavior.

custom_commands_service.py — three cleanup methods:

return_exceptions=True is correct — exceptions become values in the results list.
Exception dispatch order is sound: BotException checked first (log + skip), then BaseException re-raised. This is semantically equivalent to the original try/except BotException (non-BotException propagated in both versions).
zip(commands_data, creators) is safe; asyncio.gather order matches input order.

Security

No issues found.

Style & Conventions

No issues found. Pattern is consistent with existing parallelization in this codebase (PR #103, #106).

Suggestions

The Optional[int] vs int type mismatch on creator_id is pre-existing and correctly noted in the PR body. Worth a follow-up Pyright pass.
The three identical gather blocks in custom_commands_service.py are pre-existing structural similarity — not introduced here, not worth refactoring in this PR.

Verdict: COMMENT

Implementation is correct. Both gather patterns are semantically equivalent to their sequential predecessors. Exception handling semantics preserved. 977 tests passing. Approve when ready.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `services/decision_service.py` (modified) - `services/custom_commands_service.py` (modified) ### Findings #### Correctness No issues found. **`decision_service.py` — `find_winning_losing_pitchers`:** - `ordered_ids = [wp_id, lp_id, sv_id] + hold_ids + bsv_ids` builds the gather input in documented order. - `asyncio.gather` preserves order, so `results[0..2]` → wp/lp/sv is correct. - `results[3 : 3 + len(hold_ids)]` and `results[3 + len(hold_ids) :]` slice correctly for holders and blown_saves. - `asyncio.sleep(0, result=None)` is the correct stdlib no-op for None IDs — immediately resolves to `None`, matching the original `if pid else None` semantics. - `if p` filter on holders/blown_saves preserves original behavior. **`custom_commands_service.py` — three cleanup methods:** - `return_exceptions=True` is correct — exceptions become values in the results list. - Exception dispatch order is sound: `BotException` checked first (log + skip), then `BaseException` re-raised. This is semantically equivalent to the original `try/except BotException` (non-BotException propagated in both versions). - `zip(commands_data, creators)` is safe; `asyncio.gather` order matches input order. #### Security No issues found. #### Style & Conventions No issues found. Pattern is consistent with existing parallelization in this codebase (PR #103, #106). #### Suggestions - The `Optional[int]` vs `int` type mismatch on `creator_id` is pre-existing and correctly noted in the PR body. Worth a follow-up Pyright pass. - The three identical gather blocks in `custom_commands_service.py` are pre-existing structural similarity — not introduced here, not worth refactoring in this PR. ### Verdict: COMMENT Implementation is correct. Both gather patterns are semantically equivalent to their sequential predecessors. Exception handling semantics preserved. 977 tests passing. Approve when ready. --- *Automated review by Claude PR Reviewer*

Claude added

ai-reviewed

and removed

ai-reviewing

labels 2026-03-20 20:17:29 +00:00

cal force-pushed ai/major-domo-v2-89 from aa8330e4d3 to 65d3099a7c

2026-03-31 19:42:56 +00:00

Compare

cal merged commit deb40476a4 into main

2026-03-31 19:45:02 +00:00

cal deleted branch ai/major-domo-v2-89

2026-03-31 19:45:02 +00:00