# Promo Card Description Protection - Implementation Summary

## Problem
When running monthly Promo card updates, the system was renaming ALL existing promo cards to the current month. For example, when running May updates, it would rename April cards from "April" to "May", requiring manual cleanup.

## Root Cause
The `post_player_updates()` function in both `batters/creation.py` and `pitchers/creation.py` was performing a bulk GET of ALL players in the cardset and updating their descriptions if they didn't match the current `player_description` parameter.

**Location of issue:**
- `batters/creation.py:316-317`
- `pitchers/creation.py:405-406`

## Solution Implemented
Added logic to detect Promo cardsets and only update descriptions for NEW cards (cost == 99999).

### Changes Made

#### 1. batters/creation.py (lines 316-328)
```python
# OLD CODE:
if df_data['description'] != player_desc and 'potm' not in df_data['description'].lower():
    params = [('description', f'{player_desc}')]

# NEW CODE:
# Determine if this is a promo cardset
is_promo_cardset = 'promo' in cardset['name'].lower()

# For promo cardsets: only update description for NEW cards (cost == 99999)
# For regular cardsets: update description unless it contains 'potm'
if is_promo_cardset:
    if df_data['cost'] == 99999:
        # Only set description for NEW cards in promo cardsets
        params = [('description', f'{player_desc}')]
else:
    # Regular cardsets: update if different and not a potm card
    if df_data['description'] != player_desc and 'potm' not in df_data['description'].lower():
        params = [('description', f'{player_desc}')]
```

#### 2. pitchers/creation.py (lines 405-417)
Same logic applied to pitcher card updates.

## Behavior After Fix

### For Promo Cardsets (name contains "promo")
- ✅ **New cards** (cost = 99999) → Description is set to current month/value
- 🛡️ **Existing cards** → Description is NEVER changed
- Result: April cards keep "April", May cards keep "May"

### For Regular Cardsets (e.g., "2025 Season")
- ✅ Works exactly as before
- ✅ Updates descriptions unless they contain 'potm'

## Testing

### Unit Tests Created
Created comprehensive test suite: `tests/test_promo_description_protection.py`

**Test coverage:**
1. ✅ Promo cardsets protect existing card descriptions
2. ✅ Promo cardsets update NEW card descriptions
3. ✅ Regular cardsets update descriptions normally
4. ✅ PotM cards are never updated (both cardset types)
5. ✅ Case-insensitive promo detection
6. ✅ Non-promo cardsets not incorrectly detected
7. ✅ Monthly workflow simulation (April → May scenario)

**Run tests:**
```bash
source venv/bin/activate
python -m pytest tests/test_promo_description_protection.py -v
```

**Results:** All 7 tests PASS ✅

### Manual Testing Checklist
Before running in production, test with a small dataset:

1. **Setup test Promo cardset:**
   - Create test cardset named "Test Promos"
   - Add 2-3 existing players with description="April"

2. **Run with May data:**
   - Set `PLAYER_DESCRIPTION = 'May'`
   - Include 1 new player

3. **Verify:**
   - [ ] Existing "April" cards still show "April"
   - [ ] New card shows "May"
   - [ ] No description updates sent for existing cards

## How to Use

### For Monthly Promo Updates
No changes to your workflow! Just run as normal:

```python
# In retrosheet_data.py or similar
CARDSET_NAME = '2024 Promos'
PLAYER_DESCRIPTION = 'May'  # Current month
```

The system will now:
1. Process all players with May stats
2. Only set description="May" for NEW cards
3. Leave existing April/March/etc. cards unchanged

### For Regular Season Updates
No impact - works exactly as before.

## Files Modified
- `batters/creation.py` (lines 316-328)
- `pitchers/creation.py` (lines 405-417)
- `tests/test_promo_description_protection.py` (new file)

## Recommendations for Reducing Fragility

