diff --git a/graph/workflows/pr-review-paper-dynasty-database53-request-changes-321304.md b/graph/workflows/pr-review-paper-dynasty-database53-request-changes-321304.md new file mode 100644 index 00000000000..3210f5659d1 --- /dev/null +++ b/graph/workflows/pr-review-paper-dynasty-database53-request-changes-321304.md @@ -0,0 +1,12 @@ +--- +id: 32130490-7783-4d4e-800a-f0b3c6594c9d +type: workflow +title: "PR review: paper-dynasty-database#53 (REQUEST_CHANGES)" +tags: [pr-reviewer, paper-dynasty-database, automation] +importance: 0.5 +confidence: 0.8 +created: "2026-03-05T03:56:00.393294+00:00" +updated: "2026-03-05T03:56:00.393294+00:00" +--- + +Reviewed paper-dynasty-database PR #53 — fix: batch Paperdex lookups to avoid N+1 queries (#17). Verdict: REQUEST_CHANGES. Summary: One actionable bug: in `get_random_player` (players.py), the PR iterates `final_players` twice to batch Paperdex lookups. When no position filters are applied, `final_players` remains a Peewee ModelSelect with `ORDER BY RANDOM()`. The two passes execute separate SQL queries returning different random rows, so dex_by_player is keyed on a different set of players than those serialised — every returned player's paperdex will be silently empty. Fix: add `final_players = list(final_players)` before the player_ids comprehension. The other two patch sites (get_players and cards.py get_cards) are correct — get_players always builds a Python list, and cards.py explicitly materialises with list(all_cards).. Cost: $0.61046375