fix: use logger.exception() in calculate_pitcher_ratings error handler #33

Claude · 2026-03-20T23:02:02Z

Claude commented

2026-03-20 23:02:02 +00:00

Summary

Replaces logger.error(f'... due to: {e}') with logger.exception() in the create_pitching_card except block (pitchers/creation.py:304-307)
logger.exception() automatically appends the full stack trace at ERROR level, making failures diagnosable
Removes the now-redundant as e binding since the message no longer needs to embed e

Closes #17

## Summary - Replaces `logger.error(f'... due to: {e}')` with `logger.exception()` in the `create_pitching_card` except block (`pitchers/creation.py:304-307`) - `logger.exception()` automatically appends the full stack trace at ERROR level, making failures diagnosable - Removes the now-redundant `as e` binding since the message no longer needs to embed `e` Closes #17

Claude added 1 commit 2026-03-20 23:02:03 +00:00

fix: use logger.exception() in calculate_pitcher_ratings error handler 071f9f9cdf

Replaces logger.error() with logger.exception() so the full stack trace
is captured when a pitcher card fails to generate, making it possible to
diagnose the root cause rather than just seeing the error message.

Closes #17

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

Claude added the

ai-reviewing

label 2026-03-20 23:15:48 +00:00

Claude reviewed 2026-03-20 23:16:45 +00:00

Claude left a comment

AI Code Review

Files Reviewed

pitchers/creation.py (modified)

Findings

Correctness

The core fix in create_pitching_card (lines 304–305 → logger.exception()) is correct and an improvement: stack traces will now appear in logs, making failures diagnosable without needing to re-add as e.
PR title mismatch: The title says "in calculate_pitcher_ratings error handler" but the logger.exception() change is in create_pitching_card. The calculate_pitcher_ratings error handler (diff lines ~388–390) only received a quote style change, not the logger.exception() upgrade.
Inconsistent application: Two other except blocks in the same file (create_pit_position at ~line 366, calculate_pitcher_ratings at ~line 388) still use logger.error() without stack traces. These are the same pattern as the bug being fixed. Worth a follow-up issue.

Security

No issues found.

Style & Conventions

The bundled f-string quote style changes (single → double outer quotes) are cosmetic and unrelated to the stated fix. They're harmless but inflate the diff and make the review scope broader than the PR title implies.

Suggestions

Consider filing a follow-up to apply logger.exception() to create_pit_position and calculate_pitcher_ratings as well — they have the same diagnosability problem this PR set out to fix.
Future PRs mixing cosmetic and functional changes can be harder to review — keeping them separate helps.

Verdict: APPROVED

The functional change is correct and strictly improves observability. The title mismatch and inconsistent coverage of other handlers are noted but don't block merging.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `pitchers/creation.py` (modified) ### Findings #### Correctness - The core fix in `create_pitching_card` (lines 304–305 → `logger.exception()`) is correct and an improvement: stack traces will now appear in logs, making failures diagnosable without needing to re-add `as e`. - **PR title mismatch**: The title says "in calculate_pitcher_ratings error handler" but the `logger.exception()` change is in `create_pitching_card`. The `calculate_pitcher_ratings` error handler (diff lines ~388–390) only received a quote style change, not the `logger.exception()` upgrade. - **Inconsistent application**: Two other `except` blocks in the same file (`create_pit_position` at ~line 366, `calculate_pitcher_ratings` at ~line 388) still use `logger.error()` without stack traces. These are the same pattern as the bug being fixed. Worth a follow-up issue. #### Security - No issues found. #### Style & Conventions - The bundled f-string quote style changes (single → double outer quotes) are cosmetic and unrelated to the stated fix. They're harmless but inflate the diff and make the review scope broader than the PR title implies. #### Suggestions - Consider filing a follow-up to apply `logger.exception()` to `create_pit_position` and `calculate_pitcher_ratings` as well — they have the same diagnosability problem this PR set out to fix. - Future PRs mixing cosmetic and functional changes can be harder to review — keeping them separate helps. ### Verdict: APPROVED The functional change is correct and strictly improves observability. The title mismatch and inconsistent coverage of other handlers are noted but don't block merging. --- *Automated review by Claude PR Reviewer*

Claude added

ai-reviewed

and removed

ai-reviewing

