scramble branch worktree-packing-day-before commits 1 files 18 touched lines +151 / -54 tests ScrambleTests pass (device)

Pre-push review: Move pack-mode packing list to Day before

One commit (30771b7) relocating the pack-mode packing list from the Departure day phase to the Day before phase, plus the review fixes folded in.

At a glance

  • Pack mode moves .departureDay.dayBefore; repack stays on .dayBeforeReturn — symmetric “day-before-X” placement.
  • New Phase.packingMode computed property is the single source of truth for the packing-phase predicate and phase→mode mapping, consumed by AccordionTimeline and TripDetailView.autoExpandPhase.
  • Efficiency-neutral: phaseSubline still runs exactly twice per timeline render, just on a different row.
  • Spec divergence from accepted Phase 3/4 ACs is documented as decision_log.md Decision 11; CLAUDE.md and the UI design doc updated to match.
  • Behavioural edge: on a 2-day trip where Departure day == Day-before-return, nothing auto-expands that day (first-current-wins shadow). Accepted in the ADR.

Verdict

Ready to push

The change is small, internally consistent, and now centralised behind a single Phase.packingMode source of truth. All four review agents' findings were addressed: the spec divergence is recorded as Decision 11, stale living-docs were updated, and a PhasePackingModeTests suite pins the new mapping. Unit tests (ScrambleTests) pass on the connected device.

Caveat: the iOS Simulator is unavailable (CoreSimulator out of date), so the UI-test target compiles but was not executed. Run make test once the simulator is restored before relying on the UI suite.

Review findings

8 raised · 6 fixed · 2 skipped

Jump to findings →

Commits

Three-level explanation

What changed

Scramble shows a trip as a vertical timeline of phases (Weeks before, Day before, Departure day, …). The per-person packing checklist used to appear under Departure day. This change moves it one phase earlier, to Day before — because you pack the evening before you leave, not on the morning of departure.

Why it matters

It matches how people actually prepare for a trip, and it lines up nicely with the repacking checklist that already lives on “Day before return.” Now both checklists sit on a “day before” phase.

Key concepts

  • A phase is one step on the trip timeline.
  • The little suitcase icon and the packing summary now belong to the Day-before phase.
  • Departure day becomes an ordinary tasks-only phase.

Architecture

The “which phase hosts packing, and in which mode” knowledge was previously inlined as phase == .departureDay || phase == .dayBeforeReturn in two places (the timeline row builder and the auto-expand helper) plus a separate ternary for the PackingMode. This change introduces Phase.packingMode: PackingMode? (.dayBefore → .pack, .dayBeforeReturn → .repack, else nil) so both call-sites derive from one definition and can never drift.

Patterns

  • AccordionTimeline: let packingMode = phase.packingMode; let packing = packingMode != nil.
  • TripDetailView.autoExpandPhase: the always-expandable gate becomes if phase.packingMode != nil.
  • PhaseNode: the suitcase.fill glyph case moves to .dayBefore.

Trade-offs

A small coupling is added (the Phase enum now references PackingMode, both in the same module), bought in exchange for removing two-site duplication. The UI-test seed was shifted so the trip sits on .dayBefore (today == start - 1) and the pack summary still auto-expands.

Deep dive

The behavioural move is a constant swap, but the load-bearing detail is autoExpandPhase's first-current-wins rule over Phase.allCases order (weeksBefore, dayBefore, departureDay, duringTrip, dayBeforeReturn, returnDay, afterTrip). Because .departureDay is no longer a packing phase, two edges shift: (1) on a 1-day trip the pack list is reachable only on the prior day; (2) on a 2-day trip where departureDay and dayBeforeReturn are both .current, the scan hits departureDay first, finds it non-packing with no tasks, and returns nil — it does not fall through to the repack phase. AutoExpandTests.compressedDuringTripGuard now asserts that nil.

Architecture impact

Phase.packingMode is now the canonical predicate; any future surface that needs “is this a packing phase?” should use it rather than re-deriving. PhasePackingModeTests pins the mapping (including that .departureDay is nil), so a regression that re-adds departure-day packing fails a unit test, not just a UI test.

Edge cases & verification

Efficiency is conserved (packingSubline gated behind packingMode.map, still two phases). The divergence from accepted Phase 3 (AC 1.5 / 2.6 / 3.3) and Phase 4 (Intro / mapping / 1.1 / 1.10) ACs is recorded as Decision 11, with the accepted ACs left as historical record and the living docs (CLAUDE.md, scramble-ui-design-doc.md, implementation-phases.md) updated. ScrambleTests pass on device; the simulator is out of date so UI tests are compile-verified only.

Important changes — detailed

Phase.packingMode — single source of truth for packing placement

Scramble/Scramble/Models/Enums.swift

Why it matters. Removes the pre-existing two-site duplication of the packing-phase predicate so the timeline and auto-expand logic can never disagree about where packing lives.

What to look at. Enums.swift Phase.packingMode

Takeaway. When the same enum-classification predicate appears at 2+ call sites, hoist it to a computed property on the enum; the call sites become phase.packingMode (!= nil) and intent is centralised.
Rationale. Flagged by the code-reuse review as the one centralisation worth doing precisely because both edited call sites were already being touched.

AccordionTimeline derives packing from phase.packingMode

Scramble/Scramble/Features/Trips/AccordionTimeline.swift

Why it matters. This is the render path that decides whether a phase shows the packing summary block + sheet entry and which mode.

What to look at. AccordionTimeline.row(for:variant:proxy:)

Takeaway. packingSubline stays gated behind packingMode.map, so the O(items) scan still runs only for phases that actually host packing.
Rationale. Efficiency review confirmed the move is work-conserving (two phaseSubline passes per render, unchanged).

autoExpandPhase gate via packingMode

Scramble/Scramble/Features/Trips/TripDetailView.swift

Why it matters. Controls which current phase auto-expands on launch; .dayBefore must now be always-expandable and .departureDay must not.

What to look at. TripDetailView.autoExpandPhase

Takeaway. First-current-wins over Phase.allCases means a non-packing current phase can shadow a later packing one (the 2-day-trip edge).
Rationale. Behaviour preserved deliberately; changing the scan rule was rejected as out of scope in Decision 11.

Decision 11 ADR records the spec divergence

specs/phase-4-packing-sheet/decision_log.md

Why it matters. The move reverses accepted Phase 3/4 acceptance criteria; project convention requires an Enhanced-Nygard ADR superseding them.

What to look at. decision_log.md Decision 11

Takeaway. Accepted spec ACs are left as historical record; the ADR is the authoritative override and the living docs are updated to match.
Rationale. Mirrors the Decision 10 precedent in the same log (supersede-named-ACs + authoritative-override note).

UITestSeed phase4-pack-mode-trip shifted to .dayBefore

Scramble/Scramble/Persistence/UITestSeed.swift

Why it matters. The pack-mode UI tests rely on the seeded trip's current phase auto-expanding to reveal the summary rows.

What to look at. UITestSeed.seedPhase4 (start = day+1, end = day+6)

Takeaway. today == start - 1 puts the trip on .dayBefore so the pack summary auto-expands, matching the old departure-day setup one phase earlier.
Rationale. inferred (inferred — not stated by the author)

Key decisions

Move pack mode to Day-before (Decision 11).

Pack mode relocates from .departureDay to .dayBefore; repack stays on .dayBeforeReturn. You pack the day before departure, and the placement becomes symmetric. Recorded in specs/phase-4-packing-sheet/decision_log.md as Decision 11, superseding the named Phase 3/4 ACs.

Centralise via Phase.packingMode rather than leave two-site duplication.

The predicate and mode mapping are hoisted to a computed property on Phase (same module as PackingMode). Slight enum→mode coupling accepted to buy a single source of truth and prevent timeline/auto-expand drift.

Leave accepted spec ACs as historical; update only living docs.

The Phase 3/4 requirements.md/design.md ACs are not rewritten — the ADR supersedes them. Only the living docs (CLAUDE.md, scramble-ui-design-doc.md, implementation-phases.md, agent note) were corrected.

Do not change first-current-wins for the 2-day-trip edge.

On a 2-day trip where Departure day and Day-before-return coincide, nothing auto-expands that day. Reworking autoExpandPhase to prefer an expandable current phase was rejected as out of scope; revisit if the edge proves annoying.

Review findings

SeverityAreaFindingResolution
majorspecs/phase-4-packing-sheet (spec adherence)Behavioural reversal of accepted Phase 3 (AC 1.5/2.6/3.3) and Phase 4 (Intro/mapping/1.1/1.10) ACs with no decision-log entry.Added Decision 11 ADR superseding the named ACs, mirroring the Decision 10 precedent.
majordocs (CLAUDE.md, scramble-ui-design-doc.md, implementation-phases.md)Multiple source-of-truth docs still described packing as a Departure-day surface (glyph table, accordion-expandable list, packing-block section, impl-order).Updated all living-doc references to Day-before; left accepted spec ACs as historical record per the ADR.
minorAccordionTimeline.swift + TripDetailView.swift (reuse)The packing-phase predicate + phase->mode mapping were duplicated across two call sites (pre-existing).Hoisted to Phase.packingMode; both sites now derive from it.
minorAutoExpandTests.swift (quality)Tests hardcoded Self.date(2026, 5, 31) as 'start - 1', which would silently drift if start moved across a month boundary.Derive the date via calendar.date(byAdding: .day, value: -1, to: start).
minorPhaseNode.swift / PackingListHelpers.swift / PackingSummarySection.swift / PackingSheet.swift (doc comments)Doc comments still described the pack surface as living on the Departure phase, contradicting the code after the move.Updated all four doc comments to Day-before.
minortest coverageNothing asserted that .departureDay is no longer a packing phase (the exact regression a revert would reintroduce).Added PhasePackingModeTests pinning the full mapping incl. .departureDay -> nil.
minorautoExpandPhase (2-day-trip edge)On a 2-day trip where Departure day == Day-before-return, nothing auto-expands (first-current-wins shadow).Left as-is; documented in Decision 11 as an accepted edge. Changing the scan rule was out of scope.
minorverification (full suite)make test (full incl. UI) could not run: CoreSimulator out of date, no simulator available.Ran ScrambleTests on the connected device (pass); UI target compile-verified only. Flagged for a simulator re-run before push.

Per-file diffs

Click to expand.

Scramble/Scramble/Models/Enums.swift Modified +13 / -0
diff --git a/Scramble/Scramble/Models/Enums.swift b/Scramble/Scramble/Models/Enums.swiftindex 3823cc3..0fd21ac 100644--- a/Scramble/Scramble/Models/Enums.swift+++ b/Scramble/Scramble/Models/Enums.swift@@ -20,6 +20,19 @@ nonisolated enum Phase: String, Codable, CaseIterable, Hashable, Sendable {     case .afterTrip: "After trip"     }   }++  /// The packing surface this phase hosts, or `nil` if it hosts none.+  /// Pack mode lives on `.dayBefore` (you pack the day before departure);+  /// repack on `.dayBeforeReturn`. Single source of truth for both the+  /// "is this a packing phase?" predicate and the phase→mode mapping used by+  /// `AccordionTimeline` and `TripDetailView.autoExpandPhase`.+  var packingMode: PackingMode? {+    switch self {+    case .dayBefore: .pack+    case .dayBeforeReturn: .repack+    default: nil+    }+  } }  nonisolated enum ItemSource: String, Codable, CaseIterable, Hashable, Sendable {
Scramble/Scramble/Features/Trips/AccordionTimeline.swift Modified +2 / -3
diff --git a/Scramble/Scramble/Features/Trips/AccordionTimeline.swift b/Scramble/Scramble/Features/Trips/AccordionTimeline.swiftindex 0eafc3f..3423eac 100644--- a/Scramble/Scramble/Features/Trips/AccordionTimeline.swift+++ b/Scramble/Scramble/Features/Trips/AccordionTimeline.swift@@ -70,10 +70,9 @@ struct AccordionTimeline: View {     let phaseTasks = (trip.tasks ?? []).filter { $0.phase == phase && !$0.userDeletedOnThisTrip }     let counts = TaskListHelpers.counts(phaseTasks)     let compressed = PhaseDateMapping.isCompressed(phase, for: trip, calendar: calendar)-    let packing = phase == .departureDay || phase == .dayBeforeReturn+    let packingMode = phase.packingMode+    let packing = packingMode != nil     let colour = variant.phaseColour(for: phase)-    let packingMode: PackingMode? =-      phase == .departureDay ? .pack : (phase == .dayBeforeReturn ? .repack : nil)     let packingSubline = packingMode.map { PackingListHelpers.phaseSubline(trip, mode: $0) }      PhaseRow(
Scramble/Scramble/Features/Trips/TripDetailView.swift Modified +2 / -2
diff --git a/Scramble/Scramble/Features/Trips/TripDetailView.swift b/Scramble/Scramble/Features/Trips/TripDetailView.swiftindex 014e163..f6d29a2 100644--- a/Scramble/Scramble/Features/Trips/TripDetailView.swift+++ b/Scramble/Scramble/Features/Trips/TripDetailView.swift@@ -498,7 +498,7 @@ import os   /// Implements Req 2.3 + 3.2: finds the `.current` phase via the existing   /// `state(for:today:start:end:calendar)` helper, then gates expandability:   /// - Compressed `duringTrip` is never auto-expanded.-  /// - Packing phases (`departureDay`, `dayBeforeReturn`) are always+  /// - Packing phases (`dayBefore`, `dayBeforeReturn`) are always   ///   expandable, even with no tasks (the Phase 4 packing summary will   ///   live there).   /// - Other phases are expandable only when at least one non-soft-deleted@@ -526,7 +526,7 @@ import os     if PhaseDateMapping.isCompressed(phase, for: trip, calendar: calendar) {       return nil     }-    if phase == .departureDay || phase == .dayBeforeReturn {+    if phase.packingMode != nil {       return phase     }     let hasVisibleTask = (trip.tasks ?? []).contains { task in
Scramble/Scramble/Components/PhaseNode.swift Modified +3 / -3
diff --git a/Scramble/Scramble/Components/PhaseNode.swift b/Scramble/Scramble/Components/PhaseNode.swiftindex f06bc0b..4f8f212 100644--- a/Scramble/Scramble/Components/PhaseNode.swift+++ b/Scramble/Scramble/Components/PhaseNode.swift@@ -11,8 +11,8 @@ import SwiftUI /// /// When `isPackingPhase == true` and the state is `.current` or `.future`, an /// SF Symbol packing glyph overlays the node at ~50% of its diameter-/// (`suitcase.fill` for departure, `shippingbox.fill` for the day-before-return-/// repack). The NOW pill is owned by `PhaseRow`, not by this view.+/// (`suitcase.fill` for the day-before pack, `shippingbox.fill` for the+/// day-before-return repack). The NOW pill is owned by `PhaseRow`, not by this view. struct PhaseNode: View {   let phase: Phase   let state: PhaseNodeState@@ -85,7 +85,7 @@ struct PhaseNode: View {    private var glyphSymbol: String? {     switch phase {-    case .departureDay: "suitcase.fill"+    case .dayBefore: "suitcase.fill"     case .dayBeforeReturn: "shippingbox.fill"     default: nil     }
Scramble/Scramble/Persistence/UITestSeed.swift Modified +7 / -6
diff --git a/Scramble/Scramble/Persistence/UITestSeed.swift b/Scramble/Scramble/Persistence/UITestSeed.swiftindex 8179105..b91c79c 100644--- a/Scramble/Scramble/Persistence/UITestSeed.swift+++ b/Scramble/Scramble/Persistence/UITestSeed.swift@@ -47,8 +47,8 @@       case phase3OneDayTrip = "phase3-one-day-trip"       /// Phase 3: trip with no participants for the assignee-picker placeholder.       case phase3TripNoParticipants = "phase3-trip-no-participants"-      /// Phase 4: trip currently on `.departureDay` (today == start, end ==-      /// today+5) with two participants and a mix of packing item states. Used+      /// Phase 4: trip currently on `.dayBefore` (today == start - 1, end ==+      /// today+6) with two participants and a mix of packing item states. Used       /// by pack-mode UI tests covering the summary block, sheet groups,       /// checkbox toggle, Skip/Restore, manual add, dimmed-row counting, and       /// `WhyDisclosure` for rule-driven items.@@ -354,13 +354,14 @@     ) {       switch fixture {       case .phase4PackModeTrip:-        // Trip on `.departureDay` (today == start, end == today+5). Two+        // Trip on `.dayBefore` (today == start - 1, end == today+6). Two         // participants, each with a mix of packing item states. Includes a         // dimmed (`currentlyMatchesRules == false`, `pinnedByUser == false`)         // row to exercise the dimmed-counts path. Single qualifying trip so-        // it auto-opens; `.departureDay` auto-expands per Phase 3 rules.-        let start = day-        let end = calendar.date(byAdding: .day, value: 5, to: day) ?? day+        // it auto-opens; `.dayBefore` (the pack-mode packing phase)+        // auto-expands per Phase 3 rules.+        let start = calendar.date(byAdding: .day, value: 1, to: day) ?? day+        let end = calendar.date(byAdding: .day, value: 6, to: day) ?? day         let trip = Trip(name: "Beach Trip", startDate: start, endDate: end)         context.insert(trip) 
Scramble/Scramble/Timeline/PackingListHelpers.swift Modified +1 / -1
diff --git a/Scramble/Scramble/Timeline/PackingListHelpers.swift b/Scramble/Scramble/Timeline/PackingListHelpers.swiftindex 1717074..254dd56 100644--- a/Scramble/Scramble/Timeline/PackingListHelpers.swift+++ b/Scramble/Scramble/Timeline/PackingListHelpers.swift@@ -1,6 +1,6 @@ import Foundation -/// Which packing surface is active. `pack` is opened from the Departure phase+/// Which packing surface is active. `pack` is opened from the Day-before phase /// and operates over `unpacked` / `packed` / `excluded`. `repack` is opened /// from Day-before-return and operates over `packed` / `repacked` plus a /// read-only "Left behind" group of `unpacked` ∪ `excluded`.
Scramble/Scramble/Components/PackingSummarySection.swift Modified +1 / -1
diff --git a/Scramble/Scramble/Components/PackingSummarySection.swift b/Scramble/Scramble/Components/PackingSummarySection.swiftindex 837c1dd..7c9c302 100644--- a/Scramble/Scramble/Components/PackingSummarySection.swift+++ b/Scramble/Scramble/Components/PackingSummarySection.swift@@ -6,7 +6,7 @@ import os   import UIKit #endif -/// Per-person packing summary block rendered inside the Departure phase+/// Per-person packing summary block rendered inside the Day-before phase /// (`mode: .pack`) and the Day-before-return phase (`mode: .repack`). Each /// participant gets a `PackingSummaryRow`; tapping a row asks the parent to /// open the `PackingSheet`. Participants are sorted by snapshot `name`
Scramble/Scramble/Features/Trips/PackingSheet.swift Modified +1 / -1
diff --git a/Scramble/Scramble/Features/Trips/PackingSheet.swift b/Scramble/Scramble/Features/Trips/PackingSheet.swiftindex 3173dee..341bd98 100644--- a/Scramble/Scramble/Features/Trips/PackingSheet.swift+++ b/Scramble/Scramble/Features/Trips/PackingSheet.swift@@ -28,7 +28,7 @@ struct PackingSheetState: Identifiable { }  /// Per-person packing surface presented from a participant row inside the-/// Departure (`pack`) or Day-before-return (`repack`) phase. Owns its inner+/// Day-before (`pack`) or Day-before-return (`repack`) phase. Owns its inner /// state — active inline-add-field id, manual-add form presentation — and /// dismisses when the bound person disappears from `trip.participants` (Req 2.8). struct PackingSheet: View {
Scramble/ScrambleTests/Models/PhasePackingModeTests.swift Added +36 / -0
diff --git a/Scramble/ScrambleTests/Models/PhasePackingModeTests.swift b/Scramble/ScrambleTests/Models/PhasePackingModeTests.swiftnew file mode 100644index 0000000..b03d74e--- /dev/null+++ b/Scramble/ScrambleTests/Models/PhasePackingModeTests.swift@@ -0,0 +1,36 @@+import Testing++@testable import Scramble++/// Pins the single source of truth for "which phase hosts which packing mode".+/// `AccordionTimeline` (summary block + sheet entry) and+/// `TripDetailView.autoExpandPhase` both derive their packing behaviour from+/// `Phase.packingMode`, so this mapping defines where the packing UI appears.+@Suite("Phase.packingMode")+struct PhasePackingModeTests {++  @Test("Pack mode lives on .dayBefore")+  func dayBeforeIsPack() {+    #expect(Phase.dayBefore.packingMode == .pack)+  }++  @Test("Repack mode lives on .dayBeforeReturn")+  func dayBeforeReturnIsRepack() {+    #expect(Phase.dayBeforeReturn.packingMode == .repack)+  }++  @Test("Departure day is no longer a packing phase")+  func departureDayHostsNoPacking() {+    #expect(Phase.departureDay.packingMode == nil)+  }++  @Test("Exactly two phases are packing phases; the rest host none")+  func onlyTwoPackingPhases() {+    let packingPhases = Phase.allCases.filter { $0.packingMode != nil }+    #expect(packingPhases == [.dayBefore, .dayBeforeReturn])++    for phase in [Phase.weeksBefore, .departureDay, .duringTrip, .returnDay, .afterTrip] {+      #expect(phase.packingMode == nil)+    }+  }+}
Scramble/ScrambleTests/Trips/AutoExpandTests.swift Modified +17 / -13
diff --git a/Scramble/ScrambleTests/Trips/AutoExpandTests.swift b/Scramble/ScrambleTests/Trips/AutoExpandTests.swiftindex 83547a6..b070d23 100644--- a/Scramble/ScrambleTests/Trips/AutoExpandTests.swift+++ b/Scramble/ScrambleTests/Trips/AutoExpandTests.swift@@ -74,20 +74,21 @@ struct AutoExpandTests {   // MARK: - Packing phases are expandable even with no tasks    @Test(-    "Departure-day current, no tasks → returns .departureDay (packing phases always expandable)")+    "Day-before current, no tasks → returns .dayBefore (packing phases always expandable)")   func packingPhaseExpandableWithoutTasks() throws {     let container = try Self.makeContainer()     let context = container.mainContext      let start = Self.date(2026, 6, 1)     let end = Self.date(2026, 6, 7)-    let today = start  // today == start → .departureDay is .current+    // start - 1 → .dayBefore is .current. Derived so it tracks `start`.+    let today = Self.calendar.date(byAdding: .day, value: -1, to: start)!     let trip = Trip(name: "T", startDate: start, endDate: end)     context.insert(trip)     try context.save()      let result = TripDetailView.autoExpandPhase(for: trip, today: today, calendar: Self.calendar)-    #expect(result == .departureDay)+    #expect(result == .dayBefore)   }    @Test(@@ -133,7 +134,7 @@ struct AutoExpandTests {    // MARK: - Compressed duringTrip never selected -  @Test("Compressed duringTrip (1-day trip) is never .current; today=start returns .departureDay")+  @Test("1-day trip on the day before → returns .dayBefore (packing phase expandable)")   func compressedDuringTripFallsThroughToPacking() throws {     let container = try Self.makeContainer()     let context = container.mainContext@@ -143,12 +144,13 @@ struct AutoExpandTests {     context.insert(trip)     try context.save() -    let result = TripDetailView.autoExpandPhase(for: trip, today: day, calendar: Self.calendar)-    // Per state(for:today:start:end:calendar) on a 1-day trip with today == start:-    // - departureDay: today == start → .current  (packing, expandable)-    // - dayBeforeReturn: today == dayBeforeReturn (== start) → .current  (packing, expandable)-    // The scanning order in Phase.allCases hits departureDay first.-    #expect(result == .departureDay)+    let dayBefore = Self.calendar.date(byAdding: .day, value: -1, to: day)!+    let result = TripDetailView.autoExpandPhase(+      for: trip, today: dayBefore, calendar: Self.calendar)+    // On a 1-day trip with today == start - 1:+    // - dayBefore: today == start - 1 → .current  (packing, expandable)+    // - duringTrip is compressed and never .current.+    #expect(result == .dayBefore)   }    @Test("Compressed duringTrip is rejected if it ever surfaces as current")@@ -171,9 +173,11 @@ struct AutoExpandTests {     context.insert(task)     try context.save() -    // today == start: departureDay & dayBeforeReturn are simultaneously current.-    // Scanning order hits departureDay first.+    // today == start on a 2-day trip: departureDay & dayBeforeReturn are+    // simultaneously current. Scanning order hits departureDay first; it is+    // no longer a packing phase and has no tasks, so autoExpand returns nil.+    // The orphan duringTrip task is never returned (the point of this guard).     let result = TripDetailView.autoExpandPhase(for: trip, today: start, calendar: Self.calendar)-    #expect(result == .departureDay)+    #expect(result == nil)   } }
Scramble/ScrambleTests/Components/PhaseRowAccessibilityTests.swift Modified +2 / -2
diff --git a/Scramble/ScrambleTests/Components/PhaseRowAccessibilityTests.swift b/Scramble/ScrambleTests/Components/PhaseRowAccessibilityTests.swiftindex 0bd9adf..17ac14a 100644--- a/Scramble/ScrambleTests/Components/PhaseRowAccessibilityTests.swift+++ b/Scramble/ScrambleTests/Components/PhaseRowAccessibilityTests.swift@@ -69,12 +69,12 @@ struct PhaseRowAccessibilityTests {   @Test("Packing subline (e.g. 'Alice 2 / 3 packed') appears at the tail")   func packingSublineAppended() {     let label = PhaseRow<EmptyView>.accessibilityLabel(-      phase: .departureDay,+      phase: .dayBefore,       state: .current,       counts: PhaseCounts(completed: 0, total: 0, inactive: 0),       packingSubline: "Alice 2 / 3 packed"     )-    #expect(label == "Departure day, current phase, Alice 2 / 3 packed")+    #expect(label == "Day before, current phase, Alice 2 / 3 packed")   }    // MARK: - Hint flips with expansion state
Scramble/ScrambleUITests/PackingSheetUITests.swift Modified +10 / -9
diff --git a/Scramble/ScrambleUITests/PackingSheetUITests.swift b/Scramble/ScrambleUITests/PackingSheetUITests.swiftindex 7a292b5..7643bc6 100644--- a/Scramble/ScrambleUITests/PackingSheetUITests.swift+++ b/Scramble/ScrambleUITests/PackingSheetUITests.swift@@ -62,12 +62,12 @@ final class PackingSheetUITests: XCTestCase {   // MARK: - 1. Summary block rendering    @MainActor-  func testPackingSummaryRendersInDeparturePhase() {+  func testPackingSummaryRendersInDayBeforePhase() {     let app = launchedApp(fixture: "phase4-pack-mode-trip")     openTripDetail(app, tripName: "Beach Trip")      // Two participants seeded; both summary rows should be present once the-    // .departureDay phase auto-expands.+    // .dayBefore phase auto-expands.     let summaryRows = app.descendants(matching: .any)       .matching(NSPredicate(format: "identifier BEGINSWITH 'tripDetail.packingSummary.'"))     let predicate = NSPredicate(format: "count == 2")@@ -75,7 +75,7 @@ final class PackingSheetUITests: XCTestCase {     XCTAssertEqual(       XCTWaiter().wait(for: [expectation], timeout: 5),       .completed,-      "Departure phase should render one PackingSummaryRow per participant"+      "Day-before phase should render one PackingSummaryRow per participant"     )   } @@ -393,17 +393,18 @@ final class PackingSheetUITests: XCTestCase {     openTripDetail(app, tripName: "Beach Trip")      let header = app.descendants(matching: .any)-      .matching(identifier: "tripDetail.phaseHeader.departureDay")+      .matching(identifier: "tripDetail.phaseHeader.dayBefore")       .firstMatch     XCTAssertTrue(header.waitForExistence(timeout: 5)) -    // The departureDay header's accessibility label should append the-    // packing clause ("X to pack" or "packing ready") per Req 1.10. With-    // Arjen unpacked=1 and Sam unpacked=1 the total is 2, so the clause-    // should mention "to pack".+    // The dayBefore header's accessibility label should append the packing+    // clause ("X to pack" or "packing ready") per Req 1.10, re-pointed from+    // .departureDay to .dayBefore by decision_log Decision 11. With Arjen+    // unpacked=1 and Sam unpacked=1 the total is 2, so the clause should+    // mention "to pack".     XCTAssertTrue(       header.label.contains("to pack"),-      "Departure phase subline should include packing clause; got: \(header.label)"+      "Day-before phase subline should include packing clause; got: \(header.label)"     )   } 
specs/phase-4-packing-sheet/decision_log.md Modified +41 / -0
diff --git a/specs/phase-4-packing-sheet/decision_log.md b/specs/phase-4-packing-sheet/decision_log.mdindex 58f50fa..37a7870 100644--- a/specs/phase-4-packing-sheet/decision_log.md+++ b/specs/phase-4-packing-sheet/decision_log.md@@ -334,3 +334,44 @@ The feature was a confirmed product cost (annoying gesture) with little realised `Scramble/Scramble/Components/PackingItemRow.swift`, `Scramble/Scramble/Features/Trips/PackingSheet.swift`; removed tests in `PackingItemRowAccessibilityTests.swift` and `PackingSheetUITests.swift`; notes updated in `docs/agent-notes/packing-sheet.md` and `docs/agent-notes/accessibility.md`.  ---++## Decision 11: Pack mode lives on the Day-before phase, not Departure day++**Date**: 2026-06-28+**Status**: accepted — supersedes this spec's pack-mode placement (Introduction line 5, "Phase-to-mode mapping", Req 1 story, AC 1.1, AC 1.10) and Phase 3 (`specs/phase-3-timeline-tasks`) AC 1.5 / AC 2.6 / AC 3.3 and its compression-exemption decision, wherever they name `departureDay` as the pack-mode phase++### Context++Phase 4 placed the pack-mode packing summary (and the entry to the `PackingSheet`) on the `departureDay` phase, with repack on `dayBeforeReturn`. In use the owner found this wrong: you prepare a suitcase the day *before* you leave, not on the morning of departure. Departure day is for last things (grab toiletries, leave) — not for working a packing checklist.++### Decision++Move pack mode from `departureDay` to `dayBefore`. Repack stays on `dayBeforeReturn`. The "is this a packing phase?" predicate and the phase→mode mapping are centralised in a new `Phase.packingMode` computed property (`Scramble/Scramble/Models/Enums.swift`), consumed by `AccordionTimeline` (summary block, sheet entry, subline) and `TripDetailView.autoExpandPhase` (always-expandable gate). The `PhaseNode` suitcase glyph moves to `dayBefore`. Departure day reverts to a plain task-only phase.++### Rationale++Packing the day before departure matches how trips are actually prepared and makes the timeline symmetric: pack on `dayBefore`, repack on `dayBeforeReturn` — both "day-before-X" phases. Centralising the mapping in `Phase.packingMode` removes the pre-existing two-site duplication of the phase predicate, so the placement can never drift between the timeline and the auto-expand logic again.++### Alternatives Considered++- **Keep pack on Departure day**: Rejected — contradicts real packing behaviour; the owner explicitly asked to move it.+- **Show pack on both Day-before and Departure day**: Rejected — duplicates the same per-person summary on two adjacent phases, doubling the surface and the "where do I tap?" ambiguity for no benefit.+- **Make `autoExpandPhase` prefer an expandable current phase over the first current phase** (to fix the 2-day-trip edge below): Rejected as out of scope — it changes the documented first-current-wins rule and affects untouched phases; revisit separately if the edge proves annoying.++### Consequences++**Positive:**+- Packing is prepared on the day it actually happens; pack/repack placement is symmetric.+- `Phase.packingMode` is a single source of truth; the timeline and auto-expand can no longer disagree.+- Departure day is a quieter, task-only phase.++**Negative:**+- On a 1-day trip the pack list sits on the prior day (the `dayBefore` phase); there is no in-trip-day pack surface. Accepted.+- On a 2-day trip where Departure day and Day-before-return fall on the same calendar day, nothing auto-expands on that day: `departureDay` is scanned first under the first-current-wins rule and is no longer a packing phase, so it shadows the `dayBeforeReturn` repack surface. Edge case; not worth changing the scan rule for.+- The source-of-truth docs (`docs/scramble-ui-design-doc.md`, `CLAUDE.md`, `docs/implementation-phases.md`) and the Phase 3/4 spec ACs still describe Departure-day packing in places; this ADR is the authoritative override. The living docs were updated to match; the accepted spec ACs are left as historical record.++### Impact++`Scramble/Scramble/Models/Enums.swift` (new `Phase.packingMode`), `AccordionTimeline.swift`, `TripDetailView.swift`, `PhaseNode.swift`; doc comments in `PackingListHelpers.swift` / `PackingSummarySection.swift` / `PackingSheet.swift`; the `phase4-pack-mode-trip` seed in `UITestSeed.swift`; tests `AutoExpandTests.swift`, `PhaseRowAccessibilityTests.swift`, `PackingSheetUITests.swift`, and new `PhasePackingModeTests.swift`; notes in `docs/agent-notes/packing-sheet.md`.++---
CLAUDE.md Modified +3 / -3
diff --git a/CLAUDE.md b/CLAUDE.mdindex d0e26fe..ac2e3a0 100644--- a/CLAUDE.md+++ b/CLAUDE.md@@ -75,11 +75,11 @@ Trip Detail is a single vertical timeline of seven phases. The only tab bar in t  ### One-at-a-time accordion -Tapping a phase expands it and collapses any other expanded phase. Current phase auto-expands on launch. Phases with no content are non-expandable spine markers; packing phases (Departure, Day-before-return) are always expandable.+Tapping a phase expands it and collapses any other expanded phase. Current phase auto-expands on launch. Phases with no content are non-expandable spine markers; packing phases (Day-before, Day-before-return) are always expandable.  ### Packing reached via bottom sheet, not navigation -Tapping a person row inside Departure/Day-before-return opens a `PackingSheet` over the timeline. The timeline is **not unmounted** — scroll position is preserved on dismiss.+Tapping a person row inside Day-before/Day-before-return opens a `PackingSheet` over the timeline. The timeline is **not unmounted** — scroll position is preserved on dismiss.  ### One SwiftUI view shared by Pack and Repack modes @@ -110,7 +110,7 @@ From the UI design doc — follow this sequence when starting: 3. Timeline composition with one-at-a-time accordion 4. Task row + checkbox + assignee avatar 5. `WhyDisclosure` (tasks only; removed from packing — see phase-4 Decision 10)-6. Packing summary block on Departure / Day-before-return phases+6. Packing summary block on Day-before / Day-before-return phases 7. `PackingSheet` pack mode 8. `PackingSheet` repack mode (filter swap, read-only Left Behind group) 9. Rules engine: per-item matching condition exposure for explainability
docs/scramble-ui-design-doc.md Modified +6 / -6
diff --git a/docs/scramble-ui-design-doc.md b/docs/scramble-ui-design-doc.mdindex 6925567..c4de4ca 100644--- a/docs/scramble-ui-design-doc.md+++ b/docs/scramble-ui-design-doc.md@@ -67,7 +67,7 @@ Seven vertical nodes, one per phase, connected by a 2pt spine line. |Past                                         |Filled circle in phase colour, white checkmark inside. Spine line above also in phase colour.                                | |Current                                      |Filled circle with glow ring (`0 0 0 6pt phaseColour/22%`, `0 0 16pt phaseColour/55%`). Small “NOW” pill next to phase label.| |Future                                       |Outlined circle, dim spine line.                                                                                             |-|Packing phase (Departure / Day-before-return)|Small 🧳 / 📦 glyph inside the node when not yet past.                                                                         |+|Packing phase (Day-before / Day-before-return)|Small 🧳 / 📦 glyph inside the node when not yet past.                                                                         |  #### Phase header (next to each node) @@ -82,7 +82,7 @@ Seven vertical nodes, one per phase, connected by a 2pt spine line. - On launch, the current phase is auto-expanded. - Expanded phase scrolls to the top of the visible area on expansion. - Phases with no content (no tasks, not a packing phase) are not expandable — they render as a marker on the spine for completeness.-- Packing phases (Departure, Day-before-return) are always expandable, even if they have no tasks, because they always have a packing summary.+- Packing phases (Day-before, Day-before-return) are always expandable, even if they have no tasks, because they always have a packing summary.  ### Expanded phase content @@ -94,9 +94,9 @@ Seven vertical nodes, one per phase, connected by a 2pt spine line. - Long-press a task to see why it appears (see Explainability). - “+ Add task” dashed-border button at the end of the list. Adds a trip-specific task (`source: .manual`). -#### Packing block (Departure and Day-before-return only)+#### Packing block (Day-before and Day-before-return only) -- Section label: “Packing” (Departure) or “Repack” (Day-before-return), with subtitle “tap a person”.+- Section label: “Packing” (Day-before) or “Repack” (Day-before-return), with subtitle “tap a person”. - One row per person: avatar (26pt) + name (13pt, weight 600) + thin progress bar in person colour + status label + chevron. - Status label varies by mode:   - Pack mode: “X to pack” or “✓ ready” when all packed.@@ -120,7 +120,7 @@ A bottom sheet, ~82% screen height, presented over the timeline. The timeline is  The sheet supports two modes determined by which phase node was tapped to open it. The two modes share a single SwiftUI view; the mode flag determines group definitions, available actions, and counter text. -#### Pack mode (opened from Departure node)+#### Pack mode (opened from Day-before node)  |Group             |Filter              |Header colour| |------------------|--------------------|-------------|@@ -455,7 +455,7 @@ All theme colour pairings must pass WCAG AA. The Midnight Atlas dark variant has 1. Timeline composition with one-at-a-time accordion expand. 1. Task row + checkbox + assignee avatar. 1. `WhyDisclosure` component (used by tasks AND packing items).-1. Packing summary block (per-person rows w/ progress bar) on Departure / Day-before-return phases.+1. Packing summary block (per-person rows w/ progress bar) on Day-before / Day-before-return phases. 1. `PackingSheet` — pack mode (groups, item rows, skip/restore, add). 1. `PackingSheet` — repack mode (filter swap, read-only Left Behind group). 1. Rules engine: expose per-item matching conditions for explainability strings.
docs/implementation-phases.md Modified +1 / -1
diff --git a/docs/implementation-phases.md b/docs/implementation-phases.mdindex b41edf7..29190e2 100644--- a/docs/implementation-phases.md+++ b/docs/implementation-phases.md@@ -45,7 +45,7 @@ The other UI surface for rules-engine output.  Scope: -- Per-person packing summary block on Departure and Day-before-return phases (avatar + progress bar)+- Per-person packing summary block on Day-before and Day-before-return phases (avatar + progress bar) - `PackingSheet` — one SwiftUI view, two modes (pack / repack) per UI doc §"One component, two modes" - Pack mode groups: unpacked / packed / excluded with skip + restore actions - Repack mode groups: still in suitcase / back in suitcase / left behind (read-only)
docs/agent-notes/packing-sheet.md Modified +3 / -3
diff --git a/docs/agent-notes/packing-sheet.md b/docs/agent-notes/packing-sheet.mdindex e26582a..6624259 100644--- a/docs/agent-notes/packing-sheet.md+++ b/docs/agent-notes/packing-sheet.md@@ -1,18 +1,18 @@ # Packing sheet (Phase 4) -The per-person packing surface presented from the Departure and Day-before-return phases. Pack mode operates over `unpacked / packed / excluded`; repack mode operates over `packed / repacked` with a read-only "Left behind" group of `unpacked ∪ excluded`.+The per-person packing surface presented from the Day-before and Day-before-return phases. Pack mode lives on `.dayBefore` (you pack the day before departure, not on departure day); repack mode lives on `.dayBeforeReturn` — symmetric "day-before-X" placement. Pack mode operates over `unpacked / packed / excluded`; repack mode operates over `packed / repacked` with a read-only "Left behind" group of `unpacked ∪ excluded`.  ## Files  - `Scramble/Scramble/Timeline/PackingListHelpers.swift` — `PackingMode`, `PackingCounts`, and the helpers (`counts`, `countsByPerson`, `summaryStatus`, `progressRatio`, `phaseSubline`, `sorted`, `itemsForPerson`). The enum is `@MainActor` because the helpers traverse SwiftData `@Model` relationship arrays (`trip.packingItems`, `trip.participants`) whose ownership is tied to the main `ModelContext`. `PackingMode` and `PackingCounts` are `nonisolated` value types.-- `Scramble/Scramble/Components/PackingSummarySection.swift` — `PackingSummarySection`, `PackingSummaryRow`, and the private `PackingProgressBar`. Rendered inside the Departure / Day-before-return phase content slot, below `TaskListSection`.+- `Scramble/Scramble/Components/PackingSummarySection.swift` — `PackingSummarySection`, `PackingSummaryRow`, and the private `PackingProgressBar`. Rendered inside the Day-before / Day-before-return phase content slot, below `TaskListSection`. - `Scramble/Scramble/Components/PackingItemRow.swift` — Single row inside the sheet plus the file-scope `SheetGroup` enum (referenced from both `PackingItemRow` and `PackingSheet`). The enum carries its own `headerTitle`, `scrollAnchor`, `matches(_:)`, and `isReadOnly`. - `Scramble/Scramble/Features/Trips/PackingSheet.swift` — `PackingSheet`, `PackingSheetState`, and the private `PackingSheetHeader` and `PackingItemGroup`. - `Scramble/Scramble/Features/Trips/PackingItemForm.swift` — Sheet-on-sheet form for manual-add and rename. Carries static testable helpers (`performAdd`, `performEdit`, `isSubmitEnabled`, `cappedName`).  ## Counts and the timeline subline -`AccordionTimeline.row(for:variant:proxy:)` calls `PackingListHelpers.phaseSubline(trip, mode:)` for `.departureDay` and `.dayBeforeReturn`. The helper does **one** pass over `trip.packingItems` — earlier versions did O(participants × items) via per-person `counts(for:in:)` calls in a loop; do not regress.+`AccordionTimeline.row(for:variant:proxy:)` calls `PackingListHelpers.phaseSubline(trip, mode:)` for `.dayBefore` and `.dayBeforeReturn`. The helper does **one** pass over `trip.packingItems` — earlier versions did O(participants × items) via per-person `counts(for:in:)` calls in a loop; do not regress.  Similarly `PackingSummarySection.body` calls `PackingListHelpers.countsByPerson(trip)` **once** and then looks up each row's `PackingCounts` by `Person.id` rather than re-scanning per row. `counts(for:in:)` still exists for single-person callers and tests, but the multi-person fanout is the one to use inside `body`. 
CHANGELOG.md Modified +2 / -0
diff --git a/CHANGELOG.md b/CHANGELOG.mdindex 8452e95..4824bf9 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -34,6 +34,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).  ### Changed +- Pack-mode packing list moved from the **Departure day** phase to the **Day before** phase, so packing is prepared the day before departure (symmetric with repack on **Day before return**). The "is this a packing phase?" predicate and the phase→mode mapping are now centralised in a new `Phase.packingMode` computed property (single source of truth for `AccordionTimeline` and `TripDetailView.autoExpandPhase`); the summary block, packing-sheet entry, always-expandable gate, and the `PhaseNode` suitcase glyph all follow it onto `.dayBefore`. Departure day reverts to a plain task-only phase; on a 1-day trip the pack list lives on the prior day. The `phase4-pack-mode-trip` UI seed and the `AutoExpand` / `PhaseRow` / `PackingSheet` tests were updated, and `PhasePackingModeTests` pins the new mapping. Recorded as `specs/phase-4-packing-sheet/decision_log.md` Decision 11 (supersedes the Departure-day placement in the Phase 3/4 spec ACs); `CLAUDE.md`, `docs/scramble-ui-design-doc.md`, `docs/implementation-phases.md`, and `docs/agent-notes/packing-sheet.md` updated to match.+ - `packing-item-subitems` (T-1587) — packing-sheet note / sub-item interaction reworked from the initial reveal-on-tap affordance row into compact trailing glyphs, after device review:   - `PackingItemRow` now shows a **note glyph** (`note.text`, tinted person-colour when a note exists, secondary when empty) and a **sub-item list glyph** (`list.bullet`) just left of the Skip button, replacing the persistent "+ add item" row that pushed every interactive item onto two lines. Each glyph reveals its inline editor on demand and they are mutually exclusive.   - The **note is edited inline** via the note glyph (or by tapping the note text) instead of opening the full edit form — a new `PackingItemGroup.saveNote` commits `PackingSubItems.sanitizedNote(_:)` through the same `LocalWriteHook` chokepoint; the edit form still edits the name. `PackingSubItemsView` gains an inline note `TextField` (vertical, live 500-cap via the new `PackingSubItems.cappedNote`) alongside the existing sub-item add field, both parent-controlled via `isEditingNote` / `isAddFieldVisible` bindings; the sheet's background tap-catcher dismisses either.

Things to double-check

2-day-trip auto-expand edge.

Confirm on a real 2-day trip that the day where Departure day and Day-before-return coincide behaves acceptably — nothing auto-expands, so the user taps to reach the repack list. Accepted in the ADR but worth eyeballing on device.

UI tests not executed.

PackingSheetUITests (summary render + subline) were retargeted to .dayBefore and compile, but the simulator is unavailable so they did not run. Re-run make test once CoreSimulator is updated.