diff --git a/.planning/phases/02-constraint-validation/02-RESEARCH.md b/.planning/phases/02-constraint-validation/02-RESEARCH.md new file mode 100644 index 0000000..4cdb437 --- /dev/null +++ b/.planning/phases/02-constraint-validation/02-RESEARCH.md @@ -0,0 +1,483 @@ +# Phase 2: Constraint Validation - Research + +**Researched:** 2026-01-18 +**Domain:** Constraint validation for itinerary item positioning +**Confidence:** HIGH + +## Summary + +This research reveals that Phase 2 is largely **already implemented**. The codebase contains a complete `ItineraryConstraints` struct with 17 XCTest test cases covering all the constraint requirements from the phase description. The Phase 1 work (SortOrderProvider, Trip day derivation) provides the foundation, and `ItineraryConstraints` already validates game immutability, travel segment positioning, and custom item flexibility. + +The main gap is **migrating the tests from XCTest to Swift Testing** (@Test, @Suite) to match the project's established patterns from Phase 1. The `ItineraryTableViewController` already integrates with `ItineraryConstraints` for drag-drop validation during Phase 4's UI work. + +**Primary recommendation:** Verify existing implementation covers all CONS-* requirements, migrate tests to Swift Testing, and document the constraint API for Phase 4's UI integration. + +## Codebase Analysis + +### Key Files + +| File | Purpose | Status | +|------|---------|--------| +| `SportsTime/Core/Models/Domain/ItineraryConstraints.swift` | Constraint validation struct | **Complete** | +| `SportsTimeTests/ItineraryConstraintsTests.swift` | 17 XCTest test cases | Needs migration to Swift Testing | +| `SportsTime/Core/Models/Domain/SortOrderProvider.swift` | sortOrder calculation utilities | Complete (Phase 1) | +| `SportsTime/Core/Models/Domain/Trip.swift` | Day derivation methods | Complete (Phase 1) | +| `SportsTime/Core/Models/Domain/ItineraryItem.swift` | Unified itinerary item model | Complete | +| `SportsTime/Features/Trip/Views/ItineraryTableViewController.swift` | UITableView with drag-drop | Uses constraints | + +### Existing ItineraryConstraints API + +```swift +// Source: SportsTime/Core/Models/Domain/ItineraryConstraints.swift +struct ItineraryConstraints { + let tripDayCount: Int + private let items: [ItineraryItem] + + init(tripDayCount: Int, items: [ItineraryItem]) + + /// Check if a position is valid for an item + func isValidPosition(for item: ItineraryItem, day: Int, sortOrder: Double) -> Bool + + /// Get the valid day range for a travel item + func validDayRange(for item: ItineraryItem) -> ClosedRange? + + /// Get the games that act as barriers for visual highlighting + func barrierGames(for item: ItineraryItem) -> [ItineraryItem] +} +``` + +### ItemKind Types + +```swift +// Source: SportsTime/Core/Models/Domain/ItineraryItem.swift +enum ItemKind: Codable, Hashable { + case game(gameId: String) // CONS-01: Cannot be moved + case travel(TravelInfo) // CONS-02, CONS-03: Day range + ordering constraints + case custom(CustomInfo) // CONS-04: No constraints +} +``` + +### Integration with Drag-Drop + +The `ItineraryTableViewController` already creates and uses `ItineraryConstraints`: + +```swift +// Source: ItineraryTableViewController.swift (lines 484-494) +func reloadData( + days: [ItineraryDayData], + travelValidRanges: [String: ClosedRange], + itineraryItems: [ItineraryItem] = [] +) { + self.travelValidRanges = travelValidRanges + self.allItineraryItems = itineraryItems + self.tripDayCount = days.count + + // Rebuild constraints with new data + self.constraints = ItineraryConstraints(tripDayCount: tripDayCount, items: itineraryItems) + // ... +} +``` + +## Constraint Requirements Mapping + +### CONS-01: Games cannot be moved (fixed by schedule) + +**Status: IMPLEMENTED** + +```swift +// ItineraryConstraints.isValidPosition() +case .game: + // Games are fixed, should never be moved + return false +``` + +**Existing tests:** +- `test_gameItem_cannotBeMoved()` - Verifies games return false for any position + +**UI Integration (Phase 4):** +- `ItineraryRowItem.isReorderable` returns false for `.games` +- No drag handle appears on game rows + +### CONS-02: Travel segments constrained to valid day range + +**Status: IMPLEMENTED** + +```swift +// ItineraryConstraints.isValidTravelPosition() +let departureGameDays = gameDays(in: fromCity) +let arrivalGameDays = gameDays(in: toCity) + +let minDay = departureGameDays.max() ?? 1 // After last from-city game +let maxDay = arrivalGameDays.min() ?? tripDayCount // Before first to-city game + +guard day >= minDay && day <= maxDay else { return false } +``` + +**Existing tests:** +- `test_travel_validDayRange_simpleCase()` - Chicago Day 1, Detroit Day 3 -> range 1...3 +- `test_travel_cannotGoOutsideValidDayRange()` - Day before departure invalid +- `test_travel_validDayRange_returnsNil_whenConstraintsImpossible()` - Reversed order returns nil + +### CONS-03: Travel must be after from-city games, before to-city games on same day + +**Status: IMPLEMENTED** + +```swift +// ItineraryConstraints.isValidTravelPosition() +if departureGameDays.contains(day) { + let maxGameSortOrder = games(in: fromCity) + .filter { $0.day == day } + .map { $0.sortOrder } + .max() ?? 0 + + if sortOrder <= maxGameSortOrder { + return false // Must be AFTER all departure games + } +} + +if arrivalGameDays.contains(day) { + let minGameSortOrder = games(in: toCity) + .filter { $0.day == day } + .map { $0.sortOrder } + .min() ?? Double.greatestFiniteMagnitude + + if sortOrder >= minGameSortOrder { + return false // Must be BEFORE all arrival games + } +} +``` + +**Existing tests:** +- `test_travel_mustBeAfterDepartureGames()` - Before departure game invalid +- `test_travel_mustBeBeforeArrivalGames()` - After arrival game invalid +- `test_travel_mustBeAfterAllDepartureGamesOnSameDay()` - Between games invalid +- `test_travel_mustBeBeforeAllArrivalGamesOnSameDay()` - Between games invalid +- `test_travel_canBeAnywhereOnRestDays()` - No games = any position valid + +### CONS-04: Custom items have no constraints + +**Status: IMPLEMENTED** + +```swift +// ItineraryConstraints.isValidPosition() +case .custom: + // Custom items can go anywhere + return true +``` + +**Existing tests:** +- `test_customItem_canGoOnAnyDay()` - Days 1-5 all valid +- `test_customItem_canGoBeforeOrAfterGames()` - Any sortOrder valid + +## Standard Stack + +### Core +| Library | Version | Purpose | Why Standard | +|---------|---------|---------|--------------| +| Swift Testing | Swift 5.10+ | Test framework | Project standard from Phase 1 | +| Foundation | Swift stdlib | Date/Calendar | Already used throughout | + +### Supporting +| Library | Version | Purpose | When to Use | +|---------|---------|---------|-------------| +| XCTest | iOS 26+ | Legacy tests | Being migrated away | + +## Architecture Patterns + +### Pattern 1: Pure Function Constraint Checking + +**What:** `ItineraryConstraints.isValidPosition()` is a pure function - no side effects, deterministic + +**When to use:** All constraint validation - fast, testable, no async complexity + +**Example:** +```swift +// Source: ItineraryConstraints.swift +func isValidPosition(for item: ItineraryItem, day: Int, sortOrder: Double) -> Bool { + guard day >= 1 && day <= tripDayCount else { return false } + + switch item.kind { + case .game: + return false + case .travel(let info): + return isValidTravelPosition(fromCity: info.fromCity, toCity: info.toCity, day: day, sortOrder: sortOrder) + case .custom: + return true + } +} +``` + +### Pattern 2: Precomputed Valid Ranges + +**What:** `validDayRange(for:)` computes the valid day range once at drag start + +**When to use:** UI needs to quickly check many positions during drag + +**Example:** +```swift +// Source: ItineraryTableViewController.swift +func calculateTravelDragZones(segment: TravelSegment) { + let travelId = "travel:\(segment.fromLocation.name.lowercased())->\(segment.toLocation.name.lowercased())" + + guard let validRange = travelValidRanges[travelId] else { ... } + + // Pre-calculate ALL valid row indices + for (index, rowItem) in flatItems.enumerated() { + if validRange.contains(dayNum) { + validRows.append(index) + } else { + invalidRows.insert(index) + } + } +} +``` + +### Pattern 3: City Extraction from Game ID + +**What:** Game IDs encode city: `game-CityName-xxxx` + +**Why:** Avoids needing to look up game details during constraint checking + +**Example:** +```swift +// Source: ItineraryConstraints.swift +private func city(forGameId gameId: String) -> String? { + let components = gameId.components(separatedBy: "-") + guard components.count >= 2 else { return nil } + return components[1] +} +``` + +### Anti-Patterns to Avoid + +- **Async constraint validation:** Constraints must be synchronous for responsive drag feedback +- **Row-index based constraints:** Always use semantic (day, sortOrder), never row indices +- **Checking constraints on drop only:** Check at drag start for valid range, each position during drag + +## Don't Hand-Roll + +| Problem | Don't Build | Use Instead | Why | +|---------|-------------|-------------|-----| +| Constraint validation | Custom per-item-type logic | `ItineraryConstraints` | Already handles all cases | +| Day range calculation | Manual game day scanning | `validDayRange(for:)` | Handles edge cases | +| City matching | String equality | `city(forGameId:)` helper | Game ID format is stable | +| Position checking | Multiple conditions scattered | `isValidPosition()` | Single entry point | + +## Common Pitfalls + +### Pitfall 1: Forgetting sortOrder Constraints on Same Day + +**What goes wrong:** Travel placed on game day but ignoring sortOrder requirement + +**Why it happens:** Day range looks valid, forget to check sortOrder against games + +**How to avoid:** Always use `isValidPosition()` which checks both day AND sortOrder + +**Warning signs:** Travel placed before departure game or after arrival game + +### Pitfall 2: City Name Case Sensitivity + +**What goes wrong:** "Chicago" != "chicago" causing constraint checks to fail + +**Why it happens:** TravelInfo stores display-case cities, game IDs may differ + +**How to avoid:** The implementation already lowercases for comparison + +**Warning signs:** Valid travel rejected because city match fails + +### Pitfall 3: Empty Game Days + +**What goes wrong:** No games in a city means null/empty arrays + +**Why it happens:** Some cities might have no games yet (planning in progress) + +**How to avoid:** Implementation uses `?? 1` and `?? tripDayCount` defaults + +**Warning signs:** Constraint checks crash on cities with no games + +## Code Examples + +### Constraint Checking (Full Implementation) + +```swift +// Source: SportsTime/Core/Models/Domain/ItineraryConstraints.swift +struct ItineraryConstraints { + let tripDayCount: Int + private let items: [ItineraryItem] + + func isValidPosition(for item: ItineraryItem, day: Int, sortOrder: Double) -> Bool { + guard day >= 1 && day <= tripDayCount else { return false } + + switch item.kind { + case .game: + return false + case .travel(let info): + return isValidTravelPosition( + fromCity: info.fromCity, + toCity: info.toCity, + day: day, + sortOrder: sortOrder + ) + case .custom: + return true + } + } + + func validDayRange(for item: ItineraryItem) -> ClosedRange? { + guard case .travel(let info) = item.kind else { return nil } + + let departureGameDays = gameDays(in: info.fromCity) + let arrivalGameDays = gameDays(in: info.toCity) + + let minDay = departureGameDays.max() ?? 1 + let maxDay = arrivalGameDays.min() ?? tripDayCount + + guard minDay <= maxDay else { return nil } + return minDay...maxDay + } +} +``` + +### Test Pattern (Swift Testing) + +```swift +// Pattern from Phase 1 tests - to be applied to constraint tests +@Suite("ItineraryConstraints") +struct ItineraryConstraintsTests { + + @Test("game: cannot be moved to any position") + func game_cannotBeMoved() { + let constraints = makeConstraints(tripDays: 5, games: [gameItem]) + + #expect(constraints.isValidPosition(for: gameItem, day: 2, sortOrder: 100) == false) + #expect(constraints.isValidPosition(for: gameItem, day: 3, sortOrder: 100) == false) + } + + @Test("travel: must be after departure games on same day") + func travel_mustBeAfterDepartureGames() { + let constraints = makeConstraints(tripDays: 3, games: [chicagoGame]) + let travel = makeTravelItem(from: "Chicago", to: "Detroit") + + // Before departure game - invalid + #expect(constraints.isValidPosition(for: travel, day: 1, sortOrder: 50) == false) + + // After departure game - valid + #expect(constraints.isValidPosition(for: travel, day: 1, sortOrder: 150) == true) + } + + @Test("custom: can go anywhere") + func custom_canGoAnywhere() { + let constraints = makeConstraints(tripDays: 5) + let custom = makeCustomItem() + + for day in 1...5 { + #expect(constraints.isValidPosition(for: custom, day: day, sortOrder: 50) == true) + } + } +} +``` + +## State of the Art + +| Old Approach | Current Approach | When Changed | Impact | +|--------------|------------------|--------------|--------| +| Row-index validation | Semantic (day, sortOrder) validation | Phase 1 | Stable across reloads | +| XCTest framework | Swift Testing (@Test, @Suite) | Phase 1 | Modern, cleaner assertions | +| Constraint checking on drop | Precompute at drag start | Already implemented | Smoother drag UX | + +## Open Questions + +### Resolved by Research + +1. **Where does constraint validation live?** + - Answer: `ItineraryConstraints` struct in Domain models + - Confidence: HIGH - already implemented and integrated + +2. **How will Phase 4 call it?** + - Answer: `ItineraryTableViewController` already integrates via `self.constraints` + - Confidence: HIGH - verified in codebase + +### Minor Questions (Claude's Discretion per CONTEXT.md) + +1. **Visual styling for invalid zones?** + - Current: Alpha 0.3 dimming, gold border on barrier games + - CONTEXT.md says: "Border highlight color for valid drop zones" is Claude's discretion + - Recommendation: Keep current implementation, refine in Phase 4 if needed + +2. **sortOrder precision exhaustion handling?** + - Current: `SortOrderProvider.needsNormalization()` and `normalize()` exist + - CONTEXT.md says: "Renormalize vs. block" is Claude's discretion + - Recommendation: Renormalize proactively when detected + +## Test Strategy + +### Migration Required + +The existing 17 XCTest tests need migration to Swift Testing: + +| XCTest Method | Swift Testing Equivalent | +|---------------|--------------------------| +| `XCTestCase` class | `@Suite` struct | +| `func test_*` | `@Test("description") func` | +| `XCTAssertTrue(x)` | `#expect(x == true)` | +| `XCTAssertFalse(x)` | `#expect(x == false)` | +| `XCTAssertEqual(a, b)` | `#expect(a == b)` | +| `XCTAssertNil(x)` | `#expect(x == nil)` | + +### Test Categories to Verify + +| Category | Current Count | Coverage | +|----------|---------------|----------| +| Game immutability (CONS-01) | 1 test | Complete | +| Travel day range (CONS-02) | 4 tests | Complete | +| Travel sortOrder constraints (CONS-03) | 4 tests | Complete | +| Custom item flexibility (CONS-04) | 2 tests | Complete | +| Edge cases | 1 test | Impossible constraints | +| Barrier games | 1 test | Visual highlighting | + +## Sources + +### Primary (HIGH confidence) +- `SportsTime/Core/Models/Domain/ItineraryConstraints.swift` - Full implementation reviewed +- `SportsTimeTests/ItineraryConstraintsTests.swift` - 17 test cases reviewed +- `SportsTime/Features/Trip/Views/ItineraryTableViewController.swift` - Integration verified +- Phase 1 research and summaries - Pattern consistency verified + +### Secondary (MEDIUM confidence) +- CONTEXT.md decisions - Visual styling details are Claude's discretion +- CLAUDE.md test patterns - Swift Testing is project standard + +## Metadata + +**Confidence breakdown:** +- Constraint implementation: HIGH - Code reviewed, all CONS-* requirements met +- Test coverage: HIGH - 17 existing tests cover all requirements +- UI integration: HIGH - Already used in ItineraryTableViewController +- Migration path: HIGH - Clear XCTest -> Swift Testing mapping + +**Research date:** 2026-01-18 +**Valid until:** Indefinite (constraint logic is stable) + +## Recommendations for Planning + +### Phase 2 Scope (Refined) + +Given that `ItineraryConstraints` is already implemented and tested: + +1. **Verify existing tests cover all requirements** - Compare against CONS-01 through CONS-04 +2. **Migrate tests to Swift Testing** - Match Phase 1 patterns +3. **Add any missing edge case tests** - e.g., empty trip, single-day trip +4. **Document constraint API** - For Phase 4 UI integration reference + +### What NOT to Build + +- The constraint checking logic already exists and works +- The UI integration already exists in `ItineraryTableViewController` +- The visual feedback (dimming, barriers) already exists + +### Minimal Work Required + +Phase 2 is essentially a **verification and standardization phase**, not a building phase: +- Verify implementation matches requirements +- Standardize tests to project patterns +- Document for downstream phases