Skip to content

Add WhereData module: persistence, GPS, evidence, simulated-year tests#7

Merged
kyleve merged 26 commits into
mainfrom
where-data-foundation
May 27, 2026
Merged

Add WhereData module: persistence, GPS, evidence, simulated-year tests#7
kyleve merged 26 commits into
mainfrom
where-data-foundation

Conversation

@kyleve
Copy link
Copy Markdown
Owner

@kyleve kyleve commented May 25, 2026

Summary

Data foundation for the Where app. No UI changes; this PR is purely the model/persistence/ingestion layer that future feature work will build on.

  • WhereController actor orchestrates GPS ingestion, retroactive day entry, evidence attachments, and year reports. Sits behind a WhereStore protocol (transactional perform { ... } API) and a LocationSource protocol (requestPermission() + liveSamples async stream).
  • Production SwiftDataStore: @ModelActor over a ModelContainer, with a per-perform peer ModelContext for writes so the main context is effectively read-only and write transactions are atomic (commit on success, rollback on throw). Container mode is Storage.default.inMemory under tests, .localOnly in DEBUG, .cloudKit in release.
  • CoreLocationSource wraps CLLocationManager (Visits + significant-change). requestPermission() is async throws and reuses the CLLocationManagerDelegate authorization callback via CheckedContinuation. ScriptedLocationSource test double drives deterministic samples.
  • EvidenceBlobStore protocol kept separate from WhereStore so blob storage can be swapped later. SwiftDataStore implements both, with @Attribute(.externalStorage) so CloudKit chunks blobs as CKAssets.
  • Domain types in WhereCore: Region, Coordinate, LocationSample, DayPresence, YearReport, Evidence / EvidenceKind / EvidenceContentType, SampleSource. Region is generalized beyond US states (CA, NY, Canada, EU, other); EvidenceKind and EvidenceContentType are associated-value enums with .other(String?) for catch-all labels; SampleSource.evidenceImplied(id:kind:) carries provenance back to the originating Evidence row.
  • RegionAttributor.shared (loaded from bundled GeoJSON) with a per-region BoundingBox pre-pass before the even-odd ray-cast. GeoJSON primitives live in their own GeoJSON namespace enum (separation from the attributor).
  • Bundled polygons under Where/WhereCore/Sources/Resources/:
    • us-states.geojson — US Census 5m simplified state polygons (CA + NY indexed by feature NAME; credited in Resources/README.md).
    • canada.geojson, europeanUnion.geojson — per-region hand-bundled simplified polygons.
  • Localizable.xcstrings for user-facing region names; Region.localizedName is a switch with literal String(localized:) keys per case so the string-catalog extractor can see every key statically.
  • Tooling/conventions in AGENTS.md: no proactive PR-feedback action, CI polling stays in a subagent (local tests still block), one-commit-per-todo with passing tests, --no-open for tuist generate, prefer named structs over multi-field tuples. .swiftformat tightened to --max-width 100 with --allow-partial-wrapping false so multi-arg signatures wrap one-per-line. ./ide takes --no-open.

Out of scope

  • All UI work (calendar, year, day-detail views, GPS permission UI). RootView and the Where app are unchanged beyond Tuist/SPM wiring.
  • Higher-fidelity polygons beyond the bundled US Census 5m + hand-bundled CA/EU. The current set is good enough for the cities our tests spot-check.

Test plan

