cal/claude-memory
1
0
Fork 0
Code Issues 2 Pull Requests Actions Packages Projects Releases Wiki Activity

store: PR review: paper-dynasty-database#53 (REQUEST_CHANGES)

Browse Source
This commit is contained in:
Cal Corum 2026-03-04 21:56:00 -06:00
parent 215f29d0dd
commit d727cb4dc3
1 changed files with 12 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
graph/workflows/pr-review-paper-dynasty-database53-request-changes-321304.md Normal file
View File

@ -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