### 1. Extract Business Logic into Testable Functions
**Current issue:** The `get_player_updates()` function is nested inside `post_player_updates()` and can't be tested independently.

**Suggestion:** Extract to module-level function:
```python
def should_update_description(
    cardset_name: str,
    player_cost: int,
    current_desc: str,
    new_desc: str
) -> bool:
    """Determine if a player's description should be updated."""
    is_promo_cardset = 'promo' in cardset_name.lower()

    if is_promo_cardset:
        return player_cost == 99999
    else:
        return current_desc != new_desc and 'potm' not in current_desc.lower()
```

**Benefits:**
- Easily unit tested
- Reusable across batters and pitchers
- Clear single responsibility

### 2. Use Constants for Magic Numbers
**Current issue:** `cost == 99999` appears throughout the code with no explanation.

**Suggestion:**
```python
# In creation_helpers.py or similar
NEW_PLAYER_COST = 99999  # Sentinel value indicating a new player not yet priced

# Usage:
if df_data['cost'] == NEW_PLAYER_COST:
    ...
```

### 3. Add Type Hints
**Current issue:** No type hints make it hard to understand data structures.

**Suggestion:**
```python
from typing import Dict, List, Tuple

def post_player_updates(
    cardset: Dict[str, any],
    player_desc: str,
    is_liveseries: bool,
    to_post: bool,
    season: int
) -> int:
    """Update player metadata after card creation."""
    ...
```

### 4. Create Dedicated Configuration Dataclass
**Current issue:** Many parameters passed through multiple function calls.

**Suggestion:**
```python
from dataclasses import dataclass

@dataclass
class CardsetConfig:
    """Configuration for cardset processing."""
    cardset_id: int
    cardset_name: str
    season: int
    player_description: str
    is_liveseries: bool
    is_promo: bool
    post_players: bool
    post_batters: bool
    post_pitchers: bool

    @property
    def is_promo_cardset(self) -> bool:
        return 'promo' in self.cardset_name.lower()
```

### 5. Split Large Functions
**Current issue:** `post_player_updates()` does too much (400+ lines).

**Suggestion:** Break into smaller functions:
```python
async def post_player_updates(...):
    """Orchestrate player updates."""
    player_data = await _fetch_player_data(cardset, season)
    cost_updates = _calculate_cost_updates(player_data)
    desc_updates = _calculate_description_updates(player_data, cardset, player_desc)
    team_updates = await _calculate_team_updates(player_data, is_liveseries)

    await _apply_updates(cost_updates, desc_updates, team_updates, to_post)
```

### 6. Add Integration Tests
**Current issue:** Only unit tests exist; hard to test full workflow.

**Suggestion:** Create tests that mock the database but test the full flow:
```python
@pytest.mark.integration
async def test_promo_workflow_end_to_end():
    """Test complete promo card workflow with mocked DB."""
    # Mock DB responses
    # Run full workflow
    # Verify correct updates sent to DB
```

### 7. Add Logging for Important Decisions
**Suggestion:** Log when descriptions are/aren't updated:
```python
if is_promo_cardset:
    if df_data['cost'] == 99999:
        logger.info(f"Setting description for NEW promo card: player_id={df_data['player_id']}")
        params = [('description', f'{player_desc}')]
    else:
        logger.debug(f"Skipping description update for existing promo card: player_id={df_data['player_id']}, current_desc={df_data['description']}")
```

### 8. Document Assumptions
**Suggestion:** Add docstring to `post_player_updates()` explaining:
- When it's called
- What data it expects
- What updates it makes
- Special cases (promo, potm, etc.)

## Future Enhancements

1. **Add cardset type field to database** instead of detecting from name
2. **Add description lock field** for fine-grained control
3. **Create audit log** of all player updates
4. **Add dry-run mode** to preview changes before applying

## Contact & Support
For questions about this fix, check:
- This document
- Test file: `tests/test_promo_description_protection.py`
- Git commit with "PROMO CARD" in the message