fix: batch-fetch PitchingCardRatings instead of per-row queries (#19) #44
No reviewers
Labels
No Label
ai-changes-requested
ai-failed
ai-merged
ai-pr-opened
ai-reviewed
ai-reviewing
ai-reviewing
ai-working
bug
enhancement
evolution
performance
phase-0
phase-1a
phase-1b
phase-1c
phase-1d
security
tech-debt
todo
No Milestone
No project
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: cal/paper-dynasty-database#44
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "ai/paper-dynasty-database#19"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
PitchingCardRatings.get_or_none()calls per row (2 queries × N rows = 2N queries) with a single batchedSELECTfor all card IDs upfrontsort_pitchers()(standalone function) andsort_starters()(nested insideget_team_sp)ratings_mapdict keyed by(pitchingcard_id, vs_hand)is built once, then theapplyclosure does O(1) dict lookups per rowFiles Changed
app/routers_v2/teams.pyTest Results
No test suite. Changes verified by reading back the modified file — logic is identical, only the data access pattern changed (batch fetch + dict lookup vs per-row queries).
Other observations
# TODO: should this be max??comment insort_pitchers.get_total_opsis pre-existing and unrelated to this fix.AI Code Review
Files Reviewed
app/routers_v2/teams.py(modified)Findings
Correctness
sort_pitchers()andsort_starters().card_idsis built frompitcher_df["id"]/starter_df["id"], which maps toPitchingCardRatings.pitchingcard_id— consistent with the original per-row queries.(r.pitchingcard_id, r.vs_hand)aligns exactly with the lookups(df_data["id"], "L")/(df_data["id"], "R").return Noneguard before building the DataFrame, socard_idswill never be an empty list when the batch fetch runs.vs_hand << ["L", "R"]filter is slightly redundant (the data should only contain those values), but it's harmless and defensively correct.PitchingCardRatingsrow,vlval/vrvalwill beNoneand.obp/.slgwill raiseAttributeError. This was also true before the PR (get_or_nonealso returnsNone). Not in scope.Security
<<operator generates a parameterizedIN (...)clause — no injection risk.Style & Conventions
get_scouting_dfsis cosmetic and fine.Suggestions
Verdict: APPROVED
Clean, correct N+1 fix. The single batch
SELECTreplaces 2N per-row queries with 1 query + O(1) dict lookups, and the implementation is faithful to the original logic. Ready to merge.Automated review by Claude PR Reviewer
3db9358066toae8c20ea1c