Detail-view + menu-bar performance: per-tab observation, snapshot caches, FocusedAction wrapper#329
Merged
Merged
Conversation
Each tab now has its own TerminalTabFeature, with agentPresence scoped to the leaf via `store.scope(state: \.agentPresence, ...)`. Detail body no longer reads `store.state.agentPresence` directly, so agent-storm ticks stop invalidating the whole body and republishing focused-scene values (the upstream cause of #289 menu drops on agent-active focus). Per-tab leaves: - TerminalTabAgentBadge reads `agentStore.state.agents(across:badgesEnabled:)` so agent tool storms invalidate the badge leaf only. - TerminalTabNotificationIndicator reads `terminalState.hasUnseenNotification` itself for the same reason. Stripe carries the work-in-progress signal: - Drop GhosttySurfaceProgressOverlay / GhosttySurfaceProgressBar entirely. The existing top-of-tab stripe paints the focused surface's state (focused tab) or a worst-of aggregate (unfocused). ERROR > PAUSE > determinate > indeterminate > none. Stripe extends across L/R separator borders so the active tab reads continuous. - `TerminalTabView.accessibilityValue` carries state-aware text (\"Busy\", \"Paused\", \"Errored\", \"47 percent complete\") so VoiceOver regains the OSC-9 signal the deleted overlay used to surface. Tab UI polish: - Hotkey hint typography matches the sidebar verbatim (caption + secondary) and now sits in the trailing close-button slot. Hover swaps hint → X. - TerminalTabDivider and TerminalTabBackground's bottom border use `@Environment(\.pixelLength)` + `.separator` style. - New TerminalTabAgentBadgeContent (View, Equatable) mirrors sidebar's RunningAgentsBadgeContent so the avatar group dedupes on agent equality. - Rename `WorktreeTerminalNotification.surfaceId` → `surfaceID`. Tests: - Two state-projection regression tests pin per-tab input stability: a notification on tab B leaves tab A's unseen count unchanged, and an agent record on tab B leaves tab A's `agents(across:badgesEnabled:)` unchanged.
Tab stripe: - Extract `TerminalTabProgressStripe` and render it as an `.overlay` on `TerminalTabView` AFTER `clipShape`. Previously the stripe lived inside `TerminalTabBackground` (applied via `.background`), so the corner-radius clip trapped its `-pixelLength` horizontal padding and left a 1px gray notch at each tab boundary. The overlay sits above the clip and paints across the adjacent dividers. - When the tab carries no custom tint and no progress state, the stripe falls back to `chromeAppearance.overlayTint` at a new `secondaryAccentOpacity` (0.45 dark / 0.35 light) instead of accent. Active plain tabs now read as a subtle chrome-tone indicator, not an accent-color flash. - `TerminalTabBackground` keeps only the bottom border for inactive tabs; the border now paints with `chromeAppearance.overlayTint.opacity(separatorOpacity)` (matching `TerminalTabDivider`) instead of `.separator`. Trailing slot: - Hover wins over ⌘-pressed hotkey hint. A new `isShowingHint` gate is passed to both the hint conditional and `TerminalTabCloseButton.isShowingShortcutHint` so hovering a tab with ⌘ held swaps the hint out for the close button instead of leaving the slot empty. Toolbar title chrome: - `WorktreeToolbarTitleView` now manages its own chrome-scheme tracking 1:1 with `WindowTintColorScheme` (same env + state + notification subscription, same `manager.surfaceBackgroundColorScheme()` source). The toolbar lives outside `windowTintColorScheme`'s wrapper, so the chrome env doesn't reach there; the title view tracks the dependencies itself and injects both `\.surfaceChromeAppearance` and `\.colorScheme`. Title text now stays readable across "Supacode Terminal Theme" toggles and system Light/Dark flips, including the light-system / dark-terminal combination that previously made the branch / repo name vanish. - Render path split into a `WorktreeToolbarTitleBody` struct subview so the outer view just owns the chrome plumbing.
Detail body now reads cached projections instead of `sidebarItems[id:]` and `terminalManager`, so per-leaf agent and notification churn no longer invalidates the detail tree. Both caches recompute via a single post-reduce hook driven by an exhaustive `CacheInvalidations` switch over every action, making a missing case a compile error instead of a silent skip. `runningScriptIDs` sources from the cached slice so the running-script duplicate check doesn't pull observation through `sidebarItems[id:]`. Drop unused `repositoryAccent` / `hasMergedBadge` from the slice and collapse the local cache-flag aliases onto `CacheInvalidations.all` for single-source consistency. Negative tests in `SelectedWorktreeSliceCacheTests` pin the load-bearing #289 contract: agent and projection storms on the focused row leave the slice and notification caches byte-equal.
Late tabProjectionChanged emits arriving after tabRemoved were re-creating phantom tab states. Added a bounded recent-removals deque on TerminalsFeature and added TestStore coverage for projection short-circuiting, removal idempotence, and the replay-after-remove guard.
WorktreeCommands now observes one Equatable WorktreeMenuSnapshot cached on AppFeature.State and refreshed in a post-reduce hook. Split the body into WorktreeMainMenu, WorktreeFileMenu and a static SelectWorktreeSubmenuItems so each Commands struct re-renders only when its own observed inputs flip. Select Worktree 1..10 are now always present; the reducer beeps for out-of-range slots, and Select Next/Previous beep at the bounds instead of disabling. Adds DEBUG `Self._printChanges()` + `SupaLogger(\"DetailRender\")` on the #289 invalidation chain (WorktreeDetailView / WorktreeToolbarTitleView / WorktreeToolbarTitleBody / ContentView and the menu-snapshot diff) so a future regression on this hot path is traceable from the console.
Closure-typed focused values invalidate the AppKit menu on every body run because closures have no Equatable conformance, so SwiftUI re-publishes each time. Wrap every menu-bar focused action in a generic FocusedAction<Input> that dedupes on (isEnabled, token) so AppKit only rebuilds when something the menu actually displays changes. ContentView, WorktreeDetailView, SidebarView and ArchivedWorktreesDetailView now publish via .focusedSceneAction / .focusedAction. Consumers in WorktreeCommands, TerminalCommands, WindowCommands, SidebarCommands read the action with @focusedvalue and gate via `action?.isEnabled != true`. ContentView additionally moves the heavy commandPaletteItems / WindowTitle reads into dedicated host subviews so per-row sidebar churn no longer invalidates ContentView's body. Alert / DeleteWorktreeTarget / ArchiveWorktreeTarget / DeleteDisposition gain Hashable so they can ride the token slot. AGENTS.md documents the pattern as the default.
closeAllSurfaces (called from prune) torn down the WorktreeTerminalState without firing onTabRemoved for each tab, leaking a phantom TerminalTabFeature.State in TerminalsFeature.State.terminalTabs for every tab in every pruned worktree. The same path also never replayed per-tab projections on a new event-stream subscriber, so tabs in re-attached worktrees stayed empty in the per-tab feature collection until the next mutation. Changes: - closeAllSurfaces emits onTabRemoved for each tab in lastTabProjections before draining the per-tab caches (C1). - prune emits a new TerminalClient.Event.worktreeStateTornDown after closeAllSurfaces. TerminalsFeature drains recentlyRemovedTabIDs and any orphan terminalTabs rows for that worktree so a same-session unarchive that restores persisted tab UUIDs starts clean (C2). - recentlyRemovedTabIDs FIFO is now keyed by (worktreeID, tabID) so the same UUID restored under a different worktree would not be shadowed. - eventStream replays each WorktreeTerminalState.currentTabProjections() and currentTabProgressDisplays() to the new continuation so a fresh subscriber rebuilds terminalTabs[id:] without waiting for mutations. Wires the previously dead currentTabProjections() helper (C3 + C7). - TerminalTabID is declared so its synthesized Hashable witness stays nonisolated under Swift 6 strict concurrency. TerminalTabFeature.State.id is typed TerminalTabID instead of UUID, dropping the .rawValue conversions at every scoping site and walling off mixing with unrelated UUIDs (C12). - WorktreeTerminalState.notifications is now private(set); a DEBUG-only setNotificationsForTesting(_:) helper fans emitAllTabProjections() so the per-tab projection contract stays load-bearing in production (C10). Tests cover worktreeStateTornDown draining the FIFO + orphan rows, and the snapshot-restore-after-teardown flow that previously would have been shadowed by the FIFO.
The post-reduce hook fired recomputeWorktreeMenuSnapshotIfChanged after every reducer action. The Equatable diff prevented SwiftUI invalidation but the recompute itself (URL flatMap on the slice's pullRequest, eight scalar reads, struct allocation, eight-field equality check) ran on the agent-storm hot path the rest of the branch is trying to keep cold. `AppFeature.Action.affectsWorktreeMenuSnapshot` is an exhaustive switch (no `default`) covering every top-level case and every TerminalClient.Event variant. Hot agent-storm shapes (.agentPresence, .terminals, .terminalEvent projection/progress/tab churn, .repositories actions with empty cacheInvalidations) opt out; .settings and .terminalEvent.notificationIndicatorChanged opt in; the rest is enumerated explicitly so a new action shape is a compile error rather than a silent stale snapshot.
`.worktreeInfoEvent` was mapped to `.all` but the arm is a pure effect-launcher: HEAD-watcher ticks paid for computeSidebarStructure + computeSelectedWorktreeSlice + computeToolbarNotificationGroups (full per-repo iteration) just for the Equatable diff to find a no-op (C11). Move it to the explicit `[]` group; the downstream arms it dispatches (`.worktreeBranchNameLoaded` / `.repositoryPullRequestsLoaded`) already declare their own invalidations. `.worktreeBranchNameLoaded` was mapped to `[.sidebarStructure, .selectedWorktreeSlice]` but the handler calls `updateWorktreeName(...)`, which mutates `worktree.name` which feeds `computeToolbarNotificationGroups()` (notification group title). A branch-name reload on a worktree with notifications would leave the toolbar group's name stale until an unrelated bulk action recomputed (C5). Promote to `.all`. Extract the three-step post-reduce body to `RepositoriesFeature.State.applyCacheRecomputes(_:)`. The production hook and the test mirror both call it so a future fourth cache lands in one place instead of needing two coordinated updates (C13).
`SelectedWorktreeSlice.sidebarDisplayName` and `SidebarItemFeature.State.sidebarDisplayName` were byte-for-byte copies of the same `isMainWorktree → id-contains-"/" → URL last-path → subtitle → branchName` cascade. The cache exists so the detail title can read off the slice; a future edge-case fix to one would silently leave the other stale. Extract the cascade as a `static` on a caseless `SidebarDisplayName` enum and have both call sites delegate. `WorktreeAccent.derive(isMainWorktree:isPinned:)` does the same for the parallel two-line cascade (C6). Drop the dead `RepositoriesFeature.State.selectedWorktreeSlice(for:)` helper. Repo-wide grep shows zero callers; the documented "ad-hoc by ID" use case has no consumer (C8).
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.
Summary
FocusedAction<Input>wrapper, caching a singleWorktreeMenuSnapshotfor the Commands body, and splitting the menu into independently-observed Commands structs.TerminalTabFeatureso per-tab churn invalidates only that leaf, cachingselectedSlice+toolbarNotificationGroupsin reducer state behind an exhaustiveCacheInvalidationsswitch, and dropping the floatingGhosttySurfaceProgressOverlayin favor of the per-tab top stripe.TerminalTabFeature.Stateon prune, the recently-removed-tab FIFO is worktree-scoped and drains on teardown,eventStreamreplays per-tab projections so re-attach reseeds the per-tab collection, andTerminalTabIDisnonisolated structso the per-tab state can be keyed by the nominal type instead of rawUUID.Test plan
make test— 1290 tests, 0 failures (11 pre-existing known issues)make build-app— clean release buildmake lint/make format— cleanworktreeStateTornDowndrain, the same-session snapshot-restore flow that previously would have been shadowed, the per-tab state-projection invariants, the cachedselectedSlice/toolbarNotificationGroupscontracts under agent storms.Notes
_printChanges()/SupaLogger("DetailRender")instrumentation is intentionally retained behind#if DEBUGso a future navigating the menu causes an unexpected removal of the options #289 regression is traceable from the console.