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

fix: use max() for pitcher OPS split weighting (#6) #60

Merged
cal merged 1 commits from ai/paper-dynasty-database#6 into next-release 2026-03-10 14:43:00 +00:00
Conversation 1 Commits 1 Files Changed 1 +5 -3
cal commented 2026-03-07 22:33:16 +00:00
Owner
Copy Link

Summary

Resolves #6.

The get_total_ops formula (ops_vr + ops_vl + min(ops_vl, ops_vr)) / 3 weighted the better platoon split (lower OPS allowed), effectively hiding platoon weaknesses. Since starting pitchers face both LHH and RHH in a game, the correct behaviour is to penalise the weaker split (higher OPS allowed).

Changed min → max in both locations:

  • get_total_ops (previously line 620, now line 621) — used by sort_bullpen
  • sort_starters inner get_total_ops (previously line 701, now line 703)

Replaced the # TODO: should this be max?? comment with a brief explanatory note.

Files changed

  • app/routers_v2/teams.py

Test results

No test suite in this repo. Both changed lines verified by reading back the file.

## Summary Resolves #6. The `get_total_ops` formula `(ops_vr + ops_vl + min(ops_vl, ops_vr)) / 3` weighted the **better** platoon split (lower OPS allowed), effectively hiding platoon weaknesses. Since starting pitchers face both LHH and RHH in a game, the correct behaviour is to penalise the **weaker** split (higher OPS allowed). Changed `min` → `max` in both locations: - `get_total_ops` (previously line 620, now line 621) — used by `sort_bullpen` - `sort_starters` inner `get_total_ops` (previously line 701, now line 703) Replaced the `# TODO: should this be max??` comment with a brief explanatory note. ## Files changed - `app/routers_v2/teams.py` ## Test results No test suite in this repo. Both changed lines verified by reading back the file.
cal added 1 commit 2026-03-07 22:33:17 +00:00 
Starters face both LHH and RHH, so the OPS aggregation formula should
penalise the weaker platoon split (higher OPS allowed) rather than
reward the stronger one. Changed min(ops_vl, ops_vr) → max(ops_vl, ops_vr)
in both get_total_ops (line 621) and sort_starters (line 703) and
replaced the TODO comment with an explanatory note.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
cal added the
ai-reviewing
 label 2026-03-07 22:45:33 +00:00
cal commented 2026-03-07 22:46:50 +00:00
Author
Owner
Copy Link

AI Code Review

Files Reviewed

  • app/routers_v2/teams.py (modified)

Findings

Correctness

  • Fix is mathematically correct. The old formula (ops_vr + ops_vl + min(ops_vl, ops_vr)) / 3 gave extra weight to the pitcher's better platoon split, underweighting vulnerabilities. The new formula (ops_vr + ops_vl + max(ops_vl, ops_vr)) / 3 penalises the weaker split — appropriate since starters face both handedness in a game.
  • Both changed sites (sort_pitchers outer scope and sort_starters inner scope) are updated consistently.
  • The vlval is None or vrval is None guard returning float("inf") is unchanged and correct.
  • Edge case where ops_vl == ops_vr: max() and min() return identical values, so no behavioural difference — safe.

Minor inaccuracy in the PR description: the outer get_total_ops is described as "used by sort_bullpen" but the enclosing function is sort_pitchers. Doesn't affect correctness.

Security

  • No issues. Pure arithmetic on ORM-fetched float values.

Style & Conventions

  • No issues. Replacement comments are clear and correctly explain the intent.

Suggestions (non-blocking)

  • Lines 580–584: Dead-commented code from an older approach still uses min(). Could be cleaned up in a follow-up, but out of scope here.
  • get_total_ops is defined twice — once inside sort_pitchers and once inside sort_starters. Pre-existing duplication, out of scope, but a candidate for future refactor.

Verdict: APPROVED

Minimal, focused, and correct. The min → max change is well-reasoned and both locations updated consistently. No scope creep, no security concerns.

Automated review by Claude PR Reviewer (posted as comment — Gitea self-review restriction applies)

## AI Code Review ### Files Reviewed - `app/routers_v2/teams.py` (modified) ### Findings #### Correctness - **Fix is mathematically correct.** The old formula `(ops_vr + ops_vl + min(ops_vl, ops_vr)) / 3` gave extra weight to the pitcher's better platoon split, underweighting vulnerabilities. The new formula `(ops_vr + ops_vl + max(ops_vl, ops_vr)) / 3` penalises the weaker split — appropriate since starters face both handedness in a game. - Both changed sites (`sort_pitchers` outer scope and `sort_starters` inner scope) are updated consistently. - The `vlval is None or vrval is None` guard returning `float("inf")` is unchanged and correct. - Edge case where `ops_vl == ops_vr`: `max()` and `min()` return identical values, so no behavioural difference — safe. Minor inaccuracy in the PR description: the outer `get_total_ops` is described as "used by `sort_bullpen`" but the enclosing function is `sort_pitchers`. Doesn't affect correctness. #### Security - No issues. Pure arithmetic on ORM-fetched float values. #### Style & Conventions - No issues. Replacement comments are clear and correctly explain the intent. #### Suggestions (non-blocking) - Lines 580–584: Dead-commented code from an older approach still uses `min()`. Could be cleaned up in a follow-up, but out of scope here. - `get_total_ops` is defined twice — once inside `sort_pitchers` and once inside `sort_starters`. Pre-existing duplication, out of scope, but a candidate for future refactor. ### Verdict: APPROVED Minimal, focused, and correct. The `min` → `max` change is well-reasoned and both locations updated consistently. No scope creep, no security concerns. --- *Automated review by Claude PR Reviewer (posted as comment — Gitea self-review restriction applies)*
cal added the
ai-reviewed
 label 2026-03-07 22:47:11 +00:00
cal removed the
ai-reviewing
 label 2026-03-10 14:42:09 +00:00
cal merged commit a66ef9bd7c into next-release 2026-03-10 14:43:00 +00:00
cal deleted branch ai/paper-dynasty-database#6 2026-03-10 14:43:01 +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#60
No description provided.
Delete Branch "ai/paper-dynasty-database#6"

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?