diff --git a/AGENTS.md b/AGENTS.md index fa5c4f8e..656fd521 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -171,7 +171,7 @@ triage). **Always-on** rules every edit must honor stay in `AGENTS.md` or - 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 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 Inspector suites don't build against WhereUI at all); references follow the sources automatically via `#filePath`. +- **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 Flyover, Inspector, and Periscope 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 diff --git a/MODULE_AUDIT.md b/MODULE_AUDIT.md index 60780777..00ea06c9 100644 --- a/MODULE_AUDIT.md +++ b/MODULE_AUDIT.md @@ -1,10 +1,10 @@ # Swift Module Audit Report -Read-only review of all **14 SPM library targets**, **6 Tuist app/extension targets**, and the repo-owned **Bumper Bowling** architecture rules (~359 source / ~198 test Swift files across shipped targets, plus 2 unwired prototype sources). No code was changed. +Read-only review of all **19 SPM library targets**, **6 Tuist app/extension targets**, and the repo-owned **Bumper Bowling** architecture rules (501 source / 310 test Swift files across shipped targets, plus 3 unwired prototype sources). No code was changed. -**Date:** July 26, 2026 -**Method:** Read-only verification of every open July 19 finding against current source; file-count refresh; new-surface review of the week's landings (Periscope migration #94, Settings drill-in #111, developer HUD #115, navigation restructure #119, log-viewer tooling #107, String Catalog symbols #124, Gregorian calendars `fe99dde`, previews `52f0136`, Bumper Bowling #127, catalog serialization #135). -**Prior audit:** July 19, 2026 (~308 source / ~189 test). +**Date:** August 2, 2026 +**Method:** Read-only verification of every open July 26 finding against current source, module cluster by module cluster; file-count refresh; new-surface review of the week's landings (demo mode and `WhereScope` #150, `./test` as one front door #151, ambient snapshots and build-attributed sessions #152, resolution/store-scan race #153, spans across the app #154, `Bundle.module` env override #155, Flyover browser #156, developer tools launcher #157, Inspector boot mode #158, merged Backup/Data settings #159, Xcode 27 beta 4 re-record #161, Flyover canvas stability #166, agent-skill extraction #167, nonoptional launcher #168, remote-change filtering #169). +**Prior audit:** July 26, 2026 (~359 source / ~198 test, 14 SPM targets). > **This report carries no actionable items.** Every finding it describes is filed > in a `TODOs.md`; the root [`TODOs.md`](TODOs.md) owns the item format and says @@ -17,87 +17,58 @@ Read-only review of all **14 SPM library targets**, **6 Tuist app/extension targ ## Executive summary -What this pass turned up, before the findings were filed: - -| Severity | Count | -|----------|------:| -| Critical | 0 | -| High | 5 | -| Medium | 44 | -| Low | 48 | -| **Total** | **97** | - -| Category | Count | -|----------|------:| -| bug | 21 | -| test | 27 | -| convention | 24 | -| performance | 7 | -| duplication | 3 | -| localization | 4 | -| docs | 8 | -| design | 3 | - -**Overall:** A heavy week of landings. `LogKit` and `LogViewerUI` are **gone** — Periscope replaced them (#94) — so the target count drops to 14 SPM libraries while WhereUI grew 84 → 113 sources and WhereCore 70 → 87. Several long-standing findings closed for real: the Where app's `README.md`, the `.undetermined` launch-reason state machine, `SharedItemLoader` warning logs, `#Preview` coverage across WhereUI/WhereWidgets, and the String Catalog symbol migration (a typo'd key is now a compile error). The three **high** findings carried from July 19 are all still open — daily-summary staleness, the WhereUI tracking-toggle race, and the LifecycleKit terminal-phase race — and two new **high** ones landed: `CalendarDay.displayDate` resolves day labels through `Calendar.current`, and the brand-new `where.gregorian_calendar` Bumper rule that exists to catch exactly that is blind to the form the drift actually takes. +The heaviest week the repo has had. Three new library targets landed — **Flyover** (the screen browser, 50 sources), **Inspector** (the renamed and much-expanded `SwiftDataInspector`, now a second *boot runtime*), and **LifecycleKitUI** — while **CreditKit**, **SnapshotKit**, and **SnapshotKitTesting** are counted here for the first time (they landed on July 26 itself, after the previous pass took its inventory). The tree grew from ~359 to **501** source files, WhereUI alone 113 → 150 and WhereCore 87 → 95. Demo mode (#150) reshaped composition around `WhereScope`, and #152/#154 pushed Periscope spans through every plausibly expensive path in the app. + +**Nothing on the July 26 high list closed this week.** Four of the five carried highs — the daily-summary fan-out gap, the tracking-toggle race, `CalendarDay.displayDate`'s `Calendar.current`, and the blind `where.gregorian_calendar` rule — were re-verified against current source and are open exactly as filed. The fifth, the LifecycleKit terminal-phase race, is gone, but it went with the typed-engine rewrite (#116) that landed *on* July 26: the previous pass listed it as open against a tree that had already fixed it. What closed since is smaller and real: `WherePreferences.init(store:)` lost its default, `IntentSnippets`' preview literal is localized, the `LocationsView`/`YearView` empty-state snapshot cases exist, and `SnapshotCase` now rebuilds content per configuration. + +Three new findings were filed this pass, and one previously-filed claim was found **false** and corrected rather than carried. --- -## Top 10 highest-impact findings +## Top findings Pointers only — each one's evidence and suggested fix live in the linked file. -| # | Sev | Module | Issue | Filed in | -|---|-----|--------|-------|----------| -| 1 | **high** | Bumper Bowling | `where.gregorian_calendar` matches only an explicit `Calendar.current` base, so the rule is green while production sites drift | [`TODOs.md`](TODOs.md) P0 | -| 2 | **high** | WhereUI | `CalendarDay.displayDate` resolves through `Calendar.current`, so day labels on a non-Gregorian device render a date ~543 years off | [`Where/TODOs.md`](Where/TODOs.md) P1 | -| 3 | **high** | WhereCore | `DailySummaryReconciler.reconcile()` is absent from the post-day-change fan-out — the notification body stays stale until a foreground re-`configure` | [`Where/TODOs.md`](Where/TODOs.md) P0 | -| 4 | **high** | WhereUI | Tracking toggle race — `trackingEnabled`'s setter spawns unserialized `Task`s | [`Where/TODOs.md`](Where/TODOs.md) P1 | -| 5 | **high** | LifecycleKit | Cancel during the *last* step's `minVisible` hold isn't observed, so a superseded drive can set `phase = .ready` | [`Shared/LifecycleKit/TODOs.md`](Shared/LifecycleKit/TODOs.md) P0 | -| 6 | **medium** | WhereCore | `setPrimaryRegions(_:)` commits atomically but skips `reconcileAfterDayChange()` | [`Where/TODOs.md`](Where/TODOs.md) P1 | -| 7 | **medium** | WhereCore | `setTrackedRegion(false)` hard-deletes the row; the shipped picker now reaches it, so past-year re-attribution risk is live | [`Where/TODOs.md`](Where/TODOs.md) P1 | -| 8 | **medium** | PeriscopeCore | Orphan sweep treats an undecodable `SpanBegan` as an orphan-close candidate, silently overriding `survivesRelaunch` | [`Shared/Periscope/TODOs.md`](Shared/Periscope/TODOs.md) P1 | -| 9 | **medium** | WhereUI | Load-state UI duplicated across four views; `PresenceTimelineList` renders the *empty* state while the year is still loading | [`Where/TODOs.md`](Where/TODOs.md) P1 | -| 10 | **medium** | BroadwayCatalog | The showcase app never seeds a Broadway root, and its test bundle is an empty `struct` wired into the CI scheme | [`Shared/Broadway/TODOs.md`](Shared/Broadway/TODOs.md) P1 | +| # | Module | Issue | Filed in | +|---|--------|-------|----------| +| 1 | Bumper Bowling | `where.gregorian_calendar` matches only an explicit `Calendar` base, so the rule is green while four shipped sites drift | [`TODOs.md`](TODOs.md) P0 | +| 2 | WhereCore | `DailySummaryReconciler.reconcile()` is absent from the post-day-change fan-out — the notification body stays stale until a foreground re-`configure` | [`Where/TODOs.md`](Where/TODOs.md) P0 | +| 3 | PeriscopeCore | The pre-store-attach window is dropped, and #154's budgeted launch spans made it more expensive: a launch span's began and ended now land in different places | [`Shared/Periscope/TODOs.md`](Shared/Periscope/TODOs.md) P0 | +| 4 | WhereUI | Tracking toggle race — `trackingEnabled`'s setter spawns unserialized `Task`s | [`Where/TODOs.md`](Where/TODOs.md) P1 | +| 5 | WhereUI | `CalendarDay.displayDate` resolves through `Calendar.current`, so day labels on a non-Gregorian device render ~543 years off | [`Where/TODOs.md`](Where/TODOs.md) P1 | +| 6 | WhereUI | Notification authorization is requested during launch, unprompted — and all three preferences default to `true`, so it hits every fresh install | [`Where/TODOs.md`](Where/TODOs.md) P1 | +| 7 | Project | `WhereTests` double-links `LifecycleKit` beside `WhereUI`, and never imports it — **new** | [`TODOs.md`](TODOs.md) P1 | +| 8 | WhereCore | A store opened without a resolvable URL silently loses remote-change observation in release — **new** | [`Where/TODOs.md`](Where/TODOs.md) P2 | +| 9 | Flyover | 50 sources against 10 namesake test files — the largest untested surface in the repo — **new** | [`Shared/Flyover/TODOs.md`](Shared/Flyover/TODOs.md) P1 | +| 10 | WhereCore | `setTrackedRegion(false)` hard-deletes the row; the shipped picker reaches it, so past-year re-attribution risk is live | [`Where/TODOs.md`](Where/TODOs.md) P1 | --- ## Cross-cutting themes -### The Gregorian rule has a blind spot, and the catalog says otherwise - -`.bumper/RULES.md` states the tree "intentionally contains three violations" of `where.gregorian_calendar`, left visible so the live lint demonstrates enforcement. It doesn't: the rule filters `MemberAccessExprSyntax` on `base == "Calendar"`, which matches the spelled-out `Calendar.current` but **not** the implicit-member form (`calendar: Calendar = .current`, `startOfDay(in: .current)`) — and after `fe99dde` the implicit form is the *only* one left. CI runs `bumper lint` as a hard `severity: .error` gate and is green, which confirms it: the rule reports nothing while seven production sites drift. The same paragraph's claim about preview-coverage violations is also stale (`52f0136` closed those). A rule that reads as enforced but enforces nothing is worse than a documented convention, because it stops anyone from looking. - -### Reconciliation: same two holes, one now user-reachable - -`reconcileAfterDayChange()` still fans out to issue state and widgets only. **Daily summary** remains outside it, and **`setPrimaryRegions(_:)`** still commits without calling it. Related and newly urgent: untracking a region hard-deletes its row, and the shipped onboarding picker plus the Settings region editor both route into that path, so the past-year re-attribution risk the `SwiftDataStore` TODO describes is now something a user can trigger. - -### Presentation-layer calendar drift outlived the fix - -`fe99dde` moved the view call sites onto explicit Gregorian calendars, but the drift relocated into shared helpers: `CalendarDay.displayDate` hardcodes `.current`, and `DateRangeFormatting.abbreviated` / `PresenceTimeline.stints` *default* to it — with `PresenceTimelineList` not passing the report's calendar. Because these are the helpers every day label flows through, one line reaches the relabel, logged-days, resolution, and region drill-in screens. +### The composition rules held under the week that stress-tested them -### The navigation restructure moved the duplication, not the shape +Demo mode was the first change to demand two worlds at once, and the shape the root `AGENTS.md` prescribes — create once, inject down, model ownership as a value — absorbed it without a flag. `WhereScope` carries an open store's services, its preferences, and its log store as one value; a second world is a second scope; Flyover retains a third that is built but never activated. The parts that *didn't* fit are precisely the parts that were already global, and they surfaced as items rather than as breakage: the process-global `WhereLog` facade still routes Flyover's diagnostics into the active scope's store, and one developer surface (`OpenSpansView(system: .shared)`) reaches the global directly while `WhereModel.logSystem` exists to prevent exactly that. That is the design working — a global is now visibly the exception. -Locations / Your Year / Settings replaced the old four-tab shell, so the `PrimaryView` / `SecondaryView` / `CalendarView` load-state triplicate is gone — but the same `YearReportModel.loadState` gate is now copy-pasted across `LocationsView`, `ElsewhereView`, `ResolutionView`, and `CalendarContentView`, and `PresenceTimelineList` skipped the gate entirely (it shows "no stays" during load). A `ReportLoadGate` would now save four sites rather than three. +### Spans arrived everywhere, and landed on an open durability gap -### Periscope's durability gaps are the oldest open work in the repo +#154 put a budgeted span on every launch step, store read and commit, aggregation, detector, reconcile, intent, and widget publish. It also made Periscope's oldest P0 more expensive: a span opened before a scope attaches its store persists only its *end*, so the pair splits. Where documents the split and points at Periscope rather than working around it, which is the right call and the reason this shows up as one theme instead of two bugs — but the launch is now the single largest producer of half-persisted spans, and the item that fixes it is the same one that has been open longest. -Three items — `survivesRelaunch` resume mechanics, journaling the pre-store-attach window, and multi-process journal coordination — remain P0/P1 in `Shared/Periscope/TODOs.md` and are all confirmed unimplemented. The store-side half of relaunch policy landed (the sweep leaves surviving spans open), but nothing re-seeds `Periscope.openSpans`, so `end(for:)` in the new process still warns "without a matching begin". +### The reconciliation holes are stable, and the docs describe the version that doesn't exist -### PeriscopeTools grew fast; its live models rebuild from scratch +`reconcileAfterDayChange()` still fans out to issue state and widgets only. Daily summary remains outside it; `setPrimaryRegions(_:)` still commits without calling it; `ingest`, the bulk ingest, and `addManualSample` still publish widgets and skip the reminder/issue reconcile. None of that moved this week. What is worth separating out is that `WhereCore/README.md` and — worse — `WhereCore/AGENTS.md` state the *unified* fan-out as a rule, so an agent reading the module's own contract is told the invariant holds. A false rule in an `AGENTS.md` is a different category of stale than a false sentence in a `README.md`: it will be preserved against the code. -+9 sources / +9 tests this week (span tree, hierarchy, span history, density, Broadway stylesheet). The incremental **fetch** landed (`LogQuery.afterSequence`), but `SpanTreeModel.load` / `LogHierarchyModel.load` still rebuild the whole forest from all accumulated events on every `changes()` ping, `LogInspectorModel` re-queries full subtrees, and the new drill-ins re-read row density from `.standard` rather than the injectable `defaults` the viewer threads through. +### Test coverage is now bimodal -### Extension/app targets still defer tests to libraries +The modules with the most machinery are the best covered — PeriscopeCore 37/33, LifecycleKit 8/10, JournalKit 2/3, Inspector 23/14, all with fuzz or adversarial suites where the state space warrants it. The gap is concentrated in two places and both are recent: **Flyover** shipped 50 sources against 10 namesake tests, and **WhereUI** is 150/61 with 28 snapshot files carrying much of the real verification. Meanwhile PeriscopeTools still has 20 hosting smoke tests across 10 files that assert only "the view reached a window" — up from 18 across 9, because the pattern is still being copied into new files while the item to convert them stays open. -WhereWidgets (7/0), WhereShareExtension (5/0), and RegionViewer (1/0) ship no test bundle by design; BroadwayCatalog ships an empty one. `ShareEvidenceModel.buildPendingEvidence()` and `WhereWidgetProvider`'s midnight timeline policy remain the two gaps that are worth closing regardless of the pattern. +### Image snapshots became a repo-wide practice, and their cost is now shared -### Localization architecture is now compiler-enforced +Four image bundles (WhereUI, Flyover, Inspector, PeriscopeTools) and 273 references, up from one bundle and 260. The one-bundle-per-module rule and the measured per-`.xctest` host-process isolation it rests on both held as new bundles were added. The cost followed: Flyover's canvas needed a 1.5s settle floor to stabilize (#166), making it the fourth case paying the floor the settle-cost item wants to remove, and the first outside Where. Two references remain quarantined or wrong on purpose — Inspector's bistable dark capture behind `withKnownIssue`, and the blank VoiceOver calendar captures, confirmed this pass still un-re-recorded by reading their LFS pointer sizes. -The String Catalog symbol migration (#124) is complete and the hand-maintained key facades are gone, so a removed key breaks the build. Remaining slips are individual, not architectural: a raw `String(localized: "region.other")` in RegionKit, a hardcoded caption in `IntentSnippets`, the parallel `share.form.*` / `evidence.form.*` namespaces, and four auto-extracted literals — three in Where's catalogs, one in LifecycleKit's. +### Documentation drift is now mostly numeric -### Infrastructure that is genuinely done - -Periscope replaced LogKit/LogViewerUI outright; `.undetermined` replaced the cold-launch guess with a state that can't lie; `#Preview` coverage is complete across WhereUI/WhereWidgets; `@_spi(Testing)` is the norm for test seams (the only `…ForTesting` API is itself behind it); the SwiftData browser shipped into Settings → Developer; String Catalogs are serialized the way Xcode writes them and linted (`./xcstrings`); and `./simulator` now resolves destinations by UDID for `profile` / `flaky` / CI. +The prose contracts largely held: the Periscope group's invariants, LifecycleKit's, Inspector's twelve safety rules (each verified enforced in code this pass), CreditKit's, SnapshotKit's. What went stale was counts and lists — measured percentages quoted as current, a module stack that hadn't gained Flyover, an image-suite list of two that should be three. Those are fixed in this pass rather than filed, since they're the kind of claim that reads authoritative long after it stops being true. --- @@ -108,9 +79,9 @@ accepted as deliberate. Open work is in the linked `TODOs.md`. ### Bumper Bowling — architecture lint -The repo-owned rule set (`BumperBowling.swift`, `.bumper/Sources`, catalog in `.bumper/RULES.md`) covers **Where production sources only**: layer boundaries and forbidden imports, graph integrity, production store opening, checked-concurrency escape hatches, composition ownership (`WhereServices`, live `LocationSource`), the Gregorian calendar, the `store.perform` transaction boundary, `AppShortcutsProvider` ownership, the logging facade and logging-type placement, and `#Preview` coverage. Mutation tests live in `.bumper/Tests`; CI runs `config`, `test`, and `lint --timings` with every rule at `severity: .error`. +**Verified OK:** all 10 `where.*` rule IDs, scopes, and `.error` severities in `.bumper/RULES.md` match `WhereProjectRules.swift`; the component graph matches `BumperBowling.swift` / `WhereArchitecture.swift`. No drift. -**Verified OK:** rule IDs and scopes in `RULES.md` match `WhereProjectRules.swift`; the component graph matches `BumperBowling.swift` / `WhereArchitecture.swift`; Broadway is forbidden on WhereIntents/WhereWidgets via `forbidden_import`, matching `Project.swift`. +**Open:** the Gregorian rule's blind spot (top finding 1). Re-derived this pass: **zero** explicit `Calendar.current` spellings remain in Where production sources, so the rule's match set is provably empty while four implicit-member sites drift. **Files:** 4 rule/test sources · RULES.md ✓ · Open: [`TODOs.md`](TODOs.md) @@ -118,55 +89,79 @@ The repo-owned rule set (`BumperBowling.swift`, `.bumper/Sources`, catalog in `. ### WhereCore -**Verified OK:** backup import → full fan-out via `onImport`; summary format args (guarded by `summaryBodyContainsNoFormatPlaceholders`); `BackupError` localization; drain-only ingest skipping the full reminder reconcile; no raw-string/`os.Logger` logging left; no PII in `.public` events; `RecentActivitySummarizer`'s typed unavailability and segment cap. +**Verified OK:** #169's remote-change filter correctly scopes by `NSPersistentStoreURLKey` and ignores Periscope's store, with three regression tests; backup import still routes through the full fan-out via `onImport`; every manual-day write path still funnels through `reconcileAfterDayChange()`; `forIntents(sharingStoreOf:)` inherits the base's schedulers, so demo-derived intents can't post real notifications; `RegionAttribution` rebuilds on `changes()` with a last-good fallback on read failure; schema versioning is fully absent rather than half-shipped, as the module docs say it is. -**Files:** 87 source / 58 test · README ✓ · AGENTS ✓ · Open: [`Where/TODOs.md`](Where/TODOs.md) +**Files:** 95 source / 60 test · README ✓ · AGENTS ✓ · Open: [`Where/TODOs.md`](Where/TODOs.md) --- ### WhereUI -**Verified OK:** no closure `Binding(get:set:)` anywhere in the module (`SaveErrorAlertState`, `AddEvidenceModel`, `AppIconModel` expose computed `get`/`set`); every load-state `switch` enumerates its cases; `MainTabs` drives `YearReportModel.activate()` / `deactivate()` off `scenePhase`; every previewable `View`/`Widget` ships an in-file `#Preview`; `README.md` and `AGENTS.md` are current on the three-tab shape. +**Verified OK:** no closure `Binding(get:set:)` anywhere in the module; no `Calendar.current` in production sources (only test and preview fixtures); no empty `catch {}` in views or models; `LocationsView` pairs `tilt.start()` / `tilt.stop()`; the #150 scope model and #154's `BudgetedLaunchStep` / `.measured()` wiring match the documented shape; `README.md` and `AGENTS.md` are current on the three-tab shell, Flyover, the Inspector latch, and the scene-scoped `YearReportModel`. + +**Files:** 150 source / 61 test / 28 snapshot · README ✓ · AGENTS ✓ · Open: [`Where/TODOs.md`](Where/TODOs.md) + +--- + +### Flyover *(new)* + +The app-agnostic screen browser: a catalog of screens rendered as a zoomable canvas or a list, fed for Where by `WhereFlyoverWorld`. + +**Verified OK:** dependency scope is honored — SwiftUI, BroadwayCore/BroadwayUI, and SnapshotKit only, with no Where or persistence imports; no swallowed errors, no `try?`, no empty catches anywhere in `Sources/`; every switch over an own enum is exhaustive; no `Calendar.current`; no closure `Binding(get:set:)`; content loads go through a serial coordinator with `.task(id:)` cancellation; `FlyoverView` seeds its own Broadway root. English literals are the module's stated policy for a developer tool, and the code matches it. -**Files:** 113 source / 36 test · README ✓ · AGENTS ✓ · Open: [`Where/TODOs.md`](Where/TODOs.md) +**Files:** 50 source / 12 test / 1 snapshot · README ✓ · AGENTS ✓ · Open: [`Shared/Flyover/TODOs.md`](Shared/Flyover/TODOs.md) *(opened this pass)* --- -### LifecycleKit +### Inspector *(renamed from SwiftDataInspector; gained a boot runtime)* -**Verified OK:** cancel-and-drain no longer waits out the full `minVisible` window; duplicate step-ID `precondition`; localized `LifecycleFailureView`; background *and* `.undetermined` promotion container tests. The `.undetermined` state machine (#109) holds up: `completedStepIDs` records only steps that ran to completion, so a promotion re-drive skips finished work while still running newly-applicable steps, and promotion/teardown both funnel through the same cancel-and-drain. +**Verified OK — every one of the module's twelve safety invariants was checked against code this pass, and all twelve are enforced**, most with tests: no deletion of a configured root or a containing ancestor; SwiftData sources resolved before filesystem deletion is enabled, with the store family and `recoveryStorageURLs` protected; deletion disabled in an unresolved storage tree; recovery erasure with a second-pass latch; pending erasures completed before either runtime is constructed (`AppDelegate.swift:48-50`); no symlink escape from a configured root; configured defaults domains only, with complex values read-only and no key creation; every context and model instance on `InspectorSwiftDataStore`; tables that don't fault relationships to render; whole-store erase only where a fresh-container factory was supplied. These guard against destroying the user's data, so they were verified individually rather than sampled. -**Files:** 9 source / 11 test · README ✓ · AGENTS ✓ · Open: [`Shared/LifecycleKit/TODOs.md`](Shared/LifecycleKit/TODOs.md) +**Files:** 23 source / 14 test / 1 snapshot · README ✓ · AGENTS ✓ · Open: [`Shared/Inspector/TODOs.md`](Shared/Inspector/TODOs.md) --- -### SwiftDataInspector +### LifecycleKit & LifecycleKitUI -**Verified OK:** pagination + lazy rendering with regression tests; relationship resolution for materialized models. +**Verified OK:** the terminal-phase race is genuinely gone — the typed engine holds nothing, the splash minimum moved to `LifecycleContainer`, and `drive` publishes behind `guard case let .completed(value) = outcome, !Task.isCancelled`. The `.undetermined` state machine, single-in-flight drive serialization, memoized run-once promotion, and detached-child isolation all still hold as documented. -**Accepted:** `try?` on fetches yields empty rows/counts (documented DEBUG degradation); a bare `default:` in `defaultFormat` over `Any` (open-type dispatch). +**Accepted:** LifecycleKitUI has no `TODOs.md` — its one open item (the duplicate-ID traps) spans both modules and lives in LifecycleKit's. -**Files:** 13 source / 1 test · README ✓ · AGENTS ✓ · Open: [`Shared/SwiftDataInspector/TODOs.md`](Shared/SwiftDataInspector/TODOs.md) +**Files:** LifecycleKit 8/10 · LifecycleKitUI 5/3 · README ✓ · AGENTS ✓ · Open: [`Shared/LifecycleKit/TODOs.md`](Shared/LifecycleKit/TODOs.md) --- ### PeriscopeCore, PeriscopeUI, PeriscopeTools -**Verified OK:** Broadway does not leak below PeriscopeTools; no test touches `Periscope.shared`; `@_spi(Testing)` used for injection hooks; span-pair floors, rollback-on-failed-save, and the seeded lifecycle fuzz all still in place. PeriscopeUI is a thin DEBUG bridge with nothing outstanding. +**Verified OK:** Broadway still does not leak below PeriscopeTools; no test touches `Periscope.shared`; `@_spi(Testing)` is used for every injection hook in the new surface; span pairs still can't be split by floors, redaction, or drop policy; ambient snapshot folding, the relaunch-policy column sweep, and `SpanNode.Outcome` from #152 are implemented as designed and tested. `LogSession` correctly contributes no build attributes for an unstamped bundle. -**Files:** PeriscopeCore 35/31 · PeriscopeUI 1/2 · PeriscopeTools 24/22 · README ✓ · AGENTS ✓ · Open: [`Shared/Periscope/TODOs.md`](Shared/Periscope/TODOs.md) +**Accepted:** `LogSession.current` falls back to `"unknown"` for a missing `CFBundleShortVersionString` — checked against the "never invent a build" rule and found *not* to violate it; that rule governs `attributes`, and these are core session fields on a bundle that would have to be malformed. + +**Files:** PeriscopeCore 37/33 · PeriscopeUI 1/2 · PeriscopeTools 27/27 + 1 snapshot · README ✓ · AGENTS ✓ · Open: [`Shared/Periscope/TODOs.md`](Shared/Periscope/TODOs.md) --- -### JournalKit, StuffCore, TestHostSupport, StuffTestHost +### SnapshotKit & SnapshotKitTesting + +**Verified OK:** the four image bundles each list only `SnapshotKitTesting` in `extraPackageProducts`; all four are in the `StuffSnapshotTests` scheme and none in `Stuff-iOS-Tests`; the one-bundle-per-module arrangement and its host-process rationale still hold. `SnapshotCase`'s lazy `contentFactory` fix landed with a regression test. + +**Files:** SnapshotKit 8/3 · SnapshotKitTesting 14/11 · README ✓ · AGENTS ✓ · Open: [`Shared/SnapshotKit/TODOs.md`](Shared/SnapshotKit/TODOs.md), [`Shared/SnapshotKitTesting/TODOs.md`](Shared/SnapshotKitTesting/TODOs.md) + +--- + +### JournalKit, CreditKit, StuffCore, TestHostSupport, StuffTestHost + +**JournalKit:** strong fuzz/truncation coverage. **Files:** 2/3 · Open: [`Shared/JournalKit/TODOs.md`](Shared/JournalKit/TODOs.md) -**JournalKit:** strong fuzz/truncation coverage. **Files:** 2/3 · README ✓ · AGENTS ✓ · Open: [`Shared/JournalKit/TODOs.md`](Shared/JournalKit/TODOs.md) +**CreditKit:** `./attribution --check` is green (7 credits) — the report covers every pinned package and agent skill. **Files:** 2/3 · Open: [`Shared/CreditKit/TODOs.md`](Shared/CreditKit/TODOs.md) -**StuffCore:** intentional scaffold. **Files:** 1/1 · README ✓ · AGENTS ✓ · Open: [`Shared/StuffCore/TODOs.md`](Shared/StuffCore/TODOs.md) +**StuffCore:** intentional scaffold. **Files:** 1/1 · Open: [`Shared/StuffCore/TODOs.md`](Shared/StuffCore/TODOs.md) -**TestHostSupport:** dependency-free UIKit helpers; no dedicated bundle by design (exercised via hosted bundles), nothing open. **Files:** 1/0 · README ✓ · AGENTS ✓ +**TestHostSupport:** dependency-free UIKit helpers; no dedicated bundle by design. **Files:** 1/0 · Nothing open. -**StuffTestHost:** the WhereCore-always-embedded trade-off is documented and verified load-bearing in `Project.swift:256`; the smoke test lives in `LifecycleKitTests`. **Files:** 2/0 · README ✓ · AGENTS ✓ · Open: [`TODOs.md`](TODOs.md) (both items reach the root Tuist manifest) +**StuffTestHost:** the WhereCore embed is gone; `PACKAGE_RESOURCE_BUNDLE_PATH` now carries hosted `Bundle.module` resolution under Xcode 27 beta 4, wired on every test target and documented in `Project.swift`. **Files:** 2/0 · Open: [`TODOs.md`](TODOs.md) + +All five: README ✓ · AGENTS ✓ --- @@ -174,7 +169,7 @@ The repo-owned rule set (`BumperBowling.swift`, `.bumper/Sources`, catalog in `. **Verified OK:** stylesheet/trait/cycle behavior well tested; trait registration pairs with teardown. -**Accepted:** a bare `default:` mapping unknown `UIContentSizeCategory` to `.large` (`BTraits+Values.swift:125`, a deliberate fallback); hardcoded English in the catalog app (internal showcase). +**Accepted:** a bare `default:` mapping unknown `UIContentSizeCategory` to `.large` (a deliberate fallback); hardcoded English in the catalog app. **Files:** BroadwayCore 17/10 · BroadwayUI 6/4 · BroadwayCatalog 2/1 · README ✓ · AGENTS ✓ · Open: [`Shared/Broadway/TODOs.md`](Shared/Broadway/TODOs.md) @@ -182,47 +177,39 @@ The repo-owned rule set (`BumperBowling.swift`, `.bumper/Sources`, catalog in `. ### RegionKit & RegionViewer -**Verified OK:** the per-region catalog drives `RegionStyle`, the pickers, and the App Intents `RegionEntity` with no `Region` enum left to extend. RegionViewer ships no test bundle by design. +**Verified OK:** the per-region catalog drives `RegionStyle`, the pickers, and the App Intents `RegionEntity`; Source mode now builds from 57 per-region GeoJSON files via `RegionGeometryCatalog`, exercised indirectly by `RegionGeometryCatalogTests`. RegionViewer ships no test bundle by design. -**Files:** RegionKit 13/8 · RegionViewer 1/0 · README ✓ · AGENTS ✓ · Open: [`Where/TODOs.md`](Where/TODOs.md) +**Files:** RegionKit 14/9 · RegionViewer 1/0 · README ✓ · AGENTS ✓ · Open: [`Where/TODOs.md`](Where/TODOs.md) --- ### WhereIntents -**Verified OK:** reader/writer seams well tested (`WhereIntentReaderTests`, `WhereIntentWriterTests`); `IntentServices` handoff still covered by `IntentServicesTests` (install/park/cancel/replace) with no self-creating fallback; no Broadway double-link. +**Verified OK:** reader/writer seams well covered; `IntentServices` handoff still tested for install/park/cancel/replace with no self-creating fallback; no Broadway double-link. -**Accepted:** the per-intent `perform()` glue is untested because `@Dependency` traps outside the perform flow — the open item is to extract a seam or say so in `README.md`. +**Accepted:** the per-intent `perform()` glue is untested because `@Dependency` traps outside the perform flow — now documented in `AGENTS.md`, though not yet in `README.md`. -**Files:** 17/9 · README ✓ · AGENTS ✓ · Open: [`Where/TODOs.md`](Where/TODOs.md) +**Files:** 17/10 · README ✓ · AGENTS ✓ · Open: [`Where/TODOs.md`](Where/TODOs.md) --- -### WhereWidgets & WhereShareExtension +### WhereWidgets, WhereShareExtension, Where app -**Verified OK:** `SharedItemLoader` logs load failures at `warning`; widget gallery strings localized; `@unknown default:` on widget-family switches; no Broadway double-link in either target. The post-midnight stale snapshot is explicitly documented as intentional degradation in the provider, `README.md`, and `AGENTS.md`. +**Verified OK:** `SharedItemLoader` reports provider errors through one `LoadedValue` that can't spell "bytes *and* an error"; widget gallery strings localized; `@unknown default:` on widget-family switches; no Broadway double-link in either extension. The Where app's `AppDelegate` completes pending Inspector recovery erasures before selecting a runtime. -**Accepted:** neither target ships a test bundle (documented). +**Accepted:** neither extension ships a test bundle (documented). -**Files:** WhereWidgets 7/0 · WhereShareExtension 5/0 · README ✓ · AGENTS ✓ · Open: [`Where/TODOs.md`](Where/TODOs.md) - ---- - -### Where app - -**Verified OK:** `Where/Where/README.md` now exists and matches the three-file shell; `WhereTests` pins `.undetermined` as the launch reason under the UIScene lifecycle; delegate wiring smoke test; no Broadway double-link. Nothing open. - -**Files:** 3/1 · README ✓ · AGENTS ✓ +**Files:** WhereWidgets 7/0 · WhereShareExtension 5/0 · Where 6/2 · README ✓ · AGENTS ✓ · Open: [`Where/TODOs.md`](Where/TODOs.md) --- ## Limitations -- Static analysis only — no `tuist test`, `bumper lint`, or simulator runs in this pass (the Cloud agent runs Linux; the full suite requires macOS CI). CI status on `main` was read via `gh` and is green, which is what lets the "the Gregorian rule finds nothing" conclusion stand. -- Some findings (the LifecycleKit terminal-phase race, the tracking toggle, outbox relaunch loss) need runtime confirmation. -- Severity counts are approximate — several low-severity 1:1 test gaps are folded into module summaries rather than filed individually. -- DEBUG-only surfaces (PeriscopeTools, SwiftDataInspector) are held to a lighter standard for `try?` degradation, per their module docs. -- `Shared/Periscope/Prototypes/JournalBenchmark` (2 sources) is wired into no target and is excluded from the counts below. +- **Static analysis only** — this pass ran on Linux, where Tuist, the simulator, and `swift run bumper` are unavailable, so no test, build, or lint execution backs any claim about runtime behavior. What *was* executed: `./swiftformat --lint` (clean, 0/818) and `./attribution --check` (clean). CI status on `main` was read via `gh` and is green through `0cce6578`, which is what lets the "the Gregorian rule finds nothing" conclusion stand — a hard `.error` gate that passes over a tree containing four drifting sites can only mean the rule doesn't see them. +- Some findings need runtime confirmation: the tracking-toggle race, the outbox relaunch loss, the release-only remote-change skip, and the two quarantined snapshot instabilities. +- No severity totals are given this pass. The previous report's counts mixed filed items with folded-in summaries and couldn't be reconciled against the backlog, which is the only place a finding lives; the top-findings table above points at real items instead. +- DEBUG-only surfaces (PeriscopeTools, Inspector, Flyover) are held to a lighter standard for `try?` degradation and for localization, per their module docs. +- `Shared/Periscope/Prototypes/JournalBenchmark` (3 sources) is wired into no target and is excluded from every count. --- @@ -233,49 +220,53 @@ The repo-owned rule set (`BumperBowling.swift`, `.bumper/Sources`, catalog in `. | Module | Path | Source | Test | README | AGENTS | |--------|------|-------:|-----:|:------:|:------:| | StuffCore | `Shared/StuffCore/` | 1 | 1 | ✓ | ✓ | -| LifecycleKit | `Shared/LifecycleKit/` | 9 | 11 | ✓ | ✓ | +| CreditKit | `Shared/CreditKit/` | 2 | 3 | ✓ | ✓ | +| LifecycleKit | `Shared/LifecycleKit/` | 8 | 10 | ✓ | ✓ | +| LifecycleKitUI | `Shared/LifecycleKitUI/` | 5 | 3 | ✓ | ✓ | | JournalKit | `Shared/JournalKit/` | 2 | 3 | ✓ | ✓ | -| PeriscopeCore | `Shared/Periscope/PeriscopeCore/` | 35 | 31 | ✓ | ✓ | +| PeriscopeCore | `Shared/Periscope/PeriscopeCore/` | 37 | 33 | ✓ | ✓ | | PeriscopeUI | `Shared/Periscope/PeriscopeUI/` | 1 | 2 | ✓ | ✓ | -| PeriscopeTools | `Shared/Periscope/PeriscopeTools/` | 24 | 22 | ✓ | ✓ | -| SwiftDataInspector | `Shared/SwiftDataInspector/` | 13 | 1 | ✓ | ✓ | +| PeriscopeTools | `Shared/Periscope/PeriscopeTools/` | 27 | 27 + 1 | ✓ | ✓ | +| Inspector | `Shared/Inspector/` | 23 | 14 + 1 | ✓ | ✓ | +| Flyover | `Shared/Flyover/` | 50 | 12 + 1 | ✓ | ✓ | +| SnapshotKit | `Shared/SnapshotKit/` | 8 | 3 | ✓ | ✓ | +| SnapshotKitTesting | `Shared/SnapshotKitTesting/` | 14 | 11 | ✓ | ✓ | | TestHostSupport | `Shared/TestHostSupport/` | 1 | 0 | ✓ | ✓ | | BroadwayCore | `Shared/Broadway/BroadwayCore/` | 17 | 10 | ✓ | ✓ | | BroadwayUI | `Shared/Broadway/BroadwayUI/` | 6 | 4 | ✓ | ✓ | -| RegionKit | `Where/RegionKit/` | 13 | 8 | ✓ | ✓ | -| WhereCore | `Where/WhereCore/` | 87 | 58 | ✓ | ✓ | -| WhereUI | `Where/WhereUI/` | 113 | 36 | ✓ | ✓ | -| WhereIntents | `Where/WhereIntents/` | 17 | 9 | ✓ | ✓ | +| RegionKit | `Where/RegionKit/` | 14 | 9 | ✓ | ✓ | +| WhereCore | `Where/WhereCore/` | 95 | 60 | ✓ | ✓ | +| WhereUI | `Where/WhereUI/` | 150 | 61 + 28 | ✓ | ✓ | +| WhereIntents | `Where/WhereIntents/` | 17 | 10 | ✓ | ✓ | + +*(A `+ N` in the Test column is that module's image-snapshot bundle, which runs in the `StuffSnapshotTests` scheme rather than `Stuff-iOS-Tests`.)* ### Tuist app / extension targets | Target | Path | Source | Test | README | AGENTS | |--------|------|-------:|-----:|:------:|:------:| -| Where | `Where/Where/` | 3 | 1 | ✓ | ✓ | +| Where | `Where/Where/` | 6 | 2 | ✓ | ✓ | | WhereWidgets | `Where/WhereWidgets/` | 7 | 0 | ✓ | ✓ | | WhereShareExtension | `Where/WhereShareExtension/` | 5 | 0 | ✓ | ✓ | | RegionViewer | `Where/RegionViewer/` | 1 | 0 | ✓ | ✓ | | StuffTestHost | `Shared/StuffTestHost/` | 2 | 0 | ✓ | ✓ | | BroadwayCatalog | `Shared/Broadway/BroadwayCatalog/` | 2 | 1 | ✓ | ✓ | -**Totals:** ~359 source · ~198 test Swift files across shipped targets (plus 4 Bumper rule/test sources and 2 unwired prototype sources). +**Totals:** 501 source · 279 test + 31 image-snapshot Swift files across shipped targets (plus 4 Bumper rule/test sources and 3 unwired prototype sources). 20 unit-test bundles and 4 image bundles, all enrolled in their scheme. --- -## Changes since July 19, 2026 audit - -| Area | July 19 state | July 26 state | -|------|---------------|---------------| -| Target count | 16 SPM + 6 Tuist | **14 SPM** + 6 Tuist — LogKit and LogViewerUI deleted, replaced by Periscope (#94) | -| File count | ~308 source / ~189 test | ~359 source / ~198 test (WhereUI 84 → 113, WhereCore 70 → 87, PeriscopeTools 15 → 24, RegionKit 9 → 13) | -| Architecture lint | — | Bumper Bowling (#127): Where component graph + 10 source-level rules, hard-gated in CI — with a Gregorian blind spot and a stale catalog | -| Navigation | Primary / Elsewhere / Resolve / Settings | Locations / Your Year / Settings (#119); Elsewhere is a card, Resolve a toolbar action, data screens under Settings | -| Settings | Flat list | iOS-style drill-in screens with search (#111) | -| Developer surfaces | LogViewerUI + overlay | Liquid Glass HUD (#115), Periscope viewer with hierarchy / span tree / span history / density (#107), in-app SwiftData browser | -| Launch reason | `applicationState` guess (cold launch read as headless) | `LifecycleReason.undetermined` + promotion, with `completedStepIDs` preventing re-runs (#109) | -| Localization | Hand-maintained key facades | Generated String Catalog symbols; a removed key is a compile error (#124); catalogs serialized as Xcode writes them and linted (#135) | -| Calendars | `Calendar.current` in view call sites | Fixed at the call sites (`fe99dde`) — but relocated into `CalendarDay.displayDate` and two helper defaults | -| Preview coverage | Gaps across WhereUI | Complete; enforced by `where.preview_coverage` (`52f0136`) | -| Simulator handling | Name-based destinations | `./simulator` resolves a UDID and boots it; `profile` / `flaky` / CI all go through it (#130) | -| Device installs | Xcode UI | `./Where/install` (#110, #112) | -| Backlog | Findings split between this file and two `TODOs.md` | One backlog across eight `TODOs.md`; this report is derived and carries no items | +## Changes since July 26, 2026 audit + +| Area | July 26 state | August 2 state | +|------|---------------|----------------| +| Target count | 14 SPM + 6 Tuist | **19 SPM** + 6 Tuist — Flyover, Inspector (renamed from SwiftDataInspector), and LifecycleKitUI landed; CreditKit, SnapshotKit, and SnapshotKitTesting are counted for the first time | +| File count | ~359 source / ~198 test | **501** source / 279 test + 31 image-snapshot (WhereUI 113 → 150, WhereCore 87 → 95, Flyover 0 → 50, Inspector 13 → 23) | +| Composition | One world, opened at launch behind the onboarding gate | `WhereScope` as a value (#150) — demo mode is a second scope, Flyover retains an unactivated third, and at most one routes logs | +| Developer surfaces | Periscope viewer, HUD, in-app SwiftData browser | Plus a Path-style tools launcher (#157), the Flyover screen browser (#156), and Inspector as an alternate **boot runtime** with filesystem/defaults/SwiftData deletion (#158) | +| Spans | Periscope had spans; the app didn't use them | Every plausibly expensive path budgeted (#154); ambient state stamped on every record and sessions build-attributed (#152) | +| Image snapshots | 1 bundle, 260 references | **4 bundles** (WhereUI, Flyover, Inspector, PeriscopeTools), **273 references**, one shared scheme and CI job | +| Test invocation | `./test` landed as the one front door (#151) | Unchanged, plus `PACKAGE_RESOURCE_BUNDLE_PATH` carrying hosted `Bundle.module` resolution under Xcode 27 beta 4 (#155) | +| Agent instructions | All rules in `AGENTS.md` | GitHub and running-tests procedure extracted into skills (#167); `AGENTS.md` keeps the always-on rules | +| Backlog | 8 `TODOs.md` files | **11** — Flyover opened this pass; every module with an open item now has one | +| Highs | 5 open | 4 still open; the LifecycleKit terminal-phase race was already fixed by #116 on July 26 and the previous pass missed it | diff --git a/Shared/Broadway/TODOs.md b/Shared/Broadway/TODOs.md index b0db9b74..f555e465 100644 --- a/Shared/Broadway/TODOs.md +++ b/Shared/Broadway/TODOs.md @@ -10,7 +10,7 @@ here. # Open issues ## P1s (Should do) -- test(BroadwayCatalog) [quick-win]: Host `BroadwayCatalogTests` in `StuffTestHost` like every other hosted bundle. Today it is a hand-rolled target hosted by the BroadwayCatalog app itself (`Project.swift:545` — deps `[BroadwayCatalog, TestHostSupport]`, no `StuffTestHost`), a deviation from the convention that hosted tests run in the shared host. Rewire it through the `unitTests` helper (keeping the `BroadwayCatalog` code dependency) and confirm `tuist test BroadwayCatalogTests` stays green. (pr#149 review 2026-07-28) +- test(BroadwayCatalog) [quick-win]: Host `BroadwayCatalogTests` in `StuffTestHost` like every other hosted bundle. Today it is a hand-rolled target hosted by the BroadwayCatalog app itself (`Project.swift:575-585` — deps `[BroadwayCatalog, TestHostSupport]`, no `StuffTestHost`), a deviation from the convention that hosted tests run in the shared host. Rewire it through the `unitTests` helper (keeping the `BroadwayCatalog` code dependency) and confirm `tuist test BroadwayCatalogTests` stays green. (pr#149 review 2026-07-28) - fix(BroadwayCatalog) [quick-win]: `BroadwayApp.swift:6` never seeds `.broadwayRoot()`, so the showcase renders with no `BContext` and every `@Environment(\.bContext)` read falls back to defaults — the one app whose job is to show Broadway is the one not using it. (audit 2026-07-26) - test(BroadwayCatalog) [quick-win]: `Tests/BroadwayCatalogTests.swift:4` is an empty `struct BroadwayCatalogTests {}` wired into the `Stuff-iOS-Tests` scheme, so CI runs it and it asserts nothing. Replace it with a launch smoke test. (audit 2026-07-26) - fix(BroadwayUI) [needs-design]: A nested `BRootViewController` registers duplicate trait observers (`BRootViewController.swift:92`, documented in a source `TODO`). Latent today — Where reaches Broadway only through `whereBroadwayRoot()` / `BRootView`, neither of which nests — but it fires the moment something does. (audit 2026-07-26) diff --git a/Shared/CreditKit/TODOs.md b/Shared/CreditKit/TODOs.md index 76b79aaa..596444d7 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: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) +- 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:90-92`) 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/Flyover/TODOs.md b/Shared/Flyover/TODOs.md new file mode 100644 index 00000000..ae3cd455 --- /dev/null +++ b/Shared/Flyover/TODOs.md @@ -0,0 +1,20 @@ +# Flyover todos + +The backlog for the Flyover module — the app-agnostic developer browser that +renders a catalog of screens as a zoomable canvas or list. + +The item format and the placement rule live in the root +[`TODOs.md`](../../TODOs.md); raw notes go in [`INBOX.md`](../../INBOX.md), not +here. + +# Open issues + +## P1s (Should do) +- test [needs-design]: The module shipped 50 sources against 10 namesake test files, so most of it has no 1:1 coverage. What is covered is the model layer — catalog validation (`FlyoverCatalogTests`), the six-screen live cap (`FlyoverCanvasRenderPlan.swift:5-7`), the serial load coordinator, zoom/appearance plans — and four canvas/list reference images (`SnapshotTests/FlyoverSnapshotTests.swift`). What isn't: `FlyoverCanvasView` (viewport, preview, and focus interaction), `FlyoverRootView` (including the invalid-catalog path), `FlyoverConnectorCanvas`, `FlyoverFocusedView`, the control-bar and menu views, and the `FlyoverGroup*` types. Add tests for the behavioral types first and leave rendering to the image suite; the two together are what the root convention asks for. Flyover is a DEBUG developer tool, so this is a coverage debt to pay down rather than a shipping risk — but it is the largest untested surface in the repo. (audit 2026-08-02) + +## P2s (Nice to have) +- feat [quick-win]: An invalid catalog reports only how many problems it has, not what they are. `FlyoverRootView.swift:14-19` renders a `ContentUnavailableView` whose description is `"\(catalog.validationIssues.count) structural issue(s) must be fixed."`, so a developer who mis-wires a route learns that one thing is wrong and nothing about which. The issues are already modelled and populated (`FlyoverCatalogValidationIssue.swift`) — list them in the unavailable view, or put them behind a drill-in. This is the whole audience: an invalid catalog only ever reaches a developer. (audit 2026-08-02) + - docs [quick-win]: `README.md:99-100` says an invalid catalog "renders a diagnostic instead of a partial map", which reads as though the issues are shown. Once the view lists them the sentence becomes true; until then it oversells. (audit 2026-08-02) +- test [quick-win]: `FlyoverCanvas` is the only image case on a 1.5s `.settledAtLeast` floor outside Where (`SnapshotTests/FlyoverSnapshotTests.swift:20`), added by #166 to stabilize it. A deterministic completion signal on the content-load coordinator would let the floor come off; see the settle-floor item in [`Shared/SnapshotKitTesting/TODOs.md`](../SnapshotKitTesting/TODOs.md), which this is now part of the cost of. (audit 2026-08-02) + +# Completed issues diff --git a/Shared/Inspector/TODOs.md b/Shared/Inspector/TODOs.md index e67685cd..d525f229 100644 --- a/Shared/Inspector/TODOs.md +++ b/Shared/Inspector/TODOs.md @@ -10,14 +10,22 @@ The item format and placement rule live in the root - fix [needs-design]: The second image capture in this bundle's process can render a search-field placeholder at a different width. The dark `inspectorSurfaces.SwiftData_iPhone_dark` assertion remains quarantined with - `withKnownIssue`; the likely fix is a measured capture-pipeline warm-up in - SnapshotKitTesting, not re-recording one bistable state. (agent 2026-07-28) + `withKnownIssue` (`SnapshotTests/InspectorSnapshotTests.swift:61-73`, + `isIntermittent: true`; the light variant at `:36-43` asserts strictly); the + likely fix is a measured capture-pipeline warm-up in SnapshotKitTesting, not + re-recording one bistable state. (agent 2026-07-28) - test [quick-win]: Cover the bare-`PersistentIdentifier` relationship branch - in `SwiftDataReflection.swift`; current relationship tests materialize the - model and exercise the other branch. (audit 2026-07-26) + in `SwiftDataReflection.swift:132-137`; the relationship tests materialize + the model via key paths and so exercise the `any PersistentModel` branch at + `:125-130` instead, and `SwiftDataReflectionTests` covers metatype / + stored-values / fetch only. (audit 2026-07-26) - test [quick-win]: Add image cases for the paged row table, filesystem root, - defaults editor, and relationship drill-in. The entity list and developer - menu are covered. (pr#101 review) + defaults editor, and relationship drill-in. Covered today: the SwiftData + entity list (light strict, dark quarantined) and the root developer menu + (light + dark) — one `@Test`, four references + (`SnapshotTests/InspectorSnapshotTests.swift:18-73`, `:88-125`). Pagination + is unit-tested but unpinned visually, and `RelationshipView` has a + `#Preview` (`:130`) with no snapshot case behind it. (pr#101 review) ## Completed issues diff --git a/Shared/LifecycleKit/TODOs.md b/Shared/LifecycleKit/TODOs.md index 935dbf59..1229333f 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 (`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) +- test [quick-win]: Add a test that duplicate node IDs trap. `LaunchPlan.append` `precondition`s on a duplicate (`LaunchPlan.swift:134-140`), and `LifecycleContainer` — now in LifecycleKitUI — does the same for duplicate gate-view registrations via `assertUniqueGateTypes` (`LifecycleKitUI/Sources/LifecycleContainer.swift:98-104`), but nothing exercises either. This item covers both modules, which is why LifecycleKitUI has no file of its own. (audit 2026-07-26) # Completed issues diff --git a/Shared/Periscope/TODOs.md b/Shared/Periscope/TODOs.md index fa1a88ef..c3e15aef 100644 --- a/Shared/Periscope/TODOs.md +++ b/Shared/Periscope/TODOs.md @@ -12,11 +12,11 @@ here. ## P0s (Must do) - design(PeriscopeCore) [needs-design]: Span record modeling — `spanID`/`spanExit` bolted onto every `LogRecord` (and `bypassesFloors` as a one-off flag) feels wrong; consider `enum { case span(Span), case event(Event) }` or a dedicated span record type. Plan/build loop. (agent) -- design(PeriscopeCore) [needs-design]: Decompose `Periscope` (the type and its flat `State` — group watchdog/inspect/ambient/live-observer state into sub-structs) and `PeriscopeStore` into children per behavioral area. Plan/build loop. (agent) +- design(PeriscopeCore) [needs-design]: Decompose `Periscope` (the type and its flat `State` — group watchdog/inspect/ambient/live-observer state into sub-structs) and `PeriscopeStore` into children per behavioral area. Plan/build loop. Re-measured 2026-08-02: `Periscope.swift` is 931 lines with one flat `private struct State` (`:145-180`) holding scopes, sinks, pending, floors, `openSpans`, ambient, inspect, journal, and the drain/watchdog/auto-flush fields; `PeriscopeStore.swift` is 1230 lines and still one `@ModelActor`. Both grew with #152. (agent) - 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) -- feat(PeriscopeCore) [needs-design]: Implement `SpanRelaunchPolicy.survivesRelaunch` resume mechanics. The policy is recorded on `SpanBegan` payloads *and* persisted as the `SDLogEvent.spanRelaunchPolicy` column, 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:577`). 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) +- feat(PeriscopeCore) [needs-design]: Implement `SpanRelaunchPolicy.survivesRelaunch` resume mechanics. The policy is recorded on `SpanBegan` payloads *and* persisted as the `SDLogEvent.spanRelaunchPolicy` column, 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:86–88`; sweep filter: `PeriscopeStore.swift:237–239`; warn: `LogSpan.swift:599–600`). 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` (`:117`), so events emitted between process launch and `add(sink:)` (`Periscope.swift:232-237`, which installs the journal only for a `PeriscopeStore` sink) — 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. **#154 raised the cost of leaving this open**: every budgeted launch step now opens a span through the process-global facade before a `WhereScope` attaches its store (`MeasuredStep.swift:72-74`), so a launch span's `SpanBegan` lands in OSLog only while its `SpanEnded` persists — durations survive, pairs don't. Where documents the split rather than working around it ([`Where/AGENTS.md`](../../Where/AGENTS.md#spans)), which makes closing this the fix for both. (pr#94 review) ## P1s (Should do) - feat(PeriscopeCore) [needs-design]: Journal attachments via external storage. Instead of inlining blobs ≤64KB and omitting larger ones (`LogJournalEntry.swift:101`), write attachment bytes as files beside the journal segments (the entry referencing them by filename), clean them up with segment rotation and journal removal, and re-attach them at ingest. Removes the size cliff entirely — screenshots and payloads survive crashes too. (pr#86 review) @@ -26,14 +26,15 @@ here. ## P2s (Nice to have) - design(PeriscopeTools) [needs-design]: `SpanTreeModel` models open spans with `effectiveEnd = .distantFuture`, so every span that begins later — even an independent, concurrent one — nests under any still-open span, collapsing the later tree into one deep chain. Confirm the intended containment semantics for open spans and pin them with a test, or nest more conservatively. (pr#107 review) - test(PeriscopeTools) [quick-win]: Pin whatever semantics that lands with a case covering two overlapping *open* spans; nothing covers it today, so either outcome regresses silently. (audit 2026-07-26) -- refactor(PeriscopeTools) [quick-win]: `ScopeEventsView`, `LogInspectorView`, `SpanTreeView`, and `SpanHistoryView` seed density from `.load(from: .standard)` directly, bypassing the injectable `defaults` the viewer threads through — so those surfaces can't be pointed at an ephemeral test suite and always touch the shared standard domain. It also means a drill-in *overrides* the density the viewer already seeded rather than inheriting it. Thread `defaults` through, or read the density from the environment. (pr#107 review; was nested under the `SpanTreeRow` density no-op, closed 2026-07-28) -- perf(PeriscopeTools) [needs-design]: The incremental *fetch* is bounded per commit, but `SpanTreeModel.load` / `LogHierarchyModel.load` still rebuild the whole tree/forest from all accumulated events on every `changes()` ping — O(total spans) per commit for a long-lived viewer over a busy store. Rebuild incrementally or throttle rebuilds. (pr#107 review) - - perf(PeriscopeTools) [needs-design]: `LogInspectorModel` didn't get even the bounded fetch — it re-runs its full subtree query on every `changes()` ping, so an open inspect sheet over a busy store re-reads everything per commit. Give it the same `afterSequence` cursor, or debounce it. (audit 2026-07-26) +- refactor(PeriscopeTools) [quick-win]: `ScopeEventsView` (`:38`), `LogInspectorView` (`LogInspectable.swift:89`), `SpanTreeView` (`:25`), and `SpanHistoryView` (`:32`) seed density from `.load(from: .standard)` directly, bypassing the injectable `defaults` the viewer threads through (`PeriscopeViewer.swift:58-63`, `:85-87`) — so those surfaces can't be pointed at an ephemeral test suite and always touch the shared standard domain. It also means a drill-in *overrides* the density the viewer already seeded rather than inheriting it. Thread `defaults` through, or read the density from the environment. All four re-confirmed 2026-08-02. (pr#107 review; was nested under the `SpanTreeRow` density no-op, closed 2026-07-28) +- perf(PeriscopeTools) [needs-design]: The incremental *fetch* is bounded per commit (`LogHierarchyModel.swift:70-72`, `SpanTreeModel.swift:142-147`), but `SpanTreeModel.load` / `LogHierarchyModel.load` still rebuild the whole tree/forest from all accumulated events on every `changes()` ping (`:75`, `:153`) — O(total spans) per commit for a long-lived viewer over a busy store. Rebuild incrementally or throttle rebuilds. (pr#107 review) + - perf(PeriscopeTools) [needs-design]: `LogInspectorModel` didn't get even the bounded fetch — it re-runs its full subtree query on every `changes()` ping (`:51-68`), so an open inspect sheet over a busy store re-reads everything per commit. Give it the same `afterSequence` cursor, or debounce it. (audit 2026-07-26) + - perf(PeriscopeTools) [needs-design]: `SpanHistoryModel` has the same shape — bounded fetch, then `rebuild()` over the whole accumulated `ends` set on every commit (`SpanHistoryModel.swift:125`, `:196-199`). It arrived with the span-history feature after this item was written, so it isn't a regression, but it should be fixed by whatever fixes the parent. (audit 2026-08-02) - refactor(PeriscopeCore) [needs-design]: Reconsider the `callAsFunction` scope-derivation API. `log(SomeLog.self)` / `log(for: id)` derivation reads as an opaque function call at declaration sites; a named form (`log.scope(SomeLog.self)` / `log.subcatalog(for: id)` / `log.child(_:)`) would read clearer. Constraint: the one-expression derive-and-emit (`log(PhotoLogs.self) { … }`) exists *because* `callAsFunction` lets Swift resolve the type arg + trailing closure as one application — a named method splits it, so the emit ergonomics need a paired design (a method that also takes the trailing closure) before renaming. Affects every derivation call site + all Periscope consumers. (pr#94 review) - feat(PeriscopeCore) [quick-win]: Add non-closure emit overloads alongside the `{}` form. Today emit is only `log { .event }` / `log(attachments:) { .event }`; the closure is nice for multi-line payload builds but heavy for a bare event. Add a value form — either `log.emit(.event)` (named, no overload ambiguity) or a `log(.event)` value overload — keeping `{}` for multi-line. Additive; pairs with the derivation-naming item above. (pr#94 review) - 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) [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. Inspector carries the same hosting-smoke debt — see [`Shared/Inspector/TODOs.md`](../Inspector/TODOs.md).) (pr#101 review) +- design(PeriscopeCore) [needs-design]: No eager store handle — `PeriscopeStore.make` being `async` forces an "observe until it lands" dance on consumers, so `RootView` still watches the store identity to wire the viewer/inspector/alerter (`WhereUI/Sources/RootView.swift:148-149`). A synchronous pending-store handle (usable immediately, resolves in the background) or an `await`-readiness accessor would remove the boilerplate every app repeats. **Correction 2026-08-02:** this item used to say Where exposes an `Optional` on `WhereModel` that stays `nil` until bootstrap completes. It doesn't any more — #150 replaced the bare optional with a typed `WhereModel.LogStoreState` (`opening`/`ready`/`unavailable`/`failed`, `WhereModel.swift:32-42`) and a `WhereScope.logRouting` enum (`WhereScope.swift:53-68`), so the *consumer* side is now honest state rather than a nil that can't say why. What remains is the underlying ergonomic ask: `make` is still `async`, so the state machine has to exist at all. (agent) +- test(PeriscopeTools) [needs-design]: broken-snapshots — replace the hosting smoke tests with image snapshots. **Twenty** tests across **ten** files assert nothing but "the hosted view reached a window" (recounted 2026-08-02 — it grew by two, `LogEventDetailViewHostingTests.swift:30`, `:43`): `#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. Inspector carries the same hosting-smoke debt — see [`Shared/Inspector/TODOs.md`](../Inspector/TODOs.md).) (pr#101 review) # Completed issues diff --git a/Shared/SnapshotKit/TODOs.md b/Shared/SnapshotKit/TODOs.md index c637137c..ab36ad3c 100644 --- a/Shared/SnapshotKit/TODOs.md +++ b/Shared/SnapshotKit/TODOs.md @@ -9,6 +9,6 @@ # Open issues ## P1s (Should do) -- fix: A case's content and captured models are instantiated once and shared across every configuration — `SnapshotCase.init` (`Sources/SnapshotCase.swift`) evaluates `content()` once into an `AnyView`, and the runner re-hosts that same value (and re-runs the same `onReadyToSnapshot` closure) for each of up to 10+ configurations (`SnapshotKitTesting`'s `AssertSnapshots.swift`). `@State` re-initializes per hosting, but reference-type models captured in the builder (`PreviewSupport.loadedYearReportModel()` and every provider like it) are shared: a `.task` side effect or a pre-capture hook mutation persists into all later configurations of the case — deterministic but surprising (variant N's reference bakes in variant 1's mutations), and nothing in the `SnapshotCase`/hook docs says content is built once per case rather than per configuration. Fix: store the content closure and rebuild per configuration (isolating state), or document the one-instance-per-case contract loudly on `SnapshotCase` and the hook. (From the July 2026 snapshot-testing PR review.) +- fix: A case's captured models are shared across every configuration, because the *runner* hoists the content out of the loop. `@State` re-initializes per hosting, but reference-type models captured in the builder (`PreviewSupport.loadedYearReportModel()` and every provider like it) are shared: a `.task` side effect or a pre-capture hook mutation persists into all later configurations of the case — deterministic but surprising (variant N's reference bakes in variant 1's mutations). **Half fixed** (2026-08-02): `SnapshotCase` is no longer the culprit — it now stores a `contentFactory` and rebuilds on each `content` access (`Sources/SnapshotCase.swift:47-69`), pinned by `SnapshotCaseTests.swift:51-65`. What remains is one call site: `SnapshotKitTesting`'s provider overload reads `snapshotCase.content` **once** and passes that single `AnyView` into the inline overload (`SnapshotKitTesting/Sources/AssertSnapshots.swift:44-45`), which re-hosts the same value for every configuration (`:104-105`). Fix: pass the factory (or re-read `content` per configuration) there. (From the July 2026 snapshot-testing PR review.) # Completed issues diff --git a/Shared/SnapshotKitTesting/AGENTS.md b/Shared/SnapshotKitTesting/AGENTS.md index 14f0e9aa..37f8530d 100644 --- a/Shared/SnapshotKitTesting/AGENTS.md +++ b/Shared/SnapshotKitTesting/AGENTS.md @@ -111,14 +111,17 @@ Complements the root [`AGENTS.md`](../../AGENTS.md) — read that first. image for views past ~2000pt on iOS 27.0; don't remove the tiling without re-running the probe. Guard: `SnapshotKitTestingTests.LargeViewCaptureTests`. -- **A settle phase costs its floor, not its passes.** Measured over all 260 - references with `SNAPSHOT_TIMING=1`: 192 captures sit at 0.25-0.35s, the - `minDuration` floor plus a pass or two, and the floor accounts for ~70s of - the ~84s of settle time. The render passes themselves are ~14s across the - whole suite. So making passes cheaper is worth ~11% and removing floors is - worth ~54% — but a floor can only come off with a **deterministic completion - seam** for that case (as `root.LoggedIn` does by awaiting `launcher.run()` - from `onReadyToSnapshot`), never by introspection. +- **A settle phase costs its floor, not its passes.** Measured 2026-07-28 over + the 260 references of the time with `SNAPSHOT_TIMING=1`: 192 captures sit at + 0.25-0.35s, the `minDuration` floor plus a pass or two, and the floor accounts + for ~70s of the ~84s of settle time. The render passes themselves are ~14s + across the whole suite. So making passes cheaper is worth ~11% and removing + floors is worth ~54% — but a floor can only come off with a **deterministic + completion seam** for that case (as `root.LoggedIn` does by awaiting + `launcher.run()` from `onReadyToSnapshot`), never by introspection. Tripwire: + the suite is 273 references now (Flyover and Inspector added their own + bundles), so re-measure before quoting the percentages — the ranking of floors + over passes is what's durable. ## Three things measured and rejected — don't re-derive them @@ -139,8 +142,8 @@ Complements the root [`AGENTS.md`](../../AGENTS.md) — read that first. - **Quiescence can't replace the pixel digest.** `SNAPSHOT_SETTLE` selects `pixel` (default), `quiescence` (a `beforeWaiting` run-loop observer plus a recursive `needsLayout`/`needsDisplay`/`animationKeys` walk), or `both`, which - runs them together and reports disagreements. Run in `both` mode over all 260 - references: 226 settle phases, 134 with some disagreement, and **8 where + runs them together and reports disagreements. Run in `both` mode over the 260 + references of 2026-07-28: 226 settle phases, 134 with some disagreement, and **8 where quiescence declared settled *earlier* than the digest** — every one a `Loaded_*` case whose content arrives late. That is the one dangerous direction (it would capture a frame no reference recorded), and it is what diff --git a/Shared/SnapshotKitTesting/TODOs.md b/Shared/SnapshotKitTesting/TODOs.md index bb7bfa39..6fde557a 100644 --- a/Shared/SnapshotKitTesting/TODOs.md +++ b/Shared/SnapshotKitTesting/TODOs.md @@ -17,8 +17,9 @@ - Tile seams: `LargeViewCaptureTests` probes at unit-y 0.1/0.9 of 800pt and 3000pt views, so a stitching error localized at the 2000pt seam (or a view exactly 2000pt tall — the single-tile/threshold edge) would pass. A probe pair straddling y = 2000 would pin it. ## P1s (Should do) -- perf [needs-design]: The settle floor is the single largest remaining cost in the suite, and only about a third of it looks addressable. Measured over all 260 references with `SNAPSHOT_TIMING=1`: 192 captures are floor-bound (0.25-0.35s), the `minDuration` floor accounts for ~70s of the ~84s of settle time — roughly 54% of total capture time — and every render pass in the suite combined is ~14s. So making passes cheaper is worth little and the floor is worth everything, but introspection cannot shorten a floor (see the rejected experiments in [`AGENTS.md`](AGENTS.md)): the only route is a deterministic completion signal awaited from `onReadyToSnapshot`, as `root.LoggedIn` already does with `await launcher.run()`. - **Addressable (~22s):** the 22 configurations on `.settledAtLeast` — `YearView` and `LocationsView` (10 configurations each at 1.0s) and `RootView` (2 at 1.5s). The `root.LoggedIn` seam is already specced in [`Where/TODOs.md`](../../Where/TODOs.md); the other two need an equivalent "the report finished loading" signal on `YearReportModel`. +- perf [needs-design]: The settle floor is the single largest remaining cost in the suite, and only about a third of it looks addressable. Measured over 260 references with `SNAPSHOT_TIMING=1`: 192 captures are floor-bound (0.25-0.35s), the `minDuration` floor accounts for ~70s of the ~84s of settle time — roughly 54% of total capture time — and every render pass in the suite combined is ~14s. So making passes cheaper is worth little and the floor is worth everything, but introspection cannot shorten a floor (see the rejected experiments in [`AGENTS.md`](AGENTS.md)): the only route is a deterministic completion signal awaited from `onReadyToSnapshot`, as `root.LoggedIn` already does with `await launcher.run()`. + **Addressable (~25s):** the **24** configurations on `.settledAtLeast` — `YearView` (`YearView.swift:145`) and `LocationsView` (`LocationsView.swift:234`) at 10 configurations each (`.screenDefaults` = 5 variants × 2 devices) at 1.0s, `RootView` (`RootView.swift:276`, 2 at 1.5s), and `FlyoverCanvas` (`Shared/Flyover/SnapshotTests/FlyoverSnapshotTests.swift:20`, 2 at 1.5s). The `root.LoggedIn` seam is already specced in [`Where/TODOs.md`](../../Where/TODOs.md); `YearView`/`LocationsView` need an equivalent "the report finished loading" signal on `YearReportModel`, and Flyover an equivalent for its content-load coordinator. + **The timing numbers are stale and should be re-measured before acting.** They were taken over 260 references; the tree now carries **273** (263 WhereUI, 4 Flyover, 4 Inspector, 2 PeriscopeTools), and the Xcode 27 beta 4 re-record (#161) landed in between. The *shape* of the conclusion — floors dominate, passes don't — is unlikely to have moved; the percentages are what to redo. **Probably not addressable (~48s):** the default 0.25s floor on the ~162 remaining screen captures. An earlier version of this item proposed dropping it for component-level cases that host no navigation or tab-bar chrome — **that group is empty.** Every `.component*` case already declares `settle: .immediate`, so the captures still paying the default floor are all screen-level, where the floor is waiting for exactly the iOS 26 glass toolbar/tab-bar material adaptation it was added for. Shortening it there needs either a per-case seam for each of ~40 cases or a signal for chrome adaptation that UIKit does not publish. Worth re-checking if one appears. (agent) ## P2s (Nice to have) diff --git a/TODOs.md b/TODOs.md index 00f6df67..9e5d9441 100644 --- a/TODOs.md +++ b/TODOs.md @@ -92,13 +92,15 @@ inbox rather than here. - feat: Update the deployment target to iOS 27 — this lets us use `HistoryObserver` for CloudKit/SwiftData instead of the notification. Spans every target's minimum OS (`Package.swift`, `Project.swift`), so it sits here rather than in `Where/TODOs.md`. (human) ## 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) +- 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:123`), 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). Re-verified 2026-08-02: **four shipped sites** the rule can't see — `WhereUI/Sources/Shared/DateRangeFormatting.swift:6`, `:19`, `:33` and `WhereUI/Sources/Model/PresenceTimeline.swift:37` — plus `in: .current` in the `#Preview`/snapshot fixtures of `DayRelabelView`, `AbruptChangeDetailView`, and `FlightDayDetailView`. No explicit `Calendar.current` spelling remains in Where production sources at all, so today the rule's match set is provably empty. (audit 2026-07-26) ## P1s (Should do) +- fix(Project) [quick-win]: `WhereTests` double-links `LifecycleKit` beside `WhereUI`, and doesn't use it. The bundle lists `.package(product: "LifecycleKit")` and `.package(product: "WhereUI")` together (`Project.swift:311-315`), which is exactly what the ["Never double-link a product WhereUI already carries"](AGENTS.md#never-double-link-a-product-whereui-already-carries) rule forbids — WhereUI already carries LifecycleKit (`Package.swift:160`). It predates the rule (the dep is unchanged since #81) and is invisible to the guard, because `WhereStylesheetTests.resolvesTraitAwareTokensFromTheBroadwayRoot` lives in `WhereUITests`, not here; `WhereTests` *is* in the `Stuff-iOS-Tests` scheme, which is where the split-metadata failure reproduces. Neither of its two source files imports LifecycleKit (`WhereTests.swift`, `AppAttributionTests.swift`), so the fix is to delete the line. The rule's prose scopes itself to the `unitTests` helper's `extraPackageProducts`, which is why a hand-rolled test target slipped past it — widen the wording to any target while fixing this. (audit 2026-08-02) + - refactor(Project) [quick-win]: Three host targets likewise declare package products they never import, all of which arrive through WhereUI: `RegionViewer` lists `RegionKit` + `WhereCore` while importing only `WhereUI` (`Project.swift:292-294`), and the `Where` app lists `RegionKit` without importing it (`:196-200`). Unlike the `WhereTests` item above these are app targets rather than test bundles, so the measured `.xctest` failure mode doesn't apply and this is dead declaration rather than a proven split — worth removing with the same pass, not on its own. (audit 2026-08-02) ## P2s (Nice to have) - refactor(Scripts) [needs-design]: The root dev scripts want an overhaul — there are ten now, and three of them duplicate the same xcodebuild plumbing. `./test`, [`profile`](profile), and [`flaky`](flaky) each resolve a destination, invoke `xcodebuild`, and parse an `.xcresult` with their own inline Python. Deliberately **not** consolidated when `./test` landed: the three genuinely want different things from a run (`profile` avoids formatters for timing fidelity and passes `-showBuildTimingSummary`; `flaky` needs `-test-iterations` and per-test re-runs; only `./test` wants a progress filter), so folding them into `./test` would bend a front door into a library, and extracting a shared helper introduces a sourced-library pattern none of the scripts use today. The duplication that actually caused harm was the *documented* invocation drifting between the docs, CI, and agent sessions, and `./test` now owns that; `profile` and `flaky` are report-only tools nobody copies commands from. Worth revisiting as part of a broader pass over the scripts rather than on its own, at which point the shared pieces are: destination resolution (already funnelled through [`simulator`](simulator)), the `xcresulttool get test-results tests` walk, and the `==>` / `error:` output conventions. (human 2026-07-28) -- refactor(Scripts) [needs-design]: Evaluate `tuist xcodebuild test-without-building` as a way to retire `./test`'s affected-bundle parser. `affected_bundles` ([`test`](test):189-352) is ~165 lines of Python that regex-parses `Project.swift` to work out which bundles a diff touches, and it is the most fragile thing in the script: it infers declaration boundaries from indent level (its own comment explains why the two obvious alternatives silently under-select, and `verify_parse` exits non-zero rather than degrade to "no bundle covers these changes"). It is also **local convenience only** — CI runs `--all` / `--snapshots`, so nothing in the pipeline depends on it. Tuist 4.200.5 ships `tuist xcodebuild test-without-building`, advertised as adding selective testing to an otherwise plain xcodebuild invocation, which is the only known way to get both that and the raw output `./test` needs. **Verify the output first:** if it pipes through xcbeautify like `tuist test` does, it is a non-starter for the two reasons in `./test`'s header comment, and the parser stays. Also confirm what it does with an empty hash cache on a fresh checkout, since that is the case CI is in. (agent 2026-07-28) +- refactor(Scripts) [needs-design]: Evaluate `tuist xcodebuild test-without-building` as a way to retire `./test`'s affected-bundle parser. `affected_bundles` ([`test`](test):192-354) is ~163 lines of Python that regex-parses `Project.swift` to work out which bundles a diff touches, and it is the most fragile thing in the script: it infers declaration boundaries from indent level (its own comment explains why the two obvious alternatives silently under-select, and `verify_parse` exits non-zero rather than degrade to "no bundle covers these changes"). It is also **local convenience only** — CI runs `--all` / `--snapshots`, so nothing in the pipeline depends on it. Tuist 4.200.5 ships `tuist xcodebuild test-without-building`, advertised as adding selective testing to an otherwise plain xcodebuild invocation, which is the only known way to get both that and the raw output `./test` needs. **Verify the output first:** if it pipes through xcbeautify like `tuist test` does, it is a non-starter for the two reasons in `./test`'s header comment, and the parser stays. Also confirm what it does with an empty hash cache on a fresh checkout, since that is the case CI is in. (agent 2026-07-28) - refactor [needs-design]: Vendor the local package through Tuist instead of Xcode's SPM integration, so package products become real Tuist targets. Today [`Project.swift`](Project.swift) uses `Package.local(path: .relativeToRoot("."))`, which emits an `XCLocalSwiftPackageReference` and hands the whole package to **Xcode's** SPM integration: every product links statically into each consumer, Tuist never sees the targets, and `PackageSettings` is inert. The alternative — the arrangement Tuist actually intends, and which other projects using it don't hit these duplication problems with — declares the local package as a dependency of a `Tuist/Package.swift` and consumes products with `.external(name:)`, so Tuist generates the targets and their product types and settings become ours to set. What it would buy: `PackageSettings` (per-product `.framework`/`.staticFramework`, per-target build settings), `Config(generationOptions: .options(enforceExplicitDependencies: true))` to catch the transitive-import looseness the test bundles lean on, resource bundles that stop being copied into every consumer (the full GeoJSON set is currently embedded per bundle), and retirement of the double-linking rule as a discipline. **Prototyped — blocked on a repo-layout prerequisite, not on the mechanism.** (spike 2026-07-26) - The blocker: Tuist cannot vendor a local package whose directory *is* the project directory. `tuist generate` dies with `Fatal error: Duplicate values for key: '/Users/kve/Development/Stuff4'`. Confirmed this is specifically the root collision rather than something else about this repo: pointing `Tuist/Package.swift` at a throwaway probe package elsewhere vendored fine and advanced to graph construction (failing only with `` `LifecycleKit` is not a valid configured external dependency ``, the correct next error). Projects that use this arrangement successfully avoid the collision purely by layout — the package at the repo root with the Tuist manifests in a subdirectory — where Stuff has both at the root. - Only one escape route exists. Moving the *package* into a subdirectory is not possible: SwiftPM rejects target paths outside the package root (`target 'Outside' in package 'pkg' is outside the package root`, verified with a minimal repro), and every target here points at `Where/…` / `Shared/…`. So the **Tuist manifests** would have to move into a subdirectory, rewriting every source glob in `Project.swift` plus `./ide`, `profile`, `.github/workflows/ci.yml`, and the docs. The vendored mode also adds a `tuist install` step before generate. @@ -108,7 +110,7 @@ inbox rather than here. - The blocker: a dynamic product that *statically absorbs* a resource-bearing dependency orphans that dependency's resources. `AccessibilitySnapshotParser`'s code moves into the framework while its `.bundle` stays in the `.xctest`, and SwiftPM's generated accessor searches only `Bundle.main.resourceURL`, `Bundle(for: BundleFinder.self).resourceURL`, `Bundle.main.bundleURL` — so `Bundle.module` hits its `fatalError` and **every VoiceOver-annotated capture traps** (`AccessibilitySnapshotBaseView.parseAccessibility()` → `StringLocalization.preferredBundle(for:)`). Verified fix: copy the resource bundles into `PackageFrameworks/SnapshotKitTesting.framework/`, after which the accessibility suites pass with no pixel drift. Automating it needs a build phase against a framework Xcode's SPM integration generates — the same `Bundle.module` placement fragility the StuffTestHost WhereCore embed was, which is the main argument against landing it. - Dead end: Tuist's `PackageSettings(productTypes:)` does nothing here. The local package is wired as an `XCLocalSwiftPackageReference` and resolved by Xcode's own SPM integration, so Tuist's product-type machinery never applies — only SwiftPM's `type:`. Relatedly, `type: .dynamic` takes effect only for a product an Xcode target consumes *as a product*: WhereUI depends on the SnapshotKit *target*, so SnapshotKit stayed static despite the annotation. - Trap: **a shared DerivedData reports false negatives here.** Two separate runs reported "no frameworks produced" from an incremental build that had not re-resolved the package graph. Spike this into a fresh `-derivedDataPath` or it will lie to you. -- refactor(StuffTestHost) [quick-win]: The scene configuration name is spelled twice, in `Shared/StuffTestHost/Sources/AppDelegate.swift:12` and `Project.swift:271`, so the two can drift silently. Reaches the root Tuist manifest, which is why it sits here rather than in a StuffTestHost file. (audit 2026-07-26) +- refactor(StuffTestHost) [quick-win]: The scene configuration name (`"Default Configuration"`) is spelled twice, in `Shared/StuffTestHost/Sources/AppDelegate.swift:12` and `Project.swift:332`, so the two can drift silently. Reaches the root Tuist manifest, which is why it sits here rather than in a StuffTestHost file. (audit 2026-07-26) # Completed issues diff --git a/Where/AGENTS.md b/Where/AGENTS.md index 97106ed7..b1c7ce59 100644 --- a/Where/AGENTS.md +++ b/Where/AGENTS.md @@ -21,10 +21,18 @@ 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. -The DEBUG app has a second boot runtime from -[`Shared/Inspector`](../Shared/Inspector). `AppDelegate` selects either the -regular composition root or the standalone Inspector before launch; Inspector -is not a `WhereScope` and must never construct regular app services. +WhereUI additionally builds on two app-agnostic developer modules, both DEBUG-only +surfaces rather than layers of the stack above: + +- [`Shared/Inspector`](../Shared/Inspector) is a second **boot runtime**. + `AppDelegate` selects either the regular composition root or the standalone + Inspector before launch; Inspector is not a `WhereScope` and must never + construct regular app services. +- [`Shared/Flyover`](../Shared/Flyover) is the screen **browser**, fed by + `WhereFlyoverWorld` (`WhereUI/Sources/Developer/Flyover/`). Flyover knows + nothing about Where — it renders a catalog — so the seeded demo world, and + the rule that it is built but never activated, belong to WhereUI (see + [Demo mode](#demo-mode)). ## Layering diff --git a/Where/TODOs.md b/Where/TODOs.md index d2bf0cbc..38fc950e 100644 --- a/Where/TODOs.md +++ b/Where/TODOs.md @@ -17,32 +17,33 @@ The item format and the placement rule live in the root - fix(WhereCore): Nothing gets recorded on a day with no movement — presumably because background updates ride on GPS. Any way to guarantee a daily boot outside of GPS? (human) ## 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) +- 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()` (`:65`) fans out to issue state and widgets only — so the daily notification body stays stale until a foreground re-`configure`. The GPS path doesn't close it either: `WhereServices`' `onPersisted` hook reaches `reminders.reconcileAfterIngest` and the widget publish, never `summary.reconcile()`. 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) - 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) -- refactor(WhereCore) [needs-design]: Scope diagnostic emission so Flyover's unactivated sibling demo world cannot write its activity through the process-global `WhereLog` / `Periscope.shared` facade into the active real scope's durable diagnostic store. `WhereFlyoverWorld.build()` correctly gives the sibling a private `Periscope` and never starts its sink, but static `WhereLog` channels still bypass that injection; carry the scope's logging system through services/models or add a task-/environment-scoped routing context before treating Flyover's diagnostic activity as isolated. Domain data, preferences, widgets, notifications, and location remain in memory/no-op already. (`WhereUI/Sources/Developer/Flyover/WhereFlyoverWorld.swift`, `WhereCore/Sources/Logging/WhereLog.swift`; agent 2026-07-29) -- 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)` (`: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: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) +- refactor(WhereCore) [needs-design]: Scope diagnostic emission so Flyover's unactivated sibling demo world cannot write its activity through the process-global `WhereLog` / `Periscope.shared` facade into the active real scope's durable diagnostic store. `WhereFlyoverWorld.build()` correctly gives the sibling a private `Periscope` and never starts its sink, but static `WhereLog` channels still bypass that injection; carry the scope's logging system through services/models or add a task-/environment-scoped routing context before treating Flyover's diagnostic activity as isolated. Domain data, preferences, widgets, notifications, and location remain in memory/no-op already. (`WhereUI/Sources/Developer/Flyover/WhereFlyoverWorld.swift`, `WhereCore/Sources/Logging/WhereLog.swift:31`; agent 2026-07-29) + - fix(WhereUI) [quick-win]: One developer surface reaches `Periscope.shared` directly rather than the injected system: `DeveloperToolView.swift:57` builds `OpenSpansView(system: .shared)`, while `WhereModel.logSystem` exists precisely so nothing resolves the global (it has no default, so a test can't silently attach sinks to `.shared`). Pass `model.logSystem` through. Smaller and independently fixable, unlike the `WhereLog` facade above, which needs the routing design. (audit 2026-08-02) +- 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. `YearReportModel` already owns an explicit Gregorian calendar (`YearReportModel.swift:205-206`) — it just never reaches these helpers. `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(_:)` (`:294`) commits atomically but skips `DayJournal.reconcileAfterDayChange()` — widgets/reminders/summary don't refresh until foreground/configure. (`RegionAttribution` does rebuild, since it keys off the commit's `changes()` ping; the schedulers are what stay stale.) 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)` (`:801-813`, carrying the in-source TODO) and `setPrimaryRegions` (`:865-869`) 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(_:)` (`:74`), the bulk ingest (`:85`), and `addManualSample` (`:99`) 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. `DemoDataBuilder.swift:84` documents the bulk ingest as reconciling "widget snapshot and issue scan", which the code doesn't do — fix the comment with the code. (audit 2026-07-26) +- fix(WhereCore) [needs-design]: A durable outbox save failure is logged and swallowed (`LocationOutbox.swift:78-90`, `LocationIngestor.swift:350-351`), 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: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) +- fix(WhereCore) [needs-design]: The retry queue evicts FIFO at capacity and drops samples with a warning only (`LocationIngestor.swift:355-360`). The eviction *behavior* is pinned (`LocationIngestorTests.retryQueueEvictsOldestSampleAtCapacity`), so what's open is purely the policy call: decide whether eviction warrants user-visible degradation, then document it. (audit 2026-07-26) +- fix(WhereUI) [quick-win]: `PresenceTimelineList` returns `[]` whenever `report.report` is nil (`:11-14`), 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]: Serialize `WhereSession.trackingEnabled` mutations. The setter spawns an unserialized `Task` per assignment (`WhereSession.swift:461-465`), so rapid on/off leaves start/stop racing: `startTracking()` sets `wantsTracking = true` on entry (`:353`) 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.) +- 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 467 lines (re-counted 2026-08-02 — it has not shrunk) mixing tracking intent, authorization observation, reset pass-through, region-style mirrors, the reminder/summary/issue-alert configuration fan-out, and the widget/foreground refreshes; 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:509-511` is a single day (`start == end` → `dateSpan = .singleDay`), so no test ever renders the `.range` branch (`:198-211`) — 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: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) +- fix(WhereUI) [needs-design]: Notification authorization is requested during launch, with no context and unprompted. The launch's detached fan (`WhereLaunch.swift:162-168` — `RemindersStep` / `SummaryStep` / `IssueAlertsStep` → `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. What makes it hit *every* fresh install rather than some: all three preferences default to `true` (`WhereCore/Sources/Preferences/WherePreferences.swift:42`, `:65`, `:89`, each documented as "active out of the box"), so every step's `configure(enabled:)` takes the branch that calls `requestAuthorization()` (`ReminderReconciler.swift:81`, `DailySummaryReconciler.swift:47`, `DataIssueAlertReconciler.swift:48`). 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. Fixing it this way keeps the defaults, which are a deliberate product call; flipping them to `false` would trade the prompt for a silently-off safety net. (agent; re-verified against the post-#150/#154 launch 2026-08-02) - 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) +- test(WhereIntents) [quick-win]: The per-intent `perform()` glue — guards, snippet wiring, error→dialog mapping — is untested, because `@Dependency` traps outside the perform flow. The "say so" half landed in `WhereIntents/AGENTS.md:76-84` ("Never call an intent's `perform()` in a test…"), but `WhereIntents/README.md:106-112` still describes the testing story as read/write logic with no such caveat, so a human reader isn't told. Either extract a thin testable seam or carry the caveat into `README.md` too; 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: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) @@ -54,13 +55,13 @@ The item format and the placement rule live in the root - refactor(WhereCore): Durable write-back is **read-repair**, decoupled from read correctness: opportunistically (batched, on `.NSPersistentStoreRemoteChange` + launch) rewrite stale records to the current version and stamp it, so old builds can honor exclusion. Transforms must be deterministic + commutative so two devices healing the same record via CloudKit converge (LWW-safe). (agent) - 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: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 VoiceOver-annotated calendar captures are blank. `CalendarContentViewSnapshotTests/calendarContent.WithData_iPhone_accessibility.png` and `..._iPad_accessibility.png` are solid white inside their border. Still true 2026-08-02, and the references have not been re-recorded — read off the LFS pointers, they are 67,951 B and 175,521 B against 1,381,994 B for `locations.Loaded_iPhone_accessibility` — 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-440`, `.frame(width: calendar.day.numberSize, height:)` — the stylesheet grows the row's `minHeight` at ax5 but not `numberSize`, `WhereStylesheet.swift:41-43`), 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-287`). 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:852`, `:853`) 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: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) [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:41`–`:43`, picker at `:91`–`:95`), 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:209`–`: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`–`:225`), which clips rather than growing. `SnapshotConfiguration.Frame.fullContent` exists for exactly this (see `CalendarContentView.swift:589`–`:595`) — 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) @@ -69,26 +70,27 @@ The item format and the placement rule live in the root - 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()` (`: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(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) +- fix(WhereCore) [quick-win]: Two Core failure paths report a benign-looking default instead of an honest one. `BackupService.readArchive` logs and skips an evidence asset whose bytes can't be read (`BackupService.swift:187-200`), so the import writes metadata-only evidence with `blob: nil` (`BackupCoordinator.swift:254-256`) — a partial import that reads as a complete one; and `ReminderReconciler` contributes `0` to the badge when the issue scan throws (`ReminderReconciler.swift:201-204`), 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()` (`:24`, `:44`) has `.task(id:)` plus a post-await `Task.isCancelled` guard, but the namer itself (`LocationNamer.swift:65-75`) keeps geocoding after the year changes; `RegionDaysView`'s `DayRow` (`:126-128`) 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-66`) rather than a catalog key with placeholders. (The `#Preview` half is **closed**: `:190` now reads `Button(String(localized: .snippetLogTodayHere))`.) (audit 2026-07-26) +- fix(WhereCore) [quick-win]: Surface the `applicationSupport()` → `NoOpLocationOutbox` fallback (`LocationOutbox.swift:49-60`); 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) +- fix(WhereCore) [quick-win]: A store opened without a resolvable URL silently loses live refresh in release. `SwiftDataStore.make` only wires `PersistentStoreRemoteChangeSource` when `container.configurations.first?.url` is non-nil, and the else branch is a bare `assertionFailure` (`SwiftDataStore.swift:222-229`) — so a debug build traps but a release build returns a store that never observes `.NSPersistentStoreRemoteChange`. That is the signal a share-extension add and a CloudKit sync both arrive on, so the failure mode is "another process's writes appear only on next launch", with nothing logged. Throw from `make` instead, or log a `WhereLog` error and expose the degraded state, rather than returning a store that reads as healthy. (audit 2026-08-02) - 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 — `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) +- docs(WhereCore) [quick-win]: Refresh stale doc claims. Re-verified and re-cited 2026-08-02: `WhereCore/README.md:49-50` and `:181-182` claim every write awaits the reconcile fan-out (ingest, bulk ingest, `addManualSample`, `addEvidence`, and `setPrimaryRegions` don't — see the P1s above), `:194-195` claims failures are never swallowed into benign defaults (three counterexamples are filed as P2s above), and `:13` says the widget extension talks to `WhereServices` when widgets only read the App Group `WidgetSnapshot`. `WhereCore/AGENTS.md:80-83` and `:104-107` make the same two claims in rule form, which is worse — a rule an agent will preserve against the code. `LocationIngestor.swift:340-343` still says it logs through `os.Logger` when it emits typed `WhereLog` events. `RegionViewer/README.md:17-18` describes "all US-state features in `us-states.geojson`" — that file no longer exists; RegionKit ships 57 per-region files under `Sources/Resources/regions/` and Source mode builds from them (`RegionGeometryCatalog.swift:122-148`). `RegionKit/README.md:159-160` claims GeoJSON decoding is covered, which the `GeoJSONTests` item below 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:)` (`:230`), `SwiftDataStore.make(storage:)` (`:191`), and `WidgetDataReader`'s aggregator/attributor (`:74-75`). The composition root already knows each value. `WherePreferences.init(store:)` is **done** — #150 made `store` required, so a test can no longer silently land on `UserDefaults.standard`. `WhereServices.make(...)` carries the same shape on `locationOutbox` / `now` / `activitySummaryGenerator` and belongs in the same pass. (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) -- 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) +- perf(WhereUI) [needs-design]: Profile the `RegionSummaryCard` Canvas rosette — `ringCount` derives from size with no cap (`:106-107`). 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` (`:45`) 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` (`:124`) and `Text("\(day.dayOfMonth)")` in `CalendarContentView` (`:436`) produce `%lld` and bypass `WhereFormat`'s number styling. Both value-less entries are still in `WhereUI/Sources/Resources/Localizable.xcstrings:4-8`. (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) -- refactor(WhereUI) [needs-design]: Remove `CalendarYearGrid`'s capture-time scroll skip. `scrollToCurrentMonth` guards on `\.isCapturingSnapshot` so a capture pins the deterministic top-of-year state instead of a nondeterministic scroll landing (`CalendarContentView.swift`). It's the one remaining product-code read of the snapshot flag that isn't a substituted stand-in — a smell (product behavior forking on "are we snapshotting"). Prefer making the scroll itself deterministic under capture (or driving the capture from a pre-scrolled fixture) so the guard can go. (From the July 2026 snapshot-testing PR review.) +- refactor(WhereUI) [needs-design]: Remove `CalendarYearGrid`'s capture-time scroll skip. `scrollToCurrentMonth` guards on `\.isCapturingSnapshot` so a capture pins the deterministic top-of-year state instead of a nondeterministic scroll landing (`CalendarContentView.swift:198`). It is the one read of the flag that changes *where the view is scrolled* — a smell (product behavior forking on "are we snapshotting"), and the one thing `SnapshotKit`'s documented carve-out doesn't sanction. Prefer making the scroll itself deterministic under capture (or driving the capture from a pre-scrolled fixture) so the guard can go. **Correction 2026-08-02:** the item used to call this "the one remaining product-code read that isn't a substituted stand-in", which isn't true — `LaunchSplashView` reads it twice more, at `:141` to skip the wall-clock caption timer and at `:229` to pin the radar phase. Both are sanctioned (a wall-clock timer and a motion end-state, the two carve-outs the flag exists for), which is the distinction the original wording lost. (From the July 2026 snapshot-testing PR review.) - refactor(WhereUI) [quick-win]: The widget fixtures' pinned instant is still `1_770_000_000` (02:40 UTC / Feb 1 evening Pacific, near a day boundary). Moving it safely off midnight was skipped to avoid re-recording the widget references; the main-merge re-record makes that cheap now. (From the July 2026 snapshot-testing PR review.) -- test(WhereUI) [needs-design]: Snapshot matrix gaps — `LocationsView`/`YearView`'s empty states, `RecentActivitySummaryView.loading` (the sole user of `AppIconActivityIndicator`, so its `@MotionIsStatic` pinning is the one motion adoption without direct capture coverage), and `ManualDayView`'s range-mode add have no snapshot case. (From the July 2026 snapshot-testing PR review.) +- test(WhereUI) [needs-design]: Snapshot matrix gaps — `RecentActivitySummaryView.loading` (the sole user of `AppIconActivityIndicator`, so its `@MotionIsStatic` pinning is the one motion adoption without direct capture coverage; the case list at `:150-172` covers Loaded/Empty/Unavailable/Failed only) and `ManualDayView`'s range-mode add have no snapshot case. The `LocationsView` / `YearView` empty states this item also asked for are **done** (`LocationsView.swift:238`, `YearView.swift:149`). (From the July 2026 snapshot-testing PR review.) ## Deferred snapshot-test flakiness Known nondeterminism in the WhereUI image suites, accepted for now — scattered diff --git a/Where/WhereUI/AGENTS.md b/Where/WhereUI/AGENTS.md index 6fb1049a..a14dfdc1 100644 --- a/Where/WhereUI/AGENTS.md +++ b/Where/WhereUI/AGENTS.md @@ -57,8 +57,14 @@ and testing conventions live in the feature [`Where/AGENTS.md`](../AGENTS.md) ([`Sources/Shared/WhereDatePicker.swift`](Sources/Shared/WhereDatePicker.swift)), 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. + days. Prefer that shape: put capture handling inside a shared component + rather than branching a screen on `\.isCapturingSnapshot`. Where a view does + read the flag, it may only freeze motion at a deterministic phase or + substitute an identically-laid-out stand-in, per SnapshotKit's carve-out — + `LaunchSplashView` does both (`:141` skips the wall-clock caption timer, + `:229` pins the radar phase). `CalendarContentView:198` is the one read that + goes further, changing *where the view is scrolled*; it is a known smell + filed in [`TODOs.md`](../TODOs.md), not a pattern to copy. ## Design system — `WhereStylesheet`