From 632760e24ce7275359da43f1e6b29572f8ead972 Mon Sep 17 00:00:00 2001 From: Trey t Date: Sun, 18 Jan 2026 15:46:27 -0600 Subject: [PATCH] docs(03): research visual flattening phase Phase 3: Visual Flattening - Analyzed existing flatten implementation in ItineraryTableViewController - Documented current bucket-based approach (beforeGames/afterGames) - Recommended pure sortOrder-based sorting within each day - Identified ItineraryFlattener extraction for testability - Documented determinism test patterns Co-Authored-By: Claude Opus 4.5 --- .../03-visual-flattening/03-RESEARCH.md | 537 ++++++++++++++++++ 1 file changed, 537 insertions(+) create mode 100644 .planning/phases/03-visual-flattening/03-RESEARCH.md diff --git a/.planning/phases/03-visual-flattening/03-RESEARCH.md b/.planning/phases/03-visual-flattening/03-RESEARCH.md new file mode 100644 index 0000000..29bb131 --- /dev/null +++ b/.planning/phases/03-visual-flattening/03-RESEARCH.md @@ -0,0 +1,537 @@ +# Phase 3: Visual Flattening - Research + +**Researched:** 2026-01-18 +**Domain:** UITableView data flattening, sortOrder-based display ordering +**Confidence:** HIGH + +## Summary + +This research reveals that visual flattening is **already implemented** in `ItineraryTableViewController.reloadData()`. The existing implementation flattens hierarchical `[ItineraryDayData]` into a linear `[ItineraryRowItem]` array for UITableView display. However, the current implementation has a critical architectural issue: it uses a **hard-coded flatten order** (travel, header, before-games items, games, after-games items) rather than purely sorting by sortOrder. + +Phase 3's goal is to ensure that flattening is **deterministic and stateless** - the same semantic state (items with their day + sortOrder) should always produce identical row order. The CONTEXT.md decisions confirm: "Items just flow in sortOrder order; games are distinguished by their visual style" and "sortOrder < 0 renders ABOVE games; sortOrder >= 0 renders BELOW games." + +The existing `ItineraryRowItem` enum and day-based section structure are sound. The work needed is to refactor the flattening logic to use sortOrder as the primary sort key within each day, making the flatten algorithm a pure function of semantic state. + +**Primary recommendation:** Refactor `reloadData()` to sort all items within a day by sortOrder. Extract flattening to a pure function for testability. Add snapshot tests to verify determinism. + +## Codebase Analysis + +### Key Files + +| File | Purpose | Status | +|------|---------|--------| +| `ItineraryTableViewController.swift` | UITableView + flattening logic | Contains current implementation | +| `ItineraryTableViewWrapper.swift` | SwiftUI bridge, builds ItineraryDayData | Passes data to controller | +| `ItineraryItem.swift` | Domain model with (day, sortOrder) | Complete (Phase 1) | +| `SortOrderProvider.swift` | sortOrder utilities | Complete (Phase 1) | +| `ItineraryConstraints.swift` | Position validation | Complete (Phase 2) | + +### Existing Flatten Implementation + +The current implementation in `ItineraryTableViewController.reloadData()` (lines 484-545): + +```swift +// Source: ItineraryTableViewController.swift +func reloadData( + days: [ItineraryDayData], + travelValidRanges: [String: ClosedRange], + itineraryItems: [ItineraryItem] = [] +) { + // ... + flatItems = [] + + for day in days { + // 1. Travel that arrives on this day (renders BEFORE the day header) + if let travel = day.travelBefore { + flatItems.append(.travel(travel, dayNumber: day.dayNumber)) + } + + // 2. Day header with Add button + flatItems.append(.dayHeader(dayNumber: day.dayNumber, date: day.date)) + + // 3. Movable items split around games boundary + var beforeGames: [ItineraryRowItem] = [] + var afterGames: [ItineraryRowItem] = [] + + for row in day.items { + let so = /* get sortOrder */ + if sortOrder < 0 { + beforeGames.append(row) + } else { + afterGames.append(row) + } + } + + flatItems.append(contentsOf: beforeGames) + + // 4. Games for this day + if !day.games.isEmpty { + flatItems.append(.games(day.games, dayNumber: day.dayNumber)) + } + + flatItems.append(contentsOf: afterGames) + } +} +``` + +### Issues with Current Implementation + +1. **Travel handled specially** - `travelBefore` is a separate property, not a positioned item +2. **Hard-coded structure** - Header always first, games always between before/after sections +3. **Not purely sortOrder-driven** - Items are split into buckets by sign, then appended in bucket order +4. **Data split across layers** - `ItineraryDayData.items` vs `ItineraryDayData.travelBefore` + +### Target Architecture per CONTEXT.md + +From CONTEXT.md decisions: +- "sortOrder = 0 is the first game position; negatives appear before games, positives appear after/between" +- "Purely sequential sorting within a day - no time-of-day grouping" +- "Items just flow in sortOrder order; games are distinguished by their visual style" +- "Flattening is a pure function - stateless, same input always produces same output" + +## Standard Stack + +### Core +| Library | Version | Purpose | Why Standard | +|---------|---------|---------|--------------| +| Swift `sorted(by:)` | Swift stdlib | Sorting items | Stable sort, predictable | +| Foundation `Double` | Swift stdlib | sortOrder comparison | Already established in Phase 1 | + +### Supporting +| Library | Version | Purpose | When to Use | +|---------|---------|---------|-------------| +| Swift Testing | Swift 5.10+ | Snapshot comparison tests | Verifying determinism | + +### Alternatives Considered +| Instead of | Could Use | Tradeoff | +|------------|-----------|----------| +| In-place flattening | Separate flatten function | Function is more testable; choose function | +| UITableView sections | Single section with inline headers | Drag-drop across sections is complex; choose single section | + +## Architecture Patterns + +### Pattern 1: Pure Flatten Function + +**What:** Extract flattening logic to a pure function that takes semantic items and returns display rows +**When to use:** All flattening operations - enables unit testing, ensures determinism +**Example:** +```swift +// Source: Recommended implementation +enum ItineraryFlattener { + /// Flatten semantic items into display rows, sorted by (day, sortOrder) + /// + /// For each day: + /// 1. Day header row (always first) + /// 2. All items sorted by sortOrder + /// - sortOrder < 0: before games (travel, custom) + /// - sortOrder 100-1540: games (fixed by schedule) + /// - sortOrder > game sortOrder: after games (travel, custom) + /// + /// - Parameters: + /// - days: Day metadata (number, date) + /// - items: All ItineraryItem models with (day, sortOrder) + /// - games: RichGame data indexed by day + /// - Returns: Ordered array of display rows + static func flatten( + days: [(dayNumber: Int, date: Date)], + items: [ItineraryItem], + gamesByDay: [Int: [RichGame]] + ) -> [ItineraryRowItem] { + var result: [ItineraryRowItem] = [] + + for day in days { + // Day header always first + result.append(.dayHeader(dayNumber: day.dayNumber, date: day.date)) + + // Collect all items for this day + var dayItems: [(sortOrder: Double, row: ItineraryRowItem)] = [] + + // Add games (get sortOrder from game time) + if let games = gamesByDay[day.dayNumber], !games.isEmpty { + let gameSortOrder = SortOrderProvider.initialSortOrder( + forGameTime: games.first!.game.dateTime + ) + dayItems.append((gameSortOrder, .games(games, dayNumber: day.dayNumber))) + } + + // Add travel and custom items + for item in items where item.day == day.dayNumber { + switch item.kind { + case .travel(let info): + if let segment = /* find matching TravelSegment */ { + dayItems.append((item.sortOrder, .travel(segment, dayNumber: day.dayNumber))) + } + case .custom: + dayItems.append((item.sortOrder, .customItem(item))) + case .game: + // Games handled separately above + break + } + } + + // Sort by sortOrder and append + dayItems.sort { $0.sortOrder < $1.sortOrder } + result.append(contentsOf: dayItems.map(\.row)) + } + + return result + } +} +``` + +### Pattern 2: Games as Positioned Items + +**What:** Games have sortOrder derived from game time (100 + minutes since midnight) +**When to use:** All flattening - games are anchors, not special cases +**Example:** +```swift +// Source: SortOrderProvider.swift (existing) +static func initialSortOrder(forGameTime gameTime: Date) -> Double { + let calendar = Calendar.current + let components = calendar.dateComponents([.hour, .minute], from: gameTime) + let minutesSinceMidnight = (components.hour ?? 0) * 60 + (components.minute ?? 0) + return 100.0 + Double(minutesSinceMidnight) // Range: 100-1540 +} +``` + +### Pattern 3: Travel as Positioned Item (Not Day Property) + +**What:** Travel segments stored as ItineraryItem with (day, sortOrder), not `travelBefore` property +**When to use:** All travel handling - unified positioning model +**Example:** +```swift +// Instead of: ItineraryDayData.travelBefore +// Use: ItineraryItem with kind: .travel(TravelInfo) and its own sortOrder + +// Travel before games: sortOrder < 100 (e.g., 50.0) +// Travel between games: sortOrder between game sortOrders +// Travel after games: sortOrder > last game sortOrder +``` + +### Anti-Patterns to Avoid + +- **Bucket-then-append:** Don't split items into "before games" and "after games" buckets. Sort by sortOrder directly. +- **Special-case travel:** Travel is just an item with sortOrder. Don't use `travelBefore` day property. +- **Hard-coded order:** Don't assume "header, travel, games, custom" structure. Let sortOrder determine order. +- **Row-index based testing:** Test with sortOrder values, not "item at index 2." + +## Don't Hand-Roll + +| Problem | Don't Build | Use Instead | Why | +|---------|-------------|-------------|-----| +| Sort stability | Custom stable sort | Swift `sorted(by:)` | Already stable since Swift 5 | +| Day grouping | Manual loop with counters | `Dictionary(grouping:by:)` | Built-in, cleaner | +| Determinism verification | Manual comparison | Snapshot test with Codable | Existing test patterns | +| TravelSegment lookup | Iteration every time | Dictionary keyed by cities | O(1) lookup | + +**Key insight:** The flatten algorithm itself is simple (sort by sortOrder within each day). The complexity is in data transformation (ItineraryDayData -> unified items) which the wrapper already handles. + +## Common Pitfalls + +### Pitfall 1: Games Getting Wrong sortOrder + +**What goes wrong:** Games don't sort correctly relative to travel/custom items +**Why it happens:** Games treated as special case instead of having their own sortOrder +**How to avoid:** Always derive game sortOrder from game time using `SortOrderProvider.initialSortOrder(forGameTime:)` +**Warning signs:** Games appear at wrong position; travel appears after games when it should be before + +### Pitfall 2: Sort Instability on Equal sortOrder + +**What goes wrong:** Items with same sortOrder appear in different order across reloads +**Why it happens:** Swift sort is stable but order depends on input order for equal keys +**How to avoid:** +1. Per CONTEXT.md: "Ties shouldn't occur - system always assigns unique sortOrder when positioning" +2. Tiebreaker per CONTEXT.md: "by item type priority: games > travel > custom" if needed +**Warning signs:** Same semantic state produces different visual order + +### Pitfall 3: Day Header Position Assumption + +**What goes wrong:** Items placed "before" day header +**Why it happens:** sortOrder < 0 for "before games" doesn't mean "before day header" +**How to avoid:** Day header is always first row of each day, not a positioned item +**Warning signs:** Travel or custom items appearing above day header + +### Pitfall 4: Reload Loses Position + +**What goes wrong:** Item positions revert after data reload +**Why it happens:** Flattening not using persisted sortOrder, instead using default/computed values +**How to avoid:** Always use ItineraryItem.sortOrder from storage, never recompute for existing items +**Warning signs:** User moves item, reload happens, item snaps back to original position + +## Code Examples + +### Current Data Flow + +```swift +// Source: ItineraryTableViewWrapper.buildItineraryData() +// 1. Calculate trip days +let tripDays = calculateTripDays() + +// 2. Build travel items with (day, sortOrder) +for segment in trip.travelSegments { + // ... calculate valid range, placement + let travelItem = ItineraryItem( + tripId: trip.id, + day: placement.day, + sortOrder: placement.sortOrder, + kind: .travel(TravelInfo(...)) + ) + travelItems.append(travelItem) +} + +// 3. Build day data with custom items and travel rows +for dayDate in tripDays { + let dayNum = index + 1 + var rows: [ItineraryRowItem] = [] + + // Custom items + let customItemsForDay = itineraryItems + .filter { $0.day == dayNum && $0.isCustom } + .sorted { $0.sortOrder < $1.sortOrder } + for item in customItemsForDay { + rows.append(.customItem(item)) + } + + // Travel items + for travel in travelItems.filter({ $0.day == dayNum }) { + // ... convert to row + } + + // Sort by sortOrder + rows.sort { /* by sortOrder */ } + + days.append(ItineraryDayData( + dayNumber: dayNum, + date: dayDate, + games: gamesOnDay, + items: rows, + travelBefore: nil // Currently unused + )) +} +``` + +### Recommended Flatten Logic + +```swift +// Source: Recommended refactoring +extension ItineraryTableViewController { + /// Flatten day data into display rows + /// Pure function: same input always produces same output + private func flattenToRows(days: [ItineraryDayData]) -> [ItineraryRowItem] { + var result: [ItineraryRowItem] = [] + + for day in days { + // 1. Day header (always first in day) + result.append(.dayHeader(dayNumber: day.dayNumber, date: day.date)) + + // 2. Collect all positionable items with sortOrder + var positioned: [(sortOrder: Double, row: ItineraryRowItem)] = [] + + // Games get sortOrder from first game time + if !day.games.isEmpty { + let gameSortOrder = SortOrderProvider.initialSortOrder( + forGameTime: day.games.first!.game.dateTime + ) + positioned.append((gameSortOrder, .games(day.games, dayNumber: day.dayNumber))) + } + + // Other items already have sortOrder + for row in day.items { + let sortOrder: Double + switch row { + case .customItem(let item): + sortOrder = item.sortOrder + case .travel(let segment, _): + sortOrder = findItineraryItem(for: segment)?.sortOrder ?? 0.0 + default: + continue // Skip non-positionable items + } + positioned.append((sortOrder, row)) + } + + // 3. Sort by sortOrder (stable sort) + positioned.sort { $0.sortOrder < $1.sortOrder } + + // 4. Append in sorted order + result.append(contentsOf: positioned.map(\.row)) + } + + return result + } +} +``` + +### Determinism Test Pattern + +```swift +// Source: Recommended test approach +@Suite("ItineraryFlattener") +struct ItineraryFlattenerTests { + + @Test("same state produces identical row order") + func flatten_sameState_producesIdenticalOrder() { + let items = makeTestItems() // Fixed test data + + // Flatten twice + let result1 = ItineraryFlattener.flatten(days: days, items: items, gamesByDay: games) + let result2 = ItineraryFlattener.flatten(days: days, items: items, gamesByDay: games) + + // Compare IDs (stable identifiers) + let ids1 = result1.map(\.id) + let ids2 = result2.map(\.id) + + #expect(ids1 == ids2) + } + + @Test("item with sortOrder -1.0 appears before games") + func flatten_negativeSortOrder_appearsBeforeGames() { + let tripId = UUID() + let customItem = ItineraryItem( + tripId: tripId, + day: 1, + sortOrder: -1.0, + kind: .custom(CustomInfo(title: "Pre-game snack", icon: "fork.knife")) + ) + + let result = ItineraryFlattener.flatten( + days: [(dayNumber: 1, date: Date())], + items: [customItem], + gamesByDay: [1: testGames] // Games have sortOrder ~820 (noon) + ) + + // Find positions + let customIndex = result.firstIndex { $0.id.contains("item:") }! + let gamesIndex = result.firstIndex { $0.id.starts(with: "games:") }! + + #expect(customIndex < gamesIndex) + } + + @Test("reorder then reflatten preserves new order") + func flatten_afterReorder_preservesNewOrder() { + var items = makeTestItems() + + // Simulate reorder: move item from sortOrder 1.0 to between 2.0 and 3.0 + if let idx = items.firstIndex(where: { $0.sortOrder == 1.0 }) { + items[idx].sortOrder = 2.5 + } + + // Flatten + let result = ItineraryFlattener.flatten(days: days, items: items, gamesByDay: games) + + // Verify order reflects new sortOrder + let customIndices = result.enumerated() + .filter { $0.element.id.contains("item:") } + .map { $0.offset } + + // Items should now be in order: 2.0, 2.5, 3.0 + // (not reverted to original 1.0, 2.0, 3.0) + let sortOrders = customIndices.compactMap { idx -> Double? in + if case .customItem(let item) = result[idx] { + return item.sortOrder + } + return nil + } + + #expect(sortOrders == [2.0, 2.5, 3.0]) + } +} +``` + +## State of the Art + +| Old Approach | Current Approach | When Changed | Impact | +|--------------|------------------|--------------|--------| +| Hard-coded order | sortOrder-based sorting | Phase 3 | Deterministic flattening | +| Travel as day property | Travel as ItineraryItem | Wrapper already does this | Unified positioning | +| Games as special case | Games with derived sortOrder | Phase 1 established convention | Consistent ordering | + +**Deprecated/outdated:** +- `ItineraryDayData.travelBefore` property (should be removed) +- Bucket-based flatten (beforeGames / afterGames arrays) + +## Open Questions + +### Resolved by Research + +1. **Where does flattening live?** + - Answer: `ItineraryTableViewController.reloadData()` (current), should extract to `ItineraryFlattener` utility + - Confidence: HIGH + +2. **How do games get sortOrder?** + - Answer: `SortOrderProvider.initialSortOrder(forGameTime:)` - already established in Phase 1 + - Range: 100-1540 (minutes since midnight + 100) + - Confidence: HIGH + +3. **What's the day header position?** + - Answer: Always first in each day's section, not a positioned item + - From CONTEXT.md: "Day headers display day number + date... Headers scroll with content - not sticky" + - Confidence: HIGH + +### Claude's Discretion per CONTEXT.md + +1. **UITableView sections vs inline headers?** + - Current: Single section with inline day headers as rows + - Recommendation: Keep single section (simpler drag-drop) + +2. **Travel/custom in 100-1540 range?** + - Convention: sortOrder < 100 for "before all games", sortOrder > max game for "after all games" + - Between games: use midpoint between adjacent game sortOrders + +3. **Tiebreaker for identical sortOrder?** + - Per CONTEXT.md: "by item type priority: games > travel > custom" + - Implementation: Add secondary sort key if needed + +## Sources + +### Primary (HIGH confidence) +- `SportsTime/Features/Trip/Views/ItineraryTableViewController.swift` - Current flatten implementation (lines 484-545) +- `SportsTime/Features/Trip/Views/ItineraryTableViewWrapper.swift` - Data preparation layer +- `.planning/phases/03-visual-flattening/03-CONTEXT.md` - User decisions +- `.planning/phases/01-semantic-position-model/01-RESEARCH.md` - sortOrder conventions + +### Secondary (MEDIUM confidence) +- Phase 1 implementation: `SortOrderProvider.swift`, `ItineraryItem.swift` +- Phase 2 implementation: `ItineraryConstraints.swift` + +## Metadata + +**Confidence breakdown:** +- Current implementation: HIGH - Full code review completed +- Refactoring approach: HIGH - Clear path from current to target +- Test strategy: HIGH - Determinism tests are straightforward + +**Research date:** 2026-01-18 +**Valid until:** Indefinite (architectural pattern, not framework-dependent) + +## Recommendations for Planning + +### Phase 3 Scope + +1. **Extract flatten logic to pure function** + - Move from `reloadData()` inline to `ItineraryFlattener.flatten()` + - Enable unit testing without UITableView + +2. **Remove bucket-based flattening** + - Replace beforeGames/afterGames arrays with single sorted collection + - Sort all items by sortOrder directly + +3. **Add determinism tests** + - Test: same input -> same output + - Test: sortOrder -1.0 appears before games + - Test: reorder + reflatten preserves new order + +4. **Clean up travelBefore property** + - Verify wrapper already handles travel as positioned items + - Remove unused `travelBefore` from `ItineraryDayData` if confirmed + +### What NOT to Build + +- New sortOrder calculation logic (Phase 1 complete) +- New constraint validation (Phase 2 complete) +- Drag-drop interaction (Phase 4) + +### Dependencies + +- Phase 1: sortOrder conventions (complete) +- Phase 2: constraint validation (complete) +- Phase 4: will consume flatten output for drag-drop