Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -645,9 +645,8 @@ external agent skills. Those skills are gitignored and absent from a bare checko

Four of those rows — SwiftFormat, ShellCheck, attribution, and the Python tool
tests — are four of the six checking steps in CI's `format` job, so a Linux agent
can pre-flight most of it. Three carve-outs, all filed together in
[`TODOs.md`](TODOs.md), all from the same cause — Linux ships no
`/usr/bin/ruby`, and the pinned one is reachable only through mise. One Python
can pre-flight most of it. Three portability carve-outs remain filed together in
[`TODOs.md`](TODOs.md). They have distinct causes. One Python
contract test pins macOS bash's `126` exit status for an unlaunchable command
and reports `127` here. The Ruby retained-tool tests run each command under a
hermetic `PATH=/usr/bin:/bin`, which resolves a `#!/usr/bin/env ruby` script
Expand Down
2 changes: 1 addition & 1 deletion Ledger/TODOs.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ The item format and the placement rule live in the root
## P2s (Nice to have)
- fix(LedgerCore) [quick-win]: `LedgerServices` resolves its calendar from the device (`LedgerServices.swift:157`, `calendar: Calendar = .current` on the `@_spi(Testing)` init) and uses it for the today/this-week spend deltas (`:303-308`), so on a non-Gregorian system calendar the window boundaries `SpendHistory` differences against move — the same defect class Where forbids outright, and the parameter default also violates the repo's "avoid parameter defaults on Core APIs" rule, since the composition root already knows the value. Inject an explicit Gregorian calendar with the current time zone from the app, and pass it in tests rather than relying on the default. Lower severity than Where's equivalent: this shifts a spend window rather than corrupting stored day identity, and no value is persisted against it. (audit 2026-08-09; re-verified 2026-08-30)
- test(LedgerCore) [quick-win]: Three implementation files have no namesake test — `LedgerLog.swift`, `LedgerSettings.swift`, and `SpendSnapshot.swift`. Each is exercised indirectly through `LedgerServicesTests`, so this is 1:1-convention debt rather than untested behavior; close it as those files change rather than in one pass. The rest of the module is genuinely well covered (14 test/support files over 16 sources, including the API, Keychain, token-source, and history seams). The September 7 recount confirms 16 source and 14 test/support files; the three namesake gaps remain unchanged. (audit 2026-08-09; re-verified 2026-09-06)
- test(Ledger) [needs-design]: The `Ledger` app target ships no test bundle, so the eight sources in the SwiftUI/AppKit shell — `MenuBarLabel`, `SpendView`, `SettingsView`, `LedgerSession`, `CurrencyFormat`, `WindowVisibilityReader` — are compile-only in CI (`Ledger-macOS-Tests` builds the app but runs only `LedgerCoreTests`, `Project.swift:712-717`). This matches how the Where extension targets are treated and is documented in [`Ledger/AGENTS.md`](Ledger/AGENTS.md), so it is a deliberate gap rather than an oversight; the decision worth making is whether `CurrencyFormat` and the menu-bar label's formatting deserve a hostless bundle of their own, since they are pure value transforms that a test could pin cheaply. (audit 2026-08-09; re-verified 2026-08-30)
- test(Ledger) [needs-design]: The `Ledger` app target ships no test bundle, so the eight sources in the SwiftUI/AppKit shell — `MenuBarLabel`, `SpendView`, `SettingsView`, `LedgerSession`, `CurrencyFormat`, `WindowVisibilityReader` — are compile-only in CI (`Ledger-macOS-Tests` builds the app but runs only `LedgerCoreTests`, `Project.swift:869-874`). This matches how the Where extension targets are treated and is documented in [`Ledger/AGENTS.md`](Ledger/AGENTS.md), so it is a deliberate gap rather than an oversight; the decision worth making is whether `CurrencyFormat` and the menu-bar label's formatting deserve a hostless bundle of their own, since they are pure value transforms that a test could pin cheaply. (audit 2026-08-09; re-verified 2026-08-30)

