From c5aa92748f857b444f8add8f977b223a7763042b Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Mon, 27 Jul 2026 01:32:51 -0700 Subject: [PATCH 1/8] Purge the disproven dynamic-framework story from source comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plan step: Rewrite disproven Project.swift comments + fix stale anchor. PR #145 corrected the root AGENTS.md — nothing in this project is a dynamic framework; the local package links statically into each consumer — but the code comments it points at still taught the old mechanism. - Project.swift: the WhereUITests and WhereIntentsTests comments no longer claim WhereUI is a dynamic framework or that several .xctest bundles co-load into one StuffTestHost process (each bundle gets its own). Both now cite the corrected anchor ("Never double-link a product WhereUI already carries") and PR #145 for the mechanism. - SnapshotCaptureFlagProbe.swift / WhereStylesheet.swift: dropped the "(a dynamic framework)" parentheticals from two doc comments found by the tree-wide sweep. Comment-only changes; no behavior. Verified with ./ide --no-open (project generates). --- Project.swift | 37 ++++++++----------- .../Shared/SnapshotCaptureFlagProbe.swift | 2 +- .../Sources/Shared/WhereStylesheet.swift | 6 +-- 3 files changed, 19 insertions(+), 26 deletions(-) diff --git a/Project.swift b/Project.swift index 63426a71..60137d27 100644 --- a/Project.swift +++ b/Project.swift @@ -433,34 +433,27 @@ let project = Project( sources: ["Where/WhereCore/Tests/**"], extraPackageProducts: ["RegionKit"], ), - // WhereUITests deliberately lists no `extraPackageProducts`. WhereUI is a - // dynamic framework that statically embeds its own dependencies, so any - // product *also* linked here would land a second copy in this bundle; - // with several .xctest bundles loaded into one StuffTestHost that - // duplicates the module's type metadata, and any type-keyed lookup - // crossing the WhereUI boundary (SwiftUI `EnvironmentKey`s, - // `UITraitBridgedEnvironmentKey` bridging, the type-keyed - // BTraits/BThemes/BStylesheets containers) then silently resolves against - // the wrong copy — the writer stores under one copy's key type, the - // reader looks it up under another's. Everything the tests need - // (BroadwayCore/BroadwayUI, LifecycleKit/LifecycleKitUI, PeriscopeCore/UI/Tools, - // SwiftDataInspector, RegionKit + its GeoJSON bundle) is reached - // transitively through WhereUI. - // See "Never double-link a product a dynamic framework already - // carries" in the root AGENTS.md. + // WhereUITests deliberately lists no `extraPackageProducts`: everything it + // needs (Broadway, LifecycleKit/LifecycleKitUI, Periscope, SwiftDataInspector, + // RegionKit + its GeoJSON bundle) arrives statically through WhereUI, and + // re-listing one lands a second copy in this image, silently breaking + // type-keyed lookups — only in the full multi-bundle scheme, never in an + // isolated `tuist test WhereUITests` run. + // Guard: WhereStylesheetTests.resolvesTraitAwareTokensFromTheBroadwayRoot. + // See "Never double-link a product WhereUI already carries" in the root + // AGENTS.md; mechanism: PR #145. unitTests( name: "WhereUITests", bundleIdSuffix: "whereui", productDependency: "WhereUI", sources: ["Where/WhereUI/Tests/**"], ), - // WhereIntents depends on WhereUI (a dynamic framework) for its snippet - // cards, so — exactly like WhereUITests above — this bundle lists no - // `extraPackageProducts`: WhereUI/WhereCore/RegionKit/Broadway all arrive - // transitively, and re-listing any of them would land a duplicate copy - // that splits the module's type metadata across the WhereUI boundary. - // See "Never double-link a product a dynamic framework already - // carries" in the root AGENTS.md. + // WhereIntents depends on WhereUI for its snippet cards, so — exactly like + // WhereUITests above — this bundle lists no `extraPackageProducts`: + // WhereUI/WhereCore/RegionKit/Broadway all arrive transitively, and + // re-listing any of them would land a duplicate copy in this image. + // See "Never double-link a product WhereUI already carries" in the root + // AGENTS.md. unitTests( name: "WhereIntentsTests", bundleIdSuffix: "whereintents", diff --git a/Where/WhereUI/Sources/Shared/SnapshotCaptureFlagProbe.swift b/Where/WhereUI/Sources/Shared/SnapshotCaptureFlagProbe.swift index 96e74fad..949a49cc 100644 --- a/Where/WhereUI/Sources/Shared/SnapshotCaptureFlagProbe.swift +++ b/Where/WhereUI/Sources/Shared/SnapshotCaptureFlagProbe.swift @@ -5,7 +5,7 @@ /// Test-only cross-boundary probe for `\.isCapturingSnapshot`: green when the /// flag reads `true`, red otherwise. /// - /// This view exists to be *compiled into WhereUI* (the dynamic framework) and + /// This view exists to be *compiled into WhereUI* and /// pixel-probed from the snapshot bundle /// (`WhereUISnapshotTests.SnapshotCaptureFlagProbeTests`): the bundle /// statically embeds a second copy of `SnapshotKit` via `SnapshotKitTesting` diff --git a/Where/WhereUI/Sources/Shared/WhereStylesheet.swift b/Where/WhereUI/Sources/Shared/WhereStylesheet.swift index 966c7e75..5c3fedfc 100644 --- a/Where/WhereUI/Sources/Shared/WhereStylesheet.swift +++ b/Where/WhereUI/Sources/Shared/WhereStylesheet.swift @@ -934,9 +934,9 @@ extension View { /// /// Lives here (not called as `broadwayRoot` at each site) so callers only /// need to import `WhereUI`: `WhereWidgets` must not link `BroadwayUI` - /// directly — it already gets it through `WhereUI` (a dynamic framework), and - /// a second copy would split Broadway's type-keyed environment metadata (see - /// the root `AGENTS.md` "Targets" note). + /// directly — it already gets it through `WhereUI`, and a second copy would + /// split Broadway's type-keyed environment metadata (see the root + /// `AGENTS.md` "Targets" note). /// /// Also seeds `\.regionStyles` so descendants resolve per-region looks /// (`region` cards, calendar dots, widgets, snippets) from one place. The app From 11be497f52ff156fd5324ae4e00a2fd54b44a2e6 Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Mon, 27 Jul 2026 01:34:14 -0700 Subject: [PATCH 2/8] Delete the false bootstrap-violations paragraphs from the Bumper catalog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plan step: Run bumper lint/test; delete false bootstrap paragraphs in .bumper/RULES.md. .bumper/RULES.md claimed three calendar violations and some preview-coverage violations were deliberately left visible "during this bootstrap". Neither exists: swift run bumper lint . reports no architecture violations (all 15 rule fixture tests green), and 52f0136 closed the preview ones. The paragraphs read as an enforced-drift demonstration that the tree no longer backs. Closes the docs(Bumper) P0 sub-item in TODOs.md (moved to Completed); the parent P0 — where.gregorian_calendar matches only an explicit Calendar base and so enforces nothing — was re-confirmed and stays open. --- .bumper/RULES.md | 10 ---------- TODOs.md | 4 +++- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/.bumper/RULES.md b/.bumper/RULES.md index c36fd164..d4e60c62 100644 --- a/.bumper/RULES.md +++ b/.bumper/RULES.md @@ -98,12 +98,6 @@ fully expresses Where's constructor facts. production sources. Day and year math uses an injected Gregorian calendar or, inside WhereIntents, `Calendar.whereIntents`. -The current source tree intentionally contains three violations. They are left -visible during this bootstrap so the live Bumper lint demonstrates the -difference between a documented decision and an enforced one. The rule suite -still passes because its fixtures assert both accepted Gregorian construction -and rejected `Calendar.current` access. - The architecture DSL and standard shapers cannot distinguish two static members of the same Foundation type, so this uses a typed `MemberAccessExprSyntax` query. TheButtonHeist's @@ -140,10 +134,6 @@ typed event declarations live. that declares a `View`, `Widget`, or `WidgetBundle` struct to contain at least one `#Preview`. -The current source tree intentionally contains preview-coverage violations. -They remain visible during this bootstrap alongside the calendar violations so -the live lint demonstrates documented rules finding existing drift. - ## Transitive Broadway ownership WhereIntents and WhereWidgets explicitly forbid direct `BroadwayCore` and diff --git a/TODOs.md b/TODOs.md index e6ce408d..63b298d7 100644 --- a/TODOs.md +++ b/TODOs.md @@ -93,7 +93,6 @@ inbox rather than here. ## P0s (Must do) - fix(Bumper) [quick-win]: `where.gregorian_calendar` matches only an explicit `Calendar` base, so it enforces nothing. It filters `MemberAccessExprSyntax` on `base?.trimmedDescription == "Calendar"` (`.bumper/Sources/WhereProjectRules.swift:121`), which catches a spelled-out `Calendar.current` but not the implicit-member form (`calendar: Calendar = .current`, `startOfDay(in: .current)`) — and after the Gregorian call-site pass (`fe99dde`) the implicit form is the only one left in the tree. CI hard-gates `bumper lint` at `severity: .error` and is green, which confirms it: the rule reports nothing while production sites drift. Also match a no-base `MemberAccessExprSyntax` whose contextual type is `Calendar`, or add a lexical `.current` check scoped to calendar parameters and arguments. A rule that reads as enforced but enforces nothing is worse than a documented convention, because it stops anyone from looking. Pairs with the `CalendarDay.displayDate` P1 in [`Where/TODOs.md`](Where/TODOs.md). (audit 2026-07-26) - - docs(Bumper) [quick-win]: Correct `.bumper/RULES.md:101` and `:143`, which claim three calendar violations and some preview-coverage violations are "left visible during this bootstrap". Neither exists — the lint gate is green, and `52f0136` closed the preview ones. Delete both paragraphs, and re-add the calendar one only if the widened rule genuinely finds drift. (audit 2026-07-26) ## P1s (Should do) @@ -112,6 +111,9 @@ inbox rather than here. # Completed issues +## P0s (Must do) +- docs(Bumper) [quick-win]: Correct `.bumper/RULES.md:101` and `:143`, which claimed three calendar violations and some preview-coverage violations were "left visible during this bootstrap". Neither existed — the lint gate was green, and `52f0136` closed the preview ones. Closed by deleting both paragraphs after re-confirming a clean `swift run bumper lint .` ("No architecture violations found") and a green `swift run bumper test .`; re-add the calendar paragraph only if the widened rule genuinely finds drift. (audit 2026-07-26, closed 2026-07-27) + ## P1s (Should do) - test(CreditKit) [needs-design]: The attribution drift guard didn't detect a stale report, so the app could ship notices that don't govern the code in it. **Closed by `./attribution --check`**, a network-free mode that re-derives the expected report from `Package.swift`, `Package.resolved`, and the skills manifest and diffs it against the committed one, gated in CI beside the other lints (`.github/workflows/ci.yml`). The literal name lists in `AppAttributionTests` are gone — a test bundle can't read the manifests, so all it could compare against was a literal — and that suite now asserts only what the shipping bundle can answer. **As originally filed this item was wrong on a detail worth recording:** it claimed the old guard caught an added dependency but not a bumped one. It caught neither. Comparing the report to a hardcoded list means a dependency added without regenerating leaves report and list still agreeing, which is exactly what happened — the guard passed on a report missing the two snapshot packages that merging `main` had added. (pr#140 review, closed 2026-07-26) From 22257115f1681410a1bff5a7aeeef92481bb4282 Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Mon, 27 Jul 2026 01:37:59 -0700 Subject: [PATCH 3/8] Correct the factually wrong claims across the AGENTS.md tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plan step: Correct the ~10 confirmed-wrong AGENTS.md claims. Each claim was re-verified against Project.swift / Package.swift / source before editing: - Where/AGENTS.md: Where is an iOS/iPadOS app (destinations are iPhone/iPad; Catalyst is RegionViewer only), and App Intents run in the app process — WhereIntents is a library product linked into the app, not an extension with its own process. - WhereCore/AGENTS.md: widgets never talk to WhereServices; they read the App Group WidgetSnapshot. - Where/Where/AGENTS.md: attribution-sources.json lives at the module root, not beside Resources/attribution.json. - WhereShareExtension/AGENTS.md: the extension's compose/save path is its own ShareEvidenceModel (untested, tracked in Where/TODOs.md), not a WhereUI compose model. Closes that half of the Where/TODOs.md stale-doc-claims item. - RegionViewer/AGENTS.md: geometry-catalog coverage is RegionKitTests.RegionGeometryCatalogTests; RegionMapView coverage is WhereUISnapshotTests. - SnapshotKitTesting/AGENTS.md: StuffSnapshotTests is a scheme, not a bundle — consumers are the three per-module image bundles plus SnapshotKitTestingTests; ConcurrentCaptureTests and LargeViewCaptureTests live in SnapshotKitTestingTests; SnapshotCaptureFlagProbeTests lives in WhereUISnapshotTests; the SnapshotKit double-embed applies only to the WhereUI bundle. - Broadway/AGENTS.md + BroadwayCatalog/AGENTS.md: BroadwayCatalogTests is hosted by the BroadwayCatalog app, not StuffTestHost. - SwiftDataInspector/AGENTS.md: the fixture-name-clash caveat now states the measured per-bundle host-process model. - PeriscopeTools/AGENTS.md: dropped the 'were it ever dynamic' framing; nothing in this project builds as a dynamic framework. - CreditKit/AGENTS.md: SampleReport.json is a string constant on the SampleReport enum, not a fixture file. --- Shared/Broadway/AGENTS.md | 10 ++++--- Shared/Broadway/BroadwayCatalog/AGENTS.md | 2 +- Shared/CreditKit/AGENTS.md | 5 ++-- Shared/Periscope/PeriscopeTools/AGENTS.md | 9 +++--- Shared/SnapshotKitTesting/AGENTS.md | 34 +++++++++++++---------- Shared/SwiftDataInspector/AGENTS.md | 9 +++--- Where/AGENTS.md | 10 ++++--- Where/RegionViewer/AGENTS.md | 5 ++-- Where/TODOs.md | 2 +- Where/Where/AGENTS.md | 3 +- Where/WhereCore/AGENTS.md | 5 ++-- Where/WhereShareExtension/AGENTS.md | 5 ++-- 12 files changed, 56 insertions(+), 43 deletions(-) diff --git a/Shared/Broadway/AGENTS.md b/Shared/Broadway/AGENTS.md index ff123d5e..49e6741b 100644 --- a/Shared/Broadway/AGENTS.md +++ b/Shared/Broadway/AGENTS.md @@ -29,7 +29,8 @@ bundles in [`Project.swift`](../../Project.swift) (the shared `unitTests` helper - **`BRootViewController` defers setup** — child creation, trait observation, and context are wired on `viewIsAppearing`, so `context` is `nil` before the controller enters a valid hierarchy. -- **Broadway tests run in the shared `StuffTestHost`** via `TestHostSupport` +- **Broadway's hosted bundles (`BroadwayCoreTests`, `BroadwayUITests`) run in + the shared `StuffTestHost`** via `TestHostSupport` (`show`, `hostKeyWindow`). The host stamps its window with `isMainTestHostWindow` and `hostKeyWindow()` selects only that window — don't reintroduce a "first key window" or `UIApplication.shared.delegate?.window` @@ -37,6 +38,7 @@ bundles in [`Project.swift`](../../Project.swift) (the shared `unitTests` helper ## Testing -Hosted Swift Testing bundles (`BroadwayCoreTests`, `BroadwayUITests`, -`BroadwayCatalogTests`) run in `StuffTestHost` and link `TestHostSupport`. 1:1 -test files per the root rules. +`BroadwayCoreTests` and `BroadwayUITests` run in `StuffTestHost` and link +`TestHostSupport`; `BroadwayCatalogTests` is hosted by the **BroadwayCatalog** +app itself (its target depends on the app, not `StuffTestHost` — +[`Project.swift`](../../Project.swift)). 1:1 test files per the root rules. diff --git a/Shared/Broadway/BroadwayCatalog/AGENTS.md b/Shared/Broadway/BroadwayCatalog/AGENTS.md index fa2e6bff..f3ab9e44 100644 --- a/Shared/Broadway/BroadwayCatalog/AGENTS.md +++ b/Shared/Broadway/BroadwayCatalog/AGENTS.md @@ -14,5 +14,5 @@ Complements the root [`AGENTS.md`](../../../AGENTS.md) and the group - Declared as a Tuist `.app` target (`com.stuff.broadway.catalog`), iPhone/iPad destinations. -Tests: `BroadwayCatalogTests` in `StuffTestHost` +Tests: `BroadwayCatalogTests`, hosted by this app itself — not `StuffTestHost` (`tuist test BroadwayCatalogTests`). diff --git a/Shared/CreditKit/AGENTS.md b/Shared/CreditKit/AGENTS.md index cf6f05f6..1c6fe02c 100644 --- a/Shared/CreditKit/AGENTS.md +++ b/Shared/CreditKit/AGENTS.md @@ -75,6 +75,7 @@ thing that writes a report. `CreditKitTests` covers the manifest as a format and an API: decoding the exact JSON the generator writes, rejecting a malformed report or an unknown `kind`, round-tripping, filtering by kind, and `load` throwing for a bundle with no -report. Shared fixtures live in `CreditKitTestSupport.swift`, and -`SampleReport.json` is a literal rather than an encoder round-trip so a Swift-side +report. Shared fixtures live in `CreditKitTestSupport.swift`; its +`SampleReport.json` (a string constant on the `SampleReport` enum, not a +fixture file) is a literal rather than an encoder round-trip so a Swift-side change that breaks the wire format fails a test. diff --git a/Shared/Periscope/PeriscopeTools/AGENTS.md b/Shared/Periscope/PeriscopeTools/AGENTS.md index a8d536ee..588e66fd 100644 --- a/Shared/Periscope/PeriscopeTools/AGENTS.md +++ b/Shared/Periscope/PeriscopeTools/AGENTS.md @@ -48,11 +48,10 @@ use `PeriscopeStylesheet.default`. the picker and writes the choice back on change. - **Color decisions live in `Palette`**, not on `LogLevel` / `SpanExit.Mode` — `tint(forLevel:)` bands by severity so custom levels inherit a sensible color. -- Because PeriscopeTools links Broadway as a **static** library it can seed - Broadway directly. Were it ever to become a dynamic framework, or be embedded - in one, the root - [double-linking rule](../../../AGENTS.md#never-double-link-a-product-whereui-already-carries) - would start applying to its consumers. +- PeriscopeTools seeds Broadway directly; a consumer must not re-list + `BroadwayCore`/`BroadwayUI` beside a product that already carries them — the + root + [double-linking rule](../../../AGENTS.md#never-double-link-a-product-whereui-already-carries). ## Invariants diff --git a/Shared/SnapshotKitTesting/AGENTS.md b/Shared/SnapshotKitTesting/AGENTS.md index bbc0c809..5c88e3a5 100644 --- a/Shared/SnapshotKitTesting/AGENTS.md +++ b/Shared/SnapshotKitTesting/AGENTS.md @@ -10,9 +10,12 @@ Complements the root [`AGENTS.md`](../../AGENTS.md) — read that first. - Depends on `SnapshotKit`, `TestHostSupport`, `SnapshotTesting` (swift-snapshot-testing), and `AccessibilitySnapshot` (cashapp). It links the - comparison engine + XCTest/Testing, so it is **only** consumed by the - `StuffSnapshotTests` and `SnapshotKitTestingTests` bundles via - `extraPackageProducts` — **never** a shipping app or `StuffTestHost`. + comparison engine + XCTest/Testing, so it is **only** consumed by test + bundles via `extraPackageProducts` — the per-module image bundles + (`WhereUISnapshotTests`, `PeriscopeToolsSnapshotTests`, + `SwiftDataInspectorSnapshotTests`, gathered into the `StuffSnapshotTests` + *scheme*) and `SnapshotKitTestingTests` — **never** a shipping app or + `StuffTestHost`. - **"Process-global" here means per *process*, and that is only safe because each test bundle gets its own.** Everything below described as process-global — the safe-area swizzle's depth counter and override globals, @@ -29,17 +32,18 @@ Complements the root [`AGENTS.md`](../../AGENTS.md) — read that first. that as this module's load-bearing environmental assumption: if bundles ever start sharing a host process, this state has to become genuinely process-wide before another consumer is added. -- **The consuming bundle double-embeds `SnapshotKit`, tolerated and guarded.** +- **`WhereUISnapshotTests` double-embeds `SnapshotKit`, tolerated and guarded.** Listing this product in `extraPackageProducts` statically embeds its dependency closure — including `SnapshotKit` — into the `.xctest`, and WhereUI statically embeds its own copy into the same image: the duplicate-type-metadata hazard from the root `AGENTS.md` "Targets" note, with `\.isCapturingSnapshot` as the type-keyed cross-boundary lookup at - risk. There is no cleaner wiring (the pipeline must reach the bundle without - ever linking into the UI module), the trait lookup demonstrably resolves - across both copies today, and - `StuffSnapshotTests.SnapshotCaptureFlagProbeTests` fails loudly if the - copies ever split — see the StuffSnapshotTests comment in + risk. This applies only to that bundle — the Periscope and + SwiftDataInspector image bundles don't link WhereUI at all. There is no + cleaner wiring (the pipeline must reach the bundle without ever linking into + the UI module), the trait lookup demonstrably resolves across both copies + today, and `WhereUISnapshotTests.SnapshotCaptureFlagProbeTests` fails loudly + if the copies ever split — see the snapshot-bundle comment in `Project.swift` for the full topology. - Re-exports `SnapshotKit` and `SnapshotTesting` so consumers need one import. - Library target in [`Package.swift`](../../Package.swift). @@ -86,7 +90,7 @@ Complements the root [`AGENTS.md`](../../AGENTS.md) — read that first. `renderSnapshotImage` serializes captures through a FIFO `@MainActor` mutex, and the safe-area swizzle is depth-counted so unbalanced pairs can't flip the method-exchange parity (guarded by - `StuffSnapshotTests.ConcurrentCaptureTests`). Nested captures — a hook + `SnapshotKitTestingTests.ConcurrentCaptureTests`). Nested captures — a hook rendering another snapshot — trap. Keep the suite serial anyway: concurrent scheduling now degrades to queued-serial rather than corrupt, gaining nothing. - **Rendering runs in the host key window.** It requires `StuffTestHost`'s window @@ -103,7 +107,7 @@ Complements the root [`AGENTS.md`](../../AGENTS.md) — read that first. - **Tile-and-stitch is load-bearing, not legacy.** UIKit still renders a blank image for views taller/wider than ~2000pt on the target toolchain (iOS 27.0 — verified by a probe during development, guarded by - `StuffSnapshotTests.LargeViewCaptureTests`). Captures go through + `SnapshotKitTestingTests.LargeViewCaptureTests`). Captures go through `SnapshotWrappingViewController` + `tileAndStitchImage`; don't remove the tiling on the assumption the bug is fixed without re-running that check. @@ -120,7 +124,7 @@ the captured root while an interior `safeAreaInset` still composes). They render probed pixels via the `@_spi(Testing)` `PixelSample`/`probePixel` API rather than LFS reference images — so the bundle is fast, has no `__Snapshots__/`, and runs in the main `test` job, not the snapshot job. The matrixed image assertions -themselves are still exercised by the consumer bundle -(`StuffSnapshotTests`); the WhereUI↔bundle cross-boundary flag probe stays there -(`SnapshotCaptureFlagProbeTests`), since only a WhereUI-defined view can detect a -duplicate-`SnapshotKit` split. +themselves are still exercised by the per-module image bundles; the +WhereUI↔bundle cross-boundary flag probe lives in `WhereUISnapshotTests` +(`SnapshotCaptureFlagProbeTests`), since only a WhereUI-defined view can detect +a duplicate-`SnapshotKit` split. diff --git a/Shared/SwiftDataInspector/AGENTS.md b/Shared/SwiftDataInspector/AGENTS.md index 722f45bd..a156e770 100644 --- a/Shared/SwiftDataInspector/AGENTS.md +++ b/Shared/SwiftDataInspector/AGENTS.md @@ -59,7 +59,8 @@ and its CI job (root [`AGENTS.md`](../../AGENTS.md#targets)). It links only `SwiftDataInspector`, so nothing here builds against an app module. Snapshot the module the way a consumer gets it: a **local fixture schema**, never a host app's store, and no design-system root. Prefer adding a case there over a hosted "renders without -crashing" test, which asserts nothing about the result. Fixture `@Model` names -must not collide with the ones in [`Tests/`](Tests) — those are a separate -bundle, and duplicate model class names would clash in the runtime if both ever -loaded into one host process. +crashing" test, which asserts nothing about the result. Keep fixture `@Model` +names distinct from the ones in [`Tests/`](Tests) as hygiene — the two bundles +can't collide today because each `.xctest` gets its own host process (root +[`AGENTS.md`](../../AGENTS.md#targets)), but identical class names would clash +in the ObjC runtime if a toolchain ever co-loaded them. diff --git a/Where/AGENTS.md b/Where/AGENTS.md index 61e2e46f..0b28ec60 100644 --- a/Where/AGENTS.md +++ b/Where/AGENTS.md @@ -1,6 +1,6 @@ # Where – Feature Shape -Where is an iOS/iPadOS/macOS app for answering "what region was I in on which +Where is an iOS/iPadOS app for answering "what region was I in on which day?" It ingests passive GPS (Visits + significant-change), accepts user-asserted history (manual coordinates, whole-day overlays, evidence like boarding passes), and rolls everything up into per-day region presence and @@ -69,9 +69,11 @@ Rules the code enforces and agents must preserve: `WhereLog`, so it owns a `"RegionKit"` root — but emits into the *same* `Periscope.shared`, so the single `PeriscopeStore` sink the app attaches at launch (and `PeriscopeViewer`) sees every subtree in one stream. - - **Only the app process gets a store.** Widgets, the share extension, and - intents run in their own processes, where `Periscope.shared` stays - OSLog-only. + - **Only the app process gets a store.** Widgets and the share extension run + in their own processes, where `Periscope.shared` stays OSLog-only. App + Intents run **in the app process** (`WhereIntents` is a library product + linked into the Where app, not an extension), so their logs reach the + app's store-attached Periscope. - **An event about a store object stamps its `externalID` with that object's canonical `store://` identity** (`DataIssueID.storeURL`, otherwise `WhereStoreID`), so inspect-by-object shares the store's and backups' keys. diff --git a/Where/RegionViewer/AGENTS.md b/Where/RegionViewer/AGENTS.md index d7a6ee32..08f71e4a 100644 --- a/Where/RegionViewer/AGENTS.md +++ b/Where/RegionViewer/AGENTS.md @@ -19,5 +19,6 @@ This file complements the root [`AGENTS.md`](../../AGENTS.md) and the feature feature needs more, add it in `WhereUI`/`WhereCore`. - **The repo's only Catalyst target** — keep it buildable for `ios-macabi` (`tuist build RegionViewer` on macOS verifies). -- No test bundle; `RegionMapView` and the geometry catalog are covered from - WhereUI/WhereCore. +- No test bundle; the geometry catalog is covered by `RegionKitTests` + (`RegionGeometryCatalogTests`), and `RegionMapView` by WhereUI's snapshot + bundle (`WhereUISnapshotTests`). diff --git a/Where/TODOs.md b/Where/TODOs.md index 1750d458..7022fe33 100644 --- a/Where/TODOs.md +++ b/Where/TODOs.md @@ -81,7 +81,7 @@ The item format and the placement rule live in the root - test(WhereUI) [quick-win]: Add the missing namesake tests for `LocationNamer` (cache / coalescing) and `CalendarContentView`'s `scrolledForYear` scroll-reveal gate, which has hosting smoke only. (audit 2026-07-26) - test(WhereIntents) [quick-win]: Test `RegionSpotlightIndexer` and `WhereIntentReader.recentActivity`. (audit 2026-07-26) - test(WhereCore) [needs-design]: Close the namesake-test debt — 28 of the implementation files have no `*Tests.swift` (notably `FoundationModelSummaryGenerator`, `WherePreferences`, `WidgetTimelineRefresher`, `BackupArchive`), and `WhereCoreTests.swift` is an omnibus still holding `YearReportTests`. Split by concern as those files change rather than in one pass. (audit 2026-07-26) -- docs(WhereCore) [quick-win]: Refresh five stale doc claims — `RootView.swift:10` still describes four top-level screens; `WhereCore/README.md:48`, `:161` claim every write reconciles and errors are never swallowed; `LocationIngestor.swift:334` says it logs through `os.Logger` when it emits typed `WhereLog` events; `WhereShareExtension/AGENTS.md:21` credits the write path to a WhereUI compose model when production uses `ShareEvidenceModel`; `RegionViewer/README.md:14` describes a monolithic `us-states.geojson` and a hand-listed region set. `RegionKit/README.md:144` also claims GeoJSON decoding is covered, which the test item above would make true instead. (audit 2026-07-26) +- docs(WhereCore) [quick-win]: Refresh stale doc claims — `RootView.swift:10` still describes four top-level screens; `WhereCore/README.md:48`, `:161` claim every write reconciles and errors are never swallowed; `LocationIngestor.swift:334` says it logs through `os.Logger` when it emits typed `WhereLog` events; `RegionViewer/README.md:14` describes a monolithic `us-states.geojson` and a hand-listed region set. `RegionKit/README.md:144` also claims GeoJSON decoding is covered, which the test item above would make true instead. (The `WhereShareExtension/AGENTS.md:21` compose-model credit, originally part of this item, was fixed 2026-07-27.) (audit 2026-07-26) - refactor(WhereCore) [quick-win]: Drop the remaining Core-API parameter defaults — `DayJournal.addEvidence(_:blob:)` (`:216`), `WherePreferences.init(store:)` (`:14`), `SwiftDataStore.make(storage:)` (`:179`), and `WidgetDataReader`'s aggregator/attributor (`:74`). The composition root already knows each value. (audit 2026-07-26) - convention(WhereIntents) [quick-win]: Small polish — register `LogTripIntent` in `WhereShortcuts` (`:11` registers five) or document Shortcuts-only discovery; use `Calendar.whereIntents` for `LogDayIntent`'s default day instead of `Date()` (`:39`, no data impact today since `DayJournal` buckets Gregorian); log the App Group open failure behind `WhereIntentReader.todaySnapshot`'s `try?` (`:17`); and wrap `RegionViewer`'s `RegionMapView` in `.whereBroadwayRoot()` (`RegionViewerApp.swift:15`) so the dev tool renders with app styling. (audit 2026-07-26) - convention(RegionKit) [quick-win]: Reference a generated catalog symbol for `region.other` instead of the raw `String(localized:)` key (`RegionCatalog.swift:65`). (audit 2026-07-26) diff --git a/Where/Where/AGENTS.md b/Where/Where/AGENTS.md index 8bd21ea2..99987ab8 100644 --- a/Where/Where/AGENTS.md +++ b/Where/Where/AGENTS.md @@ -24,7 +24,8 @@ layering, and the domain rules this target merely starts up. - `Resources/AppIcon.xcassets` is managed by `./icons` (see the root [`AGENTS.md`](../../AGENTS.md#managing-app-icons)) — never hand-edit it. - `Resources/attribution.json` is the app's generated attribution report, and - `attribution-sources.json` beside it declares where the report reads from. + `attribution-sources.json` at this module's root declares where the report + reads from. Both are `./attribution`'s (see [Attribution](../../AGENTS.md#attribution)) — never hand-edit the report. This is the **only** bundle that carries one, which is why `AppAttributionTests` diff --git a/Where/WhereCore/AGENTS.md b/Where/WhereCore/AGENTS.md index e5a32611..dfa5b11f 100644 --- a/Where/WhereCore/AGENTS.md +++ b/Where/WhereCore/AGENTS.md @@ -4,8 +4,9 @@ WhereCore is the domain layer of the Where feature: the persistence boundary, GPS ingestion, per-day / per-year aggregation, data-quality detection, and the side effects that hang off a committed write (reminders, widgets, backup, on-device activity summaries). It is assembled behind one `Sendable` value — -`WhereServices` — that the UI and widgets talk to. See [`README.md`](README.md) -for the public API and how the pieces fit. +`WhereServices` — that the UI and the App Intents stack talk to (widgets never +do; they read the published `WidgetSnapshot` from the App Group). See +[`README.md`](README.md) for the public API and how the pieces fit. The **domain/presentation split and the rules WhereCore must uphold** (the `WhereServices` entry point, `WhereStore.perform` writes, the single diff --git a/Where/WhereShareExtension/AGENTS.md b/Where/WhereShareExtension/AGENTS.md index 3f96cfdc..b39c9f06 100644 --- a/Where/WhereShareExtension/AGENTS.md +++ b/Where/WhereShareExtension/AGENTS.md @@ -18,8 +18,9 @@ This file complements the root [`AGENTS.md`](../../AGENTS.md) and the feature - Presentation reuses WhereUI's public `EvidenceKind.symbolName`/`displayName`; only extension chrome lives in this target's catalog, referenced through its generated `LocalizedStringResource` symbols. -- No test bundle; the write path is covered from **WhereCore** store tests and - the **WhereUI** compose model. +- No test bundle; the store write contract is covered from **WhereCore** store + tests. This target's own compose/save model (`ShareEvidenceModel`) is + untested — tracked in [`Where/TODOs.md`](../TODOs.md). ## Invariants From 995f58a0c2360d8a9dcffa2dab6933e020ea84bf Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Mon, 27 Jul 2026 01:45:25 -0700 Subject: [PATCH 4/8] Refresh the TODO backlogs against current source MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plan step: Refresh TODOs.md files: close stale/wrong items, fix drifted citations, merge duplicates. Every open item across the five touched backlogs was re-verified against source; citations below were checked at the cited line before rewriting. - Where/TODOs.md: deleted the open P0 'Remove waitForOneRunloop' — its call sites are gone and the Completed section already records the closure, so the open copy was a leftover duplicate (the never-delete rule protects completed entries, which stay). Moved the P2 'launch can park behind a permission prompt' to Completed as invalidated: reconcileTracking() (WhereSession.swift:282-293) only reads the current authorizationStatus and never requests authorization. Cross- linked the Elsewhere-inflection P1 with its pinned broken-snapshots reference. Noted the RootView 'four screens' half of the stale-doc- claims item as already fixed (RootView.swift now reads three tabs). Refreshed ~15 drifted file:line citations. - Shared/Periscope/TODOs.md: fixed the survivesRelaunch citations (policy SpanExit.swift:83-91, warn LogSpan.swift:569) and corrected the hosting-smoke item's stale one-bundle claims — the PeriscopeToolsSnapshotTests bundle now exists per the one-bundle-per- module shape (#143). Cross-linked the same debt in SwiftDataInspector. - Shared/LifecycleKit/TODOs.md: the duplicate-gate precondition moved to LifecycleKitUI/Sources/LifecycleContainer.swift:101; added citations. - Shared/CreditKit/TODOs.md: generate-attribution.rb citations were off by ~15 lines (regex :91, API interpolation :82). --- Shared/CreditKit/TODOs.md | 2 +- Shared/LifecycleKit/TODOs.md | 2 +- Shared/Periscope/TODOs.md | 4 +-- Shared/SwiftDataInspector/TODOs.md | 2 +- Where/TODOs.md | 39 +++++++++++++++--------------- 5 files changed, 24 insertions(+), 25 deletions(-) diff --git a/Shared/CreditKit/TODOs.md b/Shared/CreditKit/TODOs.md index c5b317f6..76b79aaa 100644 --- a/Shared/CreditKit/TODOs.md +++ b/Shared/CreditKit/TODOs.md @@ -7,6 +7,6 @@ here. # Open issues ## P2s (Nice to have) -- fix [quick-win]: `github_slug` accepts anything after the host, so a malformed pin becomes a malformed API path instead of a clear error. It captures `.+?` (`Tools/generate-attribution.rb:76`) and the result is interpolated straight into `repos/#{slug}/license?ref=#{ref}` (`:67`), so a `location` of `https://github.com/foo/bar?x=y` asks for `repos/foo/bar?x=y/license?ref=…` and fails with whatever `gh` makes of that. Not a security issue: both inputs are repo-controlled (`Package.resolved`, `.agents/external-skills.json`) and `Open3.capture3` passes argv with no shell, so nothing is injectable. Constrain the capture to `[\w.-]+/[\w.-]+` so a bad pin fails as a bad pin. (pr#140 review) +- fix [quick-win]: `github_slug` accepts anything after the host, so a malformed pin becomes a malformed API path instead of a clear error. It captures `.+?` (`Tools/generate-attribution.rb:91`) and the result is interpolated straight into `repos/#{slug}/license?ref=#{ref}` (`:82`), so a `location` of `https://github.com/foo/bar?x=y` asks for `repos/foo/bar?x=y/license?ref=…` and fails with whatever `gh` makes of that. Not a security issue: both inputs are repo-controlled (`Package.resolved`, `.agents/external-skills.json`) and `Open3.capture3` passes argv with no shell, so nothing is injectable. Constrain the capture to `[\w.-]+/[\w.-]+` so a bad pin fails as a bad pin. (pr#140 review) # Completed issues diff --git a/Shared/LifecycleKit/TODOs.md b/Shared/LifecycleKit/TODOs.md index 36dbda3c..935dbf59 100644 --- a/Shared/LifecycleKit/TODOs.md +++ b/Shared/LifecycleKit/TODOs.md @@ -7,7 +7,7 @@ here. # Open issues ## P2s (Nice to have) -- test [quick-win]: Add a test that duplicate node IDs trap. `LaunchPlan.append` `precondition`s on a duplicate (and `LifecycleContainer` does the same for duplicate gate-view registrations), but nothing exercises either. (audit 2026-07-26) +- test [quick-win]: Add a test that duplicate node IDs trap. `LaunchPlan.append` `precondition`s on a duplicate (`LaunchPlan.swift:113`), and `LifecycleContainer` — now in LifecycleKitUI — does the same for duplicate gate-view registrations (`LifecycleKitUI/Sources/LifecycleContainer.swift:101`), but nothing exercises either. (audit 2026-07-26) # Completed issues diff --git a/Shared/Periscope/TODOs.md b/Shared/Periscope/TODOs.md index 45e936da..1d784329 100644 --- a/Shared/Periscope/TODOs.md +++ b/Shared/Periscope/TODOs.md @@ -16,7 +16,7 @@ here. - design(PeriscopeCore) [needs-design]: `ScopeID` derivation — hash-derived vs a concatenated, human-readable path that preserves the input for debugging. Plan/build loop. (agent) - design(PeriscopeCore) [needs-design]: `LogContextProviding` parent hierarchy — instance logs need a way to nest under a container's context (e.g. a controller inside another controller). Plan/build loop. (agent) - design(PeriscopeCore) [needs-design]: Ambient state snapshots — ambients should persist their *current state* (session-style) alongside change events, so any event joins to the system state at that moment. Plan/build loop. (Groundwork landed: ambient sources are now reference types that own their per-kind state — see the "Ambient change filtering" completed entry.) (agent) -- feat(PeriscopeCore) [needs-design]: Implement `SpanRelaunchPolicy.survivesRelaunch` resume mechanics. The policy is already recorded on `SpanBegan` payloads and the relaunch sweep honors it (surviving spans are left open, not orphan-closed), but nothing re-seeds them: `end(for:)` in the new process warns "without a matching begin" (`SpanExit.swift:86`, `LogSpan.swift:568`). Needs an async bootstrap step at store/system startup that queries unmatched surviving `SpanBegan` events and re-opens them in `Periscope.openSpans` — plus wall-clock durations for resumed spans (`ContinuousClock` instants don't survive reboot; `SpanEnded.duration` is already optional for this) and accepting that signpost intervals can't resume. (audit 2026-07-26) +- feat(PeriscopeCore) [needs-design]: Implement `SpanRelaunchPolicy.survivesRelaunch` resume mechanics. The policy is already recorded on `SpanBegan` payloads and the relaunch sweep honors it (surviving spans are left open, not orphan-closed), but nothing re-seeds them: `end(for:)` in the new process warns "without a matching begin" (policy: `SpanExit.swift:83–91`; warn: `LogSpan.swift:569`). Needs an async bootstrap step at store/system startup that queries unmatched surviving `SpanBegan` events and re-opens them in `Periscope.openSpans` — plus wall-clock durations for resumed spans (`ContinuousClock` instants don't survive reboot; `SpanEnded.duration` is already optional for this) and accepting that signpost intervals can't resume. (audit 2026-07-26) - feat(PeriscopeCore) [needs-design]: Never drop the pre-store-attach window — journal from process start. `PeriscopeStore.make` is `async` (`:84`), so events emitted between process launch and `add(sink:)` (`Periscope.swift:192`) — early launch steps, ambient start-up snapshots — reach neither the store nor today's journal (journaling only begins once an on-disk store is attached); they survive only in the in-memory recent buffer and OSLog, and are lost from the durable record. We must never drop or lose events. Fix: write to a **short-lived journal from app start, before the store is registered, reusing the JournalKit infra**; when the store attaches, ingest that bootstrap journal (dedupe by event ID like the crash-journal ingest) and delete it. Composes with — but is stronger than — a recent-buffer replay into a late-added sink (that only covers what's still buffered, not a slow/large pre-attach burst). Related: the "No eager store handle" P2 below. (pr#94 review) - docs(PeriscopeCore) [quick-win]: Note the pre-attach gap in `PeriscopeCore/README.md`'s crash-durability section until the above lands; the section currently reads as though the journal covers the whole process lifetime. (audit 2026-07-26) @@ -41,7 +41,7 @@ here. - feat(PeriscopeTools) [needs-design]: Inspect-by-object is scope-granular, not instance-granular. `.logInspectable(_:)` keys the badge/inspector to a `Log`'s *scope*, so tagging a list row (Where tags `EvidenceRow` with `WhereLog.evidence`, `LocationStatusRow` with `WhereLog.session`) surfaces the whole scope's recent events, not that one row's. Events already carry `externalID` for object correlation, but the inspector can't filter by it — a per-instance child scope (blocked on the `LogContextProviding` parent-hierarchy P0) or an `externalID`-scoped inspect entry would make true row-/object-level inspection work. (pr#94 review) - design(PeriscopeCore) [needs-design]: No eager store handle — `PeriscopeStore.make` being `async` forces an "optional store, observe until it lands" dance on consumers. Where exposes an `Optional` on `WhereModel` that stays `nil` until the bootstrap `Task` completes, and `RootView` has to watch the transition (`.onChange` of the store identity) to wire the viewer/inspector/alerter. A synchronous pending-store handle (usable immediately, resolves in the background) or an `await`-readiness accessor would remove the optional-and-observe boilerplate every app repeats. (agent) - test(PeriscopeTools) [quick-win]: Add the missing 1:1 tests for the small `*+Display` extensions. (audit 2026-07-26) -- test(PeriscopeTools) [needs-design]: broken-snapshots — replace the hosting smoke tests with image snapshots. Eighteen tests across nine files assert nothing but "the hosted view reached a window": `#expect(await waitUntil { host.view.window != nil })` in `LogEventListTests.swift:30`, `:41`, `LogHierarchyViewHostingTests.swift:23`, `:34`, `PeriscopeViewerHostingTests.swift:29`, `:42`, `ScopeEventsViewHostingTests.swift:25`, `:38`, `SpanHistoryViewHostingTests.swift:23`, `:34`, `SpanTreeViewHostingTests.swift:26`, `:37`, and the `try waitFor { host.view.window != nil }` spelling in `LogInspectableHostingTests.swift:25`, `:38`, `:50`, `LogTraceViewHostingTests.swift:23`, `OpenSpansViewHostingTests.swift:27`, `:38`. The predicate restates what `show`/`showHosted` already guarantee, so each test proves only that construction didn't crash — never what rendered, which is the part the elaborate seeding sets up (`LogHierarchyView`'s outline, the comfortable density `PeriscopeViewerHostingTests` injects, the "No Events" state `ScopeEventsViewHostingTests` documents at `:29`). The repo convention is now that an image bundle, not a hosting smoke test, owns "does this screen render" (see [`Where/WhereUI/AGENTS.md`](../../Where/WhereUI/AGENTS.md#testing) and the WhereUI suite that replaced its own smoke tests). Convert them to image snapshots over the same seeded stores, keeping any assertion that isn't the window check and deleting the files left empty. **The plumbing is already in place**: [`SnapshotTests/`](PeriscopeTools/SnapshotTests) exists and `PeriscopeViewerSnapshotTests` is the worked example — add a file per view beside it, and it compiles into the repo-wide `StuffSnapshotTests` bundle while recording references here. Do *not* add a `PeriscopeToolsSnapshotTests` bundle; there is exactly one image-snapshot bundle by design (root [`AGENTS.md`](../../AGENTS.md#targets)). The remaining work is per-view authoring, not wiring: each view needs a deterministic fixture (a frozen store, as `PeriscopeViewerSnapshotTests` does) and ideally a `SnapshotProviding` conformance in its own source file — which needs a `SnapshotKit` dependency on PeriscopeTools, since the module has no `#Preview`s at all today. `OpenSpansView` is the one view with a genuine determinism problem: its `TimelineView(.periodic(from: .now, by: 1))` ticking ages (`OpenSpansView.swift:19`) need the `\.isCapturingSnapshot` treatment. (Note the two `window != nil` checks in `Shared/LifecycleKit/Tests/` are *not* in scope: they assert the hosting helper's own lifecycle contract, which is the one place the check is the point.) (pr#101 review) +- test(PeriscopeTools) [needs-design]: broken-snapshots — replace the hosting smoke tests with image snapshots. Eighteen tests across nine files assert nothing but "the hosted view reached a window": `#expect(await waitUntil { host.view.window != nil })` in `LogEventListTests.swift:30`, `:41`, `LogHierarchyViewHostingTests.swift:23`, `:34`, `PeriscopeViewerHostingTests.swift:29`, `:42`, `ScopeEventsViewHostingTests.swift:25`, `:38`, `SpanHistoryViewHostingTests.swift:23`, `:34`, `SpanTreeViewHostingTests.swift:26`, `:37`, and the `try waitFor { host.view.window != nil }` spelling in `LogInspectableHostingTests.swift:25`, `:38`, `:50`, `LogTraceViewHostingTests.swift:23`, `OpenSpansViewHostingTests.swift:27`, `:38`. The predicate restates what `show`/`showHosted` already guarantee, so each test proves only that construction didn't crash — never what rendered, which is the part the elaborate seeding sets up (`LogHierarchyView`'s outline, the comfortable density `PeriscopeViewerHostingTests` injects, the "No Events" state `ScopeEventsViewHostingTests` documents at `:29`). The repo convention is now that an image bundle, not a hosting smoke test, owns "does this screen render" (see [`Where/WhereUI/AGENTS.md`](../../Where/WhereUI/AGENTS.md#testing) and the WhereUI suite that replaced its own smoke tests). Convert them to image snapshots over the same seeded stores, keeping any assertion that isn't the window check and deleting the files left empty. **The plumbing is already in place**: [`SnapshotTests/`](PeriscopeTools/SnapshotTests) exists and `PeriscopeViewerSnapshotTests` is the worked example — add a file per view beside it, and it compiles into the module's own `PeriscopeToolsSnapshotTests` bundle (one image bundle per module, gathered into the shared `StuffSnapshotTests` scheme — root [`AGENTS.md`](../../AGENTS.md#targets)) while recording references here. The remaining work is per-view authoring, not wiring: each view needs a deterministic fixture (a frozen store, as `PeriscopeViewerSnapshotTests` does) and ideally a `SnapshotProviding` conformance in its own source file — which needs a `SnapshotKit` dependency on PeriscopeTools, since the module has no `#Preview`s at all today. `OpenSpansView` is the one view with a genuine determinism problem: its `TimelineView(.periodic(from: .now, by: 1))` ticking ages (`OpenSpansView.swift:19`) need the `\.isCapturingSnapshot` treatment. (Note the two `window != nil` checks in `Shared/LifecycleKit/Tests/` are *not* in scope: they assert the hosting helper's own lifecycle contract, which is the one place the check is the point. SwiftDataInspector carries the same hosting-smoke debt — see [`Shared/SwiftDataInspector/TODOs.md`](../SwiftDataInspector/TODOs.md).) (pr#101 review) # Completed issues diff --git a/Shared/SwiftDataInspector/TODOs.md b/Shared/SwiftDataInspector/TODOs.md index abd5f0bd..20343430 100644 --- a/Shared/SwiftDataInspector/TODOs.md +++ b/Shared/SwiftDataInspector/TODOs.md @@ -9,6 +9,6 @@ here. ## P1s (Should do) - test [quick-win]: The bare-`PersistentIdentifier` branch of relationship resolution (`SwiftDataReflection.swift:132`) is untested — the existing relationship test faults a materialized model, which takes the other path. Add a fixture test over an unmaterialized slot. (audit 2026-07-26) - test [needs-design]: `Tests/SwiftDataInspectorTests.swift` is one 759-line file covering 13 sources, against the 1:1 convention. Split it by concern, optionally adding hosted UI smoke tests. (audit 2026-07-26) - - test [quick-win]: broken-snapshots — don't add those hosting smoke tests; add image snapshots instead. An image suite, not a "hosts without crashing" test, owns "does this screen render" — the latter asserts nothing about the result. [`SnapshotTests/`](SnapshotTests) already exists with `SwiftDataInspectorSnapshotTests` as the worked example (a local fixture schema, no design-system root); add a file per surface beside it. The root entity list is covered; the paged row table and the relationship drill-in are not. (pr#101 review) + - test [quick-win]: broken-snapshots — don't add those hosting smoke tests; add image snapshots instead. An image suite, not a "hosts without crashing" test, owns "does this screen render" — the latter asserts nothing about the result. [`SnapshotTests/`](SnapshotTests) already exists with `SwiftDataInspectorSnapshotTests` as the worked example (a local fixture schema, no design-system root); add a file per surface beside it. The root entity list is covered; the paged row table and the relationship drill-in are not. (Same debt as PeriscopeTools' hosting-smoke item — see [`Shared/Periscope/TODOs.md`](../Periscope/TODOs.md).) (pr#101 review) # Completed issues diff --git a/Where/TODOs.md b/Where/TODOs.md index 7022fe33..75148dad 100644 --- a/Where/TODOs.md +++ b/Where/TODOs.md @@ -19,31 +19,30 @@ The item format and the placement rule live in the root ## P0s (Must do) - fix(WhereCore) [needs-design]: `DailySummaryReconciler.reconcile()` is absent from the post-day-change fan-out. Its only caller is `configure` (`DailySummaryReconciler.swift:45`), and `DayJournal.reconcileAfterDayChange()` (`:63`) fans out to issue state and widgets only — so the daily notification body stays stale until a foreground re-`configure`. Add it to the fan-out (the GPS ingest hook, `reconcileAfterDayChange()`, backup `onImport`), or document the foreground-only policy. (audit 2026-07-26) - test(WhereCore) [quick-win]: Mutate data and assert the summary notification body updates without a re-`configure`. (audit 2026-07-26) -- test(WhereUI) [quick-win]: Remove the `waitForOneRunloop` calls added to UI tests; it's a flake paradise. Down to three call sites: `OnboardingTests.swift:43` and `LaunchSplashViewTests.swift:11`, `:19`. Replace with predicate polling (`waitFor` / `waitUntil` from `TestHostSupport`). (audit 2026-07-26) - perf(WhereCore) [needs-design]: Performance pass — how often is the app booting? Can we only do it on changes of, say, 1 km or more? (human) ## P1s (Should do) - fix(WhereUI) [quick-win]: `CalendarDay.displayDate` resolves through `Calendar.current` (`DateRangeFormatting.swift:33`), so every day label that flows through it — relabel, logged days, resolution details, the region drill-in — renders a wrong date on a non-Gregorian device: `startOfDay(in:)` interprets the day's Gregorian Y-M-D as *that* calendar's components, so a Buddhist-era device resolves 2026-07-26 to a date ~543 years off. `DateRangeFormatting.abbreviated` (`:6`, `:19`) and `PresenceTimeline.stints` (`PresenceTimeline.swift:37`) also *default* to `.current`, and `PresenceTimelineList` (`:12`) doesn't pass `report.calendar`. Take an explicit calendar (Gregorian + current time zone) in the helper and thread the report's calendar from the call sites. The `where.gregorian_calendar` Bumper rule that should catch this is blind to the implicit-member form — filed in the root [`TODOs.md`](../TODOs.md). (audit 2026-07-26) - fix(WhereCore) [needs-design]: `WhereServices.setPrimaryRegions(_:)` (`:285`) commits atomically but skips `DayJournal.reconcileAfterDayChange()` — widgets/reminders/summary don't refresh until foreground/configure. Route picker commits through the unified fan-out, or document the intentional deferral. (audit 2026-07-26) -- fix(WhereCore) [needs-design]: Soft-delete untracked regions. `SwiftDataStore.setTrackedRegion(false)` (`:773`) and `setPrimaryRegions` (`:838`) hard-delete the row, which drops the region from the attributor's load set — so re-aggregating a past year re-attributes that region's GPS days to `.other`. The `SwiftDataStore` TODO filed this as "when the region picker ships"; it has shipped, and both the onboarding picker and the Settings region editor now reach the delete, so this is user-reachable rather than latent. Retain the row for attribution and hide it from the pickers instead. (audit 2026-07-26) +- fix(WhereCore) [needs-design]: Soft-delete untracked regions. `SwiftDataStore.setTrackedRegion(false)` (`:756`, in-source TODO at `:773`) and `setPrimaryRegions` (`:833`) hard-delete the row, which drops the region from the attributor's load set — so re-aggregating a past year re-attributes that region's GPS days to `.other`. The `SwiftDataStore` TODO filed this as "when the region picker ships"; it has shipped, and both the onboarding picker and the Settings region editor now reach the delete, so this is user-reachable rather than latent. Retain the row for attribution and hide it from the pickers instead. (audit 2026-07-26) - fix(WhereCore) [needs-design]: `DayJournal.ingest(_:)` (`:70`), the bulk ingest (`:82`), and `addManualSample` (`:93`) publish widgets but skip the reminder/issue reconcile, so a presence change made through them leaves the badge and reminders stale. Route them through the fan-out, or mark them `@_spi(Testing)` if they aren't production write paths. (audit 2026-07-26) -- fix(WhereCore) [needs-design]: A durable outbox save failure is logged and swallowed (`LocationOutbox.swift:86`, `LocationIngestor.swift:343`), so a process death loses the in-memory sample with nothing to replay on relaunch. Handle the degraded state honestly rather than continuing as though the sample were durable. (audit 2026-07-26) +- fix(WhereCore) [needs-design]: A durable outbox save failure is logged and swallowed (`LocationOutbox.swift:86`, `LocationIngestor.swift:344`), so a process death loses the in-memory sample with nothing to replay on relaunch. Handle the degraded state honestly rather than continuing as though the sample were durable. (audit 2026-07-26) - test(WhereCore) [quick-win]: Cover outbox *save* failure with a failing-outbox double; only load failure is covered today. (audit 2026-07-26) -- fix(WhereCore) [needs-design]: The retry queue evicts FIFO at capacity and drops samples with a warning only (`LocationIngestor.swift:348`). Decide the capacity policy and whether eviction warrants user-visible degradation, then document it. (audit 2026-07-26) -- fix(WhereUI) [quick-win]: `PresenceTimelineList` returns `[]` whenever `report.report` is nil (`:10`), so the Timeline segment of Your Year renders the "no stays" empty state while the year is still loading (and during a year switch) — unlike the Calendar segment beside it, which gates on `loadState`. (audit 2026-07-26) - - refactor(WhereUI) [needs-design]: Extract a shared `ReportLoadGate`. The same `YearReportModel.loadState` gate is copy-pasted across `LocationsView.swift:59`, `ElsewhereView.swift:49`, `ResolutionView.swift:57`, and `CalendarContentView.swift:58`, and `PresenceTimelineList` skipped it entirely (above). One gate view would cover all five. (audit 2026-07-26) -- fix(WhereUI) [quick-win]: The Elsewhere entry card renders raw inflection markup instead of an agreed region count — it shows literally `^[3 region](inflect: true)`. `locations.elsewhere.subtitle` is authored for automatic grammar agreement (`^[%lld region](inflect: true)`), but the string-catalog compiler passes that markup through **verbatim** into the compiled `Localizable.strings` (unlike a real plural such as `primary.elsewhereOnly.description`, which compiles to an `NSStringLocalizedFormatKey` dict), and flattening the resource to a `String` never runs the inflection engine. Pre-existing — the catalog entry is byte-identical on `main` and predates the String Catalog symbol migration. Fix by either rendering the resource directly so SwiftUI applies inflection (`Text(.locationsElsewhereSubtitle(regionCount))` in `ElsewhereSummaryCard`, dropping the `WhereFormat` hop) or replacing the markup with an explicit plural variation. `WhereFormatTests.elsewhereCardSubtitleInflectsTheRegionCount` pins the expected output behind `withKnownIssue`, so it trips as soon as this is fixed. (agent) +- fix(WhereCore) [needs-design]: The retry queue evicts FIFO at capacity and drops samples with a warning only (`LocationIngestor.swift:349`). Decide the capacity policy and whether eviction warrants user-visible degradation, then document it. (audit 2026-07-26) +- fix(WhereUI) [quick-win]: `PresenceTimelineList` returns `[]` whenever `report.report` is nil (`:12`), so the Timeline segment of Your Year renders the "no stays" empty state while the year is still loading (and during a year switch) — unlike the Calendar segment beside it, which gates on `loadState`. (audit 2026-07-26) + - refactor(WhereUI) [needs-design]: Extract a shared `ReportLoadGate`. The same `YearReportModel.loadState` gate is copy-pasted across `LocationsView.swift:60`, `ElsewhereView.swift:50`, `ResolutionView.swift:58`, and `CalendarContentView.swift:60`, and `PresenceTimelineList` skipped it entirely (above). One gate view would cover all five. (audit 2026-07-26) +- fix(WhereUI) [quick-win]: The Elsewhere entry card renders raw inflection markup instead of an agreed region count — it shows literally `^[3 region](inflect: true)`. `locations.elsewhere.subtitle` is authored for automatic grammar agreement (`^[%lld region](inflect: true)`), but the string-catalog compiler passes that markup through **verbatim** into the compiled `Localizable.strings` (unlike a real plural such as `primary.elsewhereOnly.description`, which compiles to an `NSStringLocalizedFormatKey` dict), and flattening the resource to a `String` never runs the inflection engine. Pre-existing — the catalog entry is byte-identical on `main` and predates the String Catalog symbol migration. Fix by either rendering the resource directly so SwiftUI applies inflection (`Text(.locationsElsewhereSubtitle(regionCount))` in `ElsewhereSummaryCard`, dropping the `WhereFormat` hop) or replacing the markup with an explicit plural variation. `WhereFormatTests.elsewhereCardSubtitleInflectsTheRegionCount` pins the expected output behind `withKnownIssue`, so it trips as soon as this is fixed. The bug is also baked into the `locations.Loaded_iPad.png` reference (ledgered in the broken-snapshots cluster below) — re-record that image when this lands. (agent) - fix(WhereUI) [needs-design]: Serialize `WhereSession.trackingEnabled` mutations. The setter spawns an unserialized `Task` per assignment (`WhereSession.swift:441`), so rapid on/off leaves start/stop racing: `startTracking()` sets `wantsTracking = true` on entry and never re-reads intent before `reconcileTracking()`, so a `stopTracking()` that runs mid-flight gets undone. Coalesce behind one in-flight task (or a generation token) and re-check intent before reconciling. (audit 2026-07-26) - fix(WhereUI) [needs-design]: Split the toggle binding — `wantsTracking` for user intent vs `isTracking` for effective GPS state. `wantsTracking` already exists internally and is persisted, but the public `trackingEnabled` binds effective state for both read *and* write, so the switch animates back on its own while a start is in flight. (audit 2026-07-26) - test(WhereUI) [quick-win]: Add an adversarial test for toggle ordering (stop while a slow scripted `startTracking()` is in flight); `WhereSessionTrackingTests` covers only the launch/foreground paths today. (audit 2026-07-26) - refactor(WhereUI) [needs-design]: Split `WhereSession` into an always-on coordinator + a presentation view-model whose lifetime scopes its subscriptions. **Partial progress (July 2026):** `YearReportModel` is now scene-scoped in `MainTabs` — `activate()` / `deactivate()` on `scenePhase` drive `observeDataChanges()` and refresh, closing the headless-relaunch rescan leak that previously wired the subscription through launch `syncAuth`. `ResolveModel`, `BackupModel`, and `RemindersSettingsModel` are already view-scoped. Remaining: the coordinator is still ~460 lines mixing tracking intent, authorization, reset, and region-style mirrors; finish extracting presentation collaborators and drive any leftover reactive work from scene lifetime. (agent) - test(WhereUI) [quick-win]: `ManualDayView`'s range mode has no test coverage — including its capture-only code. The deleted `manualDayViewHostsAddModes` hosted a *range-prefilled* add (two `DatePicker`s), but the `addPrefill` snapshot case in `ManualDayView.swift` is a single day (`start == end` → `dateSpan = .singleDay`), so no test ever renders the `.range` branch — live or stand-in. The range stand-in code has never executed, and the From/Through picker row rendering is unpinned. Fix: add an `AddRange` snapshot case with a multi-day `MissingDayRange` prefill (the Resolve backfill flow the deleted test existed for). (From the July 2026 snapshot-testing PR review.) - test(WhereUI) [needs-design]: `RegionMapView`'s live `Map` branch is no longer constructed by any test. The deleted `regionMapViewHosts` mounted the real MapKit `Map` (polygon building via `clLocationCoordinates`, `mapStyle`); under capture the view always takes the `SnapshotMapStandIn` branch, so a crash or regression in the production map path — which every real user sees — would ship untested. The stand-in substitution is what the framework carve-out sanctions; the gap is purely coverage. Fix: keep one lightweight hosting test for the live branch in `WhereUITests` (this specific surface is the exception the "no hosting smoke tests" rule shouldn't swallow) — until then, this entry records the accepted gap. (From the July 2026 snapshot-testing PR review.) -- refactor(WhereUI) [needs-design]: Extract one shared region-selection form. `DayRelabelView.swift:106` renders a flat region list where `ManualDayView.swift:212` has grouped sections plus `loadGrouping()`, so the two screens disagree on how regions are picked. (audit 2026-07-26) +- refactor(WhereUI) [needs-design]: Extract one shared region-selection form. `DayRelabelView.swift:108` renders a flat region list where `ManualDayView.swift:218` has grouped sections plus `loadGrouping()` (`:255`), so the two screens disagree on how regions are picked. (audit 2026-07-26) - fix(WhereUI) [needs-design]: Notification authorization is requested during launch, with no context and unprompted. The launch's detached fan (`WhereLaunch`'s `reminders` / `summary` / `issue-alerts` steps → `session.applyReminderConfiguration()` etc. → the `services.reminders` scheduler) reaches the notification center while the app is still launching, so a fresh install shows the system "Where Would Like to Send You Notifications" alert over the splash — before the user has expressed any interest in reminders and with no in-app rationale. Observed on a fresh-install simulator screen recording: the alert lands roughly a second after the splash appears and then sits on top of the revealed app. Ask in context instead — request when the user turns reminders/summary on in Settings (or immediately after onboarding, with a sentence of explanation) — and have the launch fan only *reconcile* schedules against authorization that was already granted, never trigger the prompt. (agent) - refactor(WhereCore) [needs-design]: Rewrite the controller layer as a state machine so invariants can't exist. (human) - test(WhereIntents) [quick-win]: The per-intent `perform()` glue — guards, snippet wiring, error→dialog mapping — is untested, because `@Dependency` traps outside the perform flow. Either extract a thin testable seam or say so in `README.md`; the reader/writer seams themselves are now well covered. (audit 2026-07-26) -- refactor(WhereShareExtension) [needs-design]: Consolidate the share/add evidence form. `ShareEvidenceView.swift:65` and `AddEvidenceView.swift:85` are parallel implementations over parallel catalog namespaces (`share.form.*` / `evidence.form.*`). (audit 2026-07-26) +- refactor(WhereShareExtension) [needs-design]: Consolidate the share/add evidence form. `ShareEvidenceView.swift:68` and `AddEvidenceView.swift:37` are parallel implementations over parallel catalog namespaces (`share.form.*` / `evidence.form.*`). (audit 2026-07-26) - perf(WhereCore) [needs-design]: Consider incremental year-report reads or memoization for the widget/reminder/summary hot paths — `ReportReader.yearReport:27` and `WidgetDataReader.snapshot:85` re-aggregate a full year each time. (audit 2026-07-26) - test(WhereCore) [quick-win]: Add `WherePreferencesTests` over `InMemoryKeyValueStore`. (audit 2026-07-26) - refactor(WhereUI): What's with all the `.accessibilityIdentifier(…)` modifiers, do we need them? (human) @@ -55,38 +54,37 @@ The item format and the placement rule live in the root - design(WhereCore): Open question — the exclusion UX, where an older device progressively hides days a newer device has touched, needs a deliberate warning surface, not a silent drop. (agent) - fix(WhereUI) [needs-design]: broken-snapshots — the snapshot suite pinned genuinely broken renderings as references, flagged with `[Fix later]` review comments on PR #101 and merged anyway to land the suite. These are not flaky captures (those have their own ledger below) — each is a faithful, reproducible image of something actually wrong, so re-recording is never the fix. Fix the view, the capture frame, or the pipeline as each item says, then re-record just that reference under `Where/WhereUI/SnapshotTests/__Snapshots__/`. Most cluster on the accessibility axes `.screenDefaults` added — the ax5 Dynamic Type and VoiceOver-annotated configurations that nothing rendered before this suite existed. (pr#101 review) - fix(WhereUI) [needs-design]: broken-snapshots: the VoiceOver-annotated calendar captures are blank. `CalendarContentViewSnapshotTests/calendarContent.WithData_iPhone_accessibility.png` (66 KB) and `..._iPad_accessibility.png` (171 KB) are solid white inside their border, against 205 KB–2.2 MB for every other screen-sized `_accessibility` reference — so it's specific to `CalendarContentView`, not the annotation pipeline. `AccessibilitySnapshotViewController` renders the wrapper with `viewRenderingMode: .drawHierarchyInRect` (`Shared/SnapshotKitTesting/Sources/AccessibilitySnapshotViewController.swift:36`), and `parseAccessibility()` claims failures "surface loudly rather than producing a blank image" (`:44`) — whatever this is slips past every `ImageRenderingError` guard there. Two configurations' worth of accessibility coverage currently assert a blank image, so a real regression in them can't fail. (pr#101 review) - - fix(WhereUI) [quick-win]: broken-snapshots: the calendar day grid breaks at accessibility Dynamic Type. Every two-digit date truncates to its first digit — the 10th–31st render as "1", "2", or "3" — because the day number is clamped to a fixed square (`CalendarContentView.swift:436`, `.frame(width: calendar.day.numberSize, height:)`), and the weekday header row wraps mid-word ("Sun" over two lines, "Wed" over three) because each symbol is a plain `Text` in an equal-width grid column (`:282`). Both show in `calendarContent.WithData_iPhone_ax5.png`; the digit truncation also hits `..._iPad_ax5.png`, where the extra width goes to inter-column gaps instead of the numbers. Showing "1" where the date is 10 is wrong content, not merely tight layout. (pr#101 review) + - fix(WhereUI) [quick-win]: broken-snapshots: the calendar day grid breaks at accessibility Dynamic Type. Every two-digit date truncates to its first digit — the 10th–31st render as "1", "2", or "3" — because the day number is clamped to a fixed square (`CalendarContentView.swift:440`, `.frame(width: calendar.day.numberSize, height:)`), and the weekday header row wraps mid-word ("Sun" over two lines, "Wed" over three) because each symbol is a plain `Text` in an equal-width grid column (`:282`). Both show in `calendarContent.WithData_iPhone_ax5.png`; the digit truncation also hits `..._iPad_ax5.png`, where the extra width goes to inter-column gaps instead of the numbers. Showing "1" where the date is 10 is wrong content, not merely tight layout. (pr#101 review) - fix(WhereUI) [quick-win]: broken-snapshots: the presence timeline's leading accent bar doesn't scale with Dynamic Type. `StintRow` sizes its `Capsule` from the fixed stylesheet tokens `timeline.accentWidth`/`accentHeight` — 4×34 (`Sources/Shared/WhereStylesheet.swift:719`, `:720`) applied at `PresenceTimelineList.swift:54`–`:57` — so at ax5 it stays a 4pt stub beside ~40pt text (`presenceTimeline.WithData_iPhone_ax5.png`, `..._iPad_ax5.png`). Scale it with `@ScaledMetric`, or derive it in `WhereStylesheet.init(context:)` the way the day-grid tap targets already do. (pr#101 review) - fix(WhereUI) [needs-design]: broken-snapshots: the presence timeline row squishes horizontally at ax5 instead of restacking. `StintRow` keeps accent bar, emoji, name/date stack, and day count on one `HStack` line at every type size (`PresenceTimelineList.swift:51`), so `presenceTimeline.WithData_iPhone_ax5.png` renders "California" hyphenated over three lines beside a two-line "148 days". Switch to a `ViewThatFits` / `AnyLayout` that stacks vertically at accessibility sizes. (pr#101 review) - fix(WhereUI) [needs-design]: broken-snapshots: `YearView` overflows horizontally at ax5. In `year.Loaded_iPhone_ax5.png` the month title reads "nuary", the day grid is clipped on both edges, and the Calendar/Timeline pill runs off the trailing edge. The suspect is `YearModePicker`, a fixed-width `HStack` of two labelled segments pinned as a bottom `safeAreaInset` (`YearView.swift:36`–`:38`, `:79`), which is wider than the screen at ax5. Confirm the oversized inset is what widens the layout beneath it, then make the picker fit at accessibility sizes (icon-only, wrapped, or scrollable). (pr#101 review) - - fix(WhereUI) [quick-win]: broken-snapshots: the Resolve toolbar badge sits awkwardly on the iOS 26 glass toolbar button. `ResolveToolbarLabel` hand-rolls the badge as a red `Capsule` overlaid on the `checklist` symbol and pushes it out with a fixed `.offset(x: spacing.small, y: -spacing.small)` (`LocationsView.swift:210`–`:217`), landing it half outside the button's own glass capsule — visible in `root.LoggedIn_iPhone.png`. Use SwiftUI's `.badge()` on the toolbar item, or offset against the resolved chrome rather than a fixed spacing token. (pr#101 review) - - test(WhereUI) [quick-win]: broken-snapshots: the Resolve sheet's ax5 reference is cut off mid-content. `resolution.WithIssues_iPhone_ax5.png` slices the second issue card's subtitle at the frame's bottom edge, because the case captures at the fixed `.iPhone` device frame via `.screenDefaults` (`ResolutionView.swift:221`), which clips rather than growing. `SnapshotConfiguration.Frame.fullContent` exists for exactly this (see `CalendarContentView.swift:589`) — add a full-content ax5 case so the whole sheet is pinned instead of its first screenful. (pr#101 review) + - fix(WhereUI) [quick-win]: broken-snapshots: the Resolve toolbar badge sits awkwardly on the iOS 26 glass toolbar button. `ResolveToolbarLabel` hand-rolls the badge as a red `Capsule` overlaid on the `checklist` symbol and pushes it out with a fixed `.offset(x: spacing.small, y: -spacing.small)` (`LocationsView.swift:203`–`:217`), landing it half outside the button's own glass capsule — visible in `root.LoggedIn_iPhone.png`. Use SwiftUI's `.badge()` on the toolbar item, or offset against the resolved chrome rather than a fixed spacing token. (pr#101 review) + - test(WhereUI) [quick-win]: broken-snapshots: the Resolve sheet's ax5 reference is cut off mid-content. `resolution.WithIssues_iPhone_ax5.png` slices the second issue card's subtitle at the frame's bottom edge, because the case captures at the fixed `.iPhone` device frame via `.screenDefaults` (`ResolutionView.swift:221`), which clips rather than growing. `SnapshotConfiguration.Frame.fullContent` exists for exactly this (see `CalendarContentView.swift:592`) — add a full-content ax5 case so the whole sheet is pinned instead of its first screenful. (pr#101 review) - fix(WhereUI): broken-snapshots: `locations.Loaded_iPad.png` bakes in raw inflection markup — the Elsewhere card's subtitle renders literally as `^[3 region](inflect: true)`. This is the `locations.elsewhere.subtitle` P1 filed above, now pinned as a reference; recorded here so the image isn't mistaken for correct output, and so that reference is re-recorded when the fix lands. (pr#101 review) ## P2s (Nice to have) - feat(WhereUI) [needs-design]: Give the app a branded launch screen. `UILaunchScreen` is an empty dictionary (`Project.swift`), so the pre-main frame is plain white. Measured from a fresh-install simulator recording, a first run reads as ~1.7s of white → ~0.25s of the dark `LaunchSplashView` → the light onboarding screen, so the splash registers as a quarter-second dark blip between two light screens rather than as the app opening. A launch screen matching the splash's background + icon would make that continuous. Note this is the right layer to fix it at: the splash's own `minimumSplashDuration` hold deliberately gates only the `.ready` reveal, not a gate transition like onboarding, so lengthening the hold would just delay interactive UI. (agent) -- fix(WhereUI) [needs-design]: A launch can park indefinitely behind a system permission prompt. If location authorization is still `.notDetermined` when the launch trunk reaches `reconcile-tracking`, `WhereSession.reconcileTracking()` awaits the authorization *request*, so the splash stays up until the user answers the system alert — and if the unprompted notification alert (filed as a P1 above) is queued ahead of it, until they answer both. Not reachable on the normal path (onboarding settles location permission before the trunk resumes), but reproducible by marking `where.hasOnboarded` true without granting location, and it means a modal system prompt sits on the launch critical path. Consider reconciling against the *current* status only and letting the live authorization observer pick up the user's answer, so launch can never block on a prompt. (agent) - feat(WhereUI) [quick-win]: Bound the Periscope log store by size, not just age. `WhereLaunch.bootstrapLogging` prunes events older than the retention window (now 100 days); with the built-in ambient sources emitting continuously, a heavy-logging device could still grow the store large within the window. Add a count cap alongside the time prune — `PeriscopeStore.pruneEvents(keepingNewest:)` already exists — so the store is bounded regardless of volume. (pr#94 review) - refactor(WhereUI) [needs-design]: Make the scene-scoped model wiring compiler-checked rather than an `@Environment` lookup that fails silently. `WhereSession` (the always-on coordinator) is read from the environment, so a screen mounted without a parent injecting it resolves to a runtime fallback/precondition instead of a compile error. The scoped models (`YearReportModel`, `ResolveModel`, `BackupModel`, `RemindersSettingsModel`) are already constructor-injected; explore threading the coordinator the same way (or a non-defaulting typed `EnvironmentKey`) so a broken wiring can't build. Follow-up from the `WhereSession` split. (agent) - refactor(WhereUI) [needs-design]: Split `YearReportModel` further. Post-split it still fuses several roles for the selected year: the loaded report + everything derived from it (ranking, missing days, calendar inputs, tracked-day count), the Resolve badge *count*, the day-write intents (`setManualDay(s)`, `overrideDay`, `clearManualDay`, `clearSelectedYear`), and the Elsewhere drill-in reads (`days(in:)`, `locations(in:)`, `representativeCoordinates()`). The read-only presentation state and the write-intent/drill-in surface could be separate collaborators so a view only holds what it uses. Follow-up from the `WhereSession` split. (agent) - refactor(WhereCore) [needs-design]: Move `RegionDays` / `RegionRanking` down from `WhereUI` into `WhereCore` so `DataIssueScanner` can derive primary regions itself instead of `WhereSession` passing `primaryRegions` in. Reverses the current "ranking is a presentation concept" placement; check the widget/UI call sites still compile. (agent) - feat(WhereUI) [needs-design]: Animate the Locations ranking reorder. `RegionSummaryCard` now morphs its day count when it changes on screen (`CardStyles.DayCountStyle`), but a change that also flips the two primary regions' order still snaps the cards into their new positions — an animated count landing in a hard-cut reorder. Needs an `.animation(_:value:)` keyed on the ranking around the `ForEach` in `LocationsView`, checked against the card's `matchedTransitionSource` zoom so the two don't fight. (agent) - fix(WhereCore) [quick-win]: Two Core failure paths report a benign-looking default instead of an honest one. `BackupService.importBackup` logs and continues when an evidence asset's bytes can't be read, importing metadata-only evidence with `blob: nil` (`BackupService.swift:188`) — a partial import that reads as a complete one; and `ReminderReconciler` contributes `0` to the badge when the issue scan throws (`ReminderReconciler.swift:193`), which is indistinguishable from "no issues to resolve". Surface partial-import state, and preserve the last good badge count (or a scan-failed state). (audit 2026-07-26) -- fix(WhereUI) [needs-design]: Make `LocationNamer` cancellation-aware. `ElsewhereView.loadPlaceNames()` (`:33`) has `.task(id:)` plus a post-await `Task.isCancelled` guard, but the namer itself (`LocationNamer.swift:64`) keeps geocoding after the year changes; `RegionDaysView`'s `DayRow` (`:125`) also fires an uncapped `.task` per row, so a long day list can spawn N concurrent reverse-geocode requests (the cache only helps duplicates). Add cancellation to `name(for:)` and batch the unique coordinates on the parent view. (audit 2026-07-26) -- localization(WhereUI) [quick-win]: `IntentSnippets` composes its production caption from hardcoded English (`" in "` / `" · "`, `IntentSnippets.swift:63`) rather than a catalog key with placeholders. The `#Preview` in the same file (`:190`) also hardcodes `Button("Log today here")` when `snippet.logTodayHere` already exists. (audit 2026-07-26) +- fix(WhereUI) [needs-design]: Make `LocationNamer` cancellation-aware. `ElsewhereView.loadPlaceNames()` (`:34`) has `.task(id:)` plus a post-await `Task.isCancelled` guard, but the namer itself (`LocationNamer.swift:64`) keeps geocoding after the year changes; `RegionDaysView`'s `DayRow` (`:126`) also fires an uncapped `.task` per row, so a long day list can spawn N concurrent reverse-geocode requests (the cache only helps duplicates). Add cancellation to `name(for:)` and batch the unique coordinates on the parent view. (audit 2026-07-26) +- localization(WhereUI) [quick-win]: `IntentSnippets` composes its production caption from hardcoded English (`" in "` / `" · "`, `IntentSnippets.swift:66`) rather than a catalog key with placeholders. The `#Preview` in the same file (`:190`) also hardcodes `Button("Log today here")` when `snippet.logTodayHere` already exists. (audit 2026-07-26) - fix(WhereUI) [quick-win]: Log the failures currently collapsed into an empty result — `AppIconCatalog.load()` (`AppIconModel.swift:73`, `AppIconOption.swift:89`) renders an empty icon picker on failure, `LocationNamer.swift:86` swallows a geocode error, and `WidgetSnapshotStore.read()` (`WhereCore`, `:62`) can't distinguish a decode failure from "never written". Keep the honest empty state; add the `warning`. (audit 2026-07-26) -- fix(WhereCore) [quick-win]: Surface the `applicationSupport()` → `NoOpLocationOutbox` fallback (`LocationOutbox.swift:52`); it silently disables cross-launch retry durability. Either report it to the launch wiring or treat it as a programmer error. (audit 2026-07-26) -- test(WhereUI) [quick-win]: Close the two extension-shaped test gaps that don't need a new bundle pattern decision — `ShareEvidenceModel.buildPendingEvidence()` (exposed for testing, nothing tests it) and `WhereWidgetProvider`'s midnight reload policy (`WhereWidgetProvider.swift:36`, the extension's core scheduling logic, untested in any target). (audit 2026-07-26) -- test(RegionKit) [quick-win]: Add `GeoJSONTests.swift`. The unsupported-geometry throw (`GeoJSON.swift:62`) and malformed-coordinate drop (`:124`) are untested, and `RegionCatalog.loadFromBundle()`'s degrade-to-empty-catalog behavior (`RegionCatalog.swift:98`) is asserted only at the log-event level in `RegionLogTests` rather than at runtime. (audit 2026-07-26) +- fix(WhereCore) [quick-win]: Surface the `applicationSupport()` → `NoOpLocationOutbox` fallback (`LocationOutbox.swift:59`); it silently disables cross-launch retry durability. Either report it to the launch wiring or treat it as a programmer error. (audit 2026-07-26) +- test(WhereShareExtension, WhereWidgets) [quick-win]: Close the two extension-shaped test gaps that don't need a new bundle pattern decision — `ShareEvidenceModel.buildPendingEvidence()` (`ShareEvidenceModel.swift:126`, exposed for testing, nothing tests it) and `WhereWidgetProvider`'s midnight reload policy (`WhereWidgetProvider.swift:36`, the extension's core scheduling logic, untested in any target). (audit 2026-07-26) +- test(RegionKit) [quick-win]: Add `GeoJSONTests.swift`. The unsupported-geometry throw (`GeoJSON.swift:62`) and malformed-coordinate drop (`:124`) are untested, and `RegionCatalog.loadFromBundle()`'s degrade-to-empty-catalog behavior (`RegionCatalog.swift:97`) is asserted only at the log-event level in `RegionLogTests` rather than at runtime. (audit 2026-07-26) - test(WhereUI) [quick-win]: Add the missing namesake tests for `LocationNamer` (cache / coalescing) and `CalendarContentView`'s `scrolledForYear` scroll-reveal gate, which has hosting smoke only. (audit 2026-07-26) - test(WhereIntents) [quick-win]: Test `RegionSpotlightIndexer` and `WhereIntentReader.recentActivity`. (audit 2026-07-26) - test(WhereCore) [needs-design]: Close the namesake-test debt — 28 of the implementation files have no `*Tests.swift` (notably `FoundationModelSummaryGenerator`, `WherePreferences`, `WidgetTimelineRefresher`, `BackupArchive`), and `WhereCoreTests.swift` is an omnibus still holding `YearReportTests`. Split by concern as those files change rather than in one pass. (audit 2026-07-26) -- docs(WhereCore) [quick-win]: Refresh stale doc claims — `RootView.swift:10` still describes four top-level screens; `WhereCore/README.md:48`, `:161` claim every write reconciles and errors are never swallowed; `LocationIngestor.swift:334` says it logs through `os.Logger` when it emits typed `WhereLog` events; `RegionViewer/README.md:14` describes a monolithic `us-states.geojson` and a hand-listed region set. `RegionKit/README.md:144` also claims GeoJSON decoding is covered, which the test item above would make true instead. (The `WhereShareExtension/AGENTS.md:21` compose-model credit, originally part of this item, was fixed 2026-07-27.) (audit 2026-07-26) +- docs(WhereCore) [quick-win]: Refresh stale doc claims — `WhereCore/README.md:48`, `:161` claim every write reconciles and errors are never swallowed; `LocationIngestor.swift:334` says it logs through `os.Logger` when it emits typed `WhereLog` events; `RegionViewer/README.md:14` describes a monolithic `us-states.geojson` and a hand-listed region set. `RegionKit/README.md:144` also claims GeoJSON decoding is covered, which the test item above would make true instead. (Two halves already closed: the `RootView.swift` "four screens" doc now reads three tabs, and the `WhereShareExtension/AGENTS.md:21` compose-model credit was fixed 2026-07-27.) (audit 2026-07-26) - refactor(WhereCore) [quick-win]: Drop the remaining Core-API parameter defaults — `DayJournal.addEvidence(_:blob:)` (`:216`), `WherePreferences.init(store:)` (`:14`), `SwiftDataStore.make(storage:)` (`:179`), and `WidgetDataReader`'s aggregator/attributor (`:74`). The composition root already knows each value. (audit 2026-07-26) - convention(WhereIntents) [quick-win]: Small polish — register `LogTripIntent` in `WhereShortcuts` (`:11` registers five) or document Shortcuts-only discovery; use `Calendar.whereIntents` for `LogDayIntent`'s default day instead of `Date()` (`:39`, no data impact today since `DayJournal` buckets Gregorian); log the App Group open failure behind `WhereIntentReader.todaySnapshot`'s `try?` (`:17`); and wrap `RegionViewer`'s `RegionMapView` in `.whereBroadwayRoot()` (`RegionViewerApp.swift:15`) so the dev tool renders with app styling. (audit 2026-07-26) - convention(RegionKit) [quick-win]: Reference a generated catalog symbol for `region.other` instead of the raw `String(localized:)` key (`RegionCatalog.swift:65`). (audit 2026-07-26) - convention(WhereShareExtension) [quick-win]: Log the discarded provider `error` in `SharedItemLoader.loadDataRepresentation` (`:80`); only the nil-data branch logs today. (audit 2026-07-26) -- perf(WhereUI) [needs-design]: Profile the `RegionSummaryCard` Canvas rosette — `ringCount` derives from size with no cap (`:98`). Cap or pre-render if it shows up. (audit 2026-07-26) +- perf(WhereUI) [needs-design]: Profile the `RegionSummaryCard` Canvas rosette — `ringCount` derives from size with no cap (`:106`). Cap or pre-render if it shows up. (audit 2026-07-26) - fix(WhereUI) [quick-win]: Three literals in source get auto-extracted into the catalogs as value-less entries, which is why an IDE build had anything to write back at all (see the serialization normalization PR). They're committed as Xcode writes them; removing an entry for good means removing the literal. `Marker("", coordinate:)` in `RecordedPointsMap` produces the empty `""` key (an unlabeled dev-map pin — `Annotation` with an explicit accessibility label would say what it means); `Text("\(group.outlineCount)")` in `RegionMapView` and `Text("\(day.dayOfMonth)")` in `CalendarContentView` produce `%lld` and bypass `WhereFormat`'s number styling. (A fourth such entry, `App content`, came from a `LifecycleContainer` `#Preview`; it's gone — that preview now uses `Text(verbatim:)`, which isn't extracted.) (agent) - feat(WhereUI): Raw data browser (similar to the SwiftData browser). (human) - docs(WhereUI): Add comments to strings in the xcstrings files. (human) @@ -133,6 +131,7 @@ re-recording: - Remove `caption(forRank rank: Int) -> String?`, I don’t want the caption ## P2s (Nice to have) +- fix(WhereUI) [needs-design]: A launch can park indefinitely behind a system permission prompt — claimed `WhereSession.reconcileTracking()` awaits the location-authorization *request*, holding the splash until the user answers the system alert. (Invalidated 2026-07-27: the claim doesn't hold. `reconcileTracking()` (`WhereSession.swift:282–293`) only reads the current `authorizationStatus` and starts/stops the ingestor — it never requests authorization — and `syncAuthorization()` is documented not to prompt. The launch trunk therefore cannot block on the location alert. The unprompted *notification* request at launch is real and remains filed as a P1 above.) (agent) - The `guard let controller else { return }` in the WhereModel in WhereUI is weird — gone with the controller: `WhereModel`'s guards now read as ownership invariants (`attach(services:)` is idempotent, `startSession()` requires attached services), and the type itself is `WhereServices`. - Move test only code behind @_spi — done throughout; test seams are `@_spi(Testing)` in both Where and the shared modules (direct store mutation, failure injection, queue introspection, clock/capacity overrides), and no `…ForTesting`-named API remains (`LifecycleRunner.injectFailureForTesting`, the last one, went with the engine's retry removal). - Move `let calendar = Calendar.current` into a var on the controller? There’s a few of these — the call sites moved onto explicit Gregorian calendars owned by `YearReportModel` / `WhereModel`. What's left isn't a controller-hoisting problem but three helper-level defaults, refiled as a P1 above (`CalendarDay.displayDate`, `DateRangeFormatting.abbreviated`, `PresenceTimeline.stints`). From 558567f57745ac2da5fc74a85e9d69c031dbc838 Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Mon, 27 Jul 2026 01:45:57 -0700 Subject: [PATCH 5/8] Codify the what-not-why rule format for agent docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plan step: Codify the what-not-why rule format in the root AGENTS.md Per-module docs section. Adds the editorial standard the rest of this branch applies: a rule is an imperative sentence, at most one clause of consequence, and a pointer to the proof (guard test, PR/commit — squash merges keep PR bodies in git log — or a TODOs.md entry). Tripwires, diagnostic signatures, and decision rules stay at one line each; history narration, mechanism walkthroughs, and persuasion move to the PR that proved them. Pure groundwork: no behavior change; the shortening commits that follow execute against this standard. --- AGENTS.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 2fb2377e..2da6959e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -291,6 +291,16 @@ collaborator on a service, a pinned tool version. Name the one or two worth learning from and say where the live list is. An exhaustive copy reads authoritative long after it stops being true, which is worse than no list. +**Rules state what, not why.** A rule is an imperative sentence, at most one +clause of consequence (only when the rule would otherwise look wrong enough to +"fix"), and a pointer to the proof — the guard test, the PR number or commit +SHA (squash merges keep PR bodies reachable via `git log`), or a `TODOs.md` +entry. Keep, at one line each: **tripwires** (conditions that invalidate a +rule — "re-measure if X"), **diagnostic signatures** (the literal error text +of a failure mode), and **decision rules**. History narration, mechanism +walkthroughs, and persuasion belong in the PR that proved them — point, don't +restate. + ## Repo-level docs A few files outside the module pair carry *state* rather than rules: From d429c3229669e5e9e74f9688c74ef00cc12a581c Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Mon, 27 Jul 2026 01:52:21 -0700 Subject: [PATCH 6/8] Rewrite the Where-side AGENTS.md files to what-not-why Plan step: Rewrite Where-side AGENTS.md files to what-not-why. Fact-neutral rewrite per the codified policy: every rule, tripwire, and diagnostic signature survives; mechanism essays, history narration, and persuasion move to their proof (PR bodies reachable via git log, guard tests, TODOs.md entries). - Where/AGENTS.md 303 -> 189 lines: layering/logging rules compressed to imperatives + consequences; the About-screen design narrative reduced to its rules with a pointer to PR #140; localization and dates keep every rule, lose the rationale; install section points at --help. - WhereUI/AGENTS.md 175 -> 93: the design-system tutorial (using/adding tokens, RegionStyle resolver wiring, trait derivations) moved into README.md's Design system section, which previously pointed here for it; AGENTS.md keeps the rules and the capture-determinism seams (MotionIsStatic, WhereDatePicker). - WhereCore/AGENTS.md 174 -> 110: collaborator catalogs become pointers; the CalendarDay invariant keeps both halves (identity rule + the GPS-rebucketing scope boundary) as short rules. - WhereIntents/AGENTS.md 105 -> 80: the IntentServices handoff keeps its decision rules (parks, no self-open fallback, re-fired on relaunch); the perform()-untestable section keeps both measured traps. - Where/Where/AGENTS.md: attribution bullet tightened. - RegionKit and WhereWidgets already met the standard; untouched. Cross-file anchors into the edited files verified resolving (#layering, #testing, #swiftui-views--previews, #installing-to-a-device, #design-system--wherestylesheet). --- Where/AGENTS.md | 358 ++++++++++++----------------------- Where/Where/AGENTS.md | 18 +- Where/WhereCore/AGENTS.md | 245 +++++++++--------------- Where/WhereIntents/AGENTS.md | 133 ++++++------- Where/WhereUI/AGENTS.md | 222 +++++++--------------- Where/WhereUI/README.md | 64 ++++++- 6 files changed, 399 insertions(+), 641 deletions(-) diff --git a/Where/AGENTS.md b/Where/AGENTS.md index 0b28ec60..eceff53c 100644 --- a/Where/AGENTS.md +++ b/Where/AGENTS.md @@ -4,289 +4,173 @@ Where is an iOS/iPadOS app for answering "what region was I in on which day?" It ingests passive GPS (Visits + significant-change), accepts user-asserted history (manual coordinates, whole-day overlays, evidence like boarding passes), and rolls everything up into per-day region presence and -per-year reports. The primary use case is residency / day-count audits, so a -day "counts" for a region if **any** sample in that calendar day fell inside -the region's polygon (a single day can belong to multiple regions, e.g. a -CA→NY same-day flight). +per-year reports. A day "counts" for a region if **any** sample in that +calendar day fell inside the region's polygon, so a single day can belong to +multiple regions. This file complements the root [`AGENTS.md`](../AGENTS.md), which owns build system, formatting, and global conventions. Read that first. ## Modules -Each module's own `AGENTS.md` / `README.md` is the authority on what it is. The layering stack, bottom-up: **RegionKit** (geometry + region lookup) → **WhereCore** (domain; never imports SwiftUI/UIKit) → **WhereUI** (SwiftUI -views + view models — *not* the domain model) → the thin hosts (**Where** app, -**WhereIntents**, **WhereWidgets**, **WhereShareExtension**, **RegionViewer**). -Each layer reaches only *down*. The app target stays intentionally tiny: add -domain behavior to WhereCore, presentation to WhereUI — not there. +views + view models) → the thin hosts (**Where** app, **WhereIntents**, +**WhereWidgets**, **WhereShareExtension**, **RegionViewer**). Each layer +reaches only *down*; each module's own `AGENTS.md` / `README.md` is the +authority on what it is. Add domain behavior to WhereCore and presentation to +WhereUI — the app target stays tiny. ## Layering -Where splits **domain** from **presentation**. Keep the split sharp — views -must not grow business logic just because SwiftUI makes it easy. - | Layer | Where | Owns | |-------|-------|------| -| **Domain / services** | `WhereCore` (`WhereServices` collaborators) | Rules, detection, aggregation, persistence, side effects (reminders, widgets, backup). Unit-test here. | -| **View model** | `WhereUI` (`WhereModel`, the `WhereSession` coordinator + scope-tiered `YearReportModel` / `ResolveModel` / `BackupModel` / `RemindersSettingsModel`) | Lifecycle wiring, observable mirrors of service output, UI intent methods. Orchestrates `WhereServices`; does not reimplement Core rules. | -| **Views** | `WhereUI` (`*View`) | Layout, navigation, localized copy, bindings to the coordinator / scoped models. Calls view-model methods; does not talk to the store, run detection, or own cache/throttle policy. | +| **Domain / services** | `WhereCore` (`WhereServices` collaborators) | Rules, detection, aggregation, persistence, side effects. Unit-test here. | +| **View model** | `WhereUI` (`WhereModel`, the `WhereSession` coordinator, the scoped `YearReportModel` / `ResolveModel` / `BackupModel` / `RemindersSettingsModel`) | Lifecycle wiring, observable mirrors of service output, UI intent methods. | +| **Views** | `WhereUI` (`*View`) | Layout, navigation, localized copy, bindings. Never store I/O, detection, or cache/throttle policy. | When in doubt: if the behavior would still be correct without SwiftUI, it -belongs in `WhereCore` (or, for orchestration that exists only to serve the -UI, on the `WhereSession` coordinator or a scoped model — still not in a +belongs in `WhereCore` (or on the coordinator / a scoped model — still not a `View`). Rules the code enforces and agents must preserve: -- **`WhereServices` is the entry point** to the domain — UI never talks to the - store or location source directly. -- **All store mutations happen inside `WhereStore.perform { ... }`** (the block - owns the write transaction; the production store traps otherwise). Values - cross the persistence boundary, never SwiftData records. -- **One read path.** Every committed write (manual edit, live GPS, CloudKit - remote import) pings the single store-change signal (`WhereStore.changes()`), - and readers refresh purely off it — write intents just commit, they don't - refresh inline. The scene's `YearReportModel` subscribes while it's active; - `DataIssueScanner` drops its cache on the same signal. Launch is driven by - [`LifecycleKit`](../Shared/LifecycleKit)'s typed `LaunchPlan` (see - `WhereLaunch` in WhereUI: each step is a type whose `Input`/`Output` thread - the store scope → session scope through the trunk), rendered by +- **`WhereServices` is the domain entry point** — UI never talks to the store + or location source directly. +- **All store mutations run inside `WhereStore.perform { … }`** (the + production store traps otherwise); values cross the boundary, never + SwiftData records. +- **One read path.** Every committed write pings `WhereStore.changes()`, and + readers refresh purely off that signal — write intents commit, they don't + refresh inline. Launch is a typed [`LifecycleKit`](../Shared/LifecycleKit) + `LaunchPlan` (`WhereLaunch` in WhereUI), rendered by [`LifecycleKitUI`](../Shared/LifecycleKitUI)'s container in `RootView`. -- **All logging goes through [Periscope](../Shared/Periscope)** via `WhereCore`'s - `WhereLog` facade — never a raw string. A collaborator derives a typed - `LogEvent` leaf off a grouping scope (`WhereLog.(SomeLog.self)` / - `WhereLog.root(SomeLog.self)`; the groups are the `WhereLog` cases), and each - module keeps its own `*Log.swift` types in its `Sources/Logging/` folder rather - than beside the collaborator that emits them. The parts you can't read off the - source: - - Events log as `.public`, so **keep PII out**. `info` = success of an - important operation, `warning` = degraded-but-handled, `error`/`fault` = - outright failure; catch-path events carry a `LogAttachment.error(_:)`. Hot - paths (per-sample persist, widget throttle) stay quiet by design. - - **One process-wide system, several root scopes.** RegionKit can't see - `WhereLog`, so it owns a `"RegionKit"` root — but emits into the *same* - `Periscope.shared`, so the single `PeriscopeStore` sink the app attaches at - launch (and `PeriscopeViewer`) sees every subtree in one stream. - - **Only the app process gets a store.** Widgets and the share extension run - in their own processes, where `Periscope.shared` stays OSLog-only. App - Intents run **in the app process** (`WhereIntents` is a library product - linked into the Where app, not an extension), so their logs reach the - app's store-attached Periscope. - - **An event about a store object stamps its `externalID` with that object's - canonical `store://` identity** (`DataIssueID.storeURL`, otherwise - `WhereStoreID`), so inspect-by-object shares the store's and backups' keys. - RegionKit's parallel `region://` scheme exists because it can't see those - types — see [`RegionKit/AGENTS.md`](RegionKit/AGENTS.md). -- **Location comes through the `LocationSource` protocol** — production is - `CoreLocationSource`; tests and previews use `ScriptedLocationSource`. Besides - the passive `sampleStream`, it offers a best-effort one-shot - `requestCurrentLocation()` — used both to stamp manual entries and to log - today when the app is opened on a day with no GPS sample yet - (`LocationIngestor.captureTodayIfNeeded`, driven by `WhereSession` on launch/ - foreground); it returns `nil` rather than throwing when no fix is available. -- **Manual entries carry a `ManualEntryAudit`** (when made, an optional note, - and a best-effort capture-time `CapturedLocation`). The view-model intents - assemble it; `DayJournal`'s write methods take an explicit `audit:` (no - default) and persist it. An additive backfill can't downgrade an - authoritative row's regions, but the newer audit always wins. +- **All logging goes through [Periscope](../Shared/Periscope)** as typed + `LogEvent`s off the `WhereLog` facade, never a raw string; each module keeps + its `*Log.swift` event types in its `Sources/Logging/` folder. Not + re-derivable from source: events log `.public`, so **keep PII out**; `info` + = important success, `warning` = degraded-but-handled, `error`/`fault` = + outright failure; hot paths stay quiet by design. RegionKit emits a separate + `"RegionKit"` root into the *same* `Periscope.shared`. Only the app process + attaches a store — widgets and the share extension are OSLog-only; App + Intents run in the app process. An event about a store object stamps its + `externalID` with the object's `store://` identity; RegionKit's parallel + scheme is `region://` (see [`RegionKit/AGENTS.md`](RegionKit/AGENTS.md)). +- **Location comes through the `LocationSource` protocol** — + `CoreLocationSource` in production, `ScriptedLocationSource` in + tests/previews. The one-shot `requestCurrentLocation()` returns `nil` rather + than throwing when no fix is available. +- **Manual entries carry a `ManualEntryAudit`**; `DayJournal`'s write methods + take an explicit `audit:` (no default). An additive backfill can't downgrade + an authoritative row's regions, but the newer audit always wins. - **`WhereServices.recentActivity`** (the on-demand Foundation Models - activity summarizer, behind the `ActivitySummaryGenerating` seam) is - distinct from `WhereServices.summary` (the daily notification recap). It - caps collapsed region transitions so a long window's prompt still fits the - model's context, and model unavailability surfaces as a typed reason — never - a silent empty summary. + summarizer, behind `ActivitySummaryGenerating`) is distinct from + `WhereServices.summary` (the daily notification recap); model unavailability + surfaces as a typed reason, never a silent empty summary. ## Navigation -The logged-in shell is `MainTabs` — **three fixed tabs**: Locations, Your Year, -Settings. Everything else hangs off one of them: Elsewhere is an entry card on -Locations, Resolve a Locations toolbar button, and the data screens -(attachments, logged days, regions) live in the Settings "Data" group. `MainTabs` -owns the scene-scoped `YearReportModel` and passes it to each tab by explicit -init injection, so the wiring is compile-checked; the always-on `WhereSession` -coordinator travels in the environment instead. - -A new screen belongs *inside* that shape — a destination pushed from a tab, a -sheet, or a Settings row. Adding a fourth tab is a product decision, not a -refactor: raise it before building. - -Settings itself is a typed-route list: `SettingsSearch.swift` owns -`SettingsDestination` / `SettingsListSection` / `SettingsRoute`, and every -switch over them is exhaustive, so a new drill-in is a set of compile errors to -fill in rather than a screen you can forget to register. **About is deliberately -the last block**, below everything actionable. - -**Nothing on the About screen is a list hard-coded in the view.** It renders -three sources: the generated attribution report (via `WhereCore.AppAttribution`), -`RegionDataSource` (RegionKit) for bundled geometry, and `BuildInfo` (WhereCore) -for the running build. Adding a dependency means re-running `./attribution`; -adding a dataset means regenerating RegionKit's — see -[`CreditKit/AGENTS.md`](../Shared/CreditKit/AGENTS.md) and -[`RegionKit/AGENTS.md`](RegionKit/AGENTS.md). - -The report lives in `Where/Where/Resources/attribution.json` — the **app -target's** resources, so only the app bundle carries one. Every other bundle -(RegionViewer, `StuffTestHost`, the extensions) reads `nil` and the screen says -so, exactly as it does for an unstamped `BuildInfo`. A section with nothing of -its kind says so too, in different words: "no report at all" and "nothing of -this kind" describe different builds, and a header and footer over no rows would -describe neither. `AppAttributionTests`, in the app's own test bundle because it -is the one hosted by `Where.app`, asserts the report is usable — present, -decodable, both kinds populated. Whether it still *matches* the dependency graph -is `./attribution --check`'s job in CI, not a literal here. - -A **shipped library** and a **development tool** render as separate sections. -Development tools are the agent skills `./sync-agents` vendors *and* packages -linked only outside the app's target closure (the snapshot-testing engine, the -accessibility parser) — credited because the repository depends on them, not -because they reach a device. Keep the sections separate: one merged list would -tell a reader something untrue about the app they are running. +The logged-in shell is `MainTabs` — **three fixed tabs**: Locations, Your +Year, Settings; everything else hangs off one of them. A new screen is a +pushed destination, a sheet, or a Settings row inside that shape — a fourth +tab is a product decision to raise before building. `MainTabs` passes the +scene-scoped `YearReportModel` by explicit init injection; the always-on +`WhereSession` coordinator travels in the environment. Settings is a +typed-route list (`SettingsSearch.swift`; every switch is exhaustive), so a +new drill-in is a set of compile errors to fill in; About stays the last +block. + +The About screen renders three live sources — the generated attribution +report (`WhereCore.AppAttribution`), `RegionDataSource`, and `BuildInfo` — +never a list hard-coded in the view. A missing report or unstamped build +renders an honest empty state, and shipped libraries stay a separate section +from development tools. Design and rationale: PR #140. ## Localization -All user-facing copy resolves through each module's `Localizable.xcstrings` via -Xcode's **generated `LocalizedStringResource` symbols** (`STRING_CATALOG_GENERATE_SYMBOLS`, -set project-wide in `Project.swift`; SwiftPM package targets get it from the -toolchain). A manual catalog key `some.key` generates a `.someKey` symbol, so a -typo'd or removed key is a **compile error**, not a runtime fallback — never -write a raw `String(localized: "literal.key")` or a hand-maintained key facade. - -- **WhereUI:** reference the generated symbol directly — `Text(.tabYear)`, - `String(localized: .commonOk)`. Strings that need composition, pluralization, - a `switch`, or non-catalog number/coordinate formatting go through - [`WhereFormat`](WhereUI/Sources/Shared/WhereFormat.swift), which composes the - symbols (years use a grouping-free number style — "2026", not "2,026"; counts - use catalog plural variations). -- **WhereCore:** user-visible errors use `String(localized: .symbol)` from its - own catalog. -- **RegionKit:** region names (`Region.localizedName`) come from the - `regions.json` manifest, with an optional `localizationKey` overriding from - RegionKit's own `Localizable.xcstrings` (`bundle: .module`) — so, unlike the - other modules, region names are resolved **dynamically** and lose static - string-catalog symbols (a deliberate trade-off for a data-driven catalog). -- **Extensions** (WhereWidgets, WhereShareExtension) reference their own - generated symbols for chrome and reuse WhereUI's public presentation helpers - for shared copy. -- **DEBUG-only UI** still gets catalog entries — don't bypass localization - because a surface is dev-only. - -Add the key to the catalog as a **manual** entry first (so its symbol -generates), then reference `.thatSymbol` — never ship English literals in -SwiftUI `Text` or `errorDescription`, and never reintroduce a raw-key facade. - -The catalogs also carry a few **auto-extracted** entries with no -`extractionState` and no value — `""`, `%lld`, an English literal from a -`#Preview`. Those are Xcode's, not ours: extraction picks up every string -literal that could be a key (`Marker("", …)`, `Text("\(count)")`), and an IDE -build re-adds any that's missing, so deleting one by hand just brings it back -with a full reserialization behind it. Remove the *source* literal if you want -the entry gone — interpolating an `Int` into `Text` is worth fixing anyway (it -skips `WhereFormat`'s number styling). Catalog files stay byte-identical to -Xcode's own serialization; see [Formatting](../AGENTS.md#formatting). +All user-facing copy resolves through each module's `Localizable.xcstrings` +via Xcode's generated `LocalizedStringResource` symbols, so a typo'd or +removed key is a compile error. Add a key as a **manual** entry first (so its +symbol generates), then reference `.thatSymbol` — never a raw +`String(localized: "literal.key")`, a hand-maintained key facade, or an +English literal in `Text` / `errorDescription`. + +- **WhereUI:** reference symbols directly; composition, pluralization, and + number/coordinate formatting go through + [`WhereFormat`](WhereUI/Sources/Shared/WhereFormat.swift). +- **RegionKit:** region names resolve dynamically from `regions.json` + (+ optional `localizationKey`) — the one deliberate exception to static + symbols (see [`RegionKit/AGENTS.md`](RegionKit/AGENTS.md)). +- **Extensions** use their own generated symbols for chrome and WhereUI's + public helpers for shared copy. **DEBUG-only UI** is still localized. +- The catalogs carry a few value-less **auto-extracted** entries (`""`, + `%lld`): Xcode's, not ours — an IDE build re-adds a deleted one, so remove + the *source* literal instead. Catalogs stay byte-identical to Xcode's own + serialization (root [Formatting](../AGENTS.md#formatting)). ## Dates & presentation -- **A logical day is a `CalendarDay` (Y-M-D), not a `Date`.** It is the - timezone-independent identity every stored day-record and day comparison keys - on (see [`WhereCore/AGENTS.md`](WhereCore/AGENTS.md)); a `Date` is only for - instants (GPS bucketing, grid geometry, display), derived via - `CalendarDay.startOfDay(in:)`. Never persist a day as an absolute instant. -- **Year bounds are half-open** (`[Jan 1 year, Jan 1 year+1)`); **day ranges - are inclusive** (`Date.calendarDays(through:in:)` for instants, - `CalendarDay.days(through:)` for logical days). -- **The app is Gregorian-only.** All presence data is aggregated in a Gregorian - calendar (`DayAggregator()` defaults to Gregorian + current time zone), so any - day/year math must use a Gregorian calendar — **never `Calendar.current`**, - which on a non-Gregorian device (Buddhist, Japanese-era, …) reports a - different year and silently mismatches the stored reports. Use the calendar - the owning type vends (below), or a fresh `Calendar(identifier: .gregorian)` - with the current time zone (see `Calendar.whereIntents` in WhereIntents). -- **Inject `Calendar`, don't reach for globals** — the scene's - `YearReportModel` owns the calendar (Gregorian, current time zone) its - missing-day math uses; layout types carry the calendar they were built with. - Prefer calendar APIs over hardcoding day/weekday counts - (`Calendar.dayCount(ofYear:)` derives 365/366 rather than assuming a length). +- **A logical day is a `CalendarDay` (Y-M-D), not a `Date`** — see + [`WhereCore/AGENTS.md`](WhereCore/AGENTS.md). Never persist a day as an + absolute instant. +- **Year bounds are half-open; day ranges are inclusive** + (`Date.calendarDays(through:in:)`, `CalendarDay.days(through:)`). +- **The app is Gregorian-only: never `Calendar.current`** — a non-Gregorian + device calendar silently mismatches the stored reports. Use the calendar the + owning type vends, or a fresh `Calendar(identifier: .gregorian)` with the + current time zone (see `Calendar.whereIntents`). +- **Inject `Calendar`, don't reach for globals**; prefer calendar APIs over + hardcoded day/weekday counts (`Calendar.dayCount(ofYear:)`). - **Core layout APIs throw on failure**; views surface `ContentUnavailableView` + log, never `!`. -- Appearance tokens live in `WhereStylesheet` — see - [`WhereUI/AGENTS.md`](WhereUI/AGENTS.md) for how to read and extend it. - Shared date-range copy lives in `DateRangeFormatting`; numbers and dates use - `FormatStyle`, not string interpolation. Expensive layout computes once into - state, not per `body` pass. Sharing uses `ShareLink` / `Transferable`. +- Appearance tokens live in `WhereStylesheet` + ([`WhereUI/AGENTS.md`](WhereUI/AGENTS.md)); shared date-range copy in + `DateRangeFormatting`; numbers and dates use `FormatStyle`, not string + interpolation. Expensive layout computes once into state, not per `body` + pass. Sharing uses `ShareLink` / `Transferable`. ## SwiftUI views & previews Every previewable component in `WhereUI` (any `View`, `Widget`, or `WidgetBundle`) **must** ship at least one `#Preview` in the same file, -wrapped in `#if DEBUG` at the bottom. Don't construct services, stores, or -location sources inline — pull fixtures from -[`PreviewSupport`](WhereUI/Sources/Preview/PreviewSupport.swift) (synchronous, -in-memory, never touch disk/CloudKit/CoreLocation). Pass scoped models -explicitly (a `YearReportModel` via `report:`, a seeded `ResolveModel`) and -inject ambient app state through the environment (`WhereModel` for the app -shell, the `WhereSession` coordinator for logged-in views). Cover the states -that matter — empty, loaded, and distinct edge states — not just the happy -path. - -- **Animate transitions between distinct states** in a way that fits the - surface and its content — don't hard-cut. A view that swaps on a `LoadState` - (or shows an in-flight status) should fade/move rather than snap (e.g. - `.transition(.opacity)` on each `switch` arm plus `.animation(_:value:)`). - Hidden means *out of the tree* (`if` + transition), not opacity zero. -- **A displayed value that can change under the user morphs, too.** Live counts - (a region's day count as a sample lands) get a `.contentTransition` — but one - animates *only* inside a transaction, so it needs a paired - `.animation(_:value:)` on an ancestor or it silently hard-cuts. The transition - and its animation are one stylesheet token, since Reduce Motion changes both - (see `CardStyles.DayCountStyle`, which `RegionSummaryCard` reads resolved). -- **Derive UI dimensions; don't repeat them.** A repeated dimension gets one - named home; real chrome is measured from the live UI via a preference key / - `onGeometryChange` (see `DeveloperTabBarInset`) rather than hardcoding its - expected size; controls scale with `@ScaledMetric`; prefer semantic font - styles over fixed point sizes. -- **Custom full-screen surfaces must work under VoiceOver.** A surface that - takes over the screen carries the `.isModal` accessibility trait and posts - `.screenChanged` when crossing the modal boundary (`.layoutChanged` for - lighter transitions). Non-modal floating chrome stays reachable behind (see +wrapped in `#if DEBUG` at the bottom, built from +[`PreviewSupport`](WhereUI/Sources/Preview/PreviewSupport.swift) fixtures — +synchronous, in-memory, never disk/CloudKit/CoreLocation. Cover empty, +loaded, and distinct edge states, not just the happy path. + +- **Animate transitions between distinct states** — `.transition` on each + `switch` arm plus `.animation(_:value:)`; hidden means *out of the tree*, + not opacity zero. +- **A displayed value that can change under the user morphs, too** — a + `.contentTransition` needs a paired `.animation(_:value:)` or it silently + hard-cuts, and the transition and its animation are one stylesheet token + (see `CardStyles.DayCountStyle`). +- **Derive UI dimensions; don't repeat them** — measure real chrome via a + preference key / `onGeometryChange` (see `DeveloperTabBarInset`), scale + controls with `@ScaledMetric`, prefer semantic font styles. +- **Custom full-screen surfaces must work under VoiceOver** — the `.isModal` + trait plus `.screenChanged` across the modal boundary (see `DeveloperOverlay`). ## Adding things -- **New library target:** add to root [`Package.swift`](../Package.swift) - under `Where//Sources`, then wire a hosted test bundle in - [`Project.swift`](../Project.swift) via the `unitTests` helper. -- **New region:** it's **pure data** now — add geometry under - `RegionKit/Tools/source/`, run `ruby Where/RegionKit/Tools/generate-regions.rb` - to regenerate `RegionKit/Sources/Resources/regions/` + `regions.json` (extend - the script's id map / `NON_US` list as needed), optionally add a `region.` string + - `localizationKey`, and add a `RegionAttributorTests` spot-check. No `Region` - case, no code — `RegionStyle`, region pickers, and the App Intents - `RegionEntity` all derive from the catalog. (See - [`RegionKit/README.md`](RegionKit/README.md#adding-a-region).) +- **New library target:** root [`Package.swift`](../Package.swift) under + `Where//Sources`, plus a hosted test bundle via `Project.swift`'s + `unitTests` helper. +- **New region:** pure data — no `Region` case, no code; see + [`RegionKit/README.md`](RegionKit/README.md#adding-a-region). - **New evidence kind / sample source:** add the case and follow the compile errors through the exhaustive switches. -- **New app icon:** run `./icons --add` (see the root +- **New app icon:** `./icons --add` (root [`AGENTS.md`](../AGENTS.md#managing-app-icons)) — never hand-edit the catalogs or manifest. ## Installing to a device -`./Where/install` builds the app and installs it on a connected iPhone -straight from the CLI — no Xcode UI. It regenerates the project, builds + -code-signs the `Where` scheme with `xcodebuild` (the Debug configuration with -compiler optimizations forced on by default, so the DEBUG-only developer -surfaces survive while the app still runs at roughly Release speed; -`-allowProvisioningUpdates` so the app + extensions provision automatically), -then copies and launches it via `xcrun devicectl` (see `./Where/install ---help`). macOS-only, and it needs a signing team configured once via `./ide ---team-id `. Auto-picks the sole paired physical iPhone (booted -simulators are ignored) and prompts you to unlock it before installing; pass -`--configuration ` to build a different configuration (e.g. `Release`), -`--no-optimize` to leave the configuration's own optimization level alone, -`--device ` to disambiguate, `--no-launch` to install without -launching, `--yes` to skip the unlock prompt. +`./Where/install` builds, signs, and installs the app onto a connected iPhone +from the CLI — macOS-only, one-time `./ide --team-id ` setup. It defaults +to Debug with compiler optimizations forced on, so DEBUG-only developer +surfaces survive at near-Release speed. Options: `./Where/install --help`. ## Testing @@ -296,9 +180,9 @@ Root [testing conventions](../AGENTS.md#testing) apply. What's specific here: `Project.swift` and link `TestHostSupport` (`show(_:perform:)`, `waitFor`). - Use `ScriptedLocationSource` and `SwiftDataStore.inMemory()` — never `CoreLocationSource` or the user's on-disk/CloudKit store. The CloudKit - remote-import path is exercised with the `@_spi(Testing)` + remote-import path uses the `@_spi(Testing)` `inMemory(remoteChangeSource:)` + `ScriptedStoreRemoteChangeSource`. -- How screens render is pinned by the matrixed image snapshots in +- How screens render is pinned by the image snapshots in `WhereUI/SnapshotTests/` (the `WhereUISnapshotTests` bundle, run from the shared `StuffSnapshotTests` scheme + CI job, not `Stuff-iOS-Tests`) — see [`WhereUI/AGENTS.md`](WhereUI/AGENTS.md#testing). Don't add "hosts without diff --git a/Where/Where/AGENTS.md b/Where/Where/AGENTS.md index 99987ab8..bc0777c8 100644 --- a/Where/Where/AGENTS.md +++ b/Where/Where/AGENTS.md @@ -23,16 +23,14 @@ layering, and the domain rules this target merely starts up. it. - `Resources/AppIcon.xcassets` is managed by `./icons` (see the root [`AGENTS.md`](../../AGENTS.md#managing-app-icons)) — never hand-edit it. -- `Resources/attribution.json` is the app's generated attribution report, and - `attribution-sources.json` at this module's root declares where the report - reads from. - Both are `./attribution`'s (see - [Attribution](../../AGENTS.md#attribution)) — never hand-edit the report. This - is the **only** bundle that carries one, which is why `AppAttributionTests` - lives in this target's test bundle: it is the one hosted by `Where.app`, so - `Bundle.main` is the shipping bundle. Those tests cover the report being - usable; `./attribution --check` in CI covers it still matching the dependency - graph, which no test bundle can see. +- `Resources/attribution.json` is the app's generated attribution report; + `attribution-sources.json` at this module's root declares where it reads + from. Both are `./attribution`'s + ([Attribution](../../AGENTS.md#attribution)) — never hand-edit the report. + Only this bundle carries one, so `AppAttributionTests` lives in this + target's test bundle (the one hosted by `Where.app`, where `Bundle.main` is + the shipping bundle); `./attribution --check` in CI covers the report still + matching the dependency graph, which no test bundle can see. ## Invariants diff --git a/Where/WhereCore/AGENTS.md b/Where/WhereCore/AGENTS.md index dfa5b11f..36ca15dd 100644 --- a/Where/WhereCore/AGENTS.md +++ b/Where/WhereCore/AGENTS.md @@ -1,18 +1,15 @@ # WhereCore – Module Shape WhereCore is the domain layer of the Where feature: the persistence boundary, -GPS ingestion, per-day / per-year aggregation, data-quality detection, and the -side effects that hang off a committed write (reminders, widgets, backup, -on-device activity summaries). It is assembled behind one `Sendable` value — -`WhereServices` — that the UI and the App Intents stack talk to (widgets never -do; they read the published `WidgetSnapshot` from the App Group). See -[`README.md`](README.md) for the public API and how the pieces fit. +GPS ingestion, per-day / per-year aggregation, data-quality detection, and +the side effects that hang off a committed write. It is assembled behind one +`Sendable` value — `WhereServices` — that the UI and the App Intents stack +talk to (widgets never do; they read the published `WidgetSnapshot` from the +App Group). See [`README.md`](README.md) for the public API and collaborators. -The **domain/presentation split and the rules WhereCore must uphold** (the -`WhereServices` entry point, `WhereStore.perform` writes, the single -read-refresh signal, `WhereLog` logging, `LocationSource`, `ManualEntryAudit`) -live in the feature [`Where/AGENTS.md`](../AGENTS.md#layering) — read that and -the root [`AGENTS.md`](../../AGENTS.md) first. This file adds only the module's +The domain/presentation split and the rules WhereCore must uphold live in the +feature [`Where/AGENTS.md`](../AGENTS.md#layering) — read that and the root +[`AGENTS.md`](../../AGENTS.md) first. This file adds only the module's internal shape. ## Scope & dependencies @@ -23,153 +20,91 @@ internal shape. ## Shape & invariants -- **`WhereServices` is the composition root**, not a god-object. It wires the - focused, single-responsibility collaborators (`ReportReader` reads, - `DayJournal` writes, `LocationIngestor` GPS, `DataIssueScanner` detection, the - reminder / summary / issue-alert reconcilers, `WidgetSnapshotPublisher`, - `BackupCoordinator`, `RecentActivitySummarizer`) and owns the one - cross-collaborator operation, `reset()`. Add new behavior to the collaborator - it belongs to. -- **`WhereStore` is a value-type boundary.** Everything crossing it is a value, - never a SwiftData record; every mutation runs inside `perform { … }` (the - production `SwiftDataStore` traps otherwise), and each committed transaction - pings `changes()` — the single signal readers refresh from. The live - `ModelContainer` is surfaced only for the read-only debug inspector. - Each process opens its on-disk store **once** and injects it — the app's - launch opens it, and the App Intents stack shares it via - `WhereServices.forIntents(sharingStoreOf:)` — rather than a second caller - opening another container over the same file (concurrent first-launch - creation is how the launch once failed). -- **Primary regions *are* the tracked-region set.** The picked primary regions - (`primaryRegions()` / `setPrimaryRegions(_:)`) are the same `SDTrackedRegion` - rows `trackedRegions()` reads — picking scopes GPS attribution *and* carries - each region's `RegionAppearance` (color token / emoji / SF Symbol) + pick - order. `RegionAppearance` is data (WhereCore); the token→`Color` mapping and - option catalogs are presentation (`WhereUI`). -- **Backups mirror the persisted model — keep them lossless.** Any change to - persisted data (a new/changed `SD*` field, or a value type that crosses - `WhereStore`) must be reflected end-to-end in the backup so export/restore - never silently drops it: add it to `BackupArchive`, write it in - `BackupService.makeArchiveFile`, read it back in `BackupCoordinator.importBackup` - for **both** `.replace` and `.merge`, and add a round-trip test - (`BackupServiceTests` for the archive, `BackupCoordinatorTests` for the store - round-trip). The archive is **strict synthesized `Codable`** — no in-code - legacy decode. A shape change **bumps `BackupArchive.currentFormatVersion`** - (`readArchive` rejects any other version) and is handled out of band by - extending [`../Tools/upgrade-backup.rb`](../Tools/upgrade-backup.rb), per the - no-migration-on-read rule below. Example: v2 added `primaryRegions` (per-region - picked appearance + pick order), the tool synthesizes it from the legacy - `trackedRegions` ids, and import restores looks from it. -- **A logical day is a `CalendarDay`, not a `Date`.** Every stored user record - and day comparison keys on `CalendarDay` (year-month-day), because a `Date` is - an absolute instant and persisting a day as one makes it drift onto a - *different* day when the device changes time zones — the residency bug this - exists to prevent. Reach for a `Date` only where you genuinely need an instant - (bucketing a GPS `sample.timestamp`, grid geometry, sorting, display) and - derive it via `CalendarDay.startOfDay(in:)`. `TimeZoneIndependenceTests` is - the guard, and its doc comment carries the original bug report. - - **Scope boundary an agent would get wrong:** only *user-asserted* records - are travel-proof. A GPS `sample.timestamp` is bucketed by the *current* - calendar at read time, so a GPS-derived day can still shift by one across a - time-zone change — and with it a dismissed GPS-only border-drift or - abrupt-change issue, whose id is that re-bucketed day, can reappear. Fixing - that would mean bucketing GPS by a fixed home zone, which we deliberately - don't do: the question is "where was I on this *local* day?". -- **Composite identity keys are `store://` URLs, not joined strings.** Conform to - `WhereStoreURLCodable` (see `DataIssueID`) and build/parse with `StoreURL`; it - hands you `Codable` and a stable SwiftData string key for free, so never an - ad-hoc `type:value` string or a hand-written keyed `Codable`. Families with no - dedicated identity type (days, years, evidence, samples) get theirs from - `WhereStoreID`. `WhereStoreURLCodableTests` pins the shape, the round-trip, and - the rejection of malformed URLs. -- **No in-app data migration or legacy recovery.** A data-shape change is not - migrated on read or at boot: `SD….toValue()` reads only the current shape and - drops (fault-logs) a row it can't place — e.g. an `SDManualDay` with no - `dayKey`. The one-time path to reshape existing data is a backup **export → - transform → replace-import**, where the transform is - [`../Tools/upgrade-backup.rb`](../Tools/upgrade-backup.rb) (rekeys regions, - fills defaults, rewrites dismissal keys to `store://` URLs, resets the format - version). This is deliberate for pre-release; the durable, general successor - (per-entity schema versioning) is tracked in [`../TODOs.md`](../TODOs.md). +- **`WhereServices` is the composition root, not a god-object.** It wires + focused single-responsibility collaborators (the live list is its + initializer; `README.md` describes them) and owns the one + cross-collaborator operation, `reset()`. Add new behavior to the + collaborator it belongs to. +- **`WhereStore` is a value-type boundary.** Everything crossing it is a + value, never a SwiftData record; every mutation runs inside + `perform { … }` (the production store traps otherwise), and each committed + transaction pings `changes()`. The live `ModelContainer` is surfaced only + for the read-only debug inspector. +- **Each process opens its on-disk store once and injects it** — the app's + launch opens it; the App Intents stack shares it via + `WhereServices.forIntents(sharingStoreOf:)`. A second container over the + same file is how a fresh install once raced the launch into failure (root + [Composition](../../AGENTS.md#composition-create-once-inject-down)). +- **Primary regions *are* the tracked-region set.** `primaryRegions()` / + `setPrimaryRegions(_:)` read/write the same `SDTrackedRegion` rows as + `trackedRegions()` — picking scopes GPS attribution *and* carries each + region's `RegionAppearance` + pick order. `RegionAppearance` is data + (WhereCore); the token→`Color` mapping is presentation (WhereUI). +- **Backups mirror the persisted model — keep them lossless.** Any persisted + change is reflected end-to-end: add it to `BackupArchive`, write it in + `BackupService.makeArchiveFile`, read it back in + `BackupCoordinator.importBackup` for **both** `.replace` and `.merge`, and + add a round-trip test (`BackupServiceTests` / `BackupCoordinatorTests`). + The archive is strict synthesized `Codable` — no in-code legacy decode; a + shape change bumps `BackupArchive.currentFormatVersion` and extends + [`../Tools/upgrade-backup.rb`](../Tools/upgrade-backup.rb) instead. +- **A logical day is a `CalendarDay`, not a `Date`.** `CalendarDay` (Y-M-D) + is the timezone-independent identity every stored user record and day + comparison keys on; persisting a `Date` makes a day drift across time-zone + changes — the residency bug this exists to prevent. Use a `Date` only for + genuine instants (GPS bucketing via `CalendarDay(from:in:)`, grid geometry, + sorting, display), derived via `CalendarDay.startOfDay(in:)`. + **Scope boundary:** only user-asserted records are travel-proof — a GPS + sample is bucketed into a `CalendarDay` by the *current* calendar at read + time, so a GPS-derived day (and a dismissed GPS-only issue keyed on it) can + still shift by one across a time-zone change. Deliberate: "where was I on + this *local* day?" — don't bucket GPS by a fixed home zone. +- **Composite identity keys are `store://` URLs, not joined strings.** + Conform to `WhereStoreURLCodable` (see `DataIssueID`), building/parsing + with `StoreURL`; families without a dedicated identity type get theirs from + `WhereStoreID`. Used to stamp Periscope `LogEvent.externalID`s. +- **No in-app data migration or legacy recovery.** `SD….toValue()` reads only + the current shape and drops (fault-logs) a row it can't place. The one-time + reshape path is backup **export → transform + ([`../Tools/upgrade-backup.rb`](../Tools/upgrade-backup.rb)) → + replace-import**. Deliberate pre-release; the durable successor + (per-entity schema versioning) is filed in [`../TODOs.md`](../TODOs.md). - **Writes await their side effects.** `DayJournal` commits, then awaits the - reminder reconcile and widget publish in sequence, so a reader on the next - `changes()` ping never sees a half-applied write - (`DayJournalTests.addManualDayReconcilesAndPublishes`). `DataIssueScanner` - drops its cache on the same signal *and* is invalidated inline where a caller - needs it provably fresh (`WhereServices.reset()`) — the deterministic half of - that pair, not redundant with it. + reminder reconcile + widget publish in sequence, so a reader on the next + `changes()` ping never observes a half-applied write. +- **Post-write reconciliation is defined once.** Every write and import + routes through `DayJournal.reconcileAfterDayChange()` (or its widget-less + subset `reconcileIssueState()`) — never copy the fan-out into a new write + path. Cross-collaborator hooks take a single closure wired at the + composition root (`BackupCoordinator.onImport`). - **Detectors read aggregated input; the speed-based one needs raw fixes.** - `DataIssueScanner` builds `DataIssueInput` from the `YearReport` plus - `daySamples` — per-day GPS fixes (`.gpsVisit` / `.gpsSignificantChange` only, - sorted by timestamp), the one field that keeps per-fix timestamps. Manual and - evidence-implied samples are excluded so `FlightDayDetector`'s speed math - isn't skewed by user-asserted timestamps. -- **Post-write reconciliation is defined once.** A write or import reconciles by - calling `DayJournal.reconcileAfterDayChange()` (or its widget-less subset - `reconcileIssueState()` for dismiss/restore paths) — never by copying the - fan-out into a new write path. Cross-collaborator hooks take a single closure - wired at the composition root (`BackupCoordinator.onImport`), not an injected - list of reconcilers. **Two known paths don't yet honor this**, so don't read - it as "reconciliation has already happened" when working nearby: - `WhereServices.setPrimaryRegions(_:)` commits and pings `changes()` without - the fan-out, and the daily summary reconciles only on launch/foreground - `configure` and settings edits. Both are tracked in - [`../TODOs.md`](../TODOs.md); a new write path should route through the fan-out - rather than copy their omission. -- **`LocationSource` abstracts GPS.** Production is `CoreLocationSource` (Visits - + significant-change); tests/previews use `ScriptedLocationSource`. The - one-shot `requestCurrentLocation()` returns `nil` (never throws) when no fix - is available — it stamps a manual entry's audit trail and backs - `LocationIngestor.captureTodayIfNeeded(now:)`, which persists a fix for today - (via the normal ingest path) when the app opens on a day with no GPS sample. -- **Tracked regions live in the store, not preferences.** They're synced app - data (one `SDTrackedRegion` row per region so concurrent cross-device edits - merge; read as a `Set`, defaulting to the four when unset). `RegionAttribution` - derives the attributor from them and rebuilds on `changes()`; assemble via the - async `WhereServices.make(...)`, which reads the set. The App Intents stack - does **not** re-read it: `WhereServices.forIntents(sharingStoreOf:)` is - synchronous and non-throwing precisely because it reuses the assembled - layer's store, attributor, aggregator, and clock, swapping in only an - `IdleLocationSource` — so the two can't drift apart, and installing the stack - has no failure path. (`makeForIntents(store:now:)` is the async test seam - that derives attribution from a seeded store instead.) Detection is naturally - scoped to the set — the attributor only loads tracked-region geometry, so - `distanceToBoundary` is `nil` elsewhere. -- **Adding an external package means re-running `./attribution`.** WhereCore - links the app's only third-party package (ZIPFoundation), but it does **not** - own attribution: the report format and tooling are - [`CreditKit`](../../Shared/CreditKit/AGENTS.md)'s, and the report itself is the - app target's resource. Regenerate and commit it, or the app ships a library - whose license it doesn't reproduce. -- **`AppAttribution` and `BuildInfo` both read *this bundle*, and both treat - absence as legitimate.** Only the app target gets a stamped Info.plist and a - generated report, so a bundle without either (RegionViewer, `StuffTestHost`, - an extension) reports `nil` rather than a placeholder. `AppAttribution` splits - that from a report that is present and won't decode, which is a corrupt - resource and faults — don't collapse the two back together. See [Version and - build metadata](../../AGENTS.md#version-and-build-metadata) and - [Attribution](../../AGENTS.md#attribution). -- **Read the app's report through `AppAttribution.main`, not - `current(bundle: .main)`.** The report inlines every notice, so decoding it is - not free, and its caller is a SwiftUI default argument — which Swift - re-evaluates on every `init`, and SwiftUI re-inits views constantly. `main` - caches the decode for the process; `current(bundle:)` stays for tests and for - any bundle that isn't `.main`. -- **Impossible states trap; recoverable ones surface.** `WhereStore` methods are - `async throws` so the CloudKit-backed store can report I/O failure; a `catch` - must log via a `WhereLog` typed `LogEvent` (PII-free, `.public`) and leave - state honest — never swallow into an empty default. This module owns the - `WhereLog` facade every Where module logs through; see the feature - [`AGENTS.md`](../AGENTS.md#layering) for the logging rules themselves. + `DataIssueInput.daySamples` carries per-day GPS fixes only (`.gpsVisit` / + `.gpsSignificantChange`, sorted) — manual and evidence-implied samples are + excluded so `FlightDayDetector`'s speed math isn't skewed. +- **`LocationSource` abstracts GPS** — `CoreLocationSource` in production, + `ScriptedLocationSource` in tests/previews; `requestCurrentLocation()` + returns `nil`, never throws, and backs + `LocationIngestor.captureTodayIfNeeded(now:)`. +- **Tracked regions live in the store, not preferences** — one + `SDTrackedRegion` row per region so cross-device edits merge; read as a + `Set` defaulting to the four. `RegionAttribution` derives the attributor + from them and rebuilds on `changes()`; assemble via the async + `WhereServices.make(...)` / `forIntents()` so both attribute against the + same synced set. `distanceToBoundary` is `nil` outside the tracked set. +- **Impossible states trap; recoverable ones surface.** `WhereStore` methods + are `async throws`; a `catch` logs a typed `WhereLog` event (PII-free, + `.public`, error as `LogAttachment.error(_:)`) and leaves state honest — + never a benign-looking default. The `WhereLog` facade and every + `*Log.swift` event type live together in `Sources/Logging/`. ## Testing -Swift Testing in [`Tests/`](Tests) (`WhereCoreTests`), hosted in `StuffTestHost`. -Drive collaborators against `SwiftDataStore.inMemory()` + `ScriptedLocationSource` -— never the on-disk/CloudKit store or `CoreLocationSource`. The CloudKit -remote-import path uses the `@_spi(Testing)` `inMemory(remoteChangeSource:)` + +Swift Testing in [`Tests/`](Tests) (`WhereCoreTests`), hosted in +`StuffTestHost`. Drive collaborators against `SwiftDataStore.inMemory()` + +`ScriptedLocationSource` — never the on-disk/CloudKit store or +`CoreLocationSource`. The CloudKit remote-import path uses the +`@_spi(Testing)` `inMemory(remoteChangeSource:)` + `ScriptedStoreRemoteChangeSource`. Internal types are reached via -`@testable import WhereCore`. `InMemoryKeyValueStore` (the `KeyValueStore` test -double) ships here behind `@_spi(Testing)` + `#if DEBUG` — not in a test-only -module — so it never ships in release; test bundles get it with -`@_spi(Testing) import WhereCore`. +`@testable import WhereCore`. `InMemoryKeyValueStore` (the `KeyValueStore` +test double) ships here behind `@_spi(Testing)` + `#if DEBUG`; test bundles +import it with `@_spi(Testing) import WhereCore`. diff --git a/Where/WhereIntents/AGENTS.md b/Where/WhereIntents/AGENTS.md index 1d487119..7341657b 100644 --- a/Where/WhereIntents/AGENTS.md +++ b/Where/WhereIntents/AGENTS.md @@ -1,105 +1,80 @@ # WhereIntents – Module Shape -WhereIntents is the App Intents layer of the Where feature: the query + action -intents (and their interactive snippet cards) that expose Where to Siri, -Spotlight, and the Shortcuts app. See [`README.md`](README.md) for the intent -list and data paths. +WhereIntents is the App Intents layer of the Where feature: the query + +action intents (and their interactive snippet cards) that expose Where to +Siri, Spotlight, and the Shortcuts app. See [`README.md`](README.md) for the +intent list and data paths. This file complements the root [`AGENTS.md`](../../AGENTS.md) and the feature -[`Where/AGENTS.md`](../AGENTS.md) — read those first (they own build/format, -layering, localization, and the WhereUI duplicate-metadata rule). +[`Where/AGENTS.md`](../AGENTS.md) — read those first. ## Scope & dependencies - Dependencies live in the root [`Package.swift`](../../Package.swift). It - depends on **WhereUI** for its snippet cards — mirroring **WhereWidgets** — - so it must **not** link `BroadwayUI`/`BroadwayCore` directly (see the root - [`AGENTS.md`](../../AGENTS.md#never-double-link-a-product-whereui-already-carries)). + depends on **WhereUI** for its snippet cards, so it must **not** link + `BroadwayUI`/`BroadwayCore` directly (root + [double-link rule](../../AGENTS.md#never-double-link-a-product-whereui-already-carries)). - Intents stay **thin adapters**: they `await intentServices.current()` and - delegate to that `WhereServices`' collaborators. Domain rules, persistence, - and aggregation stay in `WhereCore`; presentation (the card bodies) stays in - `WhereUI`. Don't reimplement any of that here. + delegate to that `WhereServices`' collaborators. Domain rules stay in + `WhereCore`, card bodies in `WhereUI`. ## Invariants - **The `AppShortcutsProvider` lives in the Where app target** - (`Where/Where/Sources/WhereShortcuts.swift`), not here, so App Intents - metadata extraction reliably discovers the phrases. Intent/entity/enum types - are `public` so it can reference them. + (`Where/Where/Sources/WhereShortcuts.swift`) so metadata extraction + reliably discovers the phrases; intent/entity types are `public` for it. - **Intents never start GPS.** `WhereServices.forIntents(sharingStoreOf:)` - swaps the session's location source for an `IdleLocationSource`; a manual - entry logged from an intent records a `ManualEntryAudit` with a "Logged with - Siri" note and no captured location. + wires an `IdleLocationSource`; an intent-logged manual entry records a + "Logged with Siri" audit and no captured location. - **Resolve services through the `@Dependency`-injected `IntentServices`; - intents never open a store.** The app's `AppDelegate` owns the one - `IntentServices` instance and registers it with `AppDependencyManager` in - `didFinishLaunching` (there is no singleton of ours); every intent and - entity query declares `@Dependency private var intentServices: - IntentServices`. The launch's `open-store` step is the process's *only* - store open: it hands the session's services to the composition root - (`WhereLaunch.makeLauncher`'s `onServicesReady` hook), which derives the - store-sharing intents stack (`WhereServices.forIntents(sharingStoreOf:)`) - and installs it — re-fired on retry and reset relaunches. Intents run over - the same store instance the app opened, so a `LogDayIntent` write pings the - same `changes()` signal the running UI refreshes from (and is immediately - visible when the day-count snippet reloads). An intent that fires before - installation **parks** in `current()` (cancellation-aware) rather than - assembling its own stack — there is deliberately no self-open fallback. -- **Use `Calendar.whereIntents` for all year/day math**, never `Calendar.current` - — it's Gregorian in the current time zone, matching `DayAggregator()`, so a - spoken "this year" lines up with the aggregated report even on a non-Gregorian - device calendar. `Calendar+WhereIntentsTests` guards the alignment. -- **Snippet `perform()` is side-effect-free and re-run on reload.** The - interactive `DaysInRegionSnippetIntent.perform()` only re-reads and re-renders; - its `Button(intent:)` runs a separate action intent (`LogDayIntent`) that - mutates, then the snippet reloads. Never mutate inside a `SnippetIntent`. -- **`Region` is exposed as `RegionEntity`, not an `AppEnum`.** App Intents - requires an `AppEnum`'s `caseDisplayRepresentations` (and any - `typeDisplayRepresentation`) to be compile-time-constant literals; an entity's - per-instance `displayRepresentation` is runtime, so it reads - `Region.localizedName` and RegionKit stays the single source of a region's + intents never open a store.** The app's `AppDelegate` owns the one instance + and registers it in `didFinishLaunching`; the launch's `open-store` step is + the process's only store open, and the `onServicesReady` hook derives and + installs the store-sharing intents stack (re-fired on retry and reset + relaunches). An intent that fires before installation **parks** in + `current()` (cancellation-aware) — there is deliberately no self-open + fallback. A `LogDayIntent` write therefore pings the same `changes()` + signal the running UI refreshes from. +- **Use `Calendar.whereIntents` for all year/day math**, never + `Calendar.current` — Gregorian in the current time zone, matching + `DayAggregator()`. Guard: `Calendar+WhereIntentsTests`. +- **Snippet `perform()` is side-effect-free and re-run on reload.** Mutation + goes through a separate action intent (`LogDayIntent`); never mutate inside + a `SnippetIntent`. +- **`Region` is exposed as `RegionEntity`, not an `AppEnum`** — an `AppEnum` + requires compile-time-constant display literals, and an entity's runtime + `displayRepresentation` keeps RegionKit the single source of a region's spelling. `RegionEntity`/`RegionEntityQuery` are `rawValue`-keyed. - **Suggestions and Spotlight surface the *tracked* set; resolution is - *full-catalog*.** `RegionEntityQuery.suggestedEntities()` and - `RegionSpotlightIndexer` read the user's tracked regions via - `RegionEntity.tracked(from:)` (→ `WhereServices.trackedRegions()`), while - `entities(for:)` resolves any available region by id (so a spoken untracked - region still answers, with a zero count). + *full-catalog*** — `suggestedEntities()` / `RegionSpotlightIndexer` read + `RegionEntity.tracked(from:)`, while `entities(for:)` resolves any region + by id (a spoken untracked region still answers, with a zero count). - **App Intents static metadata is literal; dialog copy is catalog-backed.** - Titles, parameter titles, and type/case display names are `LocalizedStringResource` - literals (the framework extracts/localizes them and requires constants). - Runtime `IntentDialog` copy goes through `IntentStrings`, which composes this - module's generated `LocalizedStringResource` symbols. -- **Only the `dialog.*` / `snippet.*` / `audit.*` keys are `manual`; leave the - rest of the catalog alone.** Symbol generation runs on `manual` keys, so a new - key `IntentStrings` reads must be marked `manual` to get a symbol. Do **not** - normalize the catalog wholesale: the App Intents metadata keys are the - framework's own extracted literals (keyed by their English text), and one of - them is `%@` — marking that `manual` fails the build with *"Unable to derive a - symbol name from this key. It only contains characters that are invalid in - Swift."* + Titles and display names are `LocalizedStringResource` literals (the + framework requires constants); runtime `IntentDialog` copy goes through + `IntentStrings`, which composes this module's generated symbols. +- **Only the `dialog.*` / `snippet.*` / `audit.*` keys are `manual`; leave + the rest of the catalog alone.** The other entries are the framework's own + extracted literals, and one of them is `%@` — marking that `manual` fails + the build with *"Unable to derive a symbol name from this key."* ## Testing Swift Testing in [`Tests/`](Tests) (`WhereIntentsTests`, hosted in `StuffTestHost`). Drive intent read/write logic against -`PreviewSupport.previewServices()` (in-memory, no-op schedulers) seeded via -`DayJournal` — never the on-disk store. Follows the WhereUITests dependency -shape (no `extraPackageProducts`; everything arrives transitively through -WhereUI). Internal types are reached via `@testable import WhereIntents`. +`PreviewSupport.previewServices()` seeded via `DayJournal` — never the +on-disk store. No `extraPackageProducts`; everything arrives transitively +through WhereUI. **Never call an intent's `perform()` in a test.** `perform()` resolves its -`@Dependency` from the process-wide `AppDependencyManager` — in +`@Dependency` from the process-wide `AppDependencyManager`: in `StuffTestHost`-hosted bundles nothing registers one (the resolution traps), -and in the app-hosted `WhereTests` process the host app's own launch *does* -register a handoff over its in-memory store, which an intent would silently -ride. Test the read/write logic against injected services, and test the -handoff itself on per-test `IntentServices` instances (see -`IntentServicesTests`). The registration→`@Dependency` plumbing itself is -**not unit-testable**: the framework fatal-errors on any `@Dependency` access -outside "the intent perform flow" (a probe was tried and trapped), so that -seam is verified by invoking a Siri/Shortcuts intent on a device. Also don't -add tests that construct an `AppDelegate` — each `didFinishLaunching` -re-registers the handoff, and `AppDependencyManager`'s re-registration -behavior is undocumented (the one existing launch test plus the host app's -own launch is the tolerated known case). +and in the app-hosted `WhereTests` process an intent would silently ride the +host app's own registration. Test read/write logic against injected +services, and the handoff on per-test `IntentServices` instances +(`IntentServicesTests`). The registration→`@Dependency` plumbing is not +unit-testable — the framework fatal-errors on any `@Dependency` access +outside the intent perform flow (a probe was tried and trapped); verify it by +invoking a Siri/Shortcuts intent on a device. Don't construct extra +`AppDelegate`s in tests: each `didFinishLaunching` re-registers the handoff, +and `AppDependencyManager`'s re-registration behavior is undocumented. diff --git a/Where/WhereUI/AGENTS.md b/Where/WhereUI/AGENTS.md index efacd9ae..d6a4c9c7 100644 --- a/Where/WhereUI/AGENTS.md +++ b/Where/WhereUI/AGENTS.md @@ -1,175 +1,93 @@ # WhereUI – Module Shape WhereUI is the SwiftUI layer of the Where feature: the screens, the shared -components and widget views, and the `@Observable` view models that orchestrate -`WhereCore` for them (`WhereModel`, the `WhereSession` coordinator, and the -scope-tiered `YearReportModel` / `ResolveModel` / `BackupModel` / -`RemindersSettingsModel`). Its layering, localization, preview, and testing -conventions live in the feature [`Where/AGENTS.md`](../AGENTS.md) — read that and -the root [`AGENTS.md`](../../AGENTS.md) first. This file adds only what those -don't cover: how the module's design system (`WhereStylesheet`) is used and -extended, and how its snapshot suites are organized (see -[Testing](#testing)). +components and widget views, and the `@Observable` view models that +orchestrate `WhereCore` for them (`WhereModel`, the `WhereSession` +coordinator, and the scoped `YearReportModel` / `ResolveModel` / +`BackupModel` / `RemindersSettingsModel`). Layering, localization, preview, +and testing conventions live in the feature [`Where/AGENTS.md`](../AGENTS.md) +— read that and the root [`AGENTS.md`](../../AGENTS.md) first. ## Scope & dependencies - Presentation layer only — no domain rules, persistence, or store I/O here - (see [Layering](../AGENTS.md#layering)). Dependencies live in the root + ([Layering](../AGENTS.md#layering)). Dependencies live in the root [`Package.swift`](../../Package.swift). -- Consumers (`WhereWidgets`, `WhereIntents`) get Broadway *through* WhereUI and - must **not** link `BroadwayUI`/`BroadwayCore` themselves (see the root - [`AGENTS.md`](../../AGENTS.md#never-double-link-a-product-whereui-already-carries)). - That's why `whereBroadwayRoot()` lives here rather than being called as +- Consumers (`WhereWidgets`, `WhereIntents`) get Broadway *through* WhereUI + and must **not** link `BroadwayUI`/`BroadwayCore` themselves (root + [double-link rule](../../AGENTS.md#never-double-link-a-product-whereui-already-carries)); + that's why `whereBroadwayRoot()` lives here rather than being called as `broadwayRoot` at each site. - Continuous/looping motion (repeat-forever pulses, `TimelineView(.animation)`, typewriter reveals) must consult the shared `@MotionIsStatic` helper ([`Sources/Shared/MotionIsStatic.swift`](Sources/Shared/MotionIsStatic.swift)) for its static end-state — never hand-roll the `\.accessibilityReduceMotion` + `\.isCapturingSnapshot` pair. -- A compact `DatePicker` in a form goes through `WhereDatePicker` +- A compact form `DatePicker` goes through `WhereDatePicker` ([`Sources/Shared/WhereDatePicker.swift`](Sources/Shared/WhereDatePicker.swift)), - which substitutes a deterministic stand-in under capture — the live control's - value capsule renders relative to *today's* date, so no reference containing - one is stable across days. Views shouldn't read `\.isCapturingSnapshot` to - branch themselves; keep the capture handling inside the shared component. + which substitutes a deterministic stand-in under capture — the live control + renders relative to *today*, so no reference containing one is stable across + days. Views don't read `\.isCapturingSnapshot` to branch themselves; capture + handling stays inside the shared component. ## Design system — `WhereStylesheet` +All appearance tokens — geometry, fonts, colors, motion — live in `WhereStylesheet` -([`Sources/Shared/WhereStylesheet.swift`](Sources/Shared/WhereStylesheet.swift)) -is the single home for the app's appearance tokens — geometry, fonts, colors, -motion — expressed as a Broadway `BStylesheet`. It replaced the scattered inline -literals and the former `UIConstants` bag: a new appearance value belongs here, -not back inline in a view. - -### Using tokens - -- Read tokens in a view with `@Environment(\.stylesheet) private var - stylesheet` (e.g. `stylesheet.spacing.medium`). The active sheet is seeded - by `whereBroadwayRoot()` at the app root and in each Broadway-root-less - consumer (WhereWidgets); with no root present (isolated previews, code off - the `View` tree) resolution falls back to `WhereStylesheet.default` — use - that static directly in layout helpers and tests. -- **Resolve a variant once.** For a component with more than one look, vend a - resolved sub-spec and read it into a single property rather than branching - through the body: `RegionSummaryCard` reads `stylesheet.card[variant]` into a - `card` so its render is straight-line, with no `compact ? … : …` scattered - across ~30 values. - -### Adding tokens — prefer per-component style groups - -Group a component's whole appearance into one nested `Equatable` struct instead -of adding loose properties to the top level. The stored properties declared at -the top of `WhereStylesheet` are the live list of groups — read them there -rather than trusting a copy here. Two are worth copying as templates: -`CardStyles` (a variant axis behind a `subscript`) and `CalendarStyle` (nested -sub-parts). To add one: - -1. Define the struct in a `WhereStylesheet` extension with a doc comment saying - which component it styles and any invariants; nest further structs for - sub-parts (e.g. `CalendarStyle.MonthStyle`, `AppIconStyle.PanelStyle`). -2. Give it a `static let standard` holding the fixed geometry, and add a stored - property on `WhereStylesheet` defaulted to it. -3. If a look varies (the `compact` card), model the axis as a `Variant` enum and - expose a `subscript` on the styles struct (see `CardStyles`) so callers read - one resolved spec — don't thread a `Bool` through the view. - -Reach for a shared group only for genuinely cross-component values: the generic -point scale on `Spacing`, one-off element sizes on `Size`, app-wide colors not -owned by a single component on `Palette`, the few bespoke display faces on -`Typography`, and animation tokens on `Motion`. Per-region tints stay in -`RegionStyle` (not the stylesheet); adaptive system roles (`.secondary`) and -`.accentColor` stay inline. - -`RegionStyle` is **data-driven** and resolved through the environment: views -read `@Environment(\.regionStyles)` (a `RegionStyleResolver`) and call -`regionStyles.style(for: region)` — there is no global `region.style`. The -resolver is seeded by `whereBroadwayRoot(regionStyles:)`: the app passes -`WhereSession`'s live resolver (updated on launch + `changes()`), the widget -process one built from its `WidgetSnapshot`, and App Intents snippets one from -their services; the default empty resolver yields the fallback looks -(`RegionAppearanceCatalog.defaultAppearance(for:)`) for previews and the -region-map viewer. The catalog also owns the selectable color/emoji/symbol -option lists the picker shows. Don't reintroduce a global accessor or a -hardcoded per-region look in a view. - -### Trait-aware tokens - -Most tokens are fixed, but a slice derives from the `BContext` traits in -`init(context:)`. Start from the fixed set (property defaults / `.standard`), -then adjust only the reactive slice, so a default/system context reproduces -`WhereStylesheet.default`. Read the live set off `init(context:)`; today it grows -day-grid tap targets at accessibility Dynamic Type sizes, flattens the card glow -under Reduce Transparency, and crossfades the cards' day count under Reduce -Motion. - -**Prefer deriving an accessibility setting here over reading it in the view.** A -view reaching for `@Environment(\.accessibilityReduceMotion)` to choose between -two token sets is doing the sheet's job — vend one resolved token instead, and -make it a *single* token when the setting changes more than one value -(`CardStyles.DayCountStyle` pairs the count's morph with the animation that runs -it, because Reduce Motion swaps both). The `motion` group keeps the older -shape — full-motion values a view picks between (`motion.reducedReveal` over -`motion.reveal`, skipping `motion.captionFade`) — because the launch reveal's -fallback also swaps an `AnyTransition`, which isn't `Equatable` and so can't be a -token. - -`WhereThemes` is deliberately empty for now — the sheet derives from traits, not -themes. It is the seam a future app-wide or seasonal palette/typography theme -would plug into. - -### What not to do - -- **Don't borrow another component's style.** A component must never read a value - off another component's group to dodge defining its own — e.g. `TimelineStyle` - does not reach into `CardStyle` for a corner radius. If a component needs a - value, it gets its own property (or its own style struct); the cost of adding - a struct is the point, not something to route around. Only genuinely - cross-cutting values belong on the shared scales (`Spacing`, `Size`, `Palette`, - `Typography`, `Motion`) — and a token there should read as shared, not as one - component quietly depending on another's geometry. -- **Don't hardcode appearance in a view**, and don't collect unrelated constants - into a flat grab-bag (the old `UIConstants` smell). A new geometry / font / - color / motion value lands on the owning component group or a shared scale. -- **Don't branch a variant through the body.** Model the axis as a `Variant` enum - plus a `subscript` and read one resolved spec (see [Using - tokens](#using-tokens)) — don't scatter `compact ? … : …` or thread a `Bool` - down the view. -- **Don't bake trait-derived values into the defaults.** `.standard` / property - defaults hold the fixed set; apply the reactive slice only in `init(context:)`, - so a default/system context still reproduces `WhereStylesheet.default`. -- **Don't put per-region or adaptive-system colors in the sheet.** Per-region - tints stay in `RegionStyle`; adaptive system roles (`.secondary`) and +([`Sources/Shared/WhereStylesheet.swift`](Sources/Shared/WhereStylesheet.swift)), +a Broadway `BStylesheet` read via `@Environment(\.stylesheet)`; off the +`View` tree (layout helpers, tests) use `WhereStylesheet.default`. How to +consume and extend it — per-component style groups, variant subscripts, the +`RegionStyle` resolver — is in [`README.md`](README.md#design-system). The +rules: + +- **Never hardcode appearance in a view** or collect constants into a flat + grab-bag; a new value lands on the owning component's style group, or on a + shared scale (`Spacing`, `Size`, `Palette`, `Typography`, `Motion`) only + when genuinely cross-component. +- **Never borrow another component's style** — a component defines its own + group rather than reading a value off someone else's. +- **Resolve a variant once** (a `Variant` enum + `subscript`, see + `CardStyles`) — don't branch `compact ? … : …` through a body. +- **Don't bake trait-derived values into the defaults** — `.standard` / + property defaults hold the fixed set; the reactive slice applies only in + `init(context:)`, so a default/system context reproduces + `WhereStylesheet.default`. +- **Derive accessibility settings in the sheet, not the view** — vend one + resolved token, and a *single* token when a setting changes more than one + value (`CardStyles.DayCountStyle` pairs the morph with its animation). + Exception: the `motion` group keeps full-motion values a view picks between + (`motion.reducedReveal` over `motion.reveal`), because the launch reveal's + fallback swaps an `AnyTransition`, which isn't `Equatable` and can't be a + token. +- **Per-region tints stay in `RegionStyle`**, resolved via + `@Environment(\.regionStyles)` and seeded by + `whereBroadwayRoot(regionStyles:)` — no global accessor or hardcoded + per-region look in a view. Adaptive system roles (`.secondary`) and `.accentColor` stay inline. +- `WhereThemes` is deliberately empty — the seam a future app-wide theme + plugs into. ## Testing -`WhereStylesheetTests` pins every token's default value and the trait-aware -derivations (resolved synchronously off a `BContext`); -`WhereStylesheetEnvironmentTests` covers the `@Environment(\.stylesheet)` glue -and the `whereBroadwayRoot()` seeding, including the WhereWidgets path. Adding, -renaming, or retuning a token means updating those assertions in the same -change. Broader WhereUI testing conventions (hosted bundles, `PreviewSupport`, -required previews) live in the feature [`Where/AGENTS.md`](../AGENTS.md). - -Screens, widgets, and app-flow surfaces are pinned as matrixed image snapshots -under [`SnapshotTests/`](SnapshotTests) — those, not hosting smoke tests, own -"does this screen render". They build as this module's own -`WhereUISnapshotTests` bundle, which runs alongside the other modules' image -suites in the shared `StuffSnapshotTests` scheme and its CI job (root -[`AGENTS.md`](../../AGENTS.md#targets)). **Each view -declares its matrix once, in its own source file**, via a `SnapshotProviding` -conformance under `#if DEBUG` whose `#Preview` renders `Self.snapshotPreviews` — -so one declaration drives both the Xcode cutsheet and the image tests (the -`whereSnapshot(...)` helper + config presets live in -[`Sources/Preview/WhereSnapshot.swift`](Sources/Preview/WhereSnapshot.swift)). -The suites are **one `FooSnapshotTests` per view**, each in its own file calling -`assertSnapshots(of: FooView.self)`, so each view's references sit in their own -`__Snapshots__//` directory. They run in the -`StuffSnapshotTests` scheme and its CI job, deliberately outside -`Stuff-iOS-Tests` (see `Project.swift`). To re-record a reference, delete the PNG -under `SnapshotTests/__Snapshots__/` (LFS-tracked) and run the scheme — the -suites record `.missing`, and a recording run fails by design so it can't pass as -green. (Bulk re-records after an intentional UI change can instead forward -`TEST_RUNNER_SNAPSHOT_RECORD=failed` — see the -[SnapshotKitTesting README](../../Shared/SnapshotKitTesting/README.md#recording).) +`WhereStylesheetTests` pins every token default and trait-aware derivation; +`WhereStylesheetEnvironmentTests` covers the `@Environment(\.stylesheet)` +glue and `whereBroadwayRoot()` seeding, including the WhereWidgets path. +Adding, renaming, or retuning a token means updating those assertions in the +same change. + +Screens, widgets, and app-flow surfaces are pinned as matrixed image +snapshots under [`SnapshotTests/`](SnapshotTests) — those, not hosting smoke +tests, own "does this screen render". They build as this module's +`WhereUISnapshotTests` bundle, run from the shared `StuffSnapshotTests` +scheme and its CI job, deliberately outside `Stuff-iOS-Tests` (root +[`AGENTS.md`](../../AGENTS.md#targets)). **Each view declares its matrix +once, in its own source file**, via a `SnapshotProviding` conformance under +`#if DEBUG` whose `#Preview` renders `Self.snapshotPreviews` — one +declaration drives both the Xcode cutsheet and the image tests (helpers in +[`Sources/Preview/WhereSnapshot.swift`](Sources/Preview/WhereSnapshot.swift)); +suites are one `FooSnapshotTests` per view. To re-record a reference, delete +the PNG under `SnapshotTests/__Snapshots__/` (LFS-tracked) and run the scheme +— the suites record `.missing`, and a recording run fails by design. Bulk +re-records forward `TEST_RUNNER_SNAPSHOT_RECORD=failed` (see the +[SnapshotKitTesting README](../../Shared/SnapshotKitTesting/README.md#recording)). diff --git a/Where/WhereUI/README.md b/Where/WhereUI/README.md index c066a5f0..824cef96 100644 --- a/Where/WhereUI/README.md +++ b/Where/WhereUI/README.md @@ -117,14 +117,62 @@ model and a foreground launch runner. Appearance tokens — geometry, fonts, colors, motion — live in one place, `WhereStylesheet`, a Broadway `BStylesheet` resolved from the environment. Views read it with `@Environment(\.stylesheet)`; off the `View` tree (layout helpers, -tests) code uses `WhereStylesheet.default`. Tokens are grouped per component -(`CalendarStyle`, `AppIconStyle`, `CardStyle`, …) with shared scales for the -cross-cutting bits (`Spacing`, `Palette`, `Typography`, `Motion`). Most values -are fixed; a slice derives from accessibility traits (bigger tap targets at -large Dynamic Type, a flatter card under Reduce Transparency, a crossfaded -rather than rolling day count under Reduce Motion). See -[`AGENTS.md`](AGENTS.md#design-system--wherestylesheet) for how to consume and -extend it. +tests) code uses `WhereStylesheet.default`. The active sheet is seeded by +`whereBroadwayRoot()` at the app root and in each Broadway-root-less consumer +(WhereWidgets); with no root present, resolution falls back to `.default`. +The rules for what may and may not live in the sheet are in +[`AGENTS.md`](AGENTS.md#design-system--wherestylesheet). + +### Using tokens + +For a component with more than one look, resolve the variant once: vend a +resolved sub-spec and read it into a single property rather than branching +through the body. `RegionSummaryCard` reads `stylesheet.card[variant]` into a +`card` so its render is straight-line, with no `compact ? … : …` scattered +across ~30 values. + +### Adding tokens — per-component style groups + +Group a component's whole appearance into one nested `Equatable` struct +instead of adding loose properties to the top level. The stored properties +declared at the top of `WhereStylesheet` are the live list of groups; two are +worth copying as templates: `CardStyles` (a variant axis behind a `subscript`) +and `CalendarStyle` (nested sub-parts). To add one: + +1. Define the struct in a `WhereStylesheet` extension with a doc comment + saying which component it styles and any invariants; nest further structs + for sub-parts (e.g. `CalendarStyle.MonthStyle`, `AppIconStyle.PanelStyle`). +2. Give it a `static let standard` holding the fixed geometry, and add a + stored property on `WhereStylesheet` defaulted to it. +3. If a look varies (the `compact` card), model the axis as a `Variant` enum + and expose a `subscript` on the styles struct so callers read one resolved + spec. + +Reach for a shared group only for genuinely cross-component values: the +generic point scale on `Spacing`, one-off element sizes on `Size`, app-wide +colors not owned by a single component on `Palette`, the few bespoke display +faces on `Typography`, and animation tokens on `Motion`. + +### Trait-aware tokens + +Most tokens are fixed; a slice derives from the `BContext` traits in +`init(context:)` — read the live set off that initializer. Today it grows +day-grid tap targets at accessibility Dynamic Type sizes, flattens the card +glow under Reduce Transparency, and crossfades the cards' day count under +Reduce Motion. + +### Per-region styling + +`RegionStyle` is data-driven and resolved through the environment: views read +`@Environment(\.regionStyles)` (a `RegionStyleResolver`) and call +`regionStyles.style(for: region)`. The resolver is seeded by +`whereBroadwayRoot(regionStyles:)`: the app passes `WhereSession`'s live +resolver (updated on launch + `changes()`), the widget process one built from +its `WidgetSnapshot`, and App Intents snippets one from their services; the +default empty resolver yields the fallback looks +(`RegionAppearanceCatalog.defaultAppearance(for:)`) for previews and the +region-map viewer. The catalog also owns the selectable color/emoji/symbol +option lists the picker shows. ## Previews From 9245eb20a8643abac50098eca107352e14745758 Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Mon, 27 Jul 2026 01:56:46 -0700 Subject: [PATCH 7/8] Rewrite the Shared-side AGENTS.md files to what-not-why MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plan step: Rewrite Shared-side AGENTS.md files to what-not-why. Fact-neutral rewrite per the codified policy — every rule, tripwire, and guard-test pointer survives; mechanism essays and history move to their proof (Project.swift comments, PR #145, source doc comments, TODOs.md). - SnapshotKitTesting 129 -> 81: the process-model and double-embed essays become rules + tripwire, pointing at the snapshot-bundle comment in Project.swift (the one home for the topology); every guard test stays named (ConcurrentCaptureTests, LargeViewCaptureTests, SnapshotRenderingSupportTests, SnapshotCaptureFlagProbeTests). - PeriscopeCore 100 -> 77: each invariant keeps its one consequence clause; explanatory tails trimmed. - PeriscopeTools 109 -> 73: layout catalog collapsed to a line; design system keeps its rules; incremental-refresh invariant keeps the O(accumulated) note and TODO pointer. - CreditKit 81 -> 54: invariants keep their consequence clause, lose the persuasion. - LifecycleKitUI 64 -> 53 and LifecycleKit 87 -> 78: view-identity and identity-domain rules compressed; the deliberate-cost notes and the displayedSurfaceIdentity trap stay. - SnapshotKit 56 -> 50: hybrid-accessor mechanics now point at SnapshotCaptureFlag.swift's own docs, keeping the don't-simplify rule. - SwiftDataInspector, TestHostSupport, Periscope group, Broadway group: light trims (deleted-module history, meta sections, root-owned wiring restatements). - Untouched, already at the standard: StuffTestHost, JournalKit, BroadwayCore/UI/Catalog, PeriscopeUI, StuffCore, JournalBenchmark. No cross-file anchors point into the edited files; verified by grep. --- Shared/Broadway/AGENTS.md | 7 +- Shared/CreditKit/AGENTS.md | 95 +++++-------- Shared/LifecycleKit/AGENTS.md | 42 +++--- Shared/LifecycleKitUI/AGENTS.md | 51 +++---- Shared/Periscope/AGENTS.md | 6 - Shared/Periscope/PeriscopeCore/AGENTS.md | 120 +++++++---------- Shared/Periscope/PeriscopeTools/AGENTS.md | 107 +++++---------- Shared/SnapshotKit/AGENTS.md | 22 ++- Shared/SnapshotKitTesting/AGENTS.md | 157 ++++++++-------------- Shared/SwiftDataInspector/AGENTS.md | 22 ++- Shared/TestHostSupport/AGENTS.md | 16 +-- 11 files changed, 237 insertions(+), 408 deletions(-) diff --git a/Shared/Broadway/AGENTS.md b/Shared/Broadway/AGENTS.md index 49e6741b..c24fce69 100644 --- a/Shared/Broadway/AGENTS.md +++ b/Shared/Broadway/AGENTS.md @@ -1,9 +1,8 @@ # Broadway – Module Group Shape -Broadway is a design-system stack imported into Stuff (git history preserved) -from its own repo. It centers on `BContext` — a type-keyed environment (traits, -themes, lazily-cached stylesheets) that flows through a UIKit + SwiftUI view -hierarchy. See [`README.md`](README.md). +Broadway is a design-system stack centered on `BContext` — a type-keyed +environment (traits, themes, lazily-cached stylesheets) that flows through a +UIKit + SwiftUI view hierarchy. See [`README.md`](README.md). This file complements the root [`AGENTS.md`](../../AGENTS.md), which owns build, formatting, and global conventions. Read that first. diff --git a/Shared/CreditKit/AGENTS.md b/Shared/CreditKit/AGENTS.md index 1c6fe02c..6c9219ee 100644 --- a/Shared/CreditKit/AGENTS.md +++ b/Shared/CreditKit/AGENTS.md @@ -7,74 +7,47 @@ the root [`AGENTS.md`](../../AGENTS.md). ## Scope & dependencies -- **May import:** Foundation. Nothing else — not even logging. -- **Must not import:** any app or feature module (`WhereCore`, `WhereUI`, …), or - any UI framework. CreditKit is a leaf that anything may depend on; an edge - pointing *out* of it would defeat the reason it exists. -- **Wired in:** `Package.swift` (`CreditKit` product, consumed by `WhereCore` - and `WhereUI`) and `Project.swift` (`CreditKitTests`, in the - `Stuff-iOS-Tests` scheme). - -## Layering - -`SoftwareCredit` and `LicenseNotice` are the values; `AttributionManifest` is a -decoded report plus the two ways to get one (`decode(from:)`, -`load(from:resource:)`). Nothing holds state, nothing is a singleton, and -nothing knows how a report is presented — grouping, labeling, and translation -belong to the consuming UI. - -`Tools/generate-attribution.rb` is the other half of the module and is the only -thing that writes a report. +- **May import:** Foundation. Nothing else — not even logging. CreditKit is a + leaf that anything may depend on. +- **Must not import:** any app or feature module, or any UI framework. +- **Wired in:** `Package.swift` (`CreditKit` product) and `Project.swift` + (`CreditKitTests`, in the `Stuff-iOS-Tests` scheme). Presentation belongs to + the consuming UI; `Tools/generate-attribution.rb` is the only thing that + writes a report. ## Invariants - **CreditKit ships no credits and no notices.** A report describes one app's - dependency graph, so it belongs in that app's resources — for Where, in - `Where/Where/Resources/attribution.json`. If a license file or a `credits.json` - ever reappears under `Sources/`, the module has drifted back into being one - app's data. -- **Nothing here may name a real dependency.** `CreditKitTests` covers the - format and the API with fixtures only; asserting that some package is credited - is the *app's* test to write, because only the app knows what it links. See - `AppAttributionTests` in `Where/Where/Tests/`. -- **Failure is thrown, never logged or defaulted.** The module has no logger by - design, and a missing report is not inherently an error — only the app knows - which of its bundles are expected to carry one. Returning an empty manifest - instead of throwing would render as "nothing to credit", which is the one - wrong answer. -- **`Kind` is load-bearing, not decoration.** `.developmentTool` credits are not - in the shipping binary. A UI that renders both kinds in one undifferentiated - list would misrepresent the app, so the distinction must survive into the - presentation. Its raw values are a wire format the generator writes; renaming - a case silently invalidates every committed report. The generator validates - each source's `kind` against them before it does any work, so a config typo - fails there rather than as a decode fault inside the app. -- **Credit names are unique across a report.** `SoftwareCredit` is `Identifiable` - by `name`, so a duplicate hands a SwiftUI `ForEach` two rows with one id — and - a library's name is its repo basename, so two orgs publishing the same repo - name is all it takes. The generator enforces this (case-insensitively) over the - assembled report, since that is the only place holding every credit at once. -- **Notices are read at the pinned revision.** Not the default branch — upstream - edits a notice between releases, and shipping HEAD's text would attribute - terms that don't govern the code in the binary. -- **The generator keys off `.product(name:package:)`, not the `dependencies:` - list.** That is what keeps a package resolved for tooling alone (BumperBowling, - and swift-syntax beneath it) out of a report by construction. -- **Linking is not shipping, so `kind` is derived from reachability.** The - config's `shippedFrom` names the app's root package targets; the generator - walks the manifest's target graph from there, and a package inside that - closure is a `library` while any other linked package is a `developmentTool`. - A test-support target's dependencies (the snapshot engine, the accessibility - parser) are linked by the package but never reach a device — crediting them as - libraries would misdescribe the binary, which is the one thing `Kind` exists to - prevent. `shippedFrom` is the only hand-set part; everything downstream of it - follows from the graph, so a new dependency can't be silently misfiled. + dependency graph and lives in that app's resources (for Where, + `Where/Where/Resources/attribution.json`) — never under `Sources/` here. +- **Nothing here may name a real dependency.** `CreditKitTests` uses fixtures + only; asserting that some package is credited is the app's test + (`AppAttributionTests` in `Where/Where/Tests/`). +- **Failure is thrown, never logged or defaulted** — an empty manifest would + render as "nothing to credit", the one wrong answer. Only the app knows + which of its bundles should carry a report. +- **`Kind` is load-bearing** — a UI must keep `.developmentTool` and library + credits visually distinct. Its raw values are a wire format; renaming a case + invalidates every committed report. The generator validates each source's + `kind` up front so a config typo fails there, not as a decode fault in-app. +- **Credit names are unique across a report** (enforced case-insensitively by + the generator) — `SoftwareCredit` is `Identifiable` by `name`, and a + library's name is its repo basename. +- **Notices are read at the pinned revision**, never the default branch — + HEAD's text may not govern the code in the binary. +- **The generator keys off `.product(name:package:)`, not `dependencies:`** — + that keeps tooling-only packages (BumperBowling, swift-syntax) out of a + report by construction. +- **`kind` is derived from reachability, not declared.** `shippedFrom` names + the app's root package targets; anything inside that closure is a `library`, + any other linked package a `developmentTool` — linking is not shipping. + `shippedFrom` is the only hand-set part. ## Testing -`CreditKitTests` covers the manifest as a format and an API: decoding the exact -JSON the generator writes, rejecting a malformed report or an unknown `kind`, -round-tripping, filtering by kind, and `load` throwing for a bundle with no +`CreditKitTests` covers the manifest as a format and an API: decoding the +exact JSON the generator writes, rejecting malformed reports and unknown +`kind`s, round-tripping, filtering, and `load` throwing for a bundle with no report. Shared fixtures live in `CreditKitTestSupport.swift`; its `SampleReport.json` (a string constant on the `SampleReport` enum, not a fixture file) is a literal rather than an encoder round-trip so a Swift-side diff --git a/Shared/LifecycleKit/AGENTS.md b/Shared/LifecycleKit/AGENTS.md index 8aa67648..d43cdba1 100644 --- a/Shared/LifecycleKit/AGENTS.md +++ b/Shared/LifecycleKit/AGENTS.md @@ -27,18 +27,13 @@ system, formatting, and global conventions. Read that first. proxy seam); their generic constraints guarantee every internal cast. Never add a second erasure site or a public API that traffics in `Any`. - **One identity domain per plan.** `LaunchPlan` is generic over - `ID: Hashable & Sendable` and every combinator requires the node's - `ID` to match, so a plan can't mix domains and a node keyed for another - plan can't be composed in; `nodeIDs` gives back `[ID]`, not erased keys. - IDs erase to `AnyHashable` *inside* `LaunchPlanNode` and stay erased from - there on — the runner's memo, `LifecycleFailure.stepID`, - `LifecycleStepContext.stepID`, and `LifecycleGateHandle.id` are all - `AnyHashable`, deliberately: `LifecycleDriving` (the seam behind the - non-generic `\.lifecycle` environment value) traffics in `[LaunchPlanNode]`, - so pushing `ID` past the plan would force it onto the runner, the container's - generic list, and every splash/failure/gate closure. If you want typed - `failed(at:)` / `isRunning(_:)` assertions, that's the (deliberate) cost to - price in — it isn't an oversight. + `ID: Hashable & Sendable` and every combinator requires matching `ID`s, so + a plan can't mix domains; `nodeIDs` gives back `[ID]`, not erased keys. + IDs erase to `AnyHashable` *inside* `LaunchPlanNode` and deliberately stay + erased from there on (the runner's memo, `LifecycleFailure.stepID`, + `LifecycleGateHandle.id`) — pushing `ID` past the plan would force it onto + the runner, the container, and every splash/failure/gate closure. Untyped + `failed(at:)` assertions are the priced-in cost, not an oversight. - **Only pass-through positions may skip.** Value-producing (`init`/`then`) steps must keep `modes == .all` (plan-construction `precondition`) — a skipped producer would leave a hole in the data flow. Don't add a skip path @@ -63,19 +58,16 @@ system, formatting, and global conventions. Read that first. - **Detached children are off the critical path by construction:** they never block `.ready`, never fail the drive, and surface failures only on `detachedFailures`. -- **`.undetermined` is the honest UIScene launch reason.** Under the UIScene - lifecycle `UIApplication.applicationState` reads `.background` at - `didFinishLaunching` even for a user tap, so a consumer that can't yet tell a - headless wake from a user launch should launch `.undetermined` rather than - fabricate a `.background(cause)`. It gates to the background-safe nodes and - builds no view tree until `enterForeground()` promotes it; if no scene ever - connects it honestly stays `.undetermined`, never claiming a cause it didn't - observe. -- **Promotion resolves a not-yet-foreground launch and is idempotent.** - `enterForeground()` no-ops once the reason *is* `.userForeground`, so a - repeat call costs nothing while `.background` and `.undetermined` both - promote; consumers must only call it once the scene is genuinely `.active` - (see `RootView` in WhereUI for the `scenePhase` gating pattern). +- **`.undetermined` is the honest UIScene launch reason** — under UIScene, + `UIApplication.applicationState` reads `.background` at `didFinishLaunching` + even for a user tap, so launch `.undetermined` rather than fabricate a + `.background(cause)`. It gates to the background-safe nodes and builds no + view tree until promoted; if no scene ever connects it honestly stays + `.undetermined`. +- **Promotion is idempotent.** `enterForeground()` promotes `.background` and + `.undetermined` and no-ops on `.userForeground`; call it only once the + scene is genuinely `.active` (see `RootView` in WhereUI for the + `scenePhase` gating pattern). ## Testing diff --git a/Shared/LifecycleKitUI/AGENTS.md b/Shared/LifecycleKitUI/AGENTS.md index f5065407..2925efa4 100644 --- a/Shared/LifecycleKitUI/AGENTS.md +++ b/Shared/LifecycleKitUI/AGENTS.md @@ -21,39 +21,28 @@ build system, formatting, and global conventions. Read that first. ## Invariants - **`content` is only ever built from `.ready`'s carried value** — never - re-read from shared state. Don't add a code path that renders the app - surface without the launch output in hand. It is built as soon as that value - exists (from `phase.readyValue`), *including while a splash hold still covers - it*, so the hold warms the destination instead of paying for its `.task`s and - first layout in the frame the reveal starts. Keep it to **one** `content` call - site: rendering it from separate held/revealed branches gives SwiftUI two - identities and rebuilds the whole destination at the reveal, defeating that. + re-read from shared state. It is built as soon as the value exists, + *including under a splash hold* (the hold warms the destination). Keep it to + **one** `content` call site — separate held/revealed branches give SwiftUI + two identities and rebuild the destination at the reveal. - **No view tree when `reason.buildsNoViewTree`** — even at `.ready`. -- **Every splash-showing state resolves to one `LaunchOverlay.splash` case,** so - the splash keeps a single identity across `.launching` → each step → the hold. - Don't render it from per-phase `switch` arms: SwiftUI would treat each arm's - splash as a different view and remount it at every boundary, resetting the - animations and caption timers a long-lived splash (e.g. Where's - `LaunchSplashView`) documents as uninterrupted. -- **`minimumSplashDuration` only holds a splash that was actually shown.** The - hold is armed when the splash *appears*, so a launch already `.ready` when the - container mounts reveals immediately rather than stalling behind a minimum for - a splash nobody saw (`minimumSplashDurationDoesNotHoldWhenNoSplashWasShown` - guards this; the timing half is device-verified, not host-testable). Assert - "revealed" via the *absent splash*, not via `content` — content is built - during a hold too, so it no longer distinguishes the two. `isShowingSplash` - must read the runner's own surface, never `displayedSurfaceIdentity` — that - reports `.splash` for a held `.ready`, which would re-arm the hold from its own - release and never reveal. -- **Gate views resolve only their own handle.** The registry hands each gate - view the parked `LifecycleGateHandle`; a superseded drive's handle no-ops, - so don't route gate resolution through anything but the handle. +- **Every splash-showing state resolves to one `LaunchOverlay.splash` case** — + never per-phase `switch` arms, which remount the splash at each boundary + and reset its animations and caption timers. +- **`minimumSplashDuration` only holds a splash that was actually shown** — + armed when the splash *appears*, so an already-`.ready` mount reveals + immediately. Guard: `minimumSplashDurationDoesNotHoldWhenNoSplashWasShown` + (the timing half is device-verified, not host-testable). Assert "revealed" + via the *absent splash*, not via `content` (content is built during a hold + too); `isShowingSplash` must read the runner's own surface, never + `displayedSurfaceIdentity`, which reports `.splash` for a held `.ready` and + would re-arm the hold from its own release. +- **Gate views resolve only their own handle** — a superseded drive's handle + no-ops; don't route gate resolution through anything else. - **One registration per gate type** (construction `precondition`); a parked - gate with no registration is a programmer error — the container logs it - (`os`, subsystem `com.stuff.lifecyclekitui`) and fails the gate's handle - with `MissingGateViewError`, landing on the failure surface (visible and - named (though terminal), identical in debug and release) rather than an indefinite - splash. + gate with no registration logs (`os`, subsystem `com.stuff.lifecyclekitui`) + and fails the handle with `MissingGateViewError` onto the terminal failure + surface — never an indefinite splash. ## Testing diff --git a/Shared/Periscope/AGENTS.md b/Shared/Periscope/AGENTS.md index 95b25845..4d66467a 100644 --- a/Shared/Periscope/AGENTS.md +++ b/Shared/Periscope/AGENTS.md @@ -42,12 +42,6 @@ target and no CI job. in-memory store per test and pass it explicitly (`Log()` defaults to `.shared`, so an omitted `system:` silently joins the process-wide one). -## Open work - -Design and follow-up work spanning the modules lives in one place: -[`TODOs.md`](TODOs.md) — file deferred items there rather than in a per-module -doc. - ## Testing Hosted Swift Testing bundles (`PeriscopeCoreTests`, `PeriscopeUITests`, diff --git a/Shared/Periscope/PeriscopeCore/AGENTS.md b/Shared/Periscope/PeriscopeCore/AGENTS.md index 3e1932d8..acb09f67 100644 --- a/Shared/Periscope/PeriscopeCore/AGENTS.md +++ b/Shared/Periscope/PeriscopeCore/AGENTS.md @@ -11,89 +11,67 @@ the build system, formatting, and global conventions. Read that first. ## Scope & dependencies - **Foundation + os + SwiftData + Network + CryptoKit + JournalKit only** - (plus the ObjectiveC runtime, for `LogContextProviding`'s deallocation - trackers and `NotificationAmbientSource`'s target/selector observation). - CryptoKit is used by exactly one file — `ScopeID.swift`, for the - deterministic scope hash. No SwiftUI, no app code. UIKit is allowed **only** - inside `#if canImport(UIKit)` (ambient sources, the image-attachment - convenience). + (plus the ObjectiveC runtime for deallocation trackers and target/selector + observation; CryptoKit is used only by `ScopeID.swift`). No SwiftUI, no app + code. UIKit only inside `#if canImport(UIKit)`. - Layering: `PeriscopeUI` and `PeriscopeTools` depend on this module — never the reverse. ## Invariants - **Emitting never blocks the caller.** Log calls append to a lock-guarded - buffer synchronously; sinks (OSLog, SwiftData) drain asynchronously, and - sink delivery order is emission order with scope definitions first. - Live streams see buffered order too: observer yields happen *under* the - state lock (yields only buffer) — yielding outside it would let racing - emitters invert live delivery, e.g. a span's end before its began. -- **Scope IDs are deterministic** (hash of parent + name) — the same path is - the same scope across processes and launches; `begin`/`end` span pairing - and cross-layer links rely on this. -- **`sequence` is store-global and monotonic.** Every write takes the next - value past the highest stored (resuming across launches), so a freshly - inserted event always outranks everything already persisted. That is what - makes `LogQuery.afterSequence` a valid incremental cursor: "sequence > - what I last merged" is exactly "everything appended since", and live - viewers advance it instead of re-reading the store each commit. -- **Persistence must retain the full hierarchy** — events reference scopes - many-to-many (links), and scopes keep their parent chain. + buffer synchronously; sinks drain asynchronously in emission order, scope + definitions first. Observer yields happen *under* the state lock — yielding + outside it lets racing emitters invert live delivery (a span's end before + its began). +- **Scope IDs are deterministic** (hash of parent + name) — span pairing and + cross-layer links rely on the same path being the same scope across + processes and launches. +- **`sequence` is store-global and monotonic**, resuming past the highest + stored value across launches — that is what makes `LogQuery.afterSequence` + a valid incremental cursor. +- **Persistence retains the full hierarchy** — events reference scopes + many-to-many, and scopes keep their parent chain. - **Custom levels are values, not cases.** `LogLevel` is a struct ordered by `severity`; never switch exhaustively over "all" levels. -- **Ambient sources log change-only where the signal is chatty.** - `NetworkPathAmbientSource` keeps its last description and drops - `NWPathMonitor`'s duplicate, change-agnostic callbacks (which otherwise - flood the log); notification-based sources are deliberately *not* deduped - — their notifications post only on real transitions, and repeated memory - warnings are each a distinct event, not a duplicate to swallow. -- **Sink failures never propagate or vanish** — the store logs them to - OSLog, counts them, and persists a synthetic `StoreWriteFailed` marker - for the lost batch; the pipeline reports drops with a synthetic - `DroppedEvents` record. -- **A failed store save must roll back** (`recoverFromFailedWrite`): - the context is discarded, row caches drop, and the session row refetches - by identity — one poisoned batch must never wedge subsequent saves or - fork the session. -- **The crash journal is synchronous at emit and silent on failure.** - Every buffered record appends to the journal (when an on-disk store is - attached) *before* `record()` returns — sequence stamped under the state - lock, file I/O outside it, fault+ records `F_FULLFSYNC`. Journal failures - count and log to OSLog but never throw into the emit path. Ingest runs +- **Ambient sources log change-only where the signal is chatty** + (`NetworkPathAmbientSource` dedupes `NWPathMonitor`'s repeat callbacks); + notification-based sources are deliberately *not* deduped — each repeated + memory warning is a distinct event. +- **Sink failures never propagate or vanish** — logged to OSLog, counted, and + persisted as a synthetic `StoreWriteFailed` marker; the pipeline reports + drops with a synthetic `DroppedEvents` record. +- **A failed store save rolls back** (`recoverFromFailedWrite`) — one + poisoned batch must never wedge subsequent saves or fork the session. +- **The crash journal is synchronous at emit and silent on failure.** Every + buffered record appends before `record()` returns (sequence stamped under + the state lock, file I/O outside it, fault+ records `F_FULLFSYNC`); journal + failures count and log but never throw into the emit path. Ingest runs *before* `startSession` so recovered begans join the orphan sweep; a journal that fails ingest stays for the next launch. -- **Only app processes ingest journals.** App extensions journal their own - sessions but skip ingest (ingest deletes journals; an extension launch - must not eat the live app's) — the app's next launch recovers everyone's. - Sharing one on-disk store across *concurrently live* processes is not - yet supported; see [`TODOs.md`](../TODOs.md). -- **Payloads persist as versioned JSON** (`eventName` + `eventVersion`), not - per-event schemas — changing an event's shape must not require a SwiftData - migration. -- **Every span eventually ends, and its began is delivered first.** - `measure` closes on every path (including throw/cancellation); bounded - spans expire via the watchdog; re-begins supersede rather than refuse; - relaunch orphan-closes `endsWithProcess` spans. Don't add a span path - that can leave `openSpans` growing forever (`survivesRelaunch` resume is - the one staged exception — see [`TODOs.md`](../TODOs.md)). Registration - and the `SpanBegan` record land atomically (`LogRecorder.beginSpan`), so - a span is never closable before its began is in the pipeline; the - overflow drop policy never splits a recorded pair - (`LogEvent.isProtectedFromDropping`, an event-type opt-in the span pair - events set); and redaction is transform-only - for pair records (suppression falls back to a stripped copy). Keep all - three. -- **Span pairs floor together.** The floor decision is made once, at - begin, and the whole lifecycle follows it (`OpenSpan.beganRecorded`, - `LogRecord.bypassesFloors`): a recorded began always gets its end — - normal, expired, or superseded — even if floors rise mid-span, and a - floored began silences the entire span (overdue sentinel included). - Never a dangling half. +- **Only app processes ingest journals** — extensions journal their own + sessions but skip ingest (ingest deletes journals; an extension launch must + not eat the live app's). Concurrently live processes sharing one on-disk + store is unsupported; see [`TODOs.md`](../TODOs.md). +- **Payloads persist as versioned JSON** (`eventName` + `eventVersion`) — an + event shape change must not require a SwiftData migration. +- **Every span eventually ends, and its began is delivered first.** `measure` + closes on every path; bounded spans expire via the watchdog; re-begins + supersede; relaunch orphan-closes `endsWithProcess` spans (the + `survivesRelaunch` resume is staged — [`TODOs.md`](../TODOs.md)). Keep all + three protections: begin registration + `SpanBegan` record land atomically + (`LogRecorder.beginSpan`); the overflow drop policy never splits a recorded + pair (`LogEvent.isProtectedFromDropping`); redaction is transform-only for + pair records. +- **Span pairs floor together.** The floor decision is made once, at begin + (`OpenSpan.beganRecorded`, `LogRecord.bypassesFloors`): a recorded began + always gets its end, and a floored began silences the entire span — never a + dangling half. ## Testing Swift Testing in [`Tests/`](Tests), hosted in `StuffTestHost` (`PeriscopeCoreTests`). Use in-memory stores, fresh `Periscope` systems per -test (never the shared singleton), and injected clocks. Note that -`Log()` defaults to `.shared` — a deliberate ergonomics exception to -the no-Core-defaults rule — so tests must always pass `system:` explicitly. +test (never the shared singleton), and injected clocks. `Log()` +defaults to `.shared` — a deliberate ergonomics exception to the +no-Core-defaults rule — so tests must always pass `system:` explicitly. diff --git a/Shared/Periscope/PeriscopeTools/AGENTS.md b/Shared/Periscope/PeriscopeTools/AGENTS.md index 588e66fd..d71acc41 100644 --- a/Shared/Periscope/PeriscopeTools/AGENTS.md +++ b/Shared/Periscope/PeriscopeTools/AGENTS.md @@ -8,17 +8,6 @@ the narrative and API. This file complements the root [`AGENTS.md`](../../../AGENTS.md), which owns the build system, formatting, and global conventions. Read that first. -## Layout - -`Sources/` groups one directory per tool — `Viewer/`, `Tracer/`, `Alerts/`, -`InspectMode/`, `Spans/` (`OpenSpansView` for live spans, `SpanTreeView` for -the durable store's span tree, `SpanHistoryView` for per-kind duration -percentiles over closed spans), `Hierarchy/` (the scope-tree browser) — plus -`Components/` for the display pieces they share (event rows, the shared -`LogEventList`, detail view, level/exit display extensions) and `Styling/` for -the design system (`PeriscopeStylesheet`). Tests stay flat, named 1:1 with -their source files. - ## Scope & dependencies - **SwiftUI + PeriscopeCore + PeriscopeUI + BroadwayCore/BroadwayUI.** No app @@ -26,31 +15,29 @@ their source files. configuration. - **Intended for DEBUG / developer surfaces**; consumers gate entry points behind `#if DEBUG`. Developer-facing strings are plain literals here. +- `Sources/` groups one directory per tool, plus `Components/` for shared + display pieces and `Styling/` for the design system. Tests stay flat, 1:1 + with their source files. ## Design system — `PeriscopeStylesheet` -Appearance tokens (row geometry, badge chrome, typography, and the -severity/exit/inspect color palette) live in `PeriscopeStylesheet` +Appearance tokens live in `PeriscopeStylesheet` ([`Sources/Styling/PeriscopeStylesheet.swift`](Sources/Styling/PeriscopeStylesheet.swift)), -a Broadway `BStylesheet` — not inline in views. Read tokens with -`@Environment(\.stylesheet) private var stylesheet`; off the `View` tree (tests) -use `PeriscopeStylesheet.default`. +a Broadway `BStylesheet` — never inline in views. Read with +`@Environment(\.stylesheet)`; off the `View` tree use +`PeriscopeStylesheet.default`. -- **Each public tool view seeds its own root** with `periscopeBroadwayRoot()` - so the tooling styles correctly whether or not the host app has a Broadway - root; nesting under an app root simply re-seeds from the same system traits. -- **Row density** (`comfortable` / `compact`) is a `RowStyle` axis resolved via - `stylesheet.row[density]`; the active density rides the `\.logRowDensity` - environment value. The viewer (and inspector sheet) seed it from a - `UserDefaults`-persisted preference (`Density.load`/`save`), which defaults to - `compact` — the roomier `comfortable` is only the raw environment fallback for - rootless contexts (previews, isolated rows). The viewer's filter menu carries - the picker and writes the choice back on change. -- **Color decisions live in `Palette`**, not on `LogLevel` / `SpanExit.Mode` — - `tint(forLevel:)` bands by severity so custom levels inherit a sensible color. +- **Each public tool view seeds its own root** with `periscopeBroadwayRoot()`, + so tooling styles correctly with or without a host Broadway root. +- **Row density** (`comfortable` / `compact`) is a `RowStyle` axis resolved + via `stylesheet.row[density]`, riding the `\.logRowDensity` environment + value; the viewer seeds it from a `UserDefaults`-persisted preference + (`Density.load`/`save`, defaulting `compact`). +- **Color decisions live in `Palette`**, not on `LogLevel` / `SpanExit.Mode` + — `tint(forLevel:)` bands by severity so custom levels inherit a color. - PeriscopeTools seeds Broadway directly; a consumer must not re-list - `BroadwayCore`/`BroadwayUI` beside a product that already carries them — the - root + `BroadwayCore`/`BroadwayUI` beside a product that already carries them — + the root [double-linking rule](../../../AGENTS.md#never-double-link-a-product-whereui-already-carries). ## Invariants @@ -58,51 +45,29 @@ use `PeriscopeStylesheet.default`. - **Read-only over the store.** Tooling queries `PeriscopeCore`'s store and live buffer; it never records events of its own (except through the normal logging API). -- **The toast is hookable** — apps override the default handler rather than - this module special-casing any app. Handlers must not log at or above the - alerter threshold (they'd alert themselves in a loop). -- **`Periscope.isInspectModeEnabled` is the inspect flag's source of - truth** — `PeriscopeInspector` is its observable SwiftUI mirror, synced - both ways: the inspector writes through, and direct system writes flow - back via `inspectModeChanges()`. Either side may write; they converge. -- **Merged multi-query results sort by `(date, sequence)`** — the tracer and - inspector combine several store queries, and the store's insertion - sequence is the tiebreak that keeps same-millisecond events stable. +- **The toast is hookable** — apps override the default handler. Handlers + must not log at or above the alerter threshold (they'd alert themselves in + a loop). +- **`Periscope.isInspectModeEnabled` is the inspect flag's source of truth** + — `PeriscopeInspector` is its observable mirror, synced both ways via + `inspectModeChanges()`. +- **Merged multi-query results sort by `(date, sequence)`** — the store's + insertion sequence is the tiebreak that keeps same-millisecond events + stable. - **Live tree/hierarchy models refresh incrementally.** `LogHierarchyModel` - and `SpanTreeModel` accumulate their derived state (per-scope counts; the - begin/end pairs) and, on each `changes()` ping, fetch only events past the - highest `sequence` they've merged via `LogQuery.afterSequence` — never a - full-store re-read. The merge re-filters on `sequence` so it stays - idempotent if `run()` restarts over already-seen events. This bounds the - per-commit *fetch* by what the commit added (the in-memory forest/tree - rebuild is still O(accumulated); see TODOs) and trades exact reflection of - deletions (retention prune / clear, neither wired into the live app) for - it; a store swap makes the hosting view build a fresh model, resetting the - watermark. -- **Tool views rebind on in-place input swaps** — each view's `.task(id:)` - is keyed on store identity plus its other inputs and rebuilds the model - when they change; a new identity-relevant input must join the key, or - the view silently keeps serving the old inputs. + and `SpanTreeModel` accumulate derived state and fetch only past their + highest merged `sequence` (`LogQuery.afterSequence`) — never a full-store + re-read; the merge re-filters on `sequence` so restarts stay idempotent. + This trades exact reflection of deletions (retention prune / clear, neither + wired into the live app) for a bounded per-commit fetch; the in-memory + rebuild is still O(accumulated) — see [`TODOs.md`](../TODOs.md). A store + swap makes the hosting view build a fresh model. +- **Tool views rebind on in-place input swaps** — each view's `.task(id:)` is + keyed on store identity plus its other inputs; a new identity-relevant + input must join the key, or the view silently keeps serving the old inputs. ## Testing Swift Testing in [`Tests/`](Tests), hosted in `StuffTestHost` (`PeriscopeToolsTests`). Seed an in-memory store, drive the view models directly, and host views with `TestHostSupport`'s `show()` helpers. - -How the tools *look* is pinned separately by the image snapshots in -[`SnapshotTests/`](SnapshotTests), with references under -`SnapshotTests/__Snapshots__/` in Git LFS. Those files compile into the -`PeriscopeToolsSnapshotTests` bundle rather than `PeriscopeToolsTests` — image -suites get their own target, gathered with every other module's into the shared -`StuffSnapshotTests` scheme and its CI job (root -[`AGENTS.md`](../../../AGENTS.md#targets)). It links PeriscopeTools directly, so -nothing here builds against an app module. Seed a **frozen** store -(fixed records at timestamps pinned to a fixed instant); the process-global -Periscope store's wall-clock timestamps would churn the images. Tool views seed -their own `periscopeBroadwayRoot()`, so a snapshot needs no host-app root and -captures the tooling's real styling. - -Prefer a snapshot over a "hosts without crashing" test: a hosting test that only -asserts the view reached a window says nothing about what rendered. Converting -the existing ones is tracked in [`TODOs.md`](../TODOs.md). diff --git a/Shared/SnapshotKit/AGENTS.md b/Shared/SnapshotKit/AGENTS.md index 2f39917e..3db168e6 100644 --- a/Shared/SnapshotKit/AGENTS.md +++ b/Shared/SnapshotKit/AGENTS.md @@ -30,21 +30,15 @@ Complements the root [`AGENTS.md`](../../AGENTS.md) — read that first. - **`.accessibility` configs are preview-filtered.** `snapshotPreviews` drops them because VoiceOver annotations require the test-only library; they only render as snapshot tests. Don't "fix" previews to include them. -- **`\.isCapturingSnapshot` is for motion end-states only.** The trait-bridged - environment flag (`SnapshotCaptureFlag.swift`) is set by the test pipeline - and the preview cutsheet; a view may read it only to freeze motion at a - deterministic phase — never to change layout, content, or behavior. The one - carve-out (documented on the property): content no settle window can make - deterministic — externally-loaded substrates, system controls that render - relative to wall-clock state, wall-clock timers that flip visible state - (skipped under capture; an explicit per-case seam pins each state) — may +- **`\.isCapturingSnapshot` is for motion end-states only.** A view may read + it only to freeze motion at a deterministic phase — never to change layout, + content, or behavior. The one carve-out (documented on the property): + content no settle window can make deterministic — externally-loaded + substrates, wall-clock-dependent system controls, wall-clock timers — may substitute a placeholder of identical layout. It is a **hybrid** accessor - (like Broadway's `BContext+SwiftUI`): a pure-SwiftUI `EnvironmentKey` read - first — set synchronously by the preview cutsheet / any `.environment` - override, no `UITraitCollection` round-trip — falling back to a - `UITraitBridgedEnvironmentKey` (not `@Entry`), which is how the test - pipeline's `traitOverrides` value reaches SwiftUI through the re-hosting the - capture does. The setter mirrors into both. + (pure-SwiftUI `EnvironmentKey` first, `UITraitBridgedEnvironmentKey` + fallback; the setter mirrors into both) — mechanics and why on + `SnapshotCaptureFlag.swift`; don't simplify it to a plain `@Entry`. - **Design-system-agnostic.** SnapshotKit never imports Broadway/WhereUI; the Broadway root wrap is a consumer concern (`WhereUI`'s `whereSnapshot(...)`). diff --git a/Shared/SnapshotKitTesting/AGENTS.md b/Shared/SnapshotKitTesting/AGENTS.md index 5c88e3a5..7d7cc503 100644 --- a/Shared/SnapshotKitTesting/AGENTS.md +++ b/Shared/SnapshotKitTesting/AGENTS.md @@ -16,115 +16,66 @@ Complements the root [`AGENTS.md`](../../AGENTS.md) — read that first. `SwiftDataInspectorSnapshotTests`, gathered into the `StuffSnapshotTests` *scheme*) and `SnapshotKitTestingTests` — **never** a shipping app or `StuffTestHost`. -- **"Process-global" here means per *process*, and that is only safe because - each test bundle gets its own.** Everything below described as - process-global — the safe-area swizzle's depth counter and override globals, - `SnapshotCaptureLock`, the animations save/restore — is *module*-global, so - it is one copy per copy of this module, and each `.xctest` statically embeds - its own (verified with `nm`: every consuming bundle defines a private - `_swizzleDepth`). Two copies **co-loaded into one process** would hold - independent depth counters against the single shared `UIView` method - exchange — parity flips, captures silently render with the simulator's real - safe-area insets, and neither lock sees the other's captures. xcodebuild - gives each bundle its own `StuffTestHost` process (measured on Xcode 27 via - `ProcessInfo.processIdentifier` probes from two bundles in one scheme), so - the several image bundles and `SnapshotKitTestingTests` never collide. Treat - that as this module's load-bearing environmental assumption: if bundles ever - start sharing a host process, this state has to become genuinely - process-wide before another consumer is added. -- **`WhereUISnapshotTests` double-embeds `SnapshotKit`, tolerated and guarded.** - Listing this product in `extraPackageProducts` statically embeds its - dependency closure — including `SnapshotKit` — into the `.xctest`, and - WhereUI statically embeds its own copy into the same image: the - duplicate-type-metadata hazard from the root `AGENTS.md` "Targets" note, - with `\.isCapturingSnapshot` as the type-keyed cross-boundary lookup at - risk. This applies only to that bundle — the Periscope and - SwiftDataInspector image bundles don't link WhereUI at all. There is no - cleaner wiring (the pipeline must reach the bundle without ever linking into - the UI module), the trait lookup demonstrably resolves across both copies - today, and `WhereUISnapshotTests.SnapshotCaptureFlagProbeTests` fails loudly - if the copies ever split — see the snapshot-bundle comment in - `Project.swift` for the full topology. +- **"Process-global" state here is module-global — one copy per consuming + `.xctest` — and that is safe only because each bundle gets its own host + process.** Two copies co-loaded into one process would flip the safe-area + swizzle's parity and hide captures from each other's lock. Tripwire: if a + toolchain ever shares one host process across bundles, re-measure before + adding a consumer — topology and measurement in the snapshot-bundle comment + in `Project.swift` and the root [`AGENTS.md`](../../AGENTS.md#targets). +- **`WhereUISnapshotTests` double-embeds `SnapshotKit`, tolerated and + guarded** (this product's closure plus WhereUI's own copy in one image; the + other image bundles don't link WhereUI). Guard: + `WhereUISnapshotTests.SnapshotCaptureFlagProbeTests` fails loudly if the + copies split; mechanism: PR #145. - Re-exports `SnapshotKit` and `SnapshotTesting` so consumers need one import. - Library target in [`Package.swift`](../../Package.swift). ## Invariants an agent can't re-derive -- **The rendering pipeline is a single function, not per-call code.** All - captures (standard and accessibility) flow through the same async - `renderSnapshotImage(...)` so a config's traits/size/type are the only thing - that varies; callers assert on the returned image (its `async` is +- **The rendering pipeline is one async function.** All captures (standard and + accessibility) flow through `renderSnapshotImage(...)`; its `async` is load-bearing — a synchronous `Snapshotting` pullback could never settle - `.task`-driven content). Accessibility is just a `snapshotType`, wrapped - before the same capture — not a separate path. -- **The compare sees on-disk bytes.** Every capture is round-tripped through - PNG encoding before comparison so the perceptual diff runs on exactly what's - flushed to disk — removing it re-opens the wide-gamut in-memory vs. sRGB - reference flake (see `renderSnapshotImage`'s doc). -- **The runner fails fast, once, on setup problems.** A simulator that doesn't - match the scheme's `SNAPSHOT_EXPECTED_*` pins, or two variants that would - share one reference name, records a single clear issue and asserts nothing — - never hundreds of confusing pixel diffs. -- **An unsettled capture is a failure, not a silent fallback.** `settleContent` - returns a `SettleOutcome`, and `reportIfUnsettled` records an issue when the - budget expires with the content still changing. Capturing whatever frame - happened to be on screen is how a flaky reference lands, so don't "fix" a - timeout by widening the budget — freeze the motion behind - `\.isCapturingSnapshot`, or use `.settledAtLeast` only when the content is - genuinely slow rather than endless. -- **The settle budget bounds observed motion, not proof-of-stability.** On a - starved machine a single settle pass can cost over a second, so fewer passes - than stability needs may fit in the budget; failing then blames "content - still changing" on content never once seen to change (CI reproduced exactly - that: a static screen timing out ~50% of cold runs while its capture still - matched the reference). So `.timedOut` requires an *observed* change, a - change-free loop keeps running until it can prove stability, and only a hard - cap several budgets out gives up as `.starved` — an environment failure, not - view motion. Guarded by `SnapshotRenderingSupportTests`. -- **Captures are single-tenant per process, enforced by `SnapshotCaptureLock`.** - Every capture holds process-global state (the safe-area swizzle + override - globals, the animations flag, the one `StuffTestHost` key window) across the - settle phase's suspensions, so interleaved captures corrupt each other - (verified — in-process parallel scheduling produced 24+ spurious mismatches; - see the snapshot job comment in `.github/workflows/ci.yml`). - `renderSnapshotImage` serializes captures through a FIFO `@MainActor` mutex, - and the safe-area swizzle is depth-counted so unbalanced pairs can't flip the - method-exchange parity (guarded by - `SnapshotKitTestingTests.ConcurrentCaptureTests`). Nested captures — a hook - rendering another snapshot — trap. Keep the suite serial anyway: concurrent - scheduling now degrades to queued-serial rather than corrupt, gaining nothing. -- **Rendering runs in the host key window.** It requires `StuffTestHost`'s window - (via `TestHostSupport.hostKeyWindow()`); it is not usable from a non-hosted - bundle. -- **Determinism is pinned.** Reference images are only valid for the fixed - simulator/scale; the pipeline overrides safe-area insets and quiesces - animations so the physical device insets and in-flight transitions don't leak - into the image. It also sets `SnapshotCaptureTrait` on the captured - controller so views can read `\.isCapturingSnapshot` (SnapshotKit) and freeze - never-settling motion at a deterministic phase — set on the *content* - controller, not a wrapper, so it survives the intrinsic-measurement - re-hosting. -- **Tile-and-stitch is load-bearing, not legacy.** UIKit still renders a blank - image for views taller/wider than ~2000pt on the target toolchain (iOS 27.0 — - verified by a probe during development, guarded by - `SnapshotKitTestingTests.LargeViewCaptureTests`). Captures go through - `SnapshotWrappingViewController` + `tileAndStitchImage`; don't remove the - tiling on the assumption the bug is fixed without re-running that check. + `.task`-driven content. +- **The compare sees on-disk bytes.** Every capture round-trips through PNG + encoding before comparison; removing it re-opens the wide-gamut vs. sRGB + flake (see `renderSnapshotImage`'s doc). +- **The runner fails fast, once, on setup problems** (a simulator that doesn't + match the `SNAPSHOT_EXPECTED_*` pins, two variants sharing one reference + name) — one clear issue, never hundreds of pixel diffs. +- **An unsettled capture is a failure, not a silent fallback.** Don't "fix" a + settle timeout by widening the budget — freeze the motion behind + `\.isCapturingSnapshot`, or use `.settledAtLeast` only for genuinely slow + (not endless) content. +- **`.timedOut` requires observed motion; starvation is `.starved`.** A + change-free settle loop keeps running until it can prove stability (a + starved machine can fit fewer passes than stability needs), and only a hard + cap gives up as `.starved` — an environment failure, not view motion. + Guard: `SnapshotRenderingSupportTests`. +- **Captures are single-tenant per process** — `renderSnapshotImage` + serializes through a FIFO `@MainActor` mutex, the safe-area swizzle is + depth-counted, and nested captures trap. Keep the suite serial anyway: + concurrent scheduling degrades to queued-serial, gaining nothing. Guard: + `SnapshotKitTestingTests.ConcurrentCaptureTests`; the interleaving failure + is recorded in the snapshot job comment in `.github/workflows/ci.yml`. +- **Rendering requires `StuffTestHost`'s key window** + (`TestHostSupport.hostKeyWindow()`) — not usable from a non-hosted bundle. +- **Determinism is pinned.** The pipeline overrides safe-area insets, + quiesces animations, and sets `SnapshotCaptureTrait` on the *content* + controller (not a wrapper — it must survive the intrinsic-measurement + re-hosting) so views can freeze never-settling motion. +- **Tile-and-stitch is load-bearing, not legacy.** UIKit renders a blank + image for views past ~2000pt on iOS 27.0; don't remove the tiling without + re-running the probe. Guard: + `SnapshotKitTestingTests.LargeViewCaptureTests`. ## Testing -`SnapshotKitTestingTests` (`Tests/`, wired in `Project.swift`, in the -`Stuff-iOS-Tests` scheme) owns the pipeline's own regression tests: async-content -settle, the settle loop's ending conditions (starved-but-static settles, observed -motion times out), concurrent-capture serialization, duplicate-identifier -detection, tile-and-stitch / full-content sizing, the pre-capture hook, the -same-image capture-flag surface, and safe-area composition (the swizzle zeroes -the captured root while an interior `safeAreaInset` still composes). They render through -`renderSnapshotImage` (so they need the `StuffTestHost` key window) but assert on -probed pixels via the `@_spi(Testing)` `PixelSample`/`probePixel` API rather than -LFS reference images — so the bundle is fast, has no `__Snapshots__/`, and runs -in the main `test` job, not the snapshot job. The matrixed image assertions -themselves are still exercised by the per-module image bundles; the -WhereUI↔bundle cross-boundary flag probe lives in `WhereUISnapshotTests` -(`SnapshotCaptureFlagProbeTests`), since only a WhereUI-defined view can detect -a duplicate-`SnapshotKit` split. +`SnapshotKitTestingTests` (`Tests/`, in the `Stuff-iOS-Tests` scheme) owns the +pipeline's own regression tests. They render through `renderSnapshotImage` +(so they need the `StuffTestHost` key window) but assert on probed pixels via +the `@_spi(Testing)` `PixelSample`/`probePixel` API rather than LFS reference +images — fast, no `__Snapshots__/`, main `test` job. The matrixed image +assertions live in the per-module image bundles; the cross-boundary flag +probe stays in `WhereUISnapshotTests`, since only a WhereUI-defined view can +detect a duplicate-`SnapshotKit` split. diff --git a/Shared/SwiftDataInspector/AGENTS.md b/Shared/SwiftDataInspector/AGENTS.md index a156e770..e1547c32 100644 --- a/Shared/SwiftDataInspector/AGENTS.md +++ b/Shared/SwiftDataInspector/AGENTS.md @@ -52,15 +52,13 @@ in-memory `ModelContainer` with local `@Model` fixtures, drive wiring is covered from the consumer side (`WhereUITests`). How it renders is pinned by the image snapshots in -[`SnapshotTests/`](SnapshotTests), which build as the module's own -`SwiftDataInspectorSnapshotTests` bundle — image suites get their own target, -gathered with every other module's into the shared `StuffSnapshotTests` scheme -and its CI job (root [`AGENTS.md`](../../AGENTS.md#targets)). It links only -`SwiftDataInspector`, so nothing here builds against an app module. Snapshot the module the way a -consumer gets it: a **local fixture schema**, never a host app's store, and no -design-system root. Prefer adding a case there over a hosted "renders without -crashing" test, which asserts nothing about the result. Keep fixture `@Model` -names distinct from the ones in [`Tests/`](Tests) as hygiene — the two bundles -can't collide today because each `.xctest` gets its own host process (root -[`AGENTS.md`](../../AGENTS.md#targets)), but identical class names would clash -in the ObjC runtime if a toolchain ever co-loaded them. +[`SnapshotTests/`](SnapshotTests) — the module's own +`SwiftDataInspectorSnapshotTests` bundle, wired per the root +[`AGENTS.md`](../../AGENTS.md#targets); it links only `SwiftDataInspector`, +so nothing here builds against an app module. Snapshot the module the way a +consumer gets it — a **local fixture schema**, never a host app's store, no +design-system root — and prefer adding a case there over a hosted "renders +without crashing" test. Keep fixture `@Model` names distinct from the ones in +[`Tests/`](Tests): the bundles can't collide today (each `.xctest` gets its +own host process), but identical class names would clash in the ObjC runtime +if a toolchain ever co-loaded them. diff --git a/Shared/TestHostSupport/AGENTS.md b/Shared/TestHostSupport/AGENTS.md index fb84c990..1721637f 100644 --- a/Shared/TestHostSupport/AGENTS.md +++ b/Shared/TestHostSupport/AGENTS.md @@ -1,20 +1,16 @@ # TestHostSupport – Module Shape -UIKit hosting + run-loop helpers (`show`, `hostKeyWindow`, `waitFor`) -for the hosted Swift Testing bundles that run inside -`StuffTestHost`. The single, dependency-free home for helpers that were -previously duplicated across `WhereTesting` and `BroadwayTesting`. See -[`README.md`](README.md). +UIKit hosting + run-loop helpers (`show`, `hostKeyWindow`, `waitFor`) for the +hosted Swift Testing bundles that run inside `StuffTestHost` — the single, +dependency-free home for them. See [`README.md`](README.md). Complements the root [`AGENTS.md`](../../AGENTS.md) — read that first. ## Scope & dependencies -- **UIKit + Foundation + ObjectiveC only, no sibling deps** (ObjectiveC for the - associated-object window marker below). Keeping it dependency-free is the - whole point: both the Where and Broadway test trees link it without dragging in - the Where domain (which is why the old `broadwayUnitTests` split existed — it's - gone now that both use this module). +- **UIKit + Foundation + ObjectiveC only, no sibling deps** (ObjectiveC for + the associated-object window marker below). Keeping it dependency-free is + the point: every test tree links it without dragging in any domain module. - Library target in [`Package.swift`](../../Package.swift); consumed by hosted test bundles via the `unitTests` helper in [`Project.swift`](../../Project.swift) and by the `StuffTestHost` app. **Never linked from a shipping app target.** From 7c953c03062097fdfb639d141a0ec1fd9e646024 Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Mon, 27 Jul 2026 02:00:25 -0700 Subject: [PATCH 8/8] Apply the what-not-why policy to the root AGENTS.md Plan step: Apply the policy to the root AGENTS.md section by section. Fact-neutral compression, 764 -> 661 lines. Every rule, tripwire, and diagnostic signature survives; the arguments move to their proof: - Double-link section: the correction essay and measured symbol- coalescing nuance become the rule + the guard-test authority line + a PR #145 pointer (the mechanism also lives at the Project.swift call sites since c5aa927). - Targets: the snapshot process-model bullet keeps the measurement fact and the re-measure tripwire, pointing at the snapshot-bundle comment in Project.swift for the topology. - Attribution, agent-skills sync, version stamping, xcstrings: rules + pointers; each 'because' cut to one clause. - Conventions: identifier/keys and observer bullets compressed; Modeling state keeps the full smells list; Composition keeps all four rules with one consequence clause each. - Simulator section: both failure-mode signatures (Busy, Mach -308), the UDID-only rule, and the CI 3.5-5h boot signature all stay; narration trimmed. - Untouched: Per-module docs (owns the standard), Repo-level docs, Working in this repo / GitHub, Generating the Xcode project. Cross-file anchors into the root verified resolving (#targets, #never-double-link-a-product-whereui-already-carries, #layering, #composition-create-once-inject-down, #version-and-build-metadata, #managing-app-icons, #attribution, #formatting, #testing, the simulator anchor). --- AGENTS.md | 341 +++++++++++++++++++----------------------------------- 1 file changed, 119 insertions(+), 222 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 2da6959e..dd65135a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -66,22 +66,15 @@ Run `./ide --no-open` after adding one. ### Version and build metadata -The Where app's `CFBundleShortVersionString` / `CFBundleVersion` are stated -**explicitly** in [`Project.swift`](Project.swift) rather than left to Tuist's -`.extendingDefault` values, because Settings > About shows them — bump them -there. The commit is *not* a manifest value: a post-build script phase +Bump the Where app's `CFBundleShortVersionString` / `CFBundleVersion` +explicitly in [`Project.swift`](Project.swift) (Settings > About shows them). +The commit is stamped by a post-build script ([`Where/Where/Scripts/stamp-build-info.sh`](Where/Where/Scripts/stamp-build-info.sh)) -writes `WhereGitSHA` and `WhereGitStatus` into the built product's Info.plist, -which `WhereCore`'s `BuildInfo` reads back. - -The two constraints worth knowing before touching it: it must stay a **post** -script (it edits the plist "Process Info.plist" already wrote, and must land -before signing seals the bundle), and it must keep -`basedOnDependencyAnalysis: false`, or an unchanged source tree ships the -previous commit's SHA. It reads `.git`, so it also depends on -`ENABLE_USER_SCRIPT_SANDBOXING` staying unset (Xcode defaults it off; new -project templates set it on). Only the app is stamped — the extensions never -read these keys. +into `WhereGitSHA` / `WhereGitStatus`, read back by `WhereCore.BuildInfo`; +only the app is stamped. Tripwires: it must stay a **post** script (before +signing seals the bundle), keep `basedOnDependencyAnalysis: false` (or an +unchanged tree ships the previous commit's SHA), and needs +`ENABLE_USER_SCRIPT_SANDBOXING` unset (it reads `.git`). ## Formatting @@ -90,56 +83,31 @@ read these keys. - The pre-commit hook (enabled by `./ide` via `core.hooksPath`) formats staged `*.swift` files in place and re-stages them. - **String Catalogs are stored exactly as Xcode serializes them**, and - `./xcstrings` (`--lint` in CI) enforces it. Xcode rewrites a `.xcstrings` in - place during an *IDE* build whenever extraction finds a key the catalog - lacks — writing the entire file with its own serializer — so a catalog - written by anything else (a migration script's `json.dump`, a hand edit) - parses fine but turns the next build into thousands of lines of whitespace - churn around the one real entry. Write a catalog through Xcode, or normalize - it afterwards; the script only touches formatting, never content. + `./xcstrings` (`--lint` in CI) enforces it. A catalog written by anything + else parses fine but turns the next IDE build into thousands of lines of + whitespace churn — write catalogs through Xcode or normalize with the + script afterwards (it touches formatting only, never content). ## Attribution -An app ships an **attribution report**: every third-party work it is built with, -each carrying its license notice inline. Regenerate every app's report with: - -```bash -./attribution -``` - -The split matters. [`Shared/CreditKit`](Shared/CreditKit/AGENTS.md) owns the -*types and the reporting tool* and holds **no credits of its own**; each app -declares its sources in an `attribution-sources.json` and ships the resulting -manifest in **its own resources** (for Where, -`Where/Where/Resources/attribution.json`). A report describes one app's -dependency graph, so it is that app's data — and CreditKit stays a Foundation-only -leaf that anything may depend on, so a package pulled in by *any* module gets -credited without inverting the dependency that introduced it. - -The report is derived from what the repo already declares — packages a target -links via `.product(name:package:)` (pinned by `Package.resolved`) and the agent -skills in `.agents/external-skills.json` — with each notice read at the **pinned -revision**. So a new dependency is credited wherever it lands, and a package -resolved only for tooling (BumperBowling, swift-syntax) correctly isn't. -**Re-run `./attribution` and commit the result whenever you add or bump a -package or a skill.** `./attribution --check` fails CI if you forget: it -re-derives the expected report from `Package.swift`, `Package.resolved`, and the -skills manifest and diffs it against the committed one. It runs offline (a -notice is fetched at the pinned revision, so a matching revision means matching -text) and takes well under a second. An app's own tests can't do this job — a -test bundle can't read `Package.swift`, so all it can assert is a literal that a -stale report satisfies just as happily as a fresh one. - -`SoftwareCredit.Kind` keeps a **shipped library** apart from a **development -tool**, and that distinction must survive into any UI: a tool is credited -because the repo uses it, not because it reaches a device. **Kind is derived, -not declared** — the config's `shippedFrom` names the package targets the app -and its extensions link, and anything reachable from that closure is a library -while any other linked package is a tool. Linking is not shipping: the -snapshot-testing engine is linked by a test-support target and never reaches a -binary, and crediting it as a library would tell a reader their app contains it. -Data-source provenance for bundled geometry is separate and stays with its data, -in [`RegionKit`](Where/RegionKit/AGENTS.md). +An app ships an **attribution report** — every third-party work it is built +with, license notices inline. **Re-run `./attribution` and commit the result +whenever you add or bump a package or an agent skill**; `./attribution +--check` fails CI if you forget (offline, sub-second — an app's own tests +can't do this job, since a test bundle can't read `Package.swift`). + +- [`Shared/CreditKit`](Shared/CreditKit/AGENTS.md) owns the types and the + reporting tool and holds **no credits of its own**; each app declares its + sources in an `attribution-sources.json` and ships the report in its own + resources (for Where, `Where/Where/Resources/attribution.json`). +- The report derives from `.product(name:package:)` links (pinned by + `Package.resolved`) and `.agents/external-skills.json`, notices read at the + pinned revision — so tooling-only packages correctly aren't credited. +- **Kind is derived, not declared**: anything reachable from `shippedFrom`'s + target closure is a library, any other linked package a development tool — + linking is not shipping, and a UI must keep the two apart. +- Data-source provenance for bundled geometry stays with its data, in + [`RegionKit`](Where/RegionKit/AGENTS.md). ## Architecture lint @@ -171,30 +139,21 @@ by `./sync-agents`. - `./sync-agents --add [name]` — add an external skill from GitHub. - `./sync-agents --update` — re-fetch all external skills to the latest commit. -`.agents/external-skills.json` pins the **external** skills to a commit — -currently Swift references (SwiftUI, Swift concurrency, Swift Testing, -SwiftData) — and `.agents/skills/.gitignore` excludes those fetched copies, so -anything else under `.agents/skills/` is a **repo-owned** skill and is committed -(currently `todo-triage`). - -That manifest is also an **attribution** input: external skills are third-party -work the repo vendors, so an app's report credits them (as *development tools*, -distinct from libraries the app links). After adding a skill or running -`./sync-agents --update`, re-run `./attribution` and commit the regenerated -reports — see [`Attribution`](#attribution). - -**Cursor reads `.agents/skills/` natively** — that directory is the real home. The -`.claude/skills/` mirror exists for Claude Code, so run `./sync-agents` after -adding or editing a skill, and edit the source rather than the mirror. (Cursor -*also* loads `.claude/skills/` for Claude compatibility, so a synced skill is -discoverable twice and the winning copy is undocumented — one more reason not to -let the two drift.) A fresh clone carries only the repo-owned skills: `CLAUDE.md` -and `.claude/skills/` are gitignored, and the external four arrive with the -first `mise install`. - -A skill carries **procedure** — the steps of a job someone does occasionally. -Rules an agent must follow belong in an `AGENTS.md` or a `TODOs.md` instead, so -that nothing in this repo depends on a skill having been loaded. +`.agents/external-skills.json` pins the **external** skills to a commit; +`.agents/skills/.gitignore` excludes those fetched copies, so anything else +under `.agents/skills/` is **repo-owned** and committed. External skills are +also an **attribution** input — after adding or updating one, re-run +`./attribution` (see [Attribution](#attribution)). + +**`.agents/skills/` is the real home; edit the source, never the +`.claude/skills/` mirror**, and run `./sync-agents` after adding or editing a +skill (Cursor loads both directories, and the winning copy is undocumented — +don't let them drift). A fresh clone carries only the repo-owned skills; the +external ones arrive with the first `mise install`. + +A skill carries **procedure** — the steps of an occasional job. Rules an +agent must follow belong in an `AGENTS.md` or a `TODOs.md`, so nothing in +this repo depends on a skill having been loaded. ## Targets @@ -204,10 +163,9 @@ that nothing in this repo depends on a skill having been loaded. `AGENTS.md` says what it is and how it may be used. - Add SPM library targets in `Package.swift` and wire apps/tests in `Project.swift` (see existing `unitTests` helper). A new module also ships a root `README.md` and `AGENTS.md` — see [Per-module docs](#per-module-docs). - **CI scheme**: CI runs the explicit shared **Stuff-iOS-Tests** scheme (all test bundles) rather than the autogenerated `Stuff-Workspace` scheme. New test bundles must be added to the `Stuff-iOS-Tests` scheme in `Project.swift` or CI won't run them. -- **Image snapshots are the exception: one bundle per module, one shared scheme**: each module that owns image references has its own `*SnapshotTests` target (currently **WhereUISnapshotTests**, **PeriscopeToolsSnapshotTests**, **SwiftDataInspectorSnapshotTests**), and they are all listed in the single shared **StuffSnapshotTests** scheme, which a dedicated CI `snapshot` job runs in one invocation. They are slow and LFS-backed, so they stay deliberately **out of** `Stuff-iOS-Tests`. Reference images under any `__Snapshots__/` directory are stored in **Git LFS** (see `.gitattributes`); the CI job checks out with `lfs: true`. The framework halves live in `Shared/SnapshotKit` (shippable matrix + previews) and `Shared/SnapshotKitTesting` (test-only capture/compare pipeline). The pipeline's own regression tests are a *separate* bundle, **SnapshotKitTestingTests**, which pixel-probes captures (no LFS references) and so runs in the normal `Stuff-iOS-Tests` scheme / `test` job. -- **A new image suite gets a target, not a scheme.** Add a `*SnapshotTests` target over the module's `SnapshotTests/` directory, list only `SnapshotKitTesting` in `extraPackageProducts`, and add it to the `StuffSnapshotTests` scheme's build and test target lists — never a scheme or CI job of its own. A module's image bundle should link only what that module needs, which is the point of splitting them: the Periscope and SwiftDataInspector suites don't build against WhereUI at all. References follow the sources automatically, since swift-snapshot-testing derives the `__Snapshots__` directory from the calling file's `#filePath`. -- **Why separate snapshot bundles are safe — and what would make them unsafe.** Each `.xctest` statically embeds its own copy of `SnapshotKitTesting`, whose capture state is module-global and therefore per copy: the safe-area swizzle's depth counter and override globals, `SnapshotCaptureLock`, and the `UIView` category it installs. Two such copies **in one process** would count separate depths against the single shared `UIView` method exchange (captures silently rendering with the simulator's real safe-area insets), and neither lock would see the other's captures. That doesn't happen because **xcodebuild gives each test bundle its own `StuffTestHost` process** — measured on Xcode 27 by probing `ProcessInfo.processIdentifier` from two bundles in one scheme, which reported different PIDs on both a filtered and a full unfiltered run. This is the load-bearing assumption behind the layout: if a toolchain ever starts sharing one host process across bundles, re-measure before adding another image bundle. -- A UI module that opts into image snapshots adds a `SnapshotTests/` folder alongside the usual `Sources/`+`Tests/` skeleton, holding the bundle and its LFS-tracked `__Snapshots__/` references. +- **Image snapshots are the exception: one bundle per module, one shared scheme.** Each module owning image references has its own `*SnapshotTests` target over its `SnapshotTests/` folder, all listed in the single shared **StuffSnapshotTests** scheme and its dedicated CI `snapshot` job — slow and LFS-backed, so deliberately **out of** `Stuff-iOS-Tests`. References under any `__Snapshots__/` directory are Git LFS (`.gitattributes`; the CI job checks out with `lfs: true`). Framework halves: `Shared/SnapshotKit` (shippable matrix + previews) and `Shared/SnapshotKitTesting` (test-only pipeline, whose own regression bundle **SnapshotKitTestingTests** pixel-probes without LFS and runs in `Stuff-iOS-Tests`). +- **A new image suite gets a target, not a scheme.** Add the `*SnapshotTests` target, list only `SnapshotKitTesting` in `extraPackageProducts`, and add it to the `StuffSnapshotTests` scheme's build and test lists — never a scheme or CI job of its own. An image bundle links only what its module needs (the Periscope and SwiftDataInspector suites don't build against WhereUI at all); references follow the sources automatically via `#filePath`. +- **Separate snapshot bundles are safe because each `.xctest` gets its own `StuffTestHost` process** (measured on Xcode 27 — `ProcessInfo.processIdentifier` probes; details in the snapshot-bundle comment in [`Project.swift`](Project.swift)). Each bundle statically embeds its own copy of `SnapshotKitTesting`'s capture state, and two copies in one process would corrupt each other. Tripwire: if a toolchain ever shares one host process across bundles, re-measure before adding another image bundle. ### Never double-link a product WhereUI already carries @@ -220,32 +178,18 @@ boundary and every type-keyed lookup (SwiftUI `EnvironmentKey`s, `\.isCapturingSnapshot`, Broadway's `BTraits`/`BThemes`/`BStylesheets`) silently resolves against the wrong one. -Worth knowing rather than rediscovering: it reproduces only in the full -multi-bundle scheme, never in an isolated `tuist test WhereUITests`. The -mechanism is written out at the call site you'd be editing — the `WhereUITests` -comment in [`Project.swift`](Project.swift) — and -`WhereStylesheetTests.resolvesTraitAwareTokensFromTheBroadwayRoot` is the guard -that fails if a duplicate copy answers. - -**Nothing in this project is a dynamic framework.** Earlier versions of this -rule (and of `SnapshotKitTesting/AGENTS.md`) described WhereUI as one; that is -not what gets built. The local package is wired as an `XCLocalSwiftPackageReference` -and handed to Xcode's own SPM integration, which links every product statically -into each consumer: no `.framework` products exist and every `.app/Frameworks` -is empty. So "WhereUI carries its dependencies" means *statically embeds them -into whatever links WhereUI*, and a double-link lands two copies in one image. -The rule above is unchanged; only its stated mechanism was wrong. - -One nuance measured while establishing that, because it predicts which -duplications actually bite: **`external` symbols coalesce across images, -`non-external` ones never do.** A `public` type's metadata is external, so dyld -picks one definition and a type-keyed lookup still agrees — verified by -splitting SnapshotKit across two images and watching `\.isCapturingSnapshot` -keep working. A file-scope `private` global or an `enum`'s `static var` is -`non-external` and is genuinely per copy — which is why `SnapshotKitTesting`'s -capture state duplicates. This does not license double-linking (the historical -failure was real and is still guarded), but it does mean the guard test is the -authority on whether a given duplication is harmful, not the general claim. +It reproduces only in the full multi-bundle scheme, never in an isolated +`tuist test WhereUITests` run. Guard: +`WhereStylesheetTests.resolvesTraitAwareTokensFromTheBroadwayRoot` fails if a +duplicate copy answers. + +**Nothing in this project is a dynamic framework** — the local package is +handed to Xcode's own SPM integration, which links every product statically +into each consumer, so "WhereUI carries its dependencies" means *statically +embeds them into whatever links WhereUI*, and a double-link lands two copies +in one image. The guard test is the authority on whether a given duplication +is harmful — measured symbol-coalescing detail and the correction history: +PR #145. ## Deployment @@ -370,16 +314,12 @@ scope and invariants on top rather than restating these. knobs) or a file accretes several behavioral areas, group related properties into nested structs and split responsibilities into focused child types — don't let one god-type keep growing. -- Identifiers/keys are `Hashable` (ideally a typed enum) or `AnyHashable`, not - raw `String`s — a typed token can't silently typo into a new, untracked id, - and any `Hashable` converts to `AnyHashable` implicitly at the call site. - Prefer carrying the *concrete* type where a generic can: `LifecycleStep` - declares `associatedtype ID: Hashable & Sendable` and `LaunchPlan` is generic - over it, so a plan's nodes must share one identity domain and `nodeIDs` hands - tests back typed cases instead of erased `AnyHashable`s. Reach for - `AnyHashable` only where a generic can't reach (a non-generic environment - value, a heterogeneous container). The Where app keys its launch steps with - the `LaunchStepID` enum, and `WherePreferences` keys with a `Keys` enum. +- Identifiers/keys are `Hashable` (ideally a typed enum) or `AnyHashable`, + not raw `String`s — a typed token can't silently typo into a new, untracked + id. Prefer carrying the *concrete* type where a generic can (`LaunchPlan` + is generic over its step `ID`); reach for `AnyHashable` only where a + generic can't reach (a non-generic environment value, a heterogeneous + container). Examples: `LaunchStepID`, `WherePreferences.Keys`. - **Avoid parameter defaults on Core/store APIs.** Prefer explicit call-site arguments so new behavior isn't silently opted into. Reserve defaults for SwiftUI convenience inits and obvious zero values (`[]`, `.zero`) where @@ -448,14 +388,11 @@ scope and invariants on top rather than restating these. obvious place (see `WhereShareExtension`'s `ShareViewController`). - **Observe with a target/selector, not a retained token; every `start` has a `stop`.** Register via `addObserver(_:selector:name:object:)` with `self` - as the observer so teardown is one `removeObserver(self)` — no opaque - tokens to keep, and a restart removes-before-re-adding so it replaces - rather than doubles (see `NotificationAmbientSource`, which every built-in - notification-based ambient source subclasses). Avoid block-based - `addObserver(forName:)`: its observation stays alive in the center whether - or not you keep the returned token, so dropping the token makes it - unremovable and immortalizes everything the block captured. Any - `start…`-style observation API gets a paired `stop()` that removes it. + so teardown is one `removeObserver(self)`, and a restart + removes-before-re-adding (see `NotificationAmbientSource`). Never + block-based `addObserver(forName:)` — dropping its token makes the + observation unremovable and immortalizes everything the block captured. + Any `start…`-style observation API gets a paired `stop()`. ### Architecture and reuse @@ -487,19 +424,13 @@ scope and invariants on top rather than restating these. ### Modeling state -**Make invalid states unrepresentable.** When a set of values is only meaningful -in certain combinations, model it as a *single* type — usually an `enum`, often -with associated values — instead of several parallel properties that can drift -into nonsensical combinations. Default to one type; reach for separate stored -properties only when the values are genuinely independent, and let that be the -exception you can justify, not the reflex. A good type makes the illegal states -impossible to spell and the legal ones obvious. - -The Where app does this with `YearReportModel.LoadState` (`idle` / `loading` / -`loaded` / `failed(LoadError)`) rather than juggling `isLoading` + `error` + -`data`, and `CalendarContentView` keeps one `Result<[CalendarMonth], Error>?` instead -of separate `months` and `layoutError` properties — success and failure can't -both be set, and "not loaded yet" is the `nil`. +**Make invalid states unrepresentable.** When a set of values is only +meaningful in certain combinations, model it as a *single* type — usually an +`enum` with associated values — instead of parallel properties that can drift +into nonsensical combinations. Separate stored properties are the exception +to justify, not the reflex. (Examples: `YearReportModel.LoadState` instead of +`isLoading` + `error` + `data`; `CalendarContentView`'s single +`Result<[CalendarMonth], Error>?`.) Smells that signal a missing type: @@ -518,47 +449,34 @@ Smells that signal a missing type: ### Composition: create once, inject down **A shared resource is created exactly once, at the composition root, and -reaches every consumer by injection** — init parameters, explicit call-site -arguments, or a composition hook — never by re-resolving a global. The Where -app's SwiftData store is the template: the launch's `open-store` step performs -the process's *only* store open, `WhereServices` carries it (plus the -attribution and clock policies derived from it) to every collaborator, and the -App Intents stack is derived *from* those services -(`WhereServices.forIntents(sharingStoreOf:)`) via `WhereLaunch.makeLauncher`'s -`onServicesReady` hook. Two subsystems independently "opening the same store" -is how a fresh install once raced two `ModelContainer`s into a launch failure; -injection made that state impossible to spell rather than merely unlikely. +reaches every consumer by injection** — init parameters, explicit arguments, +or a composition hook — never by re-resolving a global. Template: the Where +app's SwiftData store (the launch's `open-store` step is the process's only +open; `WhereServices` carries it; the App Intents stack derives from it via +the `onServicesReady` hook). Two subsystems independently "opening the same +store" once raced a fresh install into a launch failure. - **No singletons or static get-or-create registries** for anything that can - be injected. A global hides the dependency edge, invites exactly the - double-create race injection prevents, and forces tests to share — and - carefully reset — process-wide state. Needing `@Suite(.serialized)` plus a - reset hook to test something is the smell; injected dependencies get - hermetic per-test instances instead. + be injected — a global invites the double-create race and forces tests to + share process-wide state. Needing `@Suite(.serialized)` plus a reset hook + is the smell; injected dependencies get hermetic per-test instances. - **When the platform instantiates the consumer** (App Intents, extension - principal classes) and constructor injection can't reach it, use the - platform's own DI seam rather than minting a singleton: the composition - root creates the value and registers it (`AppDependencyManager.shared.add` - in `didFinishLaunching`; intents resolve it with `@Dependency`). And keep - that seam a **handoff, not a factory**: the root *installs* what it created - (`IntentServices.install(_:)`), early callers **await** installation + principal classes), use the platform's DI seam, and keep it a **handoff, + not a factory**: the root installs what it created + (`IntentServices.install(_:)`), early callers await installation (`current()` parks, cancellation-aware), and the seam never creates the - resource itself. A "create it myself" fallback — however unlikely to run — - quietly reintroduces the duplicate the design exists to prevent. + resource itself — a "create it myself" fallback quietly reintroduces the + duplicate the design exists to prevent. - **Derive, don't re-derive.** A stack built from an existing layer reuses - what that layer already computed (the store, the live attributor, the - injected clock) rather than re-reading it. That keeps derivation synchronous - and non-throwing — wiring it can't fail into a half-composed state — and a - derived stack can't drift from its base (duplicate change subscriptions, - diverging clocks). + what that layer computed (the store, the live attributor, the clock) — + derivation stays synchronous and non-throwing, and can't drift from its + base. - **Re-fire composition hooks wherever the lifecycle re-creates the thing.** - `onServicesReady` fires on every session (re)start — first launch, retry - after a failed launch, the reset relaunch — so consumers always hold the - current instance, never the first one. + `onServicesReady` fires on every session (re)start, so consumers always + hold the current instance, never the first one. This is [Modeling state](#modeling-state) applied to ownership and lifetime: -one owner, created in one place, with the illegal wirings (two owners, zero -owners, a stale copy) unrepresentable. +one owner, created in one place, the illegal wirings unrepresentable. ## Generating the Xcode project @@ -621,24 +539,14 @@ immediately, and no amount of clearing DerivedData will surface it. ## Selecting a simulator — one device per checkout, addressed by UDID **Every checkout owns a simulator of its own, and `./simulator` is the only -thing that hands one out.** Two failure modes make that the rule: - -- **A shared name is ambiguous.** A dev machine usually has an "iPhone 17" on - each installed runtime (26.x, 27.0, …) — a legitimate setup for testing - multiple OS versions — and **any `xcrun simctl` command matches by name - only**. `simctl boot "iPhone 17"` (or `shutdown` / `erase`) may act on a - *different* device than the one under test, leaving it mid-transition. -- **A shared device is contended.** Several checkouts on one machine — clones, - worktrees, an agent working in each — otherwise resolve to the *same* - device, and then race each other booting it, installing and uninstalling the - same bundle ID, and erasing it out from under a run in flight. - -Both surface identically: `Application failed preflight checks (Busy)` or -`Mach error -308 — server died` / `crashed with signal kill before establishing -connection` — launch failures that look like test failures but aren't (the -suites that do run are green, and the wall time is spent before and around -them). So always pass a **UDID** to `simctl`, never a name, and get that UDID -from `./simulator`: +thing that hands one out.** A name is ambiguous (`simctl` matches by name +only, and a machine usually has an "iPhone 17" per installed runtime), and a +shared device is contended (parallel checkouts race each other booting, +installing, and erasing it). Both surface identically: `Application failed +preflight checks (Busy)`, or `Mach error -308 — server died` / `crashed with +signal kill before establishing connection` — launch failures that look like +test failures but aren't (the suites that do run are green). So always pass a +**UDID** to `simctl`, never a name, and get that UDID from `./simulator`: ```bash mise exec -- tuist test Stuff-iOS-Tests --no-selective-testing -- \ @@ -678,19 +586,13 @@ it too, so a local repro targets a device nothing else can touch. also means remembering that `simctl shutdown` is async: poll until the device actually reads `(Shutdown)` before `erase`/`boot`. -**CI takes the one exception, and still resolves by UDID.** A job owns its VM, -so the isolation a per-checkout device buys is already there — the boot step -passes `--shared`, which takes the image's existing iPhone 17 instead of -creating (and then first-booting) a device per run. Resolution is not optional -even there: the `xcode-27` image ships a single iPhone 17 today, so a -`name=…,OS=…` destination does resolve — but *booting* is the expensive half, -and a cold or wedged CoreSimulator has stretched the ~10-minute test job to -**3.5–5 hours** on that image (runs on 2026-07-23, i.e. after the -`xcrun simctl list` cache warm-up was already in place — that nudge fixes -destination *resolution*, not a slow boot). So CI resolves the UDID, -`bootstatus -b`s it, and passes `-destination "…,id=$UDID"`, under a -`timeout-minutes` cap so a stuck boot fails fast instead of holding a runner. -See [`.github/workflows/ci.yml`](.github/workflows/ci.yml). +**CI takes the one exception, and still resolves by UDID.** A job owns its +VM, so the boot step passes `--shared` (the image's existing iPhone 17, +skipping a per-run first boot) — but it still resolves the UDID, +`bootstatus -b`s it, and passes `-destination "…,id=$UDID"` under a +`timeout-minutes` cap: a cold or wedged CoreSimulator has stretched the +~10-minute test job to **3.5–5 hours** (runs on 2026-07-23). See +[`.github/workflows/ci.yml`](.github/workflows/ci.yml). ## Cursor Cloud specific instructions @@ -706,17 +608,12 @@ environment: formatting and agent sync work; builds, tests, and running the trusts the config, runs `mise install`, installs `git-lfs`, and points Git at `.githooks/`. Nothing about a cloud agent's setup lives in a dashboard. -`git-lfs` is not optional on either platform. `.githooks/` carries the Git LFS -hooks beside the pre-commit formatter, and they **exit non-zero when the binary -is missing** — so routing Git at them without it breaks checkout, merge, and -push even for work that never touches snapshot tests. Both bootstraps handle it -before setting `core.hooksPath`: `./ide` checks and instructs (`brew install -git-lfs`), the cloud script installs it from apt. - -Two consequences worth knowing. A repo-defined environment follows branches, so -a PR can change how its own agent is set up — and it **takes precedence over any -dashboard-managed personal or team environment** for this repo. And it must stay -idempotent: Cursor may re-run it against cached state. +`git-lfs` is not optional on either platform — the `.githooks/` LFS hooks +exit non-zero when the binary is missing, breaking checkout/merge/push even +for work that never touches snapshots. Both bootstraps install it before +setting `core.hooksPath`. The repo-defined environment follows branches, +**takes precedence over any dashboard-managed environment**, and must stay +idempotent (Cursor may re-run it against cached state). ### What works on Linux