10 suites across 4 bundles (WhereCoreTests, StuffCoreTests, WhereTests, WhereUITests), all green on iPhone 17 / iOS 26.2:

  • DayAggregatorTests — single-region, dual-region cross-country, transatlantic midnight, Mexico stopover (.other), empty input, calendar/timezone edges, year filtering.
  • RegionAttributorTests — positive spot-checks (SF, LA, NYC, Buffalo, Toronto, Montreal, Paris, Berlin) and negatives (Reno, Newark, Tijuana, London, Chicago, Tokyo).
  • SimulatedYearTests — full simulated 2026 with CA-heavy residency, several flights, one Canada trip, one EU trip, a 7-day no-GPS gap, retroactive backfill, evidence attachments.
  • NewYorkHeavyYearTests — mirror of SimulatedYear weighted toward NY, plus bare-majority (1-day margin) and 8-week alternation scenarios.
  • WhereControllerTests — ingest/manual/evidence round-trips, retry queue (bounded FIFO with ToggleFailingStore fault injection), manual-day upsert, per-perform rollback (throwing block discards staged writes), read-your-own-writes inside perform, peer-save visible to main read context.
  • YearReportTests — day sort ordering.
  • StorageDefaultTestsSwiftDataStore.Storage.default == .inMemory under the test runner (env-var detection so a future runner change can't silently let tests write to disk).
  • RegionTestslocalizedName returns the catalog English value for each case.
  • EvidenceKindTests / SampleSourceTests — synthesized Codable round-trip + denormalized provenance accessors.

Verification commands:

./swiftformat --lint                  # clean
mise exec -- tuist test               # all bundles, full simulator run

Notes

  • Codable wire format: YearReport.totals custom-encodes as [String: Int] keyed by Region.rawValue so JSONEncoder.sortedKeys produces deterministic output (Swift's default for [Region: Int] would emit an unkeyed array). EvidenceKind / EvidenceContentType / SampleSource use synthesized Codable (nested per-case shape, e.g. {"other":{"_0":"label"}}) — no manual init(from:) / encode(to:).
  • No InMemoryStore: an earlier iteration had a hand-written in-memory store; it's gone in favor of SwiftDataStore.inMemory() so tests exercise the same persistence path as production.
  • swift-snapshot-testing 1.19.2 is still in Package.swift even though no test currently uses it — kept for upcoming work that wants snapshot coverage of YearReport or UI artifacts.
  • Plans/ is git-ignored. Implementation plans live there during the work and aren't committed.
  • New .agents/skills/ committed: swift-testing-pro, swift-concurrency-pro, swiftdata-pro, swiftui-pro.

Made with Cursor

Plan: Plans/001-2026-05-25-where-data-foundation.md

WhereCore (extended):
- Region (CA, NY, Canada, EU, other), Coordinate, LocationSample,
  SampleSource, DayPresence, YearReport value types.
- RegionAttributor + GeoPolygon: offline point-in-polygon over bundled
  simplified GeoJSON for CA / NY / Canada / EU.
- DayAggregator: pure rule "any in-region sample on a calendar day
  counts the day for that region"; unions GPS-derived days with manual
  day entries; year-scoped reports.
- YearReport.totals encodes as a [String: Int] JSON object so
  JSONEncoder.sortedKeys yields deterministic snapshots.

WhereData (new module):
- WhereStore protocol (Sendable boundary in WhereCore values) with
  InMemoryStore and a CloudKit-synced @Modelactor SwiftDataStore;
  evidence blobs use @Attribute(.externalStorage).
- LocationSource protocol with a CoreLocation Visits +
  significant-change implementation and a ScriptedLocationSource for
  tests; authorization surfaced as an AsyncStream.
- Evidence + EvidenceBlobStore (with in-memory double).
- WhereController actor wiring ingest, retroactive manual entry,
  evidence, yearReport(for:), clearYear, and GPS lifecycle.

Tests (Swift Testing, 45 total):
- WhereCoreTests: polygon spot-checks (SF/LA/NYC/Buffalo/Toronto/
  Montreal/Paris/Berlin + Reno/Newark/Tijuana/London/Chicago/Tokyo
  negatives), DayAggregator rules including dual-region and
  transatlantic flights, calendar/timezone edges, Codable ordering.
- WhereDataTests: InMemoryStore round-trips, WhereController API,
  and a full simulated 2026 with CA/NY residency, several flights,
  one Canada trip, one EU trip, a no-GPS gap, retroactive backfill,
  and evidence. Asserts numerical totals plus two snapshots
  (assertSnapshot of YearReport as JSON, MonthlySummary as text)
  via swift-snapshot-testing 1.19.2.

Wiring:
- Package.swift: new WhereData library product, WhereCore Resources
  for the bundled GeoJSON, swift-snapshot-testing dependency.
- Project.swift: unitTests helper now takes extraPackageProducts;
  new WhereDataTests target links SnapshotTesting.

No UI changes in this pass; RootView and the Where app are untouched
beyond the package wiring. UI is a follow-up plan.

Co-authored-by: Cursor <cursoragent@cursor.com>
@kyleve
Copy link
Copy Markdown
Owner Author

kyleve commented May 25, 2026

@cursoragent review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 25, 2026

Findings:

  • High — CloudKit-backed SwiftDataStore schema is not CloudKit-compatible. SwiftDataStore.makeContainer defaults useCloudKit to true, but the models use @Attribute(.unique) and non-optional properties without defaults (SwiftDataStore.swift:23-29, :176-183, :212-219, :248-251). SwiftData/CloudKit cannot enforce unique constraints and requires CloudKit-compatible defaults/optionals, so the production container can fail to open or sync. Remove unique annotations, enforce uniqueness in store methods, and add defaults or make fields optional as appropriate.

  • High — Updating evidence metadata can permanently delete the attachment blob. WhereController.addEvidence(_:blob:) defaults blob to nil (WhereController.swift:58), while both stores replace the existing record with that exact blob value (SwiftDataStore.swift:62-79, InMemoryStore.swift:27-29). A note/kind/region edit without resupplying bytes wipes the stored attachment. Preserve the existing blob when blob == nil, or split create/update semantics explicitly.

  • Medium — GPS ingestion silently drops persistence failures. The GPS loop uses try? await store.addSample(sample) (WhereController.swift:95-99), so SwiftData save failures, duplicate IDs, CloudKit errors, or container issues are invisible and data is lost while tracking appears healthy. Surface/log failures and consider a retry/error stream instead of swallowing them.

  • Medium — CLLocationManager is used without main-thread/main-actor confinement. CoreLocationSource constructs and drives CLLocationManager from a non-@MainActor Sendable class (CoreLocationSource.swift:19-44, :52-53). Since callers can invoke this from WhereController’s actor, manager setup/start/request calls may occur off the main run loop. Mark the source or its CoreLocation-facing methods @MainActor.

  • Medium — Visit samples with unknown arrival dates can be assigned to the wrong day. When CLVisit.arrivalDate == .distantPast, the code uses Date() (CoreLocationSource.swift:87-105). Deferred visit delivery can then attribute a historical visit to “today”; prefer a valid departureDate when available or skip/flag unknown-timestamp visits.

  • Low/Medium — Bundled region resource failures are silent in release. Missing or malformed GeoJSON is skipped with only assertionFailure on decode (RegionAttributor.swift:33-40), causing whole regions to become .other in production. Since these resources are required for correctness, fail loudly or make loading throwing.

Tests not run; this was a static PR review.

Open in Web Open in Cursor 

@kyleve
Copy link
Copy Markdown
Owner Author

kyleve commented May 25, 2026

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1eb60d4409

ℹ️ 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".

Comment thread Where/WhereData/Sources/Location/CoreLocationSource.swift Outdated
kyleve and others added 2 commits May 25, 2026 13:35
- CoreLocationSource: confine to @mainactor so CLLocationManager setup,
  start/stop, and authorization calls happen on the main run loop; mark
  AsyncStream lets and CLLocationManagerDelegate methods nonisolated so
  the delegate conformance doesn't cross actors.
- CoreLocationSource: when CLVisit.arrivalDate is .distantPast, fall back
  to visit.departureDate before using Date(), to avoid attributing a
  late-delivered visit to the wrong day/year.
- SwiftDataStore: remove @Attribute(.unique) and give every stored
  property a default value so the schema is CloudKit-mirror compatible
  (uniqueness is enforced via fetch-then-delete-then-insert in the
  actor's add* methods).
- SwiftDataStore + InMemoryStore: treat addEvidence(_:, blob: nil) as
  "no change" to the attachment bytes, so metadata edits no longer wipe
  the previously stored blob. Callers can use EvidenceBlobStore.delete
  to remove a blob explicitly.
- WhereController.startGPS: log persistence failures via os.Logger
  instead of swallowing them with try?.
- RegionAttributor: log missing / unparseable bundled GeoJSON resources
  as os.fault so failures are visible in release Console output, not
  only via assertionFailure in debug builds.

Co-authored-by: Cursor <cursoragent@cursor.com>
@kyleve
Copy link
Copy Markdown
Owner Author

kyleve commented May 25, 2026

Posted by an AI agent on @kyleve's behalf.

Addressed the review findings in ba1c9ee:

  • High — CloudKit-incompatible SwiftData schema. Removed @Attribute(.unique) from SDLocationSample, SDEvidence, and SDManualDay and gave every stored property a default value so the schema works with CloudKit's mirror. Uniqueness is now enforced inside SwiftDataStore's add… methods (fetch-then-delete-then-insert), including addSample which previously didn't dedupe.
  • High — addEvidence wiping attachments. Both SwiftDataStore.addEvidence and InMemoryStore.addEvidence now treat blob: nil as "no change" — they look up the existing blob and preserve it. Callers that need to explicitly remove a blob should use EvidenceBlobStore.delete(for:).
  • Medium — GPS persistence failures silently dropped. WhereController.startGPS now logs failures via os.Logger (com.stuff.where / WhereController) instead of try?. The stream keeps running so a transient error doesn't stop tracking, but the failure is visible in Console / os_log.
  • Medium — CLLocationManager actor confinement. CoreLocationSource is now @MainActor. AsyncStream lets and continuations are nonisolated (AsyncStream.Continuation.yield is thread-safe). CLLocationManagerDelegate methods are nonisolated so the conformance doesn't cross actors, while manager setup, start/stop, and authorization calls all run on the main run loop where CoreLocation expects them.
  • Medium — Visit with unknown arrival date. locationManager(_:didVisit:) now uses visit.arrivalDate when valid, falls back to visit.departureDate, and only resorts to Date() if both are .distantPast. (Also responded inline.)
  • Low/Medium — Silent bundled region resource failures. RegionAttributor.loadFromBundle now logs missing or unparseable GeoJSON via Logger.fault (com.stuff.where / RegionAttributor) so it's visible in release Console output, in addition to the existing assertionFailure for debug builds.

Tests pass locally via mise exec -- tuist test --no-selective-testing -- -destination 'platform=iOS Simulator,name=iPhone 17,OS=26.2'.

Comment thread Where/WhereCore/Sources/DayAggregator.swift
Comment thread Where/WhereCore/Sources/DayPresence.swift Outdated
Comment thread Where/WhereCore/Sources/GeoPolygon.swift
Comment thread Where/WhereCore/Sources/LocationSample.swift Outdated
Comment thread Where/WhereCore/Sources/LocationSample.swift Outdated
Comment thread Where/WhereCore/Sources/Region.swift Outdated
kyleve and others added 10 commits May 25, 2026 14:51
- DayAggregator.yearInterval: document explicitly that this returns a
  half-open interval `[Jan 1 year, Jan 1 year+1)` so the first instant of
  the next year isn't double-counted, and so it matches the predicate
  semantics used by `SwiftDataStore`.
- InMemoryStore: switch interval filtering from `DateInterval.contains`
  (closed `[start, end]`) to a half-open `[start, end)` helper so the
  in-memory and SwiftData stores agree on year boundaries.
- DayPresence: explain in a comment why `Codable` is hand-written
  (sorting regions for deterministic snapshots), and stop relying on
  `Region: Comparable` — sort by `rawValue` inline.
- Region: drop the `Comparable` conformance now that nothing uses it.
  There's no meaningful natural ordering between regions; sort by
  `rawValue` at the call site if needed.
- LocationSample: remove the `horizontalAccuracy: Double = 0` default so
  callers have to think about (and pass) a value. Updated every call
  site in tests and `SimulatedYear`.
- WhereCoreTests: remove `region_comparableUsesRawValue` along with the
  conformance it tested.
- MonthlySummary (test helper): sort by `region.rawValue` instead of
  relying on `Region: Comparable`.

Co-authored-by: Cursor <cursoragent@cursor.com>
Add two repo-wide agent rules learned from PR #7:

- "Working on PR feedback" — agents must not proactively triage PR
  review comments (bot or human); they should summarize and wait for
  explicit go-ahead before reading more context, editing files, or
  pushing commits.
- "Waiting on CI" — agents must not block the main conversation polling
  remote CI; if CI needs to be watched, delegate to a background
  subagent. Local commands (`tuist test`, `swift build`,
  `./swiftformat --lint`, etc.) should still be awaited inline.

Co-authored-by: Cursor <cursoragent@cursor.com>
Replace the hand-drawn 14-point California bounding box and the simplified
New York outline with the US Census Bureau's 5m-resolution state polygons
(via eric.clst.org's `gz_2010_us_040_00_5m.json`).

- Add `Where/WhereCore/Sources/Resources/us-states.geojson` (~2.5 MB, 52
  features: 50 states + DC + PR). MultiPolygon per state with
  `properties.NAME` matching the state's English name.
- Add `Where/WhereCore/Sources/Resources/README.md` with per-file
  provenance and license note (US Government works are public domain;
  Census Bureau requests citation).
- Add an Acknowledgements section to `README.md` pointing at the
  Resources README and naming the data sources.

`RegionAttributor.loadFromBundle` now loads `us-states.geojson` once,
builds a `[NAME: [GeoPolygon]]` index restricted to the state names
mapped from `Region` cases (private `usStateNames` table), and pulls
California / New York polygons from the index. Non-US regions
(`.canada`, `.europeanUnion`) keep their existing per-region files.
`GeoJSONFeature` now decodes `properties.NAME` (optional `String?`) so
the index can filter by state. Polygon/MultiPolygon decoding is
unchanged; the helper `polygons(from:)` is shared between the two load
paths.

Adding another US state in the future is just a new `Region` case plus
an entry in `usStateNames` — no new file to bundle.

`./swiftformat --lint` clean; `tuist test` 45/45 passing, including all
15 RegionAttributorTests spot checks and all 5 SimulatedYearTests
snapshots (no snapshot movement against the higher-fidelity polygons).

Co-authored-by: Cursor <cursoragent@cursor.com>
When an agent posts under the user's identity (GitHub PR replies, issue
comments, review responses, Slack messages, etc.), the first line must
be a tag like `> _Posted by an AI agent on $USER's behalf._` so the
reader knows it's AI-generated and not the user speaking. The
previously-posted comments on PR #7 have been retro-edited to add the
prefix.

Co-authored-by: Cursor <cursoragent@cursor.com>
The two snapshot tests on YearReport / MonthlySummary were forcing a
lot of scaffolding onto production types just to keep their JSON / text
output deterministic. Snapshots are better used for UI; for plain data
we can just assert on the value.

- SimulatedYearTests: drop `import SnapshotTesting` and
  `@Suite(.snapshots(...))`. Replace `yearReport_jsonSnapshot` and
  `monthlySummary_dumpSnapshot` with `yearReport_perMonthBreakdown`,
  which #expects per-month region totals and dual-region day counts
  directly off the YearReport. `totalsAddUpAcrossAllRegions` already
  covers the year-level totals and structural invariants.
- Delete MonthlySummary.swift (only used by the deleted snapshot) and
  the two fixture files under __Snapshots__/SimulatedYearTests/.
- WhereCoreTests: drop dayPresence_encodesRegionsSorted and
  dayPresence_decodesBackToSameRegions; rename the suite from
  WhereCoreCodableTests to YearReportTests now that the surviving test
  is about YearReport's ascending-day init invariant.
- DayPresence: drop the custom `Codable` extension (it existed only to
  sort `regions` for snapshot determinism). Still Hashable / Sendable.
- YearReport: drop the custom `Codable` extension (it string-keyed
  totals so JSONEncoder.sortedKeys produced stable output). Still
  Hashable / Sendable; the days-ascending init invariant is kept.
- Project.swift: drop `extraPackageProducts: ["SnapshotTesting"]` from
  the WhereDataTests target. The swift-snapshot-testing package
  dependency stays in Package.swift for the upcoming UI snapshot tests
  — when those land, the wiring goes on the relevant UI test target.

Net: -2348 / +42 lines. `tuist test` 41/41 passing; `./swiftformat
--lint` clean.

Co-authored-by: Cursor <cursoragent@cursor.com>
Adds --max-width 120 with --wrap-arguments/parameters/collections set
to before-first. SwiftFormat can't enforce "wrap when >= N arguments"
directly, but at 120 columns nearly every real multi-arg function
ends up wrapped one-arg-per-line while short helpers stay inline.

Reformats existing call sites accordingly; the only awkward cases are
a few long single-arg `logger.fault(…)` lines whose interpolated
messages exceed the column limit on their own.

Co-authored-by: Cursor <cursoragent@cursor.com>
Default is `true`, which let SwiftFormat keep some args on the same
line when wrapping (e.g. `originLat: Double, originLng: Double,` in
SimulatedYear's emitFlight). `false` makes wrapping all-or-nothing:
either everything on one line, or one arg/parameter per line.

I considered --wrap-effects if-multiline and --wrap-return-type
if-multiline for the signature-level polish, but they interact badly
with wrapMultilineStatementBraces and produce ugly stacks like
`)\n    async\n{`. Skipping for now.

Co-authored-by: Cursor <cursoragent@cursor.com>
100 columns is tight enough that almost every 2+ arg function and
call site wraps one-arg-per-line, which is the de facto "wrap when
there are >= N arguments" that SwiftFormat itself can't express. A
few long single-arg `logger.fault("…")` lines get chain-split as a
nudge to extract the message; not worth refactoring those yet.

Co-authored-by: Cursor <cursoragent@cursor.com>
Plans/ is a scratch space for the agent + user during implementation;
committing them just clutters history. Marks Plans/ as gitignored and
deletes the one plan that landed on the branch.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Comment thread Where/WhereCore/Sources/GeoPolygon.swift
Comment thread Where/WhereCore/Sources/RegionAttributor.swift Outdated
Comment thread Where/WhereCore/Sources/RegionAttributor.swift
Comment thread Where/WhereCore/Sources/RegionAttributor.swift
Comment thread Where/WhereCore/Sources/RegionAttributor.swift Outdated
Comment thread Where/WhereCore/Sources/RegionAttributor.swift Outdated
Comment thread Where/WhereCore/Sources/Persistence/InMemoryStore.swift Outdated
Comment thread Where/WhereCore/Sources/Persistence/SwiftDataStore.swift Outdated
Comment thread Where/WhereCore/Sources/Persistence/SwiftDataStore.swift Outdated
Comment thread Where/WhereCore/Sources/Persistence/SwiftDataStore.swift Outdated
Comment thread Where/WhereCore/Sources/WhereController.swift
Comment thread Where/WhereCore/Sources/WhereController.swift
Comment thread Where/WhereCore/Sources/WhereController.swift
Comment thread Where/WhereCore/Tests/SimulatedYearTests.swift Outdated
kyleve and others added 2 commits May 27, 2026 13:17
Resolves all 34 unresolved review threads on PR #7. Grouped by scope so
the diff stays scannable.

Group A — quick refactors
- GeoPolygon.contains: rename loop locals (vertexCount/pointX/pointY/
  currentX/currentY/previousX/previousY/previousIndex) + ray-cast doc
  block.
- RegionAttributor: replace `[(Region, [GeoPolygon])]` tuple with a
  named `RegionPolygons` struct; assertionFailure on malformed
  coordinate pairs; doc comments on the GeoJSON decoder helpers.
- SwiftDataStore: `addEvidence(_:blob:)` -> `write(evidence:blob:)`;
  `EvidenceBlobStore.write(_:for:)` -> `write(blob:for:)`.
- WhereController.startGPS / .stopGPS are idempotent.
- Tests: force-unwrap Los_Angeles timezone (fail-fast in tests) and
  convert `static var calendar` -> `static let calendar` in
  SimulatedYearTests / NewYorkHeavyYearTests / WhereControllerTests.
- AGENTS.md: new convention bullet preferring named structs over
  tuples for multi-field or escaping values.

Group B — SwiftDataStore reshape
- New `Storage` enum (`.inMemory` / `.localOnly` / `.cloudKit`) on
  `makeContainer(storage:)` replacing twin `inMemory:useCloudKit:`
  bools.
- Every @model field is optional with no placeholder default; readers
  log a fault and skip a record whose mandatory state is missing.
- `addSample` / `write(evidence:blob:)` / `setManualDay` upsert
  (fetch → update-or-insert) instead of delete-then-insert.
- `convenience init(value:)` + `update(from:)` mirrors on every @model.
- Required `WhereStore.perform { … }` with a re-entry counter; outer
  block calls `modelContext.save()` once. Drops every per-method
  save and wraps each WhereController mutation in `perform`.

Group C — Evidence + SampleSource expansion
- `EvidenceKind.other(String?)` with hand-written Codable /
  discriminator / fromDiscriminator / knownCases (lost auto
  CaseIterable, kept a curated `knownCases` list).
- New `EvidenceContentType` (pdf / image / plainText / rawData) +
  optional `Evidence.contentType` + `SDEvidence.contentTypeRaw`.
- `SampleSource.evidenceImplied(id: UUID, kind: EvidenceKind)` with
  hand-written Codable (string discriminator + optional evidence
  fields). `SDLocationSample` gains `evidenceId` / `evidenceKindRaw`;
  `SDEvidence` gains `otherLabel`.
- New `EvidenceKindTests` + `SampleSourceTests` covering Codable
  round-trips and missing/unknown-field error paths.

Group D — LocationSource simplification
- Drop `LocationAuthorizationStatus`, `authorizationStream`,
  `setAuthorization`, `ScriptedLocationSource.initialAuthorization`.
- `LocationSource: AnyObject, Sendable`.
- `requestPermission() async throws LocationPermissionDeniedError`
  drives the prompt to completion and throws on .denied / .restricted
  (and on .authorizedWhenInUse, since the app requires Always).
- `CoreLocationSource` doc rewrite reconciles `@MainActor` class with
  `nonisolated` `CLLocationManagerDelegate` methods (the @objc
  protocol contract doesn't permit @mainactor requirements; delegate
  callbacks only yield to the thread-safe continuation).

Group E — GPS retry queue
- Bounded FIFO (`retryQueueCapacity = 1000`) on WhereController.
- Drained before every successful GPS save and on `startGPS()`; samples
  that still fail are re-queued at the tail.
- `retryQueueDepth` exposed for tests.
- New `gpsFailuresEnqueueAndDrainOnRecovery` test with a
  `ToggleFailingStore` actor + `waitUntil` async polling helper.

Group F — i18n catalog
- New `Resources/Localizable.xcstrings` with `region.california` /
  `.newYork` / `.canada` / `.europeanUnion` / `.other` keys.
- `Region.localizedName` via `NSLocalizedString(_:bundle:comment:)`
  against `Bundle.module` (uses NSLocalizedString rather than
  `String(localized:)` because the key is composed at runtime).
- `Package.swift` gains `defaultLocalization: "en"` so SPM emits the
  `en.lproj/Localizable.strings` resource into the bundle.
- Clarifying comment on `RegionAttributor.usStateNames` calling out
  that those strings are data identifiers (matched against
  `properties.NAME` in the bundled GeoJSON), not user-facing labels.

Group G — Doc-only thread replies
- 7 AI-prefixed replies posted via the GitHub API: loadUSStateIndex
  efficiency, `bundled` static, "feels weird" AsyncStream init,
  InMemoryStore per-test reset, async parity benefit, single-logger
  pattern, DayPresence Codable already removed in c598c00.

Group H — Bounding-box pre-pass
- New `BoundingBox` struct with `enclosing(_ polygons:)` helper.
- Per-`RegionPolygons` `boundingBox` computed at `loadFromBundle`.
- `RegionAttributor.region(at:)` skips the polygon ray-cast entirely
  when the bbox check fails (the 4 regions barely overlap, so a
  typical out-of-set coordinate fails the bbox check four times in a
  row at near-zero cost).

Verification
- `./swiftformat --lint` clean.
- `tuist test` green across DayAggregatorTests, InMemoryStoreTests,
  NewYorkHeavyYearTests (7), RegionAttributorTests,
  SimulatedYearTests (4), WhereControllerTests, YearReportTests,
  RegionTests, EvidenceKindTests, SampleSourceTests.
- SimulatedYearTests / NewYorkHeavyYearTests assertion counts match
  the pre-refactor baseline.

Co-authored-by: Cursor <cursoragent@cursor.com>
Two agent-workflow ergonomics updates that came out of running the PR
feedback plan.

- ide: accept `--no-open` and forward it to `tuist generate`. Default
  behavior (open Xcode after generating) is unchanged for interactive
  use; agents and scripted runs now have a way to regenerate without
  stealing focus.
- AGENTS.md `Generating the Xcode project`: new section telling
  agents to always pass `--no-open` (either via `./ide --no-open` or
  directly to `tuist generate`). Notes that `tuist test` / `tuist
  build` are CLI-only and need no flag. Also mentions `--no-open` in
  the build-system overview at the top of the file.
- AGENTS.md `Working on plans`: new section codifying one-commit-per-
  to-do with pre-commit `./swiftformat --lint` + matching `tuist
  test` checks baked into each step's definition of done. Spells out
  the in_progress → implement → check → commit → completed loop, and
  the no-push-until-asked default.

Co-authored-by: Cursor <cursoragent@cursor.com>
Comment thread Where/WhereCore/Sources/Evidence/Evidence.swift Outdated
Comment thread Where/WhereCore/Sources/Evidence/Evidence.swift Outdated
Comment thread Where/WhereCore/Sources/Evidence/Evidence.swift Outdated
kyleve and others added 2 commits May 27, 2026 13:42
SwiftData already gives us an in-memory `ModelContainer`, so the
hand-rolled `InMemoryStore` was duplicating storage behavior that
`SwiftDataStore` now implements directly. Consolidating onto one
store means tests exercise the same code path as production
(filtering, predicate semantics, upsert, perform { } save) instead of
a parallel value-type implementation that could drift.

- New `SwiftDataStore.inMemory()` static helper builds a
  `.inMemory` `ModelContainer` and wraps it in a `SwiftDataStore`.
  Each call returns an independent store, so tests get a clean slate
  per use without any reset plumbing.
- Delete `Sources/Persistence/InMemoryStore.swift` and the
  `InMemoryEvidenceBlobStore` actor from `EvidenceBlobStore.swift`
  (the latter was unreferenced and only shipped for symmetry with
  `InMemoryStore`).
- Rename `Tests/InMemoryStoreTests.swift` →
  `Tests/SwiftDataStoreTests.swift`. Adapt every mutation to flow
  through `store.perform { ... }` so the tests cover the same save-
  boundary contract production callers use.
- Update `SimulatedYearTests`, `NewYorkHeavyYearTests`,
  `WhereControllerTests`, and the embedded `ToggleFailingStore` to
  construct stores via `SwiftDataStore.inMemory()` (the make-helpers
  are now `throws`; call sites updated accordingly).

Verification: `./swiftformat --lint` clean; full `tuist test` green
across all 10 suites including the new `SwiftDataStoreTests` and the
existing `gpsFailuresEnqueueAndDrainOnRecovery` GPS-retry test
(`ToggleFailingStore` now wraps a real in-memory `SwiftDataStore`).

Co-authored-by: Cursor <cursoragent@cursor.com>
…tion

After consolidating onto a single `SwiftDataStore`, the renamed
`SwiftDataStoreTests` had become mostly duplicate coverage — every
round-trip / clear / evidence test was already exercised by
`WhereControllerTests` and the year-long Simulated/NY-heavy fixtures,
all of which now run against the same in-memory `SwiftDataStore`.

The one assertion with unique coverage was `manualDayReplacesByDate`,
which catches a `setManualDay` upsert regression that nothing else
exercises (every other test calls `setManualDay` / `addManualDay` at
most once per date). Moved that into `WhereControllerTests` as
`manualDayReplacesOnSecondCall` so it lives next to the related
`manualDayUnionsWithSamples` test and runs through the same public
controller surface.

Net: -160 / +15. `tuist test` green across all 9 remaining suites
(34 tests); `./swiftformat --lint` clean.

Co-authored-by: Cursor <cursoragent@cursor.com>
Comment thread Where/WhereCore/Sources/Persistence/SwiftDataStore.swift Outdated
Comment thread Where/WhereCore/Sources/Persistence/SwiftDataStore.swift Outdated
Comment thread Where/WhereCore/Sources/Region.swift Outdated
Comment thread Where/WhereCore/Sources/RegionAttributor.swift
Comment thread Where/WhereCore/Sources/RegionAttributor.swift Outdated
Comment thread Where/WhereCore/Sources/RegionAttributor.swift Outdated
Comment thread Where/WhereCore/Sources/RegionAttributor.swift Outdated
Comment thread Where/WhereCore/Sources/Persistence/SwiftDataStore.swift
kyleve and others added 4 commits May 27, 2026 14:21
…default

Three small polish items from PR feedback:

- `perform { ... }`: collapse the explicit `performDepth -= 1` /
  `wasOutermost` capture into a `defer` so the increment/decrement
  pair is symmetric and the read-after-decrement is the same path
  whether the block throws or returns. The "outermost flushes save"
  check still reads `performDepth == 1` before the defer fires.
- `WhereStore.addSample(_:)` -> `WhereStore.add(sample:)` to match
  the Swift API guidelines: the noun should be the parameter label,
  not part of the method name. Propagated to `SwiftDataStore`,
  `WhereController` (four GPS/ingest/manual call sites), and the
  `ToggleFailingStore` test double.
- `SwiftDataStore.Storage.default`: build- and test-aware default
  computed property — `.inMemory` when `XCTestConfigurationFilePath`
  is set (covers XCTest, Swift Testing, `xcodebuild test`, `swift
  test`), `.localOnly` in DEBUG, `.cloudKit` in release. App-level
  wiring can now read `Storage.default` instead of hard-coding
  `.cloudKit` and accidentally writing into the user's iCloud during
  tests. Tests that want a specific mode still call
  `SwiftDataStore.inMemory()` directly.
- New `StorageDefaultTests.storageDefault_isInMemoryUnderTestRunner`
  asserts `Storage.default == .inMemory` so we'd catch any
  regression of the env-var detection (env var renames, test runner
  changes, etc.) before a real test build wrote to disk.

Verification: `./swiftformat --lint` clean; `tuist test` green
across all 10 suites (35 tests, +1 from `StorageDefaultTests`).

Co-authored-by: Cursor <cursoragent@cursor.com>
… bbox

Three PR-feedback items, all in the region attribution layer:

- `RegionAttributor.bundled` -> `RegionAttributor.shared` to match
  the Swift convention (`URLSession.shared`, `JSONDecoder.shared`,
  ...). Updated `WhereController`'s default arg, both test suites,
  and the resources README.
- Extracted every GeoJSON primitive (`FeatureCollection`, `Feature`,
  `Properties`, `Geometry`, `polygons(at:)`, `polygons(from:)`,
  `makePolygon(from:)`) out of `RegionAttributor.swift` and into a
  new `GeoJSON` namespace enum in `Sources/GeoJSON.swift`. The
  attributor now consumes GeoJSON instead of defining it inline —
  separation of concerns, and the GeoJSON helpers are reusable
  outside the attributor if we ever need them. No behavior change;
  decoder shapes and `assertionFailure` semantics are identical.
- `RegionPolygons.init`: replaced the `?? BoundingBox(...)` silent
  fallback with `assertionFailure` + a named `BoundingBox.empty`
  sentinel. An empty polygon set is a programmer error (the bundle
  loader already `assertionFailure`s on missing/unparseable
  resources, so we should never reach this branch); now the error
  fires in debug builds, and release builds fall back to a
  degenerate "contains nothing" bbox so the region simply never
  matches instead of crashing on missing data.

Verification: `./swiftformat --lint` clean; `tuist test
WhereCoreTests` green across all 10 suites (35 tests).

Co-authored-by: Cursor <cursoragent@cursor.com>
`NSLocalizedString("region.\(rawValue)", ...)` was correct at runtime
but invisible to Xcode's string-catalog extractor — the key is
composed at runtime, so the extractor can't see it and the catalog
would silently drift from the enum.

Replaced with a `switch` over every case, each branch calling
`String(localized: "region.<case>", bundle: .module)` with a literal
key. Three wins:

1. Xcode genstrings / `xcrun extractLocStrings` (and the
   `LocalizedStringResource` macro pipeline) can statically find
   every key.
2. Adding a new `Region` case is now a compile error until you wire
   it up — no more silent rawValue fallback that would ship the data
   identifier in the UI.
3. `String(localized:)` is the modern Swift surface and reads
   cleanly per case.

Tests already cover both branches: `RegionTests` checks every case
returns its catalog English value (not the raw key) and
`localizedName_isNonEmpty`. All green.

Verification: `./swiftformat --lint` clean; `tuist test
WhereCoreTests` green across all 10 suites (35 tests).

Co-authored-by: Cursor <cursoragent@cursor.com>
Three connected PR-feedback items, all in the evidence model:

- `EvidenceKind` / `SampleSource`: dropped the hand-rolled
  `init(from:)` and `encode(to:)`. The synthesized Codable for an
  enum-with-associated-values is fine for our wire format and
  removes ~30 lines of error-prone glue. The on-disk SwiftData
  mapping keeps using `discriminator` + sibling columns (those are
  unchanged), and the value-level Codable is greenfield so the
  wire-format shift (`{"kind":"other","otherLabel":"x"}` ->
  `{"other":{"_0":"x"}}`) has no migration cost.

- `fromDiscriminator`: replaced `switch ... { default: nil }` with
  a `for candidate in knownCases where ...` loop wrapping an
  exhaustive `switch` over the enum (no `default:`), then a
  `return nil` after the loop. Adding a new case to `EvidenceKind`
  or `SampleSource` is now a compile error inside the switch so the
  discriminator <-> case mapping can't silently fall out of date.
  Applied to both `EvidenceKind.fromDiscriminator` and
  `SampleSource.fromDiscriminator` for consistency. `SampleSource`
  grew a private `knownCases` array (the `.evidenceImplied`
  placeholder uses zero-UUID + `.other(nil)`; it's never returned,
  only walked for discriminator matching).

- `EvidenceContentType`: converted from a `String` raw-value enum
  to an associated-value enum mirroring `EvidenceKind`. Gained
  `case other(String?)` for unclassified attachments (matching
  PR feedback "make this required, add other(String?)"). Now
  carries `discriminator` / `fromDiscriminator` / `knownCases`.

- `Evidence.contentType`: made non-optional (`EvidenceContentType`
  vs `EvidenceContentType?`). The `init` argument is required —
  no default — so every call site has to think about the rendering
  hint. Existing call sites (`WhereControllerTests`,
  `SimulatedYear`) updated to pass `.plainText` and `.other(nil)`
  respectively. The SwiftData mapping `SDEvidence` now stores
  `contentTypeDiscriminator` + `contentTypeOtherLabel` instead of
  `contentTypeRaw`; `toValue()` falls back to `.other(nil)` for
  legacy rows so nothing gets silently dropped.

- Tests: `EvidenceKindTests.unknownDiscriminator_throws` updated to
  `{"madeUp":{}}` (synthesized format). `SampleSourceTests` updated
  the two manual-encoding assertions to check the synthesized
  shapes: `manualSource_dropsEvidenceFieldsInEncoding` asserts
  `"id"`/`"kind"` strings are absent (manual encodes as
  `{"manual":{}}`); `evidenceImpliedDecoding_missingFields_throws`
  uses `{"evidenceImplied":{}}` (empty payload triggers
  `keyNotFound`).

Verification: `./swiftformat --lint` clean; `tuist test
WhereCoreTests` green across all 10 suites (35 tests).

Co-authored-by: Cursor <cursoragent@cursor.com>
Comment thread Where/WhereCore/Sources/Persistence/SwiftDataStore.swift
PR feedback: writes should go through a separate write context so the
main context is effectively read-only — addressing both rollback
semantics and future SwiftUI @query safety.

SwiftData has no true parent/child context like Core Data, but a peer
`ModelContext` on the same `ModelContainer` gives us the properties we
care about: transactional isolation and a clean main context.

Approach:

- Added `writerContext: ModelContext?` on the actor. Outermost
  `perform { ... }` creates a fresh peer context, stashes it,
  saves it on success (changes propagate to the persistent store;
  the main `modelContext` picks them up on next fetch), or
  discards it on throw (clean rollback of the entire transaction).
  Nested `perform` calls reuse the in-flight peer so they coalesce
  into a single transaction.
- Replaced the `performDepth` counter with a simpler
  `writerContext != nil` check — the same information at half the
  state.
- `mutationContext()` traps with `preconditionFailure` if a write
  is called outside `perform`. The protocol doc has always
  required `perform`-wrapping; the trap now makes the broken
  contract surface immediately instead of silently no-op-ing the
  save. Every existing call site (`WhereController`, the
  `ToggleFailingStore` test double, the `SwiftDataStore.inMemory()`
  helpers) already wraps mutations in `perform`, so no caller had
  to change.
- `readContext()` returns the writer peer when inside `perform`
  (so reads-within-the-transaction see staged writes via
  `includePendingChanges`) and the main `modelContext` otherwise
  (committed state only). The main context is now never written
  to — it's purely a read view.
- Mutating `EvidenceBlobStore` methods (`write(blob:for:)`,
  `delete(for:)`) use the writer context; `read(for:)` uses
  `readContext()`.

Tests:

- `performThrow_rollsBackEntireTransaction` — stages two
  `add(sample:)` calls inside a `perform` that throws; afterward
  `allSamples()` is empty, confirming the peer was discarded.
- `performSuccess_writesAreVisibleToReadersAfterReturn` — confirms
  peer save propagates to the main read context.
- `readsInsidePerform_seePendingWritesOnPeer` — confirms reads
  inside `perform` see writes staged on the peer (preserves the
  read-your-own-writes-in-a-transaction behavior that
  `includePendingChanges = true` previously gave us on the single
  context).

Updated the `WhereStore.perform` doc to describe the rollback +
atomic-commit semantics, since the protocol contract now matches
what implementations enforce.

Verification: `./swiftformat --lint` clean; `tuist test
WhereCoreTests` green across all 10 suites (38 tests, +3 from the
peer-context behavior tests).

Co-authored-by: Cursor <cursoragent@cursor.com>
@kyleve kyleve merged commit 4799107 into main May 27, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant