fix: batch BattingCard/BattingCardRatings lookups in lineup builder (#18) #45

cal · 2026-03-03T23:34:32Z

cal commented

2026-03-03 23:34:32 +00:00

Summary

Fixes #18 — get_bratings() in build_lineup was issuing 3 individual DB queries per player (BattingCard.get_or_none + 2× BattingCardRatings.get_or_none). For a 9-player lineup this meant up to 27+ round trips before the fix.

Changes

app/routers_v2/teams.py

Before the position loop, batch-fetch all BattingCard rows for legal_players ∪ backup_players in one query.
Batch-fetch all BattingCardRatings for those cards in a second query.
Build two dicts: _batting_cards_by_player (player_id → BattingCard) and _ratings_by_card_hand (card_id → {hand → BattingCardRatings}).
Rewrite get_bratings(player_id) to do O(1) dict lookups instead of DB calls.

DB round trips reduced from O(3N) per lineup call to O(2), regardless of roster size. Error behaviour (AttributeError when no card/rating exists) is preserved identically for the existing try/except callers.

Test plan

No automated test suite. Verified by reading back the modified function — logic is equivalent to original; only the data source changed from DB queries to pre-built dicts.

Other observations

Lines 544–576 contain a large commented-out block of dead code (an old batch approach that was never wired up). Out of scope for this PR but could be removed.

## Summary Fixes #18 — `get_bratings()` in `build_lineup` was issuing 3 individual DB queries per player (`BattingCard.get_or_none` + 2× `BattingCardRatings.get_or_none`). For a 9-player lineup this meant up to 27+ round trips before the fix. ## Changes **`app/routers_v2/teams.py`** - Before the position loop, batch-fetch all `BattingCard` rows for `legal_players` ∪ `backup_players` in one query. - Batch-fetch all `BattingCardRatings` for those cards in a second query. - Build two dicts: `_batting_cards_by_player` (player_id → BattingCard) and `_ratings_by_card_hand` (card_id → {hand → BattingCardRatings}). - Rewrite `get_bratings(player_id)` to do O(1) dict lookups instead of DB calls. DB round trips reduced from O(3N) per lineup call to O(2), regardless of roster size. Error behaviour (AttributeError when no card/rating exists) is preserved identically for the existing try/except callers. ## Test plan No automated test suite. Verified by reading back the modified function — logic is equivalent to original; only the data source changed from DB queries to pre-built dicts. ## Other observations - Lines 544–576 contain a large commented-out block of dead code (an old batch approach that was never wired up). Out of scope for this PR but could be removed.

cal added 1 commit 2026-03-03 23:34:33 +00:00

fix: batch BattingCard/BattingCardRatings lookups in lineup builder (#18 ) 6f8da16e00

Replace per-player get_or_none() calls in get_bratings() with two bulk
SELECT queries before the position loop, keyed by player_id and card+hand.
This reduces DB round trips from O(3N) to O(2) for all lineup difficulties.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

cal referenced this pull request

2026-03-03 23:34:47 +00:00

Batch BattingCard/BattingCardRatings lookups in lineup endpoints #18

cal added the

ai-reviewing

label 2026-03-03 23:46:21 +00:00

cal removed the

ai-reviewing

label 2026-03-05 03:20:22 +00:00

cal added the

ai-reviewing

label 2026-03-05 04:22:05 +00:00

cal commented

2026-03-05 04:23:25 +00:00

AI Code Review

Note: Posted as comment — Gitea blocks self-review approvals.

Files Reviewed

app/routers_v2/teams.py (modified)

Findings

Correctness

Batch fetch logic is correct. backup_players is None edge case is handled explicitly before the batch query — the two-branch if/else avoids a TypeError that would occur passing None to Peewee's << operator.
Error behaviour is preserved identically. When player_id has no BattingCard, _batting_cards_by_player.get(player_id) returns None, card_ratings becomes {}, vl_ratings/vr_ratings are None, and None.obp raises AttributeError — exactly matching the original get_or_none path. Callers' try/except AttributeError blocks continue to work unchanged.
Dict key types are consistent. {bc.player_id: bc for bc in _batch_bcards} uses the int FK value; all callers pass x.player_id or x.player.player_id — same type.
Peewee << list(model_instances) is valid — Peewee resolves model instances to their PKs for the IN clause.
Pre-existing bug (not introduced by this PR): Line 419 calls get_bratings(x.player_id) twice — once inside try, once redundantly in the else block. Worth a follow-up issue.

Security

No issues. Peewee's << (IN) operator is parameterised — no injection risk.

Style & Conventions

Underscore-prefixed names (_batch_bcards, _batting_cards_by_player, _ratings_by_card_hand) clearly signal intent as closure-captured intermediates. Consistent with the rest of the file.
Added comment explaining the batch rationale is helpful.

Suggestions

The commented-out dead code block at lines 544–576 (noted in the PR body) could be removed in a follow-up — the author correctly flagged it as out of scope.

Verdict: APPROVED

Clean, focused, correct. DB round trips reduced from O(3N) to O(2) as claimed. No scope creep, no security issues, error behaviour preserved. Ready to merge.

Automated review by Claude PR Reviewer

## AI Code Review > Note: Posted as comment — Gitea blocks self-review approvals. ### Files Reviewed - `app/routers_v2/teams.py` (modified) ### Findings #### Correctness - **Batch fetch logic is correct.** `backup_players is None` edge case is handled explicitly before the batch query — the two-branch `if/else` avoids a `TypeError` that would occur passing `None` to Peewee's `<<` operator. - **Error behaviour is preserved identically.** When `player_id` has no BattingCard, `_batting_cards_by_player.get(player_id)` returns `None`, `card_ratings` becomes `{}`, `vl_ratings`/`vr_ratings` are `None`, and `None.obp` raises `AttributeError` — exactly matching the original `get_or_none` path. Callers' `try/except AttributeError` blocks continue to work unchanged. - **Dict key types are consistent.** `{bc.player_id: bc for bc in _batch_bcards}` uses the int FK value; all callers pass `x.player_id` or `x.player.player_id` — same type. - **Peewee `<< list(model_instances)`** is valid — Peewee resolves model instances to their PKs for the `IN` clause. - **Pre-existing bug (not introduced by this PR):** Line 419 calls `get_bratings(x.player_id)` twice — once inside `try`, once redundantly in the `else` block. Worth a follow-up issue. #### Security - No issues. Peewee's `<<` (IN) operator is parameterised — no injection risk. #### Style & Conventions - Underscore-prefixed names (`_batch_bcards`, `_batting_cards_by_player`, `_ratings_by_card_hand`) clearly signal intent as closure-captured intermediates. Consistent with the rest of the file. - Added comment explaining the batch rationale is helpful. #### Suggestions - The commented-out dead code block at lines 544–576 (noted in the PR body) could be removed in a follow-up — the author correctly flagged it as out of scope. ### Verdict: APPROVED Clean, focused, correct. DB round trips reduced from O(3N) to O(2) as claimed. No scope creep, no security issues, error behaviour preserved. Ready to merge. --- *Automated review by Claude PR Reviewer*

cal added the

ai-reviewed

label 2026-03-05 04:23:47 +00:00

cal force-pushed ai/paper-dynasty-database#18 from 6f8da16e00 to 62b205bde2

2026-03-05 21:31:21 +00:00

Compare

cal merged commit f1769966a0 into next-release

2026-03-07 03:16:13 +00:00

cal deleted branch ai/paper-dynasty-database#18

2026-03-07 03:16:14 +00:00

cal referenced this issue from a commit

2026-03-07 03:16:14 +00:00

Merge pull request 'fix: batch BattingCard/BattingCardRatings lookups in lineup builder (#18)' (#45) from ai/paper-dynasty-database#18 into next-release

Sign in to join this conversation.

No reviewers

No Milestone

No project

No Assignees

1 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: cal/paper-dynasty-database#45