fix: calculate lob_2outs and rbipercent in SeasonPitchingStats (#28) #59

Merged
cal merged 1 commits from ai/major-domo-database-28 into next-release 2026-03-10 14:24:07 +00:00
Owner

Summary

  • lob_2outs and rbipercent in SeasonPitchingStats were hardcoded to 0.0 in the update_season_pitching_stats() INSERT query
  • Added SQL expressions to the pitching_stats CTE to compute both from stratplay data, using the same field semantics as the batting stats endpoint

What changed

app/dependencies.pyupdate_season_pitching_stats() CTE extended with two new computed columns:

  • lob_2outs: sum of runners on 1st/2nd/3rd that were neither scored (on_*_final != 4) nor put out (on_*_final IS NOT NULL) when the pitcher's play resulted in the 3rd out (starting_outs + outs = 3). Mirrors the batting-side count_lo*_3out logic.

  • rbipercent: (SUM(rbi) - SUM(homerun)) / total_runners_on_base — RBI allowed (excluding HR, which don't require runners) per runner opportunity. Returns 0.000 if no runners were on base.

The INSERT SELECT now references ps.lob_2outs, ps.rbipercent instead of 0.0, 0.0.

Other observations

  • app/routers_v3/stratplay/pitching.py:343 still returns "lob_2outs": 0 and "rbi%": 0 with comments "Not available in current implementation" — that endpoint uses a different live aggregation path and is out of scope for this issue, but should be a follow-up.
  • The SeasonPitchingStats model defines both fields as FloatField() (non-nullable); the new expressions produce numeric values, consistent with the schema.
## Summary - `lob_2outs` and `rbipercent` in `SeasonPitchingStats` were hardcoded to `0.0` in the `update_season_pitching_stats()` INSERT query - Added SQL expressions to the `pitching_stats` CTE to compute both from `stratplay` data, using the same field semantics as the batting stats endpoint ## What changed **`app/dependencies.py`** — `update_season_pitching_stats()` CTE extended with two new computed columns: - **`lob_2outs`**: sum of runners on 1st/2nd/3rd that were neither scored (`on_*_final != 4`) nor put out (`on_*_final IS NOT NULL`) when the pitcher's play resulted in the 3rd out (`starting_outs + outs = 3`). Mirrors the batting-side `count_lo*_3out` logic. - **`rbipercent`**: `(SUM(rbi) - SUM(homerun)) / total_runners_on_base` — RBI allowed (excluding HR, which don't require runners) per runner opportunity. Returns `0.000` if no runners were on base. The INSERT SELECT now references `ps.lob_2outs, ps.rbipercent` instead of `0.0, 0.0`. ## Other observations - `app/routers_v3/stratplay/pitching.py:343` still returns `"lob_2outs": 0` and `"rbi%": 0` with comments "Not available in current implementation" — that endpoint uses a different live aggregation path and is out of scope for this issue, but should be a follow-up. - The `SeasonPitchingStats` model defines both fields as `FloatField()` (non-nullable); the new expressions produce numeric values, consistent with the schema.
cal added 1 commit 2026-03-08 14:35:08 +00:00
fix: calculate lob_2outs and rbipercent in SeasonPitchingStats (#28)
All checks were successful
Build Docker Image / build (pull_request) Successful in 2m28s
926b03971b
Both fields were hardcoded to 0.0 in the INSERT. Added SQL expressions
to the pitching_stats CTE to calculate them from stratplay data, using
the same logic as the batting stats endpoint.

- lob_2outs: count of runners stranded when pitcher recorded the 3rd out
- rbipercent: RBI allowed (excluding HR) per runner opportunity

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
cal added the
ai-reviewing
label 2026-03-08 14:45:36 +00:00
cal reviewed 2026-03-08 14:48:44 +00:00
cal left a comment
Author
Owner

AI Code Review

Files Reviewed

  • app/dependencies.py (modified) — update_season_pitching_stats() CTE and INSERT

Findings

Correctness

  • Fix is correct. The SQL expressions for lob_2outs and rbipercent faithfully replace the hardcoded 0.0, 0.0 placeholders.
  • lob_2outs logic verified. The condition on_*_final IS NOT NULL AND on_*_final != 4 AND starting_outs + outs = 3 exactly mirrors the Peewee ORM equivalent in routers_v3/stratplay/batting.py (count_lo*_3out aliases). IS NOT NULL = runner still on base; != 4 = didn't score; starting_outs + outs = 3 = inning-ending play. Semantics are correct.
  • rbipercent logic verified. (SUM(rbi) - SUM(homerun)) / total_runners_on_base correctly uses start-of-play runner fields (on_first/on_second/on_third, not _final) for the denominator — runners on base represent opportunities. Division-by-zero is properly guarded with ELSE 0.000.
  • Column ordering correct. INSERT column list (lob_2outs, rbipercent, re24) matches SELECT values (ps.lob_2outs, ps.rbipercent, COALESCE(ps.re24 * -1, 0.0)). ON CONFLICT UPDATE SET was already correct in the base branch.
  • Field types. lob_2outs SQL produces an integer sum; model is FloatField(). PostgreSQL auto-casts on INSERT — no issue. rbipercent produces DECIMAL via ROUND(..., 3) — consistent with FloatField().
  • on_first_final field existence confirmedIntegerField(null=True) in db_engine.py:2156. All referenced fields (on_first, on_second, on_third, on_*_final, starting_outs, outs, rbi, homerun) are valid stratplay columns.

Security

  • Pre-existing: Hardcoded Discord webhook URL in send_webhook_message (line 484) — out of scope for this PR, tracked separately. The cosmetic reformatting of the surrounding block does not introduce or worsen this issue.
  • No new security issues introduced.

Style & Conventions

  • Bulk of the diff is ruff/black reformatting (single→double quotes, trailing commas, line wrapping). Expected per project setup — not a concern.
  • New SQL expressions are commented and readable.

Suggestions

  • The lob_2outs model field is FloatField() but the computed SQL value is always a non-negative integer. This is harmless (PostgreSQL casts it), but if the column were redefined as IntegerField it would better represent the data type. Not blocking.
  • app/routers_v3/stratplay/pitching.py:343 still hardcodes "lob_2outs": 0 and "rbi%": 0 — acknowledged in the PR body as a follow-up. Worth creating a separate issue to track.

Verdict: APPROVED

The implementation is correct and consistent with the existing batting-side field semantics. Both SQL expressions are logically sound, the INSERT/SELECT column alignment is verified, edge cases (empty runners, division by zero) are handled, and no new security issues are introduced.


Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/dependencies.py` (modified) — `update_season_pitching_stats()` CTE and INSERT ### Findings #### Correctness - **Fix is correct.** The SQL expressions for `lob_2outs` and `rbipercent` faithfully replace the hardcoded `0.0, 0.0` placeholders. - **`lob_2outs` logic verified.** The condition `on_*_final IS NOT NULL AND on_*_final != 4 AND starting_outs + outs = 3` exactly mirrors the Peewee ORM equivalent in `routers_v3/stratplay/batting.py` (`count_lo*_3out` aliases). `IS NOT NULL` = runner still on base; `!= 4` = didn't score; `starting_outs + outs = 3` = inning-ending play. Semantics are correct. - **`rbipercent` logic verified.** `(SUM(rbi) - SUM(homerun)) / total_runners_on_base` correctly uses start-of-play runner fields (`on_first`/`on_second`/`on_third`, not `_final`) for the denominator — runners on base represent opportunities. Division-by-zero is properly guarded with `ELSE 0.000`. - **Column ordering correct.** INSERT column list (`lob_2outs, rbipercent, re24`) matches SELECT values (`ps.lob_2outs, ps.rbipercent, COALESCE(ps.re24 * -1, 0.0)`). ON CONFLICT UPDATE SET was already correct in the base branch. - **Field types.** `lob_2outs` SQL produces an integer sum; model is `FloatField()`. PostgreSQL auto-casts on INSERT — no issue. `rbipercent` produces `DECIMAL` via `ROUND(..., 3)` — consistent with `FloatField()`. - **`on_first_final` field existence confirmed** — `IntegerField(null=True)` in `db_engine.py:2156`. All referenced fields (`on_first`, `on_second`, `on_third`, `on_*_final`, `starting_outs`, `outs`, `rbi`, `homerun`) are valid `stratplay` columns. #### Security - **Pre-existing:** Hardcoded Discord webhook URL in `send_webhook_message` (line 484) — out of scope for this PR, tracked separately. The cosmetic reformatting of the surrounding block does not introduce or worsen this issue. - No new security issues introduced. #### Style & Conventions - Bulk of the diff is ruff/black reformatting (single→double quotes, trailing commas, line wrapping). Expected per project setup — not a concern. - New SQL expressions are commented and readable. #### Suggestions - The `lob_2outs` model field is `FloatField()` but the computed SQL value is always a non-negative integer. This is harmless (PostgreSQL casts it), but if the column were redefined as `IntegerField` it would better represent the data type. Not blocking. - `app/routers_v3/stratplay/pitching.py:343` still hardcodes `"lob_2outs": 0` and `"rbi%": 0` — acknowledged in the PR body as a follow-up. Worth creating a separate issue to track. ### Verdict: APPROVED The implementation is correct and consistent with the existing batting-side field semantics. Both SQL expressions are logically sound, the INSERT/SELECT column alignment is verified, edge cases (empty runners, division by zero) are handled, and no new security issues are introduced. --- *Automated review by Claude PR Reviewer*
cal added
ai-reviewed
and removed
ai-reviewing
labels 2026-03-08 14:49:24 +00:00
cal changed target branch from main to next-release 2026-03-10 14:23:59 +00:00
cal merged commit c5382d9e8a into next-release 2026-03-10 14:24:07 +00:00
cal deleted branch ai/major-domo-database-28 2026-03-10 14:24:07 +00:00
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/major-domo-database#59
No description provided.