| id |
type |
title |
tags |
importance |
confidence |
created |
updated |
| 32130490-7783-4d4e-800a-f0b3c6594c9d |
workflow |
PR review: paper-dynasty-database#53 (REQUEST_CHANGES) |
| pr-reviewer |
| paper-dynasty-database |
| automation |
|
0.5 |
0.8 |
2026-03-05T03:56:00.393294+00:00 |
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