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
4 changes: 2 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ The executables in the repo root are the dev scripts. They are `ide`, `test`,
`swiftformat`, `sf-symbols`, `sync-agents`, `profile`, `icons`, `flaky`, `simulator`,
`worktree`, `xcstrings`, `attribution`, `shellcheck`, `codex-watchdog`, `tla-check`,
`circleci-artifacts`, `snapshot-shards`, `loc`. Each takes `--help`. Use one of these scripts instead of
hand-rolling its job. `./test` is the only way to run tests (see [Running
tests](#running-tests)). `./icons`, `./attribution`, and `./simulator` own state that is
hand-rolling its job. `./test` owns iOS tests; the native-macOS Ledger scheme is the exception
(see [Running tests](#running-tests)). `./icons`, `./attribution`, and `./simulator` own state that is
easy to corrupt by hand. `./simulator` owns a per-checkout device (see the
[`running-tests`](.agents/skills/running-tests/SKILL.md) skill).

Expand Down
2 changes: 1 addition & 1 deletion Ledger/TODOs.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ 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 (13 test files over 16 sources, including the API, Keychain, token-source, and history seams). Ledger has now shipped nothing for four consecutive windows, so all three counts are unchanged. (audit 2026-08-09; re-verified 2026-09-06)
- 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)

# Completed issues
478 changes: 194 additions & 284 deletions MODULE_AUDIT.md

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion Shared/JournalKit/TODOs.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ here.
# Open issues

## P2s (Nice to have)
- test [quick-win]: The concurrent-append test discards append errors with `try?` (`JournalTests.swift:186`), so it would pass with fewer entries than it asserts were written. Surface the error instead. (audit 2026-07-26; re-verified 2026-08-30)
- test [quick-win]: The concurrent-append test discards append errors with `try?` (`JournalTests.swift:186`). Its recovered-count, uniqueness, and per-writer-order assertions (`:194-201`) already fail when entries go missing; the old claim that missing records would pass was wrong. Surface append failures through a throwing task group so the test reports the original I/O error rather than only downstream count mismatches. (audit 2026-07-26; corrected 2026-09-07)
- test [quick-win]: `.full` sync durability is exercised by a single append (`JournalTests.swift:15`, inside `appendsRoundTripInOrder` — there is no dedicated `F_FULLFSYNC` regression); widen it to something that would actually catch a regression. (audit 2026-07-26; re-verified 2026-08-30)

# Completed issues
10 changes: 5 additions & 5 deletions Shared/Periscope/TODOs.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ here.

## P0s (Must do)
- design(PeriscopeCore) [needs-design]: Span record modeling — consider `enum { case span(Span), case event(Event) }` or a dedicated span record type instead of discriminating spans by downcast. Plan/build loop. **Correction (2026-08-09, re-confirmed 2026-08-30):** as filed, this said `spanID`/`spanExit` were "bolted onto every `LogRecord`". They aren't. Both are *computed* accessors that downcast the record's `event` (`LogSpan.swift:253-269`, which also carries `spanRelaunchPolicy`), so `LogRecord` stores exactly one span-related field — `bypassesFloors` (`LogRecord.swift:56`) — and the denormalized columns live on the persistence and journal shapes on purpose, so the sweep and queries read an indexed value instead of decoding payloads (`PeriscopeSchema.swift:50`, `:57`; `StoredLogEvent.swift:58`, `:61`; `LogJournalEntry.swift:94-98`). One detail to keep straight when reading those shapes: `StoredLogEvent` carries `spanID` and `spanExitMode` but **not** `spanRelaunchPolicy`, which lives only on `SDLogEvent` and the journal entry. That reframes the item: the case discrimination already exists as optional downcasts, so the real questions are whether to make it a typed enum and whether `bypassesFloors` belongs on the record at all — not whether to unpick stored span fields. (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]: `ScopeID` derivation — hash-derived vs a concatenated, human-readable path that preserves the input for debugging. Plan/build loop. (agent)
- design(PeriscopeCore) [needs-design]: `LogContextProviding` parent hierarchy — instance logs need a way to nest under a container's context (e.g. a controller inside another controller). Plan/build loop. (agent)
- design(PeriscopeCore) [needs-design]: 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 (`PeriscopeCore/Sources/Pipeline/Periscope.swift:149` owns the flat state; `PeriscopeCore/Sources/Store/PeriscopeStore.swift:30` owns persistence). Preserve ordering and durability guards while splitting responsibilities. (agent)
- design(PeriscopeCore) [needs-design]: `ScopeID` derivation — compare the existing SHA-256-derived UUID (`PeriscopeCore/Sources/Loggers/ScopeID.swift:22-30`) with a readable path. Scope IDs are persisted, so any alternative must preserve existing identities or specify migration; the readability request alone does not establish a correctness bug. (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). The protocol currently exposes only the system, and `instanceLog(for:)` derives a type-root/instance pair (`PeriscopeCore/Sources/Context/LogContextProviding.swift:25-31`, `:48-55`). Design a parent-context input that preserves stable instance identity and avoids cycles. (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:86-89`; the store-side half and its guard: `PeriscopeStore.swift:215-219`, `:242-245`, `PeriscopeStoreTests.swift:477-493`; the "without a matching begin" warning is `end(for:)` at `LogSpan.swift:643`, which this item previously mis-described as a relaunch-path warning). 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` (`PeriscopeStore.swift:118`), and the journal installs only when a store sink is added (`Periscope.swift:236-241`, install at `:293-297`), so events emitted between process launch and that call — 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)

Expand All @@ -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 exposes an `Optional<PeriscopeStore>` 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. **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 carries the same hosting-smoke debt — see [`Shared/Inspector/TODOs.md`](../Inspector/TODOs.md).) (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: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)

# Completed issues

Expand Down
Loading
Loading