fix: guard T4 rarity upgrade from live-series pipeline collision (#57) #63

Claude · 2026-04-08T11:09:23Z

Claude commented

2026-04-08 11:09:23 +00:00

Closes #57

What was broken

post_player_updates() in both batters/creation.py and pitchers/creation.py unconditionally overwrites rarity_id when the OPS-derived rarity differs from the current rarity. Once Phase 2 applies T4 rarity upgrades (e.g. Bronze → Silver), the next live-series run silently reverts the earned upgrade back to Bronze based on raw OPS.

Approach chosen

Strategy 1 from spec T4-3 — guard clause in post_player_updates(). Before writing a rarity downgrade, the pipeline checks whether any affected players have a fully-evolved (T4) refractor card state. If a player is T4-protected, the downgrade is skipped and logged; upgrades and same-rarity writes are unaffected.

Changes

`rarity_thresholds.py`

Added RARITY_LADDER = [5, 4, 3, 2, 1, 99] — ordered worst to best, used for ladder comparisons
Added rarity_is_downgrade(current, new) — returns True only when new is a less prestigious tier than current

`db_calls.py`

Added get_fully_evolved_players(player_ids) — calls GET /api/v2/refractor/fully-evolved?player_ids=... and returns the subset with fully_evolved=True. Falls back to empty set on any error, keeping the pipeline safe.

`batters/creation.py` and `pitchers/creation.py`

Before player_data.apply(get_player_updates, ...), a pre-flight check identifies players whose OPS rarity would be a downgrade, then fetches their T4 state in one API call
Inside get_player_updates, the elif rarity != new_rarity_id branch is guarded: downgrade writes are skipped for T4-protected players; all other rarity changes proceed normally

Database API dependency

GET /api/v2/refractor/fully-evolved?player_ids=1,2,3 must be added to paper-dynasty-database. It should return {"player_ids": [<int>, ...]} — the subset of the given player IDs where any RefractorCardState row has fully_evolved=True.

Until that endpoint is added, the guard returns an empty set and is effectively a no-op. The guard activates automatically once the database side is wired up, with no further changes needed here.

Other observations

rarity_thresholds.RARITY_LADDER also satisfies the prerequisite for issue #59 (rarity ladder utility) — next_rarity() can be built on top of this constant.
The existing _RARITY_LADDER approach was chosen over Strategy 2 (database-side rejection) and Strategy 3 (separate evolution_rarity field) to keep Phase 2 risk surface minimal. The guard is transparent and reversible.

Closes #57 ## What was broken `post_player_updates()` in both `batters/creation.py` and `pitchers/creation.py` unconditionally overwrites `rarity_id` when the OPS-derived rarity differs from the current rarity. Once Phase 2 applies T4 rarity upgrades (e.g. Bronze → Silver), the next live-series run silently reverts the earned upgrade back to Bronze based on raw OPS. ## Approach chosen **Strategy 1 from spec T4-3** — guard clause in `post_player_updates()`. Before writing a rarity downgrade, the pipeline checks whether any affected players have a fully-evolved (T4) refractor card state. If a player is T4-protected, the downgrade is skipped and logged; upgrades and same-rarity writes are unaffected. ## Changes ### `rarity_thresholds.py` - Added `RARITY_LADDER = [5, 4, 3, 2, 1, 99]` — ordered worst to best, used for ladder comparisons - Added `rarity_is_downgrade(current, new)` — returns `True` only when `new` is a less prestigious tier than `current` ### `db_calls.py` - Added `get_fully_evolved_players(player_ids)` — calls `GET /api/v2/refractor/fully-evolved?player_ids=...` and returns the subset with `fully_evolved=True`. Falls back to empty set on any error, keeping the pipeline safe. ### `batters/creation.py` and `pitchers/creation.py` - Before `player_data.apply(get_player_updates, ...)`, a pre-flight check identifies players whose OPS rarity would be a downgrade, then fetches their T4 state in one API call - Inside `get_player_updates`, the `elif rarity != new_rarity_id` branch is guarded: downgrade writes are skipped for T4-protected players; all other rarity changes proceed normally ## Database API dependency `GET /api/v2/refractor/fully-evolved?player_ids=1,2,3` must be added to `paper-dynasty-database`. It should return `{"player_ids": [<int>, ...]}` — the subset of the given player IDs where any `RefractorCardState` row has `fully_evolved=True`. **Until that endpoint is added, the guard returns an empty set and is effectively a no-op.** The guard activates automatically once the database side is wired up, with no further changes needed here. ## Other observations - `rarity_thresholds.RARITY_LADDER` also satisfies the prerequisite for issue #59 (rarity ladder utility) — `next_rarity()` can be built on top of this constant. - The existing `_RARITY_LADDER` approach was chosen over Strategy 2 (database-side rejection) and Strategy 3 (separate evolution_rarity field) to keep Phase 2 risk surface minimal. The guard is transparent and reversible.

Claude added 1 commit 2026-04-08 11:09:24 +00:00

fix: guard against T4 rarity upgrade collision in live-series pipeline (#57 ) aaa2eaa252

Closes #57

- Add RARITY_LADDER and rarity_is_downgrade() to rarity_thresholds.py
- Add get_fully_evolved_players() to db_calls.py — queries a to-be-created
  database endpoint; returns empty set safely if endpoint is unavailable
- In batters/creation.py post_player_updates(): pre-flight check identifies
  players where OPS rarity would downgrade, then guards the rarity write to
  skip any downgrade for fully-evolved (T4) cards
- Same guard added to pitchers/creation.py post_player_updates()

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

Claude referenced this pull request

2026-04-08 11:09:35 +00:00

bug: T4 rarity upgrade collision with live-series card updates #57

Claude added the

ai-reviewing

label 2026-04-08 11:47:48 +00:00

Claude reviewed 2026-04-08 11:49:33 +00:00

Claude left a comment

AI Code Review

Files Reviewed

rarity_thresholds.py (modified — added RARITY_LADDER and rarity_is_downgrade)
db_calls.py (modified — added get_fully_evolved_players)
batters/creation.py (modified — T4 guard in post_player_updates)
pitchers/creation.py (modified — T4 guard in post_player_updates, mirroring batters)

Findings

Correctness

No issues found.

RARITY_LADDER = [5, 4, 3, 2, 1, 99] ordering is correct (Common → … → HoF). Position comparisons in rarity_is_downgrade follow the ladder correctly: current_pos > new_pos means new is at a lower (worse) index → True (is a downgrade). Spot-checked Bronze→Common (1>0=True ✓), Silver→Bronze (2>1=True ✓), Bronze→Silver (1>2=False ✓).
Unknown rarity IDs return False from rarity_is_downgrade (ValueError caught), so unrecognized rarities never accidentally trigger the guard. ✓
get_fully_evolved_players correctly short-circuits on empty input, handles None response and missing "player_ids" field, and returns an empty set on any error path. The 404 fallback keeps the pipeline safe while the endpoint doesn't exist yet. ✓
Closure capture of t4_protected_ids in both get_player_updates closures is valid — the variable is computed before the nested function is defined and called. ✓
downgrade_candidates is built via .tolist(), which converts numpy types to Python native ints. set(result["player_ids"]) from JSON also produces Python ints. numpy.int64 in {int} works correctly via hash equality, so the df_data.player_id in t4_protected_ids membership check is safe. ✓
Upgrades and same-rarity writes are completely unaffected — the guard only fires when rarity_is_downgrade(...) and player_id in t4_protected_ids. ✓

Security

No issues found. No user-controlled input reaches queries or commands; no credentials introduced.

Style & Conventions

Logger format matches existing patterns throughout both creation modules. ✓
RARITY_LADDER constant placement (after threshold factories, before utility functions) is clean.
Pitchers mirrors batters with only a comment cross-reference (# T4 rarity guard: same protection as batters — see batters/creation.py.). No duplication concern at this scale.

Suggestions

get_fully_evolved_players does not guard against result["player_ids"] being None (i.e., {"player_ids": null}). set(None) would raise TypeError. This is a non-issue while the endpoint doesn't exist and very unlikely once it does, but a single extra check (if not isinstance(result.get("player_ids"), list): return set()) would make the fallback airtight.
RARITY_LADDER also satisfies the #59 prerequisite as noted — no action needed here, just confirmed the observation in the PR body is accurate.

Verdict: COMMENT

LGTM. Logic is correct, the guard degrades safely while the database endpoint is pending, and the implementation matches the Strategy 1 spec from T4-3. The one suggestion (null-guard on result["player_ids"]) is optional hardening, not a blocker.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `rarity_thresholds.py` (modified — added `RARITY_LADDER` and `rarity_is_downgrade`) - `db_calls.py` (modified — added `get_fully_evolved_players`) - `batters/creation.py` (modified — T4 guard in `post_player_updates`) - `pitchers/creation.py` (modified — T4 guard in `post_player_updates`, mirroring batters) ### Findings #### Correctness No issues found. - `RARITY_LADDER = [5, 4, 3, 2, 1, 99]` ordering is correct (Common → … → HoF). Position comparisons in `rarity_is_downgrade` follow the ladder correctly: `current_pos > new_pos` means new is at a lower (worse) index → True (is a downgrade). Spot-checked Bronze→Common (1>0=True ✓), Silver→Bronze (2>1=True ✓), Bronze→Silver (1>2=False ✓). - Unknown rarity IDs return `False` from `rarity_is_downgrade` (ValueError caught), so unrecognized rarities never accidentally trigger the guard. ✓ - `get_fully_evolved_players` correctly short-circuits on empty input, handles `None` response and missing `"player_ids"` field, and returns an empty set on any error path. The 404 fallback keeps the pipeline safe while the endpoint doesn't exist yet. ✓ - Closure capture of `t4_protected_ids` in both `get_player_updates` closures is valid — the variable is computed before the nested function is defined and called. ✓ - `downgrade_candidates` is built via `.tolist()`, which converts numpy types to Python native ints. `set(result["player_ids"])` from JSON also produces Python ints. `numpy.int64 in {int}` works correctly via hash equality, so the `df_data.player_id in t4_protected_ids` membership check is safe. ✓ - Upgrades and same-rarity writes are completely unaffected — the guard only fires when `rarity_is_downgrade(...) and player_id in t4_protected_ids`. ✓ #### Security No issues found. No user-controlled input reaches queries or commands; no credentials introduced. #### Style & Conventions - Logger format matches existing patterns throughout both creation modules. ✓ - `RARITY_LADDER` constant placement (after threshold factories, before utility functions) is clean. - Pitchers mirrors batters with only a comment cross-reference (`# T4 rarity guard: same protection as batters — see batters/creation.py.`). No duplication concern at this scale. #### Suggestions - `get_fully_evolved_players` does not guard against `result["player_ids"]` being `None` (i.e., `{"player_ids": null}`). `set(None)` would raise `TypeError`. This is a non-issue while the endpoint doesn't exist and very unlikely once it does, but a single extra check (`if not isinstance(result.get("player_ids"), list): return set()`) would make the fallback airtight. - `RARITY_LADDER` also satisfies the #59 prerequisite as noted — no action needed here, just confirmed the observation in the PR body is accurate. ### Verdict: COMMENT LGTM. Logic is correct, the guard degrades safely while the database endpoint is pending, and the implementation matches the Strategy 1 spec from T4-3. The one suggestion (null-guard on `result["player_ids"]`) is optional hardening, not a blocker. --- *Automated review by Claude PR Reviewer*

Claude added

ai-reviewed

and removed

ai-reviewing

labels 2026-04-08 11:49:57 +00:00

cal approved these changes 2026-04-08 12:25:51 +00:00

cal left a comment

Approved via pd-pr

cal added 1 commit 2026-04-08 12:25:54 +00:00

Merge branch 'main' into issue/57-bug-t4-rarity-upgrade-collision-with-live-series-c dee0349e54

cal merged commit f3da58702b into main

2026-04-08 12:26:00 +00:00