cal/paper-dynasty-database
1
0
Fork 0
Code Issues 7 Pull Requests 4 Actions Packages Projects Releases Wiki Activity

fix: guard against None rating objects in pitcher sorting functions (#13) #50

Merged
cal merged 1 commits from ai/paper-dynasty-database#13 into next-release 2026-03-05 03:29:49 +00:00
Conversation 0 Commits 1 Files Changed 1 +4
cal commented 2026-03-04 02:04:13 +00:00
Owner
Copy Link

Summary

  • Added None guards in get_total_ops inside sort_pitchers() (line ~600) and the inner sort_starters() closure (line ~679)
  • When PitchingCardRatings.get_or_none() returns None for either vs-L or vs-R rating, the function now returns float("inf") instead of raising AttributeError on .obp/.slg access
  • Pitchers with missing ratings sort to the end of the list (worst) rather than crashing the endpoint with a 500

Files Changed

  • app/routers_v2/teams.py

Test Results

No test suite in this repo. Changes verified by code review — the fix mirrors the pattern already used in the DH-finding path (try/except AttributeError), but uses an explicit None check which is cleaner for this use case.

Closes #13

## Summary - Added `None` guards in `get_total_ops` inside `sort_pitchers()` (line ~600) and the inner `sort_starters()` closure (line ~679) - When `PitchingCardRatings.get_or_none()` returns `None` for either vs-L or vs-R rating, the function now returns `float("inf")` instead of raising `AttributeError` on `.obp`/`.slg` access - Pitchers with missing ratings sort to the end of the list (worst) rather than crashing the endpoint with a 500 ## Files Changed - `app/routers_v2/teams.py` ## Test Results No test suite in this repo. Changes verified by code review — the fix mirrors the pattern already used in the DH-finding path (`try/except AttributeError`), but uses an explicit `None` check which is cleaner for this use case. Closes #13
cal added 1 commit 2026-03-04 02:04:14 +00:00 
Add None checks for vlval/vrval in get_total_ops inside sort_pitchers()
and sort_starters(). Returns float("inf") when ratings are missing so
pitchers without ratings sort to the end rather than raising AttributeError.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
cal added the
ai-reviewing
 label 2026-03-04 02:16:42 +00:00
cal reviewed 2026-03-04 02:17:58 +00:00
cal left a comment
Author
Owner
Copy Link

AI Code Review

Files Reviewed

  • app/routers_v2/teams.py (modified)

Findings

Correctness

  • Both get_total_ops inner functions — in sort_pitchers() (~line 593) and the sort_starters() closure (~line 670) — now correctly guard against None before accessing .obp/.slg.
  • float("inf") is the right sentinel: sort_values(by="total_ops") sorts ascending (lower OPS = better pitcher), so inf correctly pushes pitchers with missing ratings to the end rather than crashing with a 500.
  • Both occurrences of the pattern are fixed — no partial coverage.

Security

  • No issues. No user input flows into the new code paths.

Style & Conventions

  • Explicit None check is cleaner than try/except AttributeError for a function with a well-defined None-or-object return contract like get_or_none(). The PR author notes the existing DH path uses try/except — this slight inconsistency is acceptable since the explicit check is the more Pythonic approach here.
  • No unnecessary changes, no scope creep.

Suggestions

  • The existing # TODO: should this be max?? comment at line ~605 is unrelated to this PR but worth addressing in a follow-up — the two get_total_ops implementations are now nearly identical and could be consolidated into a shared helper to reduce duplication.

Verdict: APPROVED

Minimal, focused fix. Both affected call sites are patched, the sentinel value is semantically correct, and the code follows project conventions. ✅

Automated review by Claude PR Reviewer (posted as COMMENT — Gitea prevents self-approval)

## AI Code Review ### Files Reviewed - `app/routers_v2/teams.py` (modified) ### Findings #### Correctness - Both `get_total_ops` inner functions — in `sort_pitchers()` (~line 593) and the `sort_starters()` closure (~line 670) — now correctly guard against `None` before accessing `.obp`/`.slg`. - `float("inf")` is the right sentinel: `sort_values(by="total_ops")` sorts ascending (lower OPS = better pitcher), so `inf` correctly pushes pitchers with missing ratings to the end rather than crashing with a 500. - Both occurrences of the pattern are fixed — no partial coverage. #### Security - No issues. No user input flows into the new code paths. #### Style & Conventions - Explicit `None` check is cleaner than `try/except AttributeError` for a function with a well-defined `None`-or-object return contract like `get_or_none()`. The PR author notes the existing DH path uses `try/except` — this slight inconsistency is acceptable since the explicit check is the more Pythonic approach here. - No unnecessary changes, no scope creep. #### Suggestions - The existing `# TODO: should this be max??` comment at line ~605 is unrelated to this PR but worth addressing in a follow-up — the two `get_total_ops` implementations are now nearly identical and could be consolidated into a shared helper to reduce duplication. ### Verdict: APPROVED Minimal, focused fix. Both affected call sites are patched, the sentinel value is semantically correct, and the code follows project conventions. ✅ --- *Automated review by Claude PR Reviewer (posted as COMMENT — Gitea prevents self-approval)*
cal added
ai-reviewed
 and removed
ai-reviewing
 labels 2026-03-04 02:18:22 +00:00
cal force-pushed ai/paper-dynasty-database#13 from 8d1751c061 to 0948db0b49 2026-03-05 03:25:31 +00:00 Compare
cal merged commit d1728a804f into next-release 2026-03-05 03:29:49 +00:00
cal deleted branch ai/paper-dynasty-database#13 2026-03-05 03:29:49 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
ai-changes-requested

   
ai-failed

   
ai-merged

PR merged by AI ops agent

  
ai-pr-opened

   
ai-reviewed

   
ai-reviewing

   
ai-reviewing

   
ai-working

   
bug

   
enhancement

   
evolution

Card Evolution system

  
performance

   
phase-0

Render Pipeline Optimization

  
phase-1a

Schema & Data Foundation

  
phase-1b

API & Formula Engine

  
phase-1c

Bot Integration

  
phase-1d

Deployment

  
security

   
tech-debt

   
todo
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
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
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#50
No description provided.
Delete Branch "ai/paper-dynasty-database#13"

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?