# Completed issues
236 changes: 129 additions & 107 deletions MODULE_AUDIT.md

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions Shared/Broadway/TODOs.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ 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:660-669` — 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:817-826` — 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 `./test BroadwayCatalogTests` stays green. (pr#149 review 2026-07-28)
- fix(BroadwayCatalog) [quick-win]: `BroadwayApp.swift:6-7` 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 (`Project.swift:748`, and again at `:771`), so CI runs it and it asserts nothing. Replace it with a launch smoke test. (audit 2026-07-26; re-verified 2026-09-06)
- test(BroadwayCatalog) [quick-win]: `Tests/BroadwayCatalogTests.swift:4` is an empty `struct BroadwayCatalogTests {}` wired into the `Stuff-iOS-Tests` scheme (`Project.swift:905`, and again at `:928`), so CI runs it and it asserts nothing. Replace it with a launch smoke test. (audit 2026-07-26; re-verified 2026-09-06)
- fix(BroadwayUI) [needs-design]: A nested `BRootViewController` registers duplicate trait observers (source `TODO` at `BRootViewController.swift:92-93`; the observer is still created unconditionally at `:95-103`). Latent today — Where reaches Broadway only through `whereBroadwayRoot()` / `BRootView`, neither of which nests — but it fires the moment something does. (audit 2026-07-26)

## P2s (Nice to have)
Expand Down
4 changes: 2 additions & 2 deletions Shared/Periscope/TODOs.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ here.
- 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 already models the lifecycle honestly as `WhereModel.LogStoreState` (`Where/WhereUI/Sources/Model/WhereModel.swift:32-39`: opening/ready/unavailable/failed); `logStore` is only a computed optional projection (`:114-120`). `RootView` still watches that projection's identity (`Where/WhereUI/Sources/RootView.swift:203`) to wire the viewer/inspector/alerter. The remaining design question is the async consumer handoff, not replacing a bare optional state machine. 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. **Twenty** tests across **ten** files assert nothing but "the hosted view reached a window" (filed as eighteen across nine; grew to twenty across ten when PR #152 added a file; **held at twenty across ten** on the 2026-08-16 and 2026-08-30 recounts, so the debt has now stopped growing for two consecutive windows without being worked down): `#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`, `LogEventDetailViewHostingTests.swift:30`, `:43`, 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 *still* the only file in it (re-checked 2026-08-30, 2 reference images), so none of the conversion has happened; the bundle and its `SnapshotKitTesting` link are wired at `Project.swift:627-633` — 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:20`) 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 has separate image-coverage gaps in [`Shared/Inspector/TODOs.md`](../Inspector/TODOs.md); that file does not claim this same hosting-smoke conversion debt.) (pr#101 review; re-verified 2026-09-06 — still 20 assertions across 10 files, third consecutive audit at that count)
- design(PeriscopeCore) [needs-design]: No eager store handle — `PeriscopeStore.make` being `async` forces an "optional store, observe until it lands" dance on consumers. Where already models the lifecycle honestly as `WhereModel.LogStoreState` (`Where/WhereUI/Sources/Model/WhereModel.swift:32-39`: opening/ready/unavailable/failed); `logStore` is only a computed optional projection (`:114-120`). `RootView` still watches that projection's identity (`Where/WhereUI/Sources/RootView.swift:212`) to wire the viewer/inspector/alerter. The remaining design question is the async consumer handoff, not replacing a bare optional state machine. 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. **Twenty** tests across **ten** files assert nothing but "the hosted view reached a window" (filed as eighteen across nine; grew to twenty across ten when PR #152 added a file; **held at twenty across ten** on the 2026-08-16 and 2026-08-30 recounts, so the debt has now stopped growing for two consecutive windows without being worked down): `#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`, `LogEventDetailViewHostingTests.swift:30`, `:43`, 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 *still* the only file in it (re-checked 2026-08-30, 2 reference images), so none of the conversion has happened; the bundle and its `SnapshotKitTesting` link are wired at `Project.swift:784-790` — 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:20`) 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 has separate image-coverage gaps in [`Shared/Inspector/TODOs.md`](../Inspector/TODOs.md); that file does not claim this same hosting-smoke conversion debt.) (pr#101 review; re-verified 2026-09-06 — still 20 assertions across 10 files, third consecutive audit at that count)

# Completed issues

Expand Down
2 changes: 1 addition & 1 deletion Shared/SnapshotKitTesting/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ Read the root [`AGENTS.md`](../../AGENTS.md) first.
- **Immediate measurement never shortens final capture settling.** It skips only the intrinsic-sizing probe's settle for synchronously sized fixtures.
- **The final `.settled` / `.settledAtLeast` policy still runs.** Guards: `AsyncContentCaptureTests`.
- **A settle phase costs its floor, not its passes.**
- **Measured 2026-07-28 with `SNAPSHOT_TIMING=1` over 260 references of the time.** The suite holds 490 as of 2026-09-06. Re-measure before acting on the split below.
- **Measured 2026-07-28 with `SNAPSHOT_TIMING=1` over 260 references of the time.** Current inventory lives in [`MODULE_AUDIT.md`](../../MODULE_AUDIT.md). Re-measure before acting on the split below.
- **The conclusion (the floor dominates) is what to rely on, not the seconds.**
- **192 captures sat at 0.25-0.35s — the `minDuration` floor plus a pass or two.** The floor accounts for ~70s of the ~84s of settle time.
- **The render passes themselves are ~14s across the whole suite.** Making passes cheaper is worth ~11%. Removing floors is worth ~54%.
Expand Down
Loading
Loading