Removed the unused alignment field from DefensiveDecision model and all related code across backend and frontend. Backend changes: - models/game_models.py: Removed alignment field and validator - terminal_client/display.py: Removed alignment from display - core/ai_opponent.py: Updated log message - tests/unit/models/test_game_models.py: Removed alignment tests - tests/unit/core/test_validators.py: Removed alignment validation test Frontend changes: - types/game.ts: Removed alignment from DefensiveDecision interface - components/Decisions/DefensiveSetup.vue: * Removed alignment section from template * Removed alignment from localSetup initialization * Removed alignmentOptions array * Removed alignmentDisplay computed property * Removed alignment from hasChanges comparison * Removed alignment from visual preview (reorganized to col-span-2) Rationale: Defensive alignment is not active in the game and will not be used. Per Cal's decision, remove completely rather than keep as dead code. Tests: All 728 backend unit tests passing (100%) Session 1 Part 3 - Change #6 complete Part of cleanup work from demo review 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
8.6 KiB
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:
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:
- Remove entirely - If we never check this
- Rename to
supports_auto_mode()- Invert logic (clearer) - 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:
@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_idmakes 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:
- Keep card_id - Works for PD, need to translate for SBA
- Use roster_link_id - Unified identifier
- League-specific fields -
card_idfor PD,player_idfor 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:
- What outcomes currently require hit location?
- What SHOULD require hit location per game rules?
- 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 modelfrontend-sba/components/Decisions/DefensiveSetup.vue- UI componentbackend/app/websocket/handlers.py- Event handler
Options:
- Remove completely - Clean up unused code
- Keep but mark unused - Future feature
- 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 validationbackend/app/core/validators.py- Add rule validationfrontend-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 OffensiveDecisionfrontend-sba/components/Decisions/OffensiveApproach.vue- Complete UI redesignbackend/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)
- ✅ Remove strikes_for_out
- ✅ Remove balls_for_walk
- 🤔 Remove/refactor supports_manual_result_selection() - NEEDS DECISION
Standard Changes (1-2 hours each)
- 🔍 Pitching change identifier - NEEDS DECISION
- 🔍 Hit location enforcement - NEEDS CODE REVIEW
- 🔍 Remove defensive alignment - NEEDS DECISION
- 🔍 Remove offensive approach - NEEDS DECISION (conflicts with #10)
Complex Changes (3-4 hours each)
- 💬 Infield depths validation - NEEDS APPROVAL
- 💬 Outfield depths validation - NEEDS APPROVAL
- 💬 Replace approach with actions - NEEDS DISCUSSION (breaks tests)
- 💬 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
-
Change #3: Do we ever check
supports_manual_result_selection()? Should we rename tosupports_auto_mode()instead? -
Change #4: What identifier does the frontend have when selecting a relief pitcher from the bench?
-
Changes #6, #7: Are alignment and approach NEVER going to be used, or future features we should keep?
-
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