13 lines
1.1 KiB
Markdown
13 lines
1.1 KiB
Markdown
---
|
|
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
|