cal/paper-dynasty-card-creation
1
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases Wiki Activity

fix: use archetype role ratings in pitcher card creation (#11) #46

Merged
cal merged 1 commits from ai/paper-dynasty-card-creation#11 into main 2026-03-23 12:41:22 +00:00
Conversation 0 Commits 1 Files Changed 1 +12 -7
Claude commented 2026-03-21 14:03:05 +00:00
Collaborator
Copy Link

Closes #11

Summary

  • starter_rating, relief_rating, and closer_rating were hardcoded stubs (5/5/None) in create_pitching_card, making every custom pitcher card identical regardless of archetype
  • The chosen PitcherArchetype already carries these values (e.g. Ace: 9/3/None, Closer: 2/9/9)
  • Fix stores them in card_data when the pitcher workflow initialises its dict (alongside ratings), then reads them in create_pitching_card when writing to the database

Files changed

  • custom_cards/interactive_creator.py — two-site change: populate card_data with archetype role ratings; consume them in create_pitching_card

Other observations

  • The auto-formatter (Ruff/Black) reformatted three f-string arithmetic expressions (+ spacing) on unrelated lines; these are cosmetic and do not affect behaviour.
  • Several pre-existing Pyright warnings exist in this file (unrelated to this issue): unused variables in stub methods, and None-subscriptable issues from db_get/db_post return values.
Closes #11 ## Summary - `starter_rating`, `relief_rating`, and `closer_rating` were hardcoded stubs (`5`/`5`/`None`) in `create_pitching_card`, making every custom pitcher card identical regardless of archetype - The chosen `PitcherArchetype` already carries these values (e.g. Ace: 9/3/None, Closer: 2/9/9) - Fix stores them in `card_data` when the pitcher workflow initialises its dict (alongside `ratings`), then reads them in `create_pitching_card` when writing to the database ## Files changed - `custom_cards/interactive_creator.py` — two-site change: populate `card_data` with archetype role ratings; consume them in `create_pitching_card` ## Other observations - The auto-formatter (Ruff/Black) reformatted three f-string arithmetic expressions (`+` spacing) on unrelated lines; these are cosmetic and do not affect behaviour. - Several pre-existing Pyright warnings exist in this file (unrelated to this issue): unused variables in stub methods, and `None`-subscriptable issues from `db_get`/`db_post` return values.
Claude added 1 commit 2026-03-21 14:03:05 +00:00 
Closes #11

`starter_rating`, `relief_rating`, and `closer_rating` were hardcoded
stubs (5/5/None) in `create_pitching_card`. The chosen `PitcherArchetype`
already carries these values; now they are propagated through `card_data`
when the pitcher workflow builds its initial dict and consumed correctly
when writing the pitching card record to the database.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude added the
ai-reviewing
 label 2026-03-21 14:15:49 +00:00
Claude reviewed 2026-03-21 14:16:47 +00:00
Claude left a comment
Author
Collaborator
Copy Link

AI Code Review

Files Reviewed

  • custom_cards/interactive_creator.py (modified)

Findings

Correctness

  • The fix is correct and complete. PitcherArchetype declares starter_rating: int, relief_rating: int, and closer_rating: int | None = None — all three attributes are valid and present on every archetype instance.
  • The data flows cleanly: archetype values are stored in card_data at initialisation (Step 3 of create_player), survive the review_and_tweak loop unchanged (both tweak_archetype and manual_adjustments are stubs that return card_data unmodified), and are correctly consumed in create_pitching_card when building the API payload.
  • closer_rating=None is handled correctly — the API payload always accepted None here (the original stub used it as the default), so no schema change is needed.

Security

  • No issues. The archetype values originate from a hardcoded constant dictionary (PITCHER_ARCHETYPES), not from user input, so there is no injection or validation concern.

Style & Conventions

  • The three f-string whitespace reformats (+ → +) are cosmetic PEP 8 corrections and do not affect behaviour.
  • Minor: the comment block at the top of create_pitching_card still reads "For now, use defaults — can be enhanced later" after the TODO stubs have been replaced. This is a stale comment worth cleaning up but it does not affect correctness.

Suggestions

  • The stale comment in create_pitching_card (lines 572–573) could be updated to "Role ratings sourced from archetype" to reflect the new behaviour.

Verdict: COMMENT (LGTM)

Minimal, targeted fix that correctly wires the archetype role ratings through the pipeline. The only follow-up is the stale comment, which is not a blocker.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `custom_cards/interactive_creator.py` (modified) ### Findings #### Correctness - The fix is correct and complete. `PitcherArchetype` declares `starter_rating: int`, `relief_rating: int`, and `closer_rating: int | None = None` — all three attributes are valid and present on every archetype instance. - The data flows cleanly: archetype values are stored in `card_data` at initialisation (Step 3 of `create_player`), survive the `review_and_tweak` loop unchanged (both `tweak_archetype` and `manual_adjustments` are stubs that return `card_data` unmodified), and are correctly consumed in `create_pitching_card` when building the API payload. - `closer_rating=None` is handled correctly — the API payload always accepted `None` here (the original stub used it as the default), so no schema change is needed. #### Security - No issues. The archetype values originate from a hardcoded constant dictionary (`PITCHER_ARCHETYPES`), not from user input, so there is no injection or validation concern. #### Style & Conventions - The three f-string whitespace reformats (`+` → ` + `) are cosmetic PEP 8 corrections and do not affect behaviour. - Minor: the comment block at the top of `create_pitching_card` still reads *"For now, use defaults — can be enhanced later"* after the TODO stubs have been replaced. This is a stale comment worth cleaning up but it does not affect correctness. #### Suggestions - The stale comment in `create_pitching_card` (lines 572–573) could be updated to *"Role ratings sourced from archetype"* to reflect the new behaviour. ### Verdict: COMMENT (LGTM) Minimal, targeted fix that correctly wires the archetype role ratings through the pipeline. The only follow-up is the stale comment, which is not a blocker. --- *Automated review by Claude PR Reviewer*
Claude added
ai-reviewed
 and removed
ai-reviewing
 labels 2026-03-21 14:17:10 +00:00
cal approved these changes 2026-03-23 12:08:01 +00:00
Dismissed
cal left a comment
Owner
Copy Link

AI Code Review

Files Reviewed

  • custom_cards/interactive_creator.py (modified)

Findings

Correctness

No issues found. The fix is precise and complete:

  1. Population site (line ~182): card_data for the pitcher branch now stores starter_rating, relief_rating, and closer_rating directly from the chosen PitcherArchetype instance, alongside the existing ratings key.

  2. Consumption site (line ~583-585): The three hardcoded TODO stubs are replaced with reads from card_data, which is guaranteed to be populated before create_pitching_card is called (the flow is linear: Step 3 builds card_data, Step 4 runs the review/tweak loop, Step 5 calls create_database_records which calls create_pitching_card).

  3. Attribute existence confirmed: PitcherArchetype (defined in custom_cards/archetype_definitions.py lines 119-121) declares all three fields as typed dataclass attributes. Every archetype instance in the file carries concrete values — no attribute access can fail.

  4. Position assignment downstream is also correct: The CLAUDE.md notes that pitcher positions are assigned based on starter_rating >= 4 (SP) and closer_rating presence (CP). The archetype values follow the same convention (Ace: SR=9, Closer: SR=2/CR=9), so cards created via this path will receive the correct position assignments automatically.

Security

No issues found. No user input reaches the archetype fields — values are read from an immutable dataclass populated at archetype selection time.

Style & Conventions

  • The three f-string spacing changes (+ → +) are cosmetic formatter output (Ruff/Black). They are accurate and do not affect behaviour.
  • The stale comment # For now, use defaults - can be enhanced later at line 570-571 of create_pitching_card is now misleading — the method no longer uses defaults. This is a minor leftover but does not affect correctness.

Suggestions

  • The comment at line 570-571 (# Determine starter/relief/closer ratings based on archetype / # For now, use defaults - can be enhanced later) should be updated to reflect that values now come from card_data. Low priority — cosmetic only, not a blocker.
  • pd_cards/commands/custom.py lines 826-828 contain identical hardcoded starter_rating: 5 / relief_rating: 5 / closer_rating: None stubs in a separate YAML-based custom card path. That path is out of scope for this PR but worth tracking as a follow-up.

Verdict: APPROVED

The fix is correct, targeted, and complete. The two-site change properly threads archetype role ratings from selection through to database write, resolving all three TODO stubs. No regressions to existing batter path, no security concerns, and the diff matches the PR description exactly.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `custom_cards/interactive_creator.py` (modified) ### Findings #### Correctness No issues found. The fix is precise and complete: 1. **Population site** (line ~182): `card_data` for the pitcher branch now stores `starter_rating`, `relief_rating`, and `closer_rating` directly from the chosen `PitcherArchetype` instance, alongside the existing `ratings` key. 2. **Consumption site** (line ~583-585): The three hardcoded TODO stubs are replaced with reads from `card_data`, which is guaranteed to be populated before `create_pitching_card` is called (the flow is linear: Step 3 builds `card_data`, Step 4 runs the review/tweak loop, Step 5 calls `create_database_records` which calls `create_pitching_card`). 3. **Attribute existence confirmed**: `PitcherArchetype` (defined in `custom_cards/archetype_definitions.py` lines 119-121) declares all three fields as typed dataclass attributes. Every archetype instance in the file carries concrete values — no attribute access can fail. 4. **Position assignment downstream is also correct**: The CLAUDE.md notes that pitcher positions are assigned based on `starter_rating >= 4` (SP) and `closer_rating` presence (CP). The archetype values follow the same convention (Ace: SR=9, Closer: SR=2/CR=9), so cards created via this path will receive the correct position assignments automatically. #### Security No issues found. No user input reaches the archetype fields — values are read from an immutable dataclass populated at archetype selection time. #### Style & Conventions - The three f-string spacing changes (`+` → ` + `) are cosmetic formatter output (Ruff/Black). They are accurate and do not affect behaviour. - The stale comment `# For now, use defaults - can be enhanced later` at line 570-571 of `create_pitching_card` is now misleading — the method no longer uses defaults. This is a minor leftover but does not affect correctness. #### Suggestions - The comment at line 570-571 (`# Determine starter/relief/closer ratings based on archetype / # For now, use defaults - can be enhanced later`) should be updated to reflect that values now come from `card_data`. Low priority — cosmetic only, not a blocker. - `pd_cards/commands/custom.py` lines 826-828 contain identical hardcoded `starter_rating: 5 / relief_rating: 5 / closer_rating: None` stubs in a separate YAML-based custom card path. That path is out of scope for this PR but worth tracking as a follow-up. ### Verdict: APPROVED The fix is correct, targeted, and complete. The two-site change properly threads archetype role ratings from selection through to database write, resolving all three TODO stubs. No regressions to existing batter path, no security concerns, and the diff matches the PR description exactly. --- *Automated review by Claude PR Reviewer*
cal approved these changes 2026-03-23 12:40:58 +00:00
cal left a comment
Owner
Copy Link

Approved. Using the archetype's actual role ratings instead of hardcoded stubs is the correct fix — every pitcher card will now reflect its archetype properly.

Approved. Using the archetype's actual role ratings instead of hardcoded stubs is the correct fix — every pitcher card will now reflect its archetype properly.
cal force-pushed ai/paper-dynasty-card-creation#11 from 31f577e465 to 50ee2d0446 2026-03-23 12:41:16 +00:00 Compare
cal merged commit f2f70bfce5 into main 2026-03-23 12:41:22 +00:00
cal deleted branch ai/paper-dynasty-card-creation#11 2026-03-23 12:41:22 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
cal
Labels
Clear labels   
ai-merged

PR merged by AI ops agent

  
ai-pr-opened

   
ai-reviewed

   
ai-reviewing

   
ai-working
No Label
ai-merged
ai-pr-opened
ai-reviewed
ai-reviewing
ai-working
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 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-card-creation#46
No description provided.
Delete Branch "ai/paper-dynasty-card-creation#11"

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?