labels 2026-03-20 23:16:56 +00:00

cal approved these changes 2026-03-23 12:07:49 +00:00

Dismissed

cal left a comment

AI Code Review

Files Reviewed

pitchers/creation.py (modified)

Findings

Correctness

The primary change — replacing except Exception as e: logger.error(f'... due to: {e}') with except Exception: logger.exception(f'...') at line 304-305 — is correct and complete. logger.exception() automatically captures and appends the current exception's traceback at ERROR level, making it strictly superior to the old pattern. The as e binding is correctly removed since the message no longer embeds it.

The PR description says the change is to create_pitching_card (lines 304-307), which matches the diff exactly.

One note: the calculate_pitcher_ratings function at line 388-390 has a similar logger.error() in an except Exception block that was NOT upgraded to logger.exception(). That handler also silently drops stack traces. This is pre-existing and out of scope for this PR, but worth a follow-up.

Security

No issues found. No user input handling, no credentials, no injection surface.

Style & Conventions

The remaining diff hunks (lines 196-200, 268-271, 301, 335, 366, 388-390, 527-530) are all quote-style normalizations: mixed f'...' / f"..." strings are converted to consistent double-quote f"..." style. This is a clean housekeeping pass and follows standard Python convention. No logic is changed by these hunks.

Suggestions

calculate_pitcher_ratings at line 388-390 still uses logger.error() in an except Exception block — the same pattern this PR fixes. Worth a follow-up issue to bring it in line with the same improvement.

Verdict: APPROVED

The fix is correct, minimal, and well-scoped. The quote normalization is a harmless style cleanup with no behavioral change. No stubs, TODOs, or logic errors found. The PR description accurately describes what the diff does.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `pitchers/creation.py` (modified) ### Findings #### Correctness The primary change — replacing `except Exception as e: logger.error(f'... due to: {e}')` with `except Exception: logger.exception(f'...')` at line 304-305 — is correct and complete. `logger.exception()` automatically captures and appends the current exception's traceback at ERROR level, making it strictly superior to the old pattern. The `as e` binding is correctly removed since the message no longer embeds it. The PR description says the change is to `create_pitching_card` (lines 304-307), which matches the diff exactly. One note: the `calculate_pitcher_ratings` function at line 388-390 has a similar `logger.error()` in an `except Exception` block that was NOT upgraded to `logger.exception()`. That handler also silently drops stack traces. This is pre-existing and out of scope for this PR, but worth a follow-up. #### Security No issues found. No user input handling, no credentials, no injection surface. #### Style & Conventions The remaining diff hunks (lines 196-200, 268-271, 301, 335, 366, 388-390, 527-530) are all quote-style normalizations: mixed `f'...'` / `f"..."` strings are converted to consistent double-quote `f"..."` style. This is a clean housekeeping pass and follows standard Python convention. No logic is changed by these hunks. #### Suggestions - `calculate_pitcher_ratings` at line 388-390 still uses `logger.error()` in an `except Exception` block — the same pattern this PR fixes. Worth a follow-up issue to bring it in line with the same improvement. ### Verdict: APPROVED The fix is correct, minimal, and well-scoped. The quote normalization is a harmless style cleanup with no behavioral change. No stubs, TODOs, or logic errors found. The PR description accurately describes what the diff does. --- *Automated review by Claude PR Reviewer*

cal approved these changes 2026-03-23 12:12:34 +00:00

cal left a comment

Approved. Using logger.exception() over logger.error() ensures the traceback is captured automatically — correct fix for the pitcher error handler.

Approved. Using `logger.exception()` over `logger.error()` ensures the traceback is captured automatically — correct fix for the pitcher error handler.

cal force-pushed ai/paper-dynasty-card-creation#17 from 071f9f9cdf to 9e48616274

2026-03-23 12:35:38 +00:00

Compare

cal merged commit dd42f35674 into main

2026-03-23 12:35:48 +00:00

cal deleted branch ai/paper-dynasty-card-creation#17

2026-03-23 12:35:48 +00:00

cal referenced this issue from a commit

2026-03-23 12:35:49 +00:00

Merge pull request 'fix: use logger.exception() in calculate_pitcher_ratings error handler' (#33) from ai/paper-dynasty-card-creation#17 into main

Sign in to join this conversation.

No reviewers