Where: Liquid Glass tabbed UI (Primary / Elsewhere / Settings)#9
Conversation
The @ModelActor-generated init(modelContainer:) is not reachable from other modules, so WhereUI had no supported way to build a production (.localOnly/.cloudKit) store. Add a public make(storage:) factory mirroring inMemory(), plus a round-trip test. Co-authored-by: Cursor <cursoragent@cursor.com>
…iews Groundwork for the tabbed UI (no behavior change yet): - WhereModel (@mainactor @observable) wraps WhereController and exposes year selection, report loading, manual-day entry, GPS, and clear-year. - RegionRanking purely splits a YearReport into primary (top regions by days, excluding .other) and secondary. - RegionStyle gives each Region a symbol/emoji/tint for whimsy. - PreviewSupport seeds in-memory fixtures for #Preview and tests. Co-authored-by: Cursor <cursoragent@cursor.com>
RootView now hosts three tabs (Primary / Elsewhere / Settings) in an iOS 26 TabView with tab-bar minimize-on-scroll, owns the single WhereModel, builds the live controller in .task, and injects the model via the environment. Primary/Secondary/Settings land as thin stubs that are fleshed out in the following steps. Co-authored-by: Cursor <cursoragent@cursor.com>
PrimaryView shows the top regions (by days) for the selected year as prominent glass RegionSummaryCards inside a GlassEffectContainer, with a playful hero header, day-count headlines, loading/empty/error states, and a shared YearSelector in the toolbar. Co-authored-by: Cursor <cursoragent@cursor.com>
SecondaryView lists every non-primary region (ranking.secondary) for the selected year as compact glass cards, sharing RegionSummaryCard and the YearSelector, with loading/empty/error states and a light "just passing through" caption for brief stays. Co-authored-by: Cursor <cursoragent@cursor.com>
SettingsView gates location permission (with an Open-Settings alert on denial), a background-tracking toggle wired to WhereController GPS, a push to ManualDayEntryView for asserting/overriding a day's regions, and a confirmed destructive erase of the selected year's data. Co-authored-by: Cursor <cursoragent@cursor.com>
CoreLocation's requestAlwaysAuthorization requires both NSLocationWhenInUseUsageDescription and NSLocationAlwaysAndWhenInUseUsageDescription, so add them to the Where target's infoPlist; without them the Settings "Grant location access" button would crash on launch of the permission prompt. Co-authored-by: Cursor <cursoragent@cursor.com>
Adds pure unit tests for RegionRanking (top-N primary, .other always secondary, zero-day drop, tie-breaking, empty year, configurable count) and hosted smoke tests that mount Primary/Secondary/Settings with seeded preview data via WhereTesting.show. Co-authored-by: Cursor <cursoragent@cursor.com>
Codify branching as a pre-commit gate in "Working on plans" so plan to-dos never land directly on main/master by accident. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@cursoragent review |
|
Findings:
Verification:
  |
