Improve Periscope log viewer tooling (Broadway stylesheet, density, hierarchy) - #107
Merged
Conversation
Introduce PeriscopeStylesheet (Sources/Styling/), a Broadway BStylesheet holding row geometry, badge chrome, typography, and the severity/span-exit/ inspect color palette. Each public tool view (viewer, tracer, inspector sheet, open-spans) seeds periscopeBroadwayRoot() and reads tokens via @Environment(\.stylesheet); level/exit tints move off LogLevel/SpanExit.Mode into Palette. Adds a comfortable/compact row-density axis carried on \.logRowDensity (defaulting to comfortable — no visual change yet; the toggle lands in a follow-up). PeriscopeTools now links BroadwayCore/BroadwayUI. Adds PeriscopeStylesheetTests pinning defaults, palette mappings, trait-aware derivations, and a hosted cross-boundary environment test. Closes the Broadway-migration P1 in Shared/Periscope/TODOs.md.
The shared event row now honors a comfortable/compact density carried on \.logRowDensity. PeriscopeViewer seeds it from a UserDefaults-persisted preference (Density.load/save, defaulting to compact so the tooling reads dense out of the box), exposes a picker in its filter menu, and writes the choice back on change; the inspector sheet seeds the same preference. Injects the UserDefaults via a non-public init so the persistence is testable against an ephemeral suite. Adds density display-name and load/save round-trip tests.
PeriscopeViewer is now a two-tab surface: the existing flat Logs list plus a new Hierarchy tab (LogHierarchyView) that renders the store's LogScope tree — the hierarchy the Log<Event> API builds in code — as an expandable OutlineGroup with per-scope subtree event counts. Tapping a scope drills into its subtree's events (ScopeEventsView, reusing LogInspectorModel) with rows indented by depth to mirror the nesting. Adds a shared LogEventList component (used by the inspector and the new subtree view), a depth(of:below:) helper on LogInspectorModel, and scopeName/scopeCount typography plus a row indentStep token. Tests: LogHierarchyModelTests (forest building, counts, sorting, orphan roots, live refresh), LogHierarchyViewHostingTests, and an inspector depth test.
SpanTreeView pairs the store's SpanBegan/SpanEnded events by SpanID and nests them into a trace-style tree by time containment (a span inside another's lifetime becomes its child), showing durations, exit chips, and open spans; each row drills into the span's detail. Distinct from OpenSpansView (live, in-flight) — this reads the durable store, so it shows finished spans. Reachable from the viewer's Logs toolbar and usable standalone. Adds SpanTreeModelTests (pairing, containment nesting, open spans, live refresh), a hosting test, and shared spanBegan/spanEnded fixtures.
Live viewers that derive state from the whole store (the log-viewer hierarchy and span tree) re-read every matching event on every commit. Add a first-class incremental cursor so they can fetch only what was appended since they last looked. - LogQuery.afterSequence: only events whose store sequence is strictly greater than the cursor. Sequences are store-global and monotonic (every write takes the next value past the highest stored), so a fresh insert always outranks everything persisted -- "sequence > what I last merged" is exactly "everything appended since". - Wire it into the events(matching:) predicate alongside the other AND filters, and index SDLogEvent.sequence so the range query is a seek. - Document the monotonic-sequence invariant in PeriscopeCore/AGENTS.md.
LogHierarchyModel and SpanTreeModel loaded every matching event from the store on every changes() ping -- an unbounded full-store read purely to re-tally per-scope counts or rebuild the span tree, repeated on every commit (code review finding 1). Both now accumulate their derived state and, on each ping, fetch only the events past the highest sequence they've merged via LogQuery.afterSequence: - LogHierarchyModel keeps running per-scope directCounts and a watermark; buildForest takes precomputed counts (directCounts(in:) does the tally). - SpanTreeModel accumulates begin/end pairs and a watermark; an end arriving in a later commit pairs with an already-seen begin. - The merge re-filters on sequence so it stays idempotent if run() restarts over already-seen events (e.g. the view reappears). Per-commit work is now bounded by what the commit added rather than the whole store. This trades exact reflection of deletions (retention prune / clear -- neither wired into the live app) for that bound; a store swap rebuilds the model, resetting the watermark. Adds tests that counts accumulate without double-counting and that a span closes when its end lands in a later commit.
Files the remaining code-review findings for the log-viewer PR (finding 1, the full-store re-read, is already fixed) so none are dropped: - P1: TabView nested in the host NavigationStack (cross-tab push/toolbar bleed); hierarchy counts tally by primaryScope while the drill-in matches any linked scope in the subtree (count vs list can disagree). - P2: SpanEnded decode swallowed by try? (ended span can read as running); open-span containment nests every later span under an open one; SpanTreeView seeds an unread \.logRowDensity; secondary surfaces bypass the injectable defaults; per-commit in-memory rebuild is still O(total); model .failed states don't log. Also tightens the PeriscopeTools AGENTS note so it says the incremental *fetch* (not the rebuild) is what's bounded per commit.
Pins the incremental cursor's interaction with limit/offset (newest-first within the newer-than set) and that a cursor at/past the max sequence returns nothing.
Adds the missing coverage the review noted: - afterSequence-driven models: hierarchy counts survive a run() restart without double-counting; span pairs survive a restart without duplication; a scope with a missing parent is surfaced as a root live. - Span-tree edges: an end adjacent to the next begin stays a sibling; an end with no matching begin is ignored. - depth(of:below:): clamps to 0 for events at/above the viewed root and for scopeless events. - Density: unknown/corrupt stored value falls back to compact; a large but non-accessibility size keeps the default line limits. - New 1:1 test files for LogEventList and ScopeEventsView (both had none); a PeriscopeViewer hosting test that exercises the two-tab structure and the previously-uncalled test-only init(store:title:defaults:).
…viewer-tooling-35c4 # Conflicts: # Shared/Periscope/TODOs.md
Adds SpanHistoryView (+ SpanHistoryModel): the store's closed spans grouped by kind (SpanEnded.name), each row showing the recorded instance count and p50/p90/p95/p99 of their durations (nearest-rank, so every figure is a real observed sample; a kind whose instances never recorded a duration reports none). Tapping a kind drills into every closed span of that kind, newest first, over the shared LogEventList. Reachable from a new Span History toolbar button on the viewer's Logs tab.
kyleve
marked this pull request as ready for review
July 22, 2026 16:49
…viewer-tooling-35c4
…d out The Logs tab had four trailing bar items (filter, Span Tree, Span History, export); iOS silently drops the ones that don't fit, so Span History disappeared. Consolidate Span Tree + Span History into one Spans menu, pushed via navigationDestination, keeping the bar to three trailing items and both surfaces discoverable.
…chy control The two-tab TabView was nested inside the host NavigationStack, so its per-tab toolbar never reached the host navigation bar — inside the developer HUD (a fixed-frame panel) the entire Logs toolbar (filter, Spans menu, export) was missing. Every sibling developer tool is a single view pushed on the shared stack; make the viewer match by dropping the TabView for a nav-bar segmented Logs/Hierarchy picker. One stack now owns the bar and all drill-ins, which also resolves the cross-tab push/toolbar/searchable bleed TODO.
kyleve
pushed a commit
that referenced
this pull request
Jul 26, 2026
Weekly read-only audit refresh. Every open July 19 finding was re-verified against current source, and the week's new surface was reviewed: the Periscope migration (#94), Settings drill-in (#111), developer HUD (#115), navigation restructure (#119), log-viewer tooling (#107), String Catalog symbols (#124), the Gregorian-calendar pass (`fe99dde`), preview coverage (`52f0136`), Bumper Bowling (#127), and catalog serialization (#135). No source changed. ## Structural changes to the audit - **`LogKit` and `LogViewerUI` are gone** — Periscope replaced them, so their sections are removed and the inventory drops to **14 SPM library targets**. Counts refreshed to ~359 source / ~198 test files (WhereUI 84 → 113, WhereCore 70 → 87, PeriscopeTools 15 → 24, RegionKit 9 → 13). - New section for **Bumper Bowling**, the repo-owned architecture lint that landed this week. ## Two new high findings 1. **`where.gregorian_calendar` enforces nothing.** The rule filters `MemberAccessExprSyntax` on `base == "Calendar"`, so it matches a spelled-out `Calendar.current` but not the implicit-member form (`calendar: Calendar = .current`, `in: .current`) — which, after the Gregorian call-site pass, is the only form left in the tree. CI hard-gates `bumper lint` at `severity: .error` and is green, which confirms it finds nothing while seven production sites drift. `.bumper/RULES.md` also claims three calendar violations and some preview-coverage violations are "left visible during this bootstrap"; both claims are stale. 2. **`CalendarDay.displayDate` resolves through `Calendar.current`.** `startOfDay(in:)` interprets the day's Gregorian Y-M-D as *that* calendar's components, so on a Buddhist-era device every day label flowing through the helper — relabel, logged days, resolution details, the region drill-in — renders a date ~543 years off. `DateRangeFormatting.abbreviated` and `PresenceTimeline.stints` also default to `.current`. The three highs carried from July 19 are all still open: daily-summary staleness, the WhereUI tracking-toggle race, and the LifecycleKit terminal-phase race (now described precisely — `runSteps`' cancellation check sits at the *top* of the loop, so a cancel during the **final** step's `minVisible` hold is never observed before it returns `.completed`). ## Retired as verified fixed The Where app `README.md`; the cold-launch reason misclassification (`.undetermined` + `completedStepIDs`); `#Preview` coverage across WhereUI/WhereWidgets; the hand-maintained string-key facades; the in-app SwiftData browser; the widget post-midnight snapshot policy (now documented as intentional degradation); and `SharedItemLoader` warning logs. ## TODOs.md **`Where/TODOs.md`** — retires the `WhereModel` break-up (now 185 lines of process-lifetime state), the SwiftData browser, the `@_spi(Testing)` migration, and two items whose subject no longer exists (the `WhereModel` "controller" guard, hoisting `Calendar.current` onto the controller). Files this week's findings, pins the exact remaining `waitForOneRunloop` call sites, and promotes soft-deleting untracked regions now that the shipped picker reaches the hard delete. **`Shared/Periscope/TODOs.md`** — adds a P1 (the orphan sweep closing spans whose `survivesRelaunch` policy couldn't be decoded, silently) and three P2s (`LogInspectorModel` re-querying full subtrees per commit, `SpanHistoryModel` swallowing an `SpanEnded` decode failure, and a test pinning open-span containment). Sharpens the two density items with the user-visible consequence. **Root `AGENTS.md`** — its description of the audit cited the LogKit/LogViewerUI sections as an example of stale content surviving a refresh; this refresh removed them, so the caveat is rewritten to keep the useful half without an example that goes stale again. ## Verification Static analysis only — the Cloud agent runs Linux, so no `tuist test`, `bumper lint`, or simulator runs. CI status on `main` was read via `gh` (green) which is what lets the "the Gregorian rule finds nothing" conclusion stand. Changes are markdown-only, so `./swiftformat --lint` and `./xcstrings --lint` are unaffected.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Improves the PeriscopeTools on-device log viewer along the three axes requested for the log tooling:
PeriscopeStylesheet.comfortable/compactdensity so more events fit on screen.Log/LogScopehierarchy created in code), a dedicated span-tree view, and a span-history view (per-kind duration percentiles over closed spans).What's here
PeriscopeStylesheet(Sources/Styling/) holds row geometry, badge chrome, typography, and the severity/span-exit/inspect color palette. Every public tool view seedsperiscopeBroadwayRoot()and reads@Environment(\.stylesheet); level/exit tints moved offLogLevel/SpanExit.ModeintoPalette.comfortable/compactdensity on\.logRowDensity. The viewer seeds it from aUserDefaults-persisted preference (defaulting tocompact), exposes a picker in its filter menu, and persists on change; the inspector sheet seeds the same preference.LogHierarchyViewrenders the store'sLogScopetree as an expandable outline with per-scope subtree counts; drilling into a scope shows its subtree events with depth-indented rows (ScopeEventsView, reusingLogInspectorModel).SpanTreeViewpairs the store'sSpanBegan/SpanEndedevents bySpanIDand nests them by time containment (durations, exit chips, open spans); reachable from the Logs toolbar's Spans menu and usable standalone. Distinct fromOpenSpansView(live, in-flight).SpanHistoryViewgroups the store's closed spans by kind (SpanEnded.name), each row showing the recorded instance count and the p50/p90/p95/p99 of their durations; tapping a kind drills into every closed span of that kind, newest first.SpanHistoryModelrefreshes incrementally likeSpanTreeModel(accumulated ends +afterSequencewatermark) and computes nearest-rank percentiles, so every figure is a real observed sample (a kind whose instances never recorded a duration — all orphaned — counts them but reports no percentiles). Reachable from the Logs toolbar's Spans menu.LogEventListcomponent backs the inspector, the subtree view, and the span-history drill-in.The viewer is a single view pushed onto the host
NavigationStack(a nav-bar segmented control chooses Logs vs. Hierarchy), not a nestedTabView— aTabViewinside the stack didn't propagate its per-tab toolbar to the host bar (the whole Logs toolbar went missing inside the developer HUD) and shared one back-stack across tabs. One stack now owns the bar and every drill-in.Code-review follow-up — incremental live refresh (finding 1)
LogHierarchyViewandSpanTreeView's models previously re-read every matching event from the store on everychanges()ping — an unbounded full-store fetch, repeated on each commit. They now refresh incrementally:LogQuery.afterSequencecursor: only events whose storesequenceis strictly greater than the cursor. Sequences are store-global and monotonic, so "sequence > what I last merged" is exactly "everything appended since". Wired into theevents(matching:)predicate;SDLogEvent.sequenceis now indexed so the range query is a seek.LogHierarchyModelkeeps running per-scopedirectCounts+ a watermark;SpanTreeModelaccumulates begin/end pairs + a watermark (an end arriving in a later commit pairs with an already-seen begin). Both merges re-filter onsequenceto stay idempotent ifrun()restarts over already-seen events.SpanHistoryModelfollows the same accumulate-ends-past-the-watermark pattern.Remaining review items — filed, not dropped
The other findings from the review are recorded in
Shared/Periscope/TODOs.md(tagged "Log-viewer PR #107 review") rather than addressed here — notably: hierarchy counts tallying byprimaryScopewhile the drill-in matches any linked scope; a swallowedSpanEndeddecode; open-span containment nesting; an unread\.logRowDensityonSpanTreeView; secondary surfaces bypassing the injectabledefaults; and the O(total) in-memory rebuild. (The nested-TabViewitem is resolved by the segmented-control restructure above.)Testing
tuist test PeriscopeCoreTestsandPeriscopeToolsTestsgreen locally (iPhone 17, iOS 26.2);./swiftformat --lintclean. Coverage added to close the review's gaps:afterSequencecombined withlimit/offsetand the empty boundary; density unknown-value fallback and the non-accessibility large-size boundary; hierarchy/span idempotency across arun()restart; live missing-parent-as-root; span-tree adjacency-sibling and orphaned-end;depth(of:below:)at/above-root and scopeless clamps; new 1:1 test files forLogEventListandScopeEventsView; and aPeriscopeViewerhosting test exercising the segmented Logs/Hierarchy structure and the test-onlyinit(store:title:defaults:). Span history addsSpanHistoryModelTests(grouping/order, nearest-rank percentiles, orphaned/durationless handling, open-span exclusion, live + incremental accumulation, idempotent restart),SpanDurationPercentilesTests, and aSpanHistoryViewHostingTests. Also validated against the fullStuff-iOS-Testsscheme to confirmPeriscopeToolslinking Broadway doesn't regress the cross-bundle duplicate-copy guard.Design decisions confirmed with the requester: density persisted in
UserDefaults; Logs and Hierarchy as two surfaces of one stack-hosted viewer (nav-bar segmented control, not a nestedTabView); span tree as a dedicated view; incremental refresh fetches only events newer than the last one seen. Span history: "recorded instances" are closed spans (ends); kinds are ordered busiest-first; the drill-in lists theSpanEndedevents.