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

feat: implement tweak_archetype() and manual_adjustments() (#12) #49

Merged
cal merged 1 commits from ai/paper-dynasty-card-creation-12 into main 2026-03-23 12:42:01 +00:00
Conversation 0 Commits 1 Files Changed 1 +156 -8
Claude commented 2026-03-21 22:04:14 +00:00
Collaborator
Copy Link

Closes #12

Summary

  • tweak_archetype(): Prompts for updated archetype stats (AVG/OBP/SLG/BB%/K% vs L and R, power profile, batted-ball profile, and baserunning for batters), then recalculates D20 card ratings via the existing BatterRatingCalculator / PitcherRatingCalculator. Uses copy.copy() so the original archetype is never mutated.

  • manual_adjustments(): Lets the user choose a split (vs L or vs R), displays all 22 D20 chance fields with a running total, accepts <field_number> <value> edits in a loop, and prints a warning if the total deviates from 108 when done.

Files changed

  • custom_cards/interactive_creator.py — replaced both "Feature coming soon" stubs with working implementations; added import copy

Test results

No automated test suite. Changes verified by reading the implementation against the archetype dataclass fields and calculator signatures.

🤖 Generated with Claude Code

Closes #12 ## Summary - **`tweak_archetype()`**: Prompts for updated archetype stats (AVG/OBP/SLG/BB%/K% vs L and R, power profile, batted-ball profile, and baserunning for batters), then recalculates D20 card ratings via the existing `BatterRatingCalculator` / `PitcherRatingCalculator`. Uses `copy.copy()` so the original archetype is never mutated. - **`manual_adjustments()`**: Lets the user choose a split (vs L or vs R), displays all 22 D20 chance fields with a running total, accepts `<field_number> <value>` edits in a loop, and prints a warning if the total deviates from 108 when done. ## Files changed - `custom_cards/interactive_creator.py` — replaced both "Feature coming soon" stubs with working implementations; added `import copy` ## Test results No automated test suite. Changes verified by reading the implementation against the archetype dataclass fields and calculator signatures. 🤖 Generated with [Claude Code](https://claude.ai/claude-code)
Claude added 1 commit 2026-03-21 22:04:15 +00:00 
Closes #12

- tweak_archetype(): prompts user for updated archetype stats (avg/obp/slg/bb%/k% vs L and R, power and batted-ball profile, baserunning for batters), then recalculates D20 card ratings via the existing calculator
- manual_adjustments(): prompts user to choose a split (vs L or vs R), displays all 22 D20 chance fields with running total, accepts field-number + value edits, and warns if total deviates from 108

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

AI Code Review

Files Reviewed

  • custom_cards/interactive_creator.py (modified)

Findings

Correctness

  • tweak_archetype() — D20 fields verified. All 22 fields in D20_FIELDS match the rating dicts returned by both BatterRatingCalculator._calculate_split() and PitcherRatingCalculator._calculate_split(). The popout field (always 0.0) is present and correctly included. ✓
  • Return shape verified. tweak_archetype() returns {"ratings": ..., "baserunning": ...} for batters and {"ratings": ...} for pitchers, which is exactly what display_ratings(), manual_adjustments(), and create_batting_card() / create_pitching_card() expect. ✓
  • copy.deepcopy() in manual_adjustments() is correct — card_data["ratings"] is a list of dicts; a shallow copy would have caused in-place mutation of the caller's data. ✓
  • copy.copy() in tweak_archetype() is fine — BatterArchetype.primary_positions (a list) is shared by the shallow copy, but is never modified here so no aliasing issue. ✓
  • 108-threshold logic — abs(total - 108.0) > 0.01 is appropriate given mround() rounding to 0.05. ✓

Security

  • No issues. User input is consumed via input() and converted to float/int with explicit try/except; no injection surface.

Style & Conventions

  • Three # type: ignore comments are expected and correct: the union type BatterArchetype | PitcherArchetype prevents the type checker from narrowing through the player_type == "batter" runtime guard, and PitcherArchetype genuinely lacks xbt_pct. ✓
  • prompt_float / prompt_int are defined as local helpers inside tweak_archetype(). Given they're single-use, this is appropriately minimal per project conventions. ✓

Suggestions

  • Non-accumulating tweaks (UX limitation, non-blocking): tweak_archetype() always starts fresh from the original archetype object passed from create_player(). The modified arch is local and discarded. If the user invokes option 2 twice (e.g., tweaks AVG first, then wants to further tweak K%), the second prompt shows the original archetype defaults, not the result of the first tweak. The recalculated card_data from the first tweak is shown visually, but the prompt baselines reset. Users can work around this by using manual_adjustments() for follow-up adjustments, or by doing all stat changes in a single tweak session. Worth noting as a follow-up improvement if iterative stat-based tweaking is a common workflow.

Verdict: COMMENT

Implementation is correct and matches the PR description. Both stubs are replaced with working, well-guarded implementations. The D20_FIELDS list is accurate, copy usage is appropriate, and edge cases (empty input, out-of-range field numbers, negative values) are handled. The non-accumulating tweak behaviour is a minor UX gap but doesn't block merging. LGTM.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `custom_cards/interactive_creator.py` (modified) ### Findings #### Correctness - **`tweak_archetype()` — D20 fields verified.** All 22 fields in `D20_FIELDS` match the rating dicts returned by both `BatterRatingCalculator._calculate_split()` and `PitcherRatingCalculator._calculate_split()`. The `popout` field (always `0.0`) is present and correctly included. ✓ - **Return shape verified.** `tweak_archetype()` returns `{"ratings": ..., "baserunning": ...}` for batters and `{"ratings": ...}` for pitchers, which is exactly what `display_ratings()`, `manual_adjustments()`, and `create_batting_card()` / `create_pitching_card()` expect. ✓ - **`copy.deepcopy()` in `manual_adjustments()` is correct** — `card_data["ratings"]` is a list of dicts; a shallow copy would have caused in-place mutation of the caller's data. ✓ - **`copy.copy()` in `tweak_archetype()` is fine** — `BatterArchetype.primary_positions` (a list) is shared by the shallow copy, but is never modified here so no aliasing issue. ✓ - **108-threshold logic** — `abs(total - 108.0) > 0.01` is appropriate given `mround()` rounding to 0.05. ✓ #### Security - No issues. User input is consumed via `input()` and converted to `float`/`int` with explicit `try/except`; no injection surface. #### Style & Conventions - Three `# type: ignore` comments are expected and correct: the union type `BatterArchetype | PitcherArchetype` prevents the type checker from narrowing through the `player_type == "batter"` runtime guard, and `PitcherArchetype` genuinely lacks `xbt_pct`. ✓ - `prompt_float` / `prompt_int` are defined as local helpers inside `tweak_archetype()`. Given they're single-use, this is appropriately minimal per project conventions. ✓ #### Suggestions - **Non-accumulating tweaks (UX limitation, non-blocking):** `tweak_archetype()` always starts fresh from the *original* `archetype` object passed from `create_player()`. The modified `arch` is local and discarded. If the user invokes option 2 twice (e.g., tweaks AVG first, then wants to further tweak K%), the second prompt shows the *original* archetype defaults, not the result of the first tweak. The recalculated `card_data` from the first tweak is shown visually, but the prompt baselines reset. Users can work around this by using `manual_adjustments()` for follow-up adjustments, or by doing all stat changes in a single tweak session. Worth noting as a follow-up improvement if iterative stat-based tweaking is a common workflow. ### Verdict: COMMENT Implementation is correct and matches the PR description. Both stubs are replaced with working, well-guarded implementations. The `D20_FIELDS` list is accurate, `copy` usage is appropriate, and edge cases (empty input, out-of-range field numbers, negative values) are handled. The non-accumulating tweak behaviour is a minor UX gap but doesn't block merging. LGTM. --- *Automated review by Claude PR Reviewer*
Claude added
ai-reviewed
 and removed
ai-reviewing
 labels 2026-03-21 22:18:30 +00:00
cal approved these changes 2026-03-23 12:07:52 +00:00
Dismissed
cal left a comment
Owner
Copy Link

AI Code Review

Files Reviewed

  • custom_cards/interactive_creator.py (modified)

Findings

Correctness

  • Both stub functions are fully replaced with working implementations. No TODO/FIXME comments remain in the changed code.
  • tweak_archetype(): uses copy.copy() on the archetype before mutating it, correctly preventing any modification of the shared archetype object. Field names (avg_vs_r, obp_vs_r, slg_vs_r, bb_pct_vs_r, k_pct_vs_r, avg_vs_l, obp_vs_l, slg_vs_l, bb_pct_vs_l, k_pct_vs_l, hr_per_hit, triple_per_hit, double_per_hit, gb_pct, fb_pct, ld_pct, speed_rating, steal_jump, xbt_pct) all match their definitions in BatterArchetype and PitcherArchetype.
  • The pitcher path in tweak_archetype() correctly skips the baserunning section (guarded by if player_type == "batter"). For pitchers, speed_rating, steal_jump, and xbt_pct do not exist on PitcherArchetype, so the guard is necessary and correct.
  • The return shape from tweak_archetype() matches what review_and_tweak() expects: {"ratings": ..., "baserunning": ...} for batters, {"ratings": ...} for pitchers.
  • manual_adjustments(): uses copy.deepcopy() (appropriate since card_data contains nested dicts), correctly modifies only the user-chosen split, and prints a warning if the total deviates from 108 by more than 0.01. The 22-field D20_FIELDS list matches the exact set of D20 chance fields in both BatterRatingCalculator and PitcherRatingCalculator.
  • Input validation is solid throughout: ValueError on non-numeric input, bounds checking on field index, rejection of negative values.

Security

  • No user input is used in queries, file paths, or shell commands. Input is only cast to float/int and written back to an in-memory dict. No security issues.

Style & Conventions

  • prompt_float and prompt_int are defined as local nested functions inside tweak_archetype(). This is a reasonable choice for small helpers used only in that scope; no concern.
  • The three whitespace-only changes to display_ratings() (adding spaces around + in f-strings) are consistent with the rest of the file's style.
  • # type: ignore[arg-type] and # type: ignore[union-attr] annotations on the baserunning prompts are appropriate since mypy cannot narrow arch to BatterArchetype after the shallow copy — the guard ensures correctness at runtime.
  • One pre-existing TODO in create_pitching_card() (starter_rating, relief_rating, closer_rating hardcoded to 5/5/None) is untouched by this PR and is out of scope.

Suggestions

  • The tweak_archetype() pitcher path does not expose hard_pct, med_pct, soft_pct, pull_pct, center_pct, oppo_pct, ifh_pct, or hr_fb_pct for editing. These are present on both archetype types and influence hit/out distributions. A future improvement could expose these, but it is not a blocker for this PR's stated scope.
  • After tweaking archetype stats in tweak_archetype(), the returned card_data no longer references the modified arch object (the archetype passed into review_and_tweak() is the original). This means a second call to option 2 starts from the original archetype again rather than the previously tweaked one. This is the expected behaviour given the docstring says "adjust key percentages", but worth noting for a future iteration if users want to stack tweaks.

Verdict: APPROVED

Both stub functions are correctly implemented, field names are verified against their dataclass definitions, the mutation-safety pattern (copy.copy / copy.deepcopy) is applied correctly, and all input paths are validated. The PR does exactly what its description claims. No blocking issues found.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `custom_cards/interactive_creator.py` (modified) ### Findings #### Correctness - Both stub functions are fully replaced with working implementations. No `TODO`/`FIXME` comments remain in the changed code. - `tweak_archetype()`: uses `copy.copy()` on the archetype before mutating it, correctly preventing any modification of the shared archetype object. Field names (`avg_vs_r`, `obp_vs_r`, `slg_vs_r`, `bb_pct_vs_r`, `k_pct_vs_r`, `avg_vs_l`, `obp_vs_l`, `slg_vs_l`, `bb_pct_vs_l`, `k_pct_vs_l`, `hr_per_hit`, `triple_per_hit`, `double_per_hit`, `gb_pct`, `fb_pct`, `ld_pct`, `speed_rating`, `steal_jump`, `xbt_pct`) all match their definitions in `BatterArchetype` and `PitcherArchetype`. - The pitcher path in `tweak_archetype()` correctly skips the baserunning section (guarded by `if player_type == "batter"`). For pitchers, `speed_rating`, `steal_jump`, and `xbt_pct` do not exist on `PitcherArchetype`, so the guard is necessary and correct. - The return shape from `tweak_archetype()` matches what `review_and_tweak()` expects: `{"ratings": ..., "baserunning": ...}` for batters, `{"ratings": ...}` for pitchers. - `manual_adjustments()`: uses `copy.deepcopy()` (appropriate since `card_data` contains nested dicts), correctly modifies only the user-chosen split, and prints a warning if the total deviates from 108 by more than 0.01. The 22-field `D20_FIELDS` list matches the exact set of D20 chance fields in both `BatterRatingCalculator` and `PitcherRatingCalculator`. - Input validation is solid throughout: `ValueError` on non-numeric input, bounds checking on field index, rejection of negative values. #### Security - No user input is used in queries, file paths, or shell commands. Input is only cast to `float`/`int` and written back to an in-memory dict. No security issues. #### Style & Conventions - `prompt_float` and `prompt_int` are defined as local nested functions inside `tweak_archetype()`. This is a reasonable choice for small helpers used only in that scope; no concern. - The three whitespace-only changes to `display_ratings()` (adding spaces around `+` in f-strings) are consistent with the rest of the file's style. - `# type: ignore[arg-type]` and `# type: ignore[union-attr]` annotations on the baserunning prompts are appropriate since mypy cannot narrow `arch` to `BatterArchetype` after the shallow copy — the guard ensures correctness at runtime. - One pre-existing `TODO` in `create_pitching_card()` (`starter_rating`, `relief_rating`, `closer_rating` hardcoded to 5/5/None) is untouched by this PR and is out of scope. #### Suggestions - The `tweak_archetype()` pitcher path does not expose `hard_pct`, `med_pct`, `soft_pct`, `pull_pct`, `center_pct`, `oppo_pct`, `ifh_pct`, or `hr_fb_pct` for editing. These are present on both archetype types and influence hit/out distributions. A future improvement could expose these, but it is not a blocker for this PR's stated scope. - After tweaking archetype stats in `tweak_archetype()`, the returned `card_data` no longer references the modified `arch` object (the archetype passed into `review_and_tweak()` is the original). This means a second call to option 2 starts from the original archetype again rather than the previously tweaked one. This is the expected behaviour given the docstring says "adjust key percentages", but worth noting for a future iteration if users want to stack tweaks. ### Verdict: APPROVED Both stub functions are correctly implemented, field names are verified against their dataclass definitions, the mutation-safety pattern (`copy.copy` / `copy.deepcopy`) is applied correctly, and all input paths are validated. The PR does exactly what its description claims. No blocking issues found. --- *Automated review by Claude PR Reviewer*
cal approved these changes 2026-03-23 12:41:32 +00:00
cal left a comment
Owner
Copy Link

Approved. Solid implementations of both stubs — tweak_archetype() correctly uses copy.copy() to avoid mutating the original, and manual_adjustments() provides a clean interactive loop with the 108-total validation warning.

Approved. Solid implementations of both stubs — `tweak_archetype()` correctly uses `copy.copy()` to avoid mutating the original, and `manual_adjustments()` provides a clean interactive loop with the 108-total validation warning.
cal force-pushed ai/paper-dynasty-card-creation-12 from 9584daee57 to 962b9cf6f1 2026-03-23 12:41:56 +00:00 Compare
cal merged commit 461a469374 into main 2026-03-23 12:42:01 +00:00
cal deleted branch ai/paper-dynasty-card-creation-12 2026-03-23 12:42:01 +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#49
No description provided.
Delete Branch "ai/paper-dynasty-card-creation-12"

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?