perf: replace sequential awaits with asyncio.gather() #102

cal · 2026-03-20T14:14:31Z

cal commented

2026-03-20 14:14:31 +00:00

Summary

Replace false-parallel sequential awaits with asyncio.gather() in commands/transactions/management.py (3 transaction fetches) and services/stats_service.py (batting + pitching stats)
Add tests/test_services_stats.py with 5 tests covering get_player_stats() (both stats, one stat, no stats, exception handling)

Fixes #87

Test plan

Existing transaction command tests pass (17/17)
New stats service tests pass (5/5)
Full suite passes (962 passed, 2 skipped)

🤖 Generated with Claude Code

## Summary - Replace false-parallel sequential awaits with `asyncio.gather()` in `commands/transactions/management.py` (3 transaction fetches) and `services/stats_service.py` (batting + pitching stats) - Add `tests/test_services_stats.py` with 5 tests covering `get_player_stats()` (both stats, one stat, no stats, exception handling) Fixes #87 ## Test plan - [x] Existing transaction command tests pass (17/17) - [x] New stats service tests pass (5/5) - [x] Full suite passes (962 passed, 2 skipped) 🤖 Generated with [Claude Code](https://claude.com/claude-code)

cal added 1 commit 2026-03-20 14:14:33 +00:00

perf: replace sequential awaits with asyncio.gather() for true parallelism 9df8d77fa0

Fixes #87

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Claude added the

ai-reviewing

label 2026-03-20 14:15:49 +00:00

Claude reviewed 2026-03-20 14:17:28 +00:00

Claude left a comment

Review: perf: replace sequential awaits with asyncio.gather()

Verdict: APPROVED (posted as COMMENT — Gitea blocks self-approval)

Summary

The PR correctly identifies and fixes a false-parallel pattern in two places. Variables named _task were storing coroutine objects (not asyncio.Task objects), so the three sequential await calls in /mymoves and the two in get_player_stats were executing serially despite the naming suggesting otherwise. asyncio.gather() is the right fix and will genuinely parallelize these API calls.

Correctness

commands/transactions/management.py — my_moves gathers 3 transaction fetches concurrently. ✅ The pre-existing gather calls in /legal confirm this pattern was already established; this PR correctly extends it to mymoves. The conditional get_team_transactions(cancelled=True) remaining sequential is fine — it's conditional and can't be included in the gather without making an unconditional API call.

services/stats_service.py — Same false-sequential pattern, same fix. The try/except Exception wrapper around the gather correctly catches the first-exception propagation from gather(return_exceptions=False) and returns (None, None). ✅ The import asyncio addition is correct.

Exception semantics — With gather(return_exceptions=False) (the default), if one coroutine raises, the exception propagates immediately but other tasks are NOT cancelled — they continue running in the background. For the stats service, this means a batting API failure won't cancel the pitching request; the pitching result is simply discarded. This is acceptable behavior and matches the intent of the (None, None) fallback.

Tests (`tests/test_services_stats.py`)

5 tests are well-structured and cover the key paths: both stats, batting-only, pitching-only, no stats, and exception handling. The test_exception_returns_none_tuple test is correct — get_batting_stats raises RuntimeError, asyncio.gather propagates it, the outer try/except catches it, and (None, None) is returned. ✅

Docstrings explain both what and why, per project conventions.

One minor note: there's no explicit test for the mymoves gather change, but the existing 17/17 transaction command tests provide behavioral coverage.

Cosmetic Formatting

The diff includes extensive linter reformatting of management.py (single→double quotes, trailing commas, line length). All cosmetic, consistent with patterns from prior PRs (#77, #86, #100). No functional impact.

No Issues Found

Branch targets next-release ✅
@logged_command decorator handles exceptions from the gather in mymoves ✅
asyncio was already imported in management.py (used by the pre-existing /legal gathers) ✅
962 passed, 2 skipped ✅

## Review: perf: replace sequential awaits with asyncio.gather() **Verdict: APPROVED** (posted as COMMENT — Gitea blocks self-approval) --- ### Summary The PR correctly identifies and fixes a false-parallel pattern in two places. Variables named `_task` were storing coroutine objects (not `asyncio.Task` objects), so the three sequential `await` calls in `/mymoves` and the two in `get_player_stats` were executing serially despite the naming suggesting otherwise. `asyncio.gather()` is the right fix and will genuinely parallelize these API calls. --- ### Correctness **`commands/transactions/management.py`** — `my_moves` gathers 3 transaction fetches concurrently. ✅ The pre-existing gather calls in `/legal` confirm this pattern was already established; this PR correctly extends it to `mymoves`. The conditional `get_team_transactions(cancelled=True)` remaining sequential is fine — it's conditional and can't be included in the gather without making an unconditional API call. **`services/stats_service.py`** — Same false-sequential pattern, same fix. The `try/except Exception` wrapper around the gather correctly catches the first-exception propagation from `gather(return_exceptions=False)` and returns `(None, None)`. ✅ The `import asyncio` addition is correct. **Exception semantics** — With `gather(return_exceptions=False)` (the default), if one coroutine raises, the exception propagates immediately but other tasks are NOT cancelled — they continue running in the background. For the stats service, this means a batting API failure won't cancel the pitching request; the pitching result is simply discarded. This is acceptable behavior and matches the intent of the `(None, None)` fallback. --- ### Tests (`tests/test_services_stats.py`) 5 tests are well-structured and cover the key paths: both stats, batting-only, pitching-only, no stats, and exception handling. The `test_exception_returns_none_tuple` test is correct — `get_batting_stats` raises `RuntimeError`, `asyncio.gather` propagates it, the outer `try/except` catches it, and `(None, None)` is returned. ✅ Docstrings explain both what and why, per project conventions. One minor note: there's no explicit test for the `mymoves` gather change, but the existing 17/17 transaction command tests provide behavioral coverage. --- ### Cosmetic Formatting The diff includes extensive linter reformatting of `management.py` (single→double quotes, trailing commas, line length). All cosmetic, consistent with patterns from prior PRs (#77, #86, #100). No functional impact. --- ### No Issues Found - Branch targets `next-release` ✅ - `@logged_command` decorator handles exceptions from the gather in `mymoves` ✅ - `asyncio` was already imported in `management.py` (used by the pre-existing `/legal` gathers) ✅ - 962 passed, 2 skipped ✅

cal merged commit 6d3c7305ce into next-release

2026-03-20 14:22:50 +00:00

cal deleted branch fix/sequential-awaits

2026-03-20 14:22:50 +00:00

cal referenced this issue from a commit

2026-03-20 14:22:51 +00:00

Merge pull request 'perf: replace sequential awaits with asyncio.gather()' (#102) from fix/sequential-awaits into next-release

cal referenced this pull request

2026-03-20 15:16:07 +00:00

cleanup: remove dead maintenance mode artifacts in bot.py #104

cal referenced this pull request

2026-03-20 17:54:05 +00:00