# Proposed Changes Analysis - 2025-01-14

**Date**: 2025-01-14
**Source**: Cal's review of demo pages
**Total Changes**: 11 items

---

## Quick Wins (Simple, Low Risk) - 3 items

### ✅ Change #1: Remove `strikes_for_out` field
**Location**: `backend/app/config/base_config.py:27`
**Change**: Delete line 27
**Impact**: 1 file
**Effort**: < 5 minutes
**Risk**: None (already marked as TODO to remove)
**Recommendation**: **PROCEED IMMEDIATELY**

---

### ✅ Change #2: Remove `balls_for_walk` field
**Location**: `backend/app/config/base_config.py:28`
**Change**: Delete line 28
**Impact**: 1 file
**Effort**: < 5 minutes
**Risk**: None (already marked as TODO to remove)
**Recommendation**: **PROCEED IMMEDIATELY**

---

### 🤔 Change #3: Remove/refactor `supports_manual_result_selection()`
**Location**: `backend/app/config/base_config.py:41`
**Current**: Method on BaseGameConfig
**Question**: When would we ever use this function?

**Current Usage**:
```python
def supports_manual_result_selection(self) -> bool:
    # TODO: consider refactor: manually selecting results is default behavior
    # with PD allowing auto-results as an option
    return True
```

**Analysis**:
- SbaConfig: Always manual (no auto mode)
- PdConfig: Supports both manual and auto

**Options**:
1. **Remove entirely** - If we never check this
2. **Rename to `supports_auto_mode()`** - Invert logic (clearer)
3. **Keep as-is** - If we do use it

**Question for Cal**: Do we ever check if manual mode is supported? Or should this be `supports_auto_mode()` instead?

**Recommendation**: **NEEDS DECISION**

---

## Standard Changes (Need Review) - 4 items

### 🔍 Change #4: Pitching change player identification
**Location**: `backend/app/websocket/handlers.py` (pitching_change event)
**Current**: Uses `player_in_card_id`
**Question**: Is card ID the best way to identify the new player from active roster?

**Current Implementation**:
```python
@sio.event
async def request_pitching_change(sid, data):
    player_in_card_id = data.get("player_in_card_id")  # Card ID
```

**Analysis**:
- **PD League**: `card_id` makes sense (cards are the entity)
- **SBA League**: Uses `player_id` (different from card_id)
- **Roster**: RosterLink table has both card_id and player_id (polymorphic)

**Options**:
1. **Keep card_id** - Works for PD, need to translate for SBA
2. **Use roster_link_id** - Unified identifier
3. **League-specific fields** - `card_id` for PD, `player_id` for SBA

**Question for Cal**: What identifier does the frontend UI have access to when selecting a relief pitcher?

**Recommendation**: **NEEDS DECISION** (depends on frontend data model)

---

### 🔍 Change #5: Hit location enforcement
**Current**: Enforcing hit location on too many results
**Correct Behavior**:
- Use `PlayOutcome.requires_hit_location()` (already exists)
- Only required if runner on base who doesn't automatically score

**Files to Check**:
- `frontend-sba/components/Gameplay/ManualOutcomeEntry.vue:152` (outcomesNeedingHitLocation array)
- `backend/app/core/play_resolver.py` (validation logic)

**Analysis Needed**:
1. What outcomes currently require hit location?
2. What SHOULD require hit location per game rules?
3. Does `PlayOutcome.requires_hit_location()` match game rules?

**Recommendation**: **NEEDS CODE REVIEW** - Let me check current implementation

---

### 🔍 Change #6: Remove Defensive Alignment
**Current**: DefensiveDecision has `alignment` field
**Proposed**: Remove or mark unused (not active in game)

**Impact**:
- `backend/app/models/game_models.py` - DefensiveDecision model
- `frontend-sba/components/Decisions/DefensiveSetup.vue` - UI component
- `backend/app/websocket/handlers.py` - Event handler

**Options**:
1. **Remove completely** - Clean up unused code
2. **Keep but mark unused** - Future feature
3. **Hide from UI** - Keep model field, remove UI

**Question for Cal**: Is this NEVER going to be used, or is it a future feature?

**Recommendation**: **NEEDS DECISION**

---

### 🔍 Change #7: Remove Offensive Approach
**Current**: OffensiveDecision has `approach` field
**Proposed**: Remove or mark unused (not active in game)

**Similar to Change #6** - same questions apply

**Recommendation**: **NEEDS DECISION**

---

## Complex Changes (Workflow Changes) - 4 items

### 💬 Change #8: Infield Depths - Simplify to 3 valid options
**Current**: May have more options
**Proposed Valid Values**:
- `"infield_in"` (only legal with runner on 3rd)
- `"corners_in"` (only legal with runner on 3rd)
- `"normal"` (default)

**Impact**:
- `backend/app/models/game_models.py` - DefensiveDecision validation
- `backend/app/core/validators.py` - Add rule validation
- `frontend-sba/components/Decisions/DefensiveSetup.vue` - UI options
- Tests for validation

**Effort**: 1-2 hours
**Risk**: Medium (validation logic)

**Recommendation**: **NEEDS APPROVAL** - Good cleanup, requires validation updates

---

### 💬 Change #9: Outfield Depths - Add legal constraints
**Current**: May allow any value
**Proposed Valid Values**:
- `"normal"` (default, 99.99% of time)
- `"shallow"` (only legal when batting team could walk-off win with runner on base)

**Impact**: Same as Change #8

**Question**: How do we determine "could walk-off win"?
- Batter's team is home team
- Bottom of 9th or later
- Trailing or tied
- Runner on base

**Effort**: 1-2 hours
**Risk**: Medium (walk-off logic)

**Recommendation**: **NEEDS APPROVAL** - Adds game-accurate validation

---

### 💬 Change #10: Replace offensive approach with specific actions
**Current**: OffensiveDecision has `approach` field
**Proposed**: Replace with specific action choices:
- `"swing_away"` (default)
- `"check_jump"` (for lead runner only)
- `"hit_and_run"`
- `"sac_bunt"`
- `"squeeze_bunt"` (only legal with R3 and bases not loaded)

**Impact**:
- `backend/app/models/game_models.py` - Refactor OffensiveDecision
- `frontend-sba/components/Decisions/OffensiveApproach.vue` - Complete UI redesign
- `backend/app/websocket/handlers.py` - Update event handling
- Tests (213 tests in Phase F3 may need updates)

**Effort**: 3-4 hours
**Risk**: High (affects decision workflow, breaks existing tests)

**Recommendation**: **NEEDS DISCUSSION** - Major workflow change

---

### 💬 Change #11: Check jump only with lead runner
**Current**: May allow checking jump with any runner
**Proposed Rule**: Players may only check jump with:
- Lead runner only
- OR both runners if 1st and 3rd
- NO checking jump with trail runners

**Impact**:
- Validation logic for "check_jump" action (from Change #10)
- Frontend UI to disable invalid options

**Effort**: 30 minutes (if Change #10 implemented)
**Risk**: Low (validation rule)

**Recommendation**: **BUNDLE WITH CHANGE #10**

---

## Summary by Category

### Quick Wins (5 min each)
1. ✅ Remove strikes_for_out
2. ✅ Remove balls_for_walk
3. 🤔 Remove/refactor supports_manual_result_selection() - **NEEDS DECISION**

### Standard Changes (1-2 hours each)
4. 🔍 Pitching change identifier - **NEEDS DECISION**
5. 🔍 Hit location enforcement - **NEEDS CODE REVIEW**
6. 🔍 Remove defensive alignment - **NEEDS DECISION**
7. 🔍 Remove offensive approach - **NEEDS DECISION** (conflicts with #10)

### Complex Changes (3-4 hours each)
8. 💬 Infield depths validation - **NEEDS APPROVAL**
9. 💬 Outfield depths validation - **NEEDS APPROVAL**
10. 💬 Replace approach with actions - **NEEDS DISCUSSION** (breaks tests)
11. 💬 Check jump lead runner only - **BUNDLE WITH #10**

---

## Recommended Approach

### Phase 1: Quick Wins (15 min)
- Execute #1, #2 immediately
- Decide on #3 (remove vs refactor vs keep)

### Phase 2: Decisions Needed (discussion)
- #4: What identifier does frontend use?
- #6, #7: Remove unused fields or keep for future?
- These inform #10 (if we're removing approach anyway, don't refactor it)

### Phase 3: Code Review (30 min)
- #5: Review hit location logic, fix if needed

### Phase 4: Validation Updates (2-4 hours)
- #8, #9: Add depth validation rules
- Only if approved

### Phase 5: Workflow Refactor (4+ hours)
- #10, #11: Offensive action redesign
- Major change, do last after everything else settled
- Will break existing tests, need full re-test

---

## Questions for Cal

1. **Change #3**: Do we ever check `supports_manual_result_selection()`? Should we rename to `supports_auto_mode()` instead?

2. **Change #4**: What identifier does the frontend have when selecting a relief pitcher from the bench?

3. **Changes #6, #7**: Are alignment and approach NEVER going to be used, or future features we should keep?

4. **Change #10**: This is a major workflow change - are you sure we want to do this now (before Phase F6 integration)?

---

**Document Version**: 1.0
**Status**: Awaiting decisions on questions above