Add WhereController.addManualDays(from:through:regions:), which writes a whole inclusive day range in one perform transaction, surface it through WhereModel.setManualDays, and give ManualDayEntryView a Single day / Date range mode with From/Through pickers and a live day count. Covered by new WhereController range tests. Co-authored-by: Cursor <cursoragent@cursor.com>
Introduce PresenceTimeline, which folds a YearReport's DayPresence into maximal consecutive RegionStint runs (e.g. California Jan 1 - Feb 3, then New York Feb 3 - Mar 10, sharing the transition day). Surface it from a new navbar button on PrimaryView that presents PresenceTimelineView: a chronological list of region / date-range / duration rows. Covered by PresenceTimeline unit tests plus a hosted smoke test. Co-authored-by: Cursor <cursoragent@cursor.com>
Addresses PR review on addManualDays: move the per-day range enumeration into Date.calendarDays(through:in:) in WhereCore and cover it directly with unit tests, leaving the controller transaction thin. Co-authored-by: Cursor <cursoragent@cursor.com>
Addresses cursor[bot] review: - startTracking() now confirms/requests location permission and only flips isTracking once GPS is actually running, surfacing the Settings alert (and leaving the toggle off) on denial. - select(year:) drops the previous report so views show their loading state instead of rendering last year's data under the new year label. Co-authored-by: Cursor <cursoragent@cursor.com>
Addresses PR review: replace the hardcoded 365 with WhereModel.daysInSelectedYear (365/366 from Calendar) so the ambient progress bar scales correctly in leap years. The card default stays 365 for previews only. Co-authored-by: Cursor <cursoragent@cursor.com>
Addresses PR review: introduce UIConstants (Spacings/Padding/ CornerRadius/Size) and replace the hardcoded spacing, padding, corner-radius, and element-size numbers across the region cards, Primary, Elsewhere, and the timeline rows. Co-authored-by: Cursor <cursoragent@cursor.com>
Addresses PR review: add Resources/Localizable.xcstrings to WhereUI
(wired via Package.swift resources) and a Strings helper that resolves
every user-facing string through bundle: .module. Counts use catalog
plural variations; years are formatted with a grouping-free number
style ("2026", not "2,026") to address the formatter feedback on
Elsewhere. Adds StringsTests to lock in the wiring.
Co-authored-by: Cursor <cursoragent@cursor.com>
Adds a Sendable LocationAuthorizationStatus and surfaces it through LocationSource (currentAuthorization() + an authorizationUpdates stream) and WhereController, so the UI can reflect real authorization and react to Settings-app changes. CoreLocationSource now requests When-In-Use first then nudges the (iOS-deferred) Always upgrade without blocking, and broadcasts every authorization change. ScriptedLocationSource gains a configurable status + emitAuthorization for tests. Co-authored-by: Cursor <cursoragent@cursor.com>
Fixes two bugs and adds the requested indicator: - "Track in the background" no longer reads off on every launch — the model now syncs authorization, persists the user's tracking intent, and resumes GPS when Always is granted (start() previously never started GPS at all). - "Grant location access" now produces visible feedback: the model reflects the authorization result and the button is context-aware (Grant only when it can help; Open Settings when denied/limited). - Adds a LocationStatusRow to the Settings location section summarizing authorization + whether tracking is actually running, kept live via WhereController.authorizationUpdates(). New strings go through the catalog; WhereModelTrackingTests covers the launch reconciliation and live-update paths. Co-authored-by: Cursor <cursoragent@cursor.com>
Move controller/CLLocationManager creation off a SwiftUI .task and onto an app-launch hook so CoreLocation can deliver significant-change / visit events when the app is relaunched into the background after termination (a view's .task isn't a reliable hook with no UI). - WhereModel.bootstrap(): synchronous controller (and CLLocationManager) creation; start() now calls it. - AppDelegate (UIApplicationDelegateAdaptor) owns the single WhereModel, bootstraps synchronously in didFinishLaunching, then re-registers monitoring / reconciles tracking; RootView takes the injected model. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@cursoragent review |
|
@codex review |
|
Findings:
Verification:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1c0871a53
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| guard let controller else { return } | ||
| loadState = .loading | ||
| do { | ||
| report = try await controller.yearReport(for: selectedYear) |
There was a problem hiding this comment.
When the user picks two years quickly from YearSelector, the two Task { await model.select(year:) } calls can overlap because WhereModel is reentrant while awaiting controller.yearReport. If the earlier fetch returns after the later one, this unconditional assignment can install a report for the old year while selectedYear already shows the new year, so the UI displays mismatched totals; capture the requested year and discard results that no longer match (or serialize/cancel in-flight refreshes).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Posted by an AI agent on kyleve's behalf.
Fixed in 5f4e8b0. WhereModel.refresh() now captures the requested year and discards any result that arrives after a newer select(year:) has moved the selection on, so an out-of-order fetch can't install stale totals under the new year's label. Covered by WhereModelRefreshTests.staleYearFetchDoesNotOverwriteNewerSelection, which gates the first fetch behind the second to force the reentrancy race deterministically.
Addresses two concurrency findings from PR review bots: - stopGPS() previously cancelled the task iterating the single AsyncStream from LocationSource, which terminates the stream. A later startGPS() then iterated an already-finished stream and dropped every subsequent sample, so toggling tracking off and back on (without relaunching) silently stopped recording. The ingest task is now created once and kept alive for the controller's lifetime; stopGPS() only pauses the underlying monitoring (tracked via a new isMonitoring flag) and the task is torn down on deinit. - A second .notDetermined requestPermission() call overwrote the single pending continuation, permanently stranding the first caller. Pending waiters are now coalesced into a list: only the first drives the system prompt and all resume together on the authorization callback. Adds a regression test proving tracking survives a pause/resume. Co-authored-by: Cursor <cursoragent@cursor.com>
Addresses three PR review findings in the SwiftUI layer: - WhereModel.refresh() assigned its result unconditionally. Because the model is reentrant while awaiting yearReport, a rapid second select(year:) could let a slower older fetch install its report under the newer year's label. refresh() now captures the requested year and discards results that no longer match the selection. (Deterministic regression test gates the first fetch behind the second.) - Manual day entry always dismissed and reported success: setManualDay / setManualDays swallowed persistence errors into loadState, closing the form as if the backfill worked. These now throw; ManualDayEntryView keeps the form open and shows the error in an inline alert, and the global load state is left untouched. - The Primary tab showed "No travels logged" whenever no region ranked as primary, even when days existed only in .other. It now distinguishes "nothing tracked" from "tracked, but nothing in a headline region" and points the user at the Elsewhere tab. Adds string-catalog entries (with plural variations) and tests for each. Co-authored-by: Cursor <cursoragent@cursor.com>
Addressed the open review-bot findings in
The two codex P2 findings (stale-year race and the non-restartable location stream) are addressed in the same commits and answered in their threads. |
Tuist's autogeneration emits standalone schemes for StuffCoreTests and WhereCoreTests but, for this graph, not for WhereTests or WhereUITests — they were only reachable via the aggregate Stuff-Workspace scheme, which builds and tests everything. Declaring the two schemes explicitly lets `tuist test WhereTests` / `tuist test WhereUITests` run a single bundle. Tooling-only change; no production code affected. Co-authored-by: Cursor <cursoragent@cursor.com>
1 participant
Summary
Turns the
WhereUIstub (RootViewwas justText("Where")) into a full SwiftUI front-end driven by the existingWhereController, with a Liquid Glass tab bar, a stays timeline, manual backfill, and background-tracking controls.RootView) hosting three tabs — Primary / Elsewhere / Settings — via the iOS 26TabAPI with tab-bar minimize-on-scroll. A singleWhereModelis built at launch by anAppDelegate(@UIApplicationDelegateAdaptor) and injected through the environment;RootView.taskstill callsstart()for an idempotent foreground refresh..glassEffectRegionSummaryCards in aGlassEffectContainer, with a "Where have you been in {year}?" hero, big day-count headlines, calendar-derived progress, and loading/empty/error states. A toolbar Timeline button (alongside the year picker) opens a chronological list of continuous stays ("California Jan 1 – Feb 3, NYC Feb 3 – Mar 10, …").LocationStatusRow), a background-tracking toggle reconciled against real authorization + persisted intent, context-aware Grant access / Open Settings buttons, manual day entry (single day or an inclusive date range), and a confirmed destructive erase-year.WhereModel(@MainActor @Observable), pureRegionRankingsplit,PresenceTimeline/RegionStintgrouping, location-themedRegionStyle, sharedYearSelector, in-memoryPreviewSupport, a fully localized string catalog (Localizable.xcstringsvia aStringshelper, plural-aware), and centralized layout literals (UIConstants).Key decisions
YearReport.totals); no new domain duration model..otheris always Secondary.Enabling / supporting changes
WhereCorelocation surface: new domain-levelLocationAuthorizationStatus;LocationSourcegainsauthorizationUpdates+currentAuthorization();CoreLocationSourcemapsCLAuthorizationStatus, drives a non-blocking two-step (When-In-Use → Always) permission flow, and coalesces concurrent permission requests.WhereController:addManualDays(from:through:)range backfill (extracted into a testedDate.calendarDays(through:in:)), authorization passthroughs, and a restartable GPS lifecycle (the stream consumer outlives astopGPS()pause).AppDelegatebootstraps the controller/CLLocationManagerearly so CoreLocation can relaunch the app into the background after termination; new publicSwiftDataStore.make(storage:)factory (the@ModelActorinit isn't reachable fromWhereUI).NSLocationWhenInUseUsageDescription+NSLocationAlwaysAndWhenInUseUsageDescriptionon theWheretarget so the permission prompt doesn't crash.AGENTS.mdnow requires a feature branch before committing plan work; explicit TuistWhereTests/WhereUITestsschemes so each bundle is runnable on its own (Tuist's autogeneration only covered them via the aggregateStuff-Workspacescheme).Review feedback addressed
Human review (resolved): all user-facing strings moved into the catalog; layout literals centralized in
UIConstants; region-card year length derived fromCalendar; the year rendered with a grouping-free number format.Bot review (resolved):
stopGPS()no longer cancels the single-consumerAsyncStream; pausing then resuming tracking keeps recording instead of silently dropping samples.requestPermission()calls no longer overwrite (and strand) the pending continuation.WhereModel.refresh()discards out-of-order results so a slow older fetch can't render under the new year's label.setManualDay/setManualDaysthrow; the entry form stays open with an inline alert instead of dismissing as if it saved.Notes / out of scope
CoreLocationSource's prompt flow needs a realCLLocationManagerand isn't reachable through theScriptedLocationSourceseam.Test plan
mise exec -- tuist test Stuff-Workspace --no-selective-testing— full suite green, including:WhereControllerTestsincl.trackingResumesAfterPauseWithoutDroppingSamplesand the GPS retry/drain flowWhereModelTrackingTests(launch reconciliation across authorization states, persisted intent, live updates)WhereModelRefreshTests(deterministic stale-year race + manual-save failures)RegionRankingTests,PresenceTimelineTests,StringsTests(catalog wiring + plurals),DateCalendarDaysTests,LocationAuthorizationTestsScreenHostingTests(Primary incl. elsewhere-only state, Elsewhere, Settings, Timeline mount) +rootViewBuilds./swiftformat --lintcleanmise exec -- tuist test WhereUITests/tuist test WhereTestsrun each bundle on its own (new explicit schemes)Made